Swift: add NilCoalescingTest node to CFG

Fixes an issue where a nil-coalescing operation used in a boolean
context would result in no control flow out of the default operand of
the nil-coalescing operator.

Author: Robert Marsh

swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll

@@ -12,7 +12,8 @@ newtype TControlFlowElement =
   TPropertyObserverElement(Accessor observer, AssignExpr assign) {
     isPropertyObserverElement(observer, assign)
   } or
-  TKeyPathElement(KeyPathExpr expr)
+  TKeyPathElement(KeyPathExpr expr) or
+  TNilCoalescingTestElement(NilCoalescingExpr expr)
 
 predicate isLValue(Expr e) { any(AssignExpr assign).getDest() = e }
 
@@ -204,3 +205,15 @@ class ClosureElement extends ControlFlowElement, TClosureElement {
 
   override string toString() { result = expr.toString() }
 }
+
+class NilCoalescingElement extends ControlFlowElement, TNilCoalescingTestElement {
+  NilCoalescingExpr expr;
+
+  NilCoalescingElement() { this = TNilCoalescingTestElement(expr) }
+
+  override Location getLocation() { result = expr.getLocation() }
+
+  NilCoalescingExpr getAst() { result = expr }
+
+  override string toString() { result = "emptiness test for " + expr.toString() }
+}

swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll

@@ -1586,7 +1586,9 @@ module Exprs {
     }
   }
 
-  private class NilCoalescingTree extends AstControlFlowTree {
+  private class NilCoalescingTestTree extends LeafTree, NilCoalescingElement { }
+
+  private class NilCoalescingTree extends AstPostOrderTree {
     override NilCoalescingExpr ast;
 
     final override predicate propagatesAbnormal(ControlFlowElement child) {
@@ -1597,22 +1599,22 @@ module Exprs {
       astFirst(ast.getLeftOperand().getFullyConverted(), first)
     }
 
-    final override predicate last(ControlFlowElement last, Completion c) {
-      last.asAstNode() = ast and
-      exists(EmptinessCompletion ec | ec = c | not ec.isEmpty())
-      or
-      astLast(ast.getRightOperand().getFullyConverted(), last, c) and
-      c instanceof NormalCompletion
-    }
-
     final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
       astLast(ast.getLeftOperand().getFullyConverted(), pred, c) and
       c instanceof NormalCompletion and
-      succ.asAstNode() = ast
+      succ.(NilCoalescingTestTree).getAst() = ast
       or
-      pred.asAstNode() = ast and
-      c.(EmptinessCompletion).isEmpty() and
+      pred.(NilCoalescingTestTree).getAst() = ast and
+      exists(EmptinessCompletion ec | c = ec | ec.isEmpty()) and
       astFirst(ast.getRightOperand().getFullyConverted(), succ)
+      or
+      pred.(NilCoalescingTestTree).getAst() = ast and
+      exists(EmptinessCompletion ec | c = ec | not ec.isEmpty()) and
+      succ.asAstNode() = ast
+      or
+      astLast(ast.getRightOperand().getFullyConverted(), pred, c) and
+      c instanceof NormalCompletion and
+      succ.asAstNode() = ast
     }
   }
 

swift/ql/test/library-tests/controlflow/graph/Cfg.expected

@@ -6196,20 +6196,83 @@ cfg.swift:
 #-----|  -> x
 
 #  538| x
-#-----|  -> ??(_:_:)
+#-----|  -> x
 
 #  539| return ...
 #-----| return -> exit testNilCoalescing(x:) (normal)
 
 #  539| x
-#-----|  -> { ... }
+#-----|  -> emptiness test for ... ??(_:_:) ...
 
 #  539| ... ??(_:_:) ...
 #-----| exception -> exit testNilCoalescing(x:) (abnormal)
 #-----|  -> return ...
 
-#  539| ??(_:_:)
-#-----|  -> x
+#  539| emptiness test for ... ??(_:_:) ...
+#-----| non-empty -> ... ??(_:_:) ...
+#-----| empty -> { ... }
+
+#  539| 0
+#-----|  -> return ...
+
+#  539| return ...
+#-----|  -> ... ??(_:_:) ...
 
 #  539| { ... }
+#-----|  -> 0
+
+#  542| enter testNilCoalescing2(x:)
+#-----|  -> testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:) (abnormal)
+#-----|  -> exit testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:) (normal)
+#-----|  -> exit testNilCoalescing2(x:)
+
+#  542| testNilCoalescing2(x:)
+#-----|  -> x
+
+#  542| x
+#-----|  -> if ... then { ... } else { ... }
+
+#  543| if ... then { ... } else { ... }
+#-----|  -> StmtCondition
+
+#  543| x
+#-----|  -> emptiness test for ... ??(_:_:) ...
+
+#  543| ... ??(_:_:) ...
+#-----| exception -> exit testNilCoalescing2(x:) (abnormal)
+#-----| true -> 1
+#-----| false -> 0
+
+#  543| StmtCondition
+#-----|  -> x
+
+#  543| emptiness test for ... ??(_:_:) ...
+#-----| non-empty -> ... ??(_:_:) ...
+#-----| empty -> { ... }
+
+#  543| false
+#-----|  -> return ...
+
+#  543| return ...
 #-----|  -> ... ??(_:_:) ...
+
+#  543| { ... }
+#-----|  -> false
+
+#  544| return ...
+#-----| return -> exit testNilCoalescing2(x:) (normal)
+
+#  544| 1
+#-----|  -> return ...
+
+#  546| return ...
+#-----| return -> exit testNilCoalescing2(x:) (normal)
+
+#  546| 0
+#-----|  -> return ...

swift/ql/test/library-tests/controlflow/graph/cfg.swift

@@ -538,3 +538,11 @@ func testAsyncFor () async {
 func testNilCoalescing(x: Int?) -> Int {
   return x ?? 0
 }
+
+func testNilCoalescing2(x: Bool?) -> Int {
+  if x ?? false {
+    return 1
+  } else {
+    return 0
+  }
+}