Fully qualify type names in call hierarchy, type hierarchy and workspace symbols

Previously, we didn’t take outer types into account or only took one level of container type into account.

Fixes #1673
136078089

Author: ahoppen

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -1444,13 +1444,24 @@ extension SourceKitLSPServer {
         range: Range(symbolPosition)
       )
 
+      let containerNames = index.containerNames(of: symbolOccurrence)
+      let containerName: String?
+      if containerNames.isEmpty {
+        containerName = nil
+      } else {
+        switch symbolOccurrence.symbol.language {
+        case .cxx, .c, .objc: containerName = containerNames.joined(separator: "::")
+        case .swift: containerName = containerNames.joined(separator: ".")
+        }
+      }
+
       return WorkspaceSymbolItem.symbolInformation(
         SymbolInformation(
           name: symbolOccurrence.symbol.name,
           kind: symbolOccurrence.symbol.kind.asLspSymbolKind(),
           deprecated: nil,
           location: symbolLocation,
-          containerName: index.containerName(of: symbolOccurrence)
+          containerName: containerName
         )
       )
     }
@@ -1916,27 +1927,14 @@ extension SourceKitLSPServer {
   }
 
   private func indexToLSPCallHierarchyItem(
-    symbol: Symbol,
-    containerName: String?,
-    location: Location
-  ) -> CallHierarchyItem {
-    let name: String
-    if let containerName {
-      switch symbol.language {
-      case .objc where symbol.kind == .instanceMethod || symbol.kind == .instanceProperty:
-        name = "-[\(containerName) \(symbol.name)]"
-      case .objc where symbol.kind == .classMethod || symbol.kind == .classProperty:
-        name = "+[\(containerName) \(symbol.name)]"
-      case .cxx, .c, .objc:
-        // C shouldn't have container names for call hierarchy and Objective-C should be covered above.
-        // Fall back to using the C++ notation using `::`.
-        name = "\(containerName)::\(symbol.name)"
-      case .swift:
-        name = "\(containerName).\(symbol.name)"
-      }
-    } else {
-      name = symbol.name
+    definition: SymbolOccurrence,
+    index: CheckedIndex
+  ) -> CallHierarchyItem? {
+    guard let location = indexToLSPLocation(definition.location) else {
+      return nil
     }
+    let name = index.fullyQualifiedName(of: definition)
+    let symbol = definition.symbol
     return CallHierarchyItem(
       name: name,
       kind: symbol.kind.asLspSymbolKind(),
@@ -1976,14 +1974,7 @@ extension SourceKitLSPServer {
       guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else {
         return nil
       }
-      guard let location = indexToLSPLocation(definition.location) else {
-        return nil
-      }
-      return self.indexToLSPCallHierarchyItem(
-        symbol: definition.symbol,
-        containerName: index.containerName(of: definition),
-        location: location
-      )
+      return self.indexToLSPCallHierarchyItem(definition: definition, index: index)
     }.sorted(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) })
 
     // Ideally, we should show multiple symbols. But VS Code fails to display call hierarchies with multiple root items,
@@ -2045,38 +2036,27 @@ extension SourceKitLSPServer {
 
     // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed
     func indexToLSPCallHierarchyItem2(
-      symbol: Symbol,
-      containerName: String?,
-      location: Location
-    ) -> CallHierarchyItem {
-      return self.indexToLSPCallHierarchyItem(symbol: symbol, containerName: containerName, location: location)
+      definition: SymbolOccurrence,
+      index: CheckedIndex
+    ) -> CallHierarchyItem? {
+      return self.indexToLSPCallHierarchyItem(definition: definition, index: index)
     }
 
     let calls = callersToCalls.compactMap { (caller: Symbol, calls: [SymbolOccurrence]) -> CallHierarchyIncomingCall? in
       // Resolve the caller's definition to find its location
-      let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: caller.usr)
-      let definitionSymbolLocation = definition?.location
-      let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
-      let containerName: String? =
-        if let definition {
-          index.containerName(of: definition)
-        } else {
-          nil
-        }
+      guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: caller.usr) else {
+        return nil
+      }
 
       let locations = calls.compactMap { indexToLSPLocation2($0.location) }.sorted()
       guard !locations.isEmpty else {
         return nil
       }
+      guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else {
+        return nil
+      }
 
-      return CallHierarchyIncomingCall(
-        from: indexToLSPCallHierarchyItem2(
-          symbol: caller,
-          containerName: containerName,
-          location: definitionLocation ?? locations.first!
-        ),
-        fromRanges: locations.map(\.range)
-      )
+      return CallHierarchyIncomingCall(from: item, fromRanges: locations.map(\.range))
     }
     return calls.sorted(by: { $0.from.name < $1.from.name })
   }
