Merge pull request #2659 from aschackmull/java/remove-parexpr

yo-h · web-flow · commit 95d138bf002b · 2020-01-28T09:50:06.000-05:00
Java: Update dbscheme for ParExpr removal.
diff --git a/java/ql/src/Complexity/ComplexCondition.ql b/java/ql/src/Complexity/ComplexCondition.ql
@@ -13,7 +13,10 @@ import java
 
 predicate nontrivialLogicalOperator(BinaryExpr e) {
   e instanceof LogicExpr and
-  not e.getParent().(Expr).getKind() = e.getKind()
+  (
+    not e.getParent().(Expr).getKind() = e.getKind() or
+    e.isParenthesized()
+  )
 }
 
 Expr getSimpleParent(Expr e) {
diff --git a/java/ql/src/Likely Bugs/Arithmetic/MultiplyRemainder.ql b/java/ql/src/Likely Bugs/Arithmetic/MultiplyRemainder.ql
@@ -12,8 +12,9 @@
 
 import java
 
-from MulExpr e
+from MulExpr e, RemExpr lhs
 where
-  e.getLeftOperand() instanceof RemExpr and
+  e.getLeftOperand() = lhs and
+  not lhs.isParenthesized() and
   e.getRightOperand().getType().hasName("int")
 select e, "Result of a remainder operation multiplied by an integer."
diff --git a/java/ql/src/Likely Bugs/Arithmetic/WhitespaceContradictsPrecedence.ql b/java/ql/src/Likely Bugs/Arithmetic/WhitespaceContradictsPrecedence.ql
@@ -120,20 +120,33 @@ predicate endOfBinaryLhs(BinaryExpr expr, int line, int col) {
   )
 }
 
+/** Compute the number of parenthesis characters next to the operator. */
+int getParensNextToOp(BinaryExpr expr) {
+  exists(Expr left, Expr right, int pleft, int pright |
+    left = expr.getLeftOperand() and
+    right = expr.getRightOperand() and
+    (if left.isParenthesized() then isParenthesized(left, pleft) else pleft = 0) and
+    (if right.isParenthesized() then isParenthesized(right, pright) else pright = 0) and
+    result = pleft + pright
+  )
+}
+
 /** Compute whitespace around the operator. */
 int operatorWS(BinaryExpr expr) {
-  exists(int line, int lcol, int rcol |
+  exists(int line, int lcol, int rcol, int parens |
     endOfBinaryLhs(expr, line, lcol) and
     startOfBinaryRhs(expr, line, rcol) and
-    result = rcol - lcol + 1 - expr.getOp().length()
+    parens = getParensNextToOp(expr) and
+    result = rcol - lcol + 1 - expr.getOp().length() - parens
   )
 }
 
 /** Find nested binary expressions where the programmer may have made a precedence mistake. */
 predicate interestingNesting(BinaryExpr inner, BinaryExpr outer) {
   inner = outer.getAChildExpr() and
   not inner instanceof AssocNestedExpr and
-  not inner instanceof HarmlessNestedExpr
+  not inner instanceof HarmlessNestedExpr and
+  not inner.isParenthesized()
 }
 
 from BinaryExpr inner, BinaryExpr outer, int wsouter, int wsinner
diff --git a/java/ql/src/config/semmlecode.dbscheme b/java/ql/src/config/semmlecode.dbscheme
@@ -516,6 +516,11 @@ statementEnclosingExpr(
   int statement_id: @stmt ref
 );
 
+isParenthesized(
+  unique int id: @expr ref,
+  int parentheses: int ref
+);
+
 case @expr.kind of
    1  = @arrayaccess
 |  2  = @arraycreationexpr
@@ -570,7 +575,7 @@ case @expr.kind of
 | 51  = @castexpr
 | 52  = @newexpr
 | 53  = @conditionalexpr
-| 54  = @parexpr
+| 54  = @parexpr         // deprecated
 | 55  = @instanceofexpr
 | 56  = @localvariabledeclexpr
 | 57  = @typeliteral
diff --git a/java/ql/src/config/semmlecode.dbscheme.stats b/java/ql/src/config/semmlecode.dbscheme.stats
@@ -18307,6 +18307,59 @@
 </dependencies>
 </relation>
 <relation>
+<name>isParenthesized</name>
+<cardinality>76010</cardinality>
+<columnsizes>
+<e>
+<k>id</k>
+<v>76010</v>
+</e>
+<e>
+<k>parentheses</k>
+<v>2</v>
+</e>
+</columnsizes>
+<dependencies>
+<dep>
+<src>id</src>
+<trg>parentheses</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>1</a>
+<b>2</b>
+<v>76010</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+<dep>
+<src>parentheses</src>
+<trg>id</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>30</a>
+<b>31</b>
+<v>1</v>
+</b>
+<b>
+<a>75980</a>
+<b>75981</b>
+<v>1</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+</dependencies>
+</relation>
+<relation>
 <name>callableBinding</name>
 <cardinality>1437798</cardinality>
 <columnsizes>
diff --git a/java/ql/src/semmle/code/java/Expr.qll b/java/ql/src/semmle/code/java/Expr.qll
@@ -95,6 +95,9 @@ class Expr extends ExprParent, @expr {
     or
     exists(LambdaExpr lam | lam.asMethod() = getEnclosingCallable() and lam.isInStaticContext())
   }
+
+  /** Holds if this expression is parenthesized. */
+  predicate isParenthesized() { isParenthesized(this, _) }
 }
 
 /**
@@ -1330,7 +1333,11 @@ class VarAccess extends Expr, @varaccess {
 
   /** Gets a printable representation of this expression. */
   override string toString() {
-    result = this.getQualifier().toString() + "." + this.getVariable().getName()
+    exists(Expr q | q = this.getQualifier() |
+      if q.isParenthesized()
+      then result = "(...)." + this.getVariable().getName()
+      else result = q.toString() + "." + this.getVariable().getName()
+    )
     or
     not this.hasQualifier() and result = this.getVariable().getName()
   }
