diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index 20984bc40caf..b72a61a43849 100644 --- 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, "", &mut appl); let keep_pat_snip = snippet_with_applicability(cx, keep_arm.pat.span, "", &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_arms.stderr b/tests/ui/match_same_arms.stderr index 3c0382767c3f..4a4772da143a 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -20,13 +20,10 @@ LL | (1, .., 3) => 42, | ^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | (1, .., 3) | (.., 3) => 42, - | ~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm - | -LL - (.., 3) => 42, +LL ~ (1, .., 3) | (.., 3) => 42, +LL ~ _ => 0, | error: this match arm has an identical body to another arm @@ -36,13 +33,11 @@ LL | 51 => 1, | ^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | 51 | 42 => 1, - | ~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - 42 => 1, +LL - 51 => 1, +LL + 51 | 42 => 1, | error: this match arm has an identical body to another arm @@ -52,13 +47,10 @@ LL | 41 => 2, | ^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | 41 | 52 => 2, - | ~~~~~~~ -help: and remove this obsolete arm - | -LL - 52 => 2, +LL ~ 41 | 52 => 2, +LL ~ _ => 0, | error: this match arm has an identical body to another arm @@ -68,13 +60,11 @@ LL | 2 => 2, | ^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | 2 | 1 => 2, - | ~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - 1 => 2, +LL - 2 => 2, +LL + 2 | 1 => 2, | error: this match arm has an identical body to another arm @@ -84,13 +74,11 @@ LL | 3 => 2, | ^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | 3 | 1 => 2, - | ~~~~~ -help: and remove this obsolete arm - | -LL - 1 => 2, +LL ~ 2 => 2, +LL | +LL ~ 3 | 1 => 2, | error: this match arm has an identical body to another arm @@ -100,14 +88,11 @@ LL | 2 => 2, | ^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | 2 | 3 => 2, - | ~~~~~ -help: and remove this obsolete arm - | -LL - 3 => 2, -LL + +LL ~ 2 | 3 => 2, +LL | +LL ~ | error: this match arm has an identical body to another arm @@ -117,13 +102,11 @@ LL | CommandInfo::External { name, .. } => name.to_string(), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | CommandInfo::External { name, .. } | CommandInfo::BuiltIn { name, .. } => name.to_string(), - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - CommandInfo::BuiltIn { name, .. } => name.to_string(), +LL - CommandInfo::External { name, .. } => name.to_string(), +LL + CommandInfo::External { name, .. } | CommandInfo::BuiltIn { name, .. } => name.to_string(), | error: aborting due to 8 previous errors diff --git a/tests/ui/match_same_arms2.fixed b/tests/ui/match_same_arms2.fixed new file mode 100644 index 000000000000..b7d377f1ebff --- /dev/null +++ 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) {} +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 = 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::(todo!()), + 3 => core::convert::identity::(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 index dedd02e78733..dfd15d10c3d3 100644 --- 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) {} fn foo() -> bool { unimplemented!() diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index 3a28b5afc2b8..525a25e9287b 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -1,5 +1,5 @@ error: this match arm has an identical body to the `_` wildcard arm - --> tests/ui/match_same_arms2.rs:19:9 + --> tests/ui/match_same_arms2.rs:17:9 | LL | / 42 => { LL | | foo(); @@ -12,7 +12,7 @@ LL | | _ => { | = help: or try changing either arm body note: `_` wildcard arm here - --> tests/ui/match_same_arms2.rs:28:9 + --> tests/ui/match_same_arms2.rs:26:9 | LL | / _ => { LL | | foo(); @@ -26,119 +26,103 @@ LL | | }, = help: to override `-D warnings` add `#[allow(clippy::match_same_arms)]` error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:42:9 + --> tests/ui/match_same_arms2.rs:40:9 | LL | 51 => foo(), | ^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | 51 | 42 => foo(), - | ~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - 42 => foo(), +LL - 51 => foo(), +LL + 51 | 42 => foo(), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:48:9 + --> tests/ui/match_same_arms2.rs:46:9 | LL | None => 24, | ^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | None | Some(_) => 24, - | ~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - Some(_) => 24, +LL - None => 24, +LL + None | Some(_) => 24, | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:70:9 + --> tests/ui/match_same_arms2.rs:68:9 | LL | (None, Some(a)) => bar(a), | ^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | (None, Some(a)) | (Some(a), None) => bar(a), - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - (Some(a), None) => bar(a), +LL - (None, Some(a)) => bar(a), +LL + (None, Some(a)) | (Some(a), None) => bar(a), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:84:9 + --> tests/ui/match_same_arms2.rs:82:9 | LL | (None, Some(a)) if a == 42 => a, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | (None, Some(a)) | (Some(a), None) if a == 42 => a, - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - (Some(a), None) if a == 42 => a, +LL - (None, Some(a)) if a == 42 => a, +LL + (None, Some(a)) | (Some(a), None) if a == 42 => a, | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:89:9 + --> tests/ui/match_same_arms2.rs:87:9 | LL | (Some(a), ..) => bar(a), | ^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | (Some(a), ..) | (.., Some(a)) => bar(a), - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm - | -LL - (.., Some(a)) => bar(a), +LL ~ (Some(a), ..) | (.., Some(a)) => bar(a), +LL ~ _ => (), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:123:9 + --> tests/ui/match_same_arms2.rs:121:9 | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | (Ok(x), Some(_)) | (Ok(_), Some(x)) => println!("ok {}", x), - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | -LL - (Ok(_), Some(x)) => println!("ok {}", x), +LL ~ (Ok(x), Some(_)) | (Ok(_), Some(x)) => println!("ok {}", x), +LL ~ _ => println!("err"), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:139:9 + --> tests/ui/match_same_arms2.rs:137:9 | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | Ok(_) | Ok(3) => println!("ok"), - | ~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - Ok(3) => println!("ok"), +LL - Ok(_) => println!("ok"), +LL + Ok(_) | Ok(3) => println!("ok"), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:166:9 + --> tests/ui/match_same_arms2.rs:164:9 | LL | / 1 => { LL | | empty!(0); @@ -146,95 +130,82 @@ LL | | }, | |_________^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | 1 | 0 => { - | ~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - 0 => { LL - empty!(0); LL - }, +LL - 1 => { +LL + 1 | 0 => { | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:217:9 + --> tests/ui/match_same_arms2.rs:215:9 | LL | Foo::X(0) => 1, | ^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | Foo::X(0) | Foo::Z(_) => 1, - | ~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | -LL - Foo::Z(_) => 1, +LL ~ Foo::X(0) | Foo::Z(_) => 1, +LL | Foo::X(_) | Foo::Y(_) => 2, +LL ~ _ => 0, | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:227:9 + --> tests/ui/match_same_arms2.rs:225:9 | LL | Foo::Z(_) => 1, | ^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | Foo::Z(_) | Foo::X(0) => 1, - | ~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | -LL - Foo::X(0) => 1, +LL ~ Foo::Y(_) | Foo::Z(0) => 2, +LL ~ Foo::Z(_) | Foo::X(0) => 1, | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:250:9 + --> tests/ui/match_same_arms2.rs:248:9 | LL | Some(Bar { y: 0, x: 5, .. }) => 1, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns +help: or try merging the arm patterns and removing the obsolete arm | -LL | Some(Bar { y: 0, x: 5, .. }) | Some(Bar { x: 0, y: 5, .. }) => 1, - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm - | -LL - Some(Bar { x: 0, y: 5, .. }) => 1, +LL ~ Some(Bar { y: 10, z: 0, .. }) => 2, +LL | None => 50, +LL ~ Some(Bar { y: 0, x: 5, .. }) | Some(Bar { x: 0, y: 5, .. }) => 1, | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:264:9 + --> tests/ui/match_same_arms2.rs:262:9 | LL | 1 => cfg!(not_enable), | ^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | 1 | 0 => cfg!(not_enable), - | ~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - 0 => cfg!(not_enable), +LL - 1 => cfg!(not_enable), +LL + 1 | 0 => cfg!(not_enable), | error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:280:17 + --> tests/ui/match_same_arms2.rs:278:17 | LL | MaybeStaticStr::Borrowed(s) => s, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try changing either arm body -help: or try merging the arm patterns - | -LL | MaybeStaticStr::Borrowed(s) | MaybeStaticStr::Static(s) => s, - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: and remove this obsolete arm +help: or try merging the arm patterns and removing the obsolete arm | LL - MaybeStaticStr::Static(s) => s, +LL - MaybeStaticStr::Borrowed(s) => s, +LL + MaybeStaticStr::Borrowed(s) | MaybeStaticStr::Static(s) => s, | error: aborting due to 14 previous errors