Skip to content

Commit

Permalink
Rollup merge of rust-lang#118625 - ShE3py:expr-in-pats, r=WaffleLapkin
Browse files Browse the repository at this point in the history
Improve handling of expressions in patterns

Closes rust-lang#112593.

Methodcalls' dots in patterns are silently recovered as commas (e.g. `Foo("".len())` -> `Foo("", len())`) so extra diagnostics are emitted:
```rs
struct Foo(u8, String, u8);

fn bar(foo: Foo) -> bool {
    match foo {
        Foo(4, "yippee".yeet(), 7) => true,
        _ => false
    }
}
```
```
error: expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found `.`
 --> main.rs:5:24
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                        ^
  |                        |
  |                        expected one of `)`, `,`, `...`, `..=`, `..`, or `|`
  |                        help: missing `,`

error[E0531]: cannot find tuple struct or tuple variant `yeet` in this scope
 --> main.rs:5:25
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                         ^^^^ not found in this scope

error[E0023]: this pattern has 4 fields, but the corresponding tuple struct has 3 fields
 --> main.rs:5:13
  |
1 | struct Foo(u8, String, u8);
  |            --  ------  -- tuple struct has 3 fields
...
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |             ^  ^^^^^^^^ ^^^^^^  ^ expected 3 fields, found 4

error: aborting due to 3 previous errors
```

This PR checks for patterns that ends with a dot and a lowercase ident (as structs/variants should be uppercase):
```
error: expected a pattern, found a method call
 --> main.rs:5:16
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                ^^^^^^^^^^^^^^^ method calls are not allowed in patterns

error: aborting due to 1 previous error
```

Also check for expressions:
```rs
fn is_idempotent(x: f32) -> bool {
    match x {
        x * x => true,
        _ => false,
    }
}

fn main() {
    let mut t: [i32; 5];
    let t[0] = 1;
}
```
```
error: expected a pattern, found an expression
 --> main.rs:3:9
  |
3 |         x * x => true,
  |         ^^^^^ arbitrary expressions are not allowed in patterns

error: expected a pattern, found an expression
  --> main.rs:10:9
   |
10 |     let t[0] = 1;
   |         ^^^^ arbitrary expressions are not allowed in patterns
```

Would be cool if the compiler could suggest adding a guard for `match`es, but I've no idea how to do it.

---
`@rustbot` label +A-diagnostics +A-parser +A-patterns +C-enhancement
  • Loading branch information
matthiaskrgr authored Jan 28, 2024
2 parents 173b666 + a13fbaa commit 390f768
Show file tree
Hide file tree
Showing 29 changed files with 800 additions and 66 deletions.
14 changes: 14 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,20 @@ parse_unexpected_const_param_declaration = unexpected `const` parameter declarat
parse_unexpected_default_value_for_lifetime_in_generic_parameters = unexpected default lifetime parameter
.label = lifetime parameters cannot have default values
parse_unexpected_expr_in_pat =
expected {$is_bound ->
[true] a pattern range bound
*[false] a pattern
}, found {$is_method_call ->
[true] a method call
*[false] an expression
}
.label = {$is_method_call ->
[true] method calls
*[false] arbitrary expressions
} are not allowed in patterns
parse_unexpected_if_with_if = unexpected `if` in the condition expression
.suggestion = remove the `if`
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2415,6 +2415,18 @@ pub(crate) struct ExpectedCommaAfterPatternField {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_expr_in_pat)]
pub(crate) struct UnexpectedExpressionInPattern {
#[primary_span]
#[label]
pub span: Span,
/// Was a `RangePatternBound` expected?
pub is_bound: bool,
/// Was the unexpected expression a `MethodCallExpression`?
pub is_method_call: bool,
}

