Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 18 additions & 4 deletions clippy_lints/src/matches/collapsible_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,21 @@ fn check_arm<'tcx>(
let (paren_start, inner_if_span, paren_end) = peel_parens(cx, inner_expr.span);
let inner_if = inner_if_span.split_at(2).0;
let mut sugg = vec![(inner.then.span.shrink_to_lo(), "=> ".to_string())];

if matches!(outer_then_body.kind, ExprKind::Block(..)) {
let outer_then_open_bracket = outer_then_body
.span
.split_at(1)
.0
let block_span = outer_then_body.span;
// The Block Span can start with a label so finding actual opening braces instead of assuming it's
// first Byte
let block_snippet = snippet(cx, block_span, "");
let Some(brace_offset) = block_snippet.find('{') else {
return;
};
Comment on lines +173 to +176

@Gri-ffin Gri-ffin Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I do something like this:

    'label: /* { .... } */ {

I believe the current solution would confuse both since the comment would be included block_snippet.

View changes since the review

let brace_pos = block_span.lo() + BytePos(u32::try_from(brace_offset).unwrap());

let label_prefix = block_snippet[..brace_offset].trim();
let outer_then_open_bracket = block_span
.with_lo(brace_pos)
.with_hi(brace_pos + BytePos(1))
.with_leading_whitespace(cx)
.into_span();
let outer_then_closing_bracket = {
Expand All @@ -180,6 +190,10 @@ fn check_arm<'tcx>(
};
sugg.push((outer_arrow_end.to(outer_then_open_bracket), String::new()));
sugg.push((outer_then_closing_bracket, String::new()));

if !label_prefix.is_empty() {
sugg[0].1 = format!("=> {label_prefix} ");
}
} else {
sugg.push((outer_arrow_end.until(inner_if), " ".to_string()));
}
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/collapsible_match_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,20 @@ fn issue16894() {
//~^ collapsible_match
};
}

// https://github.com/rust-lang/rust-clippy/issues/17427
// `collapsible_match` suggested wrong spans when the outer arm's block has a
// label, deleting the label's leading `'` instead of preserving it after `=>`.
fn issue17427() {
let x: Result<Option<u8>, ()> = Ok(Some(1));
match x {
Ok(Some(_c))
if true => 'label: {
//~^ collapsible_match
let Some(_y) = Some(0) else {
break 'label;
};
},
_ => (),
}
}
18 changes: 18 additions & 0 deletions tests/ui/collapsible_match_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,21 @@ fn issue16894() {
//~^ collapsible_match
};
}

// https://github.com/rust-lang/rust-clippy/issues/17427
// `collapsible_match` suggested wrong spans when the outer arm's block has a
// label, deleting the label's leading `'` instead of preserving it after `=>`.
fn issue17427() {
let x: Result<Option<u8>, ()> = Ok(Some(1));
match x {
Ok(Some(_c)) => 'label: {
if true {
//~^ collapsible_match
let Some(_y) = Some(0) else {
break 'label;
};
}
},
_ => (),
}
}
23 changes: 22 additions & 1 deletion tests/ui/collapsible_match_fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,26 @@ LL - 11 => 0, 12 => if a == b { 1 } else { 0 }, _ => 0,
LL + 11 => 0, 12 if a == b => { 1 }, _ => 0,
|

error: aborting due to 8 previous errors
error: this `if` can be collapsed into the outer `match`
--> tests/ui/collapsible_match_fixable.rs:88:13
|
LL | / if true {
LL | |
LL | | let Some(_y) = Some(0) else {
LL | | break 'label;
LL | | };
LL | | }
| |_____________^
|
help: collapse nested if block
|
LL ~ Ok(Some(_c))
LL ~ if true => 'label: {
LL |
...
LL | };
LL ~ },
|

error: aborting due to 9 previous errors