Address review feedback

This commit is contained in:
Marcono1234
2021-02-12 20:08:00 +01:00
parent 905648e452
commit 7a6db061b5
12 changed files with 29 additions and 26 deletions

View File

@@ -14,7 +14,7 @@
import java
predicate complicatedBranch(Stmt branch) {
exists(ConditionalExpr ce | ce.getParent*() = branch) or
any(ConditionalExpr ce).getParent*() = branch or
count(MethodAccess a | a.getParent*() = branch) > 1
}

View File

@@ -36,7 +36,7 @@ predicate usefulUpcast(CastExpr e) {
)
or
// Upcasts that are performed on an operand of a ternary expression.
exists(ConditionalExpr ce | e = ce.getBranchExpr(_))
e = any(ConditionalExpr ce).getABranchExpr()
or
// Upcasts to raw types.
e.getType() instanceof RawType

View File

@@ -16,7 +16,7 @@ class CharType extends PrimitiveType {
CharType() { this.hasName("char") }
}
private Type getABranchType(ConditionalExpr ce) { result = ce.getBranchExpr(_).getType() }
private Type getABranchType(ConditionalExpr ce) { result = ce.getABranchExpr().getType() }
from ConditionalExpr ce
where

View File

@@ -17,8 +17,8 @@ import semmle.code.java.Statement
/** An expression that is used as a condition. */
class BooleanExpr extends Expr {
BooleanExpr() {
exists(ConditionalStmt s | s.getCondition() = this) or
exists(ConditionalExpr s | s.getCondition() = this)
this = any(ConditionalStmt s).getCondition() or
this = any(ConditionalExpr s).getCondition()
}
}

View File

