Address review feedback

Author: Marcono1234

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

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

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

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

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

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

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

@@ -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)
 }
 
 /**

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

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

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

@@ -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 = "...?...:..." }
 

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

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