Auto merge of #13728 - weihanglo:dedup-suggestion, r=epage

fix(cargo-fix): dont apply same suggestion twice

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()

crates/rustfix/tests/everything/dedup-suggestions.fixed.rs

@@ -0,0 +1,17 @@
+// This fixes rust-lang/rust#123304.
+// If that lint stops emitting duplicate suggestions,
+// we might need to find a substitution.
+#![warn(unsafe_op_in_unsafe_fn)]
+
+macro_rules! foo {
+    ($x:ident) => {
+        pub unsafe fn $x() { unsafe {
+            let _ = String::new().as_mut_vec();
+        }}
+    };
+}
+
+fn main() {
+    foo!(a);
+    foo!(b);
+}

crates/rustfix/tests/everything/dedup-suggestions.rs

@@ -0,0 +1,17 @@
+// This fixes rust-lang/rust#123304.
+// If that lint stops emitting duplicate suggestions,
+// we might need to find a substitution.
+#![warn(unsafe_op_in_unsafe_fn)]
+
+macro_rules! foo {
+    ($x:ident) => {
+        pub unsafe fn $x() {
+            let _ = String::new().as_mut_vec();
+        }
+    };
+}
+
+fn main() {
+    foo!(a);
+    foo!(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()),
             }
         }

tests/testsuite/fix.rs

@@ -1897,3 +1897,48 @@ warning: `foo` (lib) generated 1 warning (run `cargo fix --lib -p foo` to apply
 ")
         .run();
 }
+
+// This fixes rust-lang/rust#123304.
+// If that lint stops emitting duplicate suggestions,
+// we might need to find a substitution.
+#[cargo_test]
+fn fix_only_once_for_duplicates() {
+    let p = project()
+        .file(
+            "src/lib.rs",
+            r#"
+                #![warn(unsafe_op_in_unsafe_fn)]
+
+                macro_rules! foo {
+                    ($x:ident) => {
+                        pub unsafe fn $x() {
+                            let _ = String::new().as_mut_vec();
+                        }
+                    };
+                }
+
+                foo!(a);
+                foo!(b);
+            "#,
+        )
+        .build();
+
+    p.cargo("fix --allow-no-vcs")
+        .with_stderr(
+            "\
+[CHECKING] foo v0.0.1 ([CWD])
+[FIXED] src/lib.rs (1 fix)
+[FINISHED] `dev` profile [..]
+",
+        )
+        .run();
+
+    assert_eq!(
+        p.read_file("src/lib.rs").matches("unsafe").count(),
+        4,
+        "unsafe keyword in src/lib.rs:\n\
+            2 in lint name;\n\
+            1 from original unsafe fn;\n\
+            1 from newly-applied unsafe blocks"
+    );
+}