From 95edc06e5bcfca0ca5e6c09d8843be8b60488165 Mon Sep 17 00:00:00 2001
From: Weihang Lo <me@weihanglo.tw>
Date: Tue, 9 Apr 2024 18:10:11 -0400
Subject: [PATCH] fix(rustfix): dont apply same suggestion twice

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.
---
 crates/rustfix/src/lib.rs                       | 11 +++++++++++
 .../tests/everything/dedup-suggestions.fixed.rs |  4 ++--
 src/cargo/ops/fix.rs                            | 17 ++++++++++++++---
 tests/testsuite/fix.rs                          | 10 +++++-----
 4 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/crates/rustfix/src/lib.rs b/crates/rustfix/src/lib.rs
index 8af88cd50..2f6acaf30 100644
--- a/crates/rustfix/src/lib.rs
+++ b/crates/rustfix/src/lib.rs
@@ -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()
diff --git a/crates/rustfix/tests/everything/dedup-suggestions.fixed.rs b/crates/rustfix/tests/everything/dedup-suggestions.fixed.rs
index 9542382df..cd3b8d153 100644
--- a/crates/rustfix/tests/everything/dedup-suggestions.fixed.rs
+++ b/crates/rustfix/tests/everything/dedup-suggestions.fixed.rs
@@ -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();
-        }}}
+        }}
     };
 }
 
diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs
index 703e118e9..6c34abc37 100644
--- a/src/cargo/ops/fix.rs
+++ b/src/cargo/ops/fix.rs
@@ -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()),
             }
         }
diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs
index 53014ae83..babdd3085 100644
--- a/tests/testsuite/fix.rs
+++ b/tests/testsuite/fix.rs
@@ -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"
     );
 }