Make EqualityExpression same as ShiftExpr

pmd-java/etc/grammar/Java.jjt

@@ -2297,10 +2297,23 @@ void AndExpression() #AndExpression(>1):
   EqualityExpression() ( LOOKAHEAD(2) "&" EqualityExpression() )*
 }
 
-void EqualityExpression() #EqualityExpression(>1):
+// same as ShiftExpression
+void EqualityExpression() #void:
 {}
 {
-  InstanceOfExpression()  ( LOOKAHEAD(2) ( "==" {jjtThis.setImage("==");} | "!=" {jjtThis.setImage("!=");} ) InstanceOfExpression()  )*
+  InstanceOfExpression()  (EqualityExprTail())*
+}
+
+
+void EqualityExprTail() #EqualityExpression:
+{}
+{
+    ( "==" | "!=" )
+    {
+      jjtThis.setImage(getToken(0).getImage());
+      jjtree.extendLeft();
+    }
+    InstanceOfExpression()
 }
 
 void InstanceOfExpression() #InstanceOfExpression(>1):
@@ -2318,6 +2331,11 @@ void RelationalExpression() #RelationalExpression(>1):
     { jjtThis.setImage(getToken(0).getImage()); }
     ShiftExpression()
   )?
+  // There cannot be more than one, because it wouldn't compile
+  // From the JLS:
+  // For example, a<b<c parses as (a<b)<c, which is always a compile-time
+  // error, because the type of a<b is always boolean and < is not an operator
+  // on boolean values.
 }
 
 /**
@@ -2489,7 +2507,7 @@ void PrimaryPrefix() #void :
 | AmbiguousName()
 }
 
-/*
+/**
  * Productions that may be present after a PrimaryPrefix. The way this is written makes for a nice tree,
  * but also allows many invalid things to be written (which is fine because we parse compilable code).
  * E.g. this allows `foo().this.super::bar[0].this`

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTEqualityExpression.java

@@ -10,17 +10,28 @@ package net.sourceforge.pmd.lang.java.ast;
  * This has a precedence greater than {@link ASTAndExpression},
  * and lower than {@link ASTInstanceOfExpression} and {@link ASTRelationalExpression}.
  *
+ * <pre class="grammar">
+ *
+ * EqualityExpression ::=  {@linkplain ASTEqualityExpression EqualityExpression}  ( ( "==" | "!=" ) {@linkplain ASTInstanceOfExpression InstanceOfExpression}  )+
+ *
+ * </pre>
+ *
  * <p>Note that the children of this node are not necessarily {@link ASTInstanceOfExpression},
  * rather, they are expressions with an operator precedence greater or equal to InstanceOfExpression.
  *
- *
- * <pre class="grammar">
- *
- * EqualityExpression ::=  {@linkplain ASTInstanceOfExpression InstanceOfExpression}  ( ( "==" | "!=" ) {@linkplain ASTInstanceOfExpression InstanceOfExpression}  )+
- *
- * </pre>
+ * <p>The first child may be another EqualityExpression only
+ * if its operator is different. For example, if parentheses represent
+ * nesting:
+ * <table summary="Nesting examples">
+ * <tr><th></th><th>Parses as</th></tr>
+ *     <tr><td>{@code 1 == 2 == 3}</td><td>{@code (1 == 2 == 3)}</td></tr>
+ *     <tr><td>{@code 1 == 2 != 3}</td><td>{@code ((1 == 2) != 3)}</td></tr>
+ *     <tr><td>{@code 1 == 2 != 3 + 4}</td><td>{@code ((1 == 2) != (3 + 4))}</td></tr>
+ *     <tr><td>{@code 1 == 2 != 3 != 4}</td><td>{@code ((1 == 2) != 3 != 4)}</td></tr>
+ *     <tr><td>{@code 1 == 2 != 3 != 4 == 5}</td><td>{@code (((1 == 2) != 3 != 4) == 5)}</td></tr>
+ * </table>
  */
