From 70d900e799c731ca559113a6d1fb0c0531f91489 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Tue, 9 Dec 2025 12:40:55 +0530 Subject: [PATCH 1/2] Extend copied file mapping to all LSP requests returning locations This addresses issue #2276 by ensuring that all LSP requests that return source file locations map copied header files back to their original locations, not just jump-to-definition. Previously, only the definition request applied this mapping. Now, the following requests also adjust locations for copied files: - textDocument/references - textDocument/implementation - workspace/symbol - callHierarchy/prepare - callHierarchy/incomingCalls - callHierarchy/outgoingCalls - typeHierarchy/prepare - typeHierarchy/supertypes - typeHierarchy/subtypes This provides consistent navigation behavior, ensuring users are always taken to the original source files instead of build artifacts when possible. --- .../BuildServerManager.swift | 90 +++++ .../ClangLanguageService.swift | 19 +- Sources/SourceKitLSP/SourceKitLSPServer.swift | 358 +++++++++++++++--- .../SourceKitLSPTests/CopiedHeaderTests.swift | 234 ++++++++++++ 4 files changed, 635 insertions(+), 66 deletions(-) create mode 100644 Tests/SourceKitLSPTests/CopiedHeaderTests.swift diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index 44e133b1c..d2393ed71 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -962,6 +962,96 @@ package actor BuildServerManager: QueueBasedMessageHandler { return locations.map { locationAdjustedForCopiedFiles($0) } } + package func workspaceEditAdjustedForCopiedFiles(_ workspaceEdit: WorkspaceEdit?) async -> WorkspaceEdit? { + guard var edit = workspaceEdit else { + return nil + } + if let changes = edit.changes { + var newChanges: [DocumentURI: [TextEdit]] = [:] + for (uri, edits) in changes { + let newUri = self.locationAdjustedForCopiedFiles( + Location(uri: uri, range: Position(line: 0, utf16index: 0).. LocationsOrLocationLinksResponse? { + guard let response = response else { + return nil + } + switch response { + case .locations(let locations): + let remappedLocations = await self.locationsAdjustedForCopiedFiles(locations) + return .locations(remappedLocations) + case .locationLinks(let locationLinks): + var remappedLinks: [LocationLink] = [] + for link in locationLinks { + let newUri = self.locationAdjustedForCopiedFiles(Location(uri: link.targetUri, range: link.targetRange)).uri + remappedLinks.append(LocationLink( + originSelectionRange: link.originSelectionRange, + targetUri: newUri, + targetRange: link.targetRange, + targetSelectionRange: link.targetSelectionRange + )) + } + return .locationLinks(remappedLinks) + } + } + + package func typeHierarchyItemAdjustedForCopiedFiles(_ item: TypeHierarchyItem) async -> TypeHierarchyItem { + let adjustedLocation = await self.locationAdjustedForCopiedFiles(Location(uri: item.uri, range: item.range)) + return TypeHierarchyItem( + name: item.name, + kind: item.kind, + tags: item.tags, + detail: item.detail, + uri: adjustedLocation.uri, + range: adjustedLocation.range, + selectionRange: item.selectionRange, + data: item.data + ) + } + @discardableResult package func scheduleRecomputeCopyFileMap() -> Task { let task = Task { [previousUpdateTask = copiedFileMapUpdateTask] in diff --git a/Sources/ClangLanguageService/ClangLanguageService.swift b/Sources/ClangLanguageService/ClangLanguageService.swift index 8f414222b..2c1545242 100644 --- a/Sources/ClangLanguageService/ClangLanguageService.swift +++ b/Sources/ClangLanguageService/ClangLanguageService.swift @@ -481,7 +481,11 @@ extension ClangLanguageService { } package func declaration(_ req: DeclarationRequest) async throws -> LocationsOrLocationLinksResponse? { - return try await forwardRequestToClangd(req) + let result = try await forwardRequestToClangd(req) + guard let workspace = self.workspace.value else { + return result + } + return await workspace.buildServerManager.locationsOrLocationLinksAdjustedForCopiedFiles(result) } package func completion(_ req: CompletionRequest) async throws -> CompletionList { @@ -618,7 +622,11 @@ extension ClangLanguageService { } package func indexedRename(_ request: IndexedRenameRequest) async throws -> WorkspaceEdit? { - return try await forwardRequestToClangd(request) + let workspaceEdit = try await forwardRequestToClangd(request) + guard let workspace = self.workspace.value else { + return workspaceEdit + } + return await workspace.buildServerManager.workspaceEditAdjustedForCopiedFiles(workspaceEdit) } // MARK: - Other @@ -634,7 +642,12 @@ extension ClangLanguageService { position: renameRequest.position ) let symbolDetail = try await forwardRequestToClangd(symbolInfoRequest).only - return (try await edits ?? WorkspaceEdit(), symbolDetail?.usr) + let workspaceEdit = try await edits + guard let workspace = self.workspace.value else { + return (workspaceEdit ?? WorkspaceEdit(), symbolDetail?.usr) + } + let remappedEdit = await workspace.buildServerManager.workspaceEditAdjustedForCopiedFiles(workspaceEdit) + return (remappedEdit ?? WorkspaceEdit(), symbolDetail?.usr) } package func syntacticDocumentTests( diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 98aed8037..2de2a3346 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -1666,7 +1666,7 @@ extension SourceKitLSPServer { guard req.query.count >= minWorkspaceSymbolPatternLength else { return [] } - var symbolsAndIndex: [(symbol: SymbolOccurrence, index: CheckedIndex)] = [] + var symbolsAndIndex: [(symbol: SymbolOccurrence, index: CheckedIndex, workspace: Workspace)] = [] for workspace in workspaces { guard let index = await workspace.index(checkedFor: .deletedFiles) else { continue @@ -1690,10 +1690,10 @@ extension SourceKitLSPServer { } try Task.checkCancellation() symbolsAndIndex += symbolOccurrences.map { - return ($0, index) + return ($0, index, workspace) } } - return symbolsAndIndex.sorted(by: { $0.symbol < $1.symbol }).map { symbolOccurrence, index in + return await symbolsAndIndex.sorted(by: { $0.symbol < $1.symbol }).asyncMap { symbolOccurrence, index, workspace in let symbolPosition = Position( line: symbolOccurrence.location.line - 1, // 1-based -> 0-based // Technically we would need to convert the UTF-8 column to a UTF-16 column. This would require reading the @@ -1717,12 +1717,13 @@ extension SourceKitLSPServer { } } + let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(symbolLocation) return WorkspaceSymbolItem.symbolInformation( SymbolInformation( name: symbolOccurrence.symbol.name, kind: symbolOccurrence.symbol.kind.asLspSymbolKind(), deprecated: nil, - location: symbolLocation, + location: remappedLocation, containerName: containerName ) ) @@ -2142,7 +2143,8 @@ extension SourceKitLSPServer { // returning it to the client. if indexBasedResponse.isEmpty { return await orLog("Fallback definition request", level: .info) { - return try await languageService.definition(req) + let result = try await languageService.definition(req) + return await workspace.buildServerManager.locationsOrLocationLinksAdjustedForCopiedFiles(result) } } let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(indexBasedResponse) @@ -2202,7 +2204,8 @@ extension SourceKitLSPServer { return occurrences.compactMap { indexToLSPLocation($0.location) } } - return .locations(locations.sorted()) + let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) + return .locations(remappedLocations.sorted()) } func references( @@ -2228,7 +2231,8 @@ extension SourceKitLSPServer { } return index.occurrences(ofUSR: usr, roles: roles).compactMap { indexToLSPLocation($0.location) } } - return locations.unique.sorted() + let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) + return remappedLocations.unique.sorted() } private func indexToLSPCallHierarchyItem( @@ -2275,12 +2279,38 @@ extension SourceKitLSPServer { // 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. - let callHierarchyItems = usrs.compactMap { (usr) -> CallHierarchyItem? in + var callHierarchyItems: [CallHierarchyItem] = [] + for usr in usrs { guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else { - return nil + continue } - return self.indexToLSPCallHierarchyItem(definition: definition, index: index) - }.sorted(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) }) + let location = indexToLSPLocation(definition.location) + guard let originalLocation = location else { + continue + } + let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) + + // Create a new CallHierarchyItem with the remapped location (similar to how we handle TypeHierarchyItem) + let name = index.fullyQualifiedName(of: definition) + let symbol = definition.symbol + + let item = CallHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: nil, + uri: remappedLocation.uri, + range: remappedLocation.range, + selectionRange: remappedLocation.range, + // We encode usr and uri for incoming/outgoing call lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(remappedLocation.uri.stringValue), + ]) + ) + callHierarchyItems.append(item) + } + callHierarchyItems.sort(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, // failing with `Cannot read properties of undefined (reading 'map')`. Pick the first one. @@ -2305,7 +2335,8 @@ extension SourceKitLSPServer { func incomingCalls(_ req: CallHierarchyIncomingCallsRequest) async throws -> [CallHierarchyIncomingCall]? { guard let data = extractCallHierarchyItemData(req.item.data), - let index = await self.workspaceForDocument(uri: data.uri)?.index(checkedFor: .deletedFiles) + let workspace = await self.workspaceForDocument(uri: data.uri), + let index = await workspace.index(checkedFor: .deletedFiles) else { return [] } @@ -2327,7 +2358,7 @@ extension SourceKitLSPServer { var callersToCalls: [Symbol: [SymbolOccurrence]] = [:] for call in callOccurrences { - // Callers are all `calledBy` relations of a call to a USR in `callableUsrs`, ie. all the functions that contain a + // Callers are all `calledBy` relations of a call to a USR in `callableUSrs`, ie. all the functions that contain a // call to a USR in callableUSRs. In practice, this should always be a single item. let callers = call.relations.filter { $0.roles.contains(.containedBy) }.map(\.symbol) for caller in callers { @@ -2348,28 +2379,54 @@ extension SourceKitLSPServer { return self.indexToLSPCallHierarchyItem(definition: definition, index: index) } - let calls = callersToCalls.compactMap { (caller: Symbol, calls: [SymbolOccurrence]) -> CallHierarchyIncomingCall? in + var calls: [CallHierarchyIncomingCall] = [] + for (caller, callsList) in callersToCalls { // Resolve the caller's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: caller.usr) else { - return nil + continue } - let locations = calls.compactMap { indexToLSPLocation2($0.location) }.sorted() - guard !locations.isEmpty else { - return nil + let locations = callsList.compactMap { indexToLSPLocation2($0.location) }.sorted() + let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) + guard !remappedLocations.isEmpty else { + continue } - guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else { - return nil + + // Now we need to get the remapped location for the definition item itself + let definitionLocation = indexToLSPLocation2(definition.location) + guard let originalDefinitionLocation = definitionLocation else { + continue } + let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalDefinitionLocation) + + // Create a new CallHierarchyItem with the remapped location + let name = index.fullyQualifiedName(of: definition) + let symbol = definition.symbol + + let remappedItem = CallHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: nil, + uri: remappedDefinitionLocation.uri, + range: remappedDefinitionLocation.range, + selectionRange: remappedDefinitionLocation.range, + // We encode usr and uri for incoming/outgoing call lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(remappedDefinitionLocation.uri.stringValue), + ]) + ) - return CallHierarchyIncomingCall(from: item, fromRanges: locations.map(\.range)) + calls.append(CallHierarchyIncomingCall(from: remappedItem, fromRanges: remappedLocations.map(\.range))) } return calls.sorted(by: { $0.from.name < $1.from.name }) } func outgoingCalls(_ req: CallHierarchyOutgoingCallsRequest) async throws -> [CallHierarchyOutgoingCall]? { guard let data = extractCallHierarchyItemData(req.item.data), - let index = await self.workspaceForDocument(uri: data.uri)?.index(checkedFor: .deletedFiles) + let workspace = await self.workspaceForDocument(uri: data.uri), + let index = await workspace.index(checkedFor: .deletedFiles) else { return [] } @@ -2390,24 +2447,48 @@ extension SourceKitLSPServer { let callableUsrs = [data.usr] + index.occurrences(relatedToUSR: data.usr, roles: .accessorOf).map(\.symbol.usr) let callOccurrences = callableUsrs.flatMap { index.occurrences(relatedToUSR: $0, roles: .containedBy) } .filter(\.shouldShowInCallHierarchy) - let calls = callOccurrences.compactMap { occurrence -> CallHierarchyOutgoingCall? in + var calls: [CallHierarchyOutgoingCall] = [] + for occurrence in callOccurrences { guard occurrence.symbol.kind.isCallable else { - return nil + continue } guard let location = indexToLSPLocation2(occurrence.location) else { - return nil + continue } + let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(location) // Resolve the callee's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr) else { - return nil + continue } - guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else { - return nil + // Get the remapped location for the definition item itself + let definitionLocation = indexToLSPLocation2(definition.location) + guard let originalDefinitionLocation = definitionLocation else { + continue } + let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalDefinitionLocation) + + // Create a new CallHierarchyItem with the remapped location + let name = index.fullyQualifiedName(of: definition) + let symbol = definition.symbol + + let remappedItem = CallHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: nil, + uri: remappedDefinitionLocation.uri, + range: remappedDefinitionLocation.range, + selectionRange: remappedDefinitionLocation.range, + // We encode usr and uri for incoming/outgoing call lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(remappedDefinitionLocation.uri.stringValue), + ]) + ) - return CallHierarchyOutgoingCall(to: item, fromRanges: [location.range]) + calls.append(CallHierarchyOutgoingCall(to: remappedItem, fromRanges: [remappedLocation.range])) } return calls.sorted(by: { $0.to.name < $1.to.name }) } @@ -2425,15 +2506,25 @@ extension SourceKitLSPServer { } let symbol = definition.symbol - switch symbol.kind { - case .extension: - // Query the conformance added by this extension - let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) - if conformances.isEmpty { - name = symbol.name - } else { - name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" - } + switch symbol.kind { + case .extension: + // Query the conformance added by this extension + let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) + if conformances.isEmpty { + name = symbol.name + } else { + name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" + } + // Add the file name and line to the detail string + if let url = remappedLocation.uri.fileURL, + let basename = (try? AbsolutePath(validating: url.filePath))?.basename + { + detail = "Extension at \(basename):\(remappedLocation.range.lowerBound.line + 1)" + } else if !definition.location.moduleName.isEmpty { + detail = "Extension in \(definition.location.moduleName)" + } else { + detail = "Extension" + } // Add the file name and line to the detail string if let url = location.uri.fileURL, let basename = (try? AbsolutePath(validating: url.filePath))?.basename @@ -2493,9 +2584,10 @@ extension SourceKitLSPServer { } }.compactMap(\.usr) - let typeHierarchyItems = usrs.compactMap { (usr) -> TypeHierarchyItem? in + var typeHierarchyItems: [TypeHierarchyItem] = [] + for usr in usrs { guard let info = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else { - return nil + continue } // Filter symbols based on their kind in the index since the filter on the symbol info response might have // returned `nil` for the kind, preventing us from doing any filtering there. @@ -2503,18 +2595,64 @@ extension SourceKitLSPServer { case .unknown, .macro, .function, .variable, .field, .enumConstant, .instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor, .destructor, .conversionFunction, .parameter, .concept, .commentTag: - return nil + continue case .module, .namespace, .namespaceAlias, .enum, .struct, .class, .protocol, .extension, .union, .typealias, .using: break } - return self.indexToLSPTypeHierarchyItem( - definition: info, - moduleName: info.location.moduleName, - index: index - ) - } - .sorted(by: { $0.name < $1.name }) + + let location = indexToLSPLocation(info.location) + guard let originalLocation = location else { + continue + } + let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) + + // Create a new TypeHierarchyItem with the original location, then adjust for copied files + let name: String + let detail: String? + let symbol = info.symbol + switch symbol.kind { + case .extension: + // Query the conformance added by this extension + let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) + if conformances.isEmpty { + name = symbol.name + } else { + name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" + } + // Add the file name and line to the detail string + if let url = remappedLocation.uri.fileURL, + let basename = (try? AbsolutePath(validating: url.filePath))?.basename + { + detail = "Extension at \(basename):\(remappedLocation.range.lowerBound.line + 1)" + } else if !info.location.moduleName.isEmpty { + detail = "Extension in \(info.location.moduleName)" + } else { + detail = "Extension" + } + default: + name = index.fullyQualifiedName(of: info) + detail = info.location.moduleName + } + + let item = TypeHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: detail, + uri: originalLocation.uri, + range: originalLocation.range, + selectionRange: originalLocation.range, + // We encode usr and uri for incoming/outgoing type lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(originalLocation.uri.stringValue), + ]) + ) + let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) + typeHierarchyItems.append(remappedItem) + } + typeHierarchyItems.sort(by: { $0.name < $1.name }) if typeHierarchyItems.isEmpty { // When returning an empty array, VS Code fails with the following two errors. Returning `nil` works around those @@ -2546,7 +2684,8 @@ extension SourceKitLSPServer { func supertypes(_ req: TypeHierarchySupertypesRequest) async throws -> [TypeHierarchyItem]? { guard let data = extractTypeHierarchyItemData(req.item.data), - let index = await self.workspaceForDocument(uri: data.uri)?.index(checkedFor: .deletedFiles) + let workspace = await self.workspaceForDocument(uri: data.uri), + let index = await workspace.index(checkedFor: .deletedFiles) else { return [] } @@ -2588,24 +2727,71 @@ extension SourceKitLSPServer { // Convert occurrences to type hierarchy items let occurs = baseOccurs + retroactiveConformanceOccurs - let types = occurs.compactMap { occurrence -> TypeHierarchyItem? in + var types: [TypeHierarchyItem] = [] + for occurrence in occurs { // Resolve the supertype's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr) else { - return nil + continue } - return indexToLSPTypeHierarchyItem2( - definition: definition, - moduleName: definition.location.moduleName, - index: index + let location = indexToLSPLocation2(definition.location) + guard let originalLocation = location else { + continue + } + let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) + + // Create a new TypeHierarchyItem with the original location, then adjust for copied files + let name: String + let detail: String? + let symbol = definition.symbol + switch symbol.kind { + case .extension: + // Query the conformance added by this extension + let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) + if conformances.isEmpty { + name = symbol.name + } else { + name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" + } + // Add the file name and line to the detail string + if let url = adjustedLocation.uri.fileURL, + let basename = (try? AbsolutePath(validating: url.filePath))?.basename + { + detail = "Extension at \(basename):\(adjustedLocation.range.lowerBound.line + 1)" + } else if !definition.location.moduleName.isEmpty { + detail = "Extension in \(definition.location.moduleName)" + } else { + detail = "Extension" + } + default: + name = index.fullyQualifiedName(of: definition) + detail = definition.location.moduleName + } + + let item = TypeHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: detail, + uri: originalLocation.uri, + range: originalLocation.range, + selectionRange: originalLocation.range, + // We encode usr and uri for incoming/outgoing type lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(originalLocation.uri.stringValue), + ]) ) + let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) + types.append(remappedItem) } return types.sorted(by: { $0.name < $1.name }) } func subtypes(_ req: TypeHierarchySubtypesRequest) async throws -> [TypeHierarchyItem]? { guard let data = extractTypeHierarchyItemData(req.item.data), - let index = await self.workspaceForDocument(uri: data.uri)?.index(checkedFor: .deletedFiles) + let workspace = await self.workspaceForDocument(uri: data.uri), + let index = await workspace.index(checkedFor: .deletedFiles) else { return [] } @@ -2632,7 +2818,8 @@ extension SourceKitLSPServer { } // Convert occurrences to type hierarchy items - let types = occurs.compactMap { occurrence -> TypeHierarchyItem? in + var types: [TypeHierarchyItem] = [] + for occurrence in occurs { if occurrence.relations.count > 1 { // An occurrence with a `baseOf` or `extendedBy` relation is an occurrence inside an inheritance clause. // Such an occurrence can only be the source of a single type, namely the one that the inheritance clause belongs @@ -2640,19 +2827,64 @@ extension SourceKitLSPServer { logger.fault("Expected at most extendedBy or baseOf relation but got \(occurrence.relations.count)") } guard let related = occurrence.relations.sorted().first else { - return nil + continue } // Resolve the subtype's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: related.symbol.usr) else { - return nil + continue } - return indexToLSPTypeHierarchyItem2( - definition: definition, - moduleName: definition.location.moduleName, - index: index + let location = indexToLSPLocation2(definition.location) + guard let originalLocation = location else { + continue + } + let adjustedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) + + // Create a new TypeHierarchyItem with the original location, then adjust for copied files + let name: String + let detail: String? + let symbol = definition.symbol + switch symbol.kind { + case .extension: + // Query the conformance added by this extension + let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) + if conformances.isEmpty { + name = symbol.name + } else { + name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" + } + // Add the file name and line to the detail string + if let url = adjustedLocation.uri.fileURL, + let basename = (try? AbsolutePath(validating: url.filePath))?.basename + { + detail = "Extension at \(basename):\(adjustedLocation.range.lowerBound.line + 1)" + } else if !definition.location.moduleName.isEmpty { + detail = "Extension in \(definition.location.moduleName)" + } else { + detail = "Extension" + } + default: + name = index.fullyQualifiedName(of: definition) + detail = definition.location.moduleName + } + + let item = TypeHierarchyItem( + name: name, + kind: symbol.kind.asLspSymbolKind(), + tags: nil, + detail: detail, + uri: originalLocation.uri, + range: originalLocation.range, + selectionRange: originalLocation.range, + // We encode usr and uri for incoming/outgoing type lookups in the implementation-specific data field + data: .dictionary([ + "usr": .string(symbol.usr), + "uri": .string(originalLocation.uri.stringValue), + ]) ) + let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) + types.append(remappedItem) } return types.sorted { $0.name < $1.name } } diff --git a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift new file mode 100644 index 000000000..640299749 --- /dev/null +++ b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift @@ -0,0 +1,234 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import BuildServerIntegration +@_spi(SourceKitLSP) import BuildServerProtocol +@_spi(SourceKitLSP) import LanguageServerProtocol +@_spi(Testing) @_spi(SourceKitLSP) import SKLogging +import SKTestSupport +import SwiftExtensions +import XCTest + +class CopiedHeaderTests: SourceKitLSPTestCase { + actor BuildServer: CustomBuildServer { + let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker() + private let projectRoot: URL + + private var headerCopyDestination: URL { + projectRoot.appending(components: "header-copy", "CopiedTest.h") + } + + init(projectRoot: URL, connectionToSourceKitLSP: any Connection) { + self.projectRoot = projectRoot + } + + func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse { + return try initializationResponseSupportingBackgroundIndexing( + projectRoot: projectRoot, + outputPathsProvider: false + ) + } + + func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) -> BuildTargetSourcesResponse { + return BuildTargetSourcesResponse(items: [ + SourcesItem( + target: .dummy, + sources: [ + SourceItem( + uri: DocumentURI(projectRoot.appending(component: "Test.c")), + kind: .file, + generated: false, + dataKind: .sourceKit, + data: SourceKitSourceItemData( + language: .c, + kind: .source, + outputPath: nil, + copyDestinations: nil + ).encodeToLSPAny() + ), + SourceItem( + uri: DocumentURI(projectRoot.appending(component: "Test.h")), + kind: .file, + generated: false, + dataKind: .sourceKit, + data: SourceKitSourceItemData( + language: .c, + kind: .header, + outputPath: nil, + copyDestinations: [DocumentURI(headerCopyDestination)] + ).encodeToLSPAny() + ), + ] + ) + ]) + } + + func textDocumentSourceKitOptionsRequest( + _ request: TextDocumentSourceKitOptionsRequest + ) throws -> TextDocumentSourceKitOptionsResponse? { + return TextDocumentSourceKitOptionsResponse(compilerArguments: [ + request.textDocument.uri.pseudoPath, "-I", try headerCopyDestination.deletingLastPathComponent().filePath, + ]) + } + + func prepareTarget(_ request: BuildTargetPrepareRequest) async throws -> VoidResponse { + try FileManager.default.createDirectory( + at: headerCopyDestination.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try FileManager.default.copyItem( + at: projectRoot.appending(component: "Test.h"), + to: headerCopyDestination + ) + return VoidResponse() + } + } + + func testFindReferencesInCopiedHeader() async throws { + let project = try await CustomBuildServerTestProject( + files: [ + "Test.h": """ + void 1️⃣hello(); + """, + "Test.c": """ + #include + + void test() { + 2️⃣hello(); + } + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + try await project.testClient.send(SynchronizeRequest(copyFileMap: true)) + + let (uri, positions) = try project.openDocument("Test.c") + var response = try await project.testClient.send( + ReferencesRequest( + textDocument: TextDocumentIdentifier(uri), + position: positions["2️⃣"], + context: ReferencesContext(includeDeclaration: true) + ) + ) + response.sort() + var expected = [ + try project.location(from: "1️⃣", to: "1️⃣", in: "Test.h"), + try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c"), + ] + expected.sort() + XCTAssertEqual(response, expected) + } + + func DISABLED_testFindImplementationInCopiedHeader() async throws { + let project = try await CustomBuildServerTestProject( + files: [ + "Test.h": """ + void 1️⃣hello(); + """, + "Test.c": """ + #include + + void 2️⃣hello() {} + + void test() { + 3️⃣hello(); + } + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + try await project.testClient.send(SynchronizeRequest(copyFileMap: true)) + + let (uri, positions) = try project.openDocument("Test.c") + let response = try await project.testClient.send( + ImplementationRequest( + textDocument: TextDocumentIdentifier(uri), + position: positions["3️⃣"] + ) + ) + XCTAssertEqual( + response?.locations, + [ + try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c"), + ] + ) + } + + func testFindDeclarationInCopiedHeader() async throws { + let project = try await CustomBuildServerTestProject( + files: [ + "Test.h": """ + void 1️⃣hello2️⃣(); + """, + "Test.c": """ + #include + + void hello() {} + + void test() { + 3️⃣hello(); + } + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + try await project.testClient.send(SynchronizeRequest(copyFileMap: true)) + + let (uri, positions) = try project.openDocument("Test.c") + let response = try await project.testClient.send( + DeclarationRequest( + textDocument: TextDocumentIdentifier(uri), + position: positions["3️⃣"] + ) + ) + XCTAssertEqual( + response?.locations, + [ + try project.location(from: "1️⃣", to: "2️⃣", in: "Test.h"), + ] + ) + } + + func testWorkspaceSymbolsInCopiedHeader() async throws { + let project = try await CustomBuildServerTestProject( + files: [ + "Test.h": """ + void 1️⃣hello2️⃣(); + """, + "Test.c": """ + #include + + void test() { + hello(); + } + """, + ], + buildServer: BuildServer.self, + enableBackgroundIndexing: true + ) + try await project.testClient.send(SynchronizeRequest(copyFileMap: true)) + + _ = try project.openDocument("Test.c") + let response = try await project.testClient.send( + WorkspaceSymbolsRequest(query: "hello") + ) + let item = try XCTUnwrap(response?.only) + guard case .symbolInformation(let info) = item else { + XCTFail("Expected a symbol information") + return + } + XCTAssertEqual(info.location, try project.location(from: "1️⃣", to: "2️⃣", in: "Test.h")) + } +} From cae01c623dfda2c57ed407c50ab5adc8b15f1c78 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Wed, 10 Dec 2025 23:42:14 +0530 Subject: [PATCH 2/2] Apply code review fixes for copied file handling - Remove async from workspaceEditAdjustedForCopiedFiles - Refactor to use uriAdjustedForCopiedFiles helper - Update dictionary update logic with += - Adjust LocationLink creation to use adjusted ranges - Ensure selectionRange adjustment in prepareCallHierarchy - Provide default WorkspaceEdit in ClangLanguageService - Revert asyncMap to map and remove await in SourceKitLSPServer - Chain workspace and index retrieval in incomingCalls - Use indexToLSPCallHierarchyItem and shared helper for CallHierarchyItem - Fix indentation and remove duplicated detail setting - Use shared helper for TypeHierarchyItem - Remove .sort() from expected array in tests - Enable testFindImplementationInCopiedHeader - Add await for actor-isolated BuildServerManager calls --- .../BuildServerManager.swift | 60 +++---- .../ClangLanguageService.swift | 4 +- Sources/SourceKitLSP/SourceKitLSPServer.swift | 149 +++++++----------- .../SourceKitLSPTests/CopiedHeaderTests.swift | 4 +- 4 files changed, 84 insertions(+), 133 deletions(-) diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index d2393ed71..2ac354636 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -962,17 +962,22 @@ package actor BuildServerManager: QueueBasedMessageHandler { return locations.map { locationAdjustedForCopiedFiles($0) } } - package func workspaceEditAdjustedForCopiedFiles(_ workspaceEdit: WorkspaceEdit?) async -> WorkspaceEdit? { + private func uriAdjustedForCopiedFiles(_ uri: DocumentURI) -> DocumentURI { + guard let originalUri = cachedCopiedFileMap[uri] else { + return uri + } + return originalUri + } + + package func workspaceEditAdjustedForCopiedFiles(_ workspaceEdit: WorkspaceEdit?) -> WorkspaceEdit? { guard var edit = workspaceEdit else { return nil } if let changes = edit.changes { var newChanges: [DocumentURI: [TextEdit]] = [:] for (uri, edits) in changes { - let newUri = self.locationAdjustedForCopiedFiles( - Location(uri: uri, range: Position(line: 0, utf16index: 0).. LocationsOrLocationLinksResponse? { + package func locationsOrLocationLinksAdjustedForCopiedFiles(_ response: LocationsOrLocationLinksResponse?) -> LocationsOrLocationLinksResponse? { guard let response = response else { return nil } switch response { case .locations(let locations): - let remappedLocations = await self.locationsAdjustedForCopiedFiles(locations) + let remappedLocations = self.locationsAdjustedForCopiedFiles(locations) return .locations(remappedLocations) case .locationLinks(let locationLinks): var remappedLinks: [LocationLink] = [] for link in locationLinks { - let newUri = self.locationAdjustedForCopiedFiles(Location(uri: link.targetUri, range: link.targetRange)).uri + let adjustedTargetLocation = self.locationAdjustedForCopiedFiles(Location(uri: link.targetUri, range: link.targetRange)) + let adjustedTargetSelectionLocation = self.locationAdjustedForCopiedFiles(Location(uri: link.targetUri, range: link.targetSelectionRange)) remappedLinks.append(LocationLink( originSelectionRange: link.originSelectionRange, - targetUri: newUri, - targetRange: link.targetRange, - targetSelectionRange: link.targetSelectionRange + targetUri: adjustedTargetLocation.uri, + targetRange: adjustedTargetLocation.range, + targetSelectionRange: adjustedTargetSelectionLocation.range )) } return .locationLinks(remappedLinks) } } - package func typeHierarchyItemAdjustedForCopiedFiles(_ item: TypeHierarchyItem) async -> TypeHierarchyItem { - let adjustedLocation = await self.locationAdjustedForCopiedFiles(Location(uri: item.uri, range: item.range)) + package func typeHierarchyItemAdjustedForCopiedFiles(_ item: TypeHierarchyItem) -> TypeHierarchyItem { + let adjustedLocation = self.locationAdjustedForCopiedFiles(Location(uri: item.uri, range: item.range)) + let adjustedSelectionLocation = self.locationAdjustedForCopiedFiles(Location(uri: item.uri, range: item.selectionRange)) return TypeHierarchyItem( name: item.name, kind: item.kind, @@ -1047,7 +1039,7 @@ package actor BuildServerManager: QueueBasedMessageHandler { detail: item.detail, uri: adjustedLocation.uri, range: adjustedLocation.range, - selectionRange: item.selectionRange, + selectionRange: adjustedSelectionLocation.range, data: item.data ) } diff --git a/Sources/ClangLanguageService/ClangLanguageService.swift b/Sources/ClangLanguageService/ClangLanguageService.swift index 2c1545242..3c28ddfdc 100644 --- a/Sources/ClangLanguageService/ClangLanguageService.swift +++ b/Sources/ClangLanguageService/ClangLanguageService.swift @@ -642,9 +642,9 @@ extension ClangLanguageService { position: renameRequest.position ) let symbolDetail = try await forwardRequestToClangd(symbolInfoRequest).only - let workspaceEdit = try await edits + let workspaceEdit = try await edits ?? WorkspaceEdit() guard let workspace = self.workspace.value else { - return (workspaceEdit ?? WorkspaceEdit(), symbolDetail?.usr) + return (workspaceEdit, symbolDetail?.usr) } let remappedEdit = await workspace.buildServerManager.workspaceEditAdjustedForCopiedFiles(workspaceEdit) return (remappedEdit ?? WorkspaceEdit(), symbolDetail?.usr) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 2de2a3346..5b17e1435 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2260,6 +2260,36 @@ extension SourceKitLSPServer { ) } + private func callHierarchyItemAdjustedForCopiedFiles( + _ item: CallHierarchyItem, + workspace: Workspace + ) async -> CallHierarchyItem { + let adjustedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles( + Location(uri: item.uri, range: item.range) + ) + let adjustedSelectionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles( + Location(uri: item.uri, range: item.selectionRange) + ) + return CallHierarchyItem( + name: item.name, + kind: item.kind, + tags: item.tags, + detail: item.detail, + uri: adjustedLocation.uri, + range: adjustedLocation.range, + selectionRange: adjustedSelectionLocation.range, + data: .dictionary([ + "usr": item.data.flatMap { data in + if case let .dictionary(dict) = data { + return dict["usr"] + } + return nil + } ?? .null, + "uri": .string(adjustedLocation.uri.stringValue), + ]) + ) + } + func prepareCallHierarchy( _ req: CallHierarchyPrepareRequest, workspace: Workspace, @@ -2284,31 +2314,11 @@ extension SourceKitLSPServer { guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else { continue } - let location = indexToLSPLocation(definition.location) - guard let originalLocation = location else { + guard let item = indexToLSPCallHierarchyItem(definition: definition, index: index) else { continue } - let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) - - // Create a new CallHierarchyItem with the remapped location (similar to how we handle TypeHierarchyItem) - let name = index.fullyQualifiedName(of: definition) - let symbol = definition.symbol - - let item = CallHierarchyItem( - name: name, - kind: symbol.kind.asLspSymbolKind(), - tags: nil, - detail: nil, - uri: remappedLocation.uri, - range: remappedLocation.range, - selectionRange: remappedLocation.range, - // We encode usr and uri for incoming/outgoing call lookups in the implementation-specific data field - data: .dictionary([ - "usr": .string(symbol.usr), - "uri": .string(remappedLocation.uri.stringValue), - ]) - ) - callHierarchyItems.append(item) + let adjustedItem = await callHierarchyItemAdjustedForCopiedFiles(item, workspace: workspace) + callHierarchyItems.append(adjustedItem) } callHierarchyItems.sort(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) }) @@ -2358,7 +2368,7 @@ extension SourceKitLSPServer { var callersToCalls: [Symbol: [SymbolOccurrence]] = [:] for call in callOccurrences { - // Callers are all `calledBy` relations of a call to a USR in `callableUSrs`, ie. all the functions that contain a + // Callers are all `containedBy` relations of a call to a USR in `callableUSrs`, ie. all the functions that contain a // call to a USR in callableUSRs. In practice, this should always be a single item. let callers = call.relations.filter { $0.roles.contains(.containedBy) }.map(\.symbol) for caller in callers { @@ -2506,32 +2516,22 @@ extension SourceKitLSPServer { } let symbol = definition.symbol - switch symbol.kind { - case .extension: - // Query the conformance added by this extension - let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) - if conformances.isEmpty { - name = symbol.name - } else { - name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" - } - // Add the file name and line to the detail string - if let url = remappedLocation.uri.fileURL, - let basename = (try? AbsolutePath(validating: url.filePath))?.basename - { - detail = "Extension at \(basename):\(remappedLocation.range.lowerBound.line + 1)" - } else if !definition.location.moduleName.isEmpty { - detail = "Extension in \(definition.location.moduleName)" - } else { - detail = "Extension" - } + switch symbol.kind { + case .extension: + // Query the conformance added by this extension + let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) + if conformances.isEmpty { + name = symbol.name + } else { + name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" + } // Add the file name and line to the detail string if let url = location.uri.fileURL, let basename = (try? AbsolutePath(validating: url.filePath))?.basename { detail = "Extension at \(basename):\(location.range.lowerBound.line + 1)" - } else if let moduleName = moduleName { - detail = "Extension in \(moduleName)" + } else if !definition.location.moduleName.isEmpty { + detail = "Extension in \(definition.location.moduleName)" } else { detail = "Extension" } @@ -2602,55 +2602,16 @@ extension SourceKitLSPServer { } let location = indexToLSPLocation(info.location) - guard let originalLocation = location else { + guard location != nil else { continue } - let remappedLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalLocation) - - // Create a new TypeHierarchyItem with the original location, then adjust for copied files - let name: String - let detail: String? - let symbol = info.symbol - switch symbol.kind { - case .extension: - // Query the conformance added by this extension - let conformances = index.occurrences(relatedToUSR: symbol.usr, roles: .baseOf) - if conformances.isEmpty { - name = symbol.name - } else { - name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" - } - // Add the file name and line to the detail string - if let url = remappedLocation.uri.fileURL, - let basename = (try? AbsolutePath(validating: url.filePath))?.basename - { - detail = "Extension at \(basename):\(remappedLocation.range.lowerBound.line + 1)" - } else if !info.location.moduleName.isEmpty { - detail = "Extension in \(info.location.moduleName)" - } else { - detail = "Extension" - } - default: - name = index.fullyQualifiedName(of: info) - detail = info.location.moduleName - } - - let item = TypeHierarchyItem( - name: name, - kind: symbol.kind.asLspSymbolKind(), - tags: nil, - detail: detail, - uri: originalLocation.uri, - range: originalLocation.range, - selectionRange: originalLocation.range, - // We encode usr and uri for incoming/outgoing type lookups in the implementation-specific data field - data: .dictionary([ - "usr": .string(symbol.usr), - "uri": .string(originalLocation.uri.stringValue), - ]) - ) - let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) - typeHierarchyItems.append(remappedItem) + + let moduleName = info.location.moduleName.isEmpty ? nil : info.location.moduleName + guard let item = indexToLSPTypeHierarchyItem(definition: info, moduleName: moduleName, index: index) else { + continue + } + let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) + typeHierarchyItems.append(remappedItem) } typeHierarchyItems.sort(by: { $0.name < $1.name }) @@ -2754,10 +2715,10 @@ extension SourceKitLSPServer { name = "\(symbol.name): \(conformances.map(\.symbol.name).sorted().joined(separator: ", "))" } // Add the file name and line to the detail string - if let url = adjustedLocation.uri.fileURL, + if let url = remappedLocation.uri.fileURL, let basename = (try? AbsolutePath(validating: url.filePath))?.basename { - detail = "Extension at \(basename):\(adjustedLocation.range.lowerBound.line + 1)" + detail = "Extension at \(basename):\(remappedLocation.range.lowerBound.line + 1)" } else if !definition.location.moduleName.isEmpty { detail = "Extension in \(definition.location.moduleName)" } else { diff --git a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift index 640299749..0eb0c5c2d 100644 --- a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift +++ b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift @@ -120,16 +120,14 @@ class CopiedHeaderTests: SourceKitLSPTestCase { context: ReferencesContext(includeDeclaration: true) ) ) - response.sort() var expected = [ try project.location(from: "1️⃣", to: "1️⃣", in: "Test.h"), try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c"), ] - expected.sort() XCTAssertEqual(response, expected) } - func DISABLED_testFindImplementationInCopiedHeader() async throws { + func testFindImplementationInCopiedHeader() async throws { let project = try await CustomBuildServerTestProject( files: [ "Test.h": """