Implement textDocument/typeDefinition request by loveucifer · Pull Request #2445 · swiftlang/sourcekit-lsp

loveucifer · 2026-01-10T02:14:05Z

follow up pr referenced from - comment
to be merged after #2436

Add TypeDefinition.swift with cursorInfo USR lookup
Add cursorInfoFromTypeUSR and convertMangledTypeToUSR helpers
Register typeDefinitionProvider capability and handler
Add typeDefinition to LanguageService protocol
Add tests for local and cross-module type definitions

loveucifer · 2026-01-11T11:44:23Z

i might have missed something because i was going back and forth , lets see

ahoppen · 2026-01-11T21:56:20Z

I’ll start reviewing this after #2436 is merged and you can rebase this on top of main with all those changes in, so no need for you to keep this update until that time.

ahoppen

Very nice. I think the biggest thing will be to reconcile the resolving of the definition’s USR with indexBasedDefinition now.

ahoppen · 2026-01-17T09:10:59Z

+      return nil
+    }
+
+    // Try cursorInfo with USR lookup first - it knows the current in-memory state


There’s no second here, so I think the comment no longer applies

ahoppen · 2026-01-17T09:13:08Z

+      let location = typeInfo.symbolInfo.bestLocalDeclaration
+    {
+      return .locations([location])
+    }


Similar to my comments in the inlay type hints PR, this doesn’t handle jumping to generated interfaces. Instead of doing another ad-hoc fix like you did in the inlay hints PR, I’d really like to see this unified with the logic in indexBasedJumpToDefinition. Could you look into how we can best re-use code between both that function, inlay hints resolve and the type definition request?

ahoppen · 2026-01-17T09:15:09Z

