Merge pull request #989 from ahoppen/ahoppen/dont-use-local-loc-for-c-definition

Don’t use the best local declaration when jumping to definition of a C function

Author: ahoppen

Sources/SourceKitLSP/SourceKitServer.swift

@@ -1718,8 +1718,14 @@ extension SourceKitServer {
       return []
     }
 
-    if let bestLocalDeclaration = symbol.bestLocalDeclaration, !(symbol.isDynamic ?? true) {
-      // If we have a known non-dynamic symbol, we don't need to do an index lookup
+    if let bestLocalDeclaration = symbol.bestLocalDeclaration,
+      !(symbol.isDynamic ?? true),
+      symbol.usr?.hasPrefix("s:") ?? false /* Swift symbols have USRs starting with s: */
+    {
+      // If we have a known non-dynamic symbol within Swift, we don't need to do an index lookup.
+      // For non-Swift symbols, we need to perform an index lookup because the best local declaration will point to
+      // a header file but jump-to-definition should prefer the implementation (there's the declaration request to jump
+      // to the function's declaration).
       return [bestLocalDeclaration]
     }
 
@@ -1735,7 +1741,11 @@ extension SourceKitServer {
     }
 
     guard let index = await self.workspaceForDocument(uri: req.textDocument.uri)?.index else {
-      return []
+      if let bestLocalDeclaration = symbol.bestLocalDeclaration {
+        return [bestLocalDeclaration]
+      } else {
+        return []
+      }
     }
     guard let usr = symbol.usr else { return [] }
     logger.info("performing indexed jump-to-def with usr \(usr)")
@@ -1771,6 +1781,10 @@ extension SourceKitServer {
       }
     }
 
+    if occurrences.isEmpty, let bestLocalDeclaration = symbol.bestLocalDeclaration {
+      return [bestLocalDeclaration]
+    }
+
     return try await occurrences.asyncCompactMap { occurrence in
       if URL(fileURLWithPath: occurrence.location.path).pathExtension == "swiftinterface" {
         // If the location is in `.swiftinterface` file, use moduleName to return textual interface.

Tests/SourceKitLSPTests/DefinitionTests.swift

@@ -143,4 +143,56 @@ class DefinitionTests: XCTestCase {
       ]
     )
   }
+
+  func testJumpToCDefinitionFromSwift() async throws {
+    let ws = try await SwiftPMTestWorkspace(
+      files: [
+        "Sources/MyLibrary/include/test.h": """
+        void myFunc(void);
+        """,
+        "Sources/MyLibrary/test.c": """
+        #include "test.h"
+
+        void 1️⃣myFunc(void) {}
+        """,
+        "Sources/MySwiftLibrary/main.swift":
+          """
+        import MyLibrary
+
+        2️⃣myFunc()
+        """,
+      ],
+      manifest: """
+        // swift-tools-version: 5.7
+
+        import PackageDescription
+
+        let package = Package(
+          name: "MyLibrary",
+          targets: [
+            .target(name: "MyLibrary"),
+            .target(name: "MySwiftLibrary", dependencies: ["MyLibrary"])
+          ]
+        )
+        """,
+      build: true
+    )
+
+    let (uri, positions) = try ws.openDocument("main.swift")
+
+    let response = try await ws.testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    guard case .locations(let locations) = response else {
+      XCTFail("Expected locations response")
+      return
+    }
+
+    XCTAssertEqual(locations.count, 1)
+    let location = try XCTUnwrap(locations.first)
+    XCTAssertEqual(
+      location,
+      Location(uri: try ws.uri(for: "test.c"), range: Range(try ws.position(of: "1️⃣", in: "test.c")))
+    )
+  }
 }