diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index cb784d1ff660..d86b05e5c882 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -23,8 +23,20 @@ use super::{COLLAPSIBLE_MATCH, pat_contains_disallowed_or}; pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], msrv: Msrv) { if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) { - for arm in arms { - check_arm(cx, true, arm.pat, expr, arm.body, arm.guard, Some(els_arm.body), msrv); + let last_non_wildcard = arms.iter().rposition(|arm| !arm_is_wild_like(cx, arm)); + for (idx, arm) in arms.iter().enumerate() { + let only_wildcards_after = last_non_wildcard.is_none_or(|lnw| idx >= lnw); + check_arm( + cx, + true, + arm.pat, + expr, + arm.body, + arm.guard, + Some(els_arm.body), + msrv, + only_wildcards_after, + ); } } } @@ -37,7 +49,7 @@ pub(super) fn check_if_let<'tcx>( let_expr: &'tcx Expr<'_>, msrv: Msrv, ) { - check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv); + check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv, false); } #[expect(clippy::too_many_arguments, clippy::too_many_lines)] @@ -50,6 +62,7 @@ fn check_arm<'tcx>( outer_guard: Option<&'tcx Expr<'tcx>>, outer_else_body: Option<&'tcx Expr<'tcx>>, msrv: Msrv, + only_wildcards_after: bool, ) { let inner_expr = peel_blocks_with_stmt(outer_then_body); if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) @@ -126,6 +139,7 @@ fn check_arm<'tcx>( ); }); } else if outer_is_match // Leave if-let to the `collapsible_if` lint + && only_wildcards_after // adding a guard allows fall-through; unsafe if other arms follow && let Some(inner) = If::hir(inner_expr) && outer_pat.span.eq_ctxt(inner.cond.span) && match (outer_else_body, inner.r#else) { diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d9774426ec25..e9a2b17f31b5 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -5221,7 +5221,6 @@ impl Methods { format_collect::check(cx, expr, m_arg, m_ident_span); }, Some((sym::take, take_self_arg, [take_arg], _, _)) => { - #[expect(clippy::collapsible_match)] if self.msrv.meets(cx, msrvs::STR_REPEAT) { manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg); } @@ -5546,9 +5545,7 @@ impl Methods { (sym::open, [_]) => { open_options::check(cx, expr, recv); }, - (sym::or_else, [arg]) => - { - #[expect(clippy::collapsible_match)] + (sym::or_else, [arg]) => { if !bind_instead_of_map::check_or_else_err(cx, expr, recv, arg) { unnecessary_lazy_eval::check(cx, expr, recv, arg, "or"); } @@ -5653,9 +5650,7 @@ impl Methods { (sym::try_into, []) if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::TryInto) => { unnecessary_fallible_conversions::check_method(cx, expr); }, - (sym::to_owned, []) => - { - #[expect(clippy::collapsible_match)] + (sym::to_owned, []) => { if !suspicious_to_owned::check(cx, expr, span) { implicit_clone::check(cx, name, expr, recv); } diff --git a/tests/ui/collapsible_match.rs b/tests/ui/collapsible_match.rs index 98f2fcfdf479..51cb0168c735 100644 --- a/tests/ui/collapsible_match.rs +++ b/tests/ui/collapsible_match.rs @@ -390,6 +390,26 @@ fn take(t: T) {} fn main() {} +// https://github.com/rust-lang/rust-clippy/issues/16875 +// Adding a match guard allows fall-through to subsequent arms, which changes semantics +// when non-wildcard arms follow the arm being collapsed. +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + // should NOT lint: `_ if b == 1` is not wild-like (has a guard), so collapsing + // `Some(_)` into `Some(_) if b == 0` would let `_ if b == 1` match Some values + // that previously fell through to the no-op arm body. + match a { + Some(_) => { + if b == 0 { + res = 1; + } + }, + _ if b == 1 => res = 2, + _ => {}, + } + res +} + fn issue16705(x: Option) { fn takes_ownership(s: String) -> bool { true diff --git a/tests/ui/collapsible_match_fixable.fixed b/tests/ui/collapsible_match_fixable.fixed index db76530aee14..8e5934d3693a 100644 --- a/tests/ui/collapsible_match_fixable.fixed +++ b/tests/ui/collapsible_match_fixable.fixed @@ -28,3 +28,18 @@ fn issue16558() { _ => 1, }; } + +// https://github.com/rust-lang/rust-clippy/issues/16875 +// lint still fires when only wildcard-like arms follow (fall-through is harmless) +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + match a { + Some(_) + if b == 0 => { + //~^ collapsible_match + res = 1; + }, + _ => {}, + } + res +} diff --git a/tests/ui/collapsible_match_fixable.rs b/tests/ui/collapsible_match_fixable.rs index 94bf1d6bfdfa..e2ccae455961 100644 --- a/tests/ui/collapsible_match_fixable.rs +++ b/tests/ui/collapsible_match_fixable.rs @@ -29,3 +29,19 @@ fn issue16558() { _ => 1, }; } + +// https://github.com/rust-lang/rust-clippy/issues/16875 +// lint still fires when only wildcard-like arms follow (fall-through is harmless) +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + match a { + Some(_) => { + if b == 0 { + //~^ collapsible_match + res = 1; + } + }, + _ => {}, + } + res +} diff --git a/tests/ui/collapsible_match_fixable.stderr b/tests/ui/collapsible_match_fixable.stderr index 4d501cbd0993..0be47076fa47 100644 --- a/tests/ui/collapsible_match_fixable.stderr +++ b/tests/ui/collapsible_match_fixable.stderr @@ -46,5 +46,23 @@ LL | LL ~ , | -error: aborting due to 3 previous errors +error: this `if` can be collapsed into the outer `match` + --> tests/ui/collapsible_match_fixable.rs:39:13 + | +LL | / if b == 0 { +LL | | +LL | | res = 1; +LL | | } + | |_____________^ + | +help: collapse nested if block + | +LL ~ Some(_) +LL ~ if b == 0 => { +LL | +LL | res = 1; +LL ~ }, + | + +error: aborting due to 4 previous errors