Make suggestions machine-applicable

Nadrieril · Nadrieril · commit 60e72f4d8d46 · 2023-12-02T05:18:49.000+01:00
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
@@ -1096,15 +1096,53 @@ pub struct Arm {
     pub attrs: AttrVec,
     /// Match arm pattern, e.g. `10` in `match foo { 10 => {}, _ => {} }`
     pub pat: P<Pat>,
-    /// Match arm guard, e.g. `n > 10` in `match foo { n if n > 10 => {}, _ => {} }`
-    pub guard: Option<P<Expr>>,
+    /// Match arm guard, e.g. `if n > 10` in `match foo { n if n > 10 => {}, _ => {} }`
+    pub guard: Option<ArmGuard>,
     /// Match arm body. Omitted if the pattern is a never pattern.
-    pub body: Option<P<Expr>>,
+    pub body: Option<ArmBody>,
     pub span: Span,
     pub id: NodeId,
     pub is_placeholder: bool,
 }
 
+/// A guard of a `match` arm.
+///
+/// E.g., `if x > 10` in
+///
+/// ```
+/// match None {
+///     Some(x) if x > 10 => { println!("match!") },
+///     _ => { println!("no match!") },
+/// }
+/// ```
+#[derive(Clone, Encodable, Decodable, Debug)]
+pub struct ArmGuard {
+    /// The condition of the guard, e.g. `Some(x) = foo()` in `if Some(x) = foo()`.
+    pub cond: P<Expr>,
+    /// The span of the guard, including the leading `if`.
+    pub span: Span,
+}
+
+/// A body of a `match` arm.
+///
+/// E.g., `=> { println!("match!") }` in
+///
+/// ```
+/// match None {
+///     Some(x) if x > 10 => { println!("match!") },
+///     _ => { println!("no match!") },
+/// }
+/// ```
+#[derive(Clone, Encodable, Decodable, Debug)]
+pub struct ArmBody {
+    /// The expression of the body.
+    pub expr: P<Expr>,
+    /// The span of the body, including the leading `=>` but not a trailing `,`.
+    pub span: Span,
+    /// Whether the body expression is followed by a comma.
+    pub trailing_comma: bool,
+}
+
 /// A single field in a struct expression, e.g. `x: value` and `y` in `Foo { x: value, y }`.
 #[derive(Clone, Encodable, Decodable, Debug)]
 pub struct ExprField {
diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -452,8 +452,14 @@ pub fn noop_flat_map_arm<T: MutVisitor>(mut arm: Arm, vis: &mut T) -> SmallVec<[
     visit_attrs(attrs, vis);
     vis.visit_id(id);
     vis.visit_pat(pat);
-    visit_opt(guard, |guard| vis.visit_expr(guard));
-    visit_opt(body, |body| vis.visit_expr(body));
+    visit_opt(guard, |guard| {
+        vis.visit_expr(&mut guard.cond);
+        vis.visit_span(&mut guard.span);
+    });
+    visit_opt(body, |body| {
+        vis.visit_expr(&mut body.expr);
+        vis.visit_span(&mut body.span);
+    });
     vis.visit_span(span);
     smallvec![arm]
 }
diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs
@@ -950,8 +950,12 @@ pub fn walk_param<'a, V: Visitor<'a>>(visitor: &mut V, param: &'a Param) {
 
 pub fn walk_arm<'a, V: Visitor<'a>>(visitor: &mut V, arm: &'a Arm) {
     visitor.visit_pat(&arm.pat);
-    walk_list!(visitor, visit_expr, &arm.guard);
-    walk_list!(visitor, visit_expr, &arm.body);
+    if let Some(guard) = &arm.guard {
+        visitor.visit_expr(&guard.cond);
+    }
+    if let Some(body) = &arm.body {
+        visitor.visit_expr(&body.expr);
+    }
     walk_list!(visitor, visit_attribute, &arm.attrs);
 }
 
diff --git a/compiler/rustc_ast_lowering/messages.ftl b/compiler/rustc_ast_lowering/messages.ftl
@@ -111,7 +111,7 @@ 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 = keep only the pattern
 
 ast_lowering_never_pattern_with_guard =
     a guard on a never pattern will never be run
diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs
@@ -354,16 +354,19 @@ pub struct MatchArmWithNoBody {
 pub struct NeverPatternWithBody {
     #[primary_span]
     #[label]
-    #[suggestion(code = "", applicability = "maybe-incorrect")]
     pub span: Span,
+    #[suggestion(code = "{suggestion}", applicability = "machine-applicable")]
+    pub suggestion_span: Span,
+    pub suggestion: &'static str,
 }
 
 #[derive(Diagnostic)]
 #[diag(ast_lowering_never_pattern_with_guard)]
 pub struct NeverPatternWithGuard {
     #[primary_span]
-    #[suggestion(code = "", applicability = "maybe-incorrect")]
     pub span: Span,
+    #[suggestion(code = "", applicability = "machine-applicable")]
+    pub suggestion: Span,
 }
 
 #[derive(Diagnostic, Clone, Copy)]
diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
@@ -551,8 +551,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
         let pat = self.lower_pat(&arm.pat);
-        let mut guard = arm.guard.as_ref().map(|cond| {
-            if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &cond.kind {
+        let mut guard = arm.guard.as_ref().map(|guard| {
+            if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &guard.cond.kind {
                 hir::Guard::IfLet(self.arena.alloc(hir::Let {
                     hir_id: self.next_id(),
                     span: self.lower_span(*span),
@@ -562,7 +562,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     is_recovered: *is_recovered,
                 }))
             } else {
-                hir::Guard::If(self.lower_expr(cond))
+                hir::Guard::If(self.lower_expr(&guard.cond))
             }
         });
         let hir_id = self.next_id();
@@ -572,17 +572,28 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let body = if let Some(body) = &arm.body
             && !is_never_pattern
         {
-            self.lower_expr(body)
+            self.lower_expr(&body.expr)
         } else {
             // Either `body.is_none()` or `is_never_pattern` here.
             if !is_never_pattern {
                 let suggestion = span.shrink_to_hi();
                 self.tcx.sess.emit_err(MatchArmWithNoBody { span, suggestion });
             } else if let Some(body) = &arm.body {
-                self.tcx.sess.emit_err(NeverPatternWithBody { span: body.span });
+                let suggestion = if body.trailing_comma { "" } else { "," };
+                let mut suggestion_span = body.span;
+                if let Some(g) = &arm.guard {
+                    suggestion_span = g.span.to(suggestion_span);
+                }
+                self.tcx.sess.emit_err(NeverPatternWithBody {
+                    span: body.expr.span,
+                    suggestion_span,
+                    suggestion,
+                });
                 guard = None;
             } else if let Some(g) = &arm.guard {
-                self.tcx.sess.emit_err(NeverPatternWithGuard { span: g.span });
+                self.tcx
+                    .sess
+                    .emit_err(NeverPatternWithGuard { span: g.cond.span, suggestion: g.span });
                 guard = None;
             }
 
diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -628,14 +628,14 @@ impl<'a> State<'a> {
         self.space();
         if let Some(e) = &arm.guard {
             self.word_space("if");
-            self.print_expr(e);
+            self.print_expr(&e.cond);
             self.space();
         }
 
         if let Some(body) = &arm.body {
             self.word_space("=>");
 
-            match &body.kind {
+            match &body.expr.kind {
                 ast::ExprKind::Block(blk, opt_label) => {
                     if let Some(label) = opt_label {
                         self.print_ident(label.ident);
@@ -652,7 +652,7 @@ impl<'a> State<'a> {
                 }
                 _ => {
                     self.end(); // Close the ibox for the pattern.
-                    self.print_expr(body);
+                    self.print_expr(&body.expr);
                     self.word(",");
                 }
             }
diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
@@ -1,7 +1,7 @@
 use crate::deriving::generic::ty::*;
 use crate::deriving::generic::*;
 use crate::deriving::{path_std, pathvec_std};
-use rustc_ast::{ExprKind, ItemKind, MetaItem, PatKind};
+use rustc_ast::{ArmBody, ExprKind, ItemKind, MetaItem, PatKind};
 use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::symbol::{sym, Ident};
 use rustc_span::Span;
@@ -136,7 +136,8 @@ fn cs_partial_cmp(
                     && let Some(last) = arms.last_mut()
                     && let PatKind::Wild = last.pat.kind
                 {
-                    last.body = Some(expr2);
+                    last.body =
+                        Some(ArmBody { span: expr2.span, expr: expr2, trailing_comma: false });
                     expr1
                 } else {
                     let eq_arm = cx.arm(
diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs
@@ -1,4 +1,5 @@
 use crate::base::ExtCtxt;
+use ast::ArmBody;
 use rustc_ast::ptr::P;
 use rustc_ast::{self as ast, AttrVec, BlockCheckMode, Expr, LocalKind, PatKind, UnOp};
 use rustc_ast::{attr, token, util::literal};
@@ -505,7 +506,7 @@ impl<'a> ExtCtxt<'a> {
             attrs: AttrVec::new(),
             pat,
             guard: None,
-            body: Some(expr),
+            body: Some(ArmBody { expr, span, trailing_comma: false }),
             span,
             id: ast::DUMMY_NODE_ID,
             is_placeholder: false,
diff --git a/compiler/rustc_expand/src/placeholders.rs b/compiler/rustc_expand/src/placeholders.rs
@@ -119,7 +119,7 @@ pub fn placeholder(
         }]),
         AstFragmentKind::Arms => AstFragment::Arms(smallvec![ast::Arm {
             attrs: Default::default(),
-            body: Some(expr_placeholder()),
+            body: Some(ast::ArmBody { expr: expr_placeholder(), span, trailing_comma: false }),
             guard: None,
             id,
             pat: pat(),
diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs
@@ -1116,7 +1116,7 @@ impl EarlyLintPass for UnusedParens {
                     if let Some(body) = &a.body {
                         self.check_unused_delims_expr(
                             cx,
-                            body,
+                            &body.expr,
                             UnusedDelimsCtx::MatchArmExpr,
                             false,
                             None,
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -10,7 +10,7 @@ use super::{
 use crate::errors;
 use crate::maybe_recover_from_interpolated_ty_qpath;
 use ast::mut_visit::{noop_visit_expr, MutVisitor};
-use ast::{GenBlockKind, Pat, Path, PathSegment};
+use ast::{ArmBody, GenBlockKind, Pat, Path, PathSegment};
 use core::mem;
 use rustc_ast::ptr::P;
 use rustc_ast::token::{self, Delimiter, Token, TokenKind};
@@ -22,7 +22,7 @@ use rustc_ast::visit::Visitor;
 use rustc_ast::{self as ast, AttrStyle, AttrVec, CaptureBy, ExprField, UnOp, DUMMY_NODE_ID};
 use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty, TyKind};
 use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
-use rustc_ast::{ClosureBinder, MetaItemLit, StmtKind};
+use rustc_ast::{ArmGuard, ClosureBinder, MetaItemLit, StmtKind};
 use rustc_ast_pretty::pprust;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{
@@ -2938,7 +2938,7 @@ impl<'a> Parser<'a> {
                 let arrow_span = this.prev_token.span;
                 let arm_start_span = this.token.span;
 
-                let expr =
+                let mut expr =
                     this.parse_expr_res(Restrictions::STMT_EXPR, None).map_err(|mut err| {
                         err.span_label(arrow_span, "while parsing the `match` arm starting here");
                         err
@@ -2947,16 +2947,18 @@ impl<'a> Parser<'a> {
                 let require_comma = classify::expr_requires_semi_to_be_stmt(&expr)
                     && this.token != token::CloseDelim(Delimiter::Brace);
 
-                if !require_comma {
-                    arm_body = Some(expr);
-                    this.eat(&token::Comma);
+                let mut trailing_comma = false;
+                let mut hi = this.prev_token.span;
+                let result = if !require_comma {
+                    trailing_comma = this.eat(&token::Comma);
                     Ok(false)
                 } else if let Some(body) = this.parse_arm_body_missing_braces(&expr, arrow_span) {
-                    arm_body = Some(body);
+                    hi = this.prev_token.span;
+                    expr = body;
                     Ok(true)
                 } else {
                     let expr_span = expr.span;
-                    arm_body = Some(expr);
+                    trailing_comma = this.check(&token::Comma);
                     this.expect_one_of(&[token::Comma], &[token::CloseDelim(Delimiter::Brace)])
                         .map_err(|mut err| {
                             if this.token == token::FatArrow {
@@ -2993,7 +2995,9 @@ impl<'a> Parser<'a> {
                             }
                             err
                         })
-                }
+                };
+                arm_body = Some(ArmBody { expr, span: arrow_span.to(hi), trailing_comma });
+                result
             };
 
             let hi_span = arm_body.as_ref().map_or(span_before_body, |body| body.span);
@@ -3056,7 +3060,7 @@ impl<'a> Parser<'a> {
         })
     }
 
-    fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<P<Expr>>> {
+    fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<ArmGuard>> {
         // Used to check the `let_chains` and `if_let_guard` features mostly by scanning
         // `&&` tokens.
         fn check_let_expr(expr: &Expr) -> (bool, bool) {
@@ -3081,18 +3085,18 @@ impl<'a> Parser<'a> {
         CondChecker::new(self).visit_expr(&mut cond);
 
         let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
+        let span = if_span.to(cond.span);
         if has_let_expr {
             if does_not_have_bin_op {
                 // Remove the last feature gating of a `let` expression since it's stable.
                 self.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
             }
-            let span = if_span.to(cond.span);
             self.sess.gated_spans.gate(sym::if_let_guard, span);
         }
-        Ok(Some(cond))
+        Ok(Some(ArmGuard { cond, span }))
     }
 
-    fn parse_match_arm_pat_and_guard(&mut self) -> PResult<'a, (P<Pat>, Option<P<Expr>>)> {
+    fn parse_match_arm_pat_and_guard(&mut self) -> PResult<'a, (P<Pat>, Option<ArmGuard>)> {
         if self.token.kind == token::OpenDelim(Delimiter::Parenthesis) {
             // Detect and recover from `($pat if $cond) => $arm`.
             let left = self.token.span;
@@ -3107,6 +3111,7 @@ impl<'a> Parser<'a> {
                     if let prev_sp = self.prev_token.span
                         && let true = self.eat_keyword(kw::If) =>
                 {
+                    let if_span = self.prev_token.span;
                     // We know for certain we've found `($pat if` so far.
                     let mut cond = match self.parse_match_guard_condition() {
                         Ok(cond) => cond,
@@ -3124,7 +3129,12 @@ impl<'a> Parser<'a> {
                         span: vec![left, right],
                         sugg: errors::ParenthesesInMatchPatSugg { left, right },
                     });
-                    Ok((self.mk_pat(left.to(prev_sp), ast::PatKind::Wild), Some(cond)))
+                    let pat_span = left.to(prev_sp);
+                    let guard_span = if_span.to(cond.span);
+                    Ok((
+                        self.mk_pat(pat_span, ast::PatKind::Wild),
+                        Some(ArmGuard { cond, span: guard_span }),
+                    ))
                 }
                 Err(err) => Err(err),
             }
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -3293,8 +3293,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
     fn resolve_arm(&mut self, arm: &'ast Arm) {
         self.with_rib(ValueNS, RibKind::Normal, |this| {
             this.resolve_pattern_top(&arm.pat, PatternSource::Match);
-            walk_list!(this, visit_expr, &arm.guard);
-            walk_list!(this, visit_expr, &arm.body);
+            if let Some(guard) = &arm.guard {
+                this.visit_expr(&guard.cond);
+            }
+            if let Some(body) = &arm.body {
+                this.visit_expr(&body.expr);
+            }
         });
     }
 
diff --git a/tests/ui/never_patterns/check.fixed b/tests/ui/never_patterns/check.fixed
diff --git a/tests/ui/never_patterns/check.rs b/tests/ui/never_patterns/check.rs
diff --git a/tests/ui/never_patterns/check.stderr b/tests/ui/never_patterns/check.stderr
diff --git a/tests/ui/never_patterns/parse.stderr b/tests/ui/never_patterns/parse.stderr
diff --git a/tests/ui/pattern/never_patterns.rs b/tests/ui/pattern/never_patterns.rs

Original file line number	Diff line number	Diff line change
`@@ -628,14 +628,14 @@ impl<'a> State<'a> {`
`628`	`628`	`self.space();`
`629`	`629`	`if let Some(e) = &arm.guard {`
`630`	`630`	`self.word_space("if");`
`631`		`- self.print_expr(e);`
	`631`	`+ self.print_expr(&e.cond);`
`632`	`632`	`self.space();`
`633`	`633`	`}`
`634`	`634`
`635`	`635`	`if let Some(body) = &arm.body {`
`636`	`636`	`self.word_space("=>");`
`637`	`637`
`638`		`- match &body.kind {`
	`638`	`+ match &body.expr.kind {`
`639`	`639`	`ast::ExprKind::Block(blk, opt_label) => {`
`640`	`640`	`if let Some(label) = opt_label {`
`641`	`641`	`self.print_ident(label.ident);`
`@@ -652,7 +652,7 @@ impl<'a> State<'a> {`
`652`	`652`	`}`
`653`	`653`	`_ => {`
`654`	`654`	`self.end(); // Close the ibox for the pattern.`
`655`		`- self.print_expr(body);`
	`655`	`+ self.print_expr(&body.expr);`
`656`	`656`	`self.word(",");`
`657`	`657`	`}`
`658`	`658`	`}`