Skip to content

Commit 0ad983b

Browse files
estebankestebank
committed
Tweak "add match arm" suggestion
Better track trailing commas in match arms. Do not suggest adding trailing comma to match arm with block body. Better heuristic for "is this match in one line".
1 parent 28b83ee commit 0ad983b

File tree

52 files changed

+228
-231
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+228
-231
lines changed

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs

+38-21
Original file line numberDiff line numberDiff line change
@@ -1356,24 +1356,31 @@ fn report_non_exhaustive_match<'p, 'tcx>(
13561356
}
13571357
[only] => {
13581358
let only = &thir[*only];
1359-
let (pre_indentation, is_multiline) = if let Some(snippet) =
1360-
sm.indentation_before(only.span)
1361-
&& let Ok(with_trailing) =
1362-
sm.span_extend_while(only.span, |c| c.is_whitespace() || c == ',')
1363-
&& sm.is_multiline(with_trailing)
1364-
{
1365-
(format!("\n{snippet}"), true)
1366-
} else {
1367-
(" ".to_string(), false)
1368-
};
13691359
let only_body = &thir[only.body];
1370-
let comma = if matches!(only_body.kind, ExprKind::Block { .. })
1371-
&& only.span.eq_ctxt(only_body.span)
1372-
&& is_multiline
1360+
let pre_indentation = if let Some(snippet) = sm.indentation_before(only.span)
1361+
&& let Some(braces_span) = braces_span
1362+
&& sm.is_multiline(braces_span)
13731363
{
1374-
""
1364+
format!("\n{snippet}")
13751365
} else {
1376-
","
1366+
" ".to_string()
1367+
};
1368+
let comma = match only_body.kind {
1369+
ExprKind::Block { .. } if only_body.span.eq_ctxt(sp) => "",
1370+
ExprKind::Scope { value, .. }
1371+
if let expr = &thir[value]
1372+
&& let ExprKind::Block { .. } = expr.kind
1373+
&& expr.span.eq_ctxt(sp) =>
1374+
{
1375+
""
1376+
}
1377+
_ if sm
1378+
.span_to_snippet(only.span)
1379+
.map_or(false, |snippet| snippet.ends_with(",")) =>
1380+
{
1381+
""
1382+
}
1383+
_ => ",",
13771384
};
13781385
suggestion = Some((
13791386
only.span.shrink_to_hi(),
@@ -1385,12 +1392,22 @@ fn report_non_exhaustive_match<'p, 'tcx>(
13851392
let last = &thir[*last];
13861393
if prev.span.eq_ctxt(last.span) {
13871394
let last_body = &thir[last.body];
1388-
let comma = if matches!(last_body.kind, ExprKind::Block { .. })
1389-
&& last.span.eq_ctxt(last_body.span)
1390-
{
1391-
""
1392-
} else {
1393-
","
1395+
let comma = match last_body.kind {
1396+
ExprKind::Block { .. } if last_body.span.eq_ctxt(sp) => "",
1397+
ExprKind::Scope { value, .. }
1398+
if let expr = &thir[value]
1399+
&& let ExprKind::Block { .. } = expr.kind
1400+
&& expr.span.eq_ctxt(sp) =>
1401+
{
1402+
""
1403+
}
1404+
_ if sm
1405+
.span_to_snippet(last.span)
1406+
.map_or(false, |snippet| snippet.ends_with(",")) =>
1407+
{
1408+
""
1409+
}
1410+
_ => ",",
13941411
};
13951412
let spacing = if sm.is_multiline(prev.span.between(last.span)) {
13961413
sm.indentation_before(last.span).map(|indent| format!("\n{indent}"))

‎compiler/rustc_parse/src/parser/expr.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -3112,6 +3112,7 @@ impl<'a> Parser<'a> {
31123112
let (pat, guard) = this.parse_match_arm_pat_and_guard()?;
31133113

31143114
let span_before_body = this.prev_token.span;
3115+
let mut comma = None;
31153116
let arm_body;
31163117
let is_fat_arrow = this.check(exp!(FatArrow));
31173118
let is_almost_fat_arrow =
@@ -3174,6 +3175,7 @@ impl<'a> Parser<'a> {
31743175
arm_body = Some(expr);
31753176
// Eat a comma if it exists, though.
31763177
let _ = this.eat(exp!(Comma));
3178+
comma = Some(this.prev_token.span);
31773179
Ok(Recovered::No)
31783180
} else if let Some((span, guar)) =
31793181
this.parse_arm_body_missing_braces(&expr, arrow_span)
@@ -3221,7 +3223,8 @@ impl<'a> Parser<'a> {
32213223
}
32223224
};
32233225

3224-
let hi_span = arm_body.as_ref().map_or(span_before_body, |body| body.span);
3226+
let hi_span =
3227+
comma.unwrap_or(arm_body.as_ref().map_or(span_before_body, |body| body.span));
32253228
let arm_span = lo.to(hi_span);
32263229

32273230
// We want to recover:

‎tests/ui/consts/const_in_pattern/incomplete-slice.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ LL | E_SL_var => {}
1919
| ++++
2020
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
2121
|
22-
LL ~ E_SL => {},
22+
LL ~ E_SL => {}
2323
LL + &[] | &[_, _, ..] => todo!()
2424
|
2525

‎tests/ui/error-codes/E0004.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ LL | HastaLaVistaBaby,
1414
= note: the matched value is of type `Terminator`
1515
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1616
|
17-
LL ~ Terminator::TalkToMyHand => {},
17+
LL ~ Terminator::TalkToMyHand => {}
1818
LL + Terminator::HastaLaVistaBaby => todo!()
1919
|
2020

‎tests/ui/feature-gates/feature-gate-non_exhaustive_omitted_patterns_lint.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ LL | C,
8484
= note: the matched value is of type `Foo`
8585
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
8686
|
87-
LL ~ Foo::B => {},
87+
LL ~ Foo::B => {}
8888
LL + Foo::C => todo!()
8989
|
9090

‎tests/ui/feature-gates/feature-gate-precise_pointer_size_matching.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ LL | match 0usize {
88
= note: `usize` does not have a fixed maximum value, so half-open ranges are necessary to match exhaustively
99
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1010
|
11-
LL ~ 0..=usize::MAX => {},
11+
LL ~ 0..=usize::MAX => {}
1212
LL + usize::MAX.. => todo!()
1313
|
1414

@@ -22,7 +22,7 @@ LL | match 0isize {
2222
= note: `isize` does not have fixed minimum and maximum values, so half-open ranges are necessary to match exhaustively
2323
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
2424
|
25-
LL ~ isize::MIN..=isize::MAX => {},
25+
LL ~ isize::MIN..=isize::MAX => {}
2626
LL + ..isize::MIN | isize::MAX.. => todo!()
2727
|
2828

‎tests/ui/half-open-range-patterns/half-open-range-pats-inclusive-match-arrow.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ LL | match x {
2121
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
2222
|
2323
LL ~ 74..=> {},
24-
LL ~ i32::MIN..=-1_i32 => todo!(),
24+
LL + i32::MIN..=-1_i32 => todo!()
2525
|
2626

2727
error: aborting due to 2 previous errors

‎tests/ui/match/intended-binding-pattern-is-const.stderr

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ LL | x_var => {}
1919
| ++++
2020
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
2121
|
22-
LL | x => {}, i32::MIN..=3_i32 | 5_i32..=i32::MAX => todo!()
23-
| ++++++++++++++++++++++++++++++++++++++++++++++++
22+
LL ~ x => {}
23+
LL ~ i32::MIN..=3_i32 | 5_i32..=i32::MAX => todo!()
24+
|
2425

2526
error: aborting due to 1 previous error
2627

‎tests/ui/match/postfix-match/pf-match-exhaustiveness.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ note: `Option<i32>` defined here
1313
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1414
|
1515
LL ~ None => {},
16-
LL ~ Some(_) => todo!(),
16+
LL + Some(_) => todo!()
1717
|
1818

1919
error: aborting due to 1 previous error

‎tests/ui/match/validate-range-endpoints.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ LL | match 0i8 {
6161
= note: the matched value is of type `i8`
6262
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
6363
|
64-
LL ~ -10000..=0 => {},
64+
LL ~ -10000..=0 => {}
6565
LL + i8::MIN..=-17_i8 | 1_i8..=i8::MAX => todo!()
6666
|
6767

@@ -74,7 +74,7 @@ LL | match 0i8 {
7474
= note: the matched value is of type `i8`
7575
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
7676
|
77-
LL ~ -10000.. => {},
77+
LL ~ -10000.. => {}
7878
LL + i8::MIN..=-17_i8 => todo!()
7979
|
8080

‎tests/ui/or-patterns/exhaustiveness-non-exhaustive.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ LL | match (0u8, 0u8) {
77
= note: the matched value is of type `(u8, u8)`
88
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
99
|
10-
LL ~ (0 | 1, 2 | 3) => {},
10+
LL ~ (0 | 1, 2 | 3) => {}
1111
LL + (2_u8..=u8::MAX, _) => todo!()
1212
|
1313

@@ -20,7 +20,7 @@ LL | match ((0u8,),) {
2020
= note: the matched value is of type `((u8,),)`
2121
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
2222
|
23-
LL ~ ((0 | 1,) | (2 | 3,),) => {},
23+
LL ~ ((0 | 1,) | (2 | 3,),) => {}
2424
LL + ((4_u8..=u8::MAX)) => todo!()
2525
|
2626

@@ -33,7 +33,7 @@ LL | match (Some(0u8),) {
3333
= note: the matched value is of type `(Option<u8>,)`
3434
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
3535
|
36-
LL ~ (None | Some(0 | 1),) => {},
36+
LL ~ (None | Some(0 | 1),) => {}
3737
LL + (Some(2_u8..=u8::MAX)) => todo!()
3838
|
3939

‎tests/ui/or-patterns/issue-69875-should-have-been-expanded-earlier-non-exhaustive.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ LL | match 0 {
2121
= note: the matched value is of type `i32`
2222
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
2323
|
24-
LL ~ 0 | (1 | 2) => {},
24+
LL ~ 0 | (1 | 2) => {}
2525
LL + i32::MIN..=-1_i32 | 3_i32..=i32::MAX => todo!()
2626
|
2727

‎tests/ui/or-patterns/missing-bindings.stderr

+3-2
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,9 @@ note: `Option<foo>` defined here
264264
= note: the matched value is of type `Option<foo>`
265265
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
266266
|
267-
LL | Some(alpha | beta) => {}, None => todo!()
268-
| +++++++++++++++++
267+
LL ~ Some(alpha | beta) => {}
268+
LL ~ None => todo!()
269+
|
269270

270271
error: aborting due to 29 previous errors
271272

‎tests/ui/pattern/pattern-binding-disambiguation.stderr

+3-2
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ LL | BracedVariant{},
7777
= note: the matched value is of type `E`
7878
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
7979
|
80-
LL | UnitVariant => {}, E::TupleVariant | E::BracedVariant { } => todo!() // OK, `UnitVariant` is a unit variant pattern
81-
| ++++++++++++++++++++++++++++++++++++++++++++++++++++
80+
LL ~ UnitVariant => {}
81+
LL ~ E::TupleVariant | E::BracedVariant { } => todo!() // OK, `UnitVariant` is a unit variant pattern
82+
|
8283

8384
error[E0005]: refutable pattern in local binding
8485
--> $DIR/pattern-binding-disambiguation.rs:51:9

‎tests/ui/pattern/suggest-adding-appropriate-missing-pattern-excluding-comments.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ fn main() {
44
match Some(1) { //~ ERROR non-exhaustive patterns: `None` not covered
55
Some(1) => {}
66
// hello
7-
Some(_) => {},
7+
Some(_) => {}
88
None => todo!()
99
}
1010
}

‎tests/ui/pattern/suggest-adding-appropriate-missing-pattern-excluding-comments.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ note: `Option<i32>` defined here
1212
= note: the matched value is of type `Option<i32>`
1313
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1414
|
15-
LL ~ Some(_) => {},
15+
LL ~ Some(_) => {}
1616
LL + None => todo!()
1717
|
1818

‎tests/ui/pattern/usefulness/doc-hidden-non-exhaustive.stderr

+5-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ LL | pub enum HiddenEnum {
1212
= note: the matched value is of type `HiddenEnum`
1313
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
1414
|
15-
LL ~ HiddenEnum::B => {},
15+
LL ~ HiddenEnum::B => {}
1616
LL + _ => todo!()
1717
|
1818

@@ -33,7 +33,7 @@ LL | B,
3333
= note: the matched value is of type `HiddenEnum`
3434
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
3535
|
36-
LL ~ HiddenEnum::C => {},
36+
LL ~ HiddenEnum::C => {}
3737
LL + HiddenEnum::B => todo!()
3838
|
3939

@@ -54,7 +54,7 @@ LL | B,
5454
= note: the matched value is of type `HiddenEnum`
5555
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
5656
|
57-
LL ~ HiddenEnum::A => {},
57+
LL ~ HiddenEnum::A => {}
5858
LL + HiddenEnum::B | _ => todo!()
5959
|
6060

@@ -72,7 +72,7 @@ note: `Option<HiddenEnum>` defined here
7272
= note: the matched value is of type `Option<HiddenEnum>`
7373
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
7474
|
75-
LL ~ Some(HiddenEnum::A) => {},
75+
LL ~ Some(HiddenEnum::A) => {}
7676
LL + Some(HiddenEnum::B) | Some(_) => todo!()
7777
|
7878

@@ -93,7 +93,7 @@ LL | C,
9393
= note: the matched value is of type `InCrate`
9494
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
9595
|
96-
LL ~ InCrate::B => {},
96+
LL ~ InCrate::B => {}
9797
LL + InCrate::C => todo!()
9898
|
9999

‎tests/ui/pattern/usefulness/empty-match-check-notes.exhaustive_patterns.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ LL | match 0u8 {
7272
= note: match arms with guards don't count towards exhaustivity
7373
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
7474
|
75-
LL ~ _ if false => {},
75+
LL ~ _ if false => {}
7676
LL + 0_u8..=u8::MAX => todo!()
7777
|
7878

‎tests/ui/pattern/usefulness/empty-match-check-notes.normal.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ LL | match 0u8 {
7272
= note: match arms with guards don't count towards exhaustivity
7373
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
7474
|
75-
LL ~ _ if false => {},
75+
LL ~ _ if false => {}
7676
LL + 0_u8..=u8::MAX => todo!()
7777
|
7878

0 commit comments

Comments
 (0)