-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
cargo fix
: duplicate diagnostic detection does not handle insert-only replacements
#13027
Labels
C-bug
Category: bug
Command-fix
S-accepted
Status: Issue or feature is accepted, and has a team member available to help mentor or review
S-needs-design
Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Comments
ehuss
changed the title
duplicate diagnostic detection does not handle insert-only replacements
Nov 22, 2023
cargo fix
: duplicate diagnostic detection does not handle insert-only replacements
ehuss
added
C-bug
Category: bug
S-accepted
Status: Issue or feature is accepted, and has a team member available to help mentor or review
Command-fix
S-needs-design
Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
labels
Nov 22, 2023
rust-lang/rustfix#131 seems to work because in the case of duplicate replacements, it is enough to check if a replacement is the same before and after. A duplicate insertion wouldn't be able to do that, and would need a more fundamental fix. |
weihanglo
added a commit
to weihanglo/cargo
that referenced
this issue
Nov 1, 2024
This case also emits a insert-only suggestions (span start == end), adn doesn't rely on any lint behavior. See also * rust-lang#13728 * rust-lang#13027
weihanglo
added a commit
to weihanglo/cargo
that referenced
this issue
Nov 1, 2024
This case also emits an insert-only suggestions (span start == end), adn doesn't rely on any lint behavior. See also * rust-lang#13728 * rust-lang#13027
weihanglo
added a commit
to weihanglo/cargo
that referenced
this issue
Nov 1, 2024
This case also emits an insert-only suggestions (span start == end), and doesn't rely on any lint behavior. See also * rust-lang#13728 * rust-lang#13027
weihanglo
added a commit
to weihanglo/cargo
that referenced
this issue
Nov 1, 2024
This case also emits an insert-only suggestions (span start == end), and doesn't rely on any lint behavior. See also * rust-lang#13728 * rust-lang#13027
bors
added a commit
that referenced
this issue
Nov 5, 2024
rustfix: replace special-case duplicate handling with error ### What does this PR try to resolve? This PR changes how conflicts are handled in `rustfix`. By design, `cargo fix` repeatedly compiles the code, collects suggested changes, and applies changes that don't conflict with one another. Often, conflicts arise because the same suggestion is reported multiple times e.g., due to a macro expansion. There have been previous changes to address that specific case, but following [the PR to add transactional semantics](#14747), it makes sense to change how this is handled. Specifically, this PR adds details to `Error::AlreadyReplaced` to indicate whether the specific replacement is identical to the one it conflicts with, allowing callers to handle this case, rather than special-casing it within `replace.rs` itself. To that point, this PR changes `fix.rs` and `lib.rs` to handle identical replacements by skipping the conflicting suggestion. This is not exactly identical to their previous behavior: before, it skipped a suggestion if any _solution_ had been applied, whereas this skips it if any _replacement within a solution_ has been applied. While more expansive, this is very much the spirit of the goal described above. ### How should we test and review this PR? The existing tests have been updated to account for this change. ### Additional information See: #13027
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
C-bug
Category: bug
Command-fix
S-accepted
Status: Issue or feature is accepted, and has a team member available to help mentor or review
S-needs-design
Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
The following example:
generates a suggestion (on beta 1.55) to insert a fix like this:
However, since this diagnostic is triggered from macros, rustc emits two separate machine-applicable suggestions at the exact same spot. That causes rustfix to end up changing the code to:
which fails to compile.
rust-lang/rustfix#131 added detection to avoid applying duplicate suggestions, but I believe it does not handle the case when it is an
insert_only
suggestion.The text was updated successfully, but these errors were encountered: