Skip to content

Correct "Did you mean XYZ" messages when a dependency is not found. #8303

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.

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 6 commits into from
Mar 3, 2025

Conversation

yyvch
Copy link
Contributor

@yyvch yyvch commented Feb 22, 2025

Correct "Did you mean XYZ" messages when a dependency is not found.

Motivation:

I found that "Did you mean XYZ" messages quite often aren't helpful. Specifically, scenarios which I found very frustrating:

Scenario 1:

  • I have a package MylibKit, and I forgot to declare a product there. I have a module Awesomeness there.
  • I have a package NeatPak and I want to use Awesomeness there. I add package dependency to MylibKit and a product dependency .product(name="Awesomeness", package="MylibKit")
  • SwiftPM errors out and adds Did you mean '.product(name="Awesomeness", package="MylibKit")'. I triple-checked what it suggested vs what I had, and after some staring, I checked MylibKit and indeed, it was missing a product definition. It was pretty frustrating that it errored out with suggestion which clearly wasn't working.

Scenario 2:

  • I have a package MylibKit that I refactored out of my existing package, and I declared product Awesomeness there.
  • I have a package NeatPak and I want to use Awesomeness there. I add package dependency to MylibKit and a dependency by name "Awesomenes", making a typo.
  • SwiftPM errors out and adds Did you mean 'Awesomeness with correct spelling.
  • I update dependency to "Awesomeness".
  • Now SwiftPM suggests Did you mean '.product(name="Awesomeness", package="MylibKit")', and it's pretty frustrating why it didn't suggest it in the first place.

Modifications:

  • Update implementation of how SwiftPM gathers what's acceptable destinations for the dependency, fixing the mentioned scenarios.
  • Expanded tests to include the mentioned scenarios.
  • Extracted error generation from createResolvedPackages ever so slightly reducing its complexity

Result:

  • All tests pass.
  • More accurate error messages with better suggestions.

Before: the test scenario asserted two cases: that a missing
dependency triggers error; that a similar name includes a suggested
correction.

After: the test scenario asserts one case: that a missing dependency
triggers error.

Motivation: similar names already have dedicated tests, cleaning this
up to provide more clarity on intent of the tests.
…cenarios.

Refactor test and split into two to make it clear what scenario does it verify.
Assert looks the same between the two tests, but the setup is different:
 - testByNameDependencyWithSimilarTargetName verifies typo in dependencies
   between modules within the same package.
 - testByNameDependencyWithSimilarProductName verifies typo in dependencies
   on a product from a different package.
Before: occasionally swiftpm suggested dependencies which aren't
accurate, since it looked for global list of modules for matches,
while actual depedencies are on products.

After: swiftpm suggestions now search through
 - local modules
 - global products
 - package-specific products if explicit package is used in the deps

Motivation: inaccurate suggestions which made me distrust swiftpm.
@yyvch
Copy link
Contributor Author

yyvch commented Feb 22, 2025

@swift-ci please test

Copy link
Contributor

@bripeticca bripeticca left a comment

Choose a reason for hiding this comment

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

LGTM! And thank you for the comments in your code, really helps to clarify everything!

@yyvch yyvch requested a review from plemarquand February 26, 2025 23:30
@yyvch
Copy link
Contributor Author

yyvch commented Feb 27, 2025

@swift-ci please test

@plemarquand
Copy link
Contributor

@swift-ci test windows

@plemarquand
Copy link
Contributor

@swift-ci please test windows

@plemarquand plemarquand merged commit 3bb87a0 into swiftlang:main Mar 3, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants