diff --git a/Sources/PackageGraph/ModulesGraph+Loading.swift b/Sources/PackageGraph/ModulesGraph+Loading.swift index a95c87266c1..20d881b639f 100644 --- a/Sources/PackageGraph/ModulesGraph+Loading.swift +++ b/Sources/PackageGraph/ModulesGraph+Loading.swift @@ -685,31 +685,11 @@ private func createResolvedPackages( // found errors when there are more important errors to // resolve (like authentication issues). if !observabilityScope.errorsReportedInAnyScope { - // Emit error if a product (not module) declared in the package is also a productRef (dependency) - let declProductsAsDependency = package.products.filter { product in - lookupByProductIDs ? product.identity == productRef.identity : product.name == productRef.name - }.map {$0.modules}.flatMap{$0}.filter { t in - t.name != productRef.name - } - - // Find a product name from the available product dependencies that is most similar to the required product name. - let bestMatchedProductName = bestMatch(for: productRef.name, from: Array(allModuleNames)) - var packageContainingBestMatchedProduct: String? - if let bestMatchedProductName, productRef.name == bestMatchedProductName { - let dependentPackages = packageBuilder.dependencies.map(\.package) - for p in dependentPackages where p.modules.contains(where: { $0.name == bestMatchedProductName }) { - packageContainingBestMatchedProduct = p.identity.description - break - } - } - let error = PackageGraphError.productDependencyNotFound( - package: package.identity.description, - moduleName: moduleBuilder.module.name, - dependencyProductName: productRef.name, - dependencyPackageName: productRef.package, - dependencyProductInDecl: !declProductsAsDependency.isEmpty, - similarProductName: bestMatchedProductName, - packageContainingSimilarProduct: packageContainingBestMatchedProduct + let error = prepareProductDependencyNotFoundError( + packageBuilder: packageBuilder, + moduleBuilder: moduleBuilder, + dependency: productRef, + lookupByProductIDs: lookupByProductIDs ) packageObservabilityScope.emit(error) } @@ -838,6 +818,110 @@ private func createResolvedPackages( return IdentifiableSet(try packageBuilders.map { try $0.construct() }) } +private func prepareProductDependencyNotFoundError( + packageBuilder: ResolvedPackageBuilder, + moduleBuilder: ResolvedModuleBuilder, + dependency: Module.ProductReference, + lookupByProductIDs: Bool +) -> PackageGraphError { + let packageName = packageBuilder.package.identity.description + // Module's dependency is either a local module or a product from another package. + // If dependency is a product from the current package, that's an incorrect + // declaration of the dependency and we should show relevant error. Let's see + // if indeed the dependency matches any of the products. + let declProductsAsDependency = packageBuilder.package.products.filter { product in + lookupByProductIDs ? product.identity == dependency.identity : product.name == dependency.name + }.flatMap(\.modules).filter { t in + t.name != dependency.name + } + if !declProductsAsDependency.isEmpty { + return PackageGraphError.productDependencyNotFound( + package: packageName, + moduleName: moduleBuilder.module.name, + dependencyProductName: dependency.name, + dependencyPackageName: dependency.package, + dependencyProductInDecl: true, + similarProductName: nil, + packageContainingSimilarProduct: nil + ) + } + + // If dependency name is a typo, find best possible match from the available destinations. + // Depending on how the dependency is declared, "available destinations" might be: + // - modules within the current package + // - products across all packages in the graph + // - products from a specific package + var packageContainingBestMatchedProduct: String? + var bestMatchedProductName: String? + if dependency.package == nil { + // First assume it's a dependency on modules within the same package. + let localModules = Array(packageBuilder.modules.map(\.module.name).filter { $0 != moduleBuilder.module.name }) + bestMatchedProductName = bestMatch(for: dependency.name, from: localModules) + if bestMatchedProductName != nil { + return PackageGraphError.productDependencyNotFound( + package: packageName, + moduleName: moduleBuilder.module.name, + dependencyProductName: dependency.name, + dependencyPackageName: nil, + dependencyProductInDecl: false, + similarProductName: bestMatchedProductName, + packageContainingSimilarProduct: nil + ) + } + // Since there's no package name in the dependency declaration, and no match across + // the local modules, we assume the user actually meant to use product dependency, + // but didn't specify package to use the product from. Since products are globally + // unique, we should be able to find a good match across the graph, if the package + // is already a part of the dependency tree. + let availableProducts = Dictionary( + uniqueKeysWithValues: packageBuilder.dependencies + .flatMap { (packageDep: ResolvedPackageBuilder) -> [( + String, + String + )] in + let manifestProducts = packageDep.package.manifest.products.map(\.name) + let explicitProducts = packageDep.package.products.filter { manifestProducts.contains($0.name) } + let explicitIdsOrNames = Set(explicitProducts.map { lookupByProductIDs ? $0.identity : $0.name }) + return explicitIdsOrNames.map { ($0, packageDep.package.identity.description) } + } + ) + bestMatchedProductName = bestMatch(for: dependency.name, from: Array(availableProducts.keys)) + if bestMatchedProductName != nil { + packageContainingBestMatchedProduct = availableProducts[bestMatchedProductName!] + } + return PackageGraphError.productDependencyNotFound( + package: packageName, + moduleName: moduleBuilder.module.name, + dependencyProductName: dependency.name, + dependencyPackageName: nil, + dependencyProductInDecl: false, + similarProductName: bestMatchedProductName, + packageContainingSimilarProduct: packageContainingBestMatchedProduct + ) + } else { + // Package is explicitly listed in the product dependency, we shall search + // within the products from that package. + let availableProducts = packageBuilder.dependencies + .filter { $0.package.identity.description == dependency.package } + .flatMap { (packageDep: ResolvedPackageBuilder) -> [String] in + let manifestProducts = packageDep.package.manifest.products.map(\.name) + let explicitProducts = packageDep.package.products.filter { manifestProducts.contains($0.name) } + let explicitIdsOrNames = Set(explicitProducts.map { lookupByProductIDs ? $0.identity : $0.name }) + return Array(explicitIdsOrNames) + } + bestMatchedProductName = bestMatch(for: dependency.name, from: availableProducts) + return PackageGraphError.productDependencyNotFound( + package: packageName, + moduleName: moduleBuilder.module.name, + dependencyProductName: dependency.name, + dependencyPackageName: dependency.package, + dependencyProductInDecl: false, + similarProductName: bestMatchedProductName, + packageContainingSimilarProduct: dependency.package + ) + } +} + private func emitDuplicateProductDiagnostic( productName: String, packages: [Package], diff --git a/Tests/PackageGraphTests/ModulesGraphTests.swift b/Tests/PackageGraphTests/ModulesGraphTests.swift index 11ce25959ef..a5e649a0c16 100644 --- a/Tests/PackageGraphTests/ModulesGraphTests.swift +++ b/Tests/PackageGraphTests/ModulesGraphTests.swift @@ -957,10 +957,11 @@ final class ModulesGraphTests: XCTestCase { } } - func testProductDependencyWithSimilarName() throws { - let fs = InMemoryFileSystem(emptyFiles: - "/Foo/Sources/Foo/foo.swift", - "/Bar/Sources/Bar/bar.swift" + func testByNameDependencyWithSimilarTargetName() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/railroad/Sources/Rail/Rail.swift", + "/railroad/Sources/Spike/Spike.swift" ) let observability = ObservabilitySystem.makeForTesting() @@ -968,29 +969,211 @@ final class ModulesGraphTests: XCTestCase { fileSystem: fs, manifests: [ Manifest.createRootManifest( - displayName: "Foo", - path: "/Foo", + displayName: "railroad", + path: "/railroad", targets: [ - TargetDescription(name: "Foo", dependencies: ["Barx"]), - ]), + TargetDescription(name: "Rail", dependencies: ["Spoke"]), + TargetDescription(name: "Spike"), + ] + ), + ], + observabilityScope: observability.topScope + ) + + testDiagnostics(observability.diagnostics) { result in + result.check( + diagnostic: "product 'Spoke' required by package 'railroad' target 'Rail' not found. Did you mean 'Spike'?", + severity: .error + ) + } + } + + func testByNameDependencyWithSimilarProductName() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/weather/Sources/Rain/Rain.swift", + "/forecast/Sources/Forecast/Forecast.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + _ = try loadModulesGraph( + fileSystem: fs, + manifests: [ + Manifest.createFileSystemManifest( + displayName: "weather", + path: "/weather", + products: [ + ProductDescription(name: "Rain", type: .library(.automatic), targets: ["Rain"]), + ], + targets: [ + TargetDescription(name: "Rain"), + ] + ), Manifest.createRootManifest( - displayName: "Bar", - path: "/Bar", + displayName: "forecast", + path: "/forecast", + dependencies: [.fileSystem(path: "/weather")], targets: [ - TargetDescription(name: "Bar") - ]), + TargetDescription(name: "Forecast", dependencies: ["Rail"]), + ] + ), + ], + observabilityScope: observability.topScope + ) + + testDiagnostics(observability.diagnostics) { result in + result.check( + diagnostic: "product 'Rail' required by package 'forecast' target 'Forecast' not found. Did you mean '.product(name: \"Rain\", package: \"weather\")'?", + severity: .error + ) + } + } + + func testProductDependencyWithSimilarNamesFromMultiplePackages() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/flavors/Sources/Bitter/Bitter.swift", + "/farm/Sources/Butter/Butter.swift", + "/grocery/Sources/Grocery/Grocery.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + _ = try loadModulesGraph( + fileSystem: fs, + manifests: [ + Manifest.createFileSystemManifest( + displayName: "flavors", + path: "/flavors", + products: [ProductDescription(name: "Bitter", type: .library(.automatic), targets: ["Bitter"])], + targets: [ + TargetDescription(name: "Bitter"), + ] + ), + Manifest.createFileSystemManifest( + displayName: "farm", + path: "/farm", + products: [ProductDescription(name: "Butter", type: .library(.automatic), targets: ["Butter"])], + targets: [ + TargetDescription(name: "Butter"), + ] + ), + Manifest.createRootManifest( + displayName: "grocery", + path: "/grocery", + dependencies: [.fileSystem(path: "/farm"), .fileSystem(path: "/flavors")], + targets: [ + TargetDescription(name: "Grocery", dependencies: [ + .product(name: "Biter", package: "farm"), + .product(name: "Bitter", package: "flavors"), + ]), + ] + ), + ], + observabilityScope: observability.topScope + ) + + // We should expect matching to work only within the package we want even + // though there are lexically closer candidates in other packages. + testDiagnostics(observability.diagnostics) { result in + result.check( + diagnostic: "product 'Biter' required by package 'grocery' target 'Grocery' not found in package 'farm'. Did you mean '.product(name: \"Butter\", package: \"farm\")'?", + severity: .error + ) + } + } + + func testProductDependencyWithSimilarNamesFromProductTargetsNotProducts() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/lunch/Sources/Lunch/Lunch.swift", + "/sandwich/Sources/Sandwich/Sandwich.swift", + "/sandwich/Sources/Bread/Bread.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + _ = try loadModulesGraph( + fileSystem: fs, + manifests: [ + Manifest.createFileSystemManifest( + displayName: "sandwich", + path: "/sandwich", + products: [ProductDescription( + name: "Sandwich", + type: .library(.automatic), + targets: ["Sandwich"] + )], + targets: [ + TargetDescription(name: "Sandwich", dependencies: ["Bread"]), + TargetDescription(name: "Bread"), + ] + ), + Manifest.createRootManifest( + displayName: "lunch", + path: "/lunch", + // Depends on a product which isn't actually declared in sandwich, + // but there's a target with the same name. + dependencies: [.fileSystem(path: "/sandwich")], + targets: [ + TargetDescription(name: "Lunch", dependencies: [.product(name: "Bread", package: "sandwich")]), + ] + ), + ], + observabilityScope: observability.topScope + ) + + testDiagnostics(observability.diagnostics) { result in + result.check( + diagnostic: "product 'Bread' required by package 'lunch' target 'Lunch' not found in package 'sandwich'.", + severity: .error + ) + } + } + + func testProductDependencyWithSimilarNamesFromLocalTargetsNotPackageProducts() throws { + let fs = InMemoryFileSystem( + emptyFiles: + "/gauges/Sources/Chart/Chart.swift", + "/gauges/Sources/Value/Value.swift", + "/controls/Sources/Valve/Valve.swift" + ) + + let observability = ObservabilitySystem.makeForTesting() + _ = try loadModulesGraph( + fileSystem: fs, + manifests: [ + Manifest.createFileSystemManifest( + displayName: "controls", + path: "/controls", + products: [ProductDescription(name: "Valve", type: .library(.automatic), targets: ["Valve"])], + targets: [ + TargetDescription(name: "Valve"), + ] + ), + Manifest.createRootManifest( + displayName: "gauges", + path: "/gauges", + // Target dependency should show the local target dependency, even though + // there's a lexically-close product name in a different package. + dependencies: [.fileSystem(path: "/controls")], + targets: [ + TargetDescription(name: "Chart", dependencies: [ + "Valv", + .product(name: "Valve", package: "controls")]), + TargetDescription(name: "Value"), + ] + ), ], observabilityScope: observability.topScope ) testDiagnostics(observability.diagnostics) { result in result.check( - diagnostic: "product 'Barx' required by package 'foo' target 'Foo' not found. Did you mean 'Bar'?", + diagnostic: "product 'Valv' required by package 'gauges' target 'Chart' not found. Did you mean 'Value'?", severity: .error ) } } - + func testProductDependencyWithNonSimilarName() throws { let fs = InMemoryFileSystem(emptyFiles: "/Foo/Sources/Foo/foo.swift", @@ -2868,11 +3051,11 @@ final class ModulesGraphTests: XCTestCase { targets: [ TargetDescription( name: "aaa", - dependencies: ["zzy"], + dependencies: ["mmm"], type: .executable ) ]), - Manifest.createRootManifest( + Manifest.createFileSystemManifest( displayName: "zzz", path: "/zzz", products: [ @@ -2893,7 +3076,7 @@ final class ModulesGraphTests: XCTestCase { testDiagnostics(observability.diagnostics) { result in result.check( - diagnostic: "product 'zzy' required by package 'aaa' target 'aaa' not found. Did you mean 'zzz'?", + diagnostic: "product 'mmm' required by package 'aaa' target 'aaa' not found.", severity: .error ) }