diff --git a/java/ql/test/library-tests/constants/CompileTimeConstantExpr.expected b/java/ql/test/library-tests/constants/CompileTimeConstantExpr.expected
@@ -7,10 +7,8 @@
 | constants/Constants.java:15:15:15:37 | ... + ... |
 | constants/Constants.java:15:37:15:37 | 1 |
 | constants/Constants.java:16:23:16:41 | Initializers.SFIELD |
-| constants/Constants.java:18:15:18:18 | (...) |
 | constants/Constants.java:18:16:18:17 | 12 |
 | constants/Constants.java:19:19:19:28 | "a string" |
-| constants/Constants.java:20:17:20:23 | (...) |
 | constants/Constants.java:20:17:20:31 | ...?...:... |
 | constants/Constants.java:20:18:20:18 | 3 |
 | constants/Constants.java:20:18:20:22 | ... < ... |
diff --git a/java/ql/test/library-tests/constants/getIntValue.expected b/java/ql/test/library-tests/constants/getIntValue.expected
@@ -31,7 +31,7 @@
 | constants/Values.java:67:27:67:33 | ... & ... | 42 |
 | constants/Values.java:68:26:68:32 | ... \| ... | 42 |
 | constants/Values.java:69:27:69:33 | ... ^ ... | 42 |
-| constants/Values.java:83:19:83:22 | (...) | 42 |
+| constants/Values.java:83:20:83:21 | 42 | 42 |
 | constants/Values.java:86:25:86:35 | final_field | 42 |
 | constants/Values.java:87:33:87:34 | 42 | 42 |
 | constants/Values.java:88:25:88:35 | final_local | 42 |
