Merge pull request #2033 from ahoppen/standardized-file-paths

Allow a build system to use standardized paths while indexstore-db return realpaths

Author: ahoppen

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -1253,11 +1253,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       // it cached. We can just return it.
       return mainFile
     }
-    guard let mainFilesProvider else {
-      return uri
-    }
 
-    let mainFiles = await mainFilesProvider.mainFilesContainingFile(uri)
+    let mainFiles = await mainFiles(containing: uri)
     if mainFiles.contains(uri) {
       // If the main files contain the file itself, prefer to use that one
       return uri
@@ -1270,6 +1267,37 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     }
   }
 
+  /// Returns all main files that include the given document.
+  ///
+  /// On Darwin platforms, this also performs the following normalization: indexstore-db by itself returns realpaths
+  /// but the build system might be using standardized Darwin paths (eg. realpath is `/private/tmp` but the standardized
+  /// path is `/tmp`). If the realpath that indexstore-db returns could not be found in the build system's source files
+  /// but the standardized path is part of the source files, return the standardized path instead.
+  package func mainFiles(containing uri: DocumentURI) async -> [DocumentURI] {
+    guard let mainFilesProvider else {
+      return [uri]
+    }
+    let mainFiles = Array(await mainFilesProvider.mainFiles(containing: uri, crossLanguage: false))
+    #if canImport(Darwin)
+    if let buildableSourceFiles = try? await self.buildableSourceFiles() {
+      return mainFiles.map { mainFile in
+        if buildableSourceFiles.contains(mainFile) {
+          return mainFile
+        }
+        guard let fileURL = mainFile.fileURL else {
+          return mainFile
+        }
+        let standardized = DocumentURI(fileURL.standardizedFileURL)
+        if buildableSourceFiles.contains(standardized) {
+          return standardized
+        }
+        return mainFile
+      }
+    }
+    #endif
+    return mainFiles
+  }
+
   /// Returns the main file used for `uri`, if this is a registered file.
   ///
   /// For testing purposes only.
@@ -1317,11 +1345,9 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       filesWithUpdatedDependencies.formUnion(sourceFiles.flatMap(\.sources).map(\.uri))
     }
 
-    if let mainFilesProvider {
-      var mainFiles = await Set(events.asyncFlatMap { await mainFilesProvider.mainFilesContainingFile($0.uri) })
-      mainFiles.subtract(events.map(\.uri))
-      filesWithUpdatedDependencies.formUnion(mainFiles)
-    }
+    var mainFiles = await Set(events.asyncFlatMap { await self.mainFiles(containing: $0.uri) })
+    mainFiles.subtract(events.map(\.uri))
+    filesWithUpdatedDependencies.formUnion(mainFiles)
 
     await self.filesDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies)
   }

Sources/BuildSystemIntegration/MainFilesProvider.swift

@@ -18,13 +18,15 @@ import LanguageServerProtocol
 
 /// A type that can provide the set of main files that include a particular file.
 package protocol MainFilesProvider: Sendable {
-  /// Returns the set of main files that contain the given file.
+  /// Returns all the files that (transitively) include the header file at the given path.
   ///
-  /// For example,
+  /// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
+  ///
+  /// ### Examples
   ///
   /// ```
   /// mainFilesContainingFile("foo.cpp") == Set(["foo.cpp"])
   /// mainFilesContainingFile("foo.h") == Set(["foo.cpp", "bar.cpp"])
   /// ```
-  func mainFilesContainingFile(_: DocumentURI) async -> Set<DocumentURI>
+  func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) async -> Set<DocumentURI>
 }

Sources/SemanticIndex/CheckedIndex.swift

@@ -151,19 +151,6 @@ package final class CheckedIndex {
     return index.unitTests(referencedByMainFiles: mainFilePaths).filter { checker.isUpToDate($0.location) }
   }
 
