Take run destinations into account for SwiftPM build targets

We can have two targets with the same name in a SwiftPM workspace, one for a build target and one for the destination. We need to be able to tell them apart based on the run destination.

Author: ahoppen

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -63,6 +63,26 @@ private func getDefaultToolchain(_ toolchainRegistry: ToolchainRegistry) async -
   return await toolchainRegistry.default
 }
 
+fileprivate extension BuildTriple {
+  /// A string that can be used to identify the build triple in `ConfiguredTarget.runDestinationID`.
+  var id: String {
+    switch self {
+    case .tools:
+      return "tools"
+    case .destination:
+      return "destination"
+    }
+  }
+}
+
+fileprivate extension ConfiguredTarget {
+  init(_ buildTarget: any SwiftBuildTarget) {
+    self.init(targetID: buildTarget.name, runDestinationID: buildTarget.buildTriple.id)
+  }
+
+  static let forPackageManifest = ConfiguredTarget(targetID: "", runDestinationID: "")
+}
+
 /// Swift Package Manager build system and workspace support.
 ///
 /// This class implements the `BuildSystem` interface to provide the build settings for a Swift
@@ -98,13 +118,13 @@ public actor SwiftPMBuildSystem {
   let fileSystem: FileSystem
   private let toolchainRegistry: ToolchainRegistry
 
-  var fileToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
-  var sourceDirToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
+  var fileToTargets: [AbsolutePath: [SwiftBuildTarget]] = [:]
+  var sourceDirToTargets: [AbsolutePath: [SwiftBuildTarget]] = [:]
 
-  /// Maps target ids (aka. `ConfiguredTarget.targetID`) 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.
-  var targets: [String: (index: Int, buildTarget: 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.
+  var targets: [ConfiguredTarget: (index: Int, buildTarget: SwiftBuildTarget)] = [:]
 
   /// The URIs for which the delegate has registered for change notifications,
   /// mapped to the language the delegate specified when registering for change notifications.
@@ -292,40 +312,34 @@ extension SwiftPMBuildSystem {
 
     self.targets = Dictionary(
       try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in
-        return (key: target.name, (index, target))
+        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.fileToTarget = [AbsolutePath: SwiftBuildTarget](
+    self.fileToTargets = [AbsolutePath: [SwiftBuildTarget]](
       modulesGraph.allTargets.flatMap { target in
-        return target.sources.paths.compactMap {
+        return target.sources.paths.compactMap { (filePath) -> (key: AbsolutePath, value: [SwiftBuildTarget])? in
           guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else {
             return nil
           }
-          return (key: $0, value: buildTarget)
+          return (key: filePath, value: [buildTarget])
         }
       },
-      uniquingKeysWith: { td, _ in
-        // FIXME: is there  a preferred target?
-        return td
-      }
+      uniquingKeysWith: { $0 + $1 }
     )
 
-    self.sourceDirToTarget = [AbsolutePath: SwiftBuildTarget](
-      modulesGraph.allTargets.compactMap { (target) -> (AbsolutePath, SwiftBuildTarget)? in
+    self.sourceDirToTargets = [AbsolutePath: [SwiftBuildTarget]](
+      modulesGraph.allTargets.compactMap { (target) -> (AbsolutePath, [SwiftBuildTarget])? in
         guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else {
           return nil
         }
-        return (key: target.sources.root, value: buildTarget)
+        return (key: target.sources.root, value: [buildTarget])
       },
-      uniquingKeysWith: { td, _ in
-        // FIXME: is there  a preferred target?
-        return td
-      }
+      uniquingKeysWith: { $0 + $1 }
     )
 
     guard let delegate = self.delegate else {
@@ -365,11 +379,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       return nil
     }
 
-    if configuredTarget.targetID == "" {
+    if configuredTarget == .forPackageManifest {
       return try settings(forPackageManifest: path)
     }
 
-    guard let buildTarget = self.targets[configuredTarget.targetID]?.buildTarget else {
+    guard let buildTarget = self.targets[configuredTarget]?.buildTarget else {
       logger.error("Did not find target with name \(configuredTarget.targetID)")
       return nil
     }
@@ -399,27 +413,29 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       return []
     }
 
-    if let target = try? buildTarget(for: path) {
-      return [ConfiguredTarget(targetID: target.name, runDestinationID: "dummy")]
+    if let targets = orLog("Getting build targets for file", { try buildTargets(for: path) }), !targets.isEmpty {
+      return targets.map(ConfiguredTarget.init)
     }
 
     if path.basename == "Package.swift" {
       // We use an empty target name to represent the package manifest since an empty target name is not valid for any
       // user-defined target.
-      return [ConfiguredTarget(targetID: "", runDestinationID: "dummy")]
+      return [ConfiguredTarget.forPackageManifest]
     }
 
-    if url.pathExtension == "h", let target = try? target(forHeader: path) {
-      return [target]
+    if url.pathExtension == "h", let targets = orLog("Getting targets for header", { try targets(forHeader: path) }),
+      !targets.isEmpty
+    {
+      return targets
     }
 
     return []
   }
 
   public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
     return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in
-      let lhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
-      let rhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
+      let lhsIndex = self.targets[lhs]?.index ?? self.targets.count
+      let rhsIndex = self.targets[lhs]?.index ?? self.targets.count
       return lhsIndex < rhsIndex
     }
   }
@@ -494,19 +510,19 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
     self.watchedFiles.remove(uri)
   }
 
-  /// Returns the resolved target description for the given file, if one is known.
-  private func buildTarget(for file: AbsolutePath) throws -> SwiftBuildTarget? {
-    if let td = fileToTarget[file] {
-      return td
+  /// Returns the resolved target descriptions for the given file.
+  private func buildTargets(for file: AbsolutePath) throws -> [SwiftBuildTarget] {
+    if let targets = fileToTargets[file] {
+      return targets
     }
 
     let realpath = try resolveSymlinks(file)
-    if realpath != file, let td = fileToTarget[realpath] {
-      fileToTarget[file] = td
-      return td
+    if realpath != file, let targets = fileToTargets[realpath] {
+      fileToTargets[file] = targets
+      return targets
     }
 
-    return nil
+    return []
   }
 
   /// An event is relevant if it modifies a file that matches one of the file rules used by the SwiftPM workspace.
@@ -547,11 +563,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       guard let url = event.uri.fileURL,
         url.pathExtension == "swift",
         let absolutePath = try? AbsolutePath(validating: url.path),
-        let target = fileToTarget[absolutePath]
+        let targets = fileToTargets[absolutePath]
       else {
         continue
       }
-      filesWithUpdatedDependencies.formUnion(target.sources.map { DocumentURI($0) })
+      filesWithUpdatedDependencies.formUnion(targets.flatMap(\.sources).map(DocumentURI.init))
     }
 
     // If a `.swiftmodule` file is updated, this means that we have performed a build / are
@@ -564,7 +580,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
     // All of this shouldn't be necessary once we have background preparation, in which case we know when preparation of
     // a target has finished.
     if events.contains(where: { $0.uri.fileURL?.pathExtension == "swiftmodule" }) {
-      filesWithUpdatedDependencies.formUnion(self.fileToTarget.keys.map { DocumentURI($0.asURL) })
+      filesWithUpdatedDependencies.formUnion(self.fileToTargets.keys.map { DocumentURI($0.asURL) })
     }
     await self.fileDependenciesUpdatedDebouncer.scheduleCall(filesWithUpdatedDependencies)
   }
@@ -577,12 +593,12 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
   }
 
   public func sourceFiles() -> [SourceFileInfo] {
-    return fileToTarget.compactMap { (path, target) -> SourceFileInfo? in
+    return fileToTargets.compactMap { (path, targets) -> SourceFileInfo? in
       // We should only set mayContainTests to `true` for files from test targets
       // (https://github.com/apple/sourcekit-lsp/issues/1174).
       return SourceFileInfo(
         uri: DocumentURI(path.asURL),
-        isPartOfRootProject: target.isPartOfRootPackage,
+        isPartOfRootProject: targets.contains(where: \.isPartOfRootPackage),
         mayContainTests: true
       )
     }
@@ -615,25 +631,26 @@ extension SwiftPMBuildSystem {
     return canonicalPath == path ? nil : impl(canonicalPath)
   }
 
-  /// This finds the target the header belongs to based on its location in the file system.
-  private func target(forHeader path: AbsolutePath) throws -> ConfiguredTarget? {
-    func impl(_ path: AbsolutePath) throws -> ConfiguredTarget? {
+  /// This finds the targets the header belongs to based on its location in the file system.
+  private func targets(forHeader path: AbsolutePath) throws -> [ConfiguredTarget] {
+    func impl(_ path: AbsolutePath) throws -> [ConfiguredTarget] {
       var dir = path.parentDirectory
       while !dir.isRoot {
-        if let buildTarget = sourceDirToTarget[dir] {
-          return ConfiguredTarget(targetID: buildTarget.name, runDestinationID: "dummy")
+        if let buildTargets = sourceDirToTargets[dir], !buildTargets.isEmpty {
+          return buildTargets.map(ConfiguredTarget.init)
         }
         dir = dir.parentDirectory
       }
-      return nil
+      return []
     }
 
-    if let result = try impl(path) {
+    let result = try impl(path)
+    if !result.isEmpty {
       return result
     }
 
     let canonicalPath = try resolveSymlinks(path)
-    return try canonicalPath == path ? nil : impl(canonicalPath)
+    return try canonicalPath == path ? [] : impl(canonicalPath)
   }
 }
 

Sources/SKTestSupport/SwiftPMTestProject.swift

@@ -53,7 +53,7 @@ public class SwiftPMTestProject: MultiFileTestProject {
     for (fileLocation, contents) in files {
       let directories =
         switch fileLocation.directories.first {
-        case "Sources", "Tests":
+        case "Sources", "Tests", "Plugins":
           fileLocation.directories
         case nil:
           ["Sources", "MyLibrary"]

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -373,4 +373,57 @@ final class BackgroundIndexingTests: XCTestCase {
 
     withExtendedLifetime(project) {}
   }
+
+  func testLibraryUsedByExecutableTargetAndPackagePlugin() async throws {
+    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Lib/MyFile.swift": """
+        public func 1️⃣foo() {}
+        """,
+        "MyExec/MyExec.swift": """
+        import Lib
+        func bar() {
+          2️⃣foo()
+        }
+        """,
+        "Plugins/MyPlugin/MyPlugin.swift": """
+        import PackagePlugin
+
+        @main
+        struct MyPlugin: CommandPlugin {
+          func performCommand(context: PluginContext, arguments: [String]) async throws {}
+        }
+        """,
+      ],
+      manifest: """
+        // swift-tools-version: 5.7
+
+        import PackageDescription
+
+        let package = Package(
+          name: "MyLibrary",
+          targets: [
+           .target(name: "Lib"),
+           .executableTarget(name: "MyExec", dependencies: ["Lib"]),
+           .plugin(
+             name: "MyPlugin",
+             capability: .command(
+               intent: .sourceCodeFormatting(),
+               permissions: []
+             ),
+             dependencies: ["MyExec"]
+           )
+          ]
+        )
+        """,
+      serverOptions: backgroundIndexingOptions
+    )
+
+    let (uri, positions) = try project.openDocument("MyExec.swift")
+    let definition = try await project.testClient.send(
+      DefinitionRequest(textDocument: TextDocumentIdentifier(uri), position: positions["2️⃣"])
+    )
+    XCTAssertEqual(definition, .locations([try project.location(from: "1️⃣", to: "1️⃣", in: "MyFile.swift")]))
+  }
 }