Merge pull request #2719 from aschackmull/java/deprecate-parexpr

Java: Deprecate ParExpr

Author: yo-h

change-notes/1.24/analysis-java.md

@@ -36,3 +36,6 @@ The following changes in version 1.24 affect Java analysis in all applications.
   general file classification mechanism and thus suppression of alerts, and
   also any security queries using taint tracking, as test classes act as
   default barriers stopping taint flow.
+* Parentheses are now no longer modelled directly in the AST, that is, the
+  `ParExpr` class is empty. Instead, a parenthesized expression can be
+  identified with the `Expr.isParenthesized()` member predicate.

java/ql/src/Language Abuse/UselessUpcast.ql

@@ -15,7 +15,7 @@ import java
 predicate usefulUpcast(CastExpr e) {
   // Upcasts that may be performed to affect resolution of methods or constructors.
   exists(Call c, int i, Callable target |
-    c.getArgument(i).getProperExpr() = e and
+    c.getArgument(i) = e and
     target = c.getCallee() and
     // An upcast to the type of the corresponding parameter.
     e.getType() = target.getParameterType(i)
@@ -31,27 +31,25 @@ predicate usefulUpcast(CastExpr e) {
   )
   or
   // Upcasts of a varargs argument.
-  exists(Call c, int iArg, int iParam | c.getArgument(iArg).getProperExpr() = e |
+  exists(Call c, int iArg, int iParam | c.getArgument(iArg) = e |
     c.getCallee().getParameter(iParam).isVarargs() and iArg >= iParam
   )
   or
   // Upcasts that are performed on an operand of a ternary expression.
-  exists(ConditionalExpr ce |
-    e = ce.getTrueExpr().getProperExpr() or e = ce.getFalseExpr().getProperExpr()
-  )
+  exists(ConditionalExpr ce | e = ce.getTrueExpr() or e = ce.getFalseExpr())
   or
   // Upcasts to raw types.
   e.getType() instanceof RawType
   or
   e.getType().(Array).getElementType() instanceof RawType
   or
   // Upcasts that are performed to affect field, private method, or static method resolution.
-  exists(FieldAccess fa | e = fa.getQualifier().getProperExpr() |
+  exists(FieldAccess fa | e = fa.getQualifier() |
     not e.getExpr().getType().(RefType).inherits(fa.getField())
   )
   or
   exists(MethodAccess ma, Method m |
-    e = ma.getQualifier().getProperExpr() and
+    e = ma.getQualifier() and
     m = ma.getMethod() and
     (m.isStatic() or m.isPrivate())
   |

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

@@ -15,7 +15,7 @@ import java
 import semmle.code.java.Collections
 
 predicate isDefinitelyPositive(Expr e) {
-  isDefinitelyPositive(e.getProperExpr()) or
+  isDefinitelyPositive(e) or
   e.(IntegerLiteral).getIntValue() >= 0 or
   e.(MethodAccess).getMethod() instanceof CollectionSizeMethod or
   e.(MethodAccess).getMethod() instanceof StringLengthMethod or
@@ -24,10 +24,10 @@ predicate isDefinitelyPositive(Expr e) {
 
 from BinaryExpr t, RemExpr lhs, IntegerLiteral rhs, string parity
 where
-  t.getLeftOperand().getProperExpr() = lhs and
-  t.getRightOperand().getProperExpr() = rhs and
+  t.getLeftOperand() = lhs and
+  t.getRightOperand() = rhs and
   not isDefinitelyPositive(lhs.getLeftOperand()) and
-  lhs.getRightOperand().getProperExpr().(IntegerLiteral).getIntValue() = 2 and
+  lhs.getRightOperand().(IntegerLiteral).getIntValue() = 2 and
   (
     t instanceof EQExpr and rhs.getIntValue() = 1 and parity = "oddness"
     or

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

@@ -17,9 +17,6 @@ predicate isConstantExp(Expr e) {
   // A literal is constant.
   e instanceof Literal
   or
-  // A parenthesized expression is constant if its proper expression is.
-  isConstantExp(e.(ParExpr).getProperExpr())
-  or
   e instanceof TypeAccess
   or
   e instanceof ArrayTypeAccess
@@ -33,26 +30,22 @@ predicate isConstantExp(Expr e) {
   )
   or
   // A cast expression is constant if its expression is.
-  exists(CastExpr c | c = e | isConstantExp(c.getExpr().getProperExpr()))
+  exists(CastExpr c | c = e | isConstantExp(c.getExpr()))
   or
   // Multiplication by 0 is constant.
-  exists(MulExpr m | m = e | eval(m.getAnOperand().getProperExpr()) = 0)
+  exists(MulExpr m | m = e | eval(m.getAnOperand()) = 0)
   or
   // Integer remainder by 1 is constant.
   exists(RemExpr r | r = e |
     r.getLeftOperand().getType() instanceof IntegralType and
-    eval(r.getRightOperand().getProperExpr()) = 1
+    eval(r.getRightOperand()) = 1
   )
   or
-  exists(AndBitwiseExpr a | a = e | eval(a.getAnOperand().getProperExpr()) = 0)
+  exists(AndBitwiseExpr a | a = e | eval(a.getAnOperand()) = 0)
   or
-  exists(AndLogicalExpr a | a = e |
-    a.getAnOperand().getProperExpr().(BooleanLiteral).getBooleanValue() = false
-  )
+  exists(AndLogicalExpr a | a = e | a.getAnOperand().(BooleanLiteral).getBooleanValue() = false)
   or
-  exists(OrLogicalExpr o | o = e |
-    o.getAnOperand().getProperExpr().(BooleanLiteral).getBooleanValue() = true
-  )
+  exists(OrLogicalExpr o | o = e | o.getAnOperand().(BooleanLiteral).getBooleanValue() = true)
 }
 
 from Expr e

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

@@ -35,8 +35,8 @@ float exprBound(Expr e) {
 /** A multiplication that does not overflow. */
 predicate small(MulExpr e) {
   exists(NumType t, float lhs, float rhs, float res | t = e.getType() |
-    lhs = exprBound(e.getLeftOperand().getProperExpr()) and
-    rhs = exprBound(e.getRightOperand().getProperExpr()) and
+    lhs = exprBound(e.getLeftOperand()) and
+    rhs = exprBound(e.getRightOperand()) and
     lhs * rhs = res and
     res <= t.getOrdPrimitiveType().getMaxValue()
   )
@@ -47,7 +47,7 @@ predicate small(MulExpr e) {
  */
 Expr getRestrictedParent(Expr e) {
   result = e.getParent() and
-  (result instanceof ArithExpr or result instanceof ConditionalExpr or result instanceof ParExpr)
+  (result instanceof ArithExpr or result instanceof ConditionalExpr)
 }
 
 from ConversionSite c, MulExpr e, NumType sourceType, NumType destType

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

@@ -32,11 +32,11 @@ predicate checksReferenceEquality(EqualsMethod em) {
     eq.getAnOperand().(VarAccess).getVariable() = em.getParameter(0) and
     (
       // `{ return (ojb==this); }`
-      eq = blk.getStmt().(ReturnStmt).getResult().getProperExpr()
+      eq = blk.getStmt().(ReturnStmt).getResult()
       or
       // `{ if (ojb==this) return true; else return false; }`
       exists(IfStmt ifStmt | ifStmt = blk.getStmt() |
-        eq = ifStmt.getCondition().getProperExpr() and
+        eq = ifStmt.getCondition() and
         ifStmt.getThen().(ReturnStmt).getResult().(BooleanLiteral).getBooleanValue() = true and
         ifStmt.getElse().(ReturnStmt).getResult().(BooleanLiteral).getBooleanValue() = false
       )

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

@@ -45,7 +45,7 @@ class ReferenceEquals extends EqualsMethod {
     exists(Block b, ReturnStmt ret, EQExpr eq |
       this.getBody() = b and
       b.getStmt(0) = ret and
-      ret.getResult().getProperExpr() = eq and
+      ret.getResult() = eq and
       eq.getAnOperand() = this.getAParameter().getAnAccess() and
       (eq.getAnOperand() instanceof ThisAccess or eq.getAnOperand() instanceof FieldAccess)
     )

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

@@ -26,7 +26,7 @@ class BooleanExpr extends Expr {
 }
 
 private predicate assignAndCheck(AssignExpr e) {
-  exists(BinaryExpr c | e = c.getAChildExpr().getProperExpr() |
+  exists(BinaryExpr c | e = c.getAChildExpr() |
     c instanceof ComparisonExpr or
     c instanceof EqualityTest
   )

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

@@ -24,9 +24,6 @@ class StringValue extends Expr {
       this.(MethodAccess).getMethod() = intern
     )
     or
-    // Parenthesized expressions.
-    this.(ParExpr).getExpr().(StringValue).isInterned()
-    or
     // Ternary conditional operator.
     this.(ConditionalExpr).getTrueExpr().(StringValue).isInterned() and
     this.(ConditionalExpr).getFalseExpr().(StringValue).isInterned()
@@ -52,7 +49,7 @@ predicate variableValuesInterned(Variable v) {
   not v instanceof Parameter and
   // If the string is modified with `+=`, then the new string is not interned
   // even if the components are.
-  not exists(AssignOp append | append.getDest().getProperExpr() = v.getAnAccess())
+  not exists(AssignOp append | append.getDest() = v.getAnAccess())
 }
 
 from EqualityTest e, StringValue lhs, StringValue rhs

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

@@ -127,8 +127,6 @@ Expr overFlowCand() {
   or
   exists(SsaExplicitUpdate x | result = x.getAUse() and x.getDefiningExpr() = overFlowCand())
   or
-  result.(ParExpr).getExpr() = overFlowCand()
-  or
   result.(AssignExpr).getRhs() = overFlowCand()
   or
   result.(LocalVariableDeclExpr).getInit() = overFlowCand()
@@ -165,8 +163,6 @@ Expr increaseOrDecreaseOfVar(SsaVariable v) {
     result = x.getAUse() and x.getDefiningExpr() = increaseOrDecreaseOfVar(v)
   )
   or
-  result.(ParExpr).getExpr() = increaseOrDecreaseOfVar(v)
-  or
   result.(AssignExpr).getRhs() = increaseOrDecreaseOfVar(v)
   or
   result.(LocalVariableDeclExpr).getInit() = increaseOrDecreaseOfVar(v)