#[derive(Diagnostic)]
#[diag(parse_unexpected_paren_in_range_pat)]
pub(crate) struct UnexpectedParenInRangePat {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,19 @@ impl<'a> Parser<'a> {
) if self.restrictions.contains(Restrictions::CONST_EXPR) => {
return None;
}
// When recovering patterns as expressions, stop parsing when encountering an assignment `=`, an alternative `|`, or a range `..`.
(
Some(
AssocOp::Assign
| AssocOp::AssignOp(_)
| AssocOp::BitOr
| AssocOp::DotDot
| AssocOp::DotDotEq,
),
_,
) if self.restrictions.contains(Restrictions::IS_PAT) => {
return None;
}
(Some(op), _) => (op, self.token.span),
(None, Some((Ident { name: sym::and, span }, false))) if self.may_recover() => {
self.dcx().emit_err(errors::InvalidLogicalOperator {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ bitflags::bitflags! {
const CONST_EXPR = 1 << 2;
const ALLOW_LET = 1 << 3;
const IN_IF_GUARD = 1 << 4;
const IS_PAT = 1 << 5;
}
}

Expand Down
155 changes: 142 additions & 13 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
use super::{ForceCollect, Parser, PathStyle, TrailingToken};
use super::{ForceCollect, Parser, PathStyle, Restrictions, TrailingToken};
use crate::errors::{
self, AmbiguousRangePattern, DotDotDotForRemainingFields, DotDotDotRangeToPatternNotAllowed,
DotDotDotRestPattern, EnumPatternInsteadOfIdentifier, ExpectedBindingLeftOfAt,
ExpectedCommaAfterPatternField, GenericArgsInPatRequireTurbofishSyntax,
InclusiveRangeExtraEquals, InclusiveRangeMatchArrow, InclusiveRangeNoEnd, InvalidMutInPattern,
PatternOnWrongSideOfAt, RefMutOrderIncorrect, RemoveLet, RepeatedMutInPattern,
SwitchRefBoxOrder, TopLevelOrPatternNotAllowed, TopLevelOrPatternNotAllowedSugg,
TrailingVertNotAllowed, UnexpectedLifetimeInPattern, UnexpectedParenInRangePat,
UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam,
UnexpectedVertVertInPattern,
TrailingVertNotAllowed, UnexpectedExpressionInPattern, UnexpectedLifetimeInPattern,
UnexpectedParenInRangePat, UnexpectedParenInRangePatSugg,
UnexpectedVertVertBeforeFunctionParam, UnexpectedVertVertInPattern,
};
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, Delimiter};
use rustc_ast::token::{self, BinOpToken, Delimiter, Token};
use rustc_ast::{
self as ast, AttrVec, BindingAnnotation, ByRef, Expr, ExprKind, MacCall, Mutability, Pat,
PatField, PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax,
Expand All @@ -23,7 +23,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder, PResult};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_span::source_map::{respan, Spanned};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};
use thin_vec::{thin_vec, ThinVec};

#[derive(PartialEq, Copy, Clone)]
Expand Down Expand Up @@ -336,6 +336,95 @@ impl<'a> Parser<'a> {
}
}

/// Ensures that the last parsed pattern (or pattern range bound) is not followed by a method call or an operator.
///
/// `is_end_bound` indicates whether the last parsed thing was the end bound of a range pattern (see [`parse_pat_range_end`](Self::parse_pat_range_end))
/// in order to say "expected a pattern range bound" instead of "expected a pattern";
/// ```text
/// 0..=1 + 2
/// ^^^^^
/// ```
/// Only the end bound is spanned, and this function have no idea if there were a `..=` before `pat_span`, hence the parameter.
#[must_use = "the pattern must be discarded as `PatKind::Err` if this function returns Some"]
fn maybe_recover_trailing_expr(
&mut self,
pat_span: Span,
is_end_bound: bool,
) -> Option<ErrorGuaranteed> {
if self.prev_token.is_keyword(kw::Underscore) || !self.may_recover() {
// Don't recover anything after an `_` or if recovery is disabled.
return None;
}

// Check for `.hello()`, but allow `.Hello()` to be recovered as `, Hello()` in `parse_seq_to_before_tokens()`.
let has_trailing_method = self.check_noexpect(&token::Dot)
&& self.look_ahead(1, |tok| {
tok.ident()
.and_then(|(ident, _)| ident.name.as_str().chars().next())
.is_some_and(char::is_lowercase)
})
&& self.look_ahead(2, |tok| tok.kind == token::OpenDelim(Delimiter::Parenthesis));

// Check for operators.
// `|` is excluded as it is used in pattern alternatives and lambdas,
// `?` is included for error propagation,
// `[` is included for indexing operations,
// `[]` is excluded as `a[]` isn't an expression and should be recovered as `a, []` (cf. `tests/ui/parser/pat-lt-bracket-7.rs`)
let has_trailing_operator = matches!(self.token.kind, token::BinOp(op) if op != BinOpToken::Or)
|| self.token.kind == token::Question
|| (self.token.kind == token::OpenDelim(Delimiter::Bracket)
&& self.look_ahead(1, |tok| tok.kind != token::CloseDelim(Delimiter::Bracket)));

if !has_trailing_method && !has_trailing_operator {
// Nothing to recover here.
return None;
}

// Let's try to parse an expression to emit a better diagnostic.
let mut snapshot = self.create_snapshot_for_diagnostic();
snapshot.restrictions.insert(Restrictions::IS_PAT);

// Parse `?`, `.f`, `(arg0, arg1, ...)` or `[expr]` until they've all been eaten.
if let Ok(expr) = snapshot
.parse_expr_dot_or_call_with(
self.mk_expr_err(pat_span), // equivalent to transforming the parsed pattern into an `Expr`
pat_span,
AttrVec::new(),
)
.map_err(|err| err.cancel())
{
let non_assoc_span = expr.span;

// Parse an associative expression such as `+ expr`, `% expr`, ...
// Assignements, ranges and `|` are disabled by [`Restrictions::IS_PAT`].
if let Ok(expr) =
snapshot.parse_expr_assoc_with(0, expr.into()).map_err(|err| err.cancel())
{
// We got a valid expression.
self.restore_snapshot(snapshot);
self.restrictions.remove(Restrictions::IS_PAT);

let is_bound = is_end_bound
// is_start_bound: either `..` or `)..`
|| self.token.is_range_separator()
|| self.token.kind == token::CloseDelim(Delimiter::Parenthesis)
&& self.look_ahead(1, Token::is_range_separator);

// Check that `parse_expr_assoc_with` didn't eat a rhs.
let is_method_call = has_trailing_method && non_assoc_span == expr.span;

return Some(self.dcx().emit_err(UnexpectedExpressionInPattern {
span: expr.span,
is_bound,
is_method_call,
}));
}
}

// We got a trailing method/operator, but we couldn't parse an expression.
None
}

