Make output paths part of the buildTarget/sources request instead of a separate request

We need a mapping from source file to its output path in order to support source files that are part of multiple targets (because we need the output path to check if we have an up-to-date unit for a file in a given target). To achieve this mapping, it’s easier to tag the output path for each source file onto the `buildTarget/sources` request.

Author: ahoppen

Contributor Documentation/BSP Extensions.md

@@ -22,14 +22,14 @@ export interface SourceKitInitializeBuildResponseData {
    * for `swiftc` or `clang` invocations **/
   indexStorePath?: string;
 
-  /** Whether the server implements the `buildTarget/outputPaths` request. */
+  /** Whether the server set the `outputPath` property in the `buildTarget/sources` request */
   outputPathsProvider?: bool;
 
   /** Whether the build server supports the `buildTarget/prepare` request */
   prepareProvider?: bool;
 
   /** Whether the server implements the `textDocument/sourceKitOptions` request. */
-  sourceKitOptionsProvider?: bool;
+  sourceKitOp tionsProvider?: bool;
 
   /** The files to watch for changes.
    * Changes to these files are sent to the BSP server using `workspace/didChangeWatchedFiles`.
@@ -46,37 +46,6 @@ If `data` contains a string value for the `workDoneProgressTitle` key, then the
 
 `changes` can be `null` to indicate that all targets have changed.
 
-## `buildTarget/outputPaths`
-
-For all the source files in this target, the output paths that are used during indexing, ie. the `-index-unit-output-path` for the file, if it is specified 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.
-
-The server communicates during the initialize handshake whether this method is supported or not by setting `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
-
-- method: `buildTarget/outputPaths`
-- params: `OutputPathsParams`
-- result: `OutputPathsResult`
-
-```ts
-export interface BuildTargetOutputPathsParams {
-  /** A list of build targets to get the output paths for. */
-  targets: BuildTargetIdentifier[];
-}
-
-export interface BuildTargetOutputPathsItem {
-  /** The target these output file paths are for. */
-  target: BuildTargetIdentifier;
-
-  /** The output paths for all source files in this target. */
-  outputPaths: string[];
-}
-
-export interface BuildTargetOutputPathsResult {
-  items: BuildTargetOutputPathsItem[];
-}
-```
-
 ## `buildTarget/prepare`
 
 The prepare build target request is sent from the client to the server to prepare the given list of build targets for editor functionality.
@@ -119,6 +88,18 @@ export interface SourceKitSourceItemData {
    * inferring build settings from it. Listing header files in `buildTarget/sources` allows SourceKit-LSP to provide
    * semantic functionality for header files if they haven't been included by any main file. **/
   isHeader?: bool;
+
+  /**
+   * The output paths that are used during indexing for this file, ie. the `-index-unit-output-path`, if it is specified
+   * 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.
+   *
+   * The server communicates during the initialize handshake whether it populates this property by setting
+   * `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
+   */
+  outputPath?: string;
 }
 ```
 

Sources/BuildServerProtocol/CMakeLists.txt

@@ -2,7 +2,6 @@ add_library(BuildServerProtocol STATIC
   Messages.swift
 
   Messages/BuildShutdownRequest.swift
-  Messages/BuildTargetOutputPathsRequest.swift
   Messages/BuildTargetPrepareRequest.swift
   Messages/BuildTargetSourcesRequest.swift
   Messages/InitializeBuildRequest.swift

Sources/BuildServerProtocol/Messages.swift

@@ -14,7 +14,6 @@ public import LanguageServerProtocol
 
 fileprivate let requestTypes: [_RequestType.Type] = [
   BuildShutdownRequest.self,
-  BuildTargetOutputPathsRequest.self,
   BuildTargetPrepareRequest.self,
   BuildTargetSourcesRequest.self,
   CreateWorkDoneProgressRequest.self,

Sources/BuildServerProtocol/Messages/BuildTargetOutputPathsRequest.swift

@@ -126,9 +126,20 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
   /// semantic functionality for header files if they haven't been included by any main file.
   public var isHeader: Bool?
 
-  public init(language: Language? = nil, isHeader: Bool? = nil) {
+  /// The output paths that are used during indexing for this file, ie. the `-index-unit-output-path`, if it is specified
+  /// 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.
+  ///
+  /// The server communicates during the initialize handshake whether it populates this property by setting
+  /// `outputPathsProvider: true` in `SourceKitInitializeBuildResponseData`.
+  public var outputPath: String?
+
+  public init(language: Language? = nil, isHeader: Bool? = nil, outputPath: String? = nil) {
     self.language = language
     self.isHeader = isHeader
+    self.outputPath = outputPath
   }
 
   public init?(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) {
@@ -138,6 +149,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
     if case .bool(let isHeader) = dictionary[CodingKeys.isHeader.stringValue] {
       self.isHeader = isHeader
     }
+    if case .string(let outputFilePath) = dictionary[CodingKeys.outputPath.stringValue] {
+      self.outputPath = outputFilePath
+    }
   }
 
   public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
@@ -148,6 +162,9 @@ public struct SourceKitSourceItemData: LSPAnyCodable, Codable {
     if let isHeader {
       result[CodingKeys.isHeader.stringValue] = .bool(isHeader)
     }
+    if let outputPath {
+      result[CodingKeys.outputPath.stringValue] = .string(outputPath)
+    }
     return .dictionary(result)
   }
 }

Sources/BuildServerProtocol/Messages/BuildTargetSourcesRequest.swift

@@ -282,6 +282,7 @@ public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Send
   public var watchers: [FileSystemWatcher]?
 
   @available(*, deprecated, message: "Use initializer with alphabetical order of parameters")
+  @_disfavoredOverload
   public init(
     indexDatabasePath: String? = nil,
     indexStorePath: String? = nil,

Sources/BuildServerProtocol/Messages/InitializeBuildRequest.swift

@@ -363,8 +363,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
 
   private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
 
-  private var cachedTargetOutputPaths = RequestCache<BuildTargetOutputPathsRequest>()
-
   /// `SourceFilesAndDirectories` is a global property that only gets reset when the build targets change and thus
   /// has no real key.
   private struct SourceFilesAndDirectoriesKey: Hashable {}
@@ -609,13 +607,6 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
       }
       return !updatedTargets.intersection(cacheKey.targets).isEmpty
     }
-    self.cachedTargetOutputPaths.clear(isolation: self) { cacheKey in
-      guard let updatedTargets else {
-        // All targets might have changed
-        return true
-      }
-      return !updatedTargets.isDisjoint(with: cacheKey.targets)
-    }
     self.cachedSourceFilesAndDirectories.clearAll(isolation: self)
 
     await delegate?.buildTargetsChanged(notification.changes)
@@ -1152,7 +1143,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
 
   /// Return the output paths for all source files known to the build server.
   ///
-  /// See `BuildTargetOutputPathsRequest` for details.
+  /// See `SourceKitSourceItemData.outputFilePath` for details.
   package func outputPathInAllTargets() async throws -> [String] {
     return try await outputPaths(in: Set(buildTargets().map(\.key)))
   }
@@ -1161,29 +1152,7 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
   ///
   /// See `BuildTargetOutputPathsRequest` for details.
   package func outputPaths(in targets: Set<BuildTargetIdentifier>) async throws -> [String] {
-    guard let buildSystemAdapter = await buildSystemAdapterAfterInitialized, !targets.isEmpty else {
-      return []
-    }
-
-    let request = BuildTargetOutputPathsRequest(targets: targets.sorted { $0.uri.stringValue < $1.uri.stringValue })
-
-    // If we have a cached request for a superset of the targets, serve the result from that cache entry.
-    let fromSuperset = await orLog("Getting output paths from superset request") {
-      try await cachedTargetOutputPaths.getDerived(
-        isolation: self,
-        request,
-        canReuseKey: { targets.isSubset(of: $0.targets) },
-        transform: { BuildTargetOutputPathsResponse(items: $0.items.filter { targets.contains($0.target) }) }
-      )
-    }
-    if let fromSuperset {
-      return fromSuperset.items.flatMap(\.outputPaths)
-    }
-
-    let response = try await cachedTargetOutputPaths.get(request, isolation: self) { request in
-      try await buildSystemAdapter.send(request)
-    }
-    return response.items.flatMap(\.outputPaths)
+    return try await sourceFiles(in: targets).flatMap(\.sources).compactMap(\.sourceKitData?.outputPath)
   }
 
   /// Returns all source files in the project.

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -82,8 +82,6 @@ package enum BuildSystemMessageDependencyTracker: QueueBasedMessageHandlerDepend
     switch request {
     case is BuildShutdownRequest:
       self = .stateChange
-    case is BuildTargetOutputPathsRequest:
-      self = .stateRead
     case is BuildTargetPrepareRequest:
       self = .stateRead
     case is BuildTargetSourcesRequest:

Sources/BuildSystemIntegration/BuildSystemMessageDependencyTracker.swift

@@ -36,7 +36,8 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
   var indexDatabasePath: URL? { get async }
 
   /// Whether the build system is capable of preparing a target for indexing and determining the output paths for the
-  /// target, ie. whether the `prepare` and `buildTargetOutputPaths` methods have been implemented.
+  /// target, ie. whether the `prepare` method has been implemented and this build server populates the `outputPath`
+  /// property in the `buildTarget/sources` request.
   var supportsPreparationAndOutputPaths: Bool { get }
 
   /// Returns all targets in the build system
@@ -45,9 +46,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
   /// Returns all the source files in the given targets
   func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse
 
-  /// Returns all the output paths for the source files in the given targets.
-  func buildTargetOutputPaths(request: BuildTargetOutputPathsRequest) async throws -> BuildTargetOutputPathsResponse
-
   /// Called when files in the project change.
   func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) async
 

Sources/BuildSystemIntegration/BuiltInBuildSystem.swift

@@ -125,8 +125,6 @@ actor BuiltInBuildSystemAdapter: QueueBasedMessageHandler {
     switch request {
     case let request as RequestAndReply<BuildShutdownRequest>:
       await request.reply { VoidResponse() }
-    case let request as RequestAndReply<BuildTargetOutputPathsRequest>:
-      await request.reply { try await underlyingBuildSystem.buildTargetOutputPaths(request: request.params) }
     case let request as RequestAndReply<BuildTargetPrepareRequest>:
       await request.reply { try await underlyingBuildSystem.prepare(request: request.params) }
     case let request as RequestAndReply<BuildTargetSourcesRequest>: