Make equality, comparison and 'in' operators not associative.

I'm not using a --incompatible-change flag because it's not available in the
parser. We could pass it, but I think this is trivial to fix and unlikely to
happen in real code (if it does, there was most likely a bug).

RELNOTES[INC]:
  Operators for equality, comparison, 'in' and 'not in' are no longer associative,
  e.g.  x < y < z  is now a syntax error. Before, it was parsed as:  (x < y) < z.

PiperOrigin-RevId: 159422042

2 files changed, 41 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index ed23432bb4..46de9a9933 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -950,6 +950,7 @@ public class Parser {
     Expression expr = parseNonTupleExpression(prec + 1);
     // The loop is not strictly needed, but it prevents risks of stack overflow. Depth is
     // limited to number of different precedence levels (operatorPrecedence.size()).
+    Operator lastOp = null;
     for (;;) {
 
       if (token.kind == TokenKind.NOT) {
@@ -967,10 +968,21 @@ public class Parser {
       if (!operatorPrecedence.get(prec).contains(operator)) {
         return expr;
       }
+
+      // Operator '==' and other operators of the same precedence (e.g. '<', 'in')
+      // are not associative.
+      if (lastOp != null && operatorPrecedence.get(prec).contains(Operator.EQUALS_EQUALS)) {
+        reportError(
+            lexer.createLocation(token.left, token.right),
+            String.format("Operator '%s' is not associative with operator '%s'. Use parens.",
+                lastOp, operator));
+      }
+
       nextToken();
       Expression secondary = parseNonTupleExpression(prec + 1);
       expr = optimizeBinOpExpression(operator, expr, secondary);
       setLocation(expr, start, secondary);
+      lastOp = operator;
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index b5eedb2e90..20a70843f8 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -120,6 +120,35 @@ public class ParserTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testNonAssociativeOperators() throws Exception {
+    setFailFast(false);
+
+    parseExpression("0 < 2 < 4");
+    assertContainsError("Operator '<' is not associative with operator '<'");
+    clearEvents();
+
+    parseExpression("0 == 2 < 4");
+    assertContainsError("Operator '==' is not associative with operator '<'");
+    clearEvents();
+
+    parseExpression("1 in [1, 2] == True");
+    assertContainsError("Operator 'in' is not associative with operator '=='");
+    clearEvents();
+
+    parseExpression("1 >= 2 <= 3");
+    assertContainsError("Operator '>=' is not associative with operator '<='");
+    clearEvents();
+  }
+
+  @Test
+  public void testNonAssociativeOperatorsWithParens() throws Exception {
+    parseExpression("(0 < 2) < 4");
+    parseExpression("(0 == 2) < 4");
+    parseExpression("(1 in [1, 2]) == True");
+    parseExpression("1 >= (2 <= 3)");
+  }
+
+  @Test
   public void testUnaryMinusExpr() throws Exception {
     FuncallExpression e = (FuncallExpression) parseExpression("-5");
     FuncallExpression e2 = (FuncallExpression) parseExpression("- 5");