Skip to content

Commit

Permalink
Add suggestions for expressions in patterns
Browse files Browse the repository at this point in the history
  • Loading branch information
ShE3py committed Apr 29, 2024
1 parent 49f43c6 commit a34ced9
Show file tree
Hide file tree
Showing 22 changed files with 1,406 additions and 306 deletions.
30 changes: 30 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,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 {
Expand Down Expand Up @@ -806,6 +808,23 @@ impl DiagCtxt {
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
Expand Down Expand Up @@ -1250,6 +1269,17 @@ impl DiagCtxt {
self.create_err(err).emit()
}

/// See [`DiagCtxt::stash_diagnostic`] for details.
#[track_caller]
pub fn stash_err<'a>(
&'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
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,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)]`)
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`
Expand Down
68 changes: 68 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2441,6 +2441,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 {
Expand Down
168 changes: 167 additions & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ use super::{
AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Recovered, Trailing,
TrailingToken,
};
use crate::errors::{self, MacroExpandsToAdtField};
use crate::errors::{
self, MacroExpandsToAdtField, UnexpectedExpressionInPatternArmSugg,
UnexpectedExpressionInPatternConstSugg, UnexpectedExpressionInPatternInlineConstSugg,
};
use crate::fluent_generated as fluent;
use crate::maybe_whole;
use ast::token::IdentIsRaw;
Expand All @@ -13,6 +16,7 @@ use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, TokenKind};
use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
use rustc_ast::util::case::Case;
use rustc_ast::visit::{walk_arm, walk_pat, walk_pat_field, walk_stmt, Visitor};
use rustc_ast::{self as ast};
use rustc_ast_pretty::pprust;
use rustc_errors::{codes::*, struct_span_code_err, Applicability, PResult, StashKey};
Expand Down Expand Up @@ -2362,6 +2366,168 @@ impl<'a> Parser<'a> {
}
(AttrVec::new(), None)
};

if let Some(body) = &body
&& self.dcx().err_count() > 0
{
// 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: Option<&'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_stmt(&mut self, s: &'a Stmt) -> Self::Result {
self.stmt = Some(s);

walk_stmt(self, s)
}

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.unwrap();
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(&that.parser.dcx(), UnexpectedExpressionInPatternArmSugg::CreateGuard {
ident_span,
pat_hi: arm.pat.span.shrink_to_hi(),
ident,
expr: expr.clone(),
});
}
Some(guard) => {
err.subdiagnostic(&that.parser.dcx(), 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(
&that.parser.dcx(),
UnexpectedExpressionInPatternConstSugg {
stmt_lo: line_lo,
ident_span: expr_span,
expr,
ident,
indentation,
},
);

// help: wrap the expr in a `const { expr }`
// FIXME(inline_const): once stabilized, remove this check and remove the `(requires #[feature(inline_const])` note from the message
if that.parser.psess.unstable_features.is_nightly_build() {
err.subdiagnostic(
&that.parser.dcx(),
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 `parse_fn_body`

PatVisitor { parser: self, stmt: None, arm: None, field: None }.visit_block(body);
}

attrs.extend(inner_attrs);
Ok(body)
}
Expand Down
39 changes: 33 additions & 6 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_ast::{
PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
};
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};
Expand Down Expand Up @@ -423,10 +423,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,
));
}
}
Expand Down Expand Up @@ -584,7 +589,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),
Expand Down Expand Up @@ -756,7 +765,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)
}
})
}

Expand Down
Loading

0 comments on commit a34ced9

Please sign in to comment.