Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose

compiler/rustc_ast_lowering/messages.ftl

@@ -133,11 +133,11 @@ ast_lowering_misplaced_relax_trait_bound =
 ast_lowering_never_pattern_with_body =
     a never pattern is always unreachable
     .label = this will never be executed
-    .suggestion = remove this expression
+    .suggestion = remove the match arm expression
 
 ast_lowering_never_pattern_with_guard =
     a guard on a never pattern will never be run
-    .suggestion = remove this guard
+    .suggestion = remove the match arm guard
 
 ast_lowering_no_precise_captures_on_apit = `use<...>` precise capturing syntax not allowed in argument-position `impl Trait`
 

compiler/rustc_ast_lowering/src/errors.rs

@@ -336,7 +336,7 @@ pub(crate) struct MisplacedRelaxTraitBound {
 pub(crate) struct MatchArmWithNoBody {
     #[primary_span]
     pub span: Span,
-    #[suggestion(code = " => todo!(),", applicability = "has-placeholders")]
+    #[suggestion(code = " => todo!(),", applicability = "has-placeholders", style = "verbose")]
     pub suggestion: Span,
 }
 
@@ -345,16 +345,18 @@ pub(crate) struct MatchArmWithNoBody {
 pub(crate) struct NeverPatternWithBody {
     #[primary_span]
     #[label]
-    #[suggestion(code = "", applicability = "maybe-incorrect")]
     pub span: Span,
+    #[suggestion(code = ",", applicability = "maybe-incorrect", style = "verbose")]
+    pub removal_span: Span,
 }
 
 #[derive(Diagnostic)]
 #[diag(ast_lowering_never_pattern_with_guard)]
 pub(crate) struct NeverPatternWithGuard {
     #[primary_span]
-    #[suggestion(code = "", applicability = "maybe-incorrect")]
     pub span: Span,
+    #[suggestion(code = ",", applicability = "maybe-incorrect", style = "verbose")]
+    pub removal_span: Span,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_ast_lowering/src/expr.rs

@@ -679,6 +679,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
         {
             body
         } else {
+            let removal_span = |removal_span: Span| {
+                // Seek upwards in the macro call sites to see if we find the place where
+                // `pat!()` was called so that we can get the right span to remove.
+                let Some(pat_span) = pat.span.find_ancestor_in_same_ctxt(arm.span) else {
+                    return removal_span;
+                };
+                // - pat!() => {}
+                // + pat!(),
+                pat_span.shrink_to_hi().with_hi(arm.span.hi())
+            };
             // Either `body.is_none()` or `is_never_pattern` here.
             if !is_never_pattern {
                 if self.tcx.features().never_patterns() {
@@ -687,9 +697,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     self.dcx().emit_err(MatchArmWithNoBody { span, suggestion });
                 }
             } else if let Some(body) = &arm.body {
-                self.dcx().emit_err(NeverPatternWithBody { span: body.span });
+                self.dcx().emit_err(NeverPatternWithBody {
+                    span: body.span,
+                    removal_span: removal_span(body.span),
+                });
             } else if let Some(g) = &arm.guard {
-                self.dcx().emit_err(NeverPatternWithGuard { span: g.span });
+                self.dcx().emit_err(NeverPatternWithGuard {
+                    span: g.span,
+                    removal_span: removal_span(g.span),
+                });
             }
 
             // We add a fake `loop {}` arm body so that it typecks to `!`. The mir lowering of never

compiler/rustc_ast_lowering/src/pat.rs

@@ -20,7 +20,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         self.arena.alloc(self.lower_pat_mut(pattern))
     }
 
-    fn lower_pat_mut(&mut self, mut pattern: &Pat) -> hir::Pat<'hir> {
+    fn lower_pat_mut(&mut self, outer_pattern: &Pat) -> hir::Pat<'hir> {
+        let mut pattern = outer_pattern;
         ensure_sufficient_stack(|| {
             // loop here to avoid recursion
             let pat_hir_id = self.lower_node_id(pattern.id);
@@ -147,7 +148,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 }
             };
 
-            self.pat_with_node_id_of(pattern, node, pat_hir_id)
+            self.pat_with_node_id_of(outer_pattern, node, pat_hir_id)
         })
     }
 

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

@@ -24,7 +24,7 @@ use rustc_session::lint::builtin::{
 };
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::hygiene::DesugaringKind;
-use rustc_span::{Ident, Span};
+use rustc_span::{ExpnKind, Ident, Span};
 use rustc_trait_selection::infer::InferCtxtExt;
 use tracing::instrument;
 
@@ -543,6 +543,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
                     witnesses,
                     arms,
                     braces_span,
