diff --git a/Contributor Documentation/BSP Extensions.md b/Contributor Documentation/BSP Extensions.md index cb28c6f3d..bb3cd71e2 100644 --- a/Contributor Documentation/BSP Extensions.md +++ b/Contributor Documentation/BSP Extensions.md @@ -95,6 +95,15 @@ export interface SourceKitSourceItemData { * `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`. */ outputPath?: string; + + /** + * If this source item gets copied to a different destination during preparation, the destinations it will be copied + * to. + * + * If a user action would jump to one of these copied files, this allows SourceKit-LSP to redirect the navigation to + * the original source file instead of jumping to a file in the build directory. + */ + copyDestinations?: DocumentURI[]; } ``` diff --git a/Contributor Documentation/LSP Extensions.md b/Contributor Documentation/LSP Extensions.md index f8d2e5b2c..0ecfb52cc 100644 --- a/Contributor Documentation/LSP Extensions.md +++ b/Contributor Documentation/LSP Extensions.md @@ -795,12 +795,20 @@ export interface SynchronizeParams { * This option is experimental, guarded behind the `synchronize-for-build-system-updates` experimental feature, and * may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it. */ - buildServerUpdates?: bool + buildServerUpdates?: bool; + + /** + * Wait for the build server to update its internal mapping of copied files to their original location. + * + * This option is experimental, guarded behind the `synchronize-copy-file-map` experimental feature, and may be + * modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it. + */ + copyFileMap?: bool; /** * Wait for background indexing to finish and all index unit files to be loaded into indexstore-db. */ - index?: bool + index?: bool; } ``` diff --git a/Sources/BuildServerIntegration/BuildServerManager.swift b/Sources/BuildServerIntegration/BuildServerManager.swift index 0f648180e..3c671b41d 100644 --- a/Sources/BuildServerIntegration/BuildServerManager.swift +++ b/Sources/BuildServerIntegration/BuildServerManager.swift @@ -75,6 +75,10 @@ package struct SourceFileInfo: Sendable { /// compiler arguments for these files to provide semantic editor functionality but we can't build them. package var isBuildable: Bool + /// If this source item gets copied to a different destination during preparation, the destinations it will be copied + /// to. + package var copyDestinations: Set + fileprivate func merging(_ other: SourceFileInfo?) -> SourceFileInfo { guard let other else { return self @@ -99,7 +103,8 @@ package struct SourceFileInfo: Sendable { targetsToOutputPath: mergedTargetsToOutputPaths, isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject, mayContainTests: other.mayContainTests || mayContainTests, - isBuildable: other.isBuildable || isBuildable + isBuildable: other.isBuildable || isBuildable, + copyDestinations: copyDestinations.union(other.copyDestinations) ) } } @@ -434,6 +439,18 @@ package actor BuildServerManager: QueueBasedMessageHandler { private let cachedSourceFilesAndDirectories = Cache() + /// The latest map of copied file URIs to their original source locations. + /// + /// We don't use a `Cache` for this because we can provide reasonable functionality even without or with an + /// out-of-date copied file map - in the worst case we jump to a file in the build directory instead of the source + /// directory. + /// We don't want to block requests like definition on receiving up-to-date index information from the build server. + private var cachedCopiedFileMap: [DocumentURI: DocumentURI] = [:] + + /// The latest task to update the `cachedCopiedFileMap`. This allows us to cancel previous tasks to update the copied + /// file map when a new update is requested. + private var copiedFileMapUpdateTask: Task? + /// The `SourceKitInitializeBuildResponseData` received from the `build/initialize` request, if any. package var initializationData: SourceKitInitializeBuildResponseData? { get async { @@ -660,6 +677,7 @@ package actor BuildServerManager: QueueBasedMessageHandler { return !updatedTargets.intersection(cacheKey.targets).isEmpty } self.cachedSourceFilesAndDirectories.clearAll(isolation: self) + self.scheduleRecomputeCopyFileMap() await delegate?.buildTargetsChanged(notification.changes) await filesBuildSettingsChangedDebouncer.scheduleCall(Set(watchedFiles.keys)) @@ -839,6 +857,43 @@ package actor BuildServerManager: QueueBasedMessageHandler { } } + /// Check if the URI referenced by `location` has been copied during the preparation phase. If so, adjust the URI to + /// the original source file. + package func locationAdjustedForCopiedFiles(_ location: Location) -> Location { + guard let originalUri = cachedCopiedFileMap[location.uri] else { + return location + } + // If we regularly get issues that the copied file is out-of-sync with its original, we can check that the contents + // of the lines touched by the location match and only return the original URI if they do. For now, we avoid this + // check due to its performance cost of reading files from disk. + return Location(uri: originalUri, range: location.range) + } + + /// Check if the URI referenced by `location` has been copied during the preparation phase. If so, adjust the URI to + /// the original source file. + package func locationsAdjustedForCopiedFiles(_ locations: [Location]) -> [Location] { + return locations.map { locationAdjustedForCopiedFiles($0) } + } + + @discardableResult + package func scheduleRecomputeCopyFileMap() -> Task { + let task = Task { [previousUpdateTask = copiedFileMapUpdateTask] in + previousUpdateTask?.cancel() + await orLog("Re-computing copy file map") { + let sourceFilesAndDirectories = try await self.sourceFilesAndDirectories() + var copiedFileMap: [DocumentURI: DocumentURI] = [:] + for (file, fileInfo) in sourceFilesAndDirectories.files { + for copyDestination in fileInfo.copyDestinations { + copiedFileMap[copyDestination] = file + } + } + self.cachedCopiedFileMap = copiedFileMap + } + } + copiedFileMapUpdateTask = task + return task + } + /// Returns all the targets that the document is part of. package func targets(for document: DocumentURI) async -> [BuildTargetIdentifier] { guard let targets = await sourceFileInfo(for: document)?.targets else { @@ -1292,7 +1347,8 @@ package actor BuildServerManager: QueueBasedMessageHandler { isPartOfRootProject: isPartOfRootProject, mayContainTests: mayContainTests, isBuildable: !(target?.tags.contains(.notBuildable) ?? false) - && (sourceKitData?.kind ?? .source) == .source + && (sourceKitData?.kind ?? .source) == .source, + copyDestinations: Set(sourceKitData?.copyDestinations ?? []) ) switch sourceItem.kind { case .file: diff --git a/Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift b/Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift index 6e952a5c7..ec3722b9f 100644 --- a/Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift +++ b/Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift @@ -157,10 +157,23 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable { /// `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`. public var outputPath: String? - public init(language: Language? = nil, kind: SourceKitSourceItemKind? = nil, outputPath: String? = nil) { + /// If this source item gets copied to a different destination during preparation, the destinations it will be copied + /// to. + /// + /// If a user action would jump to one of these copied files, this allows SourceKit-LSP to redirect the navigation to + /// the original source file instead of jumping to a file in the build directory. + public var copyDestinations: [DocumentURI]? + + public init( + language: Language? = nil, + kind: SourceKitSourceItemKind? = nil, + outputPath: String? = nil, + copyDestinations: [DocumentURI]? = nil + ) { self.language = language self.kind = kind self.outputPath = outputPath + self.copyDestinations = copyDestinations } public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) { @@ -177,6 +190,14 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable { if case .string(let outputFilePath) = dictionary[CodingKeys.outputPath.stringValue] { self.outputPath = outputFilePath } + if case .array(let copyDestinations) = dictionary[CodingKeys.copyDestinations.stringValue] { + self.copyDestinations = copyDestinations.compactMap { entry in + guard case .string(let copyDestination) = entry else { + return nil + } + return try? DocumentURI(string: copyDestination) + } + } } public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny { @@ -190,6 +211,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable { if let outputPath { result[CodingKeys.outputPath.stringValue] = .string(outputPath) } + if let copyDestinations { + result[CodingKeys.copyDestinations.stringValue] = .array(copyDestinations.map { .string($0.stringValue) }) + } return .dictionary(result) } } diff --git a/Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift b/Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift index 4d25ac467..33ba049bb 100644 --- a/Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift +++ b/Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift @@ -38,11 +38,18 @@ public struct SynchronizeRequest: RequestType { /// may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it. public var buildServerUpdates: Bool? + /// Wait for the build server to update its internal mapping of copied files to their original location. + /// + /// This option is experimental, guarded behind the `synchronize-copy-file-map` experimental feature, and may be + /// modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it. + public var copyFileMap: Bool? + /// Wait for background indexing to finish and all index unit files to be loaded into indexstore-db. public var index: Bool? - public init(buildServerUpdates: Bool? = nil, index: Bool? = nil) { + public init(buildServerUpdates: Bool? = nil, copyFileMap: Bool? = nil, index: Bool? = nil) { self.buildServerUpdates = buildServerUpdates + self.copyFileMap = copyFileMap self.index = index } } diff --git a/Sources/SKOptions/ExperimentalFeatures.swift b/Sources/SKOptions/ExperimentalFeatures.swift index 8eae864bc..455f5b516 100644 --- a/Sources/SKOptions/ExperimentalFeatures.swift +++ b/Sources/SKOptions/ExperimentalFeatures.swift @@ -45,6 +45,11 @@ public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable { /// - Note: Internal option, for testing only case synchronizeForBuildSystemUpdates = "synchronize-for-build-system-updates" + /// Enable the `copyFileMap` option in the `workspace/synchronize` request. + /// + /// - Note: Internal option, for testing only + case synchronizeCopyFileMap = "synchronize-copy-file-map" + /// All non-internal experimental features. public static var allNonInternalCases: [ExperimentalFeature] { allCases.filter { !$0.isInternal } @@ -67,6 +72,8 @@ public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable { return true case .synchronizeForBuildSystemUpdates: return true + case .synchronizeCopyFileMap: + return true } } } diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index d8466f73b..0f6434697 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -27,7 +27,7 @@ import XCTest extension SourceKitLSPOptions { package static func testDefault( backgroundIndexing: Bool = true, - experimentalFeatures: Set = [] + experimentalFeatures: Set = [.synchronizeCopyFileMap] ) async throws -> SourceKitLSPOptions { let pluginPaths = try await sourceKitPluginPaths return SourceKitLSPOptions( diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 58b5eb835..491ebed67 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -2128,7 +2128,8 @@ extension SourceKitLSPServer { return try await languageService.definition(req) } } - return .locations(indexBasedResponse) + let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(indexBasedResponse) + return .locations(remappedLocations) } /// Generate the generated interface for the given module, write it to disk and return the location to which to jump @@ -2632,6 +2633,9 @@ extension SourceKitLSPServer { if req.buildServerUpdates != nil, !self.options.hasExperimentalFeature(.synchronizeForBuildSystemUpdates) { throw ResponseError.unknown("\(SynchronizeRequest.method).buildServerUpdates is an experimental request option") } + if req.copyFileMap != nil, !self.options.hasExperimentalFeature(.synchronizeCopyFileMap) { + throw ResponseError.unknown("\(SynchronizeRequest.method).copyFileMap is an experimental request option") + } for workspace in workspaces { await workspace.synchronize(req) } diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index a6a9c67ac..0b0e1bf08 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -513,6 +513,18 @@ package final class Workspace: Sendable, BuildServerManagerDelegate { await buildServerManager.waitForUpToDateBuildGraph() await indexUnitOutputPathsUpdateQueue.async {}.value } + if request.copyFileMap ?? false { + // Not using `valuePropagatingCancellation` here because that could lead us to the following scenario: + // - An update of the copy file map is scheduled because of a change in the build graph + // - We get a synchronize request + // - Scheduling a new recomputation of the copy file map cancels the previous recomputation + // - We cancel the synchronize request, which would also cancel the copy file map recomputation, leaving us with + // an outdated version + // + // Technically, we might be doing unnecessary work here if the output file map is already up-to-date. But since + // this option is mostly intended for testing purposes, this is acceptable. + await buildServerManager.scheduleRecomputeCopyFileMap().value + } if request.index ?? false { await semanticIndexManager?.waitForUpToDateIndex() uncheckedIndex?.pollForUnitChangesAndWait() diff --git a/Tests/SourceKitLSPTests/DefinitionTests.swift b/Tests/SourceKitLSPTests/DefinitionTests.swift index 78ec580f4..f87213230 100644 --- a/Tests/SourceKitLSPTests/DefinitionTests.swift +++ b/Tests/SourceKitLSPTests/DefinitionTests.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import BuildServerIntegration +import BuildServerProtocol import LanguageServerProtocol @_spi(Testing) import SKLogging import SKTestSupport @@ -587,4 +589,104 @@ class DefinitionTests: XCTestCase { ) XCTAssertEqual(response?.locations, [Location(uri: project.fileURI, range: Range(project.positions["2️⃣"]))]) } + + func testJumpToCopiedHeader() async throws { + 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.appendingPathComponent("Test.c")), + kind: .file, + generated: false, + dataKind: .sourceKit, + data: SourceKitSourceItemData( + language: .c, + kind: .source, + outputPath: nil, + copyDestinations: nil + ).encodeToLSPAny() + ), + SourceItem( + uri: DocumentURI(projectRoot.appendingPathComponent("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() + } + } + + let project = try await CustomBuildServerTestProject( + files: [ + "Test.h": """ + void hello(); + """, + "Test.c": """ + #include + + void test() { + 1️⃣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( + DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"]) + ) + XCTAssertEqual(response?.locations?.map(\.uri), [try project.uri(for: "Test.h")]) + } }