diff --git a/java/ql/test/query-tests/Nullness/NullExprDeref.expected b/java/ql/test/query-tests/Nullness/NullExprDeref.expected
@@ -1 +1 @@
-| ExprDeref.java:7:12:7:34 | (...) | This expression is dereferenced and may be null. |
+| ExprDeref.java:7:13:7:33 | ...?...:... | This expression is dereferenced and may be null. |
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/annotValue.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/annotValue.ql
@@ -0,0 +1,27 @@
+class Annotation extends @annotation {
+  string toString() { result = "annotation" }
+}
+
+class Method extends @method {
+  string toString() { result = "method" }
+}
+
+class Expr extends @expr {
+  string toString() { result = "expr" }
+}
+
+class ParExpr extends Expr, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+predicate parExprGetExpr(ParExpr pe, Expr e) { exprs(e, _, _, pe, _) }
+
+from Annotation parentid, Method id2, Expr oldvalue, Expr value
+where
+  annotValue(parentid, id2, oldvalue) and
+  if oldvalue instanceof ParExpr
+  then
+    parExprGetExpr+(oldvalue, value) and
+    not value instanceof ParExpr
+  else value = oldvalue
+select parentid, id2, value
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/callableEnclosingExpr.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/callableEnclosingExpr.ql
@@ -0,0 +1,17 @@
+class Expr extends @expr {
+  string toString() { result = "expr" }
+}
+
+class ParExpr extends Expr, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+class Callable extends @callable {
+  string toString() { result = "callable" }
+}
+
+from Expr id, Callable c
+where
+  callableEnclosingExpr(id, c) and
+  not id instanceof ParExpr
+select id, c
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/exprs.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/exprs.ql
@@ -0,0 +1,38 @@
+class ExprParent extends @exprparent {
+  string toString() { result = "exprparent" }
+}
+
+class Expr extends ExprParent, @expr {
+  override string toString() { result = "expr" }
+}
+
+class ParExpr extends Expr, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+class Type extends @type {
+  string toString() { result = "type" }
+}
+
+predicate astlink(ExprParent parent, Expr e, int idx, int parens) {
+  exprs(e, _, _, parent, idx) and
+  parens = 0 and
+  not parent instanceof ParExpr and
+  e instanceof ParExpr
+  or
+  exists(ParExpr pe |
+    exprs(e, _, _, pe, _) and
+    astlink(parent, pe, idx, parens - 1)
+  )
+}
+
+from Expr id, int kind, Type typeid, ExprParent oldparent, ExprParent parent, int oldidx, int idx
+where
+  exprs(id, kind, typeid, oldparent, oldidx) and
+  not id instanceof ParExpr and
+  if oldparent instanceof ParExpr
+  then astlink(parent, id, idx, _)
+  else (
+    parent = oldparent and idx = oldidx
+  )
+select id, kind, typeid, parent, idx
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/hasLocation.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/hasLocation.ql
@@ -0,0 +1,17 @@
+class Locatable extends @locatable {
+  string toString() { result = "locatable" }
+}
+
+class Location extends @location {
+  string toString() { result = "location" }
+}
+
+class ParExpr extends Locatable, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+from Locatable locatable, Location location
+where
+  hasLocation(locatable, location) and
+  not locatable instanceof ParExpr
+select locatable, location
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/isParenthesized.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/isParenthesized.ql
@@ -0,0 +1,29 @@
+class ExprParent extends @exprparent {
+  string toString() { result = "exprparent" }
+}
+
+class Expr extends ExprParent, @expr {
+  override string toString() { result = "expr" }
+}
+
+class ParExpr extends Expr, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+predicate astlink(ExprParent parent, Expr e, int idx, int parens) {
+  exprs(e, _, _, parent, idx) and
+  parens = 0 and
+  not parent instanceof ParExpr and
+  e instanceof ParExpr
+  or
+  exists(ParExpr pe |
+    exprs(e, _, _, pe, _) and
+    astlink(parent, pe, idx, parens - 1)
+  )
+}
+
+from Expr e, int parentheses
+where
+  astlink(_, e, _, parentheses) and
+  not e instanceof ParExpr
+select e, parentheses
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/numlines.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/numlines.ql
@@ -0,0 +1,13 @@
+class Sourceline extends @sourceline {
+  string toString() { result = "sourceline" }
+}
+
+class ParExpr extends Sourceline, @parexpr {
+  override string toString() { result = "(...)" }
+}
+
+from Sourceline id, int l, int code, int comm
+where
+  numlines(id, l, code, comm) and
+  not id instanceof ParExpr
+select id, l, code, comm
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/old.dbscheme b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/old.dbscheme
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/semmlecode.dbscheme b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/semmlecode.dbscheme
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/statementEnclosingExpr.ql b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/statementEnclosingExpr.ql
diff --git a/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/upgrade.properties b/java/upgrades/e7706df98aaefcf055f35f50582f2686f41c23bb/upgrade.properties

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-\| ExprDeref.java:7:12:7:34 \| (...) \| This expression is dereferenced and may be null. \|`
	`1`	`+\| ExprDeref.java:7:13:7:33 \| ...?...:... \| This expression is dereferenced and may be null. \|`