Merge pull request #1858 from ahoppen/symlink-target

Fix background indexing behavior if a source file is included in two targets via a symlink

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -1033,11 +1033,11 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     return response.items
   }
 
-  /// Returns all source files in the project that can be built.
+  /// Returns all source files in the project.
   ///
   /// - SeeAlso: Comment in `sourceFilesAndDirectories` for a definition of what `buildable` means.
-  package func buildableSourceFiles() async throws -> [DocumentURI: SourceFileInfo] {
-    return try await sourceFilesAndDirectories(includeNonBuildableFiles: false).files
+  package func sourceFiles(includeNonBuildableFiles: Bool) async throws -> [DocumentURI: SourceFileInfo] {
+    return try await sourceFilesAndDirectories(includeNonBuildableFiles: includeNonBuildableFiles).files
   }
 
   /// Get all files and directories that are known to the build system, ie. that are returned by a `buildTarget/sources`
@@ -1093,7 +1093,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   }
 
   package func testFiles() async throws -> [DocumentURI] {
-    return try await buildableSourceFiles().compactMap { (uri, info) -> DocumentURI? in
+    return try await sourceFiles(includeNonBuildableFiles: false).compactMap { (uri, info) -> DocumentURI? in
       guard info.isPartOfRootProject, info.mayContainTests else {
         return nil
       }

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -587,7 +587,7 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem {
         SourceItem(
           uri: DocumentURI($0),
           kind: $0.isDirectory ? .directory : .file,
-          generated: false,
+          generated: false
         )
       }
       result.append(SourcesItem(target: target, sources: sources))

Sources/SKTestSupport/MultiFileTestProject.swift

@@ -88,7 +88,9 @@ package class MultiFileTestProject {
   /// File contents can also contain `$TEST_DIR`, which gets replaced by the temporary directory.
   package init(
     files: [RelativeFileLocation: String],
-    workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
+    workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
+      [WorkspaceFolder(uri: DocumentURI($0))]
+    },
     initializationOptions: LSPAny? = nil,
     capabilities: ClientCapabilities = ClientCapabilities(),
     options: SourceKitLSPOptions = .testDefault(),

Sources/SKTestSupport/SwiftPMTestProject.swift

@@ -163,7 +163,9 @@ package class SwiftPMTestProject: MultiFileTestProject {
   package init(
     files: [RelativeFileLocation: String],
     manifest: String = SwiftPMTestProject.defaultPackageManifest,
-    workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
+    workspaces: (_ scratchDirectory: URL) async throws -> [WorkspaceFolder] = {
+      [WorkspaceFolder(uri: DocumentURI($0))]
+    },
     initializationOptions: LSPAny? = nil,
     capabilities: ClientCapabilities = ClientCapabilities(),
     options: SourceKitLSPOptions = .testDefault(),

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -291,7 +291,8 @@ package final actor SemanticIndexManager {
             filesToIndex
           } else {
             await orLog("Getting files to index") {
-              try await self.buildSystemManager.buildableSourceFiles().keys.sorted { $0.stringValue < $1.stringValue }
+              try await self.buildSystemManager.sourceFiles(includeNonBuildableFiles: false).keys
+                .sorted { $0.stringValue < $1.stringValue }
             } ?? []
           }
         if !indexFilesWithUpToDateUnit {
@@ -408,7 +409,7 @@ package final actor SemanticIndexManager {
     toCover files: some Collection<DocumentURI> & Sendable
   ) async -> [FileToIndex] {
     let sourceFiles = await orLog("Getting source files in project") {
-      Set(try await buildSystemManager.buildableSourceFiles().keys)
+      Set(try await buildSystemManager.sourceFiles(includeNonBuildableFiles: false).keys)
     }
     guard let sourceFiles else {
       return []

Sources/SourceKitLSP/Workspace.swift

@@ -62,6 +62,51 @@ fileprivate func firstNonNil<T>(
   return try await defaultValue()
 }
 
+/// Actor that caches realpaths for `sourceFilesWithSameRealpath`.
+fileprivate actor SourceFilesWithSameRealpathInferrer {
+  private let buildSystemManager: BuildSystemManager
+  private var realpathCache: [DocumentURI: DocumentURI] = [:]
+
+  init(buildSystemManager: BuildSystemManager) {
+    self.buildSystemManager = buildSystemManager
+  }
+
+  private func realpath(of uri: DocumentURI) -> DocumentURI {
+    if let cached = realpathCache[uri] {
+      return cached
+    }
+    let value = uri.symlinkTarget ?? uri
+    realpathCache[uri] = value
+    return value
+  }
+
+  /// Returns the URIs of all source files in the project that have the same realpath as a document in `documents` but
+  /// are not in `documents`.
+  ///
+  /// This is useful in 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.
+  package func sourceFilesWithSameRealpath(as documents: [DocumentURI]) async -> [DocumentURI] {
+    let realPaths = Set(documents.map { realpath(of: $0) })
+    return await orLog("Determining source files with same realpath") {
+      var result: [DocumentURI] = []
+      let filesAndDirectories = try await buildSystemManager.sourceFiles(includeNonBuildableFiles: true)
+      for file in filesAndDirectories.keys {
+        if realPaths.contains(realpath(of: file)) && !documents.contains(file) {
+          result.append(file)
+        }
+      }
+      return result
+    } ?? []
+  }
+
+  func filesDidChange(_ events: [FileEvent]) {
+    for event in events {
+      realpathCache[event.uri] = nil
+    }
+  }
+}
+
 /// Represents the configuration and state of a project or combination of projects being worked on
 /// together.
 ///
@@ -86,6 +131,8 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
   /// The build system manager to use for documents in this workspace.
   package let buildSystemManager: BuildSystemManager
 
+  private let sourceFilesWithSameRealpathInferrer: SourceFilesWithSameRealpathInferrer
+
   let options: SourceKitLSPOptions
 
   /// The source code index, if available.
@@ -126,6 +173,9 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
     self.options = options
     self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex)
     self.buildSystemManager = buildSystemManager
+    self.sourceFilesWithSameRealpathInferrer = SourceFilesWithSameRealpathInferrer(
+      buildSystemManager: buildSystemManager
+    )
     if options.backgroundIndexingOrDefault, let uncheckedIndex,
       await buildSystemManager.initializationData?.prepareProvider ?? false
     {
@@ -316,6 +366,17 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate {
   }
 
   package func filesDidChange(_ events: [FileEvent]) async {
+    // First clear any cached realpaths in `sourceFilesWithSameRealpathInferrer`.
+    await sourceFilesWithSameRealpathInferrer.filesDidChange(events)
+
+    // Now infer any edits for source files that share the same realpath as one of the modified files.
+    var events = events
+    events +=
+      await sourceFilesWithSameRealpathInferrer
+      .sourceFilesWithSameRealpath(as: events.filter { $0.type == .changed }.map(\.uri))
+      .map { FileEvent(uri: $0, type: .changed) }
+
+    // Notify all clients about the reported and inferred edits.
     await buildSystemManager.filesDidChange(events)
     await syntacticTestIndex.filesDidChange(events)
     await semanticIndexManager?.filesDidChange(events)

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -1620,6 +1620,63 @@ final class BackgroundIndexingTests: XCTestCase {
       return completionAfterEdit.items.map(\.label) == ["self", "test()"]
     }
   }
+
+  func testSymlinkedTargetReferringToSameSourceFile() async throws {
+    let project = try await SwiftPMTestProject(
+      files: [
+        "LibA/LibA.swift": """
+        public let myVar: String
+        """,
+        "Client/Client.swift": """
+        import LibASymlink
+
+        func test() {
+          print(1️⃣myVar)
+        }
+        """,
+      ],
+      manifest: """
+        let package = Package(
+          name: "MyLibrary",
+          targets: [
+           .target(name: "LibA"),
+           .target(name: "LibASymlink"),
+           .target(name: "Client", dependencies: ["LibASymlink"]),
+          ]
+        )
+        """,
+      workspaces: { scratchDirectory in
+        let sources = scratchDirectory.appendingPathComponent("Sources")
+        try FileManager.default.createSymbolicLink(
+          at: sources.appendingPathComponent("LibASymlink"),
+          withDestinationURL: sources.appendingPathComponent("LibA")
+        )
+        return [WorkspaceFolder(uri: DocumentURI(scratchDirectory))]
+      },
+      enableBackgroundIndexing: true
+    )
+
+    let (uri, positions) = try project.openDocument("Client.swift")
+    let preEditHover = try await project.testClient.send(
+      HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+    )
+    let preEditHoverContent = try XCTUnwrap(preEditHover?.contents.markupContent?.value)
+    XCTAssert(
+      preEditHoverContent.contains("String"),
+      "Pre edit hover content '\(preEditHoverContent)' does not contain 'String'"
+    )
+
+    let libAUri = try project.uri(for: "LibA.swift")
+    try "public let myVar: Int".write(to: try XCTUnwrap(libAUri.fileURL), atomically: true, encoding: .utf8)
+    project.testClient.send(DidChangeWatchedFilesNotification(changes: [FileEvent(uri: libAUri, type: .changed)]))
+
+    try await repeatUntilExpectedResult {
+      let postEditHover = try await project.testClient.send(
+        HoverRequest(textDocument: TextDocumentIdentifier(uri), position: positions["1️⃣"])
+      )
+      return try XCTUnwrap(postEditHover?.contents.markupContent?.value).contains("Int")
+    }
+  }
 }
 
 extension HoverResponseContents {