Add cargo fix support for exported private dependency warnings - #17097
Add cargo fix support for exported private dependency warnings#17097cjc0013 wants to merge 2 commits into
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
e0e0914 to
76eaa4c
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Our contributing guide asks that features generally start as issues to explore the problem as well as the design space before going into implementation.
This seems to be an an unstable feature, which offers some flexibility, though personally I'd like to see an issue happening.
Also, for writing tests, we generally follow atomic tests and tests go first before implementation, and the the diff of the test in the fix commit shows the behavior change. You can see https://epage.github.io/dev/pr-style/#c-test for more.
|
Thanks, that makes sense. This PR was intended to implement #13095, which is already open and marked accepted; I failed to link that clearly in the PR body, so I updated the body with I also see the point on atomic test-first commits. The current history does not present a test-only behavior change before implementation, so my next author-side step is to rework the branch into that shape before asking for another review pass. |
This comment has been minimized.
This comment has been minimized.
76eaa4c to
87d7360
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
87d7360 to
960a1a3
Compare
|
Thank you for all of the work you put into this. In seeing this, it got me thinking about this again, especially with how some things have shifted around public/private dependencies and I wonder if we are ready for something like this yet. See #13095 (comment). |
Fixes #13095
Add cargo fix support for exported private dependency warnings.
This teaches Cargo to turn rustc's
exported_private_dependenciesdiagnostic into a machine-applicable manifest suggestion when the referenced dependency can be matched unambiguously inCargo.toml. The same helper is used by normal diagnostic handling and by thecargo fixrustc proxy, so bothcargo checkguidance andcargo fixapplication follow the same rules.The implementation covers regular dependencies, renamed dependencies, target-specific dependency tables, inline tables, and existing
public = falsevalues. It skips workspace-inherited dependencies and ambiguous matches rather than producing an unsafe edit.Tested with:
RUSTC_BOOTSTRAP=1 CARGO_TARGET_DIR=/tmp/cargo-13095-target cargo test --test testsuite pub_priv::fix_exported_private -- --ignored --test-threads=1RUSTC_BOOTSTRAP=1 CARGO_TARGET_DIR=/tmp/cargo-13095-target cargo test --test testsuite pub_priv::check_exported_private_dependency_mentions_cargo_fix -- --ignored --test-threads=1RUSTC_BOOTSTRAP=1 CARGO_TARGET_DIR=/tmp/cargo-13095-target cargo test --test testsuite pub_priv::exported_priv_warning -- --ignored --test-threads=1