/// Parses a pattern, with a setting whether modern range patterns (e.g., `a..=b`, `a..b` are
/// allowed).
fn parse_pat_with_range_pat(
Expand Down Expand Up @@ -441,7 +530,10 @@ impl<'a> Parser<'a> {
} else if self.check(&token::OpenDelim(Delimiter::Parenthesis)) {
self.parse_pat_tuple_struct(qself, path)?
} else {
PatKind::Path(qself, path)
match self.maybe_recover_trailing_expr(span, false) {
Some(guar) => PatKind::Err(guar),
None => PatKind::Path(qself, path),
}
}
} else if matches!(self.token.kind, token::Lifetime(_))
// In pattern position, we're totally fine with using "next token isn't colon"
Expand Down Expand Up @@ -470,10 +562,17 @@ impl<'a> Parser<'a> {
} else {
// Try to parse everything else as literal with optional minus
match self.parse_literal_maybe_minus() {
Ok(begin) => match self.parse_range_end() {
Some(form) => self.parse_pat_range_begin_with(begin, form)?,
None => PatKind::Lit(begin),
},
Ok(begin) => {
let begin = match self.maybe_recover_trailing_expr(begin.span, false) {
Some(_) => self.mk_expr_err(begin.span),
None => begin,
};

match self.parse_range_end() {
Some(form) => self.parse_pat_range_begin_with(begin, form)?,
None => PatKind::Lit(begin),
}
}
Err(err) => return self.fatal_unexpected_non_pat(err, expected),
}
};
Expand Down Expand Up @@ -615,6 +714,21 @@ impl<'a> Parser<'a> {

self.parse_pat_range_begin_with(begin.clone(), form)?
}
// recover ranges with parentheses around the `(start)..`
PatKind::Err(_)
if self.may_recover()
&& let Some(form) = self.parse_range_end() =>
{
self.dcx().emit_err(UnexpectedParenInRangePat {
span: vec![open_paren, close_paren],
sugg: UnexpectedParenInRangePatSugg {
start_span: open_paren,
end_span: close_paren,
},
});

self.parse_pat_range_begin_with(self.mk_expr(pat.span, ExprKind::Err), form)?
}

// (pat) with optional parentheses
_ => PatKind::Paren(pat),
Expand Down Expand Up @@ -853,6 +967,8 @@ impl<'a> Parser<'a> {
self.parse_literal_maybe_minus()
}?;

let recovered = self.maybe_recover_trailing_expr(bound.span, true);

// recover trailing `)`
if let Some(open_paren) = open_paren {
self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
Expand All @@ -866,7 +982,10 @@ impl<'a> Parser<'a> {
});
}

Ok(bound)
Ok(match recovered {
Some(_) => self.mk_expr_err(bound.span),
None => bound,
})
}

/// Is this the start of a pattern beginning with a path?
Expand Down Expand Up @@ -929,7 +1048,17 @@ impl<'a> Parser<'a> {
.create_err(EnumPatternInsteadOfIdentifier { span: self.prev_token.span }));
}

Ok(PatKind::Ident(binding_annotation, ident, sub))
// Check for method calls after the `ident`,
// but not `ident @ subpat` as `subpat` was already checked and `ident` continues with `@`.

let pat = if sub.is_none()
&& let Some(guar) = self.maybe_recover_trailing_expr(ident.span, false)
{
PatKind::Err(guar)
} else {
PatKind::Ident(binding_annotation, ident, sub)
};
Ok(pat)
}

/// Parse a struct ("record") pattern (e.g. `Foo { ... }` or `Foo::Bar { ... }`).
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/half-open-range-patterns/range_pat_interactions1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@ fn main() {
}
match x as i32 {
0..5+1 => errors_only.push(x),
//~^ error: expected one of `=>`, `if`, or `|`, found `+`
//~^ error: expected a pattern range bound, found an expression
//~| error: exclusive range pattern syntax is experimental
1 | -3..0 => first_or.push(x),
//~^ error: exclusive range pattern syntax is experimental
y @ (0..5 | 6) => or_two.push(y),
//~^ error: exclusive range pattern syntax is experimental
y @ 0..const { 5 + 1 } => assert_eq!(y, 5),
//~^ error: exclusive range pattern syntax is experimental
//~| error: inline-const in pattern position is experimental
y @ -5.. => range_from.push(y),
y @ ..-7 => assert_eq!(y, -8),
//~^ error: exclusive range pattern syntax is experimental
y => bottom.push(y),
}
}
Expand Down
Loading

0 comments on commit 390f768

Please sign in to comment.