Use BuildDescription.traverseModules to get the target targets of a SwiftPM package

Author: ahoppen

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -182,15 +182,13 @@ package actor SwiftPMBuildSystem {
   /// The entry point via with we can access the `SourceKitLSPAPI` provided by SwiftPM.
   private var buildDescription: SourceKitLSPAPI.BuildDescription?
 
-  private var modulesGraph: ModulesGraph
+  /// Maps source and header files to the target that include them.
+  private var fileToTargets: [DocumentURI: Set<ConfiguredTarget>] = [:]
 
-  private var fileToTargets: [DocumentURI: [SwiftBuildTarget]] = [:]
-  private var sourceDirToTargets: [DocumentURI: [SwiftBuildTarget]] = [:]
-
-  /// Maps configured targets ids to their SwiftPM build target as well as an index in their topological sorting.
-  ///
-  /// Targets with lower index are more low level, ie. targets with higher indices depend on targets with lower indices.
-  private var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:]
+  /// Maps configured targets ids to their SwiftPM build target as well as the depth at which they occur in the build
+  /// graph. Top level targets on which no other target depends have a depth of `1`. Targets with dependencies have a
+  /// greater depth.
+  private var targets: [ConfiguredTarget: (buildTarget: SwiftBuildTarget, depth: Int)] = [:]
 
   /// Creates a build system using the Swift Package Manager, if this workspace is a package.
   ///
@@ -316,12 +314,6 @@ package actor SwiftPMBuildSystem {
       flags: buildFlags
     )
 
-    self.modulesGraph = try ModulesGraph(
-      rootPackages: [],
-      packages: IdentifiableSet(),
-      dependencies: [],
-      binaryArtifacts: [:]
-    )
     self.reloadPackageStatusCallback = reloadPackageStatusCallback
 
     // The debounce duration of 500ms was chosen arbitrarily without scientific research.
@@ -404,45 +396,26 @@ extension SwiftPMBuildSystem {
     /// Make sure to execute any throwing statements before setting any
     /// properties because otherwise we might end up in an inconsistent state
     /// with only some properties modified.
-    self.modulesGraph = modulesGraph
-
-    self.targets = Dictionary(
-      try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in
-        return (key: ConfiguredTarget(target), value: (index, target))
-      },
-      uniquingKeysWith: { first, second in
-        logger.fault("Found two targets with the same name \(first.buildTarget.name)")
-        return second
-      }
-    )
 
-    self.fileToTargets = [DocumentURI: [SwiftBuildTarget]](
-      modulesGraph.allModules.flatMap { target in
-        return target.sources.paths.compactMap { (filePath) -> (key: DocumentURI, value: [SwiftBuildTarget])? in
-          guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else {
-            return nil
-          }
-          return (key: DocumentURI(filePath.asURL), value: [buildTarget])
+    self.targets = [:]
+    self.fileToTargets = [:]
+    buildDescription.traverseModules { buildTarget, parent, depth in
+      let configuredTarget = ConfiguredTarget(buildTarget)
+      var depth = depth
+      if let existingDepth = targets[configuredTarget]?.depth {
+        depth = max(existingDepth, depth)
+      } else {
+        for source in buildTarget.sources + buildTarget.headers {
+          fileToTargets[DocumentURI(source), default: []].insert(configuredTarget)
         }
-      },
-      uniquingKeysWith: { $0 + $1 }
-    )
-
-    self.sourceDirToTargets = [DocumentURI: [SwiftBuildTarget]](
-      modulesGraph.allModules.compactMap { (target) -> (DocumentURI, [SwiftBuildTarget])? in
-        guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else {
-          return nil
-        }
-        return (key: DocumentURI(target.sources.root.asURL), value: [buildTarget])
-      },
-      uniquingKeysWith: { $0 + $1 }
-    )
+      }
+      targets[configuredTarget] = (buildTarget, depth)
+    }
 
-    guard let delegate = self.delegate else {
-      return
+    if let delegate = self.delegate {
+      await delegate.fileBuildSettingsChanged(self.watchedFiles)
+      await delegate.fileHandlingCapabilityChanged()
     }
-    await delegate.fileBuildSettingsChanged(self.watchedFiles)
-    await delegate.fileHandlingCapabilityChanged()
     for testFilesDidChangeCallback in testFilesDidChangeCallbacks {
       await testFilesDidChangeCallback()
     }
@@ -556,7 +529,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
 
     let targets = buildTargets(for: uri)
     if !targets.isEmpty {
-      return targets.map(ConfiguredTarget.init)
+      return targets.sorted { ($0.targetID, $0.runDestinationID) < ($1.targetID, $1.runDestinationID) }
     }
 
     if path.basename == "Package.swift"
@@ -567,10 +540,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
       return [ConfiguredTarget.forPackageManifest]
     }
 
-    if let targets = try? inferredTargets(for: path) {
-      return targets
-    }
-
     return []
   }
 
@@ -580,27 +549,27 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
 
   package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
     return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in
