Skip to content

Commit 905648e

Browse files
committed
Add ConditionalExpr.getBranchExpr(boolean)
1 parent 02578cf commit 905648e

File tree

13 files changed

+35
-65
lines changed

13 files changed

+35
-65
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ predicate usefulUpcast(CastExpr e) {
3636
)
3737
or
3838
// Upcasts that are performed on an operand of a ternary expression.
39-
exists(ConditionalExpr ce | e = ce.getTrueExpr() or e = ce.getFalseExpr())
39+
exists(ConditionalExpr ce | e = ce.getBranchExpr(_))
4040
or
4141
// Upcasts to raw types.
4242
e.getType() instanceof RawType

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@ class CharType extends PrimitiveType {
1616
CharType() { this.hasName("char") }
1717
}
1818

19-
private Type getABranchType(ConditionalExpr ce) {
20-
result = ce.getTrueExpr().getType() or
21-
result = ce.getFalseExpr().getType()
22-
}
19+
private Type getABranchType(ConditionalExpr ce) { result = ce.getBranchExpr(_).getType() }
2320

2421
from ConditionalExpr ce
2522
where

java/ql/src/Likely Bugs/Resource Leaks/CloseType.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ private predicate flowsInto(Expr e, Variable v) {
1111
or
1212
exists(CastExpr c | flowsInto(c, v) | e = c.getExpr())
1313
or
14-
exists(ConditionalExpr c | flowsInto(c, v) | e = c.getTrueExpr() or e = c.getFalseExpr())
14+
exists(ConditionalExpr c | flowsInto(c, v) | e = c.getBranchExpr(_))
1515
}
1616

