Java: Make Assignment extend BinaryExpr.

java/ql/lib/semmle/code/java/Expr.qll

@@ -392,7 +392,7 @@ class ArrayInit extends Expr, @arrayinit {
  * element assignments since there the assignment destination is not directly
  * the array variable but instead an `ArrayAccess`.
  */
-class Assignment extends Expr, @assignment {
+class Assignment extends BinaryExpr, @assignment {
   /** Gets the destination (left-hand side) of the assignment. */
   Expr getDest() { result.isNthChildOf(this, 0) }
 
@@ -417,6 +417,8 @@ class Assignment extends Expr, @assignment {
  * For example, `x = 23`.
  */
 class AssignExpr extends Assignment, @assignexpr {
+  override string getOp() { result = "=" }
+
   override string getAPrimaryQlClass() { result = "AssignExpr" }
 }
 
@@ -445,7 +447,7 @@ class AssignOp extends Assignment, @assignop {
   override Expr getSource() { result.getParent() = this }
 
   /** Gets a string representation of the assignment operator of this compound assignment. */
-  /*abstract*/ string getOp() { result = "??=" }
+  /*abstract*/ override string getOp() { result = "??=" }
 
   /** Gets a printable representation of this expression. */
   override string toString() { result = "..." + this.getOp() + "..." }

java/ql/lib/semmle/code/java/PrettyPrintAst.qll

@@ -207,23 +207,6 @@ private class PpArrayInit extends PpAst, ArrayInit {
   override PpAst getChild(int i) { exists(int j | result = this.getInit(j) and i = 1 + 2 * j) }
 }
 
-private class PpAssignment extends PpAst, Assignment {
-  override string getPart(int i) {
-    i = 1 and
-    this instanceof AssignExpr and
-    result = " = "
-    or
-    i = 1 and
-    result = " " + this.(AssignOp).getOp() + " "
-  }
-
-  override PpAst getChild(int i) {
-    i = 0 and result = this.getDest()
-    or
-    i = 2 and result = this.getRhs()
-  }
-}
-
 private class PpLiteral extends PpAst, Literal {
   override string getPart(int i) { i = 0 and result = this.getLiteral() }
 }

java/ql/lib/semmle/code/java/arithmetic/Overflow.qll

@@ -93,8 +93,7 @@ class ArithExpr extends Expr {
     ) and
     forall(Expr e |
       e = this.(BinaryExpr).getAnOperand() or
-      e = this.(UnaryAssignExpr).getOperand() or
-      e = this.(AssignOp).getSource()
+      e = this.(UnaryAssignExpr).getOperand()
     |
       e.getType() instanceof NumType
     )
@@ -114,21 +113,17 @@ class ArithExpr extends Expr {
    */
   Expr getLeftOperand() {
     result = this.(BinaryExpr).getLeftOperand() or
-    result = this.(UnaryAssignExpr).getOperand() or
-    result = this.(AssignOp).getDest()
+    result = this.(UnaryAssignExpr).getOperand()
   }
 
   /**
    * Gets the right-hand operand if this is a binary expression.
    */
-  Expr getRightOperand() {
-    result = this.(BinaryExpr).getRightOperand() or result = this.(AssignOp).getRhs()
-  }
+  Expr getRightOperand() { result = this.(BinaryExpr).getRightOperand() }
 
   /** Gets an operand of this arithmetic expression. */
   Expr getAnOperand() {
     result = this.(BinaryExpr).getAnOperand() or
-    result = this.(UnaryAssignExpr).getOperand() or
-    result = this.(AssignOp).getSource()
+    result = this.(UnaryAssignExpr).getOperand()
   }
 }

java/ql/lib/semmle/code/java/controlflow/Guards.qll

@@ -179,13 +179,7 @@ private module GuardsInput implements SharedGuards::InputSig<Location, ControlFl
     }
   }
 
-  abstract private class BinExpr extends Expr {
-    Expr getAnOperand() {
-      result = this.(BinaryExpr).getAnOperand() or result = this.(AssignOp).getSource()
-    }
-  }
-
-  class AndExpr extends BinExpr {
+  class AndExpr extends BinaryExpr {
     AndExpr() {
       this instanceof AndBitwiseExpr or
       this instanceof AndLogicalExpr or
@@ -193,7 +187,7 @@ private module GuardsInput implements SharedGuards::InputSig<Location, ControlFl
     }
   }
 
-  class OrExpr extends BinExpr {
+  class OrExpr extends BinaryExpr {
     OrExpr() {
       this instanceof OrBitwiseExpr or
       this instanceof OrLogicalExpr or

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

@@ -53,8 +53,6 @@ private predicate unboxed(Expr e) {
       assign.getDest().getType() instanceof PrimitiveType and assign.getSource() = e
     )
     or
-    exists(AssignOp assign | assign.getSource() = e and assign.getType() instanceof PrimitiveType)
-    or
     exists(EqualityTest eq |
       eq.getAnOperand() = e and eq.getAnOperand().getType() instanceof PrimitiveType
     )
@@ -62,6 +60,7 @@ private predicate unboxed(Expr e) {
     exists(BinaryExpr bin |
       bin.getAnOperand() = e and
       not bin instanceof EqualityTest and
+      not bin instanceof AssignExpr and
       bin.getType() instanceof PrimitiveType
     )
     or

java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll

@@ -86,23 +86,7 @@ module Sem implements Semantic<Location> {
 
   class ConstantIntegerExpr = RU::ConstantIntegerExpr;
 
-  abstract class BinaryExpr extends Expr {
-    Expr getLeftOperand() {
-      result = this.(J::BinaryExpr).getLeftOperand() or result = this.(J::AssignOp).getDest()
-    }
-
-    Expr getRightOperand() {
-      result = this.(J::BinaryExpr).getRightOperand() or result = this.(J::AssignOp).getRhs()
-    }
-
-    final Expr getAnOperand() { result = this.getLeftOperand() or result = this.getRightOperand() }
-
-    final predicate hasOperands(Expr e1, Expr e2) {
-      this.getLeftOperand() = e1 and this.getRightOperand() = e2
-      or
-      this.getLeftOperand() = e2 and this.getRightOperand() = e1
-    }
-  }
+  class BinaryExpr = J::BinaryExpr;
 
   class AddExpr extends BinaryExpr {
     AddExpr() { this instanceof J::AddExpr or this instanceof J::AssignAddExpr }

java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll

@@ -161,13 +161,9 @@ module Private {
       this instanceof J::AssignUnsignedRightShiftExpr and result = TUnsignedRightShiftOp()
     }
 
-    Expr getLeftOperand() {
-      result = this.(J::BinaryExpr).getLeftOperand() or result = this.(J::AssignOp).getDest()
-    }
+    Expr getLeftOperand() { result = this.(J::BinaryExpr).getLeftOperand() }
 
-    Expr getRightOperand() {
-      result = this.(J::BinaryExpr).getRightOperand() or result = this.(J::AssignOp).getRhs()
-    }
+    Expr getRightOperand() { result = this.(J::BinaryExpr).getRightOperand() }
   }
 
   predicate ssaRead = RU::ssaRead/2;

java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll

@@ -73,7 +73,8 @@ module InsecureRandomnessConfig implements DataFlow::ConfigSig {
   predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
 
   predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
-    n1.asExpr() = n2.asExpr().(BinaryExpr).getAnOperand()
+    n1.asExpr() = n2.asExpr().(BinaryExpr).getAnOperand() and
+    not n2.asExpr() instanceof AssignExpr
     or
     n1.asExpr() = n2.asExpr().(UnaryExpr).getOperand()
     or

java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll

@@ -31,10 +31,7 @@ class RightShiftOp extends Expr {
     this instanceof AssignUnsignedRightShiftExpr
   }
 
-  private Expr getLhs() {
-    this.(BinaryExpr).getLeftOperand() = result or
-    this.(Assignment).getDest() = result
-  }
+  private Expr getLhs() { this.(BinaryExpr).getLeftOperand() = result }
 
   /**
    * Gets the variable that is shifted.

java/ql/lib/semmle/code/java/security/RandomQuery.qll

@@ -54,9 +54,8 @@ private module PredictableSeedFlowConfig implements DataFlow::ConfigSig {
 private module PredictableSeedFlow = DataFlow::Global<PredictableSeedFlowConfig>;
 
 private predicate predictableCalcStep(Expr e1, Expr e2) {
-  e2.(BinaryExpr).hasOperands(e1, any(PredictableSeedExpr p))
-  or
-  exists(AssignOp a | a = e2 | e1 = a.getDest() and a.getRhs() instanceof PredictableSeedExpr)
+  e2.(BinaryExpr).hasOperands(e1, any(PredictableSeedExpr p)) and
+  not e2 instanceof AssignExpr
   or
   exists(ConstructorCall cc, TypeNumber t | cc = e2 |
     cc.getArgument(0) = e1 and

java/ql/src/Likely Bugs/Arithmetic/OctalLiteral.ql

@@ -19,6 +19,7 @@ where
   lit.getLiteral() = val and
   val.regexpMatch("0[0-7][0-7]+") and
   lit.getParent() instanceof BinaryExpr and
+  not lit.getParent() instanceof Assignment and
   not lit.getParent() instanceof BitwiseExpr and
   not lit.getParent() instanceof ComparisonExpr
 select lit, "Integer literal starts with 0."

java/ql/src/Likely Bugs/Arithmetic/WhitespaceContradictsPrecedence.ql

@@ -145,6 +145,7 @@ int operatorWS(BinaryExpr expr) {
 /** Find nested binary expressions where the programmer may have made a precedence mistake. */
 predicate interestingNesting(BinaryExpr inner, BinaryExpr outer) {
   inner = outer.getAChildExpr() and
+  not outer instanceof Assignment and
   not inner instanceof AssocNestedExpr and
   not inner instanceof HarmlessNestedExpr and
   not inner.isParenthesized()

java/ql/src/Likely Bugs/Comparison/CompareIdenticalValues.ql

@@ -58,6 +58,7 @@ predicate equal(Expr left, Expr right) {
     sameVariable(left, right, _)
     or
     exists(BinaryExpr bLeft, BinaryExpr bRight | bLeft = left and bRight = right |
+      not bLeft instanceof Assignment and
       bLeft.getKind() = bRight.getKind() and
       equal(bLeft.getLeftOperand(), bRight.getLeftOperand()) and
       equal(bLeft.getRightOperand(), bRight.getRightOperand())

java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.ql

@@ -101,17 +101,10 @@ Expr overFlowCand() {
   |
     bin instanceof AddExpr or
     bin instanceof MulExpr or
-    bin instanceof LeftShiftExpr
-  )
-  or
-  exists(AssignOp op |
-    result = op and
-    positive(op.getDest()) and
-    positive(op.getRhs())
-  |
-    op instanceof AssignAddExpr or
-    op instanceof AssignMulExpr or
-    op instanceof AssignLeftShiftExpr
+    bin instanceof LeftShiftExpr or
+    bin instanceof AssignAddExpr or
+    bin instanceof AssignMulExpr or
+    bin instanceof AssignLeftShiftExpr
   )
   or
   exists(AddExpr add, CompileTimeConstantExpr c |

java/ql/src/Violations of Best Practice/legacy/AutoBoxing.ql

@@ -36,6 +36,7 @@ Variable flowTarget(Expr arg) {
  */
 predicate unboxed(BoxedExpr e) {
   exists(BinaryExpr bin | e = bin.getAnOperand() |
+    not bin instanceof Assignment and
     if bin instanceof EqualityTest or bin instanceof ComparisonExpr
     then bin.getAnOperand() instanceof PrimitiveExpr
     else bin instanceof PrimitiveExpr

java/ql/test-kotlin1/library-tests/exprs/binop.ql

@@ -41,4 +41,5 @@ MaybeElement rhs(BinaryExpr e) {
 }
 
 from Expr e
+where not e instanceof Assignment
 select e, lhs(e), rhs(e)

java/ql/test-kotlin2/library-tests/exprs/binop.ql

@@ -41,4 +41,5 @@ MaybeElement rhs(BinaryExpr e) {
 }
 
 from Expr e
+where not e instanceof Assignment
 select e, lhs(e), rhs(e)

java/ql/test/query-tests/StringComparison/StringComparison.expected

@@ -1,3 +1,3 @@
-| StringComparison.java:23:6:23:19 | ... == ... | String values compared with  == . |
-| StringComparison.java:26:6:26:16 | ... == ... | String values compared with  == . |
-| StringComparison.java:29:6:29:20 | ... == ... | String values compared with  == . |
+| StringComparison.java:23:6:23:19 | ... == ... | String values compared with ==. |
+| StringComparison.java:26:6:26:16 | ... == ... | String values compared with ==. |
+| StringComparison.java:29:6:29:20 | ... == ... | String values compared with ==. |