Skip to content

Commit

Permalink
Added warning/error for if-let-chain ambiguity.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidtwco committed Aug 31, 2018
1 parent 685fb54 commit 2ce56c5
Show file tree
Hide file tree
Showing 5 changed files with 250 additions and 0 deletions.
53 changes: 53 additions & 0 deletions src/libsyntax/parse/parser.rs
Expand Up @@ -3327,6 +3327,8 @@ impl<'a> Parser<'a> {
let pats = self.parse_pats()?;
self.expect(&token::Eq)?;
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
self.while_if_let_ambiguity(&expr);

let thn = self.parse_block()?;
let (hi, els) = if self.eat_keyword(keywords::Else) {
let expr = self.parse_else_expr()?;
Expand All @@ -3337,6 +3339,56 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(lo.to(hi), ExprKind::IfLet(pats, expr, thn, els), attrs))
}

/// With eRFC 2497, we need to check whether an expression is ambigious and warn or error
/// depending on the edition, this function handles that.
fn while_if_let_ambiguity(&self, expr: &P<Expr>) {
if let Some((span, op_kind)) = self.while_if_let_expr_ambiguity(&expr) {
let message = format!("ambigious use of `{}`", op_kind.to_string());
let mut err = if self.span.edition() >= Edition::Edition2018 {
self.diagnostic().struct_span_err(span, &message)
} else {
self.diagnostic().struct_span_warn(span, &message)
};

let note = if self.span.edition() >= Edition::Edition2018 {
"This will be a error until the `let_chains` feature is stabilized."
} else {
"This will be a error in Rust 2018 until the `let_chains` feature is stabilized."
};
err.note(note);

if let Ok(snippet) = self.sess.source_map().span_to_snippet(span) {
err.span_suggestion(
span, "consider adding parenthesis", format!("({})", snippet),
);
}

err.emit();
}
}

/// With eRFC 2497 adding if-let chains, there is a requirement that the parsing of
/// `&&` and `||` in a if-let statement be unambigious. This function returns a span and
/// a `BinOpKind` (either `&&` or `||` depending on what was ambigious) if it is determined
/// that the current expression parsed is ambigious and will break in future.
fn while_if_let_expr_ambiguity(&self, expr: &P<Expr>) -> Option<(Span, BinOpKind)> {
debug!("while_if_let_expr_ambiguity: expr.node: {:?}", expr.node);
match &expr.node {
ExprKind::Binary(op, _, _) if op.node == BinOpKind::And || op.node == BinOpKind::Or => {
Some((expr.span, op.node))
},
ExprKind::Range(ref lhs, ref rhs, _) => {
let lhs_ambigious = lhs.as_ref()
.and_then(|lhs| self.while_if_let_expr_ambiguity(lhs));
let rhs_ambigious = rhs.as_ref()
.and_then(|rhs| self.while_if_let_expr_ambiguity(rhs));

lhs_ambigious.or(rhs_ambigious)
}
_ => None,
}
}

// `move |args| expr`
fn parse_lambda_expr(&mut self,
attrs: ThinVec<Attribute>)
Expand Down Expand Up @@ -3437,6 +3489,7 @@ impl<'a> Parser<'a> {
let pats = self.parse_pats()?;
self.expect(&token::Eq)?;
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
self.while_if_let_ambiguity(&expr);
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
attrs.extend(iattrs);
let span = span_lo.to(body.span);
Expand Down
50 changes: 50 additions & 0 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2015.rs
@@ -0,0 +1,50 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2015
// compile-pass

// Enabling `ireffutable_let_patterns` isn't necessary for what this tests, but it makes coming up
// with examples easier.
#![feature(irrefutable_let_patterns)]

#[allow(irrefutable_let_patterns)]
fn main() {
use std::ops::Range;

if let Range { start: _, end: _ } = true..true && false { }
//~^ WARN error in 2018

if let Range { start: _, end: _ } = true..true || false { }
//~^ WARN error in 2018

while let Range { start: _, end: _ } = true..true && false { }
//~^ WARN error in 2018

while let Range { start: _, end: _ } = true..true || false { }
//~^ WARN error in 2018

if let true = false && false { }
//~^ WARN error in 2018

while let true = (1 == 2) && false { }
//~^ WARN error in 2018

// The following cases are not an error as parenthesis are used to
// clarify intent:

if let Range { start: _, end: _ } = true..(true || false) { }

if let Range { start: _, end: _ } = true..(true && false) { }

while let Range { start: _, end: _ } = true..(true || false) { }

while let Range { start: _, end: _ } = true..(true && false) { }
}
48 changes: 48 additions & 0 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2015.stderr
@@ -0,0 +1,48 @@
warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:22:47
|
LL | if let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

warning: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:25:47
|
LL | if let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:28:50
|
LL | while let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

warning: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:31:50
|
LL | while let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:34:19
|
LL | if let true = false && false { }
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:37:22
|
LL | while let true = (1 == 2) && false { }
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.

49 changes: 49 additions & 0 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2018.rs
@@ -0,0 +1,49 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// edition:2018

// Enabling `ireffutable_let_patterns` isn't necessary for what this tests, but it makes coming up
// with examples easier.
#![feature(irrefutable_let_patterns)]

#[allow(irrefutable_let_patterns)]
fn main() {
use std::ops::Range;

if let Range { start: _, end: _ } = true..true && false { }
//~^ ERROR ambigious use of `&&`

if let Range { start: _, end: _ } = true..true || false { }
//~^ ERROR ambigious use of `||`

while let Range { start: _, end: _ } = true..true && false { }
//~^ ERROR ambigious use of `&&`

while let Range { start: _, end: _ } = true..true || false { }
//~^ ERROR ambigious use of `||`

if let true = false && false { }
//~^ ERROR ambigious use of `&&`

while let true = (1 == 2) && false { }
//~^ ERROR ambigious use of `&&`

// The following cases are not an error as parenthesis are used to
// clarify intent:

if let Range { start: _, end: _ } = true..(true || false) { }

if let Range { start: _, end: _ } = true..(true && false) { }

while let Range { start: _, end: _ } = true..(true || false) { }

while let Range { start: _, end: _ } = true..(true && false) { }
}
50 changes: 50 additions & 0 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2018.stderr
@@ -0,0 +1,50 @@
error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:21:47
|
LL | if let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2018.rs:24:47
|
LL | if let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:27:50
|
LL | while let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2018.rs:30:50
|
LL | while let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:33:19
|
LL | if let true = false && false { }
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:36:22
|
LL | while let true = (1 == 2) && false { }
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.

error: aborting due to 6 previous errors

0 comments on commit 2ce56c5

Please sign in to comment.