+  func testTypeDefinitionClassType() async throws {
+    let testClient = try await TestSourceKitLSPClient()
+    let uri = DocumentURI(for: .swift)
+
+    let positions = testClient.openDocument(
+      """
+      class 1️⃣MyClass {}
+      let 2️⃣instance = MyClass()
+      """,
+      uri: uri
+    )
+
+    let response = try await testClient.send(
+      TypeDefinitionRequest(
+        textDocument: TextDocumentIdentifier(uri),
+        position: positions["2️⃣"]
+      )
+    )
+
+    guard case .locations(let locations) = response, let location = locations.first else {
+      XCTFail("Expected location response")
+      return
+    }
+
+    XCTAssertEqual(location.uri, uri)
+    XCTAssertEqual(location.range, Range(positions["1️⃣"]))
+  }
+
+  func testTypeDefinitionEnumType() async throws {


Classes and enums are just the same as a struct in the type system, so these two tests don’t really provide much value and I think we can delete them.

ahoppen · 2026-01-17T09:16:37Z

      completionProvider: completionOptions,
      signatureHelpProvider: signatureHelpOptions,
      definitionProvider: .bool(true),
+      typeDefinitionProvider: .bool(true),


Can you also advertise the typeDefinitionProvider here?

ahoppen · 2026-01-17T09:17:20Z

+  func typeDefinition(_ request: TypeDefinitionRequest) async throws -> LocationsOrLocationLinksResponse? {
+    throw ResponseError.requestNotImplemented(TypeDefinitionRequest.self)
+  }
+


Should we also forward the request to clangd in the ClangLanguageService? Not sure if clangd supports the request right now but if it does in the future, we’ll pick it up that way.

ahoppen · 2026-01-17T09:17:48Z

+    case let request as RequestAndReply<TypeDefinitionRequest>:
+      await self.handleRequest(for: request, requestHandler: self.typeDefinition)


This list is ordered alphabetically? Could insert this at t?

ahoppen · 2026-01-18T09:37:06Z

Instead of adding a new requirement to LanguageService like you did, which also doesn’t unify the implementation for the inlay type resolve, I was thinking to pull definitionLocations and definitionInInterface out to top-level functions (or maybe their own type if it makes sense, probably not but not sure) and then call it from indexBasedDefinition in SourceKitLSPServer, the type definition request in SwiftLanguageService and also the inlay hint resolve. I just checked and indexBasedDefinition mostly deals with logic to jump to definitions instead of declarations (only really applicable to the C family of languages), so I don’t think we need to generalize any of that except for potentially uniquing the results, which we can just copy to all three locations.

- Add DefinitionLocations.swift to CMakeLists.txt - Add proper documentation comments to functions in DefinitionLocations.swift - Make indexToLSPLocation private since it's only used within the file - Revert unrelated comment changes in InlayHintResolve.swift - Use .only instead of .first in InlayHintResolve to avoid ambiguous types - Refactor TypeDefinition.swift to use cleaner control flow with guard/else - Add test for jumping to generated interface (String)

…tLSPServer

ahoppen

Sorry for the delayed review.

I left a few comments inline but there are also quite a few comments from my previous review outstanding. Please address those and then I can take another look at this.

loveucifer · 2026-01-29T16:59:19Z

Regarding the isDynamic check - your comment said that "indexBasedDefinition mostly deals with logic to jump to definitions instead of declarations (only really applicable to the C family of languages)", so I left tout the isDynamic handling out of the shared definitionLocations function since:
Type definition and inlay hint resolve don't need dynamic symbol handling afaik they just need to find the type's location
Only indexBasedDefinition in SourceKitLSPServer needs the isDynamic logic for proper jump-to-definition behavior

What i thought was to do is this
keep the shared definitionLocations simple (for type definition + inlay hints)
keep indexBasedDefinition in SourceKitLSPServer with its own isDynamic handling
Is this the right way or should the shared function also include the isDynamic logic?

ahoppen · 2026-01-31T06:55:10Z

Oh, sorry, you‘re right about isDynamic. I think I was partially confused because the old definitionLocations still exists and I would have expected the isDynamic check to show up somewhere in the diff where it stuck around but it didn’t. That being said, could you still address all comments from my previous review? Eg. #2445 (comment) but all of them really.

loveucifer · 2026-01-31T07:20:43Z

okie got it

ahoppen

Nice, I only have a few small things left, otherwise this looks good to me. Looking forward to using this request 😄

ahoppen

Let’s 🚢 it

ahoppen · 2026-02-09T10:56:14Z

@swift-ci Please test

loveucifer · 2026-02-09T13:46:21Z

@swift-ci please test windows

loveucifer · 2026-02-10T03:54:20Z

@swift-ci please test

loveucifer · 2026-02-10T10:17:23Z

@swift-ci please test windows

ahoppen · 2026-02-10T10:56:37Z

@swift-ci Please test Windows

loveucifer · 2026-02-10T16:03:47Z

yaay 🎊

loveucifer requested review from ahoppen, hamishknight and rintaro as code owners January 10, 2026 02:14

loveucifer mentioned this pull request Jan 11, 2026

add go-to-definition for inlay hints (#2318) #2436

Merged

loveucifer added a commit to loveucifer/sourcekit-lsp that referenced this pull request Jan 12, 2026

Remove typeDefinition remnants - belongs in follow-up PR swiftlang#2445

a5e0ef3

loveucifer force-pushed the type-definition-request branch from 486f903 to df6ce2c Compare January 16, 2026 12:32

Implement textDocument/typeDefinition request

d8b8d71

loveucifer force-pushed the type-definition-request branch 3 times, most recently from b5ed7e9 to d8b8d71 Compare January 16, 2026 12:54

some improvements

94ad50d

loveucifer force-pushed the type-definition-request branch from c6ad839 to 94ad50d Compare January 16, 2026 13:00

ahoppen reviewed Jan 17, 2026

View reviewed changes

loveucifer added 3 commits January 17, 2026 16:17

address comments

2a360c0

unify typeDefinition

b7e76ff

fix type annotation

82a7726

unify definition locations

0e52fed

ahoppen reviewed Jan 18, 2026

View reviewed changes

loveucifer added 2 commits January 19, 2026 17:12

Deduplicate helper functions between DefinitionLocations and SourceKi…

5ee63b7

…tLSPServer

ahoppen reviewed Jan 29, 2026

View reviewed changes

Comment thread Sources/SourceKitLSP/DefinitionLocations.swift

Comment thread Sources/SourceKitLSP/DefinitionLocations.swift Outdated

Comment thread Sources/SourceKitLSP/DefinitionLocations.swift

loveucifer force-pushed the type-definition-request branch from e4bc7d7 to 5ee63b7 Compare January 29, 2026 16:50

Use any LanguageService instead of closures

cccdde2

Remove duplicate definitionLocations and call shared function

d73aba2

ahoppen reviewed Feb 8, 2026

View reviewed changes

Comment thread Sources/SourceKitLSP/SourceKitLSPServer.swift Outdated

Comment thread Sources/SwiftLanguageService/InlayHintResolve.swift Outdated

Comment thread Sources/SwiftLanguageService/TypeDefinition.swift Outdated

Comment thread Sources/SwiftLanguageService/TypeDefinition.swift Outdated

loveucifer added 2 commits February 8, 2026 23:20

Optimize index usage for typeDefinition and inlay hint resolve requests.

6028898

swift format

e786a14

ahoppen approved these changes Feb 9, 2026

View reviewed changes

ahoppen reviewed Feb 9, 2026

View reviewed changes

Comment thread Sources/SwiftLanguageService/TypeDefinition.swift

Add TypeDefinition.swift to CMakeLists.txt

f1f46f5

ahoppen merged commit 0ad0ac4 into swiftlang:main Feb 11, 2026
3 checks passed

		case let request as RequestAndReply<TypeDefinitionRequest>:
		await self.handleRequest(for: request, requestHandler: self.typeDefinition)

Conversation

loveucifer commented Jan 10, 2026

Uh oh!

loveucifer commented Jan 11, 2026

Uh oh!

ahoppen commented Jan 11, 2026

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Jan 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

loveucifer commented Jan 29, 2026

Uh oh!

ahoppen commented Jan 31, 2026

Uh oh!

loveucifer commented Jan 31, 2026

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Feb 9, 2026

Uh oh!

loveucifer commented Feb 9, 2026

Uh oh!

Uh oh!

loveucifer commented Feb 10, 2026

Uh oh!

loveucifer commented Feb 10, 2026

Uh oh!

ahoppen commented Feb 10, 2026

Uh oh!

loveucifer commented Feb 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects