Java: Make Assignment extend BinaryExpr.

Author: aschackmull

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,21 +179,15 @@ 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
       this instanceof AssignAndExpr
     }
   }
 
-  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,15 +53,14 @@ 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
     )
     or
     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