Support indexing a file in the context of multiple targets

If a source file is part of multiple targets, we should index it in the context of all of those targets because the different targets may produce different USRs since they might use different build settings to interpret the file.

Author: ahoppen

Contributor Documentation/BSP Extensions.md

@@ -29,7 +29,7 @@ export interface SourceKitInitializeBuildResponseData {
   prepareProvider?: bool;
 
   /** Whether the server implements the `textDocument/sourceKitOptions` request. */
-  sourceKitOp tionsProvider?: bool;
+  sourceKitOptionsProvider?: bool;
 
   /** The files to watch for changes.
    * Changes to these files are sent to the BSP server using `workspace/didChangeWatchedFiles`.
@@ -94,7 +94,7 @@ export interface SourceKitSourceItemData {
    * in the compiler arguments or the file that is passed as `-o`, if `-index-unit-output-path` is not specified.
    *
    * This allows SourceKit-LSP to remove index entries for source files that are removed from a target but remain
-   * present on disk.
+   * present on disk and to index a file that is part of multiple targets in the context of each target.
    *
    * The server communicates during the initialize handshake whether it populates this property by setting
    * `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.

Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift

@@ -71,6 +71,14 @@ public struct SourceItem: Codable, Hashable, Sendable {
   /// Language-specific metadata about this source item.
   public var data: LSPAny?
 
+  /// If `dataKind` is `sourceKit`, the `data` interpreted as `SourceKitSourceItemData`, otherwise `nil`.
+  public var sourceKitData: SourceKitSourceItemData? {
+    guard dataKind == .sourceKit else {
+      return nil
+    }
+    return SourceKitSourceItemData(fromLSPAny: data)
+  }
+
   public init(
     uri: URI,
     kind: SourceItemKind,

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -26,9 +26,40 @@ import struct TSCBasic.RelativePath
 
 fileprivate typealias RequestCache<Request: RequestType & Hashable> = Cache<Request, Request.Response>
 
+/// An output path returned from the build server in the `SourceItem.data.outputPath` field.
+package enum OutputPath: Hashable, Comparable, CustomLogStringConvertible {
+  /// An output path returned from the build server.
+  case path(String)
+
+  /// The build server does not support output paths.
+  case notSupported
+
+  package var description: String {
+    switch self {
+    case .notSupported: return "<output path not supported>"
+    case .path(let path): return path
+    }
+  }
+
+  package var redactedDescription: String {
+    switch self {
+    case .notSupported: return "<output path not supported>"
+    case .path(let path): return path.hashForLogging
+    }
+  }
+}
+
 package struct SourceFileInfo: Sendable {
+  /// Maps the targets that this source file is a member of to the output path the file has within that target.
+  ///
+  /// The value in the dictionary can be:
+  ///  - `.path` if the build server supports output paths and produced a result
+  ///  - `.notSupported` if the build server does not support output paths.
+  ///  - `nil` if the build server supports output paths but did not return an output path for this file in this target.
+  package var targetsToOutputPaths: [BuildTargetIdentifier: OutputPath?]
+
   /// The targets that this source file is a member of
-  package var targets: Set<BuildTargetIdentifier>
+  package var targets: some Collection<BuildTargetIdentifier> & Sendable { targetsToOutputPaths.keys }
 
   /// `true` if this file belongs to the root project that the user is working on. It is false, if the file belongs
   /// to a dependency of the project.
@@ -48,8 +79,24 @@ package struct SourceFileInfo: Sendable {
     guard let other else {
       return self
     }
+    let mergedTargetsToOutputPaths = targetsToOutputPaths.merging(
+      other.targetsToOutputPaths,
+      uniquingKeysWith: { lhs, rhs in
+        if lhs == rhs {
+          return lhs
+        }
+        logger.error("Received mismatching output files: \(lhs?.forLogging) vs \(rhs?.forLogging)")
+        // Deterministically pick an output file if they mismatch. But really, this shouldn't happen.
+        switch (lhs, rhs) {
+        case (let lhs?, nil): return lhs
+        case (nil, let rhs?): return rhs
+        case (nil, nil): return nil  // Should be handled above already
+        case (let lhs?, let rhs?): return min(lhs, rhs)
+        }
+      }
+    )
     return SourceFileInfo(
-      targets: targets.union(other.targets),
+      targetsToOutputPaths: mergedTargetsToOutputPaths,
       isPartOfRootProject: other.isPartOfRootProject || isPartOfRootProject,
       mayContainTests: other.mayContainTests || mayContainTests,
       isBuildable: other.isBuildable || isBuildable
@@ -69,15 +116,6 @@ private struct BuildTargetInfo {
   var dependents: Set<BuildTargetIdentifier>
 }
 
-fileprivate extension SourceItem {
-  var sourceKitData: SourceKitSourceItemData? {
-    guard dataKind == .sourceKit else {
-      return nil
-    }
-    return SourceKitSourceItemData(fromLSPAny: data)
-  }
-}
-
 fileprivate extension BuildTarget {
   var sourceKitData: SourceKitBuildTarget? {
     guard dataKind == .sourceKit else {
@@ -743,26 +781,52 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
     return languageFromBuildSystem ?? Language(inferredFromFileExtension: document)
   }
 
-  /// Returns all the targets that the document is part of.
-  package func targets(for document: DocumentURI) async -> Set<BuildTargetIdentifier> {
+  /// Retrieve information about the given source file within the build server.
+  package func sourceFileInfo(for document: DocumentURI) async -> SourceFileInfo? {
     return await orLog("Getting targets for source file") {
-      var result: Set<BuildTargetIdentifier> = []
+      var result: SourceFileInfo? = nil
       let filesAndDirectories = try await sourceFilesAndDirectories()
-      if let targets = filesAndDirectories.files[document]?.targets {
-        result.formUnion(targets)
+      if let info = filesAndDirectories.files[document] {
+        result = result?.merging(info) ?? info
       }
       if !filesAndDirectories.directories.isEmpty, let documentPathComponents = document.fileURL?.pathComponents {
         for (_, (directoryPathComponents, info)) in filesAndDirectories.directories {
           guard let directoryPathComponents else {
             continue
           }
           if isDescendant(documentPathComponents, of: directoryPathComponents) {
-            result.formUnion(info.targets)
+            result = result?.merging(info) ?? info
           }
         }
       }
       return result
-    } ?? []
+    }
+  }
+
+  /// Returns all the targets that the document is part of.
+  package func targets(for document: DocumentURI) async -> [BuildTargetIdentifier] {
+    guard let targets = await sourceFileInfo(for: document)?.targets else {
+      return []
+    }
+    return Array(targets)
+  }
+
+  /// The `OutputPath` with which `uri` should be indexed in `target`. This may return:
+  ///  - `.path` if the build server supports output paths and produced a result
+  ///  - `.notSupported` if the build server does not support output paths. In this case we will assume that the index
+  ///    for `uri` is up-to-date if we have any up-to-date unit for it.
+  ///  - `nil` if the build server supports output paths but did not return an output path for `uri` in `target`.
+  package func outputPath(for document: DocumentURI, in target: BuildTargetIdentifier) async throws -> OutputPath? {
+    guard await initializationData?.outputPathsProvider ?? false else {
+      // Early exit if the build server doesn't support output paths.
+      return .notSupported
+    }
+    guard let sourceFileInfo = await sourceFileInfo(for: document),
+      let outputPath = sourceFileInfo.targetsToOutputPaths[target]
+    else {
+      return nil
+    }
+    return outputPath
   }
 
   /// Returns the `BuildTargetIdentifier` that should be used for semantic functionality of the given document.
@@ -1045,7 +1109,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
 
   /// Returns the list of targets that might depend on the given target and that need to be re-prepared when a file in
   /// `target` is modified.
-  package func targets(dependingOn targetIds: Set<BuildTargetIdentifier>) async -> [BuildTargetIdentifier] {
+  package func targets(dependingOn targetIds: some Collection<BuildTargetIdentifier>) async -> [BuildTargetIdentifier] {
     guard
       let buildTargets = await orLog("Getting build targets for dependents", { try await self.buildTargets() })
     else {
@@ -1144,7 +1208,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   /// Return the output paths for all source files known to the build server.
   ///
   /// See `SourceKitSourceItemData.outputFilePath` for details.
-  package func outputPathInAllTargets() async throws -> [String] {
+  package func outputPathsInAllTargets() async throws -> [String] {
     return try await outputPaths(in: Set(buildTargets().map(\.key)))
   }
 
@@ -1180,6 +1244,8 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   /// - Important: This method returns both buildable and non-buildable source files. Callers need to check
   /// `SourceFileInfo.isBuildable` if they are only interested in buildable source files.
   private func sourceFilesAndDirectories() async throws -> SourceFilesAndDirectories {
+    let supportsOutputPaths = await initializationData?.outputPathsProvider ?? false
+
     return try await cachedSourceFilesAndDirectories.get(
       SourceFilesAndDirectoriesKey(),
       isolation: self
@@ -1194,12 +1260,21 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
         let isPartOfRootProject = !(target?.tags.contains(.dependency) ?? false)
         let mayContainTests = target?.tags.contains(.test) ?? true
         for sourceItem in sourcesItem.sources {
+          let sourceKitData = sourceItem.sourceKitData
+          let outputPath: OutputPath? =
+            if !supportsOutputPaths {
+              .notSupported
+            } else if let outputPath = sourceKitData?.outputPath {
+              .path(outputPath)
+            } else {
+              nil
+            }
           let info = SourceFileInfo(
-            targets: [sourcesItem.target],
+            targetsToOutputPaths: [sourcesItem.target: outputPath],
             isPartOfRootProject: isPartOfRootProject,
             mayContainTests: mayContainTests,
             isBuildable: !(target?.tags.contains(.notBuildable) ?? false)
-              && !(sourceItem.sourceKitData?.isHeader ?? false)
+              && !(sourceKitData?.isHeader ?? false)
           )
           switch sourceItem.kind {
           case .file:

Sources/SKTestSupport/CustomBuildServerTestProject.swift

@@ -125,6 +125,7 @@ package extension CustomBuildServer {
       initializeData: SourceKitInitializeBuildResponseData(
         indexDatabasePath: try projectRoot.appendingPathComponent("index-db").filePath,
         indexStorePath: try projectRoot.appendingPathComponent("index-store").filePath,
+        outputPathsProvider: true,
         prepareProvider: true,
         sourceKitOptionsProvider: true
       )

Sources/SemanticIndex/CheckedIndex.swift

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+package import BuildSystemIntegration
 package import Foundation
 @preconcurrency package import IndexStoreDB
 package import LanguageServerProtocol
@@ -129,7 +130,7 @@ package final class CheckedIndex {
   }
 
   package func symbols(inFilePath path: String) -> [Symbol] {
-    guard self.hasUpToDateUnit(for: DocumentURI(filePath: path, isDirectory: false)) else {
+    guard self.hasAnyUpToDateUnit(for: DocumentURI(filePath: path, isDirectory: false)) else {
       return []
     }
     return index.symbols(inFilePath: path)
@@ -147,15 +148,28 @@ package final class CheckedIndex {
 
   /// Return `true` if a unit file has been indexed for the given file path after its last modification date.
   ///
-  /// This means that at least a single build configuration of this file has been indexed since its last modification.
-  ///
   /// If `mainFile` is passed, then `url` is a header file that won't have a unit associated with it. `mainFile` is
-  /// assumed to be a file that imports `url`. To check that `url` has an up-to-date unit, check that the latest unit
-  /// for `mainFile` is newer than the mtime of the header file at `url`.
-  package func hasUpToDateUnit(for uri: DocumentURI, mainFile: DocumentURI? = nil) -> Bool {
+  ///
+  /// This means that at least a single build configuration of this file has been indexed since its last modification.
+  /// This method does not care about which target (identified by output path in the index) produced the up-to-date
+  /// unit.
+  package func hasAnyUpToDateUnit(for uri: DocumentURI, mainFile: DocumentURI? = nil) -> Bool {
     return checker.indexHasUpToDateUnit(for: uri, mainFile: mainFile, index: index)
   }
 
+  /// Return `true` if a unit file with the given output path has been indexed after its last modification date of
+  /// `uri`.
+  ///
+  /// If `outputPath` is `notSupported`, this behaves the same as `hasAnyUpToDateUnit`.
+  package func hasUpToDateUnit(for uri: DocumentURI, mainFile: DocumentURI? = nil, outputPath: OutputPath) -> Bool {
+    switch outputPath {
+    case .path(let outputPath):
+      return checker.indexHasUpToDateUnit(for: uri, outputPath: outputPath, index: index)
+    case .notSupported:
+      return self.hasAnyUpToDateUnit(for: uri, mainFile: mainFile)
+    }
+  }
+
   /// Returns true if the file at the given URI has a different content in the document manager than on-disk. This is
   /// the case if the user made edits to the file but didn't save them yet.
   ///
@@ -393,14 +407,9 @@ private struct IndexOutOfDateChecker {
     }
   }
 
-  /// Return `true` if a unit file has been indexed for the given file path after its last modification date.
-  ///
-  /// This means that at least a single build configuration of this file has been indexed since its last modification.
-  ///
-  /// If `mainFile` is passed, then `filePath` is a header file that won't have a unit associated with it. `mainFile` is
-  /// assumed to be a file that imports `url`. To check that `url` has an up-to-date unit, check that the latest unit
-  /// for `mainFile` is newer than the mtime of the header file at `url`.
-  mutating func indexHasUpToDateUnit(for filePath: DocumentURI, mainFile: DocumentURI?, index: IndexStoreDB) -> Bool {
+  /// Checks if we have a unit that's up to date for the given source file, assuming that the unit in question has been
+  /// modified at the date returned by `unitModificationDate`.
+  private mutating func unitIsUpToDate(for filePath: DocumentURI, unitModificationDate: () -> Date?) -> Bool {
     switch checkLevel {
     case .inMemoryModifiedFiles(let documentManager):
       if fileHasInMemoryModifications(filePath, documentManager: documentManager) {
@@ -411,10 +420,7 @@ private struct IndexOutOfDateChecker {
       // If there are no in-memory modifications check if there are on-disk modifications.
       fallthrough
     case .modifiedFiles:
-      guard
-        let filePathStr = orLog("Realpath for up-to-date", { try (mainFile ?? filePath).fileURL?.realpath.filePath }),
-        let lastUnitDate = index.dateOfLatestUnitFor(filePath: filePathStr)
-      else {
+      guard let lastUnitDate = unitModificationDate() else {
         return false
       }
       do {
@@ -436,6 +442,33 @@ private struct IndexOutOfDateChecker {
     }
   }
 
+  /// Return `true` if a unit file has been indexed for the given file path after its last modification date.
+  ///
+  /// This means that at least a single build configuration of this file has been indexed since its last modification.
+  mutating func indexHasUpToDateUnit(for filePath: DocumentURI, mainFile: DocumentURI?, index: IndexStoreDB) -> Bool {
+    return unitIsUpToDate(
+      for: filePath,
+      unitModificationDate: {
+        let filePathStr = orLog("Realpath for up-to-date", { try (mainFile ?? filePath).fileURL?.realpath.filePath })
+        guard let filePathStr else {
+          return nil
+        }
+        return index.dateOfLatestUnitFor(filePath: filePathStr)
+      }
+    )
+  }
+
+  /// Return `true` if a unit file has been indexed for the given file path after its last modification date.
+  ///
+  /// This means that at least a single build configuration of this file has been indexed since its last modification.
+  ///
+  /// If `mainFile` is passed, then `filePath` is a header file that won't have a unit associated with it. `mainFile` is
+  /// assumed to be a file that imports `url`. To check that `url` has an up-to-date unit, check that the latest unit
+  /// for `mainFile` is newer than the mtime of the header file at `url`.
+  mutating func indexHasUpToDateUnit(for filePath: DocumentURI, outputPath: String, index: IndexStoreDB) -> Bool {
+    return unitIsUpToDate(for: filePath, unitModificationDate: { index.dateOfUnitFor(outputPath: outputPath) })
+  }
+
   // MARK: - Cached check primitives
 
   /// `documentManager` must always be the same between calls to `hasFileInMemoryModifications` since it is not part of

Sources/SemanticIndex/PreparationTaskDescription.swift

@@ -37,7 +37,7 @@ package struct PreparationTaskDescription: IndexTaskDescription {
   /// The build system manager that is used to get the toolchain and build settings for the files to index.
   private let buildSystemManager: BuildSystemManager
 
-  private let preparationUpToDateTracker: UpToDateTracker<BuildTargetIdentifier>
+  private let preparationUpToDateTracker: UpToDateTracker<BuildTargetIdentifier, DummySecondaryKey>
 
   /// See `SemanticIndexManager.logMessageToIndexLog`.
   private let logMessageToIndexLog:
@@ -62,7 +62,7 @@ package struct PreparationTaskDescription: IndexTaskDescription {
   init(
     targetsToPrepare: [BuildTargetIdentifier],
     buildSystemManager: BuildSystemManager,
-    preparationUpToDateTracker: UpToDateTracker<BuildTargetIdentifier>,
+    preparationUpToDateTracker: UpToDateTracker<BuildTargetIdentifier, DummySecondaryKey>,
     logMessageToIndexLog: @escaping @Sendable (
       _ message: String, _ type: WindowMessageType, _ structure: StructuredLogKind
     ) -> Void,