Merge pull request #5150 from Marcono1234/marcono1234/conditional-expr-branch

Java: Add ConditionalExpr.getBranchExpr(boolean)

Author: aschackmull

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

@@ -268,9 +268,7 @@ predicate exprModulus(Expr e, Bound b, int val, int mod) {
 private predicate condExprBranchModulus(
   ConditionalExpr cond, boolean branch, Bound b, int val, int mod
 ) {
-  exprModulus(cond.getTrueExpr(), b, val, mod) and branch = true
-  or
-  exprModulus(cond.getFalseExpr(), b, val, mod) and branch = false
+  exprModulus(cond.getBranchExpr(branch), b, val, mod)
 }
 
 private predicate addModulus(Expr add, boolean isLeft, Bound b, int val, int mod) {

csharp/ql/src/semmle/code/csharp/dataflow/internal/rangeanalysis/RangeUtils.qll

@@ -415,5 +415,15 @@ module ExprNode {
 
     /** Gets the "else" expression of this conditional expression. */
     ExprNode getFalseExpr() { hasChild(e, e.getElse(), this, result) }
+
+    /**
+     * If `branch` is `true` gets the "then" expression, if `false` gets the
+     * "else" expression of this conditional expression.
+     */
+    ExprNode getBranchExpr(boolean branch) {
+      branch = true and result = getTrueExpr()
+      or
+      branch = false and result = getFalseExpr()
+    }
   }
 }

java/ql/src/Language Abuse/MissedTernaryOpportunity.ql

@@ -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
 }
 

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

@@ -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.getTrueExpr() or e = ce.getFalseExpr())
+  e = any(ConditionalExpr ce).getABranchExpr()
   or
   // Upcasts to raw types.
   e.getType() instanceof RawType

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

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

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

@@ -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()
   }
 }
 

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

@@ -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.getTrueExpr() or e = c.getFalseExpr())
+  exists(ConditionalExpr c | flowsInto(c, v) | e = c.getABranchExpr())
 }
 
 /**

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

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

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

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

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

@@ -293,7 +293,7 @@ private module ControlFlowGraphImpl {
     exists(ConditionalExpr condexpr |
       condexpr.getCondition() = b
       or
-      (condexpr.getTrueExpr() = b or condexpr.getFalseExpr() = b) and
+      condexpr.getABranchExpr() = b and
       inBooleanContext(condexpr)
     )
     or
@@ -706,8 +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.getFalseExpr(), last, completion) or
-      last(condexpr.getTrueExpr(), last, completion)
+      last(condexpr.getABranchExpr(), last, completion)
     )
     or
     exists(InstanceOfExpr ioe | ioe.isPattern() and ioe = n |
@@ -915,14 +914,10 @@ private module ControlFlowGraphImpl {
     )
     or
     // Control flows to the corresponding branch depending on the boolean completion of the condition.
-    exists(ConditionalExpr e |
+    exists(ConditionalExpr e, boolean branch |
       last(e.getCondition(), n, completion) and
-      completion = BooleanCompletion(true, _) and
-      result = first(e.getTrueExpr())
-      or
-      last(e.getCondition(), n, completion) and
-      completion = BooleanCompletion(false, _) and
-      result = first(e.getFalseExpr())
+      completion = BooleanCompletion(branch, _) and
+      result = first(e.getBranchExpr(branch))
     )
     or
     exists(InstanceOfExpr ioe | ioe.isPattern() |