Skip to content

Commit 882c990

Browse files
committed
Add support for copied header files to SourceKit-LSP
If a build server copies files (eg. header) to the build directory during preparation and those copied files are referenced for semantic functionality, we would currently jump to the file in the build directory. Teach SourceKit-LSP about files that are copied during preparation and if we detect that we are jumping to such a file, jump to the original file instead. So far only the definition request checks the copied file paths. Adding support for copied file paths in the other requests will be a follow-up change.
1 parent 47ca76b commit 882c990

File tree

10 files changed

+237
-8
lines changed

10 files changed

+237
-8
lines changed

Contributor Documentation/BSP Extensions.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,15 @@ export interface SourceKitSourceItemData {
9595
* `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
9696
*/
9797
outputPath?: string;
98+
99+
/**
100+
* If this source item gets copied to a different destination during preparation, the destinations it will be copied
101+
* to.
102+
*
103+
* If a user action would jump to one of these copied files, this allows SourceKit-LSP to redirect the navigation to
104+
* the original source file instead of jumping to a file in the build directory.
105+
*/
106+
copyDestinations?: DocumentURI[];
98107
}
99108
```
100109

Contributor Documentation/LSP Extensions.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,12 +795,20 @@ export interface SynchronizeParams {
795795
* This option is experimental, guarded behind the `synchronize-for-build-system-updates` experimental feature, and
796796
* may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
797797
*/
798-
buildServerUpdates?: bool
798+
buildServerUpdates?: bool;
799+
800+
/**
801+
* Wait for the build server to update its internal mapping of copied files to their original location.
802+
*
803+
* This option is experimental, guarded behind the `synchronize-copy-file-map` experimental feature, and may be
804+
* modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
805+
*/
806+
copyFileMap?: bool;
799807

800808
/**
801809
* Wait for background indexing to finish and all index unit files to be loaded into indexstore-db.
802810
*/
803-
index?: bool
811+
index?: bool;
804812
}
805813
```
806814

Sources/BuildServerIntegration/BuildServerManager.swift

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ package struct SourceFileInfo: Sendable {
7575
/// compiler arguments for these files to provide semantic editor functionality but we can't build them.
7676
package var isBuildable: Bool
7777

78+
/// If this source item gets copied to a different destination during preparation, the destinations it will be copied
79+
/// to.
80+
package var copyDestinations: Set<DocumentURI>
81+
7882
fileprivate func merging(_ other: SourceFileInfo?) -> SourceFileInfo {
7983
guard let other else {
8084
return self
@@ -99,7 +103,8 @@ package struct SourceFileInfo: Sendable {
99103
targetsToOutputPath: mergedTargetsToOutputPaths,
100104
isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject,
101105
mayContainTests: other.mayContainTests || mayContainTests,
102-
isBuildable: other.isBuildable || isBuildable
106+
isBuildable: other.isBuildable || isBuildable,
107+
copyDestinations: copyDestinations.union(other.copyDestinations)
103108
)
104109
}
105110
}
@@ -434,6 +439,18 @@ package actor BuildServerManager: QueueBasedMessageHandler {
434439

435440
private let cachedSourceFilesAndDirectories = Cache<SourceFilesAndDirectoriesKey, SourceFilesAndDirectories>()
436441

442+
/// The latest map of copied file URIs to their original source locations.
443+
///
444+
/// We don't use a `Cache` for this because we can provide reasonable functionality even without or with an
445+
/// out-of-date copied file map - in the worst case we jump to a file in the build directory instead of the source
446+
/// directory.
447+
/// We don't want to block requests like definition on receiving up-to-date index information from the build server.
448+
private var cachedCopiedFileMap: [DocumentURI: DocumentURI] = [:]
449+
450+
/// The latest task to update the `cachedCopiedFileMap`. This allows us to cancel previous tasks to update the copied
451+
/// file map when a new update is requested.
452+
private var copiedFileMapUpdateTask: Task<Void, Never>?
453+
437454
/// The `SourceKitInitializeBuildResponseData` received from the `build/initialize` request, if any.
438455
package var initializationData: SourceKitInitializeBuildResponseData? {
439456
get async {
@@ -660,6 +677,7 @@ package actor BuildServerManager: QueueBasedMessageHandler {
660677
return !updatedTargets.intersection(cacheKey.targets).isEmpty
661678
}
662679
self.cachedSourceFilesAndDirectories.clearAll(isolation: self)
680+
self.scheduleRecomputeCopyFileMap()
663681

664682
await delegate?.buildTargetsChanged(notification.changes)
665683
await filesBuildSettingsChangedDebouncer.scheduleCall(Set(watchedFiles.keys))
@@ -839,6 +857,43 @@ package actor BuildServerManager: QueueBasedMessageHandler {
839857
}
840858
}
841859

860+
/// Check if the URI referenced by `location` has been copied during the preparation phase. If so, adjust the URI to
861+
/// the original source file.
862+
package func locationAdjustedForCopiedFiles(_ location: Location) -> Location {
863+
guard let originalUri = cachedCopiedFileMap[location.uri] else {
864+
return location
865+
}
866+
// If we regularly get issues that the copied file is out-of-sync with its original, we can check that the contents
867+
// of the lines touched by the location match and only return the original URI if they do. For now, we avoid this
868+
// check due to its performance cost of reading files from disk.
869+
return Location(uri: originalUri, range: location.range)
870+
}
871+
872+
/// Check if the URI referenced by `location` has been copied during the preparation phase. If so, adjust the URI to
873+
/// the original source file.
874+
package func locationsAdjustedForCopiedFiles(_ locations: [Location]) -> [Location] {
875+
return locations.map { locationAdjustedForCopiedFiles($0) }
876+
}
877+
878+
@discardableResult
879+
package func scheduleRecomputeCopyFileMap() -> Task<Void, Never> {
880+
let task = Task { [previousUpdateTask = copiedFileMapUpdateTask] in
881+
previousUpdateTask?.cancel()
882+
await orLog("Re-computing copy file map") {
883+
let sourceFilesAndDirectories = try await self.sourceFilesAndDirectories()
884+
var copiedFileMap: [DocumentURI: DocumentURI] = [:]
885+
for (file, fileInfo) in sourceFilesAndDirectories.files {
886+
for copyDestination in fileInfo.copyDestinations {
887+
copiedFileMap[copyDestination] = file
888+
}
889+
}
890+
self.cachedCopiedFileMap = copiedFileMap
891+
}
892+
}
893+
copiedFileMapUpdateTask = task
894+
return task
895+
}
896+
842897
/// Returns all the targets that the document is part of.
843898
package func targets(for document: DocumentURI) async -> [BuildTargetIdentifier] {
844899
guard let targets = await sourceFileInfo(for: document)?.targets else {
@@ -1292,7 +1347,8 @@ package actor BuildServerManager: QueueBasedMessageHandler {
12921347
isPartOfRootProject: isPartOfRootProject,
12931348
mayContainTests: mayContainTests,
12941349
isBuildable: !(target?.tags.contains(.notBuildable) ?? false)
1295-
&& (sourceKitData?.kind ?? .source) == .source
1350+
&& (sourceKitData?.kind ?? .source) == .source,
1351+
copyDestinations: Set(sourceKitData?.copyDestinations ?? [])
12961352
)
12971353
switch sourceItem.kind {
12981354
case .file:

Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,23 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
157157
/// `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
158158
public var outputPath: String?
159159

160-
public init(language: Language? = nil, kind: SourceKitSourceItemKind? = nil, outputPath: String? = nil) {
160+
/// If this source item gets copied to a different destination during preparation, the destinations it will be copied
161+
/// to.
162+
///
163+
/// If a user action would jump to one of these copied files, this allows SourceKit-LSP to redirect the navigation to
164+
/// the original source file instead of jumping to a file in the build directory.
165+
public var copyDestinations: [DocumentURI]?
166+
167+
public init(
168+
language: Language? = nil,
169+
kind: SourceKitSourceItemKind? = nil,
170+
outputPath: String? = nil,
171+
copyDestinations: [DocumentURI]? = nil
172+
) {
161173
self.language = language
162174
self.kind = kind
163175
self.outputPath = outputPath
176+
self.copyDestinations = copyDestinations
164177
}
165178

166179
public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) {
@@ -177,6 +190,14 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
177190
if case .string(let outputFilePath) = dictionary[CodingKeys.outputPath.stringValue] {
178191
self.outputPath = outputFilePath
179192
}
193+
if case .array(let copyDestinations) = dictionary[CodingKeys.copyDestinations.stringValue] {
194+
self.copyDestinations = copyDestinations.compactMap { entry in
195+
guard case .string(let copyDestination) = entry else {
196+
return nil
197+
}
198+
return try? DocumentURI(string: copyDestination)
199+
}
200+
}
180201
}
181202

182203
public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
@@ -190,6 +211,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
190211
if let outputPath {
191212
result[CodingKeys.outputPath.stringValue] = .string(outputPath)
192213
}
214+
if let copyDestinations {
215+
result[CodingKeys.copyDestinations.stringValue] = .array(copyDestinations.map { .string($0.stringValue) })
216+
}
193217
return .dictionary(result)
194218
}
195219
}

