From eb72e10886e6734a409a75674d2d7626f1ece612 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Fri, 22 Nov 2024 21:14:46 +0100 Subject: [PATCH] Fully qualify type names in call hierarchy, type hierarchy and workspace symbols MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we didn’t take outer types into account or only took one level of container type into account. Fixes #1673 rdar://136078089 --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 226 +++++++++--------- .../CallHierarchyTests.swift | 2 +- .../TypeHierarchyTests.swift | 42 ++++ 3 files changed, 156 insertions(+), 114 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 6cedb6c50..d362f471c 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/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,8 +2377,7 @@ 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 { @@ -2417,7 +2385,7 @@ fileprivate extension CheckedIndex { 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) } } @@ -2425,7 +2393,7 @@ fileprivate extension CheckedIndex { 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: ".") + } } } diff --git a/Tests/SourceKitLSPTests/CallHierarchyTests.swift b/Tests/SourceKitLSPTests/CallHierarchyTests.swift index 21b24572b..7919a0ea7 100644 --- a/Tests/SourceKitLSPTests/CallHierarchyTests.swift +++ b/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, diff --git a/Tests/SourceKitLSPTests/TypeHierarchyTests.swift b/Tests/SourceKitLSPTests/TypeHierarchyTests.swift index a1ed51a7b..771c80abe 100644 --- a/Tests/SourceKitLSPTests/TypeHierarchyTests.swift +++ b/Tests/SourceKitLSPTests/TypeHierarchyTests.swift @@ -206,6 +206,48 @@ final class TypeHierarchyTests: XCTestCase { ) XCTAssertNil(response) } + + func testNestedTypeNameInSubtypes() async throws { + let project = try await IndexedSingleSwiftFileTestProject( + """ + protocol 1️⃣MyProto {} + struct Outer { + struct 2️⃣Mid: MyProto { + struct 3️⃣Inner: MyProto {} + } + } + """ + ) + let item = try await project.prepareTypeHierarchy(at: "1️⃣") + let subtypes = try await project.testClient.send(TypeHierarchySubtypesRequest(item: item)) + assertEqualIgnoringData( + subtypes, + [ + TypeHierarchyItem(name: "Outer.Mid", kind: .struct, location: "2️⃣", in: project), + TypeHierarchyItem(name: "Outer.Mid.Inner", kind: .struct, location: "3️⃣", in: project), + ] + ) + } + + func testNestedTypeNameInSupertypes() async throws { + let project = try await IndexedSingleSwiftFileTestProject( + """ + class Outer { + class 1️⃣Mid { + class 2️⃣Inner: Mid {} + } + } + """ + ) + let item = try await project.prepareTypeHierarchy(at: "2️⃣") + let supertypes = try await project.testClient.send(TypeHierarchySupertypesRequest(item: item)) + assertEqualIgnoringData( + supertypes, + [ + TypeHierarchyItem(name: "Outer.Mid", kind: .class, location: "1️⃣", in: project) + ] + ) + } } // MARK: - Utilities