[Build/PackageGraph] Switch BuildSubset.{product, target} and ModulesGraph.{product, target}(...) to use optional destination

This is mostly an NFC change. It makes more sense to default `destination`
to "undefined" when applicable because it's not always possible to know
intended destination based on user input.

Author: xedin

Sources/Build/BuildOperation.swift

@@ -526,9 +526,15 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             // FIXME: This is super unfortunate that we might need to load the package graph.
             let graph = try getPackageGraph()
 
+            let buildTriple: BuildTriple? = if let destination {
+                    destination == .host ? .tools : .destination
+                } else {
+                    nil
+                }
+
             let product = graph.product(
                 for: productName,
-                destination: destination == .host ? .tools : .destination
+                destination: buildTriple
             )
 
             guard let product else {
@@ -556,9 +562,15 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             // FIXME: This is super unfortunate that we might need to load the package graph.
             let graph = try getPackageGraph()
 
+            let buildTriple: BuildTriple? = if let destination {
+                    destination == .host ? .tools : .destination
+                } else {
+                    nil
+                }
+
             let target = graph.target(
                 for: targetName,
-                destination: destination == .host ? .tools : .destination
+                destination: buildTriple
             )
 
             guard let target else {
@@ -669,7 +681,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         // Emit warnings about any unhandled files in authored packages. We do this after applying build tool plugins, once we know what files they handled.
         // rdar://113256834 This fix works for the plugins that do not have PreBuildCommands.
         let targetsToConsider: [ResolvedModule]
-        if let subset = subset, let recursiveDependencies = try 
+        if let subset = subset, let recursiveDependencies = try
             subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) {
             targetsToConsider = recursiveDependencies
         } else {
@@ -939,18 +951,30 @@ extension BuildSubset {
         case .allExcludingTests:
             return graph.reachableTargets.filter { $0.type != .test }
         case .product(let productName, let destination):
+            let buildTriple: BuildTriple? = if let destination {
+                    destination == .host ? .tools : .destination
+                } else {
+                    nil
+                }
+
             guard let product = graph.product(
                 for: productName,
-                destination: destination == .host ? .tools : .destination
+                destination: buildTriple
             ) else {
                 observabilityScope.emit(error: "no product named '\(productName)'")
                 return nil
             }
             return try product.recursiveTargetDependencies()
         case .target(let targetName, let destination):
+            let buildTriple: BuildTriple? = if let destination {
+                    destination == .host ? .tools : .destination
+                } else {
+                    nil
+                }
+
             guard let target = graph.target(
                 for: targetName,
-                destination: destination == .host ? .tools : .destination
+                destination: buildTriple
             ) else {
                 observabilityScope.emit(error: "no target named '\(targetName)'")
                 return nil

Sources/Commands/Utilities/PluginDelegate.swift

@@ -385,7 +385,7 @@ final class PluginDelegate: PluginInvocationDelegate {
 
         // Find the target in the build operation's package graph; it's an error if we don't find it.
         let packageGraph = try buildSystem.getPackageGraph()
-        guard let target = packageGraph.target(for: targetName, destination: .destination) else {
+        guard let target = packageGraph.target(for: targetName) else {
             throw StringError("could not find a target named “\(targetName)”")
         }
 

Sources/PackageGraph/ModulesGraph.swift

@@ -166,55 +166,59 @@ public struct ModulesGraph {
         package.dependencies.compactMap { self.package(for: $0) }
     }
 
-    public func product(for name: String, destination: BuildTriple) -> ResolvedProduct? {
+    /// Find a product given a name and an optional destination. If a destination is not specified
+    /// this method uses `.destination` and falls back to `.tools` for macros, plugins, and tests.
+    public func product(for name: String, destination: BuildTriple? = .none) -> ResolvedProduct? {
         func findProduct(name: String, destination: BuildTriple) -> ResolvedProduct? {
             self.allProducts.first { $0.name == name && $0.buildTriple == destination }
         }
 
-        if let product = findProduct(name: name, destination: destination) {
-            return product
+        if let destination {
+            return findProduct(name: name, destination: destination)
         }
 
-        // FIXME: This is a temporary workaround and needs to be handled by the callers.
+        if let product = findProduct(name: name, destination: .destination) {
+            return product
+        }
 
         // It's possible to request a build of a macro, a plugin, or a test via `swift build`
         // which won't have the right destination set because it's impossible to indicate it.
         //
         // Same happens with `--test-product` - if one of the test targets directly references
         // a macro then all if its targets and the product itself become `host`.
-        if destination == .destination {
-            if let toolsProduct = findProduct(name: name, destination: .tools),
-                toolsProduct.type == .macro || toolsProduct.type == .plugin || toolsProduct.type == .test
-            {
-                return toolsProduct
-            }
+        if let toolsProduct = findProduct(name: name, destination: .tools),
+            toolsProduct.type == .macro || toolsProduct.type == .plugin || toolsProduct.type == .test
+        {
+            return toolsProduct
         }
 
         return nil
     }
 
-    public func target(for name: String, destination: BuildTriple) -> ResolvedModule? {
+    /// Find a target given a name and an optional destination. If a destination is not specified
+    /// this method uses `.destination` and falls back to `.tools` for macros, plugins, and tests.
+    public func target(for name: String, destination: BuildTriple? = .none) -> ResolvedModule? {
         func findModule(name: String, destination: BuildTriple) -> ResolvedModule? {
             self.allTargets.first { $0.name == name && $0.buildTriple == destination }
         }
 
-        if let module = findModule(name: name, destination: destination) {
-            return module
+        if let destination {
+            return findModule(name: name, destination: destination)
         }
 
-        // FIXME: This is a temporary workaround and needs to be handled by the callers.
+        if let module = findModule(name: name, destination: .destination) {
+            return module
+        }
 
         // It's possible to request a build of a macro, a plugin or a test via `swift build`
         // which won't have the right destination set because it's impossible to indicate it.
         //
         // Same happens with `--test-product` - if one of the test targets directly references
         // a macro then all if its targets and the product itself become `host`.
-        if destination == .destination {
-            if let toolsModule = findModule(name: name, destination: .tools),
-                toolsModule.type == .macro || toolsModule.type == .plugin || toolsModule.type == .test
-            {
-                return toolsModule
-            }
+        if let toolsModule = findModule(name: name, destination: .tools),
+            toolsModule.type == .macro || toolsModule.type == .plugin || toolsModule.type == .test
+        {
+            return toolsModule
         }
 
         return nil

Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

@@ -24,11 +24,13 @@ public enum BuildSubset {
     /// Represents the subset of all products and targets.
     case allIncludingTests
 
-    /// Represents a specific product.
-    case product(String, for: BuildParameters.Destination = .target)
+    /// Represents a specific product. Allows to set a specific
+    /// destination if it's known.
+    case product(String, for: BuildParameters.Destination? = .none)
 
-    /// Represents a specific target.
-    case target(String, for: BuildParameters.Destination = .target)
+    /// Represents a specific target. Allows to set a specific
+    /// destination if it's known.
+    case target(String, for: BuildParameters.Destination? = .none)
 }
 
 /// A protocol that represents a build system used by SwiftPM for all build operations. This allows factoring out the

Sources/SPMTestSupport/PackageGraphTester.swift

@@ -88,7 +88,7 @@ public final class PackageGraphResult {
 
     public func checkTarget(
         _ name: String,
-        destination: BuildTriple = .destination,
+        destination: BuildTriple? = .none,
         file: StaticString = #file,
         line: UInt = #line,
         body: (ResolvedTargetResult) -> Void
@@ -113,7 +113,7 @@ public final class PackageGraphResult {
 
     public func checkProduct(
         _ name: String,
-        destination: BuildTriple = .destination,
+        destination: BuildTriple? = .none,
         file: StaticString = #file,
         line: UInt = #line,
         body: (ResolvedProductResult) -> Void

Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift

@@ -83,7 +83,7 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 }
             }
 
-            result.checkProduct("MMIOMacros", destination: .destination) { result in
+            result.checkProduct("MMIOMacros") { result in
                 result.check(buildTriple: .tools)
             }
 
@@ -152,11 +152,11 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 }
             }
 
-            result.checkTarget("MMIOMacros", destination: .destination) { result in
+            result.checkTarget("MMIOMacros") { result in
                 result.check(buildTriple: .tools)
             }
 
-            result.checkTarget("MMIOMacrosTests", destination: .destination) { result in
+            result.checkTarget("MMIOMacrosTests") { result in
                 result.check(buildTriple: .tools)
             }
 
@@ -190,15 +190,15 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 result.check(buildTriple: .tools)
             }
 
-            result.checkProduct("MMIOPlugin", destination: .destination) { result in
+            result.checkProduct("MMIOPlugin") { result in
                 result.check(buildTriple: .tools)
             }
 
             result.checkTarget("MMIOPlugin", destination: .tools) { result in
                 result.check(buildTriple: .tools)
             }
 
-            result.checkTarget("MMIOPlugin", destination: .destination) { result in
+            result.checkTarget("MMIOPlugin") { result in
                 result.check(buildTriple: .tools)
             }
 
@@ -207,7 +207,7 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 result.check(dependencies: "MMIOPlugin", "MMIOMacros")
             }
 
-            result.checkTarget("MMIOMacro+PluginTests", destination: .destination) { result in
+            result.checkTarget("MMIOMacro+PluginTests") { result in
                 result.check(buildTriple: .tools)
                 result.check(dependencies: "MMIOPlugin", "MMIOMacros")
             }