Skip to content

Commit 0531521

Browse files
authored
Rollup merge of #97298 - compiler-errors:if-else-stmt-braces, r=davidtwco
Parse expression after `else` as a condition if followed by `{` Fixes #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.
2 parents 3569a42 + 9be37b2 commit 0531521

File tree

3 files changed

+147
-5
lines changed

3 files changed

+147
-5
lines changed

compiler/rustc_parse/src/parser/expr.rs

+57-5
Original file line numberDiff line numberDiff line change
@@ -2010,6 +2010,12 @@ impl<'a> Parser<'a> {
20102010
Ok(self.mk_expr(blk.span, ExprKind::Block(blk, opt_label), attrs))
20112011
}
20122012

2013+
/// Parse a block which takes no attributes and has no label
2014+
fn parse_simple_block(&mut self) -> PResult<'a, P<Expr>> {
2015+
let blk = self.parse_block()?;
2016+
Ok(self.mk_expr(blk.span, ExprKind::Block(blk, None), AttrVec::new()))
2017+
}
2018+
20132019
/// Recover on an explicitly quantified closure expression, e.g., `for<'a> |x: &'a u8| *x + 1`.
20142020
fn recover_quantified_closure_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
20152021
let lo = self.token.span;
@@ -2157,14 +2163,22 @@ impl<'a> Parser<'a> {
21572163
let lo = self.prev_token.span;
21582164
let cond = self.parse_cond_expr()?;
21592165

2166+
self.parse_if_after_cond(attrs, lo, cond)
2167+
}
2168+
2169+
fn parse_if_after_cond(
2170+
&mut self,
2171+
attrs: AttrVec,
2172+
lo: Span,
2173+
cond: P<Expr>,
2174+
) -> PResult<'a, P<Expr>> {
21602175
let missing_then_block_binop_span = || {
21612176
match cond.kind {
21622177
ExprKind::Binary(Spanned { span: binop_span, .. }, _, ref right)
21632178
if let ExprKind::Block(..) = right.kind => Some(binop_span),
21642179
_ => None
21652180
}
21662181
};
2167-
21682182
// Verify that the parsed `if` condition makes sense as a condition. If it is a block, then
21692183
// verify that the last statement is either an implicit return (no `;`) or an explicit
21702184
// return. This won't catch blocks with an explicit `return`, but that would be caught by
@@ -2256,15 +2270,53 @@ impl<'a> Parser<'a> {
22562270

22572271
/// Parses an `else { ... }` expression (`else` token already eaten).
22582272
fn parse_else_expr(&mut self) -> PResult<'a, P<Expr>> {
2259-
let ctx_span = self.prev_token.span; // `else`
2273+
let else_span = self.prev_token.span; // `else`
22602274
let attrs = self.parse_outer_attributes()?.take_for_recovery(); // For recovery.
22612275
let expr = if self.eat_keyword(kw::If) {
22622276
self.parse_if_expr(AttrVec::new())?
2277+
} else if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) {
2278+
self.parse_simple_block()?
22632279
} else {
2264-
let blk = self.parse_block()?;
2265-
self.mk_expr(blk.span, ExprKind::Block(blk, None), AttrVec::new())
2280+
let snapshot = self.create_snapshot_for_diagnostic();
2281+
let first_tok = super::token_descr(&self.token);
2282+
let first_tok_span = self.token.span;
2283+
match self.parse_expr() {
2284+
Ok(cond)
2285+
// If it's not a free-standing expression, and is followed by a block,
2286+
// then it's very likely the condition to an `else if`.
2287+
if self.check(&TokenKind::OpenDelim(Delimiter::Brace))
2288+
&& classify::expr_requires_semi_to_be_stmt(&cond) =>
2289+
{
2290+
self.struct_span_err(first_tok_span, format!("expected `{{`, found {first_tok}"))
2291+
.span_label(else_span, "expected an `if` or a block after this `else`")
2292+
.span_suggestion(
2293+
cond.span.shrink_to_lo(),
2294+
"add an `if` if this is the condition to an chained `if` statement after the `else`",
2295+
"if ".to_string(),
2296+
Applicability::MaybeIncorrect,
2297+
).multipart_suggestion(
2298+
"... otherwise, place this expression inside of a block if it is not an `if` condition",
2299+
vec![
2300+
(cond.span.shrink_to_lo(), "{ ".to_string()),
2301+
(cond.span.shrink_to_hi(), " }".to_string()),
2302+
],
2303+
Applicability::MaybeIncorrect,
2304+
)
2305+
.emit();
2306+
self.parse_if_after_cond(AttrVec::new(), cond.span.shrink_to_lo(), cond)?
2307+
}
2308+
Err(e) => {
2309+
e.cancel();
2310+
self.restore_snapshot(snapshot);
2311+
self.parse_simple_block()?
2312+
},
2313+
Ok(_) => {
2314+
self.restore_snapshot(snapshot);
2315+
self.parse_simple_block()?
2316+
},
2317+
}
22662318
};
2267-
self.error_on_if_block_attrs(ctx_span, true, expr.span, &attrs);
2319+
self.error_on_if_block_attrs(else_span, true, expr.span, &attrs);
22682320
Ok(expr)
22692321
}
22702322

