Skip to content

Commit 2ce56c5

Browse files
committed
Added warning/error for if-let-chain ambiguity.
With eRFC 2497, previously accepted ambigious syntax regarding use of `&&` and `||` in if-let and while-let statements should now warn or error depending on the edition. This commit takes a naive approach to detecting ambigious use of `&&` or `||` and will probably need fine tuned to handle all cases.
1 parent 685fb54 commit 2ce56c5

File tree

5 files changed

+250
-0
lines changed

5 files changed

+250
-0
lines changed

src/libsyntax/parse/parser.rs

+53
Original file line numberDiff line numberDiff line change
@@ -3327,6 +3327,8 @@ impl<'a> Parser<'a> {
33273327
let pats = self.parse_pats()?;
33283328
self.expect(&token::Eq)?;
33293329
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
3330+
self.while_if_let_ambiguity(&expr);
3331+
33303332
let thn = self.parse_block()?;
33313333
let (hi, els) = if self.eat_keyword(keywords::Else) {
33323334
let expr = self.parse_else_expr()?;
@@ -3337,6 +3339,56 @@ impl<'a> Parser<'a> {
33373339
Ok(self.mk_expr(lo.to(hi), ExprKind::IfLet(pats, expr, thn, els), attrs))
33383340
}
33393341

3342+
/// With eRFC 2497, we need to check whether an expression is ambigious and warn or error
3343+
/// depending on the edition, this function handles that.
3344+
fn while_if_let_ambiguity(&self, expr: &P<Expr>) {
3345+
if let Some((span, op_kind)) = self.while_if_let_expr_ambiguity(&expr) {
3346+
let message = format!("ambigious use of `{}`", op_kind.to_string());
3347+
let mut err = if self.span.edition() >= Edition::Edition2018 {
3348+
self.diagnostic().struct_span_err(span, &message)
3349+
} else {
3350+
self.diagnostic().struct_span_warn(span, &message)
3351+
};
3352+
3353+
let note = if self.span.edition() >= Edition::Edition2018 {
3354+
"This will be a error until the `let_chains` feature is stabilized."
3355+
} else {
3356+
"This will be a error in Rust 2018 until the `let_chains` feature is stabilized."
3357+
};
3358+
err.note(note);
3359+
3360+
if let Ok(snippet) = self.sess.source_map().span_to_snippet(span) {
3361+
err.span_suggestion(
3362+
span, "consider adding parenthesis", format!("({})", snippet),
3363+
);
3364+
}
3365+
3366+
err.emit();
3367+
}
3368+
}
3369+
3370+
/// With eRFC 2497 adding if-let chains, there is a requirement that the parsing of
3371+
/// `&&` and `||` in a if-let statement be unambigious. This function returns a span and
3372+
/// a `BinOpKind` (either `&&` or `||` depending on what was ambigious) if it is determined
3373+
/// that the current expression parsed is ambigious and will break in future.
3374+
fn while_if_let_expr_ambiguity(&self, expr: &P<Expr>) -> Option<(Span, BinOpKind)> {
3375+
debug!("while_if_let_expr_ambiguity: expr.node: {:?}", expr.node);
3376+
match &expr.node {
3377+
ExprKind::Binary(op, _, _) if op.node == BinOpKind::And || op.node == BinOpKind::Or => {
3378+
Some((expr.span, op.node))
3379+
},
3380+
ExprKind::Range(ref lhs, ref rhs, _) => {
3381+
let lhs_ambigious = lhs.as_ref()
3382+
.and_then(|lhs| self.while_if_let_expr_ambiguity(lhs));
3383+
let rhs_ambigious = rhs.as_ref()
3384+
.and_then(|rhs| self.while_if_let_expr_ambiguity(rhs));
3385+
3386+
lhs_ambigious.or(rhs_ambigious)
3387+
}
3388+
_ => None,
3389+
}
3390+
}
3391+
33403392
// `move |args| expr`
33413393
fn parse_lambda_expr(&mut self,
33423394
attrs: ThinVec<Attribute>)
@@ -3437,6 +3489,7 @@ impl<'a> Parser<'a> {
34373489
let pats = self.parse_pats()?;
34383490
self.expect(&token::Eq)?;
34393491
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
3492+
self.while_if_let_ambiguity(&expr);
34403493
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
34413494
attrs.extend(iattrs);
34423495
let span = span_lo.to(body.span);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2015
12+
// compile-pass
13+
14+
// Enabling `ireffutable_let_patterns` isn't necessary for what this tests, but it makes coming up
15+
// with examples easier.
16+
#![feature(irrefutable_let_patterns)]
17+
18+
#[allow(irrefutable_let_patterns)]
19+
fn main() {
20+
use std::ops::Range;
21+
22+
if let Range { start: _, end: _ } = true..true && false { }
23+
//~^ WARN error in 2018
24+
25+
if let Range { start: _, end: _ } = true..true || false { }
26+
//~^ WARN error in 2018
27+
28+
while let Range { start: _, end: _ } = true..true && false { }
29+
//~^ WARN error in 2018
30+
31+
while let Range { start: _, end: _ } = true..true || false { }
32+
//~^ WARN error in 2018
33+
34+
if let true = false && false { }
35+
//~^ WARN error in 2018
36+
37+
while let true = (1 == 2) && false { }
38+
//~^ WARN error in 2018
39+
40+
// The following cases are not an error as parenthesis are used to
41+
// clarify intent:
42+
43+
if let Range { start: _, end: _ } = true..(true || false) { }
44+
45+
if let Range { start: _, end: _ } = true..(true && false) { }
46+
47+
while let Range { start: _, end: _ } = true..(true || false) { }
48+
49+
while let Range { start: _, end: _ } = true..(true && false) { }
50+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
warning: ambigious use of `&&`
2+
--> $DIR/syntax-ambiguity-2015.rs:22:47
3+
|
4+
LL | if let Range { start: _, end: _ } = true..true && false { }
5+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
6+
|
7+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
8+
9+
warning: ambigious use of `||`
10+
--> $DIR/syntax-ambiguity-2015.rs:25:47
11+
|
12+
LL | if let Range { start: _, end: _ } = true..true || false { }
13+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
14+
|
15+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
16+
17+
warning: ambigious use of `&&`
18+
--> $DIR/syntax-ambiguity-2015.rs:28:50
19+
|
20+
LL | while let Range { start: _, end: _ } = true..true && false { }
21+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
22+
|
23+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
24+
25+
warning: ambigious use of `||`
26+
--> $DIR/syntax-ambiguity-2015.rs:31:50
27+
|
28+
LL | while let Range { start: _, end: _ } = true..true || false { }
29+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
30+
|
31+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
32+
33+
warning: ambigious use of `&&`
34+
--> $DIR/syntax-ambiguity-2015.rs:34:19
35+
|
36+
LL | if let true = false && false { }
37+
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
38+
|
39+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
40+
41+
warning: ambigious use of `&&`
42+
--> $DIR/syntax-ambiguity-2015.rs:37:22
43+
|
44+
LL | while let true = (1 == 2) && false { }
45+
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
46+
|
47+
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
48+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// edition:2018
12+
13+
// Enabling `ireffutable_let_patterns` isn't necessary for what this tests, but it makes coming up
14+
// with examples easier.
15+
#![feature(irrefutable_let_patterns)]
16+
17+
#[allow(irrefutable_let_patterns)]
18+
fn main() {
19+
use std::ops::Range;
20+
21+
if let Range { start: _, end: _ } = true..true && false { }
22+
//~^ ERROR ambigious use of `&&`
23+
24+
if let Range { start: _, end: _ } = true..true || false { }
25+
//~^ ERROR ambigious use of `||`
26+
27+
while let Range { start: _, end: _ } = true..true && false { }
28+
//~^ ERROR ambigious use of `&&`
29+
30+
while let Range { start: _, end: _ } = true..true || false { }
31+
//~^ ERROR ambigious use of `||`
32+
33+
if let true = false && false { }
34+
//~^ ERROR ambigious use of `&&`
35+
36+
while let true = (1 == 2) && false { }
37+
//~^ ERROR ambigious use of `&&`
38+
39+
// The following cases are not an error as parenthesis are used to
40+
// clarify intent:
41+
42+
if let Range { start: _, end: _ } = true..(true || false) { }
43+
44+
if let Range { start: _, end: _ } = true..(true && false) { }
45+
46+
while let Range { start: _, end: _ } = true..(true || false) { }
47+
48+
while let Range { start: _, end: _ } = true..(true && false) { }
49+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
error: ambigious use of `&&`
2+
--> $DIR/syntax-ambiguity-2018.rs:21:47
3+
|
4+
LL | if let Range { start: _, end: _ } = true..true && false { }
5+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
6+
|
7+
= note: This will be a error until the `let_chains` feature is stabilized.
8+
9+
error: ambigious use of `||`
10+
--> $DIR/syntax-ambiguity-2018.rs:24:47
11+
|
12+
LL | if let Range { start: _, end: _ } = true..true || false { }
13+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
14+
|
15+
= note: This will be a error until the `let_chains` feature is stabilized.
16+
17+
error: ambigious use of `&&`
18+
--> $DIR/syntax-ambiguity-2018.rs:27:50
19+
|
20+
LL | while let Range { start: _, end: _ } = true..true && false { }
21+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
22+
|
23+
= note: This will be a error until the `let_chains` feature is stabilized.
24+
25+
error: ambigious use of `||`
26+
--> $DIR/syntax-ambiguity-2018.rs:30:50
27+
|
28+
LL | while let Range { start: _, end: _ } = true..true || false { }
29+
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
30+
|
31+
= note: This will be a error until the `let_chains` feature is stabilized.
32+
33+
error: ambigious use of `&&`
34+
--> $DIR/syntax-ambiguity-2018.rs:33:19
35+
|
36+
LL | if let true = false && false { }
37+
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
38+
|
39+
= note: This will be a error until the `let_chains` feature is stabilized.
40+
41+
error: ambigious use of `&&`
42+
--> $DIR/syntax-ambiguity-2018.rs:36:22
43+
|
44+
LL | while let true = (1 == 2) && false { }
45+
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
46+
|
47+
= note: This will be a error until the `let_chains` feature is stabilized.
48+
49+
error: aborting due to 6 previous errors
50+

0 commit comments

Comments
 (0)