Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase accuracy of if condition misparse suggestion #133051

Merged
merged 4 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading