Skip to content

Commit

Permalink
Rollup merge of #133051 - estebank:cond-misparse, r=jieyouxu
Browse files Browse the repository at this point in the history
Increase accuracy of `if` condition misparse suggestion

Fix #132656.

Look at the expression that was parsed when trying to recover from a bad `if` condition to determine what was likely intended by the user beyond "maybe this was meant to be an `else` body".

```
error: expected `{`, found `map`
  --> $DIR/missing-dot-on-if-condition-expression-fixable.rs:4:30
   |
LL |     for _ in [1, 2, 3].iter()map(|x| x) {}
   |                              ^^^ expected `{`
   |
help: you might have meant to write a method call
   |
LL |     for _ in [1, 2, 3].iter().map(|x| x) {}
   |                              +
```

If a macro statement has been parsed after `else`, suggest a missing `if`:

```
error: expected `{`, found `falsy`
  --> $DIR/else-no-if.rs:47:12
   |
LL |     } else falsy! {} {
   |       ---- ^^^^^
   |       |
   |       expected an `if` or a block after this `else`
   |
help: add an `if` if this is the condition of a chained `else if` statement
   |
LL |     } else if falsy! {} {
   |            ++
```
  • Loading branch information
jieyouxu authored Nov 17, 2024
2 parents 2d9690d + 6913194 commit 2f62fd3
Show file tree
Hide file tree
Showing 17 changed files with 435 additions and 36 deletions.
11 changes: 10 additions & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2683,6 +2683,13 @@ impl<'a> Parser<'a> {
// ^^
// }
//
// We account for macro calls that were meant as conditions as well.
//
// if ... {
// } else if macro! { foo bar } {
// ^^
// }
//
// If $cond is "statement-like" such as ExprKind::While then we
// want to suggest wrapping in braces.
//
Expand All @@ -2693,7 +2700,9 @@ impl<'a> Parser<'a> {
// }
// ^
if self.check(&TokenKind::OpenDelim(Delimiter::Brace))
&& classify::expr_requires_semi_to_be_stmt(&cond) =>
&& (classify::expr_requires_semi_to_be_stmt(&cond)
|| matches!(cond.kind, ExprKind::MacCall(..)))
=>
{
self.dcx().emit_err(errors::ExpectedElseBlock {
first_tok_span,
Expand Down
109 changes: 102 additions & 7 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ impl<'a> Parser<'a> {
}

fn error_block_no_opening_brace_msg(&mut self, msg: Cow<'static, str>) -> Diag<'a> {
let prev = self.prev_token.span;
let sp = self.token.span;
let mut e = self.dcx().struct_span_err(sp, msg);
let do_not_suggest_help = self.token.is_keyword(kw::In) || self.token == token::Colon;
Expand Down Expand Up @@ -514,8 +515,97 @@ impl<'a> Parser<'a> {
} else {
stmt.span
};
self.suggest_fixes_misparsed_for_loop_head(
&mut e,
prev.between(sp),
stmt_span,
&stmt.kind,
);
}
Err(e) => {
self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
e.cancel();
}
_ => {}
}
e.span_label(sp, "expected `{`");
e
}

fn suggest_fixes_misparsed_for_loop_head(
&self,
e: &mut Diag<'_>,
between: Span,
stmt_span: Span,
stmt_kind: &StmtKind,
) {
match (&self.token.kind, &stmt_kind) {
(token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr))
if let ExprKind::Call(..) = expr.kind =>
{
// for _ in x y() {}
e.span_suggestion_verbose(
between,
"you might have meant to write a method call",
".".to_string(),
Applicability::MaybeIncorrect,
);
}
(token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr))
if let ExprKind::Field(..) = expr.kind =>
{
// for _ in x y.z {}
e.span_suggestion_verbose(
between,
"you might have meant to write a field access",
".".to_string(),
Applicability::MaybeIncorrect,
);
}
(token::CloseDelim(Delimiter::Brace), StmtKind::Expr(expr))
if let ExprKind::Struct(expr) = &expr.kind
&& let None = expr.qself
&& expr.path.segments.len() == 1 =>
{
// This is specific to "mistyped `if` condition followed by empty body"
//
// for _ in x y {}
e.span_suggestion_verbose(
between,
"you might have meant to write a field access",
".".to_string(),
Applicability::MaybeIncorrect,
);
}
(token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr))
if let ExprKind::Lit(lit) = expr.kind
&& let None = lit.suffix
&& let token::LitKind::Integer | token::LitKind::Float = lit.kind =>
{
// for _ in x 0 {}
// for _ in x 0.0 {}
e.span_suggestion_verbose(
between,
format!("you might have meant to write a field access"),
".".to_string(),
Applicability::MaybeIncorrect,
);
}
(token::OpenDelim(Delimiter::Brace), StmtKind::Expr(expr))
if let ExprKind::Loop(..)
| ExprKind::If(..)
| ExprKind::While(..)
| ExprKind::Match(..)
| ExprKind::ForLoop { .. }
| ExprKind::TryBlock(..)
| ExprKind::Ret(..)
| ExprKind::Closure(..)
| ExprKind::Struct(..)
| ExprKind::Try(..) = expr.kind =>
{
// These are more likely to have been meant as a block body.
e.multipart_suggestion(
"try placing this code inside a block",
"you might have meant to write this as part of a block",
vec![
(stmt_span.shrink_to_lo(), "{ ".to_string()),
(stmt_span.shrink_to_hi(), " }".to_string()),
Expand All @@ -524,14 +614,19 @@ impl<'a> Parser<'a> {
Applicability::MaybeIncorrect,
);
}
Err(e) => {
self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
e.cancel();
(token::OpenDelim(Delimiter::Brace), _) => {}
(_, _) => {
e.multipart_suggestion(
"you might have meant to write this as part of a block",
vec![
(stmt_span.shrink_to_lo(), "{ ".to_string()),
(stmt_span.shrink_to_hi(), " }".to_string()),
],
// Speculative; has been misleading in the past (#46836).
Applicability::MaybeIncorrect,
);
}
_ => {}
}
e.span_label(sp, "expected `{`");
e
}

fn error_block_no_opening_brace<T>(&mut self) -> PResult<'a, T> {
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/issues/issue-39848.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ LL | if $tgt.has_$field() {}
LL | get_opt!(bar, foo);
| ------------------ in this macro invocation
= note: this error originates in the macro `get_opt` (in Nightly builds, run with -Z macro-backtrace for more info)
help: try placing this code inside a block
help: you might have meant to write a method call
|
LL | if $tgt.has_{ $field() } {}
| + +
LL | if $tgt.has_.$field() {}
| +

error: aborting due to 1 previous error

2 changes: 1 addition & 1 deletion tests/ui/let-else/let-else-if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: conditional `else if` is not supported for `let...else`
LL | let Some(_) = Some(()) else if true {
| ^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL ~ let Some(_) = Some(()) else { if true {
LL |
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lint/issue-104392.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | { unsafe 92 }
| |
| while parsing this `unsafe` expression
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | { unsafe { 92 } }
| + +
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/missing/missing-block-hint.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ note: the `if` expression is missing a block after this condition
|
LL | if (foo)
| ^^^^^
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | { bar; }
| + +
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/parser/bad-if-statements.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ note: the `if` expression is missing a block after this condition
|
LL | if true x
| ^^^^
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | if true { x }
| + +
Expand Down Expand Up @@ -65,7 +65,7 @@ note: the `if` expression is missing a block after this condition
|
LL | if true x else {}
| ^^^^
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | if true { x } else {}
| + +
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/parser/block-no-opening-brace.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | loop
LL | let x = 0;
| ^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | { let x = 0; }
| + +
Expand All @@ -21,7 +21,7 @@ LL | while true
LL | let x = 0;
| ^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | { let x = 0; }
| + +
Expand All @@ -32,7 +32,7 @@ error: expected `{`, found keyword `let`
LL | let x = 0;
| ^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | { let x = 0; }
| + +
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/closure-return-syntax.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: expected `{`, found `22`
LL | let x = || -> i32 22;
| ^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | let x = || -> i32 { 22 };
| + +
Expand Down
18 changes: 10 additions & 8 deletions tests/ui/parser/else-no-if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ error: expected `{`, found `falsy`
LL | } else falsy();
| ^^^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | } else { falsy() };
| + +
Expand All @@ -41,7 +41,7 @@ error: expected `{`, found keyword `loop`
LL | } else loop{}
| ^^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | } else { loop{} }
| + +
Expand All @@ -65,7 +65,7 @@ error: expected `{`, found `falsy`
LL | } else falsy!();
| ^^^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | } else { falsy!() };
| + +
Expand All @@ -74,20 +74,22 @@ error: expected `{`, found `falsy`
--> $DIR/else-no-if.rs:47:12
|
LL | } else falsy! {} {
| ^^^^^ expected `{`
| ---- ^^^^^
| |
| expected an `if` or a block after this `else`
|
help: try placing this code inside a block
help: add an `if` if this is the condition of a chained `else if` statement
|
LL | } else { falsy! {} } {
| + +
LL | } else if falsy! {} {
| ++

error: expected `{`, found `falsy`
--> $DIR/else-no-if.rs:54:12
|
LL | } else falsy! {};
| ^^^^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | } else { falsy! {} };
| + +
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/parser/label-after-block-like.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ note: the `if` expression is missing a block after this condition
|
LL | if let () = () 'a {}
| ^^^^^^^^^^^
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | if let () = () { 'a {} }
| + +
Expand Down Expand Up @@ -53,7 +53,7 @@ note: the `if` expression is missing a block after this condition
|
LL | if true 'a {}
| ^^^^
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | if true { 'a {} }
| + +
Expand All @@ -80,7 +80,7 @@ LL | loop 'a {}
| |
| while parsing this `loop` expression
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | loop { 'a {} }
| + +
Expand Down Expand Up @@ -108,7 +108,7 @@ LL | while true 'a {}
| | this `while` condition successfully parsed
| while parsing the body of this `while` expression
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | while true { 'a {} }
| + +
Expand Down Expand Up @@ -136,7 +136,7 @@ LL | while let () = () 'a {}
| | this `while` condition successfully parsed
| while parsing the body of this `while` expression
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | while let () = () { 'a {} }
| + +
Expand All @@ -161,7 +161,7 @@ error: expected `{`, found `'a`
LL | for _ in 0..0 'a {}
| ^^ expected `{`
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | for _ in 0..0 { 'a {} }
| + +
Expand All @@ -188,7 +188,7 @@ LL | unsafe 'a {}
| |
| while parsing this `unsafe` expression
|
help: try placing this code inside a block
help: you might have meant to write this as part of a block
|
LL | unsafe { 'a {} }
| + +
Expand Down
Loading

0 comments on commit 2f62fd3

Please sign in to comment.