From 1cc69c484eadec5e54f178a9fcfaff21b5d25cf4 Mon Sep 17 00:00:00 2001
From: Daniel Grunwald <daniel@danielgrunwald.de>
Date: Thu, 8 Jan 2015 01:44:01 +0100
Subject: [PATCH] RFC 558: Require parentheses for chained comparisons

Fixes #20724.
---
 src/libsyntax/ast_util.rs                     | 10 ++++++--
 src/libsyntax/parse/parser.rs                 | 24 ++++++++++++++++++-
 .../require-parens-for-chained-comparison.rs  | 23 ++++++++++++++++++
 src/test/compile-fail/unsized2.rs             |  5 +++-
 4 files changed, 58 insertions(+), 4 deletions(-)
 create mode 100644 src/test/compile-fail/require-parens-for-chained-comparison.rs

diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs
index 871f1237aee..836deeffde1 100644
--- a/src/libsyntax/ast_util.rs
+++ b/src/libsyntax/ast_util.rs
@@ -85,6 +85,13 @@ pub fn is_shift_binop(b: BinOp) -> bool {
     }
 }
 
+pub fn is_comparison_binop(b: BinOp) -> bool {
+    match b {
+        BiEq | BiLt | BiLe | BiNe | BiGt | BiGe => true,
+        _ => false
+    }
+}
+
 /// Returns `true` if the binary operator takes its arguments by value
 pub fn is_by_value_binop(b: BinOp) -> bool {
     match b {
@@ -317,8 +324,7 @@ pub fn operator_prec(op: ast::BinOp) -> uint {
       BiBitAnd                  =>  8u,
       BiBitXor                  =>  7u,
       BiBitOr                   =>  6u,
-      BiLt | BiLe | BiGe | BiGt =>  4u,
-      BiEq | BiNe               =>  3u,
+      BiLt | BiLe | BiGe | BiGt | BiEq | BiNe => 3u,
       BiAnd                     =>  2u,
       BiOr                      =>  1u
   }
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 92e0395eca4..6624f3cd9a8 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -16,7 +16,7 @@
 use ast::{RegionTyParamBound, TraitTyParamBound, TraitBoundModifier};
 use ast::{ProvidedMethod, Public, Unsafety};
 use ast::{Mod, BiAdd, Arg, Arm, Attribute, BindByRef, BindByValue};
-use ast::{BiBitAnd, BiBitOr, BiBitXor, BiRem, Block};
+use ast::{BiBitAnd, BiBitOr, BiBitXor, BiRem, BiLt, BiGt, Block};
 use ast::{BlockCheckMode, CaptureByRef, CaptureByValue, CaptureClause};
 use ast::{Crate, CrateConfig, Decl, DeclItem};
 use ast::{DeclLocal, DefaultBlock, UnDeref, BiDiv, EMPTY_CTXT, EnumDef, ExplicitSelf};
@@ -2906,6 +2906,9 @@ pub fn parse_more_binops(&mut self, lhs: P<Expr>, min_prec: uint) -> P<Expr> {
         let cur_opt = self.token.to_binop();
         match cur_opt {
             Some(cur_op) => {
+                if ast_util::is_comparison_binop(cur_op) {
+                    self.check_no_chained_comparison(&*lhs, cur_op)
+                }
                 let cur_prec = operator_prec(cur_op);
                 if cur_prec > min_prec {
                     self.bump();
@@ -2934,6 +2937,25 @@ pub fn parse_more_binops(&mut self, lhs: P<Expr>, min_prec: uint) -> P<Expr> {
         }
     }
 
+    /// Produce an error if comparison operators are chained (RFC #558).
+    /// We only need to check lhs, not rhs, because all comparison ops
+    /// have same precedence and are left-associative
+    fn check_no_chained_comparison(&mut self, lhs: &Expr, outer_op: ast::BinOp) {
+        debug_assert!(ast_util::is_comparison_binop(outer_op));
+        match lhs.node {
+            ExprBinary(op, _, _) if ast_util::is_comparison_binop(op) => {
+                let op_span = self.span;
+                self.span_err(op_span,
+                    "Chained comparison operators require parentheses");
+                if op == BiLt && outer_op == BiGt {
+                    self.span_help(op_span,
+                        "Use ::< instead of < if you meant to specify type arguments.");
+                }
+            }
+            _ => {}
+        }
+    }
+
     /// Parse an assignment expression....
     /// actually, this seems to be the main entry point for
     /// parsing an arbitrary expression.
diff --git a/src/test/compile-fail/require-parens-for-chained-comparison.rs b/src/test/compile-fail/require-parens-for-chained-comparison.rs
new file mode 100644
index 00000000000..7513815ad73
--- /dev/null
+++ b/src/test/compile-fail/require-parens-for-chained-comparison.rs
@@ -0,0 +1,23 @@
+// Copyright 2015 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.
+
+fn f<T>() {}
+
+fn main() {
+    false == false == false;
+    //~^ ERROR: Chained comparison operators require parentheses
+
+    false == 0 < 2;
+    //~^ ERROR: Chained comparison operators require parentheses
+
+    f<X>();
+    //~^ ERROR: Chained comparison operators require parentheses
+    //~^^ HELP: Use ::< instead of < if you meant to specify type arguments.
+}
diff --git a/src/test/compile-fail/unsized2.rs b/src/test/compile-fail/unsized2.rs
index c5f9e8d5991..604f7ba3255 100644
--- a/src/test/compile-fail/unsized2.rs
+++ b/src/test/compile-fail/unsized2.rs
@@ -13,5 +13,8 @@
 fn f<X>() {}
 
 pub fn main() {
-    f<type>(); //~ ERROR expected identifier, found keyword `type`
+    f<type>();
+    //~^ ERROR expected identifier, found keyword `type`
+    //~^^ ERROR: Chained comparison operators require parentheses
+    //~^^^ HELP: Use ::< instead of < if you meant to specify type arguments.
 }