From d254020f3c4adc9d9bfd7902938e77fa0de91cfe Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 4 Sep 2025 00:44:43 +0200 Subject: [PATCH 1/3] misc --- clippy_lints/src/matches/collapsible_match.rs | 28 +++++++++---------- tests/ui/collapsible_match.rs | 24 ++++++++-------- tests/ui/collapsible_match.stderr | 12 ++++---- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index aaf559fc4439..11e79333a694 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -50,15 +50,17 @@ fn check_arm<'tcx>( if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) && let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner { IfLetOrMatch::IfLet(scrutinee, pat, _, els, _) => Some((scrutinee, pat, els)), - IfLetOrMatch::Match(scrutinee, arms, ..) => if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none()) - // if there are more than two arms, collapsing would be non-trivial - // one of the arms must be "wild-like" - && let Some(wild_idx) = arms.iter().rposition(|a| arm_is_wild_like(cx, a)) - { - let (then, els) = (&arms[1 - wild_idx], &arms[wild_idx]); - Some((scrutinee, then.pat, Some(els.body))) - } else { - None + IfLetOrMatch::Match(scrutinee, arms, ..) => { + if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none()) + // if there are more than two arms, collapsing would be non-trivial + // one of the arms must be "wild-like" + && let Some(wild_idx) = arms.iter().rposition(|a| arm_is_wild_like(cx, a)) + { + let (then, els) = (&arms[1 - wild_idx], &arms[wild_idx]); + Some((scrutinee, then.pat, Some(els.body))) + } else { + None + } }, } && outer_pat.span.eq_ctxt(inner_scrutinee.span) @@ -68,8 +70,8 @@ fn check_arm<'tcx>( && !pat_contains_disallowed_or(cx, inner_then_pat, msrv) // the binding must come from the pattern of the containing match arm // .... => match { .. } - && let (Some(binding_span), is_innermost_parent_pat_struct) - = find_pat_binding_and_is_innermost_parent_pat_struct(outer_pat, binding_id) + && let (Some(binding_span), is_innermost_parent_pat_struct) = + find_pat_binding_and_is_innermost_parent_pat_struct(outer_pat, binding_id) // the "else" branches must be equal && match (outer_else_body, inner_else_body) { (None, None) => true, @@ -77,9 +79,7 @@ fn check_arm<'tcx>( (Some(a), Some(b)) => SpanlessEq::new(cx).eq_expr(a, b), } // the binding must not be used in the if guard - && outer_guard.is_none_or( - |e| !is_local_used(cx, e, binding_id) - ) + && outer_guard.is_none_or(|e| !is_local_used(cx, e, binding_id)) // ...or anywhere in the inner expression && match inner { IfLetOrMatch::IfLet(_, _, body, els, _) => { diff --git a/tests/ui/collapsible_match.rs b/tests/ui/collapsible_match.rs index 8931a3aa09c6..7a9c8d14d13f 100644 --- a/tests/ui/collapsible_match.rs +++ b/tests/ui/collapsible_match.rs @@ -304,18 +304,6 @@ pub fn test_2(x: Issue9647) { } } -// https://github.com/rust-lang/rust-clippy/issues/14281 -fn lint_emitted_at_right_node(opt: Option>) { - let n = match opt { - #[expect(clippy::collapsible_match)] - Some(n) => match n { - Ok(n) => n, - _ => return, - }, - None => return, - }; -} - pub fn issue_14155() { let mut arr = ["a", "b", "c"]; if let Some(last) = arr.last() { @@ -357,6 +345,18 @@ pub fn issue_14155() { } } +// https://github.com/rust-lang/rust-clippy/issues/14281 +fn lint_emitted_at_right_node(opt: Option>) { + let n = match opt { + #[expect(clippy::collapsible_match)] + Some(n) => match n { + Ok(n) => n, + _ => return, + }, + None => return, + }; +} + fn make() -> T { unimplemented!() } diff --git a/tests/ui/collapsible_match.stderr b/tests/ui/collapsible_match.stderr index 14b1c1b187e4..ebde46cf8a72 100644 --- a/tests/ui/collapsible_match.stderr +++ b/tests/ui/collapsible_match.stderr @@ -251,7 +251,7 @@ LL | if let Some(u) = a { | ^^^^^^^ with this pattern error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:322:9 + --> tests/ui/collapsible_match.rs:310:9 | LL | / match *last { LL | | @@ -263,7 +263,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:321:17 + --> tests/ui/collapsible_match.rs:309:17 | LL | if let Some(last) = arr.last() { | ^^^^ ---------- use: `arr.last().copied()` @@ -274,7 +274,7 @@ LL | "a" | "b" => { | ^^^^^^^^^ with this pattern error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:332:9 + --> tests/ui/collapsible_match.rs:320:9 | LL | / match &last { LL | | @@ -286,7 +286,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:331:17 + --> tests/ui/collapsible_match.rs:319:17 | LL | if let Some(last) = arr.last() { | ^^^^ ---------- use: `arr.last().as_ref()` @@ -297,7 +297,7 @@ LL | &&"a" | &&"b" => { | ^^^^^^^^^^^^^ with this pattern error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:342:9 + --> tests/ui/collapsible_match.rs:330:9 | LL | / match &mut last { LL | | @@ -309,7 +309,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:341:17 + --> tests/ui/collapsible_match.rs:329:17 | LL | if let Some(mut last) = arr.last_mut() { | ^^^^^^^^ -------------- use: `arr.last_mut().as_mut()` From a212bd48c41fdcad962edc45f2f1570e5771fd0b Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 4 Sep 2025 01:17:03 +0200 Subject: [PATCH 2/3] misc: put the `: ` in the right place in the struct field suggestion --- clippy_lints/src/matches/collapsible_match.rs | 2 +- tests/ui/collapsible_match.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 11e79333a694..71d34d705d2b 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -103,7 +103,7 @@ fn check_arm<'tcx>( // collapsing patterns need an explicit field name in struct pattern matching // ex: Struct {x: Some(1)} let replace_msg = if is_innermost_parent_pat_struct { - format!(", prefixed by `{}`:", snippet(cx, binding_span, "their field name")) + format!(", prefixed by `{}: `", snippet(cx, binding_span, "their field name")) } else { String::new() }; diff --git a/tests/ui/collapsible_match.stderr b/tests/ui/collapsible_match.stderr index ebde46cf8a72..4d81f91ecc89 100644 --- a/tests/ui/collapsible_match.stderr +++ b/tests/ui/collapsible_match.stderr @@ -230,7 +230,7 @@ help: the outer pattern can be modified to include the inner pattern LL | if let Issue9647::A { a, .. } = x { | ^ replace this binding LL | if let Some(u) = a { - | ^^^^^^^ with this pattern, prefixed by `a`: + | ^^^^^^^ with this pattern, prefixed by `a: ` error: this `if let` can be collapsed into the outer `if let` --> tests/ui/collapsible_match.rs:299:9 From fba062b0cae9f92d6e61ebbca0fbbbffdf01bc93 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 4 Sep 2025 01:16:03 +0200 Subject: [PATCH 3/3] fix(collapsible_match): exclude binding modes from struct field pattern suggestions --- clippy_lints/src/matches/collapsible_match.rs | 17 +++++----- tests/ui/collapsible_match.rs | 21 +++++++++++++ tests/ui/collapsible_match.stderr | 31 ++++++++++++++----- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 71d34d705d2b..a02e6dd96a1f 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -13,6 +13,7 @@ use rustc_hir::LangItem::OptionNone; use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatExpr, PatExprKind, PatKind}; use rustc_lint::LateContext; use rustc_span::Span; +use rustc_span::symbol::Ident; use super::{COLLAPSIBLE_MATCH, pat_contains_disallowed_or}; @@ -70,7 +71,7 @@ fn check_arm<'tcx>( && !pat_contains_disallowed_or(cx, inner_then_pat, msrv) // the binding must come from the pattern of the containing match arm // .... => match { .. } - && let (Some(binding_span), is_innermost_parent_pat_struct) = + && let (Some((binding_ident, binding_span)), is_innermost_parent_pat_struct) = find_pat_binding_and_is_innermost_parent_pat_struct(outer_pat, binding_id) // the "else" branches must be equal && match (outer_else_body, inner_else_body) { @@ -103,7 +104,7 @@ fn check_arm<'tcx>( // collapsing patterns need an explicit field name in struct pattern matching // ex: Struct {x: Some(1)} let replace_msg = if is_innermost_parent_pat_struct { - format!(", prefixed by `{}: `", snippet(cx, binding_span, "their field name")) + format!(", prefixed by `{binding_ident}: `") } else { String::new() }; @@ -140,16 +141,16 @@ fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { } } -fn find_pat_binding_and_is_innermost_parent_pat_struct(pat: &Pat<'_>, hir_id: HirId) -> (Option, bool) { - let mut span = None; +fn find_pat_binding_and_is_innermost_parent_pat_struct(pat: &Pat<'_>, hir_id: HirId) -> (Option<(Ident, Span)>, bool) { + let mut binding = None; let mut is_innermost_parent_pat_struct = false; - pat.walk_short(|p| match &p.kind { + pat.walk_short(|p| match p.kind { // ignore OR patterns PatKind::Or(_) => false, - PatKind::Binding(_bm, _, _ident, _) => { + PatKind::Binding(_bm, _, ident, _) => { let found = p.hir_id == hir_id; if found { - span = Some(p.span); + binding = Some((ident, p.span)); } !found }, @@ -158,7 +159,7 @@ fn find_pat_binding_and_is_innermost_parent_pat_struct(pat: &Pat<'_>, hir_id: Hi true }, }); - (span, is_innermost_parent_pat_struct) + (binding, is_innermost_parent_pat_struct) } /// Builds a chain of reference-manipulation method calls (e.g., `.as_ref()`, `.as_mut()`, diff --git a/tests/ui/collapsible_match.rs b/tests/ui/collapsible_match.rs index 7a9c8d14d13f..84f958ee8458 100644 --- a/tests/ui/collapsible_match.rs +++ b/tests/ui/collapsible_match.rs @@ -304,6 +304,27 @@ pub fn test_2(x: Issue9647) { } } +mod issue_13287 { + enum Token { + Name, + Other, + } + + struct Error { + location: u32, + token: Option, + } + + fn struct_field_pat_with_binding_mode(err: Option) { + if let Some(Error { ref token, .. }) = err { + if let Some(Token::Name) = token { + //~^ collapsible_match + println!("token used as a ref"); + } + } + } +} + pub fn issue_14155() { let mut arr = ["a", "b", "c"]; if let Some(last) = arr.last() { diff --git a/tests/ui/collapsible_match.stderr b/tests/ui/collapsible_match.stderr index 4d81f91ecc89..d217948d4ca6 100644 --- a/tests/ui/collapsible_match.stderr +++ b/tests/ui/collapsible_match.stderr @@ -250,8 +250,25 @@ LL | if let Issue9647::A { a: Some(a), .. } = x { LL | if let Some(u) = a { | ^^^^^^^ with this pattern +error: this `if let` can be collapsed into the outer `if let` + --> tests/ui/collapsible_match.rs:320:13 + | +LL | / if let Some(Token::Name) = token { +LL | | +LL | | println!("token used as a ref"); +LL | | } + | |_____________^ + | +help: the outer pattern can be modified to include the inner pattern + --> tests/ui/collapsible_match.rs:319:29 + | +LL | if let Some(Error { ref token, .. }) = err { + | ^^^^^^^^^ replace this binding +LL | if let Some(Token::Name) = token { + | ^^^^^^^^^^^^^^^^^ with this pattern, prefixed by `token: ` + error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:310:9 + --> tests/ui/collapsible_match.rs:331:9 | LL | / match *last { LL | | @@ -263,7 +280,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:309:17 + --> tests/ui/collapsible_match.rs:330:17 | LL | if let Some(last) = arr.last() { | ^^^^ ---------- use: `arr.last().copied()` @@ -274,7 +291,7 @@ LL | "a" | "b" => { | ^^^^^^^^^ with this pattern error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:320:9 + --> tests/ui/collapsible_match.rs:341:9 | LL | / match &last { LL | | @@ -286,7 +303,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:319:17 + --> tests/ui/collapsible_match.rs:340:17 | LL | if let Some(last) = arr.last() { | ^^^^ ---------- use: `arr.last().as_ref()` @@ -297,7 +314,7 @@ LL | &&"a" | &&"b" => { | ^^^^^^^^^^^^^ with this pattern error: this `match` can be collapsed into the outer `if let` - --> tests/ui/collapsible_match.rs:330:9 + --> tests/ui/collapsible_match.rs:351:9 | LL | / match &mut last { LL | | @@ -309,7 +326,7 @@ LL | | } | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> tests/ui/collapsible_match.rs:329:17 + --> tests/ui/collapsible_match.rs:350:17 | LL | if let Some(mut last) = arr.last_mut() { | ^^^^^^^^ -------------- use: `arr.last_mut().as_mut()` @@ -319,5 +336,5 @@ LL | if let Some(mut last) = arr.last_mut() { LL | &mut &mut "a" | &mut &mut "b" => { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ with this pattern -error: aborting due to 16 previous errors +error: aborting due to 17 previous errors