Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 7, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d254020
misc
ada4a Sep 3, 2025
a212bd4
misc: put the `: ` in the right place in the struct field suggestion
ada4a Sep 3, 2025
fba062b
fix(collapsible_match): exclude binding modes from struct field patte…
ada4a Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 22 additions & 21 deletions clippy_lints/src/matches/collapsible_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
Expand All @@ -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, _) => {
Expand All @@ -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()
};
Expand Down Expand Up @@ -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
},
Expand All @@ -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()`,
Expand Down
41 changes: 31 additions & 10 deletions tests/ui/collapsible_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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!()
}
Expand Down
33 changes: 25 additions & 8 deletions tests/ui/collapsible_match.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 | |
Expand All @@ -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()`
Expand All @@ -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 | |
Expand All @@ -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()`
Expand All @@ -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 | |
Expand All @@ -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()`
Expand All @@ -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