-
Notifications
You must be signed in to change notification settings - Fork 318
[DocC Live Preview] Support parameter and return type disambiguations #2216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,29 @@ extension DocumentationLanguageService { | |
throw ResponseError.requestFailed(doccDocumentationError: .indexNotAvailable) | ||
} | ||
guard let symbolLink = DocCSymbolLink(linkString: symbolName), | ||
let symbolOccurrence = index.primaryDefinitionOrDeclarationOccurrence(ofDocCSymbolLink: symbolLink) | ||
let symbolOccurrence = try await index.primaryDefinitionOrDeclarationOccurrence( | ||
ofDocCSymbolLink: symbolLink, | ||
fetchSymbolGraph: { location in | ||
guard let symbolWorkspace = try await workspaceForDocument(uri: location.documentUri), | ||
let languageService = try await languageService(for: location.documentUri, .swift, in: symbolWorkspace) | ||
as? SwiftLanguageService | ||
else { | ||
throw ResponseError.internalError("Unable to find Swift language service for \(location.documentUri)") | ||
} | ||
return try await languageService.withSnapshotFromDiskOpenedInSourcekitd( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, we run multiple cursor info requests for a single docc request, right? Wit this implementation we need to open a new sourcekitd document for every request, which means that we also need to build as many ASTs, which is really the expensive part for the cursor info request. Would it be possible to only open each document once? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is correct. I can cache the snapshots instead of throwing them away, but that will require a rework of FWIW I don't expect there to be too many lookups, but it'll only get worse with the size of the project and the number of colliding symbol names... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m happy to take this change as-is as well and address it in a follow-up PR (or file an issue if you don’t think that you’ll get to it soon). |
||
uri: location.documentUri, | ||
fallbackSettingsAfterTimeout: false | ||
) { (snapshot, compileCommand) in | ||
let (_, _, symbolGraph) = try await languageService.cursorInfo( | ||
snapshot, | ||
compileCommand: compileCommand, | ||
Range(snapshot.position(of: location)), | ||
includeSymbolGraph: true | ||
) | ||
return symbolGraph | ||
} | ||
} | ||
) | ||
else { | ||
throw ResponseError.requestFailed(doccDocumentationError: .symbolNotFound(symbolName)) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be just a
filter
?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to use
asyncFilter(_:)
, but got the following error when I tried it:and it doesn't look like we can easily make
CheckedIndex
conform toSendable
.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like none of the functions in
Sequence+AsyncMap.swift
need@_inheritActorContext
and@Sendable
. Removing those also allows you to useasyncFilter
here. Not sure why I added those annotations. But we can also make that a follow-up change.