1717
/**

java/ql/src/Security/CWE/CWE-190/ArithmeticCommon.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,7 @@ predicate upcastToWiderType(Expr e) {
148148
or
149149
exists(Parameter p | p.getAnArgument() = e and t2 = p.getType())
150150
or
151-
exists(ConditionalExpr cond | cond.getTrueExpr() = e or cond.getFalseExpr() = e |
152-
t2 = cond.getType()
153-
)
151+
exists(ConditionalExpr cond | cond.getBranchExpr(_) | t2 = cond.getType())
154152
)
155153
}
156154

java/ql/src/Violations of Best Practice/legacy/AutoBoxing.ql

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ predicate unboxed(BoxedExpr e) {
4545
or
4646
flowTarget(e).getType() instanceof PrimitiveType
4747
or
48-
exists(ConditionalExpr cond | cond instanceof PrimitiveExpr |
49-
cond.getTrueExpr() = e or cond.getFalseExpr() = e
50-
)
48+
exists(ConditionalExpr cond | cond instanceof PrimitiveExpr | cond.getBranchExpr(_) = e)
5149
}
5250

5351
/**

java/ql/src/semmle/code/java/ControlFlowGraph.qll

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ private module ControlFlowGraphImpl {
293293
exists(ConditionalExpr condexpr |
294294
condexpr.getCondition() = b
295295
or
296-
(condexpr.getTrueExpr() = b or condexpr.getFalseExpr() = b) and
296+
condexpr.getBranchExpr(_) = b and
297297
inBooleanContext(condexpr)
298298
)
299299
or
@@ -706,8 +706,7 @@ private module ControlFlowGraphImpl {
706706
or
707707
// The last node of a `ConditionalExpr` is in either of its branches.
708708
exists(ConditionalExpr condexpr | condexpr = n |
709-
last(condexpr.getFalseExpr(), last, completion) or
710-
last(condexpr.getTrueExpr(), last, completion)
709+
last(condexpr.getBranchExpr(_), last, completion)
711710
)
712711
or
713712
exists(InstanceOfExpr ioe | ioe.isPattern() and ioe = n |
@@ -915,14 +914,10 @@ private module ControlFlowGraphImpl {
915914
)
916915
or
917916
// Control flows to the corresponding branch depending on the boolean completion of the condition.
918-
exists(ConditionalExpr e |
917+
exists(ConditionalExpr e, boolean branch |
919918
last(e.getCondition(), n, completion) and
920-
completion = BooleanCompletion(true, _) and
921-
result = first(e.getTrueExpr())
922-
or
923-
last(e.getCondition(), n, completion) and
924-
completion = BooleanCompletion(false, _) and
925-
result = first(e.getFalseExpr())
919+
completion = BooleanCompletion(branch, _) and
920+
result = first(e.getBranchExpr(branch))
926921
)
927922
or
928923
exists(InstanceOfExpr ioe | ioe.isPattern() |

java/ql/src/semmle/code/java/Expr.qll

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,7 @@ class CompileTimeConstantExpr extends Expr {
186186
ce = this and
187187
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
188188
|
189-
if condition = true
190-
then result = ce.getTrueExpr().(CompileTimeConstantExpr).getStringValue()
191-
else result = ce.getFalseExpr().(CompileTimeConstantExpr).getStringValue()
189+
result = ce.getBranchExpr(condition).(CompileTimeConstantExpr).getStringValue()
192190
)
193191
or
194192
exists(Variable v | this = v.getAnAccess() |
@@ -297,9 +295,7 @@ class CompileTimeConstantExpr extends Expr {
297295
ce = this and
298296
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
299297
|
300-
if condition = true
301-
then result = ce.getTrueExpr().(CompileTimeConstantExpr).getBooleanValue()
302-
else result = ce.getFalseExpr().(CompileTimeConstantExpr).getBooleanValue()
298+
ce.getBranchExpr(condition).(CompileTimeConstantExpr).getBooleanValue()
303299
)
304300
or
305301
// Simple or qualified names where the variable is final and the initializer is a constant.
@@ -382,9 +378,7 @@ class CompileTimeConstantExpr extends Expr {
382378
ce = this and
383379
condition = ce.getCondition().(CompileTimeConstantExpr).getBooleanValue()
384380
|
385-
if condition = true
386-
then result = ce.getTrueExpr().(CompileTimeConstantExpr).getIntValue()
387-
else result = ce.getFalseExpr().(CompileTimeConstantExpr).getIntValue()
381+
result = ce.getBranchExpr(condition).(CompileTimeConstantExpr).getIntValue()
388382
)
389383
or
390384
// If a `Variable` is a `CompileTimeConstantExpr`, its value is its initializer.
@@ -1186,8 +1180,7 @@ class ChooseExpr extends Expr {
11861180

11871181
/** Gets a result expression of this `switch` or conditional expression. */
11881182
Expr getAResultExpr() {
1189-
result = this.(ConditionalExpr).getTrueExpr() or
1190-
result = this.(ConditionalExpr).getFalseExpr() or
1183+
result = this.(ConditionalExpr).getBranchExpr(_) or
11911184
result = this.(SwitchExpr).getAResult()
11921185
}
11931186
}
@@ -1213,6 +1206,15 @@ class ConditionalExpr extends Expr, @conditionalexpr {
12131206
*/
12141207
Expr getFalseExpr() { result.isNthChildOf(this, 2) }
12151208

1209+
/**
1210+
* Gets the expression that is evaluated by the specific branch of this
1211+
* conditional expression. If `true` that is `getTrueExpr()`, if `false`
1212+
* it is `getFalseExpr()`.
1213+
*/
1214+
Expr getBranchExpr(boolean branch) {
1215+
if branch = true then result = getTrueExpr() else result = getFalseExpr()
1216+
}
1217+
12161218
/** Gets a printable representation of this expression. */
12171219
override string toString() { result = "...?...:..." }
12181220

java/ql/src/semmle/code/java/controlflow/internal/GuardsLogic.qll

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,15 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) {
4040
)
4141
or
4242
exists(ConditionalExpr cond, boolean branch, BooleanLiteral boollit, boolean boolval |
43-
cond.getTrueExpr() = boollit and branch = true
44-
or
45-
cond.getFalseExpr() = boollit and branch = false
43+
cond.getBranchExpr(branch) = boollit
4644
|
4745
cond = g1 and
4846
boolval = boollit.getBooleanValue() and
4947
b1 = boolval.booleanNot() and
5048
(
5149
g2 = cond.getCondition() and b2 = branch.booleanNot()
5250
or
53-
g2 = cond.getTrueExpr() and b2 = b1
54-
or
55-
g2 = cond.getFalseExpr() and b2 = b1
51+
g2 = cond.getBranchExpr(_) and b2 = b1
5652
)
5753
)
5854
or
@@ -216,9 +212,7 @@ private predicate hasPossibleUnknownValue(SsaVariable v) {
216212
* `ConditionalExpr`s.
217213
*/
218214
private Expr possibleValue(Expr e) {
219-
result = possibleValue(e.(ConditionalExpr).getTrueExpr())
220-
or
221-
result = possibleValue(e.(ConditionalExpr).getFalseExpr())
215+
result = possibleValue(e.(ConditionalExpr).getBranchExpr(_))
222216
or
223217
result = e and not e instanceof ConditionalExpr
224218
}
@@ -316,9 +310,7 @@ private predicate conditionalAssign(SsaVariable v, Guard guard, boolean branch,
316310
v.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() = c and
317311
guard = c.getCondition()
318312
|
319-
branch = true and e = c.getTrueExpr()
320-
or
321-
branch = false and e = c.getFalseExpr()
313+
e = c.getBranchExpr(branch)
322314
)
323315
or
324316
exists(SsaExplicitUpdate upd, SsaPhiNode phi |

java/ql/src/semmle/code/java/dataflow/ModulusAnalysis.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,7 @@ predicate exprModulus(Expr e, Bound b, int val, int mod) {
268268
private predicate condExprBranchModulus(
269269
ConditionalExpr cond, boolean branch, Bound b, int val, int mod
270270
) {
271-
exprModulus(cond.getTrueExpr(), b, val, mod) and branch = true
272-
or
273-
exprModulus(cond.getFalseExpr(), b, val, mod) and branch = false
271+
exprModulus(cond.getBranchExpr(branch), b, val, mod)
274272
}
275273

276274
private predicate addModulus(Expr add, boolean isLeft, Bound b, int val, int mod) {

java/ql/src/semmle/code/java/dataflow/Nullness.qll

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,9 @@ private predicate varConditionallyNull(SsaExplicitUpdate v, ConditionBlock cond,
438438
v.getDefiningExpr().(VariableAssign).getSource() = condexpr and
439439
condexpr.getCondition() = cond.getCondition()
440440
|
441-
condexpr.getTrueExpr() = nullExpr() and
441+
condexpr.getBranchExpr(branch) = nullExpr() and
442442
branch = true and
443-
not condexpr.getFalseExpr() = nullExpr()
444-
or
445-
condexpr.getFalseExpr() = nullExpr() and
446-
branch = false and
447-
not condexpr.getTrueExpr() = nullExpr()
443+
not condexpr.getBranchExpr(branch.booleanNot()) = nullExpr()
448444
)
449445
or
450446
v.getDefiningExpr().(VariableAssign).getSource() = nullExpr() and

0 commit comments

Comments
 (0)