diff options
| -rw-r--r-- | clippy_lints/src/casts/cast_possible_truncation.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/matches/match_same_arms.rs | 66 | ||||
| -rw-r--r-- | tests/ui/match_same_arms2.rs | 14 | ||||
| -rw-r--r-- | tests/ui/match_same_arms2.stderr | 51 |
4 files changed, 87 insertions, 47 deletions
diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index f99a51e2b88..fbbb559319e 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -131,8 +131,7 @@ pub(super) fn check( let cast_from_ptr_size = def.repr().int.map_or(true, |ty| matches!(ty, IntegerType::Pointer(_),)); let suffix = match (cast_from_ptr_size, is_isize_or_usize(cast_to)) { - (false, false) if from_nbits > to_nbits => "", - (true, false) if from_nbits > to_nbits => "", + (_, false) if from_nbits > to_nbits => "", (false, true) if from_nbits > 64 => "", (false, true) if from_nbits > 32 => " on targets with 32-bit wide pointers", _ => return, diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index d645e6c6c05..ae302f0f075 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -64,38 +64,50 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { let min_index = usize::min(lindex, rindex); let max_index = usize::max(lindex, rindex); - let mut local_map: HirIdMap<HirId> = HirIdMap::default(); - let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { - if let Some(a_id) = path_to_local(a) - && let Some(b_id) = path_to_local(b) - && let entry = match local_map.entry(a_id) { - HirIdMapEntry::Vacant(entry) => entry, - // check if using the same bindings as before - HirIdMapEntry::Occupied(entry) => return *entry.get() == b_id, - } + let check_eq_with_pat = |expr_a: &Expr<'_>, expr_b: &Expr<'_>| { + let mut local_map: HirIdMap<HirId> = HirIdMap::default(); + let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { + if let Some(a_id) = path_to_local(a) + && let Some(b_id) = path_to_local(b) + && let entry = match local_map.entry(a_id) { + HirIdMapEntry::Vacant(entry) => entry, + // 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) - && 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) - { - entry.insert(b_id); - true - } else { - false - } - }; - // Arms with a guard are ignored, those can’t always be merged together - // If both arms overlap with an arm in between then these can't be merged either. - !(backwards_blocking_idxs[max_index] > min_index && forwards_blocking_idxs[min_index] < max_index) - && lhs.guard.is_none() - && rhs.guard.is_none() - && SpanlessEq::new(cx) - .expr_fallback(eq_fallback) - .eq_expr(lhs.body, rhs.body) + && 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) + { + entry.insert(b_id); + true + } else { + false + } + }; + + SpanlessEq::new(cx) + .expr_fallback(eq_fallback) + .eq_expr(expr_a, expr_b) // these checks could be removed to allow unused bindings && bindings_eq(lhs.pat, local_map.keys().copied().collect()) && bindings_eq(rhs.pat, local_map.values().copied().collect()) + }; + + let check_same_guard = || match (&lhs.guard, &rhs.guard) { + (None, None) => true, + (Some(lhs_guard), Some(rhs_guard)) => check_eq_with_pat(lhs_guard, rhs_guard), + _ => false, + }; + + let check_same_body = || check_eq_with_pat(lhs.body, rhs.body); + + // Arms with different guard are ignored, those can’t always be merged together + // If both arms overlap with an arm in between then these can't be merged either. + !(backwards_blocking_idxs[max_index] > min_index && forwards_blocking_idxs[min_index] < max_index) + && check_same_guard() + && check_same_body() }; let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index 3428ff45906..85ad0962eb4 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -67,6 +67,20 @@ fn match_same_arms() { _ => (), } + // 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)) { + (Some(a), None) if a == 42 => a, + (None, Some(a)) if a == 42 => a, //~ ERROR: this match arm has an identical body to another arm + _ => 0, + }; + match (Some(42), Some(42)) { (Some(a), ..) => bar(a), //~ ERROR: this match arm has an identical body to another arm (.., Some(a)) => bar(a), diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index 6abbb582763..f4c38c1af89 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -71,7 +71,22 @@ LL | (Some(a), None) => bar(a), | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:71:9 + --> tests/ui/match_same_arms2.rs:80:9 + | +LL | (None, Some(a)) if a == 42 => a, + | ---------------^^^^^^^^^^^^^^^^ + | | + | help: try merging the arm patterns: `(None, Some(a)) | (Some(a), None)` + | + = help: or try changing either arm body +note: other arm here + --> tests/ui/match_same_arms2.rs:79:9 + | +LL | (Some(a), None) if a == 42 => a, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this match arm has an identical body to another arm + --> tests/ui/match_same_arms2.rs:85:9 | LL | (Some(a), ..) => bar(a), | -------------^^^^^^^^^^ @@ -80,13 +95,13 @@ LL | (Some(a), ..) => bar(a), | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:72:9 + --> tests/ui/match_same_arms2.rs:86:9 | LL | (.., Some(a)) => bar(a), | ^^^^^^^^^^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:105:9 + --> tests/ui/match_same_arms2.rs:119:9 | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ----------------^^^^^^^^^^^^^^^^^^^^^^^^ @@ -95,13 +110,13 @@ LL | (Ok(x), Some(_)) => println!("ok {}", x), | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:106:9 + --> tests/ui/match_same_arms2.rs:120:9 | LL | (Ok(_), Some(x)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:121:9 + --> tests/ui/match_same_arms2.rs:135:9 | LL | Ok(_) => println!("ok"), | -----^^^^^^^^^^^^^^^^^^ @@ -110,13 +125,13 @@ LL | Ok(_) => println!("ok"), | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:120:9 + --> tests/ui/match_same_arms2.rs:134:9 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:148:9 + --> tests/ui/match_same_arms2.rs:162:9 | LL | 1 => { | ^ help: try merging the arm patterns: `1 | 0` @@ -128,7 +143,7 @@ LL | | }, | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:145:9 + --> tests/ui/match_same_arms2.rs:159:9 | LL | / 0 => { LL | | empty!(0); @@ -136,7 +151,7 @@ LL | | }, | |_________^ error: match expression looks like `matches!` macro - --> tests/ui/match_same_arms2.rs:167:16 + --> tests/ui/match_same_arms2.rs:181:16 | LL | let _ans = match x { | ________________^ @@ -150,7 +165,7 @@ LL | | }; = help: to override `-D warnings` add `#[allow(clippy::match_like_matches_macro)]` error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:199:9 + --> tests/ui/match_same_arms2.rs:213:9 | LL | Foo::X(0) => 1, | ---------^^^^^ @@ -159,13 +174,13 @@ LL | Foo::X(0) => 1, | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:201:9 + --> tests/ui/match_same_arms2.rs:215:9 | LL | Foo::Z(_) => 1, | ^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:209:9 + --> tests/ui/match_same_arms2.rs:223:9 | LL | Foo::Z(_) => 1, | ---------^^^^^ @@ -174,13 +189,13 @@ LL | Foo::Z(_) => 1, | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:207:9 + --> tests/ui/match_same_arms2.rs:221:9 | LL | Foo::X(0) => 1, | ^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:232:9 + --> tests/ui/match_same_arms2.rs:246:9 | LL | Some(Bar { y: 0, x: 5, .. }) => 1, | ----------------------------^^^^^ @@ -189,13 +204,13 @@ LL | Some(Bar { y: 0, x: 5, .. }) => 1, | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:229:9 + --> tests/ui/match_same_arms2.rs:243:9 | LL | Some(Bar { x: 0, y: 5, .. }) => 1, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this match arm has an identical body to another arm - --> tests/ui/match_same_arms2.rs:246:9 + --> tests/ui/match_same_arms2.rs:260:9 | LL | 1 => cfg!(not_enable), | -^^^^^^^^^^^^^^^^^^^^ @@ -204,10 +219,10 @@ LL | 1 => cfg!(not_enable), | = help: or try changing either arm body note: other arm here - --> tests/ui/match_same_arms2.rs:245:9 + --> tests/ui/match_same_arms2.rs:259:9 | LL | 0 => cfg!(not_enable), | ^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 13 previous errors +error: aborting due to 14 previous errors |