Skip to content

Commit 60c5f8d

Browse files
committed
Fix background indexing behavior if a source file is included in two targets via a symlink
Consider the following scenario: A project has target A containing A.swift an target B containing B.swift. B.swift is a symlink to A.swift. When A.swift is modified, both the dependencies of A and B need to be marked as having an out-of-date preparation status, not just A.
1 parent f900b4e commit 60c5f8d

File tree

6 files changed

+90
-8
lines changed

6 files changed

+90
-8
lines changed

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,26 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
671671
return languageFromBuildSystem ?? Language(inferredFromFileExtension: document)
672672
}
673673

674+
/// Returns the URIs of all source files in the project that have the same realpath as a document in `documents` but
675+
/// are not in `documents`.
676+
///
677+
/// This is useful in the following scenario: A project has target A containing A.swift an target B containing B.swift
678+
/// B.swift is a symlink to A.swift. When A.swift is modified, both the dependencies of A and B need to be marked as
679+
/// having an out-of-date preparation status, not just A.
680+
package func sourceFilesWithSameRealpath(as documents: [DocumentURI]) async -> [DocumentURI] {
681+
let realPaths = Set(documents.map { $0.symlinkTarget ?? $0 })
682+
return await orLog("") {
683+
var result: [DocumentURI] = []
684+
let filesAndDirectories = try await sourceFilesAndDirectories(includeNonBuildableFiles: true)
685+
for file in filesAndDirectories.files.keys {
686+
if realPaths.contains(file.symlinkTarget ?? file) && !realPaths.contains(file) {
687+
result.append(file)
688+
}
689+
}
690+
return result
691+
} ?? []
692+
}
693+
674694
/// Returns all the targets that the document is part of.
675695
package func targets(for document: DocumentURI) async -> Set<BuildTargetIdentifier> {
676696
return await orLog("Getting targets for source file") {
@@ -1155,10 +1175,10 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
11551175
var targetsWithUpdatedDependencies: Set<BuildTargetIdentifier> = []
11561176
// If a Swift file within a target is updated, reload all the other files within the target since they might be
11571177
// referring to a function in the updated file.
1158-
let targetsWithChangedSwiftFiles =
1159-
await events
1160-
.filter { Language(inferredFromFileExtension: $0.uri) == .swift }
1161-
.asyncFlatMap { await self.targets(for: $0.uri) }
1178+
var swiftFiles = events.filter { Language(inferredFromFileExtension: $0.uri) == .swift }.map(\.uri)
1179+
swiftFiles += await sourceFilesWithSameRealpath(as: swiftFiles)
1180+
1181+
let targetsWithChangedSwiftFiles = await swiftFiles.asyncFlatMap { await self.targets(for: $0) }
11621182
targetsWithUpdatedDependencies.formUnion(targetsWithChangedSwiftFiles)
11631183

11641184
// If a `.swiftmodule` file is updated, this means that we have performed a build / are

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
582582
SourceItem(
583583
uri: DocumentURI($0),
584584
kind: $0.isDirectory ? .directory : .file,
585-
generated: false,
585+
generated: false
586586
)
587587
}
588588
result.append(SourcesItem(target: target, sources: sources))

Sources/SKTestSupport/MultiFileTestProject.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ package class MultiFileTestProject {
8888
/// File contents can also contain `$TEST_DIR`, which gets replaced by the temporary directory.
8989
package init(
9090
files: [RelativeFileLocation: String],
91-
workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
91+
workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
92+
[WorkspaceFolder(uri: DocumentURI($0))]
93+
},
9294
initializationOptions: LSPAny? = nil,
9395
capabilities: ClientCapabilities = ClientCapabilities(),
9496
options: SourceKitLSPOptions = .testDefault(),

Sources/SKTestSupport/SwiftPMTestProject.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,9 @@ package class SwiftPMTestProject: MultiFileTestProject {
163163
package init(
164164
files: [RelativeFileLocation: String],
165165
manifest: String = SwiftPMTestProject.defaultPackageManifest,
166-
workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
166+
workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
167+
[WorkspaceFolder(uri: DocumentURI($0))]
168+
},
167169
initializationOptions: LSPAny? = nil,
168170
capabilities: ClientCapabilities = ClientCapabilities(),
169171
options: SourceKitLSPOptions = .testDefault(),

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,9 @@ package final actor SemanticIndexManager {
348348
package func filesDidChange(_ events: [FileEvent]) async {
349349
// We only re-index the files that were changed and don't re-index any of their dependencies. See the
350350
// `Documentation/Files_To_Reindex.md` file.
351-
let changedFiles = events.map(\.uri)
351+
var changedFiles = events.map(\.uri)
352352
await indexStoreUpToDateTracker.markOutOfDate(changedFiles)
353+
changedFiles += await buildSystemManager.sourceFilesWithSameRealpath(as: changedFiles)
353354

354355
let targets = await changedFiles.asyncMap { await buildSystemManager.targets(for: $0) }.flatMap { $0 }
355356
let dependentTargets = await buildSystemManager.targets(dependingOn: Set(targets))

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,6 +1631,63 @@ final class BackgroundIndexingTests: XCTestCase {
16311631
return completionAfterEdit.items.map(\.label) == ["self", "test()"]
16321632
}
16331633
}
1634+
1635+
func testSymlinkedTargetReferringToSameSourceFile() async throws {
1636+
let project = try await SwiftPMTestProject(
1637+
files: [
1638+
"LibA/LibA.swift": """
1639+
public let myVar: String
1640+
""",
1641+
"Client/Client.swift": """
1642+
import LibASymlink
1643+
1644+
func test() {
1645+
print(1️⃣myVar)
1646+
}
1647+
""",
1648+
],
1649+
manifest: """
1650+
let package = Package(
1651+
name: "MyLibrary",
1652+
targets: [
1653+
.target(name: "LibA"),
1654+
.target(name: "LibASymlink"),
1655+
.target(name: "Client", dependencies: ["LibASymlink"]),
1656+
]
1657+
)
1658+
""",
1659+
workspaces: { scratchDirectory in
1660+
let sources = scratchDirectory.appendingPathComponent("Sources")
1661+
try FileManager.default.createSymbolicLink(
1662+
at: sources.appendingPathComponent("LibASymlink"),
1663+
withDestinationURL: sources.appendingPathComponent("LibA")
1664+
)
1665+
return [WorkspaceFolder(uri: DocumentURI(scratchDirectory))]
1666+
},
1667+
enableBackgroundIndexing: true
1668+
)
1669+
1670+
let (uri, positions) = try project.openDocument("Client.swift")
1671+
let preEditHover = try await project.testClient.send(
1672+
HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
1673+
)
1674+
let preEditHoverContent = try XCTUnwrap(preEditHover?.contents.markupContent?.value)
1675+
XCTAssert(
1676+
preEditHoverContent.contains("String"),
1677+
"Pre edit hover content '\(preEditHoverContent)' does not contain 'String'"
1678+
)
1679+
1680+
let libAUri = try project.uri(for: "LibA.swift")
1681+
try "public let myVar: Int".write(to: try XCTUnwrap(libAUri.fileURL), atomically: true, encoding: .utf8)
1682+
project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: libAUri, type: .changed)]))
1683+
1684+
try await repeatUntilExpectedResult {
1685+
let postEditHover = try await project.testClient.send(
1686+
HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
1687+
)
1688+
return try XCTUnwrap(postEditHover?.contents.markupContent?.value).contains("Int")
1689+
}
1690+
}
16341691
}
16351692

16361693
extension HoverResponseContents {

0 commit comments

Comments
 (0)