Skip to content

Commit

Permalink
fix(rustfix): dont apply same suggestion twice
Browse files Browse the repository at this point in the history
This assumes that if any of the machine applicable fixes in
a diagnostic suggestion is a duplicate, we should see the
entire suggestion as a duplicate.
  • Loading branch information
weihanglo committed Apr 10, 2024
1 parent 5b05a3b commit 95edc06
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
11 changes: 11 additions & 0 deletions crates/rustfix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,19 @@ impl CodeFix {

/// Applies multiple `suggestions` to the given `code`.
pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<String, Error> {
let mut already_applied = HashSet::new();
let mut fix = CodeFix::new(code);
for suggestion in suggestions.iter().rev() {
// This assumes that if any of the machine applicable fixes in
// a diagnostic suggestion is a duplicate, we should see the
// entire suggestion as a duplicate.
if suggestion
.solutions
.iter()
.any(|sol| !already_applied.insert(sol))
{
continue;
}
fix.apply(suggestion)?;
}
fix.finish()
Expand Down
4 changes: 2 additions & 2 deletions crates/rustfix/tests/everything/dedup-suggestions.fixed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

macro_rules! foo {
($x:ident) => {
pub unsafe fn $x() { unsafe { unsafe {
pub unsafe fn $x() { unsafe {
let _ = String::new().as_mut_vec();
}}}
}}
};
}

Expand Down
17 changes: 14 additions & 3 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,12 +735,23 @@ fn rustfix_and_fix(
});
let mut fixed = CodeFix::new(&code);

// As mentioned above in `rustfix_crate`, we don't immediately warn
// about suggestions that fail to apply here, and instead we save them
// off for later processing.
let mut already_applied = HashSet::new();
for suggestion in suggestions.iter().rev() {
// This assumes that if any of the machine applicable fixes in
// a diagnostic suggestion is a duplicate, we should see the
// entire suggestion as a duplicate.
if suggestion
.solutions
.iter()
.any(|sol| !already_applied.insert(sol))
{
continue;
}
match fixed.apply(suggestion) {
Ok(()) => fixed_file.fixes_applied += 1,
// As mentioned above in `rustfix_crate`, we don't immediately
// warn about suggestions that fail to apply here, and instead
// we save them off for later processing.
Err(e) => fixed_file.errors_applying_fixes.push(e.to_string()),
}
}
Expand Down
10 changes: 5 additions & 5 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1924,21 +1924,21 @@ fn fix_only_once_for_duplicates() {
.build();

p.cargo("fix --allow-no-vcs")
.with_stderr_contains(
.with_stderr(
"\
[CHECKING] foo v0.0.1 ([CWD])
[FIXED] src/lib.rs (2 fixes)
[FIXED] src/lib.rs (1 fix)
[FINISHED] `dev` profile [..]
",
)
.with_stderr_contains("[WARNING] unnecessary `unsafe` block[..]")
.run();

assert_eq!(
p.read_file("src/lib.rs").matches("unsafe").count(),
5,
4,
"unsafe keyword in src/lib.rs:\n\
2 in lint name;\n\
1 from original unsafe fn;\n\
2 from newly-applied unsafe blocks"
1 from newly-applied unsafe blocks"
);
}

0 comments on commit 95edc06

Please sign in to comment.