src/test/ui/parser/else-no-if.rs

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
fn foo() {
2+
if true {
3+
} else false {
4+
//~^ ERROR expected `{`, found keyword `false`
5+
}
6+
}
7+
8+
fn foo2() {
9+
if true {
10+
} else falsy() {
11+
//~^ ERROR expected `{`, found `falsy`
12+
}
13+
}
14+
15+
fn foo3() {
16+
if true {
17+
} else falsy();
18+
//~^ ERROR expected `{`, found `falsy`
19+
}
20+
21+
fn foo4() {
22+
if true {
23+
} else loop{}
24+
//~^ ERROR expected `{`, found keyword `loop`
25+
{}
26+
}
27+
28+
fn falsy() -> bool {
29+
false
30+
}
31+
32+
fn main() {}

src/test/ui/parser/else-no-if.stderr

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
error: expected `{`, found keyword `false`
2+
--> $DIR/else-no-if.rs:3:12
3+
|
4+
LL | } else false {
5+
| ---- ^^^^^
6+
| |
7+
| expected an `if` or a block after this `else`
8+
|
9+
help: add an `if` if this is the condition to an chained `if` statement after the `else`
10+
|
11+
LL | } else if false {
12+
| ++
13+
help: ... otherwise, place this expression inside of a block if it is not an `if` condition
14+
|
15+
LL | } else { false } {
16+
| + +
17+
18+
error: expected `{`, found `falsy`
19+
--> $DIR/else-no-if.rs:10:12
20+
|
21+
LL | } else falsy() {
22+
| ---- ^^^^^
23+
| |
24+
| expected an `if` or a block after this `else`
25+
|
26+
help: add an `if` if this is the condition to an chained `if` statement after the `else`
27+
|
28+
LL | } else if falsy() {
29+
| ++
30+
help: ... otherwise, place this expression inside of a block if it is not an `if` condition
31+
|
32+
LL | } else { falsy() } {
33+
| + +
34+
35+
error: expected `{`, found `falsy`
36+
--> $DIR/else-no-if.rs:17:12
37+
|
38+
LL | } else falsy();
39+
| ^^^^^ expected `{`
40+
|
41+
help: try placing this code inside a block
42+
|
43+
LL | } else { falsy() };
44+
| + +
45+
46+
error: expected `{`, found keyword `loop`
47+
--> $DIR/else-no-if.rs:23:12
48+
|
49+
LL | } else loop{}
50+
| ^^^^ expected `{`
51+
|
52+
help: try placing this code inside a block
53+
|
54+
LL | } else { loop{} }
55+
| + +
56+
57+
error: aborting due to 4 previous errors
58+

0 commit comments

Comments
 (0)