Migrate targets(dependingOn:) to BSP

Author: ahoppen

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -340,10 +340,6 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
     return nil
   }
 
-  package func targets(dependingOn targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    return nil
-  }
-
   package func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) {
     // BuildServerBuildSystem does not support syntactic test discovery or background indexing.
     // (https://github.com/swiftlang/sourcekit-lsp/issues/1173).

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -489,8 +489,14 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     return await buildSystem?.underlyingBuildSystem.topologicalSort(of: targets)
   }
 
-  package func targets(dependingOn targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]? {
-    return await buildSystem?.underlyingBuildSystem.targets(dependingOn: targets)
+  /// 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] {
+    guard let buildTargets = await orLog("Getting build targets for dependencies", { try await self.buildTargets() })
+    else {
+      return []
+    }
+    return buildTargets.filter { $0.dependencies.contains(anyIn: targetIds) }.map { $0.id }
   }
 
   package func prepare(

Sources/BuildSystemIntegration/BuiltInBuildSystem.swift

@@ -113,15 +113,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
   /// `nil` if the build system doesn't support topological sorting of targets.
   func topologicalSort(of targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]?
 
-  /// 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.
-  ///
-  /// The returned list can be an over-approximation, in which case the indexer will perform more work than strictly
-  /// necessary by scheduling re-preparation of a target where it isn't necessary.
-  ///
-  /// Returning `nil` indicates that all targets should be considered depending on the given target.
-  func targets(dependingOn targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]?
-
   /// The toolchain that should be used to open the given document.
   ///
   /// If `nil` is returned, then the default toolchain for the given language is used.

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -163,10 +163,6 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem {
     return nil
   }
 
-  package func targets(dependingOn targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    return nil
-  }
-
   private func database(for uri: DocumentURI) -> CompilationDatabase? {
     if let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) {
       return database(for: path)

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -215,6 +215,8 @@ package actor SwiftPMBuildSystem {
   /// greater depth.
   private var targets: [BuildTargetIdentifier: (buildTarget: SwiftBuildTarget, depth: Int)] = [:]
 
+  private var targetDependencies: [BuildTargetIdentifier: Set<BuildTargetIdentifier>] = [:]
+
   static package func projectRoot(
     for path: TSCBasic.AbsolutePath,
     options: SourceKitLSPOptions
@@ -442,6 +444,8 @@ extension SwiftPMBuildSystem {
 
     self.targets = [:]
     self.fileToTargets = [:]
+    self.targetDependencies = [:]
+
     buildDescription.traverseModules { buildTarget, parent, depth in
       let targetIdentifier = orLog("Getting build target identifier") { try BuildTargetIdentifier(buildTarget) }
       guard let targetIdentifier else {
@@ -455,6 +459,11 @@ extension SwiftPMBuildSystem {
           fileToTargets[DocumentURI(source), default: []].insert(targetIdentifier)
         }
       }
+      if let parent,
+        let parentIdentifier = orLog("Getting parent build target identifier", { try BuildTargetIdentifier(parent) })
+      {
+        self.targetDependencies[parentIdentifier, default: []].insert(targetIdentifier)
+      }
       targets[targetIdentifier] = (buildTarget, depth)
     }
 
@@ -527,8 +536,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
         capabilities: BuildTargetCapabilities(),
         // Be conservative with the languages that might be used in the target. SourceKit-LSP doesn't use this property.
         languageIds: [.c, .cpp, .objective_c, .objective_cpp, .swift],
-        // FIXME: (BSP migration) List the target's dependencies
-        dependencies: []
+        dependencies: self.targetDependencies[targetId, default: []].sorted { $0.uri.stringValue < $1.uri.stringValue }
       )
     }
     return BuildTargetsResponse(targets: targets)
@@ -635,27 +643,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
     }
   }
 
-  package func targets(dependingOn targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    let targetDepths = targets.compactMap { self.targets[$0]?.depth }
-    let minimumTargetDepth: Int?
-    if targetDepths.count == targets.count {
-      minimumTargetDepth = targetDepths.max()
-    } else {
-      // One of the targets didn't have an entry in self.targets. We don't know what might depend on it.
-      minimumTargetDepth = nil
-    }
-
-    // Files that occur before the target in the topological sorting don't depend on it.
-    // Ideally, we should consult the dependency graph here for more accurate dependency analysis instead of relying on
-    // a flattened list (https://github.com/swiftlang/sourcekit-lsp/issues/1312).
-    return self.targets.compactMap { (targets, value) -> BuildTargetIdentifier? in
-      if let minimumTargetDepth, value.depth >= minimumTargetDepth {
-        return nil
-      }
-      return targets
-    }
-  }
-
   package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse {
     // TODO: Support preparation of multiple targets at once. (https://github.com/swiftlang/sourcekit-lsp/issues/1262)
     for target in request.targets {

Sources/BuildSystemIntegration/TestBuildSystem.swift

@@ -80,9 +80,5 @@ package actor TestBuildSystem: BuiltInBuildSystem {
     return nil
   }
 
-  package func targets(dependingOn targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    return nil
-  }
-
   package func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) async {}
 }

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -348,25 +348,14 @@ package final actor SemanticIndexManager {
     await indexStoreUpToDateTracker.markOutOfDate(changedFiles)
 
     let targets = await changedFiles.asyncMap { await buildSystemManager.targets(for: $0) }.flatMap { $0 }
-    if let dependentTargets = await buildSystemManager.targets(dependingOn: targets) {
-      logger.info(
-        """
-        Marking targets as out-of-date: \
-        \(String(dependentTargets.map(\.uri.stringValue).joined(separator: ", ")))
-        """
-      )
-      await preparationUpToDateTracker.markOutOfDate(dependentTargets)
-    } else {
-      logger.info("Marking all targets as out-of-date")
-      await preparationUpToDateTracker.markAllKnownOutOfDate()
-      // `markAllOutOfDate` only marks targets out-of-date that have been indexed before. Also mark all targets with
-      // in-progress preparation out of date. So we don't get into the following situation, which would result in an
-      // incorrect up-to-date status of a target
-      //  - Target preparation starts for the first time
-      //  - Files changed
-      //  - Target preparation finishes.
-      await preparationUpToDateTracker.markOutOfDate(inProgressPreparationTasks.keys)
-    }
+    let dependentTargets = await buildSystemManager.targets(dependingOn: Set(targets))
+    logger.info(
+      """
+      Marking targets as out-of-date: \
+      \(String(dependentTargets.map(\.uri.stringValue).joined(separator: ", ")))
+      """
+    )
+    await preparationUpToDateTracker.markOutOfDate(dependentTargets)
 
     await scheduleBackgroundIndex(files: changedFiles, indexFilesWithUpToDateUnit: false)
   }

Sources/SwiftExtensions/CMakeLists.txt

@@ -10,6 +10,7 @@ add_library(SwiftExtensions STATIC
   PipeAsStringHandler.swift
   ResultExtensions.swift
   Sequence+AsyncMap.swift
+  Sequence+ContainsAnyIn.swift
   Task+WithPriorityChangedHandler.swift
   ThreadSafeBox.swift
 )

Sources/SwiftExtensions/Sequence+ContainsAnyIn.swift

@@ -0,0 +1,18 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+extension Sequence {
+  /// Returns `true` if the intersection of `self` and `other` is not empty.
+  package func contains(anyIn other: Set<Element>) -> Bool where Element: Hashable {
+    return self.contains { other.contains($0) }
+  }
+}