Address PR review feedback

- 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)

Sources/SourceKitLSP/CMakeLists.txt

@@ -4,6 +4,7 @@ add_library(SourceKitLSP STATIC
   DocumentManager.swift
   DocumentSnapshot+FromFileContents.swift
   DocumentSnapshot+PositionConversions.swift
+  DefinitionLocations.swift
   GeneratedInterfaceDocumentURLData.swift
   Hooks.swift
   IndexProgressManager.swift

Sources/SourceKitLSP/DefinitionLocations.swift

@@ -15,21 +15,28 @@ import IndexStoreDB
 @_spi(SourceKitLSP) import SKLogging
 package import SemanticIndex
 
-/// converts a symbol index location to an LSP location
-func indexToLSPLocation(_ location: SymbolLocation) -> Location? {
+/// Converts a location from the symbol index to an LSP location.
+///
+/// - Parameter location: The symbol index location
+/// - Returns: The LSP location
+private func indexToLSPLocation(_ location: SymbolLocation) -> Location? {
   guard !location.path.isEmpty else { return nil }
   return Location(
     uri: location.documentUri,
     range: Range(
       Position(
+        // 1-based -> 0-based
+        // Note that we still use max(0, ...) as a fallback if the location is zero.
         line: max(0, location.line - 1),
+        // Technically we would need to convert the UTF-8 column to a UTF-16 column. This would require reading the
+        // file. In practice they almost always coincide, so we accept the incorrectness here to avoid the file read.
         utf16index: max(0, location.utf8Column - 1)
       )
     )
   )
 }
 
-/// returns the definition location for a symbol, handling generated interfaces for SDK types
+/// Return the locations for jump to definition from the given `SymbolDetails`.
 package func definitionLocations(
   for symbol: SymbolDetails,
   originatorUri: DocumentURI,
@@ -112,7 +119,11 @@ package func definitionLocations(
   return occurrences.compactMap { indexToLSPLocation($0.location) }.sorted()
 }
 
-/// generates a swift interface and returns location for the given symbol
+/// 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`.
+///
+/// `originatorUri` is the URI of the file from which the definition request is performed. It is used to determine the
+/// compiler arguments to generate the generated interface.
 package func definitionInInterface(
   moduleName: String,
   groupName: String?,

Sources/SwiftLanguageService/InlayHintResolve.swift

@@ -16,6 +16,7 @@ import IndexStoreDB
 import SemanticIndex
 import SourceKitD
 import SourceKitLSP
+import SwiftExtensions
 
 extension SwiftLanguageService {
   /// Resolves an inlay hint by looking up the type definition location.
@@ -59,7 +60,10 @@ extension SwiftLanguageService {
     return hint
   }
 
-  /// looks up the definition location for the type at the given position
+  /// Looks up the definition location for the type at the given position.
+  ///
+  /// This is used by inlay hint resolution to enable go-to-definition on type hints.
+  /// For SDK types, this returns a location in the generated interface.
   func lookupTypeDefinitionLocation(
     snapshot: DocumentSnapshot,
     position: Position
@@ -98,6 +102,6 @@ extension SwiftLanguageService {
       }
     )
 
-    return locations.first
+    return locations.only
   }
 }

Sources/SwiftLanguageService/TypeDefinition.swift

@@ -17,7 +17,7 @@ import SourceKitD
 import SourceKitLSP
 
 extension SwiftLanguageService {
-  /// handles the textDocument/typeDefinition request
+  /// Handles the textDocument/typeDefinition request.
   package func typeDefinition(_ request: TypeDefinitionRequest) async throws -> LocationsOrLocationLinksResponse? {
     let uri = request.textDocument.uri
     let position = request.position
@@ -36,28 +36,35 @@ extension SwiftLanguageService {
     let dict = try await send(sourcekitdRequest: \.cursorInfo, skreq, snapshot: snapshot)
     let documentManager = try self.documentManager
 
-    // if cursor is on a type symbol itself, use its USR directly
-    var symbol: SymbolDetails?
+    // Helper to get symbol from typeUsr
+    func symbolFromTypeUsr() async throws -> SymbolDetails? {
+      guard let typeUsr: String = dict[keys.typeUsr] else {
+        return nil
+      }
+      guard let typeInfo = try await cursorInfoFromTypeUSR(typeUsr, in: snapshot) else {
+        return nil
+      }
+      return typeInfo.symbolInfo
+    }
+
+    // If the cursor is on a type symbol itself, use its USR directly.
+    // Otherwise get the type of the symbol at this position.
+    let symbol: SymbolDetails
     if let cursorInfo = CursorInfo(dict, snapshot: snapshot, documentManager: documentManager, sourcekitd: sourcekitd) {
       switch cursorInfo.symbolInfo.kind {
       case .class, .struct, .enum, .interface, .typeParameter:
         symbol = cursorInfo.symbolInfo
       default:
-        break
+        guard let typeSymbol = try await symbolFromTypeUsr() else {
+          return nil
+        }
+        symbol = typeSymbol
       }
-    }
-
-    // otherwise get the type of the symbol at this position
-    if symbol == nil {
-      guard let typeUsr: String = dict[keys.typeUsr] else {
+    } else {
+      guard let typeSymbol = try await symbolFromTypeUsr() else {
         return nil
       }
-      let typeInfo = try await cursorInfoFromTypeUSR(typeUsr, in: snapshot)
-      symbol = typeInfo?.symbolInfo
-    }
-
-    guard let symbol else {
-      return nil
+      symbol = typeSymbol
     }
 
     let locations = try await SourceKitLSP.definitionLocations(

Tests/SourceKitLSPTests/TypeDefinitionTests.swift

@@ -12,6 +12,7 @@
 
 @_spi(SourceKitLSP) import LanguageServerProtocol
 import SKTestSupport
+import SwiftExtensions
 import XCTest
 
 final class TypeDefinitionTests: SourceKitLSPTestCase {
@@ -171,4 +172,35 @@ final class TypeDefinitionTests: SourceKitLSPTestCase {
     XCTAssertEqual(location.uri, uri)
     XCTAssertEqual(location.range, Range(positions["1️⃣"]))
   }
+
+  func testTypeDefinitionGeneratedInterface() async throws {
+    let testClient = try await TestSourceKitLSPClient()
+    let uri = DocumentURI(for: .swift)
+
+    let positions = testClient.openDocument(
+      """
+      let 1️⃣x = "hello"
+      """,
+      uri: uri
+    )
+
+    let response = try await testClient.send(
+      TypeDefinitionRequest(
+        textDocument: TextDocumentIdentifier(uri),
+        position: positions["1️⃣"]
+      )
+    )
+
+    guard case .locations(let locations) = response, let location = locations.only else {
+      XCTFail("Expected single location response")
+      return
+    }
+
+    // Should jump to String in the generated Swift interface
+    XCTAssertTrue(
+      location.uri.pseudoPath.hasSuffix(".swiftinterface"),
+      "Expected swiftinterface file, got: \(location.uri.pseudoPath)"
+    )
+    assertContains(location.uri.pseudoPath, "String")
+  }
 }