Add suggestions for expressions in patterns

Author: ShE3py

compiler/rustc_errors/src/lib.rs

@@ -535,6 +535,8 @@ pub enum StashKey {
     /// Query cycle detected, stashing in favor of a better error.
     Cycle,
     UndeterminedMacroResolution,
+    /// Used by `Parser::maybe_recover_trailing_expr`
+    ExprInPat,
 }
 
 fn default_track_diagnostic<R>(diag: DiagInner, f: &mut dyn FnMut(DiagInner) -> R) -> R {
@@ -837,6 +839,23 @@ impl<'a> DiagCtxtHandle<'a> {
         Some(Diag::new_diagnostic(self, diag))
     }
 
+    /// Steals a previously stashed error with the given `Span` and
+    /// [`StashKey`] as the key, and cancels it if found.
+    /// Panics if the found diagnostic's level isn't `Level::Error`.
+    pub fn steal_err(&self, span: Span, key: StashKey, _: ErrorGuaranteed) -> bool {
+        let key = (span.with_parent(None), key);
+        // FIXME(#120456) - is `swap_remove` correct?
+        self.inner
+            .borrow_mut()
+            .stashed_diagnostics
+            .swap_remove(&key)
+            .inspect(|(diag, guar)| {
+                assert_eq!(diag.level, Error);
+                assert!(guar.is_some())
+            })
+            .is_some()
+    }
+
     /// Steals a previously stashed error with the given `Span` and
     /// [`StashKey`] as the key, modifies it, and emits it. Returns `None` if
     /// no matching diagnostic is found. Panics if the found diagnostic's level
@@ -1281,6 +1300,17 @@ impl<'a> DiagCtxtHandle<'a> {
         self.create_err(err).emit()
     }
 
+    /// See [`DiagCtxt::stash_diagnostic`] for details.
+    #[track_caller]
+    pub fn stash_err(
+        &'a self,
+        span: Span,
+        key: StashKey,
+        err: impl Diagnostic<'a>,
+    ) -> ErrorGuaranteed {
+        self.create_err(err).stash(span, key).unwrap()
+    }
+
     /// Ensures that an error is printed. See `Level::DelayedBug`.
     //
     // No `#[rustc_lint_diagnostics]` and no `impl Into<DiagMessage>` because bug messages aren't

compiler/rustc_parse/messages.ftl

@@ -782,6 +782,14 @@ parse_unexpected_expr_in_pat =
 
     .label = arbitrary expressions are not allowed in patterns
 
+parse_unexpected_expr_in_pat_const_sugg = extract the expression into a `const` and refer to it
+
+parse_unexpected_expr_in_pat_create_guard_sugg = check the value in an arm guard
+
+parse_unexpected_expr_in_pat_inline_const_sugg = wrap the expression in a inline const (requires `{"#"}![feature(inline_const_pat)]`)
+
+parse_unexpected_expr_in_pat_update_guard_sugg = check the value in the arm guard
+
 parse_unexpected_if_with_if = unexpected `if` in the condition expression
     .suggestion = remove the `if`
 

compiler/rustc_parse/src/errors.rs

@@ -2424,6 +2424,74 @@ pub(crate) struct UnexpectedExpressionInPattern {
     pub is_bound: bool,
 }
 
+#[derive(Subdiagnostic)]
+pub(crate) enum UnexpectedExpressionInPatternArmSugg {
+    #[multipart_suggestion(
+        parse_unexpected_expr_in_pat_create_guard_sugg,
+        applicability = "maybe-incorrect"
+    )]
+    CreateGuard {
+        /// The span of the `PatKind:Err` to be transformed into a `PatKind::Ident`.
+        #[suggestion_part(code = "{ident}")]
+        ident_span: Span,
+        /// The end of the match arm's pattern.
+        #[suggestion_part(code = " if {ident} == {expr}")]
+        pat_hi: Span,
+        /// The suggested identifier.
+        ident: String,
+        /// `ident_span`'s snippet.
+        expr: String,
+    },
+    #[multipart_suggestion(
+        parse_unexpected_expr_in_pat_update_guard_sugg,
+        applicability = "maybe-incorrect"
+    )]
+    UpdateGuard {
+        /// The span of the `PatKind:Err` to be transformed into a `PatKind::Ident`.
+        #[suggestion_part(code = "{ident}")]
+        ident_span: Span,
+        /// The beginning of the match arm guard's expression.
+        #[suggestion_part(code = "(")]
+        guard_lo: Span,
+        /// The end of the match arm guard's expression.
+        #[suggestion_part(code = ") && {ident} == {expr}")]
+        guard_hi: Span,
+        /// The suggested identifier.
+        ident: String,
+        /// `ident_span`'s snippet.
+        expr: String,
+    },
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(parse_unexpected_expr_in_pat_const_sugg, applicability = "has-placeholders")]
+pub(crate) struct UnexpectedExpressionInPatternConstSugg {
+    /// The beginning of statement's line.
+    #[suggestion_part(code = "{indentation}const {ident}: _ = {expr};\n")]
+    pub stmt_lo: Span,
+    /// The span of the `PatKind:Err` to be transformed into a `PatKind::Ident`.
+    #[suggestion_part(code = "{ident}")]
+    pub ident_span: Span,
+    /// The suggested identifier.
+    pub ident: String,
+    /// `ident_span`'s snippet.
+    pub expr: String,
+    /// The statement's block's indentation.
+    pub indentation: String,
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(
+    parse_unexpected_expr_in_pat_inline_const_sugg,
+    applicability = "maybe-incorrect"
+)]
+pub(crate) struct UnexpectedExpressionInPatternInlineConstSugg {
+    #[suggestion_part(code = "const {{ ")]
+    pub start_span: Span,
+    #[suggestion_part(code = " }}")]
+    pub end_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(parse_unexpected_paren_in_range_pat)]
 pub(crate) struct UnexpectedParenInRangePat {

compiler/rustc_parse/src/parser/item.rs

@@ -2406,6 +2406,7 @@ impl<'a> Parser<'a> {
             }
             (AttrVec::new(), None)
         };
