chore: use multipart_suggestions for match_same_arms

scottgerring · scottgerring · commit 9ac0f8c10e08 · 2024-12-09T17:38:49.000+01:00
diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs
@@ -74,8 +74,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
                         // check if using the same bindings as before
                         HirIdMapEntry::Occupied(entry) => return *entry.get() == b_id,
                     }
-                // the names technically don't have to match; this makes the lint more conservative
-                && cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id)
+                    // the names technically don't have to match; this makes the lint more conservative
+                    && cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id)
                     && cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b)
                     && pat_contains_local(lhs.pat, a_id)
                     && pat_contains_local(rhs.pat, b_id)
@@ -149,16 +149,12 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
                     let move_pat_snip = snippet_with_applicability(cx, move_arm.pat.span, "<pat2>", &mut appl);
                     let keep_pat_snip = snippet_with_applicability(cx, keep_arm.pat.span, "<pat1>", &mut appl);
 
-                    diag.span_suggestion(
-                        keep_arm.pat.span,
-                        "or try merging the arm patterns",
-                        format!("{keep_pat_snip} | {move_pat_snip}"),
-                        appl,
-                    )
-                    .span_suggestion(
-                        adjusted_arm_span(cx, move_arm.span),
-                        "and remove this obsolete arm",
-                        "",
+                    diag.multipart_suggestion(
+                        "or try merging the arm patterns and removing the obsolete arm",
+                        vec![
+                            (keep_arm.pat.span, format!("{keep_pat_snip} | {move_pat_snip}")),
+                            (adjusted_arm_span(cx, move_arm.span), String::new()),
+                        ],
                         appl,
                     )
                     .help("try changing either arm body");
diff --git a/tests/ui/match_same_arms2.fixed b/tests/ui/match_same_arms2.fixed
@@ -0,0 +1,259 @@
+#![warn(clippy::match_same_arms)]
+#![allow(
+    clippy::disallowed_names,
+    clippy::diverging_sub_expression,
+    clippy::uninlined_format_args,
+    clippy::match_single_binding,
+    clippy::match_like_matches_macro
+)]
+
+fn bar<T>(_: T) {}
+fn foo() -> bool {
+    unimplemented!()
+}
+
+fn match_same_arms() {
+    let _ = match 42 {
+        _ => {
+            foo();
+            let mut a = 42 + [23].len() as i32;
+            if true {
+                a += 7;
+            }
+            a = -31 - a;
+            a
+        },
+    };
+    //~^^^^^^^^^^^^^^^^^^^ ERROR: this match arm has an identical body to the `_` wildcard arm
+
+    let _ = match 42 {
+        51 | 42 => foo(), //~ ERROR: this match arm has an identical body to another arm
+        _ => true,
+    };
+
+    let _ = match Some(42) {
+        None | Some(_) => 24, //~ ERROR: this match arm has an identical body to another arm
+    };
+
+    let _ = match Some(42) {
+        Some(foo) => 24,
+        None => 24,
+    };
+
+    let _ = match Some(42) {
+        Some(42) => 24,
+        Some(a) => 24, // bindings are different
+        None => 0,
+    };
+
+    let _ = match Some(42) {
+        Some(a) if a > 0 => 24,
+        Some(a) => 24, // one arm has a guard
+        None => 0,
+    };
+
+    match (Some(42), Some(42)) {
+        (None, Some(a)) | (Some(a), None) => bar(a), //~ ERROR: this match arm has an identical body to another arm
+        _ => (),
+    }
+
+    // No warning because guards are different
+    let _ = match Some(42) {
+        Some(a) if a == 42 => a,
+        Some(a) if a == 24 => a,
+        Some(_) => 24,
+        None => 0,
+    };
+
+    let _ = match (Some(42), Some(42)) {
+        (None, Some(a)) | (Some(a), None) if a == 42 => a, //~ ERROR: this match arm has an identical body to another arm
+        _ => 0,
+    };
+
+    match (Some(42), Some(42)) {
+        (Some(a), ..) | (.., Some(a)) => bar(a), //~ ERROR: this match arm has an identical body to another arm
+        _ => (),
+    }
+
+    let _ = match Some(()) {
+        Some(()) => 0.0,
+        None => -0.0,
+    };
+
+    match (Some(42), Some("")) {
+        (Some(a), None) => bar(a),
+        (None, Some(a)) => bar(a), // bindings have different types
+        _ => (),
+    }
+
+    let x: Result<i32, &str> = Ok(3);
+
+    // No warning because of the guard.
+    match x {
+        Ok(x) if x * x == 64 => println!("ok"),
+        Ok(_) => println!("ok"),
+        Err(_) => println!("err"),
+    }
+
+    // This used to be a false positive; see issue #1996.
+    match x {
+        Ok(3) => println!("ok"),
+        Ok(x) if x * x == 64 => println!("ok 64"),
+        Ok(_) => println!("ok"),
+        Err(_) => println!("err"),
+    }
+
+    match (x, Some(1i32)) {
+        (Ok(x), Some(_)) | (Ok(_), Some(x)) => println!("ok {}", x), //~ ERROR: this match arm has an identical body to another arm
+        _ => println!("err"),
+    }
+
+    // No warning; different types for `x`.
+    match (x, Some(1.0f64)) {
+        (Ok(x), Some(_)) => println!("ok {}", x),
+        (Ok(_), Some(x)) => println!("ok {}", x),
+        _ => println!("err"),
+    }
+
+    // False negative #2251.
+    match x {
+        Ok(_tmp) => println!("ok"),
+        Ok(_) | Ok(3) => println!("ok"), //~ ERROR: this match arm has an identical body to another arm
+        Err(_) => {
+            unreachable!();
+        },
+    }
+
+    // False positive #1390
+    macro_rules! empty {
+        ($e:expr) => {};
+    }
+    match 0 {
+        0 => {
+            empty!(0);
+        },
+        1 => {
+            empty!(1);
+        },
+        x => {
+            empty!(x);
+        },
+    };
+
+    // still lint if the tokens are the same
+    match 0 {
+        1 | 0 => {
+            empty!(0);
+        },
+        x => {
+            empty!(x);
+        },
+    }
+    //~^^^^^^^ ERROR: this match arm has an identical body to another arm
+
+    match_expr_like_matches_macro_priority();
+}
+
+fn match_expr_like_matches_macro_priority() {
+    enum E {
+        A,
+        B,
+        C,
+    }
+    let x = E::A;
+    let _ans = match x {
+        E::A => false,
+        E::B => false,
+        _ => true,
+    };
+}
+
+fn main() {
+    let _ = match Some(0) {
+        Some(0) => 0,
+        Some(1) => 1,
+        #[cfg(feature = "foo")]
+        Some(2) => 2,
+        _ => 1,
+    };
+
+    enum Foo {
+        X(u32),
+        Y(u32),
+        Z(u32),
+    }
+
+    // Don't lint. `Foo::X(0)` and `Foo::Z(_)` overlap with the arm in between.
+    let _ = match Foo::X(0) {
+        Foo::X(0) => 1,
+        Foo::X(_) | Foo::Y(_) | Foo::Z(0) => 2,
+        Foo::Z(_) => 1,
+        _ => 0,
+    };
+
+    // Suggest moving `Foo::Z(_)` up.
+    let _ = match Foo::X(0) {
+        Foo::X(0) | Foo::Z(_) => 1, //~ ERROR: this match arm has an identical body to another arm
+        Foo::X(_) | Foo::Y(_) => 2,
+        _ => 0,
+    };
+
+    // Suggest moving `Foo::X(0)` down.
+    let _ = match Foo::X(0) {
+        Foo::Y(_) | Foo::Z(0) => 2,
+        Foo::Z(_) | Foo::X(0) => 1, //~ ERROR: this match arm has an identical body to another arm
+        _ => 0,
+    };
+
+    // Don't lint.
+    let _ = match 0 {
+        -2 => 1,
+        -5..=50 => 2,
+        -150..=88 => 1,
+        _ => 3,
+    };
+
+    struct Bar {
+        x: u32,
+        y: u32,
+        z: u32,
+    }
+
+    // Lint.
+    let _ = match None {
+        Some(Bar { y: 10, z: 0, .. }) => 2,
+        None => 50,
+        Some(Bar { y: 0, x: 5, .. }) | Some(Bar { x: 0, y: 5, .. }) => 1, //~ ERROR: this match arm has an identical body to another arm
+        _ => 200,
+    };
+
+    let _ = match 0 {
+        0 => todo!(),
+        1 => todo!(),
+        2 => core::convert::identity::<u32>(todo!()),
+        3 => core::convert::identity::<u32>(todo!()),
+        _ => 5,
+    };
+
+    let _ = match 0 {
+        1 | 0 => cfg!(not_enable),
+        _ => false,
+    };
+}
+
+// issue #8919, fixed on https://github.com/rust-lang/rust/pull/97312
+mod with_lifetime {
+    enum MaybeStaticStr<'a> {
+        Static(&'static str),
+        Borrowed(&'a str),
+    }
+
+    impl<'a> MaybeStaticStr<'a> {
+        fn get(&self) -> &'a str {
+            match *self {
+                MaybeStaticStr::Borrowed(s) | MaybeStaticStr::Static(s) => s,
+                //~^ ERROR: this match arm has an identical body to another arm
+            }
+        }
+    }
+}
diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs
@@ -7,8 +7,6 @@
     clippy::match_like_matches_macro
 )]
 
-//@no-rustfix: need to change the suggestion to a multipart suggestion
-
 fn bar<T>(_: T) {}
 fn foo() -> bool {
     unimplemented!()
diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr
