Merge pull request #1074 from ahoppen/ahoppen/dont-report-definition-and-declaration

Only report a single item in prepareCallHierarchy

Author: ahoppen

Sources/SourceKitLSP/SourceKitServer.swift

@@ -1752,10 +1752,7 @@ extension SourceKitServer {
     }
     guard let usr = symbol.usr else { return [] }
     logger.info("performing indexed jump-to-def with usr \(usr)")
-    var occurrences = index.occurrences(ofUSR: usr, roles: [.definition])
-    if occurrences.isEmpty {
-      occurrences = index.occurrences(ofUSR: usr, roles: [.declaration])
-    }
+    var occurrences = index.definitionOrDeclarationOccurrences(ofUSR: usr)
     if symbol.isDynamic ?? true {
       lazy var transitiveReceiverUsrs: [String]? = {
         if let receiverUsrs = symbol.receiverUsrs {
@@ -1861,7 +1858,7 @@ extension SourceKitServer {
   /// 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 inteface.
+  /// compiler arguments to generate the generated interface.
   func definitionInInterface(
     moduleName: String,
     symbolUSR: String?,
@@ -1898,12 +1895,12 @@ extension SourceKitServer {
       return nil
     }
     guard let usr = symbol.usr else { return nil }
-    var occurrances = index.occurrences(ofUSR: usr, roles: .baseOf)
-    if occurrances.isEmpty {
-      occurrances = index.occurrences(relatedToUSR: usr, roles: .overrideOf)
+    var occurrences = index.occurrences(ofUSR: usr, roles: .baseOf)
+    if occurrences.isEmpty {
+      occurrences = index.occurrences(relatedToUSR: usr, roles: .overrideOf)
     }
 
-    return .locations(occurrances.compactMap { indexToLSPLocation($0.location) })
+    return .locations(occurrences.compactMap { indexToLSPLocation($0.location) })
   }
 
   func references(
@@ -1968,17 +1965,23 @@ extension SourceKitServer {
     }
     // For call hierarchy preparation we only locate the definition
     guard let usr = symbol.usr else { return nil }
-    return index.occurrences(ofUSR: usr, roles: [.definition, .declaration])
-      .compactMap { info -> CallHierarchyItem? in
-        guard let location = indexToLSPLocation(info.location) else {
-          return nil
-        }
-        return self.indexToLSPCallHierarchyItem(
-          symbol: info.symbol,
-          moduleName: info.location.moduleName,
-          location: location
-        )
-      }
+
+    // Only return a single call hierarchy item. Returning multiple doesn't make sense because they will all have the
+    // same USR (because we query them by USR) and will thus expand to the exact same call hierarchy.
+    // Also, VS Code doesn't seem to like multiple call hierarchy items being returned and fails to display any results
+    // if they are, failing with `Cannot read properties of undefined (reading 'map')`.
+    guard let definition = index.definitionOrDeclarationOccurrences(ofUSR: usr).first else {
+      return nil
+    }
+    guard let location = indexToLSPLocation(definition.location) else {
+      return nil
+    }
+    let callHierarchyItem = self.indexToLSPCallHierarchyItem(
+      symbol: definition.symbol,
+      moduleName: definition.location.moduleName,
+      location: location
+    )
+    return [callHierarchyItem]
   }
 
   /// Extracts our implementation-specific data about a call hierarchy
@@ -2260,6 +2263,18 @@ private let maxWorkspaceSymbolResults = 4096
 
 public typealias Diagnostic = LanguageServerProtocol.Diagnostic
 
+fileprivate extension IndexStoreDB {
+  /// If there are any definition occurrences of the given USR, return these.
+  /// Otherwise return declaration occurrences.
+  func definitionOrDeclarationOccurrences(ofUSR usr: String) -> [SymbolOccurrence] {
+    let definitions = occurrences(ofUSR: usr, roles: [.definition])
+    if !definitions.isEmpty {
+      return definitions
+    }
+    return occurrences(ofUSR: usr, roles: [.declaration])
+  }
+}
+
 extension IndexSymbolKind {
   func asLspSymbolKind() -> SymbolKind {
     switch self {
@@ -2333,8 +2348,8 @@ fileprivate func transitiveSubtypeClosure(ofUsrs usrs: [String], index: IndexSto
   var result: [String] = []
   for usr in usrs {
     result.append(usr)
-    let directSubtypes = index.occurrences(ofUSR: usr, roles: .baseOf).flatMap { occurance in
-      occurance.relations.filter { $0.roles.contains(.baseOf) }.map(\.symbol.usr)
+    let directSubtypes = index.occurrences(ofUSR: usr, roles: .baseOf).flatMap { occurrence in
+      occurrence.relations.filter { $0.roles.contains(.baseOf) }.map(\.symbol.usr)
     }
     let transitiveSubtypes = transitiveSubtypeClosure(ofUsrs: directSubtypes, index: index)
     result += transitiveSubtypes

Tests/SourceKitLSPTests/CallHierarchyTests.swift

@@ -183,4 +183,45 @@ final class CallHierarchyTests: XCTestCase {
       ]
     )
   }
+
+  func testReportSingleItemInPrepareCallHierarchy() async throws {
+    let ws = try await SwiftPMTestWorkspace(
+      files: [
+        "MyLibrary/include/lib.h": """
+        struct FilePathIndex {
+          void 1️⃣foo();
+        };
+        """,
+        "MyLibrary/lib.cpp": """
+        #include "lib.h"
+        void FilePathIndex::2️⃣foo() {}
+        """,
+      ],
+      build: true
+    )
+    let (uri, positions) = try ws.openDocument("lib.h", language: .cpp)
+    let result = try await ws.testClient.send(
+      CallHierarchyPrepareRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+
+    // Test that we don't provide both the definition in .cpp and the declaration on .h
+    XCTAssertEqual(
+      result,
+      [
+        CallHierarchyItem(
+          name: "foo",
+          kind: .method,
+          tags: nil,
+          detail: "",
+          uri: try ws.uri(for: "lib.cpp"),
+          range: try Range(ws.position(of: "2️⃣", in: "lib.cpp")),
+          selectionRange: try Range(ws.position(of: "2️⃣", in: "lib.cpp")),
+          data: LSPAny.dictionary([
+            "usr": .string("c:@S@FilePathIndex@F@foo#"),
+            "uri": .string(try ws.uri(for: "lib.cpp").stringValue),
+          ])
+        )
+      ]
+    )
+  }
 }