+                    expr_span,
                 ));
             }
         }
@@ -1210,6 +1211,7 @@ fn report_non_exhaustive_match<'p, 'tcx>(
     witnesses: Vec<WitnessPat<'p, 'tcx>>,
     arms: &[ArmId],
     braces_span: Option<Span>,
+    expr_span: Span,
 ) -> ErrorGuaranteed {
     let is_empty_match = arms.is_empty();
     let non_empty_enum = match scrut_ty.kind() {
@@ -1350,43 +1352,59 @@ fn report_non_exhaustive_match<'p, 'tcx>(
                 format!(" {{{indentation}{more}{suggested_arm},{indentation}}}",),
             ));
         }
-        [only] => {
+        [only] if let Some(braces_span) = braces_span => {
             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)
+                && 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(),
                 format!("{comma}{pre_indentation}{suggested_arm}"),
             ));
         }
-        [.., prev, last] => {
+        [.., prev, last] if braces_span.is_some() => {
             let prev = &thir[*prev];
             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}"))
@@ -1401,7 +1419,25 @@ fn report_non_exhaustive_match<'p, 'tcx>(
                 }
             }
         }
-        _ => {}
+        _ => {
+            if let Some(data) = expr_span.macro_backtrace().next()
+                && let ExpnKind::Macro(macro_kind, name) = data.kind
+            {
+                let macro_kind = macro_kind.descr();
+                // We don't want to point at the macro invocation place as that is already shown
+                // or talk about macro-backtrace and the macro's name, as we are already doing
+                // that as part of this note.
+                let mut span: MultiSpan = expr_span.with_ctxt(data.call_site.ctxt()).into();
+                span.push_span_label(data.def_site, "");
+                err.span_note(
+                    span,
+                    format!(
+                        "within {macro_kind} `{name}`, this `match` expression doesn't expand to \
+                         cover all patterns",
+                    ),
+                );
+            }
+        }
     }
 
     let msg = format!(

compiler/rustc_parse/src/parser/expr.rs

@@ -3074,6 +3074,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 =
@@ -3143,6 +3144,7 @@ impl<'a> Parser<'a> {
                 {
                     let body = this.mk_expr_err(span, guar);
                     arm_body = Some(body);
+                    let _ = this.eat(exp!(Comma));
                     Ok(Recovered::Yes(guar))
                 } else {
                     let expr_span = expr.span;
@@ -3183,8 +3185,12 @@ impl<'a> Parser<'a> {
                     })
                 }
             };
+            if let TokenKind::Comma = this.prev_token.kind {
+                comma = Some(this.prev_token.span);
+            }
 
-            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:

src/tools/clippy/clippy_lints/src/loops/single_element_loop.rs

@@ -71,7 +71,7 @@ pub(super) fn check<'tcx>(
     {
         let mut applicability = Applicability::MachineApplicable;
         let mut pat_snip = snippet_with_applicability(cx, pat.span, "..", &mut applicability);
-        if matches!(pat.kind, PatKind::Or(..)) {
+        if matches!(pat.kind, PatKind::Or(..)) && !pat_snip.starts_with("(") {
             pat_snip = format!("({pat_snip})").into();
         }
         let mut arg_snip = snippet_with_applicability(cx, arg_expression.span, "..", &mut applicability);

src/tools/clippy/tests/ui/match_same_arms.stderr

@@ -9,15 +9,15 @@ note: `_` wildcard arm here
   --> tests/ui/match_same_arms.rs:14:9
    |
 LL |         _ => 0,
-   |         ^^^^^^
+   |         ^^^^^^^
    = note: `-D clippy::match-same-arms` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::match_same_arms)]`
 
 error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:18:9
    |
 LL |         (1, .., 3) => 42,
-   |         ^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -30,7 +30,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:25:9
    |
 LL |         51 => 1,
-   |         ^^^^^^^
+   |         ^^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -44,7 +44,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:26:9
    |
 LL |         41 => 2,
-   |         ^^^^^^^
+   |         ^^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -57,7 +57,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:33:9
    |
 LL |         2 => 2,
-   |         ^^^^^^
+   |         ^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -71,7 +71,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:35:9
    |
 LL |         3 => 2,
-   |         ^^^^^^
+   |         ^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -85,7 +85,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:33:9
    |
 LL |         2 => 2,
-   |         ^^^^^^
+   |         ^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm
@@ -99,7 +99,7 @@ error: this match arm has an identical body to another arm
   --> tests/ui/match_same_arms.rs:52:17
    |
 LL |                 CommandInfo::External { name, .. } => name.to_string(),
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: try changing either arm body
 help: or try merging the arm patterns and removing the obsolete arm