@@ -2095,11 +2075,10 @@ extension SourceKitLSPServer {
 
     // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed
     func indexToLSPCallHierarchyItem2(
-      symbol: Symbol,
-      containerName: String?,
-      location: Location
-    ) -> CallHierarchyItem {
-      return self.indexToLSPCallHierarchyItem(symbol: symbol, containerName: containerName, location: location)
+      definition: SymbolOccurrence,
+      index: CheckedIndex
+    ) -> CallHierarchyItem? {
+      return self.indexToLSPCallHierarchyItem(definition: definition, index: index)
     }
 
     let callableUsrs = [data.usr] + index.occurrences(relatedToUSR: data.usr, roles: .accessorOf).map(\.symbol.usr)
@@ -2113,37 +2092,32 @@ extension SourceKitLSPServer {
       }
 
       // Resolve the callee's definition to find its location
-      let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr)
-      let definitionSymbolLocation = definition?.location
-      let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
-      let containerName: String? =
-        if let definition {
-          index.containerName(of: definition)
-        } else {
-          nil
-        }
+      guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr) else {
+        return nil
+      }
 
-      return CallHierarchyOutgoingCall(
-        to: indexToLSPCallHierarchyItem2(
-          symbol: occurrence.symbol,
-          containerName: containerName,
-          location: definitionLocation ?? location  // Use occurrence location as fallback
-        ),
-        fromRanges: [location.range]
-      )
+      guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else {
+        return nil
+      }
+
+      return CallHierarchyOutgoingCall(to: item, fromRanges: [location.range])
     }
     return calls.sorted(by: { $0.to.name < $1.to.name })
   }
 
   private func indexToLSPTypeHierarchyItem(
-    symbol: Symbol,
+    definition: SymbolOccurrence,
     moduleName: String?,
-    location: Location,
     index: CheckedIndex
-  ) -> TypeHierarchyItem {
+  ) -> TypeHierarchyItem? {
     let name: String
     let detail: String?
 
+    guard let location = indexToLSPLocation(definition.location) else {
+      return nil
+    }
+
+    let symbol = definition.symbol
     switch symbol.kind {
     case .extension:
       // Query the conformance added by this extension
@@ -2164,7 +2138,7 @@ extension SourceKitLSPServer {
         detail = "Extension"
       }
     default:
-      name = symbol.name
+      name = index.fullyQualifiedName(of: definition)
       detail = moduleName
     }
 
@@ -2213,16 +2187,12 @@ extension SourceKitLSPServer {
       }
       .compactMap(\.usr)
     let typeHierarchyItems = usrs.compactMap { (usr) -> TypeHierarchyItem? in
-      guard
-        let info = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr),
-        let location = indexToLSPLocation(info.location)
-      else {
+      guard let info = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else {
         return nil
       }
       return self.indexToLSPTypeHierarchyItem(
-        symbol: info.symbol,
+        definition: info,
         moduleName: info.location.moduleName,
-        location: location,
         index: index
       )
     }
@@ -2287,30 +2257,28 @@ extension SourceKitLSPServer {
 
     // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed
     func indexToLSPTypeHierarchyItem2(
-      symbol: Symbol,
+      definition: SymbolOccurrence,
       moduleName: String?,
-      location: Location,
       index: CheckedIndex
-    ) -> TypeHierarchyItem {
-      return self.indexToLSPTypeHierarchyItem(symbol: symbol, moduleName: moduleName, location: location, index: index)
+    ) -> TypeHierarchyItem? {
+      return self.indexToLSPTypeHierarchyItem(
+        definition: definition,
+        moduleName: moduleName,
+        index: index
+      )
     }
 
     // Convert occurrences to type hierarchy items
     let occurs = baseOccurs + retroactiveConformanceOccurs
     let types = occurs.compactMap { occurrence -> TypeHierarchyItem? in
-      guard let location = indexToLSPLocation2(occurrence.location) else {
+      // Resolve the supertype's definition to find its location
+      guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr) else {
         return nil
       }
 
-      // Resolve the supertype's definition to find its location
-      let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr)
-      let definitionSymbolLocation = definition?.location
-      let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
-
       return indexToLSPTypeHierarchyItem2(
-        symbol: occurrence.symbol,
-        moduleName: definitionSymbolLocation?.moduleName,
-        location: definitionLocation ?? location,  // Use occurrence location as fallback
+        definition: definition,
+        moduleName: definition.location.moduleName,
         index: index
       )
     }
@@ -2334,12 +2302,15 @@ extension SourceKitLSPServer {
 
     // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed
     func indexToLSPTypeHierarchyItem2(
-      symbol: Symbol,
+      definition: SymbolOccurrence,
       moduleName: String?,
-      location: Location,
       index: CheckedIndex
-    ) -> TypeHierarchyItem {
-      return self.indexToLSPTypeHierarchyItem(symbol: symbol, moduleName: moduleName, location: location, index: index)
+    ) -> TypeHierarchyItem? {
+      return self.indexToLSPTypeHierarchyItem(
+        definition: definition,
+        moduleName: moduleName,
+        index: index
+      )
     }
 
     // Convert occurrences to type hierarchy items
@@ -2350,20 +2321,18 @@ extension SourceKitLSPServer {
         // to.
         logger.fault("Expected at most extendedBy or baseOf relation but got \(occurrence.relations.count)")
       }
-      guard let related = occurrence.relations.sorted().first, let location = indexToLSPLocation2(occurrence.location)
-      else {
+      guard let related = occurrence.relations.sorted().first else {
         return nil
       }
 
       // Resolve the subtype's definition to find its location
-      let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: related.symbol.usr)
-      let definitionSymbolLocation = definition.map(\.location)
-      let definitionLocation = definitionSymbolLocation.flatMap(indexToLSPLocation2)
+      guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: related.symbol.usr) else {
+        return nil
+      }
 
       return indexToLSPTypeHierarchyItem2(
-        symbol: related.symbol,
-        moduleName: definitionSymbolLocation?.moduleName,
-        location: definitionLocation ?? location,  // Use occurrence location as fallback
+        definition: definition,
+        moduleName: definition.location.moduleName,
         index: index
       )
     }
