Skip to content

Improve error message about the PackageGraphError.productDependencyNotFound #7419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 30, 2024
Merged

Improve error message about the PackageGraphError.productDependencyNotFound #7419

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b26a745
make allTargetNames Type Array<String>
k-kohey Mar 24, 2024
3b12d72
add "Did you mean '.product(...) to description"
k-kohey Mar 25, 2024
d9cb4af
fix comment
k-kohey Mar 25, 2024
daa29ac
Update Sources/PackageGraph/ModulesGraph+Loading.swift
k-kohey Mar 29, 2024
13f0a2a
these lines will be deleted as they have been moved elsewhere
k-kohey Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions Sources/PackageGraph/ModulesGraph+Loading.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,8 @@ private func createResolvedPackages(
)
try dupProductsChecker.run(lookupByProductIDs: moduleAliasingUsed, observabilityScope: observabilityScope)

// The set of all target names.
var allTargetNames = Set<String>()
// The array of all target names.
let allTargetNames = packageBuilders.reduce([]) { $0 + $1.targets }.map(\.target.name)

// Track if multiple targets are found with the same name.
var foundDuplicateTarget = false
Expand Down Expand Up @@ -494,7 +494,8 @@ private func createResolvedPackages(
// Establish dependencies in each target.
for targetBuilder in packageBuilder.targets {
// Record if we see a duplicate target.
foundDuplicateTarget = foundDuplicateTarget || !allTargetNames.insert(targetBuilder.target.name).inserted
let isDuplicateTarget = 1 < allTargetNames.filter { $0 == targetBuilder.target.name }.count
foundDuplicateTarget = foundDuplicateTarget || isDuplicateTarget

// Directly add all the system module dependencies.
targetBuilder.dependencies += implicitSystemTargetDeps.map { .target($0, conditions: []) }
Expand Down Expand Up @@ -524,13 +525,22 @@ private func createResolvedPackages(
} else {
// 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(allTargetNames))
var packageContainingBestMatchedProduct: String?
if let bestMatchedProductName, productRef.name == bestMatchedProductName {
let dependentPackages = packageBuilder.dependencies.map(\.package)
for p in dependentPackages where p.targets.contains(where: { $0.name == bestMatchedProductName }) {
packageContainingBestMatchedProduct = p.identity.description
break
}
}
let error = PackageGraphError.productDependencyNotFound(
package: package.identity.description,
targetName: targetBuilder.target.name,
dependencyProductName: productRef.name,
dependencyPackageName: productRef.package,
dependencyProductInDecl: !declProductsAsDependency.isEmpty,
similarProductName: bestMatchedProductName
similarProductName: bestMatchedProductName,
packageContainingSimilarProduct: packageContainingBestMatchedProduct
)
packageObservabilityScope.emit(error)
}
Expand Down Expand Up @@ -568,7 +578,7 @@ private func createResolvedPackages(
// If a target with similar name was encountered before, we emit a diagnostic.
if foundDuplicateTarget {
var duplicateTargets = [String: [Package]]()
for targetName in allTargetNames.sorted() {
for targetName in Set(allTargetNames).sorted() {
let packages = packageBuilders
.filter({ $0.targets.contains(where: { $0.target.name == targetName }) })
.map{ $0.package }
Expand Down
8 changes: 5 additions & 3 deletions Sources/PackageGraph/ModulesGraph.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ enum PackageGraphError: Swift.Error {
case cycleDetected((path: [Manifest], cycle: [Manifest]))

/// The product dependency not found.
case productDependencyNotFound(package: String, targetName: String, dependencyProductName: String, dependencyPackageName: String?, dependencyProductInDecl: Bool, similarProductName: String?)
case productDependencyNotFound(package: String, targetName: String, dependencyProductName: String, dependencyPackageName: String?, dependencyProductInDecl: Bool, similarProductName: String?, packageContainingSimilarProduct: String?)

/// The package dependency already satisfied by a different dependency package
case dependencyAlreadySatisfiedByIdentifier(package: String, dependencyLocation: String, otherDependencyURL: String, identity: PackageIdentity)
Expand Down Expand Up @@ -230,12 +230,14 @@ extension PackageGraphError: CustomStringConvertible {
(cycle.path + cycle.cycle).map({ $0.displayName }).joined(separator: " -> ") +
" -> " + cycle.cycle[0].displayName

case .productDependencyNotFound(let package, let targetName, let dependencyProductName, let dependencyPackageName, let dependencyProductInDecl, let similarProductName):
case .productDependencyNotFound(let package, let targetName, let dependencyProductName, let dependencyPackageName, let dependencyProductInDecl, let similarProductName, let packageContainingSimilarProduct):
if dependencyProductInDecl {
return "product '\(dependencyProductName)' is declared in the same package '\(package)' and can't be used as a dependency for target '\(targetName)'."
} else {
var description = "product '\(dependencyProductName)' required by package '\(package)' target '\(targetName)' \(dependencyPackageName.map{ "not found in package '\($0)'" } ?? "not found")."
if let similarProductName {
if let similarProductName, let packageContainingSimilarProduct {
description += " Did you mean '.product(name: \"\(similarProductName)\", package: \"\(packageContainingSimilarProduct)\")'?"
Copy link
Contributor Author

@k-kohey k-kohey Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote the following test code to test the output of this message.

func test_() throws {
    let fs = InMemoryFileSystem(emptyFiles:
        "/zzz/Sources/zzz/main.swift",
        "/aaa/Sources/aaa/source.swift"
    )

    let observability = ObservabilitySystem.makeForTesting()
    let g = try loadModulesGraph(
        fileSystem: fs,
        manifests: [
            Manifest.createRootManifest(
                displayName: "zzz",
                path: "/zzz",
                dependencies: [
                    .remoteSourceControl(
                        url: "https://example.com/example.git",
                        requirement: .exact("1.3.0")
                    )
                ],
                products: [],
                targets: [
                    TargetDescription(
                        name: "zzz",
                        dependencies: ["aaa"],
                        type: .executable
                    )
                ]),
            Manifest.createRemoteSourceControlManifest(
                displayName: "aaa",
                url: "https://example.com/example.git",
                path: "/aaa",
                version: "1.3.0",
                products: [
                    ProductDescription(
                        name: "aaa",
                        type: .library(.automatic),
                        targets: ["aaa"]
                    )
                ],
                targets: [
                    TargetDescription(name: "aaa")
                ]
            )
        ],
        observabilityScope: observability.topScope
    )

    testDiagnostics(observability.diagnostics) { result in
        result.check(
            diagnostic: "error: 'zzz': product 'aaa' required by package 'zzz' target 'repro' not found. Did you mean '.product(name: \"aaa\", package: \"aaa\")'?",
            severity: .error
        )
    }
}

However, this test fails with the message "test_(): failed - No diagnostics left to check".
I would like to know how to solve this problem.

} else if let similarProductName {
description += " Did you mean '\(similarProductName)'?"
}
return description
Expand Down
51 changes: 51 additions & 0 deletions Tests/PackageGraphTests/ModulesGraphTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2676,6 +2676,57 @@ final class ModulesGraphTests: XCTestCase {

XCTAssertEqual(observability.diagnostics.count, 0, "unexpected diagnostics: \(observability.diagnostics.map { $0.description })")
}

func testDependencyResolutionWithErrorMessages() throws {
let fs = InMemoryFileSystem(emptyFiles:
"/aaa/Sources/aaa/main.swift",
"/zzz/Sources/zzz/source.swift"
)

let observability = ObservabilitySystem.makeForTesting()
let _ = try loadModulesGraph(
fileSystem: fs,
manifests: [
Manifest.createRootManifest(
displayName: "aaa",
path: "/aaa",
dependencies: [
.localSourceControl(path: "/zzz", requirement: .upToNextMajor(from: "1.0.0"))
],
products: [],
targets: [
TargetDescription(
name: "aaa",
dependencies: ["zzy"],
type: .executable
)
]),
Manifest.createRootManifest(
displayName: "zzz",
path: "/zzz",
products: [
ProductDescription(
name: "zzz",
type: .library(.automatic),
targets: ["zzz"]
)
],
targets: [
TargetDescription(
name: "zzz"
)
])
],
observabilityScope: observability.topScope
)

testDiagnostics(observability.diagnostics) { result in
result.check(
diagnostic: "product 'zzy' required by package 'aaa' target 'aaa' not found. Did you mean 'zzz'?",
Copy link
Contributor Author

@k-kohey k-kohey Mar 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fix will cause the error message to include "Did you mean...".
Previously, this message was not displayed depending on the name of the package directory.

severity: .error
)
}
}
}


Expand Down