-public class ASTEqualityExpression extends AbstractJavaTypeNode implements ASTExpression {
+public class ASTEqualityExpression extends AbstractLrBinaryExpr implements ASTExpression {
     public ASTEqualityExpression(int id) {
         super(id);
     }
@@ -40,12 +51,4 @@ public class ASTEqualityExpression extends AbstractJavaTypeNode implements ASTEx
         visitor.visit(this, data);
     }
 
-
-    /**
-     * Returns the image of the operator, i.e. "==" or "!=".
-     */
-    public String getOperator() {
-        return getImage();
-    }
-
 }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractLrBinaryExpr.java

@@ -7,8 +7,7 @@ package net.sourceforge.pmd.lang.java.ast;
 import java.util.Iterator;
 
 /**
- * Base class for {@link ASTAdditiveExpression}, {@link ASTMultiplicativeExpression},
- * and {@link ASTShiftExpression}, which use the same parsing scheme.
+ * Base class for some expressions that are parsed left-recursively.
  *
  * @author Clément Fournier
  */

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTEqualityExpressionTest.kt

@@ -0,0 +1,138 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.ast
+
+import net.sourceforge.pmd.lang.ast.test.shouldBe
+import net.sourceforge.pmd.lang.java.ast.BinaryOp.*
+
+
+class ASTEqualityExpressionTest : ParserTestSpec({
+
+    parserTest("Simple equality expression should be flat") {
+
+        "1 == 2 == 3" should matchExpr<ASTEqualityExpression> {
+            it::getOp shouldBe EQ
+            it::getOperator shouldBe "=="
+
+
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 1
+            }
+
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 2
+            }
+
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 3
+            }
+        }
+
+
+        "1 != 2 != 3 * 5" should matchExpr<ASTEqualityExpression> {
+            it::getOp shouldBe NE
+            it::getOperator shouldBe "!="
+
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 1
+            }
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 2
+            }
+            child<ASTMultiplicativeExpression> {
+                it::getOperator shouldBe "*"
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 3
+                }
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 5
+                }
+            }
+        }
+    }
+
+    parserTest("Changing operators should push a new node") {
+
+        "1 == 2 != 3" should matchExpr<ASTEqualityExpression> {
+            it::getOp shouldBe NE
+
+            child<ASTEqualityExpression> {
+                it::getOp shouldBe EQ
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 1
+                }
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 2
+                }
+            }
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 3
+            }
+        }
+
+        "1 == 4 == 2 != 3" should matchExpr<ASTEqualityExpression> {
+            it::getOp shouldBe NE
+
+            child<ASTEqualityExpression> {
+                it::getOp shouldBe EQ
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 1
+                }
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 4
+                }
+
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 2
+                }
+            }
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 3
+            }
+        }
+
+        // ((((1 + 4 + 2) - 3) + 4) - 1)
+        "1 == 4 == 2 != 3 == 4 != 1" should matchExpr<ASTEqualityExpression> {
+            it::getOp shouldBe NE
+
+            child<ASTEqualityExpression> {
+                it::getOp shouldBe EQ
+
+                child<ASTEqualityExpression> {
+                    it::getOp shouldBe NE
+
+                    child<ASTEqualityExpression> {
+                        it::getOp shouldBe EQ
+
+                        child<ASTNumericLiteral> {
+                            it::getValueAsInt shouldBe 1
+                        }
+                        child<ASTNumericLiteral> {
+                            it::getValueAsInt shouldBe 4
+                        }
+                        child<ASTNumericLiteral> {
+                            it::getValueAsInt shouldBe 2
+                        }
+                    }
+                    child<ASTNumericLiteral> {
+                        it::getValueAsInt shouldBe 3
+                    }
+                }
+                child<ASTNumericLiteral> {
+                    it::getValueAsInt shouldBe 4
+                }
+            }
+            child<ASTNumericLiteral> {
+                it::getValueAsInt shouldBe 1
+            }
+        }
+    }
+
+})