+
         attrs.extend(inner_attrs);
         Ok(body)
     }

compiler/rustc_parse/src/parser/pat.rs

@@ -6,21 +6,23 @@ use crate::errors::{
     InclusiveRangeExtraEquals, InclusiveRangeMatchArrow, InclusiveRangeNoEnd, InvalidMutInPattern,
     PatternOnWrongSideOfAt, RemoveLet, RepeatedMutInPattern, SwitchRefBoxOrder,
     TopLevelOrPatternNotAllowed, TopLevelOrPatternNotAllowedSugg, TrailingVertNotAllowed,
-    UnexpectedExpressionInPattern, UnexpectedLifetimeInPattern, UnexpectedParenInRangePat,
-    UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
-    UnexpectedVertVertInPattern,
+    UnexpectedExpressionInPattern, UnexpectedExpressionInPatternArmSugg,
+    UnexpectedExpressionInPatternConstSugg, UnexpectedExpressionInPatternInlineConstSugg,
+    UnexpectedLifetimeInPattern, UnexpectedParenInRangePat, UnexpectedParenInRangePatSugg,
+    UnexpectedVertVertBeforeFunctionParam, UnexpectedVertVertInPattern,
 };
 use crate::parser::expr::{could_be_unclosed_char_literal, DestructuredFloat, LhsExpr};
 use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole};
 use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor};
 use rustc_ast::ptr::P;
 use rustc_ast::token::{self, BinOpToken, Delimiter, Token};
+use rustc_ast::visit::{walk_arm, walk_pat, walk_pat_field, Visitor};
 use rustc_ast::{
-    self as ast, AttrVec, BindingMode, ByRef, Expr, ExprKind, MacCall, Mutability, Pat, PatField,
-    PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
+    self as ast, Arm, AttrVec, BindingMode, ByRef, Expr, ExprKind, LocalKind, MacCall, Mutability,
+    Pat, PatField, PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax, Stmt, StmtKind,
 };
 use rustc_ast_pretty::pprust;
-use rustc_errors::{Applicability, Diag, PResult};
+use rustc_errors::{Applicability, Diag, PResult, StashKey};
 use rustc_session::errors::ExprParenthesesNeeded;
 use rustc_span::source_map::{respan, Spanned};
 use rustc_span::symbol::{kw, sym, Ident};