@@ -11,7 +11,7 @@ private predicate flowsInto(Expr e, Variable v) {
or
exists(CastExpr c | flowsInto(c, v) | e = c.getExpr())
or
exists(ConditionalExpr c | flowsInto(c, v) | e = c.getBranchExpr(_))
exists(ConditionalExpr c | flowsInto(c, v) | e = c.getABranchExpr())
}
/**

View File

@@ -148,7 +148,7 @@ predicate upcastToWiderType(Expr e) {
or
exists(Parameter p | p.getAnArgument() = e and t2 = p.getType())
or
exists(ConditionalExpr cond | cond.getBranchExpr(_) | t2 = cond.getType())
exists(ConditionalExpr cond | cond.getABranchExpr() = e | t2 = cond.getType())
)
}

View File

@@ -45,7 +45,7 @@ predicate unboxed(BoxedExpr e) {
or
flowTarget(e).getType() instanceof PrimitiveType
or
exists(ConditionalExpr cond | cond instanceof PrimitiveExpr | cond.getBranchExpr(_) = e)
exists(ConditionalExpr cond | cond instanceof PrimitiveExpr | cond.getABranchExpr() = e)
}
/**

View File

@@ -293,7 +293,7 @@ private module ControlFlowGraphImpl {
exists(ConditionalExpr condexpr |
condexpr.getCondition() = b
or
condexpr.getBranchExpr(_) = b and
condexpr.getABranchExpr() = b and
inBooleanContext(condexpr)
)
or
@@ -706,7 +706,7 @@ private module ControlFlowGraphImpl {
or
// The last node of a `ConditionalExpr` is in either of its branches.
exists(ConditionalExpr condexpr | condexpr = n |
last(condexpr.getBranchExpr(_), last, completion)
last(condexpr.getABranchExpr(), last, completion)
)
or
exists(InstanceOfExpr ioe | ioe.isPattern() and ioe = n |

View File

@@ -184,8 +184,7 @@ class CompileTimeConstantExpr extends Expr {
// Ternary conditional, with compile-time constant condition.
exists(ConditionalExpr ce, boolean condition |
ce = this and
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
|
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue() and
result = ce.getBranchExpr(condition).(CompileTimeConstantExpr).getStringValue()
)
or
@@ -293,9 +292,8 @@ class CompileTimeConstantExpr extends Expr {
// Ternary expressions, where the `true` and `false` expressions are boolean compile-time constants.
exists(ConditionalExpr ce, boolean condition |
ce = this and
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
|
ce.getBranchExpr(condition).(CompileTimeConstantExpr).getBooleanValue()
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue() and
result = ce.getBranchExpr(condition).(CompileTimeConstantExpr).getBooleanValue()
)
or
// Simple or qualified names where the variable is final and the initializer is a constant.
@@ -376,8 +374,7 @@ class CompileTimeConstantExpr extends Expr {
// Ternary conditional, with compile-time constant condition.
exists(ConditionalExpr ce, boolean condition |
ce = this and
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
|
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue() and
result = ce.getBranchExpr(condition).(CompileTimeConstantExpr).getIntValue()
)
or
@@ -1180,7 +1177,7 @@ class ChooseExpr extends Expr {
/** Gets a result expression of this `switch` or conditional expression. */
Expr getAResultExpr() {
result = this.(ConditionalExpr).getBranchExpr(_) or
result = this.(ConditionalExpr).getABranchExpr() or
result = this.(SwitchExpr).getAResult()
}
}
@@ -1212,9 +1209,17 @@ class ConditionalExpr extends Expr, @conditionalexpr {
* it is `getFalseExpr()`.
*/
Expr getBranchExpr(boolean branch) {
if branch = true then result = getTrueExpr() else result = getFalseExpr()
branch = true and result = getTrueExpr()
or
branch = false and result = getFalseExpr()
}
/**
* Gets the expressions that is evaluated by one of the branches (`true`
* or `false` branch) of this conditional expression.
*/
Expr getABranchExpr() { result = getBranchExpr(_) }
/** Gets a printable representation of this expression. */
override string toString() { result = "...?...:..." }

View File

@@ -40,15 +40,14 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) {
)
or
exists(ConditionalExpr cond, boolean branch, BooleanLiteral boollit, boolean boolval |
cond.getBranchExpr(branch) = boollit
|
cond.getBranchExpr(branch) = boollit and
cond = g1 and
boolval = boollit.getBooleanValue() and
b1 = boolval.booleanNot() and
(
g2 = cond.getCondition() and b2 = branch.booleanNot()
or
g2 = cond.getBranchExpr(_) and b2 = b1
g2 = cond.getABranchExpr() and b2 = b1
)
)
or
@@ -212,7 +211,7 @@ private predicate hasPossibleUnknownValue(SsaVariable v) {
* `ConditionalExpr`s.
*/
private Expr possibleValue(Expr e) {
result = possibleValue(e.(ConditionalExpr).getBranchExpr(_))
result = possibleValue(e.(ConditionalExpr).getABranchExpr())
or
result = e and not e instanceof ConditionalExpr
}

View File

@@ -191,7 +191,7 @@ private predicate varMaybeNull(SsaVariable v, string msg, Expr reason) {
// Comparisons in finally blocks are excluded since missing exception edges in the CFG could otherwise yield FPs.
not exists(TryStmt try | try.getFinally() = e.getEnclosingStmt().getEnclosingStmt*()) and
(
exists(ConditionalExpr c | c.getCondition().getAChildExpr*() = e) or
e = any(ConditionalExpr c).getCondition().getAChildExpr*() or
not exists(MethodAccess ma | ma.getAnArgument().getAChildExpr*() = e)
) and
// Don't use a guard as reason if there is a null assignment.
@@ -439,7 +439,6 @@ private predicate varConditionallyNull(SsaExplicitUpdate v, ConditionBlock cond,
condexpr.getCondition() = cond.getCondition()
|
condexpr.getBranchExpr(branch) = nullExpr() and
branch = true and
not condexpr.getBranchExpr(branch.booleanNot()) = nullExpr()
)
or

View File

@@ -7,7 +7,7 @@ import semmle.code.java.JDKAnnotations
Expr valueFlow(Expr src) {
result = src
or
exists(ConditionalExpr c | result = c | src = c.getBranchExpr(_))
result.(ConditionalExpr).getABranchExpr() = src
}
/**