-  /// Returns all the files that (transitively) include the header file at the given path.
-  ///
-  /// If `crossLanguage` is set to `true`, Swift files that import a header through a module will also be reported.
-  package func mainFilesContainingFile(uri: DocumentURI, crossLanguage: Bool = false) -> [DocumentURI] {
-    return index.mainFilesContainingFile(path: uri.pseudoPath, crossLanguage: crossLanguage).compactMap {
-      let uri = DocumentURI(filePath: $0, isDirectory: false)
-      guard checker.indexHasUpToDateUnit(for: uri, mainFile: nil, index: self.index) else {
-        return nil
-      }
-      return uri
-    }
-  }
-
   /// Returns all unit test symbols in the index.
   package func unitTests() -> [SymbolOccurrence] {
     return index.unitTests().filter { checker.isUpToDate($0.location) }

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -462,19 +462,16 @@ package final actor SemanticIndexManager {
     guard let sourceFiles else {
       return []
     }
-    let deletedFilesIndex = index.checked(for: .deletedFiles)
     let modifiedFilesIndex = index.checked(for: .modifiedFiles)
 
     let filesToReIndex =
       await files
-      .asyncFilter {
+      .asyncCompactMap { uri -> (FileToIndex, Date?)? in
         // First, check if we know that the file is up-to-date, in which case we don't need to hit the index or file
         // system at all
-        if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate($0) {
-          return false
+        if !indexFilesWithUpToDateUnits, await indexStoreUpToDateTracker.isUpToDate(uri) {
+          return nil
         }
-        return true
-      }.compactMap { uri -> (FileToIndex, Date?)? in
         if sourceFiles.contains(uri) {
           if !indexFilesWithUpToDateUnits, modifiedFilesIndex.hasUpToDateUnit(for: uri) {
             return nil
@@ -487,9 +484,7 @@ package final actor SemanticIndexManager {
         // Deterministically pick a main file. This ensures that we always pick the same main file for a header. This way,
         // 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 =
-          deletedFilesIndex
-          .mainFilesContainingFile(uri: uri, crossLanguage: false)
+        let mainFile = await buildSystemManager.mainFiles(containing: uri)
           .sorted(by: { $0.stringValue < $1.stringValue }).first
         guard let mainFile else {
           logger.log("Not indexing \(uri) because its main file could not be inferred")

Sources/SourceKitLSP/IndexStoreDB+MainFilesProvider.swift

@@ -28,11 +28,16 @@ import SemanticIndex
 import SwiftExtensions
 #endif
 
-extension UncheckedIndex {
-  package func mainFilesContainingFile(_ uri: DocumentURI) -> Set<DocumentURI> {
+extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {
+  /// - Important: This may return realpaths when the build system might not be using realpaths. Use
+  ///   `BuildSystemManager.mainFiles(containing:)` to work around that problem.
+  package func mainFiles(containing uri: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
     let mainFiles: Set<DocumentURI>
     if let filePath = orLog("File path to get main files", { try uri.fileURL?.filePath }) {
-      let mainFilePaths = Set(self.underlyingIndexStoreDB.mainFilesContainingFile(path: filePath))
+      let mainFilePaths = self.underlyingIndexStoreDB.mainFilesContainingFile(
+        path: filePath,
+        crossLanguage: crossLanguage
+      )
       mainFiles = Set(
         mainFilePaths
           .filter { FileManager.default.fileExists(atPath: $0) }
@@ -41,9 +46,7 @@ extension UncheckedIndex {
     } else {
       mainFiles = []
     }
-    logger.info("mainFilesContainingFile(\(uri.forLogging)) -> \(mainFiles)")
+    logger.info("Main files for \(uri.forLogging): \(mainFiles)")
     return mainFiles
   }
 }
-
-extension UncheckedIndex: BuildSystemIntegration.MainFilesProvider {}

Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift

@@ -327,7 +327,7 @@ private final actor ManualMainFilesProvider: MainFilesProvider {
     self.mainFiles[file] = mainFiles
   }
 
-  func mainFilesContainingFile(_ file: DocumentURI) -> Set<DocumentURI> {
+  func mainFiles(containing file: DocumentURI, crossLanguage: Bool) -> Set<DocumentURI> {
     if let result = mainFiles[file] {
       return result
     }

Tests/SourceKitLSPTests/WorkspaceTests.swift

@@ -1315,6 +1315,71 @@ final class WorkspaceTests: XCTestCase {
     )
     try XCTAssertEqual(XCTUnwrap(prepareUpToDateAgain).didPrepareTarget, false)
   }
+
+  func testBuildSystemUsesStandardizedFileUrlsInsteadOfRealpath() async throws {
+    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())")
+    try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)
+
+    defer {
+      if cleanScratchDirectories {
+        try? FileManager.default.removeItem(at: scratchDirectory)
+      }
+    }
+
+    _ = try MultiFileTestProject.writeFilesToDisk(
+      files: [
+        "test.h": "",
+        "test.c": """
+        #include "test.h"
+        """,
+        "compile_commands.json": """
+        [
+          {
+            "directory": "$TEST_DIR_BACKSLASH_ESCAPED",
+            "arguments": [
+              "clang",
+              "$TEST_DIR_BACKSLASH_ESCAPED/test.c",
+              "-DHAVE_SETTINGS",
+              "-index-store-path",
+              "$TEST_DIR_BACKSLASH_ESCAPED/index"
+            ],
+            "file": "test.c",
+            "output": "$TEST_DIR_BACKSLASH_ESCAPED/build/test.o"
+          }
+        ]
+        """,
+      ],
+      scratchDirectory: scratchDirectory
+    )
+
+    let clang = try unwrap(await ToolchainRegistry.forTesting.default?.clang)
+    try await Process.checkNonZeroExit(
+      arguments: [
+        clang.filePath, "-index-store-path", scratchDirectory.appendingPathComponent("index").filePath,
+        scratchDirectory.appendingPathComponent("test.c").filePath,
+        "-fsyntax-only",
+      ]
+    )
+
+    let testClient = try await TestSourceKitLSPClient(
+      options: .testDefault(experimentalFeatures: [.sourceKitOptionsRequest]),
+      workspaceFolders: [WorkspaceFolder(uri: DocumentURI(scratchDirectory), name: nil)]
+    )
+
+    // Check that we can infer build settings for the header from its main file. indexstore-db stores this main file
+    // path as `/private/tmp` while the build system only knows about it as `/tmp`.
+    let options = try await testClient.send(
+      SourceKitOptionsRequest(
+        textDocument: TextDocumentIdentifier(scratchDirectory.appendingPathComponent("test.h")),
+        prepareTarget: false,
+        allowFallbackSettings: false
+      )
+    )
+    XCTAssert(options.compilerArguments.contains("-DHAVE_SETTINGS"))
+  }
 }
 
 fileprivate let defaultSDKArgs: String = {