-      let lhsIndex = self.targets[lhs]?.index ?? self.targets.count
-      let rhsIndex = self.targets[rhs]?.index ?? self.targets.count
-      return lhsIndex < rhsIndex
+      let lhsDepth = self.targets[lhs]?.depth ?? 0
+      let rhsDepth = self.targets[rhs]?.depth ?? 0
+      return lhsDepth > rhsDepth
     }
   }
 
   package func targets(dependingOn targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
-    let targetIndices = targets.compactMap { self.targets[$0]?.index }
-    let minimumTargetIndex: Int?
-    if targetIndices.count == targets.count {
-      minimumTargetIndex = targetIndices.min()
+    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.
-      minimumTargetIndex = nil
+      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 { (configuredTarget, value) -> ConfiguredTarget? in
-      if let minimumTargetIndex, value.index <= minimumTargetIndex {
+      if let minimumTargetDepth, value.depth >= minimumTargetDepth {
         return nil
       }
       return configuredTarget
@@ -725,7 +694,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
   }
 
   /// Returns the resolved target descriptions for the given file, if one is known.
-  private func buildTargets(for file: DocumentURI) -> [SwiftBuildTarget] {
+  private func buildTargets(for file: DocumentURI) -> Set<ConfiguredTarget> {
     if let targets = fileToTargets[file] {
       return targets
     }
@@ -775,7 +744,9 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
       guard event.uri.fileURL?.pathExtension == "swift", let targets = fileToTargets[event.uri] else {
         continue
       }
-      filesWithUpdatedDependencies.formUnion(targets.flatMap(\.sources).map(DocumentURI.init))
+      for target in targets {
+        filesWithUpdatedDependencies.formUnion(self.targets[target]?.buildTarget.sources.map(DocumentURI.init) ?? [])
+      }
     }
 
     // If a `.swiftmodule` file is updated, this means that we have performed a build / are
@@ -801,66 +772,28 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
   }
 
   package func sourceFiles() -> [SourceFileInfo] {
-    return fileToTargets.compactMap { (uri, targets) -> SourceFileInfo? in
-      // We should only set mayContainTests to `true` for files from test targets
-      // (https://github.com/swiftlang/sourcekit-lsp/issues/1174).
-      return SourceFileInfo(
-        uri: uri,
-        isPartOfRootProject: targets.contains(where: \.isPartOfRootPackage),
-        mayContainTests: true
-      )
+    var sourceFiles: [DocumentURI: SourceFileInfo] = [:]
+    for (buildTarget, depth) in self.targets.values {
+      for sourceFile in buildTarget.sources {
+        let uri = DocumentURI(sourceFile)
+        sourceFiles[uri] = SourceFileInfo(
+          uri: uri,
+          isPartOfRootProject: depth == 1 || (sourceFiles[uri]?.isPartOfRootProject ?? false),
+          mayContainTests: true
+        )
+      }
     }
+    return sourceFiles.values.sorted { $0.uri.pseudoPath < $1.uri.pseudoPath }
   }
 
   package func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async {
     testFilesDidChangeCallbacks.append(callback)
   }
-}
-
-extension SwiftPMBuildSystem {
-
-  // MARK: Implementation details
 
   /// Retrieve settings for a package manifest (Package.swift).
   private func settings(forPackageManifest path: AbsolutePath) throws -> FileBuildSettings? {
-    func impl(_ path: AbsolutePath) -> FileBuildSettings? {
-      for package in modulesGraph.packages where path == package.manifest.path {
-        let compilerArgs = workspace.interpreterFlags(for: package.path) + [path.pathString]
-        return FileBuildSettings(compilerArguments: compilerArgs)
-      }
-      return nil
-    }
-
-    if let result = impl(path) {
-      return result
-    }
-
-    let canonicalPath = try resolveSymlinks(path)
-    return canonicalPath == path ? nil : impl(canonicalPath)
-  }
-
-  /// This finds the target a file belongs to based on its location in the file system.
-  ///
-  /// This is primarily intended to find the target a header belongs to.
-  private func inferredTargets(for path: AbsolutePath) throws -> [ConfiguredTarget] {
-    func impl(_ path: AbsolutePath) throws -> [ConfiguredTarget] {
-      var dir = path.parentDirectory
-      while !dir.isRoot {
-        if let buildTargets = sourceDirToTargets[DocumentURI(dir.asURL)] {
-          return buildTargets.map(ConfiguredTarget.init)
-        }
-        dir = dir.parentDirectory
-      }
-      return []
-    }
-
-    let result = try impl(path)
-    if !result.isEmpty {
-      return result
-    }
-
-    let canonicalPath = try resolveSymlinks(path)
-    return try canonicalPath == path ? [] : impl(canonicalPath)
+    let compilerArgs = workspace.interpreterFlags(for: path.parentDirectory) + [path.pathString]
+    return FileBuildSettings(compilerArguments: compilerArgs)
   }
 }
 

Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift

@@ -656,8 +656,8 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         .compilerArguments
       XCTAssertNotNil(argsManifest)
 
-      assertArgumentsDoNotContain(manifest.pathString, arguments: argsManifest ?? [])
-      assertArgumentsContain(try resolveSymlinks(manifest).pathString, arguments: argsManifest ?? [])
+      assertArgumentsContain(manifest.pathString, arguments: argsManifest ?? [])
+      assertArgumentsDoNotContain(try resolveSymlinks(manifest).pathString, arguments: argsManifest ?? [])
     }
   }