Skip to content

Commit

Permalink
Rollup merge of rust-lang#97298 - compiler-errors:if-else-stmt-braces…
Browse files Browse the repository at this point in the history
…, r=davidtwco

Parse expression after `else` as a condition if followed by `{`

Fixes rust-lang#49361.

Two things:
1. This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅
2. Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an `if`" case is the right one.
  • Loading branch information
Dylan-DPC authored May 24, 2022
2 parents 0aa1109 + 9be37b2 commit bc1fe98
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 5 deletions.
62 changes: 57 additions & 5 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2010,6 +2010,12 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(blk.span, ExprKind::Block(blk, opt_label), attrs))
}

/// Parse a block which takes no attributes and has no label
fn parse_simple_block(&mut self) -> PResult<'a, P<Expr>> {
let blk = self.parse_block()?;
Ok(self.mk_expr(blk.span, ExprKind::Block(blk, None), AttrVec::new()))
}

/// Recover on an explicitly quantified closure expression, e.g., `for<'a> |x: &'a u8| *x + 1`.
fn recover_quantified_closure_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
Expand Down Expand Up @@ -2157,14 +2163,22 @@ impl<'a> Parser<'a> {
let lo = self.prev_token.span;
let cond = self.parse_cond_expr()?;

self.parse_if_after_cond(attrs, lo, cond)
}

fn parse_if_after_cond(
&mut self,
attrs: AttrVec,
lo: Span,
cond: P<Expr>,
) -> PResult<'a, P<Expr>> {
let missing_then_block_binop_span = || {
match cond.kind {
ExprKind::Binary(Spanned { span: binop_span, .. }, _, ref right)
if let ExprKind::Block(..) = right.kind => Some(binop_span),
_ => None
}
};

// Verify that the parsed `if` condition makes sense as a condition. If it is a block, then
// verify that the last statement is either an implicit return (no `;`) or an explicit
// return. This won't catch blocks with an explicit `return`, but that would be caught by
Expand Down Expand Up @@ -2256,15 +2270,53 @@ impl<'a> Parser<'a> {

/// Parses an `else { ... }` expression (`else` token already eaten).
fn parse_else_expr(&mut self) -> PResult<'a, P<Expr>> {
let ctx_span = self.prev_token.span; // `else`
let else_span = self.prev_token.span; // `else`
let attrs = self.parse_outer_attributes()?.take_for_recovery(); // For recovery.
let expr = if self.eat_keyword(kw::If) {
self.parse_if_expr(AttrVec::new())?
} else if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) {
self.parse_simple_block()?
} else {
let blk = self.parse_block()?;
self.mk_expr(blk.span, ExprKind::Block(blk, None), AttrVec::new())
let snapshot = self.create_snapshot_for_diagnostic();
let first_tok = super::token_descr(&self.token);
let first_tok_span = self.token.span;
match self.parse_expr() {
Ok(cond)
// If it's not a free-standing expression, and is followed by a block,
// then it's very likely the condition to an `else if`.
if self.check(&TokenKind::OpenDelim(Delimiter::Brace))
&& classify::expr_requires_semi_to_be_stmt(&cond) =>
{
self.struct_span_err(first_tok_span, format!("expected `{{`, found {first_tok}"))
.span_label(else_span, "expected an `if` or a block after this `else`")
.span_suggestion(
cond.span.shrink_to_lo(),
"add an `if` if this is the condition to an chained `if` statement after the `else`",
"if ".to_string(),
Applicability::MaybeIncorrect,
).multipart_suggestion(
"... otherwise, place this expression inside of a block if it is not an `if` condition",
vec![
(cond.span.shrink_to_lo(), "{ ".to_string()),
(cond.span.shrink_to_hi(), " }".to_string()),
],
Applicability::MaybeIncorrect,
)
.emit();
self.parse_if_after_cond(AttrVec::new(), cond.span.shrink_to_lo(), cond)?
}
Err(e) => {
e.cancel();
self.restore_snapshot(snapshot);
self.parse_simple_block()?
},
Ok(_) => {
self.restore_snapshot(snapshot);
self.parse_simple_block()?
},
}
};
self.error_on_if_block_attrs(ctx_span, true, expr.span, &attrs);
self.error_on_if_block_attrs(else_span, true, expr.span, &attrs);
Ok(expr)
}

Expand Down
32 changes: 32 additions & 0 deletions src/test/ui/parser/else-no-if.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
fn foo() {
if true {
} else false {
//~^ ERROR expected `{`, found keyword `false`
}
}

fn foo2() {
if true {
} else falsy() {
//~^ ERROR expected `{`, found `falsy`
}
}

fn foo3() {
if true {
} else falsy();
//~^ ERROR expected `{`, found `falsy`
}

fn foo4() {
if true {
} else loop{}
//~^ ERROR expected `{`, found keyword `loop`
{}
}

fn falsy() -> bool {
false
}

fn main() {}
58 changes: 58 additions & 0 deletions src/test/ui/parser/else-no-if.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
error: expected `{`, found keyword `false`
--> $DIR/else-no-if.rs:3:12
|
LL | } else false {
| ---- ^^^^^
| |
| expected an `if` or a block after this `else`
|
help: add an `if` if this is the condition to an chained `if` statement after the `else`
|
LL | } else if false {
| ++
help: ... otherwise, place this expression inside of a block if it is not an `if` condition
|
LL | } else { false } {
| + +

error: expected `{`, found `falsy`
--> $DIR/else-no-if.rs:10:12
|
LL | } else falsy() {
| ---- ^^^^^
| |
| expected an `if` or a block after this `else`
|
help: add an `if` if this is the condition to an chained `if` statement after the `else`
|
LL | } else if falsy() {
| ++
help: ... otherwise, place this expression inside of a block if it is not an `if` condition
|
LL | } else { falsy() } {
| + +

error: expected `{`, found `falsy`
--> $DIR/else-no-if.rs:17:12
|
LL | } else falsy();
| ^^^^^ expected `{`
|
help: try placing this code inside a block
|
LL | } else { falsy() };
| + +

error: expected `{`, found keyword `loop`
--> $DIR/else-no-if.rs:23:12
|
LL | } else loop{}
| ^^^^ expected `{`
|
help: try placing this code inside a block
|
LL | } else { loop{} }
| + +

error: aborting due to 4 previous errors

0 comments on commit bc1fe98

Please sign in to comment.