fix(collapsible_match): exclude binding modes from struct field pattern suggestions (#15608)

Fixes #13287

changelog: [`collapsible_match`]: exclude binding modes from struct
field pattern suggestions

Author: dswij

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};
 
@@ -50,15 +51,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,18 +71,16 @@ 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
         // ..<local>.. => match <local> { .. }
-        && 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_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) {
             (None, None) => true,
             (None, Some(e)) | (Some(e), None) => is_unit_expr(e),
             (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, _) => {
@@ -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<Span>, 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()`,

tests/ui/collapsible_match.rs

@@ -304,16 +304,25 @@ pub fn test_2(x: Issue9647) {
     }
 }
 
-// https://github.com/rust-lang/rust-clippy/issues/14281
-fn lint_emitted_at_right_node(opt: Option<Result<u64, String>>) {
-    let n = match opt {
-        #[expect(clippy::collapsible_match)]
-        Some(n) => match n {
-            Ok(n) => n,
-            _ => return,
-        },
-        None => return,
-    };
+mod issue_13287 {
+    enum Token {
+        Name,
+        Other,
+    }
+
+    struct Error {
+        location: u32,
+        token: Option<Token>,
+    }
+
+    fn struct_field_pat_with_binding_mode(err: Option<Error>) {
+        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() {
@@ -357,6 +366,18 @@ pub fn issue_14155() {
     }
 }
 
+// https://github.com/rust-lang/rust-clippy/issues/14281
+fn lint_emitted_at_right_node(opt: Option<Result<u64, String>>) {
+    let n = match opt {
+        #[expect(clippy::collapsible_match)]
+        Some(n) => match n {
+            Ok(n) => n,
+            _ => return,
+        },
+        None => return,
+    };
+}
+
 fn make<T>() -> T {
     unimplemented!()
 }

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
@@ -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:322: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:321: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:332: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:331: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:342: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:341: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