From 70d900e799c731ca559113a6d1fb0c0531f91489 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Tue, 9 Dec 2025 12:40:55 +0530 Subject: [PATCH 1/8] 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/8] 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": """ From 2d8c99371293c4a37b599cecb8994bd393698f85 Mon Sep 17 00:00:00 2001 From: loveucifer Date: Tue, 16 Dec 2025 08:11:38 +0530 Subject: [PATCH 3/8] Address remaining code review feedback - Refactor supertypes/subtypes to use indexToLSPTypeHierarchyItem helper instead of duplicating ~80 lines of TypeHierarchyItem creation code - Remove unused workaround helper functions (indexToLSPLocation2, indexToLSPTypeHierarchyItem2) - Fix test ordering: use deterministic sorted order instead of Set comparison - Enable testFindImplementationInCopiedHeader test - Add implementation request support for C/C++/ObjC functions with separate declaration and definition (finds definition when declarations exist without definitions at the same location) - Fix whitespace/indentation issues --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 158 +++--------------- .../SourceKitLSPTests/CopiedHeaderTests.swift | 14 +- 2 files changed, 34 insertions(+), 138 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 5b17e1435..2a8732cf1 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2201,6 +2201,22 @@ extension SourceKitLSPServer { if occurrences.isEmpty { occurrences = index.occurrences(relatedToUSR: usr, roles: .overrideOf) } + // for C/C++/objC functions with separate declaration and definition, + // "implementation" means the definition. Only use this fallback if there's + // a declaration without a definition at the same location. + if occurrences.isEmpty { + let declarations = index.occurrences(ofUSR: usr, roles: .declaration) + let definitions = index.occurrences(ofUSR: usr, roles: .definition) + // check if there are declarations that don't have a definition at the same location + let hasDeclarationOnlyLocations = declarations.contains { decl in + !definitions.contains { def in + def.location.path == decl.location.path && def.location.line == decl.location.line + } + } + if hasDeclarationOnlyLocations { + occurrences = definitions + } + } return occurrences.compactMap { indexToLSPLocation($0.location) } } @@ -2407,7 +2423,9 @@ extension SourceKitLSPServer { guard let originalDefinitionLocation = definitionLocation else { continue } - let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalDefinitionLocation) + let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles( + originalDefinitionLocation + ) // Create a new CallHierarchyItem with the remapped location let name = index.fullyQualifiedName(of: definition) @@ -2477,7 +2495,9 @@ extension SourceKitLSPServer { guard let originalDefinitionLocation = definitionLocation else { continue } - let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles(originalDefinitionLocation) + let remappedDefinitionLocation = await workspace.buildServerManager.locationAdjustedForCopiedFiles( + originalDefinitionLocation + ) // Create a new CallHierarchyItem with the remapped location let name = index.fullyQualifiedName(of: definition) @@ -2605,7 +2625,7 @@ extension SourceKitLSPServer { guard location != nil else { continue } - + let moduleName = info.location.moduleName.isEmpty ? nil : info.location.moduleName guard let item = indexToLSPTypeHierarchyItem(definition: info, moduleName: moduleName, index: index) else { continue @@ -2668,24 +2688,6 @@ extension SourceKitLSPServer { return index.occurrences(relatedToUSR: related.symbol.usr, roles: .baseOf) } - // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed - func indexToLSPLocation2(_ location: SymbolLocation) -> Location? { - return self.indexToLSPLocation(location) - } - - // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed - func indexToLSPTypeHierarchyItem2( - definition: SymbolOccurrence, - moduleName: String?, - index: CheckedIndex - ) -> TypeHierarchyItem? { - return self.indexToLSPTypeHierarchyItem( - definition: definition, - moduleName: moduleName, - index: index - ) - } - // Convert occurrences to type hierarchy items let occurs = baseOccurs + retroactiveConformanceOccurs var types: [TypeHierarchyItem] = [] @@ -2695,54 +2697,10 @@ extension SourceKitLSPServer { continue } - let location = indexToLSPLocation2(definition.location) - guard let originalLocation = location else { + let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName + guard let item = indexToLSPTypeHierarchyItem(definition: definition, moduleName: moduleName, index: index) 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 = 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" - } - 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) } @@ -2760,24 +2718,6 @@ extension SourceKitLSPServer { // Resolve child types and extensions let occurs = index.occurrences(ofUSR: data.usr, roles: [.baseOf, .extendedBy]) - // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed - func indexToLSPLocation2(_ location: SymbolLocation) -> Location? { - return self.indexToLSPLocation(location) - } - - // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed - func indexToLSPTypeHierarchyItem2( - definition: SymbolOccurrence, - moduleName: String?, - index: CheckedIndex - ) -> TypeHierarchyItem? { - return self.indexToLSPTypeHierarchyItem( - definition: definition, - moduleName: moduleName, - index: index - ) - } - // Convert occurrences to type hierarchy items var types: [TypeHierarchyItem] = [] for occurrence in occurs { @@ -2796,54 +2736,10 @@ extension SourceKitLSPServer { continue } - let location = indexToLSPLocation2(definition.location) - guard let originalLocation = location else { + let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName + guard let item = indexToLSPTypeHierarchyItem(definition: definition, moduleName: moduleName, index: index) 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) } diff --git a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift index 0eb0c5c2d..bae9fb3f9 100644 --- a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift +++ b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift @@ -113,16 +113,16 @@ class CopiedHeaderTests: SourceKitLSPTestCase { try await project.testClient.send(SynchronizeRequest(copyFileMap: true)) let (uri, positions) = try project.openDocument("Test.c") - var response = try await project.testClient.send( + let response = try await project.testClient.send( ReferencesRequest( textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"], context: ReferencesContext(includeDeclaration: true) ) ) - var expected = [ - try project.location(from: "1️⃣", to: "1️⃣", in: "Test.h"), + let expected = [ try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c"), + try project.location(from: "1️⃣", to: "1️⃣", in: "Test.h"), ] XCTAssertEqual(response, expected) } @@ -158,7 +158,7 @@ class CopiedHeaderTests: SourceKitLSPTestCase { XCTAssertEqual( response?.locations, [ - try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c"), + try project.location(from: "2️⃣", to: "2️⃣", in: "Test.c") ] ) } @@ -194,7 +194,7 @@ class CopiedHeaderTests: SourceKitLSPTestCase { XCTAssertEqual( response?.locations, [ - try project.location(from: "1️⃣", to: "2️⃣", in: "Test.h"), + try project.location(from: "1️⃣", to: "2️⃣", in: "Test.h") ] ) } @@ -203,7 +203,7 @@ class CopiedHeaderTests: SourceKitLSPTestCase { let project = try await CustomBuildServerTestProject( files: [ "Test.h": """ - void 1️⃣hello2️⃣(); + void 1️⃣hello(); """, "Test.c": """ #include @@ -227,6 +227,6 @@ class CopiedHeaderTests: SourceKitLSPTestCase { XCTFail("Expected a symbol information") return } - XCTAssertEqual(info.location, try project.location(from: "1️⃣", to: "2️⃣", in: "Test.h")) + XCTAssertEqual(info.location, try project.location(from: "1️⃣", to: "1️⃣", in: "Test.h")) } } From 71374c1a041f918431d5c921e944fcc5d6be895a Mon Sep 17 00:00:00 2001 From: loveucifer <134506987+loveucifer@users.noreply.github.com> Date: Tue, 16 Dec 2025 08:15:41 +0530 Subject: [PATCH 4/8] Update Sources/SourceKitLSP/SourceKitLSPServer.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 2a8732cf1..0fd040e1d 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2550,8 +2550,8 @@ extension SourceKitLSPServer { let basename = (try? AbsolutePath(validating: url.filePath))?.basename { detail = "Extension at \(basename):\(location.range.lowerBound.line + 1)" - } else if !definition.location.moduleName.isEmpty { - detail = "Extension in \(definition.location.moduleName)" + } else if let moduleName = moduleName, !moduleName.isEmpty { + detail = "Extension in \(moduleName)" } else { detail = "Extension" } From d254ce1b5df998275f12953b3a623e33e1ed7bdd Mon Sep 17 00:00:00 2001 From: loveucifer <134506987+loveucifer@users.noreply.github.com> Date: Tue, 16 Dec 2025 08:16:07 +0530 Subject: [PATCH 5/8] Update Sources/SourceKitLSP/SourceKitLSPServer.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 0fd040e1d..b296ac0be 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2621,8 +2621,7 @@ extension SourceKitLSPServer { break } - let location = indexToLSPLocation(info.location) - guard location != nil else { + guard indexToLSPLocation(info.location) != nil else { continue } From 21adc24144b6c94e46379d63661a020c38180ee6 Mon Sep 17 00:00:00 2001 From: loveucifer <134506987+loveucifer@users.noreply.github.com> Date: Wed, 17 Dec 2025 19:57:52 +0530 Subject: [PATCH 6/8] some more fixes --- .../BuildServerManager.swift | 55 ++++++-- Sources/SourceKitLSP/SourceKitLSPServer.swift | 124 ++++-------------- 2 files changed, 69 insertions(+), 110 deletions(-) diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index 2ac354636..408011481 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -1005,7 +1005,9 @@ package actor BuildServerManager: QueueBasedMessageHandler { return edit } - package func locationsOrLocationLinksAdjustedForCopiedFiles(_ response: LocationsOrLocationLinksResponse?) -> LocationsOrLocationLinksResponse? { + package func locationsOrLocationLinksAdjustedForCopiedFiles( + _ response: LocationsOrLocationLinksResponse? + ) -> LocationsOrLocationLinksResponse? { guard let response = response else { return nil } @@ -1016,14 +1018,20 @@ package actor BuildServerManager: QueueBasedMessageHandler { case .locationLinks(let locationLinks): var remappedLinks: [LocationLink] = [] for link in locationLinks { - 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: adjustedTargetLocation.uri, - targetRange: adjustedTargetLocation.range, - targetSelectionRange: adjustedTargetSelectionLocation.range - )) + 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: adjustedTargetLocation.uri, + targetRange: adjustedTargetLocation.range, + targetSelectionRange: adjustedTargetSelectionLocation.range + ) + ) } return .locationLinks(remappedLinks) } @@ -1031,7 +1039,9 @@ package actor BuildServerManager: QueueBasedMessageHandler { 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)) + let adjustedSelectionLocation = self.locationAdjustedForCopiedFiles( + Location(uri: item.uri, range: item.selectionRange) + ) return TypeHierarchyItem( name: item.name, kind: item.kind, @@ -1044,6 +1054,31 @@ package actor BuildServerManager: QueueBasedMessageHandler { ) } + package func callHierarchyItemAdjustedForCopiedFiles(_ item: CallHierarchyItem) -> CallHierarchyItem { + let adjustedLocation = self.locationAdjustedForCopiedFiles(Location(uri: item.uri, range: item.range)) + let adjustedSelectionLocation = self.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), + ]) + ) + } + @discardableResult package func scheduleRecomputeCopyFileMap() -> Task { let task = Task { [previousUpdateTask = copiedFileMapUpdateTask] in diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index b296ac0be..db46fd355 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, workspace: Workspace)] = [] + var symbolsIndexAndLocations: [(symbol: SymbolOccurrence, index: CheckedIndex, location: Location)] = [] for workspace in workspaces { guard let index = await workspace.index(checkedFor: .deletedFiles) else { continue @@ -1689,23 +1689,24 @@ extension SourceKitLSPServer { return true } try Task.checkCancellation() - symbolsAndIndex += symbolOccurrences.map { - return ($0, index, workspace) - } - } - 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 - // file. In practice they almost always coincide, so we accept the incorrectness here to avoid the file read. - utf16index: symbolOccurrence.location.utf8Column - 1 - ) - let symbolLocation = Location( - uri: symbolOccurrence.location.documentUri, - range: Range(symbolPosition) - ) + // Batch all locations for this workspace and remap them in one call + let locations = symbolOccurrences.map { symbolOccurrence -> Location 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 + // file. In practice they almost always coincide, so we accept the incorrectness here to avoid the file read. + utf16index: symbolOccurrence.location.utf8Column - 1 + ) + return Location(uri: symbolOccurrence.location.documentUri, range: Range(symbolPosition)) + } + let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) + for (symbolOccurrence, remappedLocation) in zip(symbolOccurrences, remappedLocations) { + symbolsIndexAndLocations.append((symbolOccurrence, index, remappedLocation)) + } + } + return symbolsIndexAndLocations.sorted(by: { $0.symbol < $1.symbol }).map { symbolOccurrence, index, location in let containerNames = index.containerNames(of: symbolOccurrence) let containerName: String? if containerNames.isEmpty { @@ -1717,13 +1718,12 @@ 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: remappedLocation, + location: location, containerName: containerName ) ) @@ -2276,36 +2276,6 @@ 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, @@ -2333,7 +2303,7 @@ extension SourceKitLSPServer { guard let item = indexToLSPCallHierarchyItem(definition: definition, index: index) else { continue } - let adjustedItem = await callHierarchyItemAdjustedForCopiedFiles(item, workspace: workspace) + let adjustedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) callHierarchyItems.append(adjustedItem) } callHierarchyItems.sort(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) }) @@ -2384,7 +2354,7 @@ extension SourceKitLSPServer { var callersToCalls: [Symbol: [SymbolOccurrence]] = [:] for call in callOccurrences { - // Callers are all `containedBy` 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 { @@ -2418,33 +2388,10 @@ extension SourceKitLSPServer { continue } - // Now we need to get the remapped location for the definition item itself - let definitionLocation = indexToLSPLocation2(definition.location) - guard let originalDefinitionLocation = definitionLocation else { + guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) 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), - ]) - ) + let remappedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) calls.append(CallHierarchyIncomingCall(from: remappedItem, fromRanges: remappedLocations.map(\.range))) } @@ -2490,33 +2437,10 @@ extension SourceKitLSPServer { continue } - // Get the remapped location for the definition item itself - let definitionLocation = indexToLSPLocation2(definition.location) - guard let originalDefinitionLocation = definitionLocation else { + guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) 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), - ]) - ) + let remappedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) calls.append(CallHierarchyOutgoingCall(to: remappedItem, fromRanges: [remappedLocation.range])) } From f3237fad7466dfdd343dde14bfcf0d99c7fefa23 Mon Sep 17 00:00:00 2001 From: loveucifer <134506987+loveucifer@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:03:50 +0530 Subject: [PATCH 7/8] more and more fixes --- Sources/SourceKitLSP/SourceKitLSPServer.swift | 189 +++++++++++------- 1 file changed, 113 insertions(+), 76 deletions(-) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index db46fd355..80055d846 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -1666,12 +1666,11 @@ extension SourceKitLSPServer { guard req.query.count >= minWorkspaceSymbolPatternLength else { return [] } - var symbolsIndexAndLocations: [(symbol: SymbolOccurrence, index: CheckedIndex, location: Location)] = [] + var symbolsIndexAndWorkspaces: [(symbol: SymbolOccurrence, index: CheckedIndex, workspace: Workspace)] = [] for workspace in workspaces { guard let index = await workspace.index(checkedFor: .deletedFiles) else { continue } - var symbolOccurrences: [SymbolOccurrence] = [] index.forEachCanonicalSymbolOccurrence( containing: req.query, anchorStart: false, @@ -1685,28 +1684,23 @@ extension SourceKitLSPServer { guard !symbol.location.isSystem && !symbol.roles.contains(.accessorOf) else { return true } - symbolOccurrences.append(symbol) + symbolsIndexAndWorkspaces.append((symbol, index, workspace)) return true } try Task.checkCancellation() + } - // Batch all locations for this workspace and remap them in one call - let locations = symbolOccurrences.map { symbolOccurrence -> Location 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 - // file. In practice they almost always coincide, so we accept the incorrectness here to avoid the file read. - utf16index: symbolOccurrence.location.utf8Column - 1 - ) - return Location(uri: symbolOccurrence.location.documentUri, range: Range(symbolPosition)) - } - let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) + var result: [WorkspaceSymbolItem] = [] + for (symbolOccurrence, index, workspace) in symbolsIndexAndWorkspaces.sorted(by: { $0.symbol < $1.symbol }) { + 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 + // file. In practice they almost always coincide, so we accept the incorrectness here to avoid the file read. + utf16index: symbolOccurrence.location.utf8Column - 1 + ) + let symbolLocation = Location(uri: symbolOccurrence.location.documentUri, range: Range(symbolPosition)) + let location = await workspace.buildServerManager.locationAdjustedForCopiedFiles(symbolLocation) - for (symbolOccurrence, remappedLocation) in zip(symbolOccurrences, remappedLocations) { - symbolsIndexAndLocations.append((symbolOccurrence, index, remappedLocation)) - } - } - return symbolsIndexAndLocations.sorted(by: { $0.symbol < $1.symbol }).map { symbolOccurrence, index, location in let containerNames = index.containerNames(of: symbolOccurrence) let containerName: String? if containerNames.isEmpty { @@ -1718,16 +1712,19 @@ extension SourceKitLSPServer { } } - return WorkspaceSymbolItem.symbolInformation( - SymbolInformation( - name: symbolOccurrence.symbol.name, - kind: symbolOccurrence.symbol.kind.asLspSymbolKind(), - deprecated: nil, - location: location, - containerName: containerName + result.append( + WorkspaceSymbolItem.symbolInformation( + SymbolInformation( + name: symbolOccurrence.symbol.name, + kind: symbolOccurrence.symbol.kind.asLspSymbolKind(), + deprecated: nil, + location: location, + containerName: containerName + ) ) ) } + return result } /// Forwards a SymbolInfoRequest to the appropriate toolchain service for this document. @@ -2293,20 +2290,25 @@ extension SourceKitLSPServer { // For call hierarchy preparation we only locate the definition let usrs = symbols.compactMap(\.usr) + // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed + func indexToLSPCallHierarchyItem2( + definition: SymbolOccurrence, + index: CheckedIndex + ) -> CallHierarchyItem? { + return self.indexToLSPCallHierarchyItem(definition: definition, index: index) + } + // 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. - var callHierarchyItems: [CallHierarchyItem] = [] - for usr in usrs { + let callHierarchyItems = await usrs.asyncCompactMap { (usr) -> CallHierarchyItem? in guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else { - continue + return nil } - guard let item = indexToLSPCallHierarchyItem(definition: definition, index: index) else { - continue + guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else { + return nil } - let adjustedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) - callHierarchyItems.append(adjustedItem) - } - callHierarchyItems.sort(by: { Location(uri: $0.uri, range: $0.range) < Location(uri: $1.uri, range: $1.range) }) + return await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) + }.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, // failing with `Cannot read properties of undefined (reading 'map')`. Pick the first one. @@ -2375,25 +2377,24 @@ extension SourceKitLSPServer { return self.indexToLSPCallHierarchyItem(definition: definition, index: index) } - var calls: [CallHierarchyIncomingCall] = [] - for (caller, callsList) in callersToCalls { + let calls = await callersToCalls.asyncCompactMap { (caller, callsList) -> CallHierarchyIncomingCall? in // Resolve the caller's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: caller.usr) else { - continue + return nil } let locations = callsList.compactMap { indexToLSPLocation2($0.location) }.sorted() let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(locations) guard !remappedLocations.isEmpty else { - continue + return nil } guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else { - continue + return nil } let remappedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) - calls.append(CallHierarchyIncomingCall(from: remappedItem, fromRanges: remappedLocations.map(\.range))) + return CallHierarchyIncomingCall(from: remappedItem, fromRanges: remappedLocations.map(\.range)) } return calls.sorted(by: { $0.from.name < $1.from.name }) } @@ -2422,27 +2423,26 @@ 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) - var calls: [CallHierarchyOutgoingCall] = [] - for occurrence in callOccurrences { + let calls = await callOccurrences.asyncCompactMap { occurrence -> CallHierarchyOutgoingCall? in guard occurrence.symbol.kind.isCallable else { - continue + return nil } guard let location = indexToLSPLocation2(occurrence.location) else { - continue + return nil } 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 { - continue + return nil } guard let item = indexToLSPCallHierarchyItem2(definition: definition, index: index) else { - continue + return nil } let remappedItem = await workspace.buildServerManager.callHierarchyItemAdjustedForCopiedFiles(item) - calls.append(CallHierarchyOutgoingCall(to: remappedItem, fromRanges: [remappedLocation.range])) + return CallHierarchyOutgoingCall(to: remappedItem, fromRanges: [remappedLocation.range]) } return calls.sorted(by: { $0.to.name < $1.to.name }) } @@ -2474,7 +2474,7 @@ extension SourceKitLSPServer { let basename = (try? AbsolutePath(validating: url.filePath))?.basename { detail = "Extension at \(basename):\(location.range.lowerBound.line + 1)" - } else if let moduleName = moduleName, !moduleName.isEmpty { + } else if let moduleName = moduleName { detail = "Extension in \(moduleName)" } else { detail = "Extension" @@ -2528,10 +2528,27 @@ extension SourceKitLSPServer { } }.compactMap(\.usr) - var typeHierarchyItems: [TypeHierarchyItem] = [] - for usr in usrs { + // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed + func indexToLSPLocation2(_ location: SymbolLocation) -> Location? { + return self.indexToLSPLocation(location) + } + + // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed + func indexToLSPTypeHierarchyItem2( + definition: SymbolOccurrence, + moduleName: String?, + index: CheckedIndex + ) -> TypeHierarchyItem? { + return self.indexToLSPTypeHierarchyItem( + definition: definition, + moduleName: moduleName, + index: index + ) + } + + let typeHierarchyItems = await usrs.asyncCompactMap { (usr) -> TypeHierarchyItem? in guard let info = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: usr) else { - continue + return nil } // 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. @@ -2539,24 +2556,22 @@ extension SourceKitLSPServer { case .unknown, .macro, .function, .variable, .field, .enumConstant, .instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor, .destructor, .conversionFunction, .parameter, .concept, .commentTag: - continue + return nil case .module, .namespace, .namespaceAlias, .enum, .struct, .class, .protocol, .extension, .union, .typealias, .using: break } - guard indexToLSPLocation(info.location) != nil else { - continue + guard indexToLSPLocation2(info.location) != nil else { + return nil } let moduleName = info.location.moduleName.isEmpty ? nil : info.location.moduleName - guard let item = indexToLSPTypeHierarchyItem(definition: info, moduleName: moduleName, index: index) else { - continue + guard let item = indexToLSPTypeHierarchyItem2(definition: info, moduleName: moduleName, index: index) else { + return nil } - let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) - typeHierarchyItems.append(remappedItem) - } - typeHierarchyItems.sort(by: { $0.name < $1.name }) + return await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) + }.sorted(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 @@ -2611,21 +2626,32 @@ extension SourceKitLSPServer { return index.occurrences(relatedToUSR: related.symbol.usr, roles: .baseOf) } + // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed + func indexToLSPTypeHierarchyItem2( + definition: SymbolOccurrence, + moduleName: String?, + index: CheckedIndex + ) -> TypeHierarchyItem? { + return self.indexToLSPTypeHierarchyItem( + definition: definition, + moduleName: moduleName, + index: index + ) + } + // Convert occurrences to type hierarchy items let occurs = baseOccurs + retroactiveConformanceOccurs - var types: [TypeHierarchyItem] = [] - for occurrence in occurs { + let types = await occurs.asyncCompactMap { occurrence -> TypeHierarchyItem? in // Resolve the supertype's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: occurrence.symbol.usr) else { - continue + return nil } let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName - guard let item = indexToLSPTypeHierarchyItem(definition: definition, moduleName: moduleName, index: index) else { - continue + guard let item = indexToLSPTypeHierarchyItem2(definition: definition, moduleName: moduleName, index: index) else { + return nil } - let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) - types.append(remappedItem) + return await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) } return types.sorted(by: { $0.name < $1.name }) } @@ -2641,9 +2667,21 @@ extension SourceKitLSPServer { // Resolve child types and extensions let occurs = index.occurrences(ofUSR: data.usr, roles: [.baseOf, .extendedBy]) + // TODO: Remove this workaround once https://github.com/swiftlang/swift/issues/75600 is fixed + func indexToLSPTypeHierarchyItem2( + definition: SymbolOccurrence, + moduleName: String?, + index: CheckedIndex + ) -> TypeHierarchyItem? { + return self.indexToLSPTypeHierarchyItem( + definition: definition, + moduleName: moduleName, + index: index + ) + } + // Convert occurrences to type hierarchy items - var types: [TypeHierarchyItem] = [] - for occurrence in occurs { + let types = await occurs.asyncCompactMap { occurrence -> TypeHierarchyItem? in 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 @@ -2651,20 +2689,19 @@ extension SourceKitLSPServer { logger.fault("Expected at most extendedBy or baseOf relation but got \(occurrence.relations.count)") } guard let related = occurrence.relations.sorted().first else { - continue + return nil } // Resolve the subtype's definition to find its location guard let definition = index.primaryDefinitionOrDeclarationOccurrence(ofUSR: related.symbol.usr) else { - continue + return nil } let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName - guard let item = indexToLSPTypeHierarchyItem(definition: definition, moduleName: moduleName, index: index) else { - continue + guard let item = indexToLSPTypeHierarchyItem2(definition: definition, moduleName: moduleName, index: index) else { + return nil } - let remappedItem = await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) - types.append(remappedItem) + return await workspace.buildServerManager.typeHierarchyItemAdjustedForCopiedFiles(item) } return types.sorted { $0.name < $1.name } } From 29b1f4c449a6c9eb44b1692097692583d591f4f4 Mon Sep 17 00:00:00 2001 From: loveucifer <134506987+loveucifer@users.noreply.github.com> Date: Fri, 19 Dec 2025 19:52:17 +0530 Subject: [PATCH 8/8] few more fixes --- .../BuildServerManager.swift | 27 +++++------- Sources/SourceKitLSP/SourceKitLSPServer.swift | 43 ++++++------------- .../SourceKitLSPTests/CopiedHeaderTests.swift | 36 ---------------- 3 files changed, 23 insertions(+), 83 deletions(-) diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index 408011481..35784efa4 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -982,25 +982,23 @@ package actor BuildServerManager: QueueBasedMessageHandler { edit.changes = newChanges } if let documentChanges = edit.documentChanges { - var newDocumentChanges: [WorkspaceEditDocumentChange] = [] - for change in documentChanges { + edit.documentChanges = documentChanges.map { change in switch change { case .textDocumentEdit(var textEdit): textEdit.textDocument.uri = self.uriAdjustedForCopiedFiles(textEdit.textDocument.uri) - newDocumentChanges.append(.textDocumentEdit(textEdit)) + return .textDocumentEdit(textEdit) case .createFile(var create): create.uri = self.uriAdjustedForCopiedFiles(create.uri) - newDocumentChanges.append(.createFile(create)) + return .createFile(create) case .renameFile(var rename): rename.oldUri = self.uriAdjustedForCopiedFiles(rename.oldUri) rename.newUri = self.uriAdjustedForCopiedFiles(rename.newUri) - newDocumentChanges.append(.renameFile(rename)) + return .renameFile(rename) case .deleteFile(var delete): delete.uri = self.uriAdjustedForCopiedFiles(delete.uri) - newDocumentChanges.append(.deleteFile(delete)) + return .deleteFile(delete) } } - edit.documentChanges = newDocumentChanges } return edit } @@ -1016,21 +1014,18 @@ package actor BuildServerManager: QueueBasedMessageHandler { let remappedLocations = self.locationsAdjustedForCopiedFiles(locations) return .locations(remappedLocations) case .locationLinks(let locationLinks): - var remappedLinks: [LocationLink] = [] - for link in locationLinks { + let remappedLinks = locationLinks.map { link -> LocationLink in 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: adjustedTargetLocation.uri, - targetRange: adjustedTargetLocation.range, - targetSelectionRange: adjustedTargetSelectionLocation.range - ) + return LocationLink( + originSelectionRange: link.originSelectionRange, + targetUri: adjustedTargetLocation.uri, + targetRange: adjustedTargetLocation.range, + targetSelectionRange: adjustedTargetSelectionLocation.range ) } return .locationLinks(remappedLinks) diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 80055d846..81b9ff08a 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -1690,8 +1690,8 @@ extension SourceKitLSPServer { try Task.checkCancellation() } - var result: [WorkspaceSymbolItem] = [] - for (symbolOccurrence, index, workspace) in symbolsIndexAndWorkspaces.sorted(by: { $0.symbol < $1.symbol }) { + return await symbolsIndexAndWorkspaces.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 @@ -1712,19 +1712,16 @@ extension SourceKitLSPServer { } } - result.append( - WorkspaceSymbolItem.symbolInformation( - SymbolInformation( - name: symbolOccurrence.symbol.name, - kind: symbolOccurrence.symbol.kind.asLspSymbolKind(), - deprecated: nil, - location: location, - containerName: containerName - ) + return WorkspaceSymbolItem.symbolInformation( + SymbolInformation( + name: symbolOccurrence.symbol.name, + kind: symbolOccurrence.symbol.kind.asLspSymbolKind(), + deprecated: nil, + location: location, + containerName: containerName ) ) } - return result } /// Forwards a SymbolInfoRequest to the appropriate toolchain service for this document. @@ -2198,22 +2195,6 @@ extension SourceKitLSPServer { if occurrences.isEmpty { occurrences = index.occurrences(relatedToUSR: usr, roles: .overrideOf) } - // for C/C++/objC functions with separate declaration and definition, - // "implementation" means the definition. Only use this fallback if there's - // a declaration without a definition at the same location. - if occurrences.isEmpty { - let declarations = index.occurrences(ofUSR: usr, roles: .declaration) - let definitions = index.occurrences(ofUSR: usr, roles: .definition) - // check if there are declarations that don't have a definition at the same location - let hasDeclarationOnlyLocations = declarations.contains { decl in - !definitions.contains { def in - def.location.path == decl.location.path && def.location.line == decl.location.line - } - } - if hasDeclarationOnlyLocations { - occurrences = definitions - } - } return occurrences.compactMap { indexToLSPLocation($0.location) } } @@ -2566,7 +2547,7 @@ extension SourceKitLSPServer { return nil } - let moduleName = info.location.moduleName.isEmpty ? nil : info.location.moduleName + let moduleName = info.location.moduleName guard let item = indexToLSPTypeHierarchyItem2(definition: info, moduleName: moduleName, index: index) else { return nil } @@ -2647,7 +2628,7 @@ extension SourceKitLSPServer { return nil } - let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName + let moduleName = definition.location.moduleName guard let item = indexToLSPTypeHierarchyItem2(definition: definition, moduleName: moduleName, index: index) else { return nil } @@ -2697,7 +2678,7 @@ extension SourceKitLSPServer { return nil } - let moduleName = definition.location.moduleName.isEmpty ? nil : definition.location.moduleName + let moduleName = definition.location.moduleName guard let item = indexToLSPTypeHierarchyItem2(definition: definition, moduleName: moduleName, index: index) else { return nil } diff --git a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift index bae9fb3f9..125ae6152 100644 --- a/Tests/SourceKitLSPTests/CopiedHeaderTests.swift +++ b/Tests/SourceKitLSPTests/CopiedHeaderTests.swift @@ -127,42 +127,6 @@ class CopiedHeaderTests: SourceKitLSPTestCase { XCTAssertEqual(response, expected) } - func 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: [