From 75c549baa1c90157b25ccb6cf08f8df12e14c43e Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 29 Jan 2020 15:52:07 +0100 Subject: [PATCH 1/3] Java: Deprecate ParExpr. --- java/ql/src/Language Abuse/UselessUpcast.ql | 10 ++-- .../src/Likely Bugs/Arithmetic/BadCheckOdd.ql | 8 +-- .../ConstantExpAppearsNonConstant.ql | 15 +++--- .../Likely Bugs/Arithmetic/IntMultToLong.ql | 6 +-- .../DefineEqualsWhenAddingFields.ql | 4 +- .../Comparison/MissingInstanceofInEquals.ql | 2 +- .../Comparison/NoAssignInBooleanExprs.ql | 2 +- .../Comparison/StringComparison.ql | 5 +- .../Comparison/UselessComparisonTest.ql | 4 -- .../Concurrency/DoubleCheckedLocking.qll | 2 - .../Concurrency/NonSynchronizedOverride.ql | 2 - .../Concurrency/SynchSetUnsynchGet.ql | 2 +- .../Likely Typos/ContradictoryTypeChecks.ql | 2 +- .../Likely Bugs/Resource Leaks/CloseType.qll | 2 - .../src/Likely Bugs/Statements/Chaining.qll | 2 +- .../Statements/ReturnValueIgnored.ql | 2 +- .../Security/CWE/CWE-190/ArithmeticCommon.qll | 7 ++- .../CWE/CWE-681/NumericCastCommon.qll | 2 +- .../src/Security/CWE/CWE-835/InfiniteLoop.ql | 2 - .../Boolean Logic/SimplifyBoolExpr.ql | 4 +- .../ConfusingOverloading.ql | 1 - java/ql/src/semmle/code/java/Concurrency.qll | 3 +- .../src/semmle/code/java/ControlFlowGraph.qll | 7 --- java/ql/src/semmle/code/java/Conversions.qll | 2 +- java/ql/src/semmle/code/java/Expr.qll | 49 ++++++++----------- java/ql/src/semmle/code/java/Reflection.qll | 2 +- java/ql/src/semmle/code/java/StringFormat.qll | 5 +- java/ql/src/semmle/code/java/Variable.qll | 2 +- .../code/java/comparison/Comparison.qll | 4 -- .../semmle/code/java/controlflow/Guards.qll | 10 ++-- .../java/controlflow/UnreachableBlocks.qll | 4 -- .../java/controlflow/internal/GuardsLogic.qll | 20 +++----- .../controlflow/internal/Preconditions.qll | 8 +-- .../code/java/dataflow/IntegerGuards.qll | 9 ++-- .../code/java/dataflow/ModulusAnalysis.qll | 2 - .../semmle/code/java/dataflow/NullGuards.qll | 3 -- .../semmle/code/java/dataflow/Nullness.qll | 12 ++--- .../code/java/dataflow/RangeAnalysis.qll | 8 +-- .../semmle/code/java/dataflow/RangeUtils.qll | 6 --- java/ql/src/semmle/code/java/dataflow/SSA.qll | 2 - .../code/java/dataflow/SignAnalysis.qll | 2 - .../semmle/code/java/dataflow/TypeFlow.qll | 2 - .../java/dataflow/internal/DataFlowUtil.qll | 2 - .../code/java/deadcode/DeadEnumConstant.qll | 2 - .../code/java/dispatch/DispatchFlow.qll | 2 - .../src/semmle/code/java/dispatch/ObjFlow.qll | 2 - .../code/java/frameworks/Assertions.qll | 2 +- .../test/library-tests/guards/guardslogic.ql | 1 - 48 files changed, 87 insertions(+), 172 deletions(-) diff --git a/java/ql/src/Language Abuse/UselessUpcast.ql b/java/ql/src/Language Abuse/UselessUpcast.ql index 5c34d4d4cff..18584da159c 100644 --- a/java/ql/src/Language Abuse/UselessUpcast.ql +++ b/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,13 +31,13 @@ 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() + e = ce.getTrueExpr() or e = ce.getFalseExpr() ) or // Upcasts to raw types. @@ -46,12 +46,12 @@ predicate usefulUpcast(CastExpr e) { 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()) | diff --git a/java/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.ql b/java/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.ql index ead6919f8be..70f34bcb077 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.ql +++ b/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 diff --git a/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql b/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql index fb55aeb7b2b..eb4e86e65c9 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql +++ b/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,25 +30,25 @@ 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 + a.getAnOperand().(BooleanLiteral).getBooleanValue() = false ) or exists(OrLogicalExpr o | o = e | - o.getAnOperand().getProperExpr().(BooleanLiteral).getBooleanValue() = true + o.getAnOperand().(BooleanLiteral).getBooleanValue() = true ) } diff --git a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql b/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql index 9371e57b029..d6b7b97199e 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/IntMultToLong.ql +++ b/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 diff --git a/java/ql/src/Likely Bugs/Comparison/DefineEqualsWhenAddingFields.ql b/java/ql/src/Likely Bugs/Comparison/DefineEqualsWhenAddingFields.ql index c32095ccbb3..be186d2b2e2 100644 --- a/java/ql/src/Likely Bugs/Comparison/DefineEqualsWhenAddingFields.ql +++ b/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 ) diff --git a/java/ql/src/Likely Bugs/Comparison/MissingInstanceofInEquals.ql b/java/ql/src/Likely Bugs/Comparison/MissingInstanceofInEquals.ql index eb044e4ecb0..eacba3ad4ec 100644 --- a/java/ql/src/Likely Bugs/Comparison/MissingInstanceofInEquals.ql +++ b/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) ) diff --git a/java/ql/src/Likely Bugs/Comparison/NoAssignInBooleanExprs.ql b/java/ql/src/Likely Bugs/Comparison/NoAssignInBooleanExprs.ql index b04f8de49b5..600257d1c60 100644 --- a/java/ql/src/Likely Bugs/Comparison/NoAssignInBooleanExprs.ql +++ b/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 ) diff --git a/java/ql/src/Likely Bugs/Comparison/StringComparison.ql b/java/ql/src/Likely Bugs/Comparison/StringComparison.ql index 1bb67c04a67..4681feafc00 100644 --- a/java/ql/src/Likely Bugs/Comparison/StringComparison.ql +++ b/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 diff --git a/java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.ql b/java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.ql index af37df936e0..964a5a6060a 100644 --- a/java/ql/src/Likely Bugs/Comparison/UselessComparisonTest.ql +++ b/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) diff --git a/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.qll b/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.qll index 63293099c5b..ac279049ed1 100644 --- a/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.qll +++ b/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.qll @@ -13,8 +13,6 @@ private Expr getAFieldRead(Field f) { v.getDefiningExpr().(VariableAssign).getSource() = getAFieldRead(f) ) or - result.(ParExpr).getExpr() = getAFieldRead(f) - or result.(AssignExpr).getSource() = getAFieldRead(f) } diff --git a/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql b/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql index de7e5bc5f6c..b6518ba9bdb 100644 --- a/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql +++ b/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql @@ -27,8 +27,6 @@ predicate delegatingSuperCall(Expr e, Method target) { ) or delegatingSuperCall(e.(CastExpr).getExpr(), target) - or - delegatingSuperCall(e.(ParExpr).getExpr(), target) } /** diff --git a/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql b/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql index df620ca08d8..cbbc07f475c 100644 --- a/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql +++ b/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql @@ -23,7 +23,7 @@ import java */ predicate isSynchronizedByBlock(Method m) { exists(SynchronizedStmt sync, Expr on | - sync = m.getBody().getAChild*() and on = sync.getExpr().getProperExpr() + sync = m.getBody().getAChild*() and on = sync.getExpr() | if m.isStatic() then on.(TypeLiteral).getTypeName().getType() = m.getDeclaringType() diff --git a/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql b/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql index 6a9fc9b7051..73cb99be0c3 100644 --- a/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql +++ b/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql @@ -17,7 +17,7 @@ import semmle.code.java.dataflow.SSA /** `ioe` is of the form `va instanceof t`. */ predicate instanceOfCheck(InstanceOfExpr ioe, VarAccess va, RefType t) { - ioe.getExpr().getProperExpr() = va and + ioe.getExpr() = va and ioe.getTypeName().getType().(RefType).getSourceDeclaration() = t } diff --git a/java/ql/src/Likely Bugs/Resource Leaks/CloseType.qll b/java/ql/src/Likely Bugs/Resource Leaks/CloseType.qll index c319f2568d6..16f8d6f8bcc 100644 --- a/java/ql/src/Likely Bugs/Resource Leaks/CloseType.qll +++ b/java/ql/src/Likely Bugs/Resource Leaks/CloseType.qll @@ -9,8 +9,6 @@ import semmle.code.java.frameworks.Mockito private predicate flowsInto(Expr e, Variable v) { e = v.getAnAssignedValue() or - exists(ParExpr p | flowsInto(p, v) | e = p.getExpr()) - or exists(CastExpr c | flowsInto(c, v) | e = c.getExpr()) or exists(ConditionalExpr c | flowsInto(c, v) | e = c.getTrueExpr() or e = c.getFalseExpr()) diff --git a/java/ql/src/Likely Bugs/Statements/Chaining.qll b/java/ql/src/Likely Bugs/Statements/Chaining.qll index 315f0e32c09..1e1c7187e30 100644 --- a/java/ql/src/Likely Bugs/Statements/Chaining.qll +++ b/java/ql/src/Likely Bugs/Statements/Chaining.qll @@ -48,7 +48,7 @@ private predicate nonChainingReturn(Method m, ReturnStmt ret) { or // A method on the wrong object is called. not ( - delegateCall.getQualifier().getProperExpr() instanceof ThisAccess or + delegateCall.getQualifier() instanceof ThisAccess or not exists(delegateCall.getQualifier()) ) or diff --git a/java/ql/src/Likely Bugs/Statements/ReturnValueIgnored.ql b/java/ql/src/Likely Bugs/Statements/ReturnValueIgnored.ql index 88d786a7159..d2c2c555ae2 100644 --- a/java/ql/src/Likely Bugs/Statements/ReturnValueIgnored.ql +++ b/java/ql/src/Likely Bugs/Statements/ReturnValueIgnored.ql @@ -78,7 +78,7 @@ predicate relevantMethodCall(MethodAccess ma, Method m) { ma.getMethod() = m and not m.getReturnType().hasName("void") and (not isMockingMethod(m) or isMustBeQualifierMockingMethod(m)) and - not isMockingMethod(ma.getQualifier().getProperExpr().(MethodAccess).getMethod()) + not isMockingMethod(ma.getQualifier().(MethodAccess).getMethod()) } predicate methodStats(Method m, int used, int total, int percentage) { diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticCommon.qll b/java/ql/src/Security/CWE/CWE-190/ArithmeticCommon.qll index c59c6376ae9..9d081e34420 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticCommon.qll +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticCommon.qll @@ -14,7 +14,7 @@ private import semmle.code.java.controlflow.internal.GuardsLogic predicate narrowerThanOrEqualTo(ArithExpr exp, NumType numType) { exp.getType().(NumType).widerThan(numType) implies - exists(CastExpr cast | cast.getAChildExpr().getProperExpr() = exp | + exists(CastExpr cast | cast.getAChildExpr() = exp | numType.widerThanOrEqualTo(cast.getType().(NumType)) ) } @@ -54,11 +54,11 @@ private Guard sizeGuard(SsaVariable v, boolean branch, boolean upper) { positive(pos) and upper = true | - comp.getLesserOperand().getProperExpr() = add and + comp.getLesserOperand() = add and comp.getGreaterOperand().(IntegerLiteral).getIntValue() = 0 and branch = false or - comp.getGreaterOperand().getProperExpr() = add and + comp.getGreaterOperand() = add and comp.getLesserOperand().(IntegerLiteral).getIntValue() = 0 and branch = true ) @@ -157,7 +157,6 @@ predicate upcastToWiderType(Expr e) { /** Holds if the result of `exp` has certain bits filtered by a bitwise and. */ private predicate inBitwiseAnd(Expr exp) { exists(AndBitwiseExpr a | a.getAnOperand() = exp) or - inBitwiseAnd(exp.(ParExpr).getExpr()) or inBitwiseAnd(exp.(LShiftExpr).getAnOperand()) or inBitwiseAnd(exp.(RShiftExpr).getAnOperand()) or inBitwiseAnd(exp.(URShiftExpr).getAnOperand()) diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastCommon.qll b/java/ql/src/Security/CWE/CWE-681/NumericCastCommon.qll index e66e71ebd9f..4887c3fefad 100644 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastCommon.qll +++ b/java/ql/src/Security/CWE/CWE-681/NumericCastCommon.qll @@ -29,7 +29,7 @@ class RightShiftOp extends Expr { Variable getShiftedVariable() { getLhs() = result.getAnAccess() or - getLhs().getProperExpr().(AndBitwiseExpr).getAnOperand() = result.getAnAccess() + getLhs().(AndBitwiseExpr).getAnOperand() = result.getAnAccess() } } diff --git a/java/ql/src/Security/CWE/CWE-835/InfiniteLoop.ql b/java/ql/src/Security/CWE/CWE-835/InfiniteLoop.ql index 728d2b17930..cc02dfb3f09 100644 --- a/java/ql/src/Security/CWE/CWE-835/InfiniteLoop.ql +++ b/java/ql/src/Security/CWE/CWE-835/InfiniteLoop.ql @@ -48,8 +48,6 @@ predicate subCondition(Expr cond, Expr subcond, boolean negated) { or subCondition(cond.(OrLogicalExpr).getAnOperand(), subcond, negated) or - subCondition(cond.(ParExpr).getExpr(), subcond, negated) - or subCondition(cond.(LogNotExpr).getExpr(), subcond, negated.booleanNot()) } diff --git a/java/ql/src/Violations of Best Practice/Boolean Logic/SimplifyBoolExpr.ql b/java/ql/src/Violations of Best Practice/Boolean Logic/SimplifyBoolExpr.ql index db8fc26614e..178f5dbd847 100644 --- a/java/ql/src/Violations of Best Practice/Boolean Logic/SimplifyBoolExpr.ql +++ b/java/ql/src/Violations of Best Practice/Boolean Logic/SimplifyBoolExpr.ql @@ -84,9 +84,9 @@ where or conditionalWithBool(e, pattern, rewrite) or - e.(LogNotExpr).getExpr().getProperExpr().(ComparisonOrEquality).negate(pattern, rewrite) + e.(LogNotExpr).getExpr().(ComparisonOrEquality).negate(pattern, rewrite) or - e.(LogNotExpr).getExpr().getProperExpr() instanceof LogNotExpr and + e.(LogNotExpr).getExpr() instanceof LogNotExpr and pattern = "!!A" and rewrite = "A" select e, "Expressions of the form \"" + pattern + "\" can be simplified to \"" + rewrite + "\"." diff --git a/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql b/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql index 07c071ad2d4..1d8509c6e03 100644 --- a/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql +++ b/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql @@ -84,7 +84,6 @@ private predicate confusinglyOverloaded(Method m, Method n) { private predicate wrappedAccess(Expr e, MethodAccess ma) { e = ma or - wrappedAccess(e.(ParExpr).getExpr(), ma) or wrappedAccess(e.(CastExpr).getExpr(), ma) } diff --git a/java/ql/src/semmle/code/java/Concurrency.qll b/java/ql/src/semmle/code/java/Concurrency.qll index 29202c8cc40..61e76525ec8 100644 --- a/java/ql/src/semmle/code/java/Concurrency.qll +++ b/java/ql/src/semmle/code/java/Concurrency.qll @@ -14,8 +14,7 @@ predicate locallySynchronizedOn(Expr e, SynchronizedStmt sync, Variable v) { */ predicate locallySynchronizedOnThis(Expr e, RefType thisType) { exists(SynchronizedStmt sync | e.getEnclosingStmt().getEnclosingStmt+() = sync | - sync.getExpr().getProperExpr().(ThisAccess).getType().(RefType).getSourceDeclaration() = - thisType + sync.getExpr().(ThisAccess).getType().(RefType).getSourceDeclaration() = thisType ) or exists(SynchronizedCallable c | c = e.getEnclosingCallable() | diff --git a/java/ql/src/semmle/code/java/ControlFlowGraph.qll b/java/ql/src/semmle/code/java/ControlFlowGraph.qll index 9c7cc976d46..13e6524352a 100644 --- a/java/ql/src/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/src/semmle/code/java/ControlFlowGraph.qll @@ -289,8 +289,6 @@ private module ControlFlowGraphImpl { logexpr.(UnaryExpr).getExpr() = b and inBooleanContext(logexpr) ) or - exists(ParExpr parexpr | parexpr.getExpr() = b and inBooleanContext(parexpr)) - or exists(ConditionalExpr condexpr | condexpr.getCondition() = b or @@ -574,8 +572,6 @@ private module ControlFlowGraphImpl { or result = first(n.(PostOrderNode).firstChild()) or - result = first(n.(ParExpr).getExpr()) - or result = first(n.(SynchronizedStmt).getExpr()) or result = n and @@ -708,9 +704,6 @@ private module ControlFlowGraphImpl { last(condexpr.getTrueExpr(), last, completion) ) or - // Parentheses are skipped in the CFG. - last(n.(ParExpr).getExpr(), last, completion) - or // The last node of a node executed in post-order is the node itself. n.(PostOrderNode).mayCompleteNormally() and last = n and completion = NormalCompletion() or diff --git a/java/ql/src/semmle/code/java/Conversions.qll b/java/ql/src/semmle/code/java/Conversions.qll index 460b2c24ff2..9d55f1297fc 100644 --- a/java/ql/src/semmle/code/java/Conversions.qll +++ b/java/ql/src/semmle/code/java/Conversions.qll @@ -49,7 +49,7 @@ class AssignmentConversionContext extends ConversionSite { AssignmentConversionContext() { this = v.getAnAssignedValue() or - exists(Assignment a | a.getDest().getProperExpr() = v.getAnAccess() and this = a.getSource()) + exists(Assignment a | a.getDest() = v.getAnAccess() and this = a.getSource()) } override Type getConversionTarget() { result = v.getType() } diff --git a/java/ql/src/semmle/code/java/Expr.qll b/java/ql/src/semmle/code/java/Expr.qll index 51312dd1dc5..08f869c01f2 100755 --- a/java/ql/src/semmle/code/java/Expr.qll +++ b/java/ql/src/semmle/code/java/Expr.qll @@ -46,8 +46,12 @@ class Expr extends ExprParent, @expr { */ int getKind() { exprs(this, result, _, _, _) } - /** Gets this expression with any surrounding parentheses removed. */ - Expr getProperExpr() { + /** + * DEPRECATED: This is no longer necessary. See `Expr.isParenthesized()`. + * + * Gets this expression with any surrounding parentheses removed. + */ + deprecated Expr getProperExpr() { result = this.(ParExpr).getExpr().getProperExpr() or result = this and not this instanceof ParExpr @@ -152,9 +156,6 @@ class CompileTimeConstantExpr extends Expr { e.getFalseExpr().isCompileTimeConstant() ) or - // Parenthesized expressions whose contained expression is a constant expression. - this.(ParExpr).getExpr().isCompileTimeConstant() - or // Access to a final variable initialized by a compile-time constant. exists(Variable v | this = v.getAnAccess() | v.isFinal() and @@ -169,8 +170,6 @@ class CompileTimeConstantExpr extends Expr { string getStringValue() { result = this.(StringLiteral).getRepresentedString() or - result = this.(ParExpr).getExpr().(CompileTimeConstantExpr).getStringValue() - or result = this.(AddExpr).getLeftOperand().(CompileTimeConstantExpr).getStringValue() + this.(AddExpr).getRightOperand().(CompileTimeConstantExpr).getStringValue() @@ -296,9 +295,6 @@ class CompileTimeConstantExpr extends Expr { else result = ce.getFalseExpr().(CompileTimeConstantExpr).getBooleanValue() ) or - // Parenthesized expressions containing a boolean value. - result = this.(ParExpr).getExpr().(CompileTimeConstantExpr).getBooleanValue() - or // Simple or qualified names where the variable is final and the initializer is a constant. exists(Variable v | this = v.getAnAccess() | result = v.getInitializer().(CompileTimeConstantExpr).getBooleanValue() @@ -385,8 +381,6 @@ class CompileTimeConstantExpr extends Expr { else result = ce.getFalseExpr().(CompileTimeConstantExpr).getIntValue() ) or - result = this.(ParExpr).getExpr().(CompileTimeConstantExpr).getIntValue() - or // If a `Variable` is a `CompileTimeConstantExpr`, its value is its initializer. exists(Variable v | this = v.getAnAccess() | result = v.getInitializer().(CompileTimeConstantExpr).getIntValue() @@ -640,12 +634,8 @@ class BinaryExpr extends Expr, @binaryexpr { /** Gets the operand on the right-hand side of this binary expression. */ Expr getRightOperand() { result.isNthChildOf(this, 1) } - /** Gets an operand (left or right), with any parentheses removed. */ - Expr getAnOperand() { - exists(Expr r | r = this.getLeftOperand() or r = this.getRightOperand() | - result = r.getProperExpr() - ) - } + /** Gets an operand (left or right). */ + Expr getAnOperand() { result = this.getLeftOperand() or result = this.getRightOperand() } /** The operands of this binary expression are `e` and `f`, in either order. */ predicate hasOperands(Expr e, Expr f) { @@ -761,17 +751,14 @@ class NEExpr extends BinaryExpr, @neexpr { * A bitwise expression. * * This includes expressions involving the operators - * `&`, `|`, `^`, or `~`, - * possibly parenthesized. + * `&`, `|`, `^`, or `~`. */ class BitwiseExpr extends Expr { BitwiseExpr() { - exists(Expr proper | proper = this.getProperExpr() | - proper instanceof AndBitwiseExpr or - proper instanceof OrBitwiseExpr or - proper instanceof XorBitwiseExpr or - proper instanceof BitNotExpr - ) + this instanceof AndBitwiseExpr or + this instanceof OrBitwiseExpr or + this instanceof XorBitwiseExpr or + this instanceof BitNotExpr } } @@ -1128,10 +1115,14 @@ class SwitchExpr extends Expr, @switchexpr { override string toString() { result = "switch (...)" } } -/** A parenthesised expression. */ -class ParExpr extends Expr, @parexpr { +/** + * DEPRECATED: Use `Expr.isParenthesized()` instead. + * + * A parenthesised expression. + */ +deprecated class ParExpr extends Expr, @parexpr { /** Gets the expression inside the parentheses. */ - Expr getExpr() { result.getParent() = this } + deprecated Expr getExpr() { result.getParent() = this } /** Gets a printable representation of this expression. */ override string toString() { result = "(...)" } diff --git a/java/ql/src/semmle/code/java/Reflection.qll b/java/ql/src/semmle/code/java/Reflection.qll index 0c6432e2409..3b0189e17ae 100644 --- a/java/ql/src/semmle/code/java/Reflection.qll +++ b/java/ql/src/semmle/code/java/Reflection.qll @@ -284,7 +284,7 @@ class NewInstance extends MethodAccess { * cast. */ private Type getCastInferredConstructedTypes() { - exists(CastExpr cast | cast.getExpr() = this or cast.getExpr().(ParExpr).getExpr() = this | + exists(CastExpr cast | cast.getExpr() = this | result = cast.getType() or // If we cast the result of this method, then this is either the type specified, or a diff --git a/java/ql/src/semmle/code/java/StringFormat.qll b/java/ql/src/semmle/code/java/StringFormat.qll index 075471d73ca..410d83818cc 100644 --- a/java/ql/src/semmle/code/java/StringFormat.qll +++ b/java/ql/src/semmle/code/java/StringFormat.qll @@ -246,8 +246,7 @@ private predicate formatStringFragment(Expr fmt) { e.(AddExpr).getLeftOperand() = fmt or e.(AddExpr).getRightOperand() = fmt or e.(ConditionalExpr).getTrueExpr() = fmt or - e.(ConditionalExpr).getFalseExpr() = fmt or - e.(ParExpr).getExpr() = fmt + e.(ConditionalExpr).getFalseExpr() = fmt ) } @@ -268,8 +267,6 @@ private predicate formatStringValue(Expr e, string fmtvalue) { or e.getType() instanceof EnumType and fmtvalue = "x" // dummy value or - formatStringValue(e.(ParExpr).getExpr(), fmtvalue) - or exists(Variable v | e = v.getAnAccess() and v.isFinal() and diff --git a/java/ql/src/semmle/code/java/Variable.qll b/java/ql/src/semmle/code/java/Variable.qll index 9a281786ac4..d63bcc6fe1c 100755 --- a/java/ql/src/semmle/code/java/Variable.qll +++ b/java/ql/src/semmle/code/java/Variable.qll @@ -17,7 +17,7 @@ class Variable extends @variable, Annotatable, Element, Modifiable { exists(LocalVariableDeclExpr e | e.getVariable() = this and result = e.getInit()) or exists(AssignExpr e | - e.getDest().getProperExpr() = this.getAnAccess() and result = e.getSource() + e.getDest() = this.getAnAccess() and result = e.getSource() ) } diff --git a/java/ql/src/semmle/code/java/comparison/Comparison.qll b/java/ql/src/semmle/code/java/comparison/Comparison.qll index 8a869d8a2e4..27ed9271e99 100644 --- a/java/ql/src/semmle/code/java/comparison/Comparison.qll +++ b/java/ql/src/semmle/code/java/comparison/Comparison.qll @@ -6,10 +6,6 @@ import java * Used as basis for the transitive closure in `exprImplies`. */ private predicate exprImpliesStep(Expr e1, boolean b1, Expr e2, boolean b2) { - e1.(ParExpr).getProperExpr() = e2 and - b2 = b1 and - (b1 = true or b1 = false) - or e1.(LogNotExpr).getExpr() = e2 and b2 = b1.booleanNot() and (b1 = true or b1 = false) diff --git a/java/ql/src/semmle/code/java/controlflow/Guards.qll b/java/ql/src/semmle/code/java/controlflow/Guards.qll index c382e7963cc..82cb1d997ae 100644 --- a/java/ql/src/semmle/code/java/controlflow/Guards.qll +++ b/java/ql/src/semmle/code/java/controlflow/Guards.qll @@ -103,8 +103,8 @@ class Guard extends ExprParent { */ BasicBlock getBasicBlock() { result = this.(Expr).getBasicBlock() or - result = this.(SwitchCase).getSwitch().getExpr().getProperExpr().getBasicBlock() or - result = this.(SwitchCase).getSwitchExpr().getExpr().getProperExpr().getBasicBlock() + result = this.(SwitchCase).getSwitch().getExpr().getBasicBlock() or + result = this.(SwitchCase).getSwitchExpr().getExpr().getBasicBlock() } /** @@ -115,9 +115,9 @@ class Guard extends ExprParent { */ predicate isEquality(Expr e1, Expr e2, boolean polarity) { exists(Expr exp1, Expr exp2 | equalityGuard(this, exp1, exp2, polarity) | - e1 = exp1.getProperExpr() and e2 = exp2.getProperExpr() + e1 = exp1 and e2 = exp2 or - e2 = exp1.getProperExpr() and e1 = exp2.getProperExpr() + e2 = exp1 and e1 = exp2 ) } @@ -265,7 +265,7 @@ private predicate equalityGuard(Guard g, Expr e1, Expr e2, boolean polarity) { exists(ConstCase cc | cc = g and polarity = true and - cc.getSelectorExpr().getProperExpr() = e1 and + cc.getSelectorExpr() = e1 and cc.getValue() = e2 and strictcount(cc.getValue(_)) = 1 ) diff --git a/java/ql/src/semmle/code/java/controlflow/UnreachableBlocks.qll b/java/ql/src/semmle/code/java/controlflow/UnreachableBlocks.qll index c7be6d2494d..5ce27fda434 100644 --- a/java/ql/src/semmle/code/java/controlflow/UnreachableBlocks.qll +++ b/java/ql/src/semmle/code/java/controlflow/UnreachableBlocks.qll @@ -93,8 +93,6 @@ class ConstantExpr extends Expr { // A binary expression where both sides are constant this.(BinaryExpr).getLeftOperand() instanceof ConstantExpr and this.(BinaryExpr).getRightOperand() instanceof ConstantExpr - or - this.(ParExpr).getExpr() instanceof ConstantExpr ) } @@ -108,8 +106,6 @@ class ConstantExpr extends Expr { or result = this.(FieldRead).getField().(ConstantField).getConstantValue().getBooleanValue() or - result = this.(ParExpr).getExpr().(ConstantExpr).getBooleanValue() - or // Handle binary expressions that have integer operands and a boolean result. exists(BinaryExpr b, int left, int right | b = this and diff --git a/java/ql/src/semmle/code/java/controlflow/internal/GuardsLogic.qll b/java/ql/src/semmle/code/java/controlflow/internal/GuardsLogic.qll index 6fd8d08a658..ba2fd8c01c0 100644 --- a/java/ql/src/semmle/code/java/controlflow/internal/GuardsLogic.qll +++ b/java/ql/src/semmle/code/java/controlflow/internal/GuardsLogic.qll @@ -19,10 +19,6 @@ private import semmle.code.java.dataflow.IntegerGuards * Restricted to BaseSSA-based reasoning. */ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) { - g1.(ParExpr).getExpr() = g2 and - b1 = b2 and - (b1 = true or b1 = false) - or g1.(AndBitwiseExpr).getAnOperand() = g2 and b1 = true and b2 = true or g1.(OrBitwiseExpr).getAnOperand() = g2 and b1 = false and b2 = false @@ -220,13 +216,11 @@ private predicate hasPossibleUnknownValue(SsaVariable v) { * `ConditionalExpr`s. Parentheses are also removed. */ private Expr possibleValue(Expr e) { - result = possibleValue(e.(ParExpr).getExpr()) - or result = possibleValue(e.(ConditionalExpr).getTrueExpr()) or result = possibleValue(e.(ConditionalExpr).getFalseExpr()) or - result = e and not e instanceof ParExpr and not e instanceof ConditionalExpr + result = e and not e instanceof ConditionalExpr } /** @@ -253,7 +247,7 @@ private predicate possibleValue(SsaVariable v, boolean fromBackEdge, Expr e, Abs not hasPossibleUnknownValue(v) and exists(SsaExplicitUpdate def | def = getADefinition(v, fromBackEdge) and - e = possibleValue(def.getDefiningExpr().(VariableAssign).getSource().getProperExpr()) and + e = possibleValue(def.getDefiningExpr().(VariableAssign).getSource()) and k.getExpr() = e ) } @@ -305,7 +299,7 @@ private predicate guardControlsPhiBranch( SsaExplicitUpdate upd, SsaPhiNode phi, Guard guard, boolean branch, Expr e ) { guard.directlyControls(upd.getBasicBlock(), branch) and - upd.getDefiningExpr().(VariableAssign).getSource().getProperExpr() = e and + upd.getDefiningExpr().(VariableAssign).getSource() = e and upd = phi.getAPhiInput() and guard.getBasicBlock().bbStrictlyDominates(phi.getBasicBlock()) } @@ -319,12 +313,12 @@ private predicate guardControlsPhiBranch( */ private predicate conditionalAssign(SsaVariable v, Guard guard, boolean branch, Expr e) { exists(ConditionalExpr c | - v.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource().getProperExpr() = c and - guard = c.getCondition().getProperExpr() + v.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() = c and + guard = c.getCondition() | - branch = true and e = c.getTrueExpr().getProperExpr() + branch = true and e = c.getTrueExpr() or - branch = false and e = c.getFalseExpr().getProperExpr() + branch = false and e = c.getFalseExpr() ) or exists(SsaExplicitUpdate upd, SsaPhiNode phi | diff --git a/java/ql/src/semmle/code/java/controlflow/internal/Preconditions.qll b/java/ql/src/semmle/code/java/controlflow/internal/Preconditions.qll index 9f7fbc14aa4..0fd367d65b4 100644 --- a/java/ql/src/semmle/code/java/controlflow/internal/Preconditions.qll +++ b/java/ql/src/semmle/code/java/controlflow/internal/Preconditions.qll @@ -24,9 +24,9 @@ predicate conditionCheckMethod(Method m, boolean checkTrue) { not m.isOverridable() and p.getType() instanceof BooleanType and m.getBody().getStmt(0) = ifstmt and - ifstmt.getCondition().getProperExpr() = cond and + ifstmt.getCondition() = cond and ( - cond.(LogNotExpr).getExpr().getProperExpr().(VarAccess).getVariable() = p and checkTrue = true + cond.(LogNotExpr).getExpr().(VarAccess).getVariable() = p and checkTrue = true or cond.(VarAccess).getVariable() = p and checkTrue = false ) and @@ -41,9 +41,9 @@ predicate conditionCheckMethod(Method m, boolean checkTrue) { not m.isOverridable() and m.getBody().getStmt(0).(ExprStmt).getExpr() = ma and conditionCheck(ma, ct) and - ma.getArgument(0).getProperExpr() = arg and + ma.getArgument(0) = arg and ( - arg.(LogNotExpr).getExpr().getProperExpr().(VarAccess).getVariable() = p and + arg.(LogNotExpr).getExpr().(VarAccess).getVariable() = p and checkTrue = ct.booleanNot() or arg.(VarAccess).getVariable() = p and checkTrue = ct diff --git a/java/ql/src/semmle/code/java/dataflow/IntegerGuards.qll b/java/ql/src/semmle/code/java/dataflow/IntegerGuards.qll index 5fb592272fb..29e2b3ec8a1 100644 --- a/java/ql/src/semmle/code/java/dataflow/IntegerGuards.qll +++ b/java/ql/src/semmle/code/java/dataflow/IntegerGuards.qll @@ -10,7 +10,6 @@ private import RangeAnalysis /** Gets an expression that might have the value `i`. */ private Expr exprWithIntValue(int i) { result.(ConstantIntegerExpr).getIntValue() = i or - result.(ParExpr).getExpr() = exprWithIntValue(i) or result.(ConditionalExpr).getTrueExpr() = exprWithIntValue(i) or result.(ConditionalExpr).getFalseExpr() = exprWithIntValue(i) } @@ -142,25 +141,25 @@ Expr intBoundGuard(RValue x, boolean branch_with_lower_bound_k, int k) { x.getVariable().getType() instanceof IntegralType | // c < x - comp.getLesserOperand().getProperExpr() = c and + comp.getLesserOperand() = c and comp.isStrict() and branch_with_lower_bound_k = true and val + 1 = k or // c <= x - comp.getLesserOperand().getProperExpr() = c and + comp.getLesserOperand() = c and not comp.isStrict() and branch_with_lower_bound_k = true and val = k or // x < c - comp.getGreaterOperand().getProperExpr() = c and + comp.getGreaterOperand() = c and comp.isStrict() and branch_with_lower_bound_k = false and val = k or // x <= c - comp.getGreaterOperand().getProperExpr() = c and + comp.getGreaterOperand() = c and not comp.isStrict() and branch_with_lower_bound_k = false and val + 1 = k diff --git a/java/ql/src/semmle/code/java/dataflow/ModulusAnalysis.qll b/java/ql/src/semmle/code/java/dataflow/ModulusAnalysis.qll index 876c746d7c0..1fb3eb201a8 100644 --- a/java/ql/src/semmle/code/java/dataflow/ModulusAnalysis.qll +++ b/java/ql/src/semmle/code/java/dataflow/ModulusAnalysis.qll @@ -76,8 +76,6 @@ private Expr modExpr(Expr arg, int mod) { c.getIntValue() = mod - 1 and result.(AndBitwiseExpr).hasOperands(arg, c) ) - or - result.(ParExpr).getExpr() = modExpr(arg, mod) } /** diff --git a/java/ql/src/semmle/code/java/dataflow/NullGuards.qll b/java/ql/src/semmle/code/java/dataflow/NullGuards.qll index b30c6f70f18..e4e7ec33e2c 100644 --- a/java/ql/src/semmle/code/java/dataflow/NullGuards.qll +++ b/java/ql/src/semmle/code/java/dataflow/NullGuards.qll @@ -11,7 +11,6 @@ private import IntegerGuards /** Gets an expression that is always `null`. */ Expr alwaysNullExpr() { result instanceof NullLiteral or - result.(ParExpr).getExpr() = alwaysNullExpr() or result.(CastExpr).getExpr() = alwaysNullExpr() } @@ -60,8 +59,6 @@ Expr clearlyNotNullExpr(Expr reason) { reason = result ) or - result.(ParExpr).getExpr() = clearlyNotNullExpr(reason) - or result.(CastExpr).getExpr() = clearlyNotNullExpr(reason) or result.(AssignExpr).getSource() = clearlyNotNullExpr(reason) diff --git a/java/ql/src/semmle/code/java/dataflow/Nullness.qll b/java/ql/src/semmle/code/java/dataflow/Nullness.qll index c3faeaf0ff2..644790bea6b 100644 --- a/java/ql/src/semmle/code/java/dataflow/Nullness.qll +++ b/java/ql/src/semmle/code/java/dataflow/Nullness.qll @@ -45,7 +45,6 @@ private import semmle.code.java.frameworks.Assertions /** Gets an expression that may be `null`. */ Expr nullExpr() { result instanceof NullLiteral or - result.(ParExpr).getExpr() = nullExpr() or result.(ConditionalExpr).getTrueExpr() = nullExpr() or result.(ConditionalExpr).getFalseExpr() = nullExpr() or result.(AssignExpr).getSource() = nullExpr() or @@ -131,11 +130,8 @@ predicate dereference(Expr e) { * The `VarAccess` is included for nicer error reporting. */ private ControlFlowNode varDereference(SsaVariable v, VarAccess va) { - exists(Expr e | - dereference(e) and - e = sameValue(v, va) and - result = e.getProperExpr() - ) + dereference(result) and + result = sameValue(v, va) } /** @@ -442,8 +438,8 @@ private predicate nullDerefCandidate(SsaVariable origin, VarAccess va) { /** A variable that is assigned `null` if the given condition takes the given branch. */ private predicate varConditionallyNull(SsaExplicitUpdate v, ConditionBlock cond, boolean branch) { exists(ConditionalExpr condexpr | - v.getDefiningExpr().(VariableAssign).getSource().getProperExpr() = condexpr and - condexpr.getCondition().getProperExpr() = cond.getCondition() + v.getDefiningExpr().(VariableAssign).getSource() = condexpr and + condexpr.getCondition() = cond.getCondition() | condexpr.getTrueExpr() = nullExpr() and branch = true and diff --git a/java/ql/src/semmle/code/java/dataflow/RangeAnalysis.qll b/java/ql/src/semmle/code/java/dataflow/RangeAnalysis.qll index e6b2e22d83c..435b976fde2 100644 --- a/java/ql/src/semmle/code/java/dataflow/RangeAnalysis.qll +++ b/java/ql/src/semmle/code/java/dataflow/RangeAnalysis.qll @@ -131,7 +131,7 @@ private predicate boundCondition( or exists(SubExpr sub, ConstantIntegerExpr c, int d | // (v - d) - e < c - comp.getLesserOperand().getProperExpr() = sub and + comp.getLesserOperand() = sub and comp.getGreaterOperand() = c and sub.getLeftOperand() = ssaRead(v, d) and sub.getRightOperand() = e and @@ -139,7 +139,7 @@ private predicate boundCondition( delta = d + c.getIntValue() or // (v - d) - e > c - comp.getGreaterOperand().getProperExpr() = sub and + comp.getGreaterOperand() = sub and comp.getLesserOperand() = c and sub.getLeftOperand() = ssaRead(v, d) and sub.getRightOperand() = e and @@ -147,7 +147,7 @@ private predicate boundCondition( delta = d + c.getIntValue() or // e - (v - d) < c - comp.getLesserOperand().getProperExpr() = sub and + comp.getLesserOperand() = sub and comp.getGreaterOperand() = c and sub.getLeftOperand() = e and sub.getRightOperand() = ssaRead(v, d) and @@ -155,7 +155,7 @@ private predicate boundCondition( delta = d - c.getIntValue() or // e - (v - d) > c - comp.getGreaterOperand().getProperExpr() = sub and + comp.getGreaterOperand() = sub and comp.getLesserOperand() = c and sub.getLeftOperand() = e and sub.getRightOperand() = ssaRead(v, d) and diff --git a/java/ql/src/semmle/code/java/dataflow/RangeUtils.qll b/java/ql/src/semmle/code/java/dataflow/RangeUtils.qll index 08d46f6bb80..22e363d9e3c 100644 --- a/java/ql/src/semmle/code/java/dataflow/RangeUtils.qll +++ b/java/ql/src/semmle/code/java/dataflow/RangeUtils.qll @@ -26,8 +26,6 @@ private predicate nonNullSsaFwdStep(SsaVariable v, SsaVariable phi) { } private predicate nonNullDefStep(Expr e1, Expr e2) { - e2.(ParExpr).getExpr() = e1 - or exists(ConditionalExpr cond | cond = e2 | cond.getTrueExpr() = e1 and cond.getFalseExpr() instanceof NullLiteral or @@ -104,8 +102,6 @@ class ConstantIntegerExpr extends Expr { Expr ssaRead(SsaVariable v, int delta) { result = v.getAUse() and delta = 0 or - result.(ParExpr).getExpr() = ssaRead(v, delta) - or exists(int d1, ConstantIntegerExpr c | result.(AddExpr).hasOperands(ssaRead(v, d1), c) and delta = d1 - c.getIntValue() @@ -264,8 +260,6 @@ predicate ssaUpdateStep(SsaExplicitUpdate v, Expr e, int delta) { * Holds if `e1 + delta` equals `e2`. */ predicate valueFlowStep(Expr e2, Expr e1, int delta) { - e2.(ParExpr).getExpr() = e1 and delta = 0 - or e2.(AssignExpr).getSource() = e1 and delta = 0 or e2.(PlusExpr).getExpr() = e1 and delta = 0 diff --git a/java/ql/src/semmle/code/java/dataflow/SSA.qll b/java/ql/src/semmle/code/java/dataflow/SSA.qll index ee397dd478f..561fdb35c5e 100644 --- a/java/ql/src/semmle/code/java/dataflow/SSA.qll +++ b/java/ql/src/semmle/code/java/dataflow/SSA.qll @@ -1129,7 +1129,5 @@ Expr sameValue(SsaVariable v, VarAccess va) { or result.(AssignExpr).getSource() = sameValue(v, va) or - result.(ParExpr).getExpr() = sameValue(v, va) - or result.(RefTypeCastExpr).getExpr() = sameValue(v, va) } diff --git a/java/ql/src/semmle/code/java/dataflow/SignAnalysis.qll b/java/ql/src/semmle/code/java/dataflow/SignAnalysis.qll index 7e9a22dbb85..60758069232 100644 --- a/java/ql/src/semmle/code/java/dataflow/SignAnalysis.qll +++ b/java/ql/src/semmle/code/java/dataflow/SignAnalysis.qll @@ -476,8 +476,6 @@ private Sign exprSign(Expr e) { ( unknownSign(e) or - result = exprSign(e.(ParExpr).getExpr()) - or exists(SsaVariable v | v.getAUse() = e | result = ssaSign(v, any(SsaReadPositionBlock bb | bb.getBlock() = e.getBasicBlock())) or diff --git a/java/ql/src/semmle/code/java/dataflow/TypeFlow.qll b/java/ql/src/semmle/code/java/dataflow/TypeFlow.qll index ec3d14b8159..e9f32aadbbe 100644 --- a/java/ql/src/semmle/code/java/dataflow/TypeFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/TypeFlow.qll @@ -109,8 +109,6 @@ private predicate step(TypeFlowNode n1, TypeFlowNode n2) { or n2.asExpr() = n1.asSsa().getAUse() or - n2.asExpr().(ParExpr).getExpr() = n1.asExpr() - or n2.asExpr().(CastExpr).getExpr() = n1.asExpr() and not n2.asExpr().getType() instanceof PrimitiveType or diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll index ffa1a86f688..889e0c0c8a6 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -390,8 +390,6 @@ predicate simpleLocalFlowStep(Node node1, Node node2) { or ThisFlow::adjacentThisRefs(node1.(PostUpdateNode).getPreUpdateNode(), node2) or - node2.asExpr().(ParExpr).getExpr() = node1.asExpr() - or node2.asExpr().(CastExpr).getExpr() = node1.asExpr() or node2.asExpr().(ConditionalExpr).getTrueExpr() = node1.asExpr() diff --git a/java/ql/src/semmle/code/java/deadcode/DeadEnumConstant.qll b/java/ql/src/semmle/code/java/deadcode/DeadEnumConstant.qll index 1c19a9e20cf..c29b167d31e 100644 --- a/java/ql/src/semmle/code/java/deadcode/DeadEnumConstant.qll +++ b/java/ql/src/semmle/code/java/deadcode/DeadEnumConstant.qll @@ -11,8 +11,6 @@ Expr valueFlow(Expr src) { src = c.getTrueExpr() or src = c.getFalseExpr() ) - or - src = result.(ParExpr).getExpr() } /** diff --git a/java/ql/src/semmle/code/java/dispatch/DispatchFlow.qll b/java/ql/src/semmle/code/java/dispatch/DispatchFlow.qll index 4194a4cd97c..0b2b871e129 100644 --- a/java/ql/src/semmle/code/java/dispatch/DispatchFlow.qll +++ b/java/ql/src/semmle/code/java/dispatch/DispatchFlow.qll @@ -192,8 +192,6 @@ private predicate flowStep(RelevantNode n1, RelevantNode n2) { n2.asExpr().(MethodAccess).getMethod() = getValue ) or - n2.asExpr().(ParExpr).getExpr() = n1.asExpr() - or n2.asExpr().(CastExpr).getExpr() = n1.asExpr() or n2.asExpr().(ConditionalExpr).getTrueExpr() = n1.asExpr() diff --git a/java/ql/src/semmle/code/java/dispatch/ObjFlow.qll b/java/ql/src/semmle/code/java/dispatch/ObjFlow.qll index c0740a43364..4d97bfdc3cc 100644 --- a/java/ql/src/semmle/code/java/dispatch/ObjFlow.qll +++ b/java/ql/src/semmle/code/java/dispatch/ObjFlow.qll @@ -98,8 +98,6 @@ private predicate step(Node n1, Node n2) { n2.asExpr().(FieldRead).getField() = f ) or - n2.asExpr().(ParExpr).getExpr() = n1.asExpr() - or n2.asExpr().(CastExpr).getExpr() = n1.asExpr() or n2.asExpr().(ConditionalExpr).getTrueExpr() = n1.asExpr() diff --git a/java/ql/src/semmle/code/java/frameworks/Assertions.qll b/java/ql/src/semmle/code/java/frameworks/Assertions.qll index eb210d5e0d7..810af5ca41e 100644 --- a/java/ql/src/semmle/code/java/frameworks/Assertions.qll +++ b/java/ql/src/semmle/code/java/frameworks/Assertions.qll @@ -109,6 +109,6 @@ predicate assertFail(BasicBlock bb, ControlFlowNode n) { n = m.getACheck(any(BooleanLiteral b | b.getBooleanValue() = true)) ) or exists(AssertFailMethod m | n = m.getACheck()) or - n.(AssertStmt).getExpr().getProperExpr().(BooleanLiteral).getBooleanValue() = false + n.(AssertStmt).getExpr().(BooleanLiteral).getBooleanValue() = false ) } diff --git a/java/ql/test/library-tests/guards/guardslogic.ql b/java/ql/test/library-tests/guards/guardslogic.ql index 516a8670473..afbb313d664 100644 --- a/java/ql/test/library-tests/guards/guardslogic.ql +++ b/java/ql/test/library-tests/guards/guardslogic.ql @@ -4,6 +4,5 @@ import semmle.code.java.controlflow.Guards from Guard g, BasicBlock bb, boolean branch where g.controls(bb, branch) and - not g instanceof ParExpr and g.getEnclosingCallable().getDeclaringType().hasName("Logic") select g, branch, bb From 843fd37c7576a48c9bed533c4e6949cf79e6d6fc Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 29 Jan 2020 15:55:36 +0100 Subject: [PATCH 2/3] Java: Add change note. --- change-notes/1.24/analysis-java.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/change-notes/1.24/analysis-java.md b/change-notes/1.24/analysis-java.md index cfdd157d13f..b618d367a1e 100644 --- a/change-notes/1.24/analysis-java.md +++ b/change-notes/1.24/analysis-java.md @@ -32,3 +32,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. From d8b842298c797ce77b8892284633456b82ffa888 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Thu, 30 Jan 2020 10:54:54 +0100 Subject: [PATCH 3/3] Java: Autoformat. --- java/ql/src/Language Abuse/UselessUpcast.ql | 4 +--- .../Arithmetic/ConstantExpAppearsNonConstant.ql | 8 ++------ java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql | 4 +--- java/ql/src/semmle/code/java/Variable.qll | 4 +--- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/java/ql/src/Language Abuse/UselessUpcast.ql b/java/ql/src/Language Abuse/UselessUpcast.ql index 18584da159c..d8b5761c909 100644 --- a/java/ql/src/Language Abuse/UselessUpcast.ql +++ b/java/ql/src/Language Abuse/UselessUpcast.ql @@ -36,9 +36,7 @@ predicate usefulUpcast(CastExpr e) { ) or // Upcasts that are performed on an operand of a ternary expression. - exists(ConditionalExpr ce | - e = ce.getTrueExpr() or e = ce.getFalseExpr() - ) + exists(ConditionalExpr ce | e = ce.getTrueExpr() or e = ce.getFalseExpr()) or // Upcasts to raw types. e.getType() instanceof RawType diff --git a/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql b/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql index eb4e86e65c9..90471c9d0c5 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql +++ b/java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql @@ -43,13 +43,9 @@ predicate isConstantExp(Expr e) { or exists(AndBitwiseExpr a | a = e | eval(a.getAnOperand()) = 0) or - exists(AndLogicalExpr a | a = e | - a.getAnOperand().(BooleanLiteral).getBooleanValue() = false - ) + exists(AndLogicalExpr a | a = e | a.getAnOperand().(BooleanLiteral).getBooleanValue() = false) or - exists(OrLogicalExpr o | o = e | - o.getAnOperand().(BooleanLiteral).getBooleanValue() = true - ) + exists(OrLogicalExpr o | o = e | o.getAnOperand().(BooleanLiteral).getBooleanValue() = true) } from Expr e diff --git a/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql b/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql index cbbc07f475c..8f91ae89211 100644 --- a/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql +++ b/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql @@ -22,9 +22,7 @@ import java * (for static methods) or a `synchronized(this){...}` block (for instance methods). */ predicate isSynchronizedByBlock(Method m) { - exists(SynchronizedStmt sync, Expr on | - sync = m.getBody().getAChild*() and on = sync.getExpr() - | + exists(SynchronizedStmt sync, Expr on | sync = m.getBody().getAChild*() and on = sync.getExpr() | if m.isStatic() then on.(TypeLiteral).getTypeName().getType() = m.getDeclaringType() else on.(ThisAccess).getType().(RefType).getSourceDeclaration() = m.getDeclaringType() diff --git a/java/ql/src/semmle/code/java/Variable.qll b/java/ql/src/semmle/code/java/Variable.qll index d63bcc6fe1c..44c7e617d3f 100755 --- a/java/ql/src/semmle/code/java/Variable.qll +++ b/java/ql/src/semmle/code/java/Variable.qll @@ -16,9 +16,7 @@ class Variable extends @variable, Annotatable, Element, Modifiable { Expr getAnAssignedValue() { exists(LocalVariableDeclExpr e | e.getVariable() = this and result = e.getInit()) or - exists(AssignExpr e | - e.getDest() = this.getAnAccess() and result = e.getSource() - ) + exists(AssignExpr e | e.getDest() = this.getAnAccess() and result = e.getSource()) } /** Gets the initializer expression of this variable. */