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".

Author: estebank

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

@@ -1356,24 +1356,31 @@ fn report_non_exhaustive_match<'p, 'tcx>(
         }
         [only] => {
             let only = &thir[*only];
-            let (pre_indentation, is_multiline) = if let Some(snippet) =
-                sm.indentation_before(only.span)
-                && let Ok(with_trailing) =
-                    sm.span_extend_while(only.span, |c| c.is_whitespace() || c == ',')
-                && sm.is_multiline(with_trailing)
-            {
-                (format!("\n{snippet}"), true)
-            } else {
-                (" ".to_string(), false)
-            };
             let only_body = &thir[only.body];
-            let comma = if matches!(only_body.kind, ExprKind::Block { .. })
-                && only.span.eq_ctxt(only_body.span)
-                && is_multiline
+            let pre_indentation = if let Some(snippet) = sm.indentation_before(only.span)
+                && let Some(braces_span) = braces_span
+                && sm.is_multiline(braces_span)
             {
-                ""
+                format!("\n{snippet}")
             } else {
-                ","
+                " ".to_string()
+            };
+            let comma = match only_body.kind {
+                ExprKind::Block { .. } if only_body.span.eq_ctxt(sp) => "",
+                ExprKind::Scope { value, .. }
+                    if let expr = &thir[value]
+                        && let ExprKind::Block { .. } = expr.kind
+                        && expr.span.eq_ctxt(sp) =>
+                {
+                    ""
+                }
+                _ if sm
+                    .span_to_snippet(only.span)
+                    .map_or(false, |snippet| snippet.ends_with(",")) =>
+                {
+                    ""
+                }
+                _ => ",",
             };
             suggestion = Some((
                 only.span.shrink_to_hi(),
@@ -1385,12 +1392,22 @@ fn report_non_exhaustive_match<'p, 'tcx>(
             let last = &thir[*last];
             if prev.span.eq_ctxt(last.span) {
                 let last_body = &thir[last.body];
-                let comma = if matches!(last_body.kind, ExprKind::Block { .. })
-                    && last.span.eq_ctxt(last_body.span)
-                {
-                    ""
-                } else {
-                    ","
+                let comma = match last_body.kind {
+                    ExprKind::Block { .. } if last_body.span.eq_ctxt(sp) => "",
+                    ExprKind::Scope { value, .. }
+                        if let expr = &thir[value]
+                            && let ExprKind::Block { .. } = expr.kind
+                            && expr.span.eq_ctxt(sp) =>
+                    {
+                        ""
+                    }
+                    _ if sm
+                        .span_to_snippet(last.span)
+                        .map_or(false, |snippet| snippet.ends_with(",")) =>
+                    {
+                        ""
+                    }
+                    _ => ",",
                 };
                 let spacing = if sm.is_multiline(prev.span.between(last.span)) {
                     sm.indentation_before(last.span).map(|indent| format!("\n{indent}"))

compiler/rustc_parse/src/parser/expr.rs

@@ -3112,6 +3112,7 @@ impl<'a> Parser<'a> {
             let (pat, guard) = this.parse_match_arm_pat_and_guard()?;
 
             let span_before_body = this.prev_token.span;
+            let mut comma = None;
             let arm_body;
             let is_fat_arrow = this.check(exp!(FatArrow));
             let is_almost_fat_arrow =
@@ -3174,6 +3175,7 @@ impl<'a> Parser<'a> {
                     arm_body = Some(expr);
                     // Eat a comma if it exists, though.
                     let _ = this.eat(exp!(Comma));
+                    comma = Some(this.prev_token.span);
                     Ok(Recovered::No)
                 } else if let Some((span, guar)) =
                     this.parse_arm_body_missing_braces(&expr, arrow_span)
@@ -3221,7 +3223,8 @@ impl<'a> Parser<'a> {
                 }
             };
 
-            let hi_span = arm_body.as_ref().map_or(span_before_body, |body| body.span);
+            let hi_span =
+                comma.unwrap_or(arm_body.as_ref().map_or(span_before_body, |body| body.span));
             let arm_span = lo.to(hi_span);
 
             // We want to recover:

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

@@ -19,7 +19,7 @@ LL |         E_SL_var => {}
    |             ++++
 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
    |
-LL ~         E_SL => {},
+LL ~         E_SL => {}
 LL +         &[] | &[_, _, ..] => todo!()
    |
 

tests/ui/error-codes/E0004.stderr

@@ -14,7 +14,7 @@ LL |     HastaLaVistaBaby,
    = note: the matched value is of type `Terminator`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         Terminator::TalkToMyHand => {},
+LL ~         Terminator::TalkToMyHand => {}
 LL +         Terminator::HastaLaVistaBaby => todo!()
    |
 

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

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

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

@@ -8,7 +8,7 @@ LL |     match 0usize {
    = note: `usize` does not have a fixed maximum value, so half-open ranges are necessary to match exhaustively
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         0..=usize::MAX => {},
+LL ~         0..=usize::MAX => {}
 LL +         usize::MAX.. => todo!()
    |
 
@@ -22,7 +22,7 @@ LL |     match 0isize {
    = note: `isize` does not have fixed minimum and maximum values, so half-open ranges are necessary to match exhaustively
 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
    |
-LL ~         isize::MIN..=isize::MAX => {},
+LL ~         isize::MIN..=isize::MAX => {}
 LL +         ..isize::MIN | isize::MAX.. => todo!()
    |
 

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

@@ -21,7 +21,7 @@ LL |     match x {
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
 LL ~         74..=> {},
-LL ~         i32::MIN..=-1_i32 => todo!(),
+LL +         i32::MIN..=-1_i32 => todo!()
    |
 
 error: aborting due to 2 previous errors

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

@@ -19,8 +19,9 @@ LL |         x_var => {}
    |          ++++
 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
    |
-LL |         x => {}, i32::MIN..=3_i32 | 5_i32..=i32::MAX => todo!()
-   |                ++++++++++++++++++++++++++++++++++++++++++++++++
+LL ~         x => {}
+LL ~         i32::MIN..=3_i32 | 5_i32..=i32::MAX => todo!()
+   |
 
 error: aborting due to 1 previous error
 

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

@@ -13,7 +13,7 @@ note: `Option<i32>` defined here
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
 LL ~         None => {},
-LL ~         Some(_) => todo!(),
+LL +         Some(_) => todo!()
    |
 
 error: aborting due to 1 previous error

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

@@ -61,7 +61,7 @@ LL |     match 0i8 {
    = note: the matched value is of type `i8`
 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
    |
-LL ~         -10000..=0 => {},
+LL ~         -10000..=0 => {}
 LL +         i8::MIN..=-17_i8 | 1_i8..=i8::MAX => todo!()
    |
 
@@ -74,7 +74,7 @@ LL |     match 0i8 {
    = note: the matched value is of type `i8`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         -10000.. => {},
+LL ~         -10000.. => {}
 LL +         i8::MIN..=-17_i8 => todo!()
    |
 

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

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

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

@@ -21,7 +21,7 @@ LL |     match 0 {
    = note: the matched value is of type `i32`
 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
    |
-LL ~         0 | (1 | 2) => {},
+LL ~         0 | (1 | 2) => {}
 LL +         i32::MIN..=-1_i32 | 3_i32..=i32::MAX => todo!()
    |
 

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

@@ -264,8 +264,9 @@ note: `Option<foo>` defined here
    = note: the matched value is of type `Option<foo>`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL |         Some(alpha | beta) => {}, None => todo!()
-   |                                 +++++++++++++++++
+LL ~         Some(alpha | beta) => {}
+LL ~         None => todo!()
+   |
 
 error: aborting due to 29 previous errors
 

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

@@ -77,8 +77,9 @@ LL |     BracedVariant{},
    = note: the matched value is of type `E`
 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
    |
-LL |         UnitVariant => {}, E::TupleVariant | E::BracedVariant {  } => todo!() // OK, `UnitVariant` is a unit variant pattern
-   |                          ++++++++++++++++++++++++++++++++++++++++++++++++++++
+LL ~         UnitVariant => {}
+LL ~         E::TupleVariant | E::BracedVariant {  } => todo!() // OK, `UnitVariant` is a unit variant pattern
+   |
 
 error[E0005]: refutable pattern in local binding
   --> $DIR/pattern-binding-disambiguation.rs:51:9

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

@@ -4,7 +4,7 @@ fn main() {
     match Some(1) { //~ ERROR non-exhaustive patterns: `None` not covered
         Some(1) => {}
         // hello
-        Some(_) => {},
+        Some(_) => {}
         None => todo!()
     }
 }

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

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

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

@@ -12,7 +12,7 @@ LL | pub enum HiddenEnum {
    = note: the matched value is of type `HiddenEnum`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         HiddenEnum::B => {},
+LL ~         HiddenEnum::B => {}
 LL +         _ => todo!()
    |
 
@@ -33,7 +33,7 @@ LL |     B,
    = note: the matched value is of type `HiddenEnum`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         HiddenEnum::C => {},
+LL ~         HiddenEnum::C => {}
 LL +         HiddenEnum::B => todo!()
    |
 
@@ -54,7 +54,7 @@ LL |     B,
    = note: the matched value is of type `HiddenEnum`
 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
    |
-LL ~         HiddenEnum::A => {},
+LL ~         HiddenEnum::A => {}
 LL +         HiddenEnum::B | _ => todo!()
    |
 
@@ -72,7 +72,7 @@ note: `Option<HiddenEnum>` defined here
    = note: the matched value is of type `Option<HiddenEnum>`
 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
    |
-LL ~         Some(HiddenEnum::A) => {},
+LL ~         Some(HiddenEnum::A) => {}
 LL +         Some(HiddenEnum::B) | Some(_) => todo!()
    |
 
@@ -93,7 +93,7 @@ LL |     C,
    = note: the matched value is of type `InCrate`
 help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
-LL ~         InCrate::B => {},
+LL ~         InCrate::B => {}
 LL +         InCrate::C => todo!()
    |
 

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

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

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

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