improve and simplify validation of target based dependencies (#3280)

motivation: SwiftPM 5.2 introduced target based dependencies which added complexity at code level and to users, this PR tries to both simplify the "rules" and the code

changes:
* allow target dependency in the form of ".product(name:, package:)" where package parameter is the last segment of the dependecy URL, which is the most intuitive choice
* change validation code to encourage the above from instead of encouraging adding "name" attibute to the dependency decleration, as we want to get away from adding this attribute in the long run
* add several tests that capture the numerous permutations we are coding for

Author: tomerd

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -141,7 +141,6 @@ extension PackageGraph {
         )
 
         let rootPackages = resolvedPackages.filter{ rootManifestSet.contains($0.manifest) }
-
         checkAllDependenciesAreUsed(rootPackages, diagnostics)
 
         return try PackageGraph(
@@ -414,37 +413,23 @@ private func createResolvedPackages(
                     continue
                 }
 
-                // If package name is mentioned, ensure it is valid.
-                if let packageName = productRef.package {
-                    // Find the declared package and check that it contains
-                    // the product we found above.
-                    guard let dependencyPackage = packageMapByNameForTargetDependencyResolutionOnly[packageName], dependencyPackage.products.contains(product) else {
-                        let error = PackageGraphError.productDependencyIncorrectPackage(
-                            name: productRef.name,
-                            package: packageName
-                        )
-                        diagnostics.emit(error, location: diagnosticLocation())
-                        continue
-                    }
-                } else if packageBuilder.package.manifest.toolsVersion >= .v5_2 {
-                    // Starting in 5.2, and target-based dependency, we require target product dependencies to
-                    // explicitly reference the package containing the product, or for the product, package and
-                    // dependency to share the same name. We don't check this in manifest loading for root-packages so
-                    // we can provide a more detailed diagnostic here.
+                // Starting in 5.2, and target-based dependency, we require target product dependencies to
+                // explicitly reference the package containing the product, or for the product, package and
+                // dependency to share the same name. We don't check this in manifest loading for root-packages so
+                // we can provide a more detailed diagnostic here.
+                if packageBuilder.package.manifest.toolsVersion >= .v5_2 && productRef.package == nil{
                     let referencedPackageIdentity = identityResolver.resolveIdentity(for: product.packageBuilder.package.manifest.packageLocation)
-                    guard let packageDependency = (packageBuilder.package.manifest.dependencies.first { package in
+                    guard let referencedPackageDependency = (packageBuilder.package.manifest.dependencies.first { package in
                         return package.identity == referencedPackageIdentity
                     }) else {
                         throw InternalError("dependency reference for \(product.packageBuilder.package.manifest.packageLocation) not found")
                     }
-
-                    let packageName = product.packageBuilder.package.name
-                    if productRef.name != packageDependency.nameForTargetDependencyResolutionOnly || packageDependency.nameForTargetDependencyResolutionOnly != packageName {
+                    let referencedPackageName = referencedPackageDependency.nameForTargetDependencyResolutionOnly
+                    if productRef.name !=  referencedPackageName {
                         let error = PackageGraphError.productDependencyMissingPackage(
                             productName: productRef.name,
                             targetName: targetBuilder.target.name,
-                            packageName: packageName,
-                            packageDependency: packageDependency
+                            packageDependency: referencedPackageDependency
                         )
                         diagnostics.emit(error, location: diagnosticLocation())
                     }

Sources/PackageGraph/PackageGraph.swift

@@ -21,9 +21,6 @@ enum PackageGraphError: Swift.Error {
     /// The product dependency not found.
     case productDependencyNotFound(dependencyProductName: String, dependencyPackageName: String?, packageName: String, targetName: String)
 
-    /// The product dependency was found but the package name did not match.
-    case productDependencyIncorrectPackage(name: String, package: String)
-
     /// The package dependency name does not match the package name.
     case incorrectPackageDependencyName(dependencyPackageName: String, dependencyName: String, dependencyLocation: String, resolvedPackageName: String, resolvedPackageURL: String)
 
@@ -37,7 +34,6 @@ enum PackageGraphError: Swift.Error {
     case productDependencyMissingPackage(
         productName: String,
         targetName: String,
-        packageName: String,
         packageDependency: PackageDependencyDescription
     )
 
@@ -173,9 +169,6 @@ extension PackageGraphError: CustomStringConvertible {
         case .productDependencyNotFound(let dependencyProductName, let dependencyPackageName, let packageName, let targetName):
             return "product '\(dependencyProductName)' \(dependencyPackageName.map{ "not found in package '\($0)'" } ?? "not found"). it is required by package '\(packageName)' target '\(targetName)'."
 
-        case .productDependencyIncorrectPackage(let name, let package):
-            return "product dependency '\(name)' in package '\(package)' not found"
-
         case .incorrectPackageDependencyName(let dependencyPackageName, let dependencyName, let dependencyURL, let resolvedPackageName, let resolvedPackageURL):
             return """
                 '\(dependencyPackageName)' dependency on '\(dependencyURL)' has an explicit name '\(dependencyName)' which does not match the \
@@ -191,32 +184,14 @@ extension PackageGraphError: CustomStringConvertible {
         case .productDependencyMissingPackage(
                 let productName,
                 let targetName,
-                let packageName,
                 let packageDependency
             ):
 
-            var solutionSteps: [String] = []
-
-            // If the package dependency name is the same as the package name, or if the product name and package name
-            // don't correspond, we need to rewrite the target dependency to explicit specify the package name.
-            if packageDependency.nameForTargetDependencyResolutionOnly == packageName || productName != packageName {
-                solutionSteps.append("""
-                    reference the package in the target dependency with '.product(name: "\(productName)", package: \
-                    "\(packageName)")'
-                    """)
-            }
-
-            // If the name of the product and the package are the same, or if the package dependency implicit name
-            // deduced from the URL is not correct, we need to rewrite the package dependency declaration to specify the
-            // package name.
-            if productName == packageName || packageDependency.nameForTargetDependencyResolutionOnly != packageName {
-                let dependencySwiftRepresentation = packageDependency.swiftRepresentation(overridingName: packageName)
-                solutionSteps.append("""
-                    provide the name of the package dependency with '\(dependencySwiftRepresentation)'
-                    """)
-            }
+            let solution = """
+            reference the package in the target dependency with '.product(name: "\(productName)", package: \
+            "\(packageDependency.nameForTargetDependencyResolutionOnly)")'
+            """
 
-            let solution = solutionSteps.joined(separator: " and ")
             return "dependency '\(productName)' in target '\(targetName)' requires explicit declaration; \(solution)"
 
         case .duplicateProduct(let product, let packages):

Sources/PackageModel/Manifest.swift

@@ -272,7 +272,7 @@ public final class Manifest: ObjectIdentifierProtocol {
             return nil
         }
 
-        return dependencies.first(where: { $0.nameForTargetDependencyResolutionOnly == packageName })
+        return self.dependencies.first(where: { $0.nameForTargetDependencyResolutionOnly == packageName })
     }
 
     /// Registers a required product with a particular dependency if possible, or registers it as unknown.