Sources/LanguageServerProtocol/Requests/SynchronizeRequest.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,18 @@ public struct SynchronizeRequest: RequestType {
3838
/// may be modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
3939
public var buildServerUpdates: Bool?
4040

41+
/// Wait for the build server to update its internal mapping of copied files to their original location.
42+
///
43+
/// This option is experimental, guarded behind the `synchronize-copy-file-map` experimental feature, and may be
44+
/// modified or removed in future versions of SourceKit-LSP without notice. Do not rely on it.
45+
public var copyFileMap: Bool?
46+
4147
/// Wait for background indexing to finish and all index unit files to be loaded into indexstore-db.
4248
public var index: Bool?
4349

44-
public init(buildServerUpdates: Bool? = nil, index: Bool? = nil) {
50+
public init(buildServerUpdates: Bool? = nil, copyFileMap: Bool? = nil, index: Bool? = nil) {
4551
self.buildServerUpdates = buildServerUpdates
52+
self.copyFileMap = copyFileMap
4653
self.index = index
4754
}
4855
}

Sources/SKOptions/ExperimentalFeatures.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable {
4545
/// - Note: Internal option, for testing only
4646
case synchronizeForBuildSystemUpdates = "synchronize-for-build-system-updates"
4747

48+
/// Enable the `copyFileMap` option in the `workspace/synchronize` request.
49+
///
50+
/// - Note: Internal option, for testing only
51+
case synchronizeCopyFileMap = "synchronize-copy-file-map"
52+
4853
/// All non-internal experimental features.
4954
public static var allNonInternalCases: [ExperimentalFeature] {
5055
allCases.filter { !$0.isInternal }
@@ -67,6 +72,8 @@ public enum ExperimentalFeature: String, Codable, Sendable, CaseIterable {
6772
return true
6873
case .synchronizeForBuildSystemUpdates:
6974
return true
75+
case .synchronizeCopyFileMap:
76+
return true
7077
}
7178
}
7279
}

Sources/SKTestSupport/TestSourceKitLSPClient.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import XCTest
2727
extension SourceKitLSPOptions {
2828
package static func testDefault(
2929
backgroundIndexing: Bool = true,
30-
experimentalFeatures: Set<ExperimentalFeature> = []
30+
experimentalFeatures: Set<ExperimentalFeature> = [.synchronizeCopyFileMap]
3131
) async throws -> SourceKitLSPOptions {
3232
let pluginPaths = try await sourceKitPluginPaths
3333
return SourceKitLSPOptions(

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2128,7 +2128,8 @@ extension SourceKitLSPServer {
21282128
return try await languageService.definition(req)
21292129
}
21302130
}
2131-
return .locations(indexBasedResponse)
2131+
let remappedLocations = await workspace.buildServerManager.locationsAdjustedForCopiedFiles(indexBasedResponse)
2132+
return .locations(remappedLocations)
21322133
}
21332134

21342135
/// 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 {
26322633
if req.buildServerUpdates != nil, !self.options.hasExperimentalFeature(.synchronizeForBuildSystemUpdates) {
26332634
throw ResponseError.unknown("\(SynchronizeRequest.method).buildServerUpdates is an experimental request option")
26342635
}
2636+
if req.copyFileMap != nil, !self.options.hasExperimentalFeature(.synchronizeCopyFileMap) {
2637+
throw ResponseError.unknown("\(SynchronizeRequest.method).copyFileMap is an experimental request option")
2638+
}
26352639
for workspace in workspaces {
26362640
await workspace.synchronize(req)
26372641
}

Sources/SourceKitLSP/Workspace.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,18 @@ package final class Workspace: Sendable, BuildServerManagerDelegate {
513513
await buildServerManager.waitForUpToDateBuildGraph()
514514
await indexUnitOutputPathsUpdateQueue.async {}.value
515515
}
516+
if request.copyFileMap ?? false {
517+
// Not using `valuePropagatingCancellation` here because that could lead us to the following scenario:
518+
// - An update of the copy file map is scheduled because of a change in the build graph
519+
// - We get a synchronize request
520+
// - Scheduling a new recomputation of the copy file map cancels the previous recomputation
521+
// - We cancel the synchronize request, which would also cancel the copy file map recomputation, leaving us with
522+
// an outdated version
523+
//
524+
// Technically, we might be doing unnecessary work here if the output file map is already up-to-date. But since
525+
// this option is mostly intended for testing purposes, this is acceptable.
526+
await buildServerManager.scheduleRecomputeCopyFileMap().value
527+
}
516528
if request.index ?? false {
517529
await semanticIndexManager?.waitForUpToDateIndex()
518530
uncheckedIndex?.pollForUnitChangesAndWait()

Tests/SourceKitLSPTests/DefinitionTests.swift

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import BuildServerIntegration
14+
import BuildServerProtocol
1315
import LanguageServerProtocol
1416
@_spi(Testing) import SKLogging
1517
import SKTestSupport
@@ -587,4 +589,104 @@ class DefinitionTests: XCTestCase {
587589
)
588590
XCTAssertEqual(response?.locations, [Location(uri: project.fileURI, range: Range(project.positions["2️⃣"]))])
589591
}
592+
593+
func testJumpToCopiedHeader() async throws {
594+
actor BuildServer: CustomBuildServer {
595+
let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker()
596+
private let projectRoot: URL
597+
598+
private var headerCopyDestination: URL {
599+
projectRoot.appending(components: "header-copy", "CopiedTest.h")
600+
}
601+
602+
init(projectRoot: URL, connectionToSourceKitLSP: any Connection) {
603+
self.projectRoot = projectRoot
604+
}
605+
606+
func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse {
607+
return try initializationResponseSupportingBackgroundIndexing(
608+
projectRoot: projectRoot,
609+
outputPathsProvider: false
610+
)
611+
}
612+
613+
func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) -> BuildTargetSourcesResponse {
614+
return BuildTargetSourcesResponse(items: [
615+
SourcesItem(
616+
target: .dummy,
617+
sources: [
618+
SourceItem(
619+
uri: DocumentURI(projectRoot.appendingPathComponent("Test.c")),
620+
kind: .file,
621+
generated: false,
622+
dataKind: .sourceKit,
623+
data: SourceKitSourceItemData(
624+
language: .c,
625+
kind: .source,
626+
outputPath: nil,
627+
copyDestinations: nil
628+
).encodeToLSPAny()
629+
),
630+
SourceItem(
631+
uri: DocumentURI(projectRoot.appendingPathComponent("Test.h")),
632+
kind: .file,
633+
generated: false,
634+
dataKind: .sourceKit,
635+
data: SourceKitSourceItemData(
636+
language: .c,
637+
kind: .header,
638+
outputPath: nil,
639+
copyDestinations: [DocumentURI(headerCopyDestination)]
640+
).encodeToLSPAny()
641+
),
642+
]
643+
)
644+
])
645+
}
646+
647+
func textDocumentSourceKitOptionsRequest(
648+
_ request: TextDocumentSourceKitOptionsRequest
649+
) throws -> TextDocumentSourceKitOptionsResponse? {
650+
return TextDocumentSourceKitOptionsResponse(compilerArguments: [
651+
request.textDocument.uri.pseudoPath, "-I", try headerCopyDestination.deletingLastPathComponent().filePath,
652+
])
653+
}
654+
655+
func prepareTarget(_ request: BuildTargetPrepareRequest) async throws -> VoidResponse {
656+
try FileManager.default.createDirectory(
657+
at: headerCopyDestination.deletingLastPathComponent(),
658+
withIntermediateDirectories: true
659+
)
660+
try FileManager.default.copyItem(
661+
at: projectRoot.appending(component: "Test.h"),
662+
to: headerCopyDestination
663+
)
664+
return VoidResponse()
665+
}
666+
}
667+
668+
let project = try await CustomBuildServerTestProject(
669+
files: [
670+
"Test.h": """
671+
void hello();
672+
""",
673+
"Test.c": """
674+
#include <CopiedTest.h>
675+
676+
void test() {
677+
1️⃣hello();
678+
}
679+
""",
680+
],
681+
buildServer: BuildServer.self,
682+
enableBackgroundIndexing: true,
683+
)
684+
try await project.testClient.send(SynchronizeRequest(copyFileMap: true))
685+
686+
let (uri, positions) = try project.openDocument("Test.c")
687+
let response = try await project.testClient.send(
688+
DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
689+
)
690+
XCTAssertEqual(response?.locations?.map(\.uri), [try project.uri(for: "Test.h")])
691+
}
590692
}

0 commit comments

Comments
 (0)