-
Notifications
You must be signed in to change notification settings - Fork 373
add go-to-definition for inlay hints (#2318) #2436
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
Changes from 4 commits
8e300ee
f46a31b
27cf04e
564a45b
3630ab9
9e12af1
81610b3
22e7ac0
0695d3c
ceaf87b
0b02672
45b1598
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 |
|---|---|---|
|
|
@@ -234,6 +234,7 @@ package protocol LanguageService: AnyObject, Sendable { | |
| func definition(_ request: DefinitionRequest) async throws -> LocationsOrLocationLinksResponse? | ||
|
|
||
| func declaration(_ request: DeclarationRequest) async throws -> LocationsOrLocationLinksResponse? | ||
| func typeDefinition(_ request: TypeDefinitionRequest) async throws -> LocationsOrLocationLinksResponse? | ||
|
Member
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 think |
||
| func documentSymbolHighlight(_ req: DocumentHighlightRequest) async throws -> [DocumentHighlight]? | ||
| func foldingRange(_ req: FoldingRangeRequest) async throws -> [FoldingRange]? | ||
| func documentSymbol(_ req: DocumentSymbolRequest) async throws -> DocumentSymbolResponse? | ||
|
|
@@ -248,6 +249,7 @@ package protocol LanguageService: AnyObject, Sendable { | |
| func colorPresentation(_ req: ColorPresentationRequest) async throws -> [ColorPresentation] | ||
| func codeAction(_ req: CodeActionRequest) async throws -> CodeActionRequestResponse? | ||
| func inlayHint(_ req: InlayHintRequest) async throws -> [InlayHint] | ||
| func inlayHintResolve(_ req: InlayHintResolveRequest) async throws -> InlayHint | ||
| func codeLens(_ req: CodeLensRequest) async throws -> [CodeLens] | ||
| func documentDiagnostic(_ req: DocumentDiagnosticsRequest) async throws -> DocumentDiagnosticReport | ||
| func documentFormatting(_ req: DocumentFormattingRequest) async throws -> [TextEdit]? | ||
|
|
@@ -427,6 +429,10 @@ package extension LanguageService { | |
| throw ResponseError.requestNotImplemented(DeclarationRequest.self) | ||
| } | ||
|
|
||
| func typeDefinition(_ request: TypeDefinitionRequest) async throws -> LocationsOrLocationLinksResponse? { | ||
| throw ResponseError.requestNotImplemented(TypeDefinitionRequest.self) | ||
| } | ||
|
|
||
| func documentSymbolHighlight(_ req: DocumentHighlightRequest) async throws -> [DocumentHighlight]? { | ||
| throw ResponseError.requestNotImplemented(DocumentHighlightRequest.self) | ||
| } | ||
|
|
@@ -471,6 +477,11 @@ package extension LanguageService { | |
| throw ResponseError.requestNotImplemented(InlayHintRequest.self) | ||
| } | ||
|
|
||
| func inlayHintResolve(_ req: InlayHintResolveRequest) async throws -> InlayHint { | ||
| // default: return hint unchanged | ||
|
Member
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 don’t think this comment provides much value, so I’d just remove it. |
||
| return req.inlayHint | ||
| } | ||
|
|
||
| func codeLens(_ req: CodeLensRequest) async throws -> [CodeLens] { | ||
| throw ResponseError.requestNotImplemented(CodeLensRequest.self) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -790,6 +790,8 @@ extension SourceKitLSPServer: QueueBasedMessageHandler { | |
| await self.handleRequest(for: request, requestHandler: self.declaration) | ||
| case let request as RequestAndReply<DefinitionRequest>: | ||
| await self.handleRequest(for: request, requestHandler: self.definition) | ||
| case let request as RequestAndReply<TypeDefinitionRequest>: | ||
| await self.handleRequest(for: request, requestHandler: self.typeDefinition) | ||
| case let request as RequestAndReply<DoccDocumentationRequest>: | ||
| await self.handleRequest(for: request, requestHandler: self.doccDocumentation) | ||
| case let request as RequestAndReply<DocumentColorRequest>: | ||
|
|
@@ -832,6 +834,8 @@ extension SourceKitLSPServer: QueueBasedMessageHandler { | |
| initialized = true | ||
| case let request as RequestAndReply<InlayHintRequest>: | ||
| await self.handleRequest(for: request, requestHandler: self.inlayHint) | ||
| case let request as RequestAndReply<InlayHintResolveRequest>: | ||
| await request.reply { try await inlayHintResolve(request: request.params) } | ||
| case let request as RequestAndReply<IsIndexingRequest>: | ||
| await request.reply { try await self.isIndexing(request.params) } | ||
| case let request as RequestAndReply<OutputPathsRequest>: | ||
|
|
@@ -1098,7 +1102,7 @@ extension SourceKitLSPServer { | |
| let inlayHintOptions = | ||
| await registry.clientHasDynamicInlayHintRegistration | ||
| ? nil | ||
| : ValueOrBool.value(InlayHintOptions(resolveProvider: false)) | ||
| : ValueOrBool.value(InlayHintOptions(resolveProvider: true)) | ||
|
|
||
| let semanticTokensOptions = | ||
| await registry.clientHasDynamicSemanticTokensRegistration | ||
|
|
@@ -1144,6 +1148,7 @@ extension SourceKitLSPServer { | |
| completionProvider: completionOptions, | ||
| signatureHelpProvider: signatureHelpOptions, | ||
| definitionProvider: .bool(true), | ||
| typeDefinitionProvider: .bool(true), | ||
| implementationProvider: .bool(true), | ||
| referencesProvider: .bool(true), | ||
| documentHighlightProvider: .bool(true), | ||
|
|
@@ -1901,6 +1906,24 @@ extension SourceKitLSPServer { | |
| return try await languageService.inlayHint(req) | ||
| } | ||
|
|
||
| func inlayHintResolve( | ||
| request: InlayHintResolveRequest | ||
| ) async throws -> InlayHint { | ||
| // inlay hints store the uri in data for resolution | ||
| // extract uri from the lspany dictionary | ||
|
Member
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. Similar comment on the comment here. This exactly explains what’s being done below, so I don’t think it provides much value. In general, I’m a fan of comments that describe why something is done, not what is being done. |
||
| guard case .dictionary(let dict) = request.inlayHint.data, | ||
| case .string(let uriString) = dict["uri"], | ||
| let uri = try? DocumentURI(string: uriString) | ||
| else { | ||
| return request.inlayHint | ||
| } | ||
| guard let workspace = await self.workspaceForDocument(uri: uri) else { | ||
| return request.inlayHint | ||
| } | ||
| let language = try documentManager.latestSnapshot(uri.buildSettingsFile).language | ||
| return try await primaryLanguageService(for: uri, language, in: workspace).inlayHintResolve(request) | ||
| } | ||
|
|
||
| func documentDiagnostic( | ||
| _ req: DocumentDiagnosticsRequest, | ||
| workspace: Workspace, | ||
|
|
@@ -2150,6 +2173,14 @@ extension SourceKitLSPServer { | |
| return .locations(remappedLocations) | ||
| } | ||
|
|
||
| func typeDefinition( | ||
| _ req: TypeDefinitionRequest, | ||
| workspace: Workspace, | ||
| languageService: any LanguageService | ||
| ) async throws -> LocationsOrLocationLinksResponse? { | ||
| return try await languageService.typeDefinition(req) | ||
| } | ||
|
|
||
| /// Generate the generated interface for the given module, write it to disk and return the location to which to jump | ||
| /// to get to the definition of `symbolUSR`. | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -207,4 +207,48 @@ extension SwiftLanguageService { | |||||||||||||
| additionalParameters: appendAdditionalParameters | ||||||||||||||
| ) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// converts a mangled type string to a USR format. | ||||||||||||||
| /// mangled types start with `$s` while USRs start with `s:`. | ||||||||||||||
| /// for instance `$sSS` becomes `s:SS` (for String type). | ||||||||||||||
|
Member
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. Could we file an issue in github.com/swiftlang/swift that the cursor info request should return a proper USR here, so we can reference that issue and hopefully get rid of this hack when it’s no longer needed?
Suggested change
|
||||||||||||||
| private func convertMangledTypeToUSR(_ mangledType: String) -> String { | ||||||||||||||
| if mangledType.hasPrefix("$s") { | ||||||||||||||
| return "s:" + mangledType.dropFirst(2) | ||||||||||||||
| } | ||||||||||||||
| // already in USR format or unknown format, return as-is | ||||||||||||||
| return mangledType | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /// get cursor info for a type by looking up its USR. | ||||||||||||||
| /// this takes a mangled type (from `key.typeusr`) and converts it to a proper USR | ||||||||||||||
| /// (by replacing `$s` prefix with `s:`), then queries cursorInfo with that USR. | ||||||||||||||
| /// | ||||||||||||||
| /// - parameters: | ||||||||||||||
| /// - mangledType: the mangled type string (e.g., `$sSS` for String) | ||||||||||||||
| /// - uri: document URI for context (used to get compile command) | ||||||||||||||
| /// - returns: cursorInfo for the type declaration, or nil if not found | ||||||||||||||
|
loveucifer marked this conversation as resolved.
Outdated
|
||||||||||||||
| func cursorInfoFromTypeUSR( | ||||||||||||||
| _ mangledType: String, | ||||||||||||||
| in uri: DocumentURI | ||||||||||||||
|
Member
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 think this should take a In the inlay type resolve request, we should include the document version in the |
||||||||||||||
| ) async throws -> CursorInfo? { | ||||||||||||||
| let usr = convertMangledTypeToUSR(mangledType) | ||||||||||||||
|
|
||||||||||||||
| let snapshot = try await self.latestSnapshot(for: uri) | ||||||||||||||
| let compileCommand = await self.compileCommand(for: uri, fallbackAfterTimeout: true) | ||||||||||||||
| let documentManager = try self.documentManager | ||||||||||||||
|
|
||||||||||||||
| let keys = self.keys | ||||||||||||||
|
|
||||||||||||||
| let skreq = sourcekitd.dictionary([ | ||||||||||||||
| keys.cancelOnSubsequentRequest: 0, | ||||||||||||||
| keys.usr: usr, | ||||||||||||||
| keys.sourceFile: snapshot.uri.sourcekitdSourceFile, | ||||||||||||||
| keys.primaryFile: snapshot.uri.primaryFile?.pseudoPath, | ||||||||||||||
| keys.compilerArgs: compileCommand?.compilerArgs as [any SKDRequestValue]?, | ||||||||||||||
| ]) | ||||||||||||||
|
|
||||||||||||||
| let dict = try await send(sourcekitdRequest: \.cursorInfo, skreq, snapshot: snapshot) | ||||||||||||||
|
|
||||||||||||||
| return CursorInfo(dict, snapshot: snapshot, documentManager: documentManager, sourcekitd: sourcekitd) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
Member
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. Could you add this file to CMakeLists.txt to make the tests pass on Windows? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import Foundation | ||
| import IndexStoreDB | ||
| @_spi(SourceKitLSP) package import LanguageServerProtocol | ||
| import SemanticIndex | ||
| import SourceKitD | ||
| import SourceKitLSP | ||
|
|
||
| extension SwiftLanguageService { | ||
| /// resolves an inlay hint by looking up the type definition location | ||
| package func inlayHintResolve(_ req: InlayHintResolveRequest) async throws -> InlayHint { | ||
| var hint = req.inlayHint | ||
|
|
||
| // only resolve type hints that have stored data | ||
| // extract uri and position from the lspany dictionary | ||
| guard hint.kind == .type, | ||
| case .dictionary(let dict) = hint.data, | ||
| case .string(let uriString) = dict["uri"], | ||
| let uri = try? DocumentURI(string: uriString), | ||
| case .dictionary(let posDict) = dict["position"], | ||
| case .int(let line) = posDict["line"], | ||
| case .int(let character) = posDict["character"] | ||
| else { | ||
| return hint | ||
| } | ||
| let position = Position(line: line, utf16index: character) | ||
|
|
||
| // get the type usr by calling cursor info at the variable position | ||
| let typeLocation = try await lookupTypeDefinitionLocation( | ||
| uri: uri, | ||
| position: position | ||
| ) | ||
|
|
||
| guard let typeLocation else { | ||
| return hint | ||
| } | ||
|
|
||
| // return new hint with label parts that have location for go-to-definition | ||
| if case .string(let labelText) = hint.label { | ||
| return InlayHint( | ||
| position: hint.position, | ||
| label: .parts([InlayHintLabelPart(value: labelText, location: typeLocation)]), | ||
| kind: hint.kind, | ||
| textEdits: hint.textEdits, | ||
| tooltip: hint.tooltip, | ||
| paddingLeft: hint.paddingLeft, | ||
| paddingRight: hint.paddingRight, | ||
| data: hint.data | ||
| ) | ||
| } | ||
|
|
||
| return hint | ||
| } | ||
|
|
||
| /// Looks up the definition location for the type at the given position. | ||
| /// | ||
| /// This is used by both inlay hint resolution and the typeDefinition request. | ||
| /// It works by: | ||
| /// 1. Getting the type USR (mangled name) from cursorInfo at the position | ||
| /// 2. Converting the mangled type ($s prefix) to a proper USR (s: prefix) | ||
| /// 3. Looking up the type definition in the index or via cursorInfo | ||
| func lookupTypeDefinitionLocation( | ||
| uri: DocumentURI, | ||
| position: Position | ||
| ) async throws -> Location? { | ||
| // Step 1: Get type USR from cursor info at the position | ||
| let snapshot = try await self.latestSnapshot(for: uri) | ||
| let compileCommand = await self.compileCommand(for: uri, fallbackAfterTimeout: false) | ||
|
|
||
| let skreq = sourcekitd.dictionary([ | ||
| keys.cancelOnSubsequentRequest: 0, | ||
| keys.offset: snapshot.utf8Offset(of: position), | ||
| keys.sourceFile: snapshot.uri.sourcekitdSourceFile, | ||
| keys.primaryFile: snapshot.uri.primaryFile?.pseudoPath, | ||
| keys.compilerArgs: compileCommand?.compilerArgs as [any SKDRequestValue]?, | ||
| ]) | ||
|
|
||
| let dict = try await send(sourcekitdRequest: \.cursorInfo, skreq, snapshot: snapshot) | ||
|
|
||
| // Get the type USR (this is of a mangled type like "$sSS" for String) | ||
| guard let typeUsr: String = dict[keys.typeUsr] else { | ||
| return nil | ||
| } | ||
|
|
||
| // step 2: Convert mangled type to proper USR | ||
| // The typeUsr is a mangled type like "$s4test6MyTypeVD" for struct MyType | ||
| // To get the declaration USR, we need to: | ||
| // 1. Replace "$s" prefix with "s:" | ||
| // 2. Strip the trailing "D" which is a mangling suffix (type descriptor) | ||
| var mangledName = typeUsr | ||
| if mangledName.hasPrefix("$s") { | ||
| mangledName = "s:" + mangledName.dropFirst(2) | ||
| } | ||
|
Member
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. Isn’t this duplicated with |
||
| // Strip trailing 'D' (type descriptor suffix in mangling) | ||
| if mangledName.hasSuffix("D") { | ||
| mangledName = String(mangledName.dropLast()) | ||
| } | ||
|
Member
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. Uh, this seems iffy to me. I really would prefer if we didn’t start interpreting mangled names, I just feel like there lie dragons that way (I’m somewhat fine with the Would you be able to handle USRs with a
Contributor
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. "D" in this case just means "type mangling", dropping it isn't guaranteed to give you the equivalent decl USR though, e.g you could have generic arguments present. The USR cursor info request should give back the right decl USR to use, I think it would make more sense to use that (though I don't know if an index lookup is still necessary in that case?) |
||
| let usr = mangledName | ||
|
|
||
| // step 3: Try index lookup first (works well for local and external types) | ||
| if let workspace = await sourceKitLSPServer?.workspaceForDocument(uri: uri), | ||
|
loveucifer marked this conversation as resolved.
Outdated
|
||
| let index = await workspace.index(checkedFor: .deletedFiles), | ||
| let occurrence = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) | ||
| { | ||
| let definitionUri = DocumentURI(filePath: occurrence.location.path, isDirectory: false) | ||
| let definitionPosition = Position( | ||
| line: occurrence.location.line - 1, | ||
| utf16index: occurrence.location.utf8Column - 1 | ||
| ) | ||
|
Member
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. Interpreting UTF-8 as UTF-16 columns isn’t correct if there are eg. emojis in the line. There are functions on |
||
| return Location(uri: definitionUri, range: Range(definitionPosition)) | ||
| } | ||
|
|
||
| // Fallback: Try cursorInfo with USR (for types not in index) | ||
|
loveucifer marked this conversation as resolved.
Outdated
|
||
| if let typeInfo = try await cursorInfoFromTypeUSR(typeUsr, in: uri), | ||
| let location = typeInfo.symbolInfo.bestLocalDeclaration | ||
| { | ||
| return location | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| } | ||
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 don’t think this comment is providing any value. Let’s just remove it.