Merge pull request #2061 from ahoppen/normalized-path

Do not apply file path normalization in `mainFiles(containing:)` to the file itself

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -1312,6 +1312,19 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     #if canImport(Darwin)
     if let buildableSourceFiles = try? await self.buildableSourceFiles() {
       return mainFiles.map { mainFile in
+        if mainFile == uri {
+          // Do not apply the standardized file normalization to the source file itself. Otherwise we would get the
+          // following behavior:
+          //  - We have a build system that uses standardized file paths and index a file as /tmp/test.c
+          //  - We are asking for the main files of /private/tmp/test.c
+          //  - Since indexstore-db uses realpath for everything, we find the unit for /tmp/test.c as a unit containg
+          //    /private/tmp/test.c, which has /private/tmp/test.c as the main file.
+          //  - If we applied the path normalization, we would normalize /private/tmp/test.c to /tmp/test.c, thus
+          //    reporting that /tmp/test.c is a main file containing /private/tmp/test.c,
+          // But that doesn't make sense (it would, in fact cause us to treat /private/tmp/test.c as a header file that
+          // we should index using /tmp/test.c as a main file.
+          return mainFile
+        }
         if buildableSourceFiles.contains(mainFile) {
           return mainFile
         }

Sources/SKTestSupport/CustomBuildServerTestProject.swift

@@ -17,7 +17,7 @@ package import LanguageServerProtocol
 import LanguageServerProtocolExtensions
 import SKLogging
 package import SKOptions
-import SourceKitLSP
+package import SourceKitLSP
 import SwiftExtensions
 import ToolchainRegistry
 import XCTest
@@ -245,21 +245,24 @@ package final class CustomBuildServerTestProject<BuildServer: CustomBuildServer>
     files: [RelativeFileLocation: String],
     buildServer buildServerType: BuildServer.Type,
     options: SourceKitLSPOptions? = nil,
+    hooks: Hooks = Hooks(),
     enableBackgroundIndexing: Bool = false,
+    testScratchDir: URL? = nil,
     testName: String = #function
   ) async throws {
-    let hooks: Hooks = Hooks(
-      buildSystemHooks: BuildSystemHooks(injectBuildServer: { [buildServerBox] projectRoot, connectionToSourceKitLSP in
-        let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
-        buildServerBox.value = buildServer
-        return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
-      })
-    )
+    var hooks = hooks
+    XCTAssertNil(hooks.buildSystemHooks.injectBuildServer)
+    hooks.buildSystemHooks.injectBuildServer = { [buildServerBox] projectRoot, connectionToSourceKitLSP in
+      let buildServer = BuildServer(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
+      buildServerBox.value = buildServer
+      return LocalConnection(receiverName: "TestBuildSystem", handler: buildServer)
+    }
     try await super.init(
       files: files,
       options: options,
       hooks: hooks,
       enableBackgroundIndexing: enableBackgroundIndexing,
+      testScratchDir: testScratchDir,
       testName: testName
     )
   }

Sources/SKTestSupport/MultiFileTestProject.swift

@@ -139,10 +139,11 @@ package class MultiFileTestProject {
     enableBackgroundIndexing: Bool = false,
     usePullDiagnostics: Bool = true,
     preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
+    testScratchDir overrideTestScratchDir: URL? = nil,
     cleanUp: (@Sendable () -> Void)? = nil,
     testName: String = #function
   ) async throws {
-    scratchDirectory = try testScratchDir(testName: testName)
+    scratchDirectory = try overrideTestScratchDir ?? testScratchDir(testName: testName)
     self.fileData = try Self.writeFilesToDisk(files: files, scratchDirectory: scratchDirectory)
 
     self.testClient = try await TestSourceKitLSPClient(

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -481,6 +481,7 @@ package final actor SemanticIndexManager {
         // if we request the same header to be indexed twice, we'll pick the same unit file the second time around,
         // realize that its timestamp is later than the modification date of the header and we don't need to re-index.
         let mainFile = await buildSystemManager.mainFiles(containing: uri)
+          .filter { sourceFiles.contains($0) }
           .sorted(by: { $0.stringValue < $1.stringValue }).first
         guard let mainFile else {
           logger.log("Not indexing \(uri) because its main file could not be inferred")

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -2210,6 +2210,77 @@ final class BackgroundIndexingTests: XCTestCase {
       }
     )
   }
+
+  func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
+    try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")
+
+    final class BuildSystem: CustomBuildServer {
+      let inProgressRequestsTracker = CustomBuildServerInProgressRequestTracker()
+      private let projectRoot: URL
+      private var testFileURL: URL { projectRoot.appendingPathComponent("test.c").standardized }
+
+      required init(projectRoot: URL, connectionToSourceKitLSP: any LanguageServerProtocol.Connection) {
+        self.projectRoot = projectRoot
+      }
+
+      func initializeBuildRequest(_ request: InitializeBuildRequest) async throws -> InitializeBuildResponse {
+        return initializationResponse(
+          initializeData: SourceKitInitializeBuildResponseData(
+            indexDatabasePath: try projectRoot.appendingPathComponent("index-db").filePath,
+            indexStorePath: try projectRoot.appendingPathComponent("index-store").filePath,
+            prepareProvider: true,
+            sourceKitOptionsProvider: true
+          )
+        )
+      }
+
+      func buildTargetSourcesRequest(_ request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse {
+        return BuildTargetSourcesResponse(items: [
+          SourcesItem(target: .dummy, sources: [SourceItem(uri: URI(testFileURL), kind: .file, generated: false)])
+        ])
+      }
+
+      func textDocumentSourceKitOptionsRequest(
+        _ request: TextDocumentSourceKitOptionsRequest
+      ) async throws -> TextDocumentSourceKitOptionsResponse? {
+        return TextDocumentSourceKitOptionsResponse(compilerArguments: [request.textDocument.uri.pseudoPath])
+      }
+    }
+
+    let scratchDirectory = URL(fileURLWithPath: "/tmp")
+      .appendingPathComponent("sourcekitlsp-test")
+      .appendingPathComponent(testScratchName())
+    let indexedFiles = ThreadSafeBox<[DocumentURI]>(initialValue: [])
+    let project = try await CustomBuildServerTestProject(
+      files: [
+        "test.c": "void x() {}"
+      ],
+      buildServer: BuildSystem.self,
+      hooks: Hooks(
+        indexHooks: IndexHooks(
+          updateIndexStoreTaskDidStart: { task in
+            indexedFiles.withLock { indexedFiles in
+              indexedFiles += task.filesToIndex.map(\.file.sourceFile)
+            }
+          }
+        )
+      ),
+      enableBackgroundIndexing: true,
+      testScratchDir: scratchDirectory
+    )
+    try await project.testClient.send(PollIndexRequest())
+
+    // Ensure that changing `/private/tmp/.../test.c` only causes `/tmp/.../test.c` to be indexed, not
+    // `/private/tmp/.../test.c`.
+    indexedFiles.value = []
+    let testFileURL = try XCTUnwrap(project.uri(for: "test.c").fileURL?.realpath)
+    try await "void y() {}".writeWithRetry(to: testFileURL)
+    project.testClient.send(
+      DidChangeWatchedFilesNotification(changes: [FileEvent(uri: DocumentURI(testFileURL), type: .changed)])
+    )
+    try await project.testClient.send(PollIndexRequest())
+    XCTAssertEqual(indexedFiles.value, [try project.uri(for: "test.c")])
+  }
 }
 
 extension HoverResponseContents {

Tests/SourceKitLSPTests/WorkspaceTests.swift

@@ -1303,7 +1303,8 @@ final class WorkspaceTests: XCTestCase {
     try SkipUnless.platformIsDarwin("The realpath vs standardized path difference only exists on macOS")
 
     // Explicitly create a directory at /tmp (which is a standardized path but whose realpath is /private/tmp)
-    let scratchDirectory = URL(fileURLWithPath: "/tmp").appendingPathComponent("sourcekitlsp-test-\(UUID())")
+    let scratchDirectory = URL(fileURLWithPath: "/tmp")
+      .appendingPathComponent(testScratchName())
     try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)
 
     defer {