@@ -2408,24 +2377,23 @@ private let maxWorkspaceSymbolResults = 4096
 package typealias Diagnostic = LanguageServerProtocol.Diagnostic
 
 fileprivate extension CheckedIndex {
-  /// Get the name of the symbol that is a parent of this symbol, if one exists
-  func containerName(of symbol: SymbolOccurrence) -> String? {
+  func containerNames(of symbol: SymbolOccurrence) -> [String] {
     // The container name of accessors is the container of the surrounding variable.
     let accessorOf = symbol.relations.filter { $0.roles.contains(.accessorOf) }
     if let primaryVariable = accessorOf.sorted().first {
       if accessorOf.count > 1 {
         logger.fault("Expected an occurrence to an accessor of at most one symbol, not multiple")
       }
       if let primaryVariable = primaryDefinitionOrDeclarationOccurrence(ofUSR: primaryVariable.symbol.usr) {
-        return containerName(of: primaryVariable)
+        return containerNames(of: primaryVariable)
       }
     }
 
     let containers = symbol.relations.filter { $0.roles.contains(.childOf) }
     if containers.count > 1 {
       logger.fault("Expected an occurrence to a child of at most one symbol, not multiple")
     }
-    return containers.filter {
+    let container = containers.filter {
       switch $0.symbol.kind {
       case .module, .namespace, .enum, .struct, .class, .protocol, .extension, .union:
         return true
@@ -2434,7 +2402,39 @@ fileprivate extension CheckedIndex {
         .destructor, .conversionFunction, .parameter, .using, .concept, .commentTag:
         return false
       }
-    }.sorted().first?.symbol.name
+    }.sorted().first
+
+    if let container {
+      if let containerDefinition = primaryDefinitionOrDeclarationOccurrence(ofUSR: container.symbol.usr) {
+        return self.containerNames(of: containerDefinition) + [container.symbol.name]
+      }
+      return [container.symbol.name]
+    } else {
+      return []
+    }
+  }
+
+  /// Take the name of containers into account to form a fully-qualified name for the given symbol.
+  /// This means that we will form names of nested types and type-qualify methods.
+  func fullyQualifiedName(of symbolOccurrence: SymbolOccurrence) -> String {
+    let symbol = symbolOccurrence.symbol
+    let containerNames = containerNames(of: symbolOccurrence)
+    guard let containerName = containerNames.last else {
+      // No containers, so nothing to do.
+      return symbol.name
+    }
+    switch symbol.language {
+    case .objc where symbol.kind == .instanceMethod || symbol.kind == .instanceProperty:
+      return "-[\(containerName) \(symbol.name)]"
+    case .objc where symbol.kind == .classMethod || symbol.kind == .classProperty:
+      return "+[\(containerName) \(symbol.name)]"
+    case .cxx, .c, .objc:
+      // C shouldn't have container names for call hierarchy and Objective-C should be covered above.
+      // Fall back to using the C++ notation using `::`.
+      return (containerNames + [symbol.name]).joined(separator: "::")
+    case .swift:
+      return (containerNames + [symbol.name]).joined(separator: ".")
+    }
   }
 }
 

Tests/SourceKitLSPTests/CallHierarchyTests.swift

@@ -833,7 +833,7 @@ final class CallHierarchyTests: XCTestCase {
       [
         CallHierarchyIncomingCall(
           from: CallHierarchyItem(
-            name: "Bar.init()",
+            name: "Outer.Bar.init()",
             kind: .constructor,
             tags: nil,
             uri: project.fileURI,