Skip to content

Commit d1ba2d2

Browse files
compiler-errorscompiler-errors
committed
Improve parsing errors and suggestions for bad if statements
1 parent 3bdec3c commit d1ba2d2

21 files changed

+325
-143
lines changed

‎compiler/rustc_parse/src/parser/expr.rs

+66-40
Original file line numberDiff line numberDiff line change
@@ -2248,36 +2248,59 @@ impl<'a> Parser<'a> {
22482248
&mut self,
22492249
attrs: AttrVec,
22502250
lo: Span,
2251-
cond: P<Expr>,
2251+
mut cond: P<Expr>,
22522252
) -> PResult<'a, P<Expr>> {
2253-
let missing_then_block_binop_span = || {
2254-
match cond.kind {
2255-
ExprKind::Binary(Spanned { span: binop_span, .. }, _, ref right)
2256-
if let ExprKind::Block(..) = right.kind => Some(binop_span),
2257-
_ => None
2253+
let cond_span = cond.span;
2254+
// Tries to interpret `cond` as either a missing expression if it's a block,
2255+
// or as an unfinished expression if it's a binop and the RHS is a block.
2256+
// We could probably add more recoveries here too...
2257+
let mut recover_block_from_condition = |this: &mut Self| {
2258+
let block = match &mut cond.kind {
2259+
ExprKind::Binary(Spanned { span: binop_span, .. }, _, right)
2260+
if let ExprKind::Block(_, None) = right.kind => {
2261+
this.error_missing_if_then_block(lo, cond_span.shrink_to_lo().to(*binop_span), true).emit();
2262+
std::mem::replace(right, this.mk_expr_err(binop_span.shrink_to_hi()))
2263+
},
2264+
ExprKind::Block(_, None) => {
2265+
this.error_missing_if_cond(lo, cond_span).emit();
2266+
std::mem::replace(&mut cond, this.mk_expr_err(cond_span.shrink_to_hi()))
2267+
}
2268+
_ => {
2269+
return None;
2270+
}
2271+
};
2272+
if let ExprKind::Block(block, _) = &block.kind {
2273+
Some(block.clone())
2274+
} else {
2275+
unreachable!()
22582276
}
22592277
};
2260-
// Verify that the parsed `if` condition makes sense as a condition. If it is a block, then
2261-
// verify that the last statement is either an implicit return (no `;`) or an explicit
2262-
// return. This won't catch blocks with an explicit `return`, but that would be caught by
2263-
// the dead code lint.
2264-
let thn = if self.token.is_keyword(kw::Else) || !cond.returns() {
2265-
if let Some(binop_span) = missing_then_block_binop_span() {
2266-
self.error_missing_if_then_block(lo, None, Some(binop_span)).emit();
2267-
self.mk_block_err(cond.span)
2278+
// Parse then block
2279+
let thn = if self.token.is_keyword(kw::Else) {
2280+
if let Some(block) = recover_block_from_condition(self) {
2281+
block
22682282
} else {
2269-
self.error_missing_if_cond(lo, cond.span)
2283+
self.error_missing_if_then_block(lo, cond_span, false).emit();
2284+
self.mk_block_err(cond_span.shrink_to_hi())
22702285
}
22712286
} else {
22722287
let attrs = self.parse_outer_attributes()?.take_for_recovery(); // For recovery.
2273-
let not_block = self.token != token::OpenDelim(Delimiter::Brace);
2274-
let block = self.parse_block().map_err(|err| {
2275-
if not_block {
2276-
self.error_missing_if_then_block(lo, Some(err), missing_then_block_binop_span())
2288+
let block = if self.check(&token::OpenDelim(Delimiter::Brace)) {
2289+
self.parse_block()?
2290+
} else {
2291+
if let Some(block) = recover_block_from_condition(self) {
2292+
block
22772293
} else {
2278-
err
2294+
// Parse block, which will always fail, but we can add a nice note to the error
2295+
self.parse_block().map_err(|mut err| {
2296+
err.span_note(
2297+
cond_span,
2298+
"the `if` expression is missing a block after this condition",
2299+
);
2300+
err
2301+
})?
22792302
}
2280-
})?;
2303+
};
22812304
self.error_on_if_block_attrs(lo, false, block.span, &attrs);
22822305
block
22832306
};
@@ -2288,31 +2311,34 @@ impl<'a> Parser<'a> {
22882311
fn error_missing_if_then_block(
22892312
&self,
22902313
if_span: Span,
2291-
err: Option<DiagnosticBuilder<'a, ErrorGuaranteed>>,
2292-
binop_span: Option<Span>,
2314+
cond_span: Span,
2315+
is_unfinished: bool,
22932316
) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
2294-
let msg = "this `if` expression has a condition, but no block";
2295-
2296-
let mut err = if let Some(mut err) = err {
2297-
err.span_label(if_span, msg);
2298-
err
2317+
let mut err = self.struct_span_err(
2318+
if_span,
2319+
"this `if` expression is missing a block after the condition",
2320+
);
2321+
if is_unfinished {
2322+
err.span_help(cond_span, "this binary operation is possibly unfinished");
22992323
} else {
2300-
self.struct_span_err(if_span, msg)
2301-
};
2302-
2303-
if let Some(binop_span) = binop_span {
2304-
err.span_help(binop_span, "maybe you forgot the right operand of the condition?");
2324+
err.span_help(cond_span.shrink_to_hi(), "add a block here");
23052325
}
2306-
23072326
err
23082327
}
23092328

2310-
fn error_missing_if_cond(&self, lo: Span, span: Span) -> P<ast::Block> {
2311-
let sp = self.sess.source_map().next_point(lo);
2312-
self.struct_span_err(sp, "missing condition for `if` expression")
2313-
.span_label(sp, "expected if condition here")
2314-
.emit();
2315-
self.mk_block_err(span)
2329+
fn error_missing_if_cond(
2330+
&self,
2331+
lo: Span,
2332+
span: Span,
2333+
) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
2334+
let next_span = self.sess.source_map().next_point(lo);
2335+
let mut err = self.struct_span_err(next_span, "missing condition for `if` expression");
2336+
err.span_label(next_span, "expected condition here");
2337+
err.span_label(
2338+
self.sess.source_map().start_point(span),
2339+
"if this block is the condition of the `if` expression, then it must be followed by another block"
2340+
);
2341+
err
23162342
}
23172343

23182344
/// Parses the condition of a `if` or `while` expression.

‎compiler/rustc_parse/src/parser/stmt.rs

+16-3
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,23 @@ impl<'a> Parser<'a> {
432432
//
433433
// which is valid in other languages, but not Rust.
434434
match self.parse_stmt_without_recovery(false, ForceCollect::No) {
435-
// If the next token is an open brace (e.g., `if a b {`), the place-
436-
// inside-a-block suggestion would be more likely wrong than right.
435+
// If the next token is an open brace, e.g., we have:
436+
//
437+
// if expr other_expr {
438+
// ^ ^ ^- lookahead(1) is a brace
439+
// | |- current token is not "else"
440+
// |- (statement we just parsed)
441+
//
442+
// the place-inside-a-block suggestion would be more likely wrong than right.
443+
//
444+
// FIXME(compiler-errors): this should probably parse an arbitrary expr and not
445+
// just lookahead one token, so we can see if there's a brace after _that_,
446+
// since we want to protect against:
447+
// `if 1 1 + 1 {` being suggested as `if { 1 } 1 + 1 {`
448+
// + +
437449
Ok(Some(_))
438-
if self.look_ahead(1, |t| t == &token::OpenDelim(Delimiter::Brace))
450+
if (!self.token.is_keyword(kw::Else)
451+
&& self.look_ahead(1, |t| t == &token::OpenDelim(Delimiter::Brace)))
439452
|| do_not_suggest_help => {}
440453
// Do not suggest `if foo println!("") {;}` (as would be seen in test for #46836).
441454
Ok(Some(Stmt { kind: StmtKind::Empty, .. })) => {}

‎src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr

+8-2
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,16 @@ LL | println!("Then when?");
2525
error: expected `{`, found `;`
2626
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:20:31
2727
|
28-
LL | if not // lack of braces is [sic]
29-
| -- this `if` expression has a condition, but no block
3028
LL | println!("Then when?");
3129
| ^ expected `{`
30+
|
31+
note: the `if` expression is missing a block after this condition
32+
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:19:8
33+
|
34+
LL | if not // lack of braces is [sic]
35+
| ________^
36+
LL | | println!("Then when?");
37+
| |______________________________^
3238

3339
error: unexpected `2` after identifier
3440
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:26:24

‎src/test/ui/expr/if/if-without-block.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
fn main() {
22
let n = 1;
33
if 5 == {
4-
//~^ NOTE this `if` expression has a condition, but no block
4+
//~^ ERROR this `if` expression is missing a block after the condition
55
println!("five");
66
}
77
}
8-
//~^ ERROR expected `{`, found `}`
9-
//~| NOTE expected `{`

‎src/test/ui/expr/if/if-without-block.stderr

+6-9
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
1-
error: expected `{`, found `}`
2-
--> $DIR/if-without-block.rs:7:1
1+
error: this `if` expression is missing a block after the condition
2+
--> $DIR/if-without-block.rs:3:5
33
|
44
LL | if 5 == {
5-
| -- this `if` expression has a condition, but no block
6-
...
7-
LL | }
8-
| ^ expected `{`
5+
| ^^
96
|
10-
help: maybe you forgot the right operand of the condition?
11-
--> $DIR/if-without-block.rs:3:10
7+
help: this binary operation is possibly unfinished
8+
--> $DIR/if-without-block.rs:3:8
129
|
1310
LL | if 5 == {
14-
| ^^
11+
| ^^^^
1512

1613
error: aborting due to previous error
1714

‎src/test/ui/issues/issue-39848.stderr

+9-3
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,19 @@ error: expected `{`, found `foo`
22
--> $DIR/issue-39848.rs:3:21
33
|
44
LL | if $tgt.has_$field() {}
5-
| -- ^^^^^^ expected `{`
6-
| |
7-
| this `if` expression has a condition, but no block
5+
| ^^^^^^ expected `{`
86
...
97
LL | get_opt!(bar, foo);
108
| ------------------ in this macro invocation
119
|
10+
note: the `if` expression is missing a block after this condition
11+
--> $DIR/issue-39848.rs:3:12
12+
|
13+
LL | if $tgt.has_$field() {}
14+
| ^^^^^^^^^
15+
...
16+
LL | get_opt!(bar, foo);
17+
| ------------------ in this macro invocation
1218
= note: this error originates in the macro `get_opt` (in Nightly builds, run with -Z macro-backtrace for more info)
1319
help: try placing this code inside a block
1420
|

‎src/test/ui/missing/missing-block-hint.stderr

+12-5
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,25 @@ error: expected `{`, found `=>`
22
--> $DIR/missing-block-hint.rs:3:18
33
|
44
LL | if (foo) => {}
5-
| -- ^^ expected `{`
6-
| |
7-
| this `if` expression has a condition, but no block
5+
| ^^ expected `{`
6+
|
7+
note: the `if` expression is missing a block after this condition
8+
--> $DIR/missing-block-hint.rs:3:12
9+
|
10+
LL | if (foo) => {}
11+
| ^^^^^
812

913
error: expected `{`, found `bar`
1014
--> $DIR/missing-block-hint.rs:7:13
1115
|
12-
LL | if (foo)
13-
| -- this `if` expression has a condition, but no block
1416
LL | bar;
1517
| ^^^ expected `{`
1618
|
19+
note: the `if` expression is missing a block after this condition
20+
--> $DIR/missing-block-hint.rs:6:12
21+
|
22+
LL | if (foo)
23+
| ^^^^^
1724
help: try placing this code inside a block
1825
|
1926
LL | { bar; }

‎src/test/ui/parser/bad-if-statements.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
fn a() {
2+
if {}
3+
//~^ ERROR missing condition for `if` expression
4+
}
5+
6+
fn b() {
7+
if true && {}
8+
//~^ ERROR this `if` expression is missing a block after the condition
9+
}
10+
11+
fn c() {
12+
let x = {};
13+
if true x
14+
//~^ ERROR expected `{`, found `x`
15+
}
16+
17+
fn a2() {
18+
if {} else {}
19+
//~^ ERROR missing condition for `if` expression
20+
}
21+
22+
fn b2() {
23+
if true && {} else {}
24+
//~^ ERROR this `if` expression is missing a block after the condition
25+
}
26+
27+
fn c2() {
28+
let x = {};
29+
if true x else {}
30+
//~^ ERROR expected `{`, found `x`
31+
}
32+
33+
fn d() {
34+
if true else {}
35+
//~^ ERROR this `if` expression is missing a block after the condition
36+
}
37+
38+
fn main() {}

‎src/test/ui/parser/bad-if-statements.stderr

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
error: missing condition for `if` expression
2+
--> $DIR/bad-if-statements.rs:2:7
3+
|
4+
LL | if {}
5+
| ^- if this block is the condition of the `if` expression, then it must be followed by another block
6+
| |
7+
| expected condition here
8+
9+
error: this `if` expression is missing a block after the condition
10+
--> $DIR/bad-if-statements.rs:7:5
11+
|
12+
LL | if true && {}
13+
| ^^
14+
|
15+
help: this binary operation is possibly unfinished
16+
--> $DIR/bad-if-statements.rs:7:8
17+
|
18+
LL | if true && {}
19+
| ^^^^^^^
20+
21+
error: expected `{`, found `x`
22+
--> $DIR/bad-if-statements.rs:13:13
23+
|
24+
LL | if true x
25+
| ^ expected `{`
26+
|
27+
note: the `if` expression is missing a block after this condition
28+
--> $DIR/bad-if-statements.rs:13:8
29+
|
30+
LL | if true x
31+
| ^^^^
32+
help: try placing this code inside a block
33+
|
34+
LL | if true { x }
35+
| + +
36+
37+
error: missing condition for `if` expression
38+
--> $DIR/bad-if-statements.rs:18:7
39+
|
40+
LL | if {} else {}
41+
| ^- if this block is the condition of the `if` expression, then it must be followed by another block
42+
| |
43+
| expected condition here
44+
45+
error: this `if` expression is missing a block after the condition
46+
--> $DIR/bad-if-statements.rs:23:5
47+
|
48+
LL | if true && {} else {}
49+
| ^^
50+
|
51+
help: this binary operation is possibly unfinished
52+
--> $DIR/bad-if-statements.rs:23:8
53+
|
54+
LL | if true && {} else {}
55+
| ^^^^^^^
56+
57+
error: expected `{`, found `x`
58+
--> $DIR/bad-if-statements.rs:29:13
59+
|
60+
LL | if true x else {}
61+
| ^ expected `{`
62+
|
63+
note: the `if` expression is missing a block after this condition
64+
--> $DIR/bad-if-statements.rs:29:8
65+
|
66+
LL | if true x else {}
67+
| ^^^^
68+
help: try placing this code inside a block
69+
|
70+
LL | if true { x } else {}
71+
| + +
72+
73+
error: this `if` expression is missing a block after the condition
74+
--> $DIR/bad-if-statements.rs:34:5
75+
|
76+
LL | if true else {}
77+
| ^^
78+
|
79+
help: add a block here
80+
--> $DIR/bad-if-statements.rs:34:12
81+
|
82+
LL | if true else {}
83+
| ^
84+
85+
error: aborting due to 7 previous errors
86+

0 commit comments

Comments
 (0)