@@ -422,10 +424,15 @@ impl<'a> Parser<'a> {
                     || self.token.kind == token::CloseDelim(Delimiter::Parenthesis)
                         && self.look_ahead(1, Token::is_range_separator);
 
+                let span = expr.span;
+
                 return Some((
-                    self.dcx()
-                        .emit_err(UnexpectedExpressionInPattern { span: expr.span, is_bound }),
-                    expr.span,
+                    self.dcx().stash_err(
+                        span,
+                        StashKey::ExprInPat,
+                        UnexpectedExpressionInPattern { span, is_bound },
+                    ),
+                    span,
                 ));
             }
         }
@@ -434,6 +441,161 @@ impl<'a> Parser<'a> {
         None
     }
 
+    pub(super) fn maybe_emit_stashed_expr_in_pat(&mut self, stmt: &Stmt) {
+        if self.dcx().has_errors().is_none() {
+            return;
+        }
+
+        // WIP: once a fn body has been parsed, we walk through all its patterns,
+        // and emit now what errors `maybe_recover_trailing_expr()` stashed,
+        // with suggestions depending on which statement the pattern is.
+
+        struct PatVisitor<'a> {
+            /// `self`
+            parser: &'a Parser<'a>,
+            /// The current statement.
+            stmt: &'a Stmt,
+            /// The current match arm.
+            arm: Option<&'a Arm>,
+            /// The current struct field.
+            field: Option<&'a PatField>,
+        }
+
+        impl<'a> Visitor<'a> for PatVisitor<'a> {
+            fn visit_arm(&mut self, a: &'a Arm) -> Self::Result {
+                self.arm = Some(a);
+                walk_arm(self, a);
+                self.arm = None;
+            }
+
+            fn visit_pat_field(&mut self, fp: &'a PatField) -> Self::Result {
+                self.field = Some(fp);
+                walk_pat_field(self, fp);
+                self.field = None;
+            }
+
+            fn visit_pat(&mut self, p: &'a Pat) -> Self::Result {
+                // Looks for stashed `ExprInPat` errors in `stash_span`, and emit them with suggestions.
+                // `stash_span` is contained in `expr_span`, the latter being larger in borrow patterns;
+                // ```txt
+                // &mut x.y
+                // -----^^^ `stash_span`
+                // |
+                // `expr_span`
+                // ```
+                let emit_now = |that: &Self, stash_span: Span, expr_span: Span| -> Self::Result {
+                    that.parser.dcx().try_steal_modify_and_emit_err(
+                        stash_span,
+                        StashKey::ExprInPat,
+                        |err| {
+                            let sm = that.parser.psess.source_map();
+                            let stmt = that.stmt;
+                            let line_lo = sm.span_extend_to_line(stmt.span).shrink_to_lo();
+                            let indentation = sm.indentation_before(stmt.span).unwrap_or_default();
+                            let expr = that.parser.span_to_snippet(expr_span).unwrap();
+
+                            err.span.replace(stash_span, expr_span);
+
+                            if let StmtKind::Let(local) = &stmt.kind {
+                                // If we have an `ExprInPat`, the user tried to assign a value to another value,
+                                // which doesn't makes much sense.
+                                match &local.kind {
+                                    LocalKind::Decl => {}
+                                    LocalKind::Init(_) => {}
+                                    LocalKind::InitElse(_, _) => {}
+                                }
+                            } else {
+                                // help: use an arm guard `if val == expr`
+                                if let Some(arm) = &self.arm {
+                                    let (ident, ident_span) = match self.field {
+                                        Some(field) => (
+                                            field.ident.to_string(),
+                                            field.ident.span.to(expr_span),
+                                        ),
+                                        None => ("val".to_owned(), expr_span),
+                                    };
+
+                                    match &arm.guard {
+                                        None => {
+                                            err.subdiagnostic(
+                                                UnexpectedExpressionInPatternArmSugg::CreateGuard {
+                                                    ident_span,
+                                                    pat_hi: arm.pat.span.shrink_to_hi(),
+                                                    ident,
+                                                    expr: expr.clone(),
+                                                },
+                                            );
+                                        }
+                                        Some(guard) => {
+                                            err.subdiagnostic(
+                                                UnexpectedExpressionInPatternArmSugg::UpdateGuard {
+                                                    ident_span,
+                                                    guard_lo: guard.span.shrink_to_lo(),
+                                                    guard_hi: guard.span.shrink_to_hi(),
+                                                    ident,
+                                                    expr: expr.clone(),
+                                                },
+                                            );
+                                        }
+                                    }
+                                }
+
+                                // help: extract the expr into a `const VAL: _ = expr`
+                                let ident = match self.field {
+                                    Some(field) => field.ident.as_str().to_uppercase(),
+                                    None => "VAL".to_owned(),
+                                };
+                                err.subdiagnostic(UnexpectedExpressionInPatternConstSugg {
+                                    stmt_lo: line_lo,
+                                    ident_span: expr_span,
+                                    expr,
+                                    ident,
+                                    indentation,
+                                });
+
+                                // help: wrap the expr in a `const { expr }`
+                                // FIXME(inline_const_pat): once stabilized, remove this check and remove the `(requires #[feature(inline_const_pat)]` note from the message
+                                if that.parser.psess.unstable_features.is_nightly_build() {
+                                    err.subdiagnostic(
+                                        UnexpectedExpressionInPatternInlineConstSugg {
+                                            start_span: expr_span.shrink_to_lo(),
+                                            end_span: expr_span.shrink_to_hi(),
+                                        },
+                                    );
+                                }
+                            }
+                        },
+                    );
+                }; // end of `emit_now` closure, we're back in `visit_pat`
+
+                match &p.kind {
+                    // Base expression
+                    PatKind::Err(_) => emit_now(self, p.span, p.span),
+                    // Sub-patterns
+                    PatKind::Box(subpat) | PatKind::Ref(subpat, _)
+                        if matches!(subpat.kind, PatKind::Err(_)) =>
+                    {
+                        emit_now(self, subpat.span, p.span)
+                    }
+                    // Sub-expressions
+                    PatKind::Range(start, end, _) => {
+                        if let Some(start) = start {
+                            emit_now(self, start.span, start.span);
+                        }
+
+                        if let Some(end) = end {
+                            emit_now(self, end.span, end.span);
+                        }
+                    }
+                    // Walk continuation
+                    _ => walk_pat(self, p),
+                }
+            }
+        } // end of `PatVisitor` impl, we're back in `maybe_emit_stashed_expr_in_pat`
+
+        PatVisitor { parser: self, stmt, arm: None, field: None }.visit_stmt(stmt);
+    }
+
     /// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
     /// allowed).
     fn parse_pat_with_range_pat(
@@ -583,7 +745,11 @@ impl<'a> Parser<'a> {
 
                     match self.parse_range_end() {
                         Some(form) => self.parse_pat_range_begin_with(begin, form)?,
-                        None => PatKind::Lit(begin),
+                        None => match &begin.kind {
+                            // Avoid `PatKind::Lit(ExprKind::Err)`
+                            ExprKind::Err(guar) => PatKind::Err(*guar),
+                            _ => PatKind::Lit(begin),
+                        },
                     }
                 }
                 Err(err) => return self.fatal_unexpected_non_pat(err, expected),
@@ -755,7 +921,25 @@ impl<'a> Parser<'a> {
 
         Ok(match self.maybe_recover_trailing_expr(open_paren.to(self.prev_token.span), false) {
             None => pat,
-            Some((guar, _)) => PatKind::Err(guar),
+            Some((guar, _)) => {
+                // We just recovered a bigger expression, so cancel its children
+                // (e.g. `(1 + 2) * 3`, cancel “`1 + 2` is not a pattern”).
+                match pat {
+                    PatKind::Paren(pat) => {
+                        self.dcx().steal_err(pat.span, StashKey::ExprInPat, guar);
+                    }
+
+                    PatKind::Tuple(fields) => {
+                        for pat in fields {
+                            self.dcx().steal_err(pat.span, StashKey::ExprInPat, guar);
+                        }
+                    }
+
+                    _ => unreachable!(),
+                }
+
+                PatKind::Err(guar)
+            }
         })
     }