Swift: add CFG and PrintAst consistency queries, enabling them in CI

swift/actions/run-ql-tests/action.yml

@@ -26,6 +26,7 @@ runs:
           --check-repeated-labels \
           --check-redefined-labels \
           --check-use-before-definition \
+          --consistency-queries "${{ github.workspace }}/swift/ql/consistency-queries" \
           --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" \
           ${{ inputs.flags }} \
           swift/ql/test

swift/ql/consistency-queries/PrintAstConsistency.ql

@@ -0,0 +1 @@
+import codeql.swift.printast.Consistency

swift/ql/consistency-queries/qlpack.yml

@@ -0,0 +1,4 @@
+name: codeql/swift-consistency-queries
+groups: [swift, test, consistency-queries]
+dependencies:
+  codeql/swift-all: ${workspace}

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

@@ -111,31 +111,21 @@ module Stmts {
 
     override predicate propagatesAbnormal(ControlFlowElement node) { none() }
 
-    private predicate isBodyOfTapExpr() { any(TapExpr tap).getBody() = ast }
-
-    // Note: If the brace statement is the body of a `TapExpr`, the first element is the variable
-    // declaration (see https://github.com/apple/swift/blob/main/include/swift/AST/Expr.h#L848)
-    // that's initialized by the `TapExpr`. In `TapExprTree` we've already visited this declaration,
-    // along with its initializer. So we skip the first element here.
-    private AstNode getFirstElement() {
-      if this.isBodyOfTapExpr() then result = ast.getElement(1) else result = ast.getFirstElement()
-    }
-
     override predicate first(ControlFlowElement first) {
       this.firstInner(first)
       or
-      not exists(this.getFirstElement()) and first.asAstNode() = ast
+      not exists(ast.getFirstElement()) and first.asAstNode() = ast
     }
 
     override predicate last(ControlFlowElement last, Completion c) {
       this.lastInner(last, c)
       or
-      not exists(this.getFirstElement()) and
+      not exists(ast.getFirstElement()) and
       last.asAstNode() = ast and
       c instanceof SimpleCompletion
     }
 
-    predicate firstInner(ControlFlowElement first) { astFirst(this.getFirstElement(), first) }
+    predicate firstInner(ControlFlowElement first) { astFirst(ast.getFirstElement(), first) }
 
     /** Gets the body of the i'th `defer` statement. */
     private BraceStmt getDeferStmtBody(int i) {
@@ -969,6 +959,9 @@ module Decls {
         result.asAstNode() = ast.getPattern(j).getFullyUnresolved()
       )
       or
+      // PropertyWrapperBackingVarBinding is a special case as it shares the init
+      // with the original variable binding (see also PrintAstNode.qll)
+      ast != any(VarDecl d).getPropertyWrapperBackingVarBinding() and
       exists(int j |
         i = 2 * j + 1 and
         result.asAstNode() = ast.getInit(j).getFullyConverted()
@@ -1397,20 +1390,12 @@ module Exprs {
     override TapExpr ast;
 
     final override ControlFlowElement getChildElement(int i) {
-      // We first visit the local variable declaration.
+      // First we visit the expression that gives the local variable (the first element in the body) its initial value.
       i = 0 and
-      result.asAstNode() = ast.getVar()
-      or
-      // Then we visit the expression that gives the local variable its initial value.
-      i = 1 and
       result.asAstNode() = ast.getSubExpr().getFullyConverted()
       or
-      // And finally, we visit the body that potentially mutates the local variable.
-      // Note that the CFG for the body will skip the first element in the
-      // body because it's guaranteed to be the variable declaration
-      // that we've already visited at i = 0. See the explanation
-      // in `BraceStmtTree` for why this is necessary.
-      i = 2 and
+      // Then, we visit the body that potentially mutates the local variable.
+      i = 1 and
       result.asAstNode() = ast.getBody()
     }
   }

swift/ql/lib/codeql/swift/printast/Consistency.qll

@@ -0,0 +1,34 @@
+private import codeql.swift.printast.PrintAstNode
+
+query predicate doubleChildren(
+  PrintAstNode parent, int index, string label1, PrintAstNode child1, string label2,
+  PrintAstNode child2
+) {
+  child1 != child2 and
+  parent.hasChild(child1, index, label1) and
+  parent.hasChild(child2, index, label2)
+}
+
+query predicate doubleIndexes(
+  PrintAstNode parent, int index1, string label1, int index2, string label2, PrintAstNode child
+) {
+  index1 != index2 and
+  parent.hasChild(child, index1, label1) and
+  parent.hasChild(child, index2, label2)
+}
+
+query predicate doubleParents(
+  PrintAstNode parent1, string label1, PrintAstNode parent2, string label2, PrintAstNode child
+) {
+  parent1 != parent2 and
+  parent1.hasChild(child, _, label1) and
+  parent2.hasChild(child, _, label2)
+}
+
+private predicate isChildOf(PrintAstNode parent, PrintAstNode child) {
+  parent.hasChild(child, _, _)
+}
+
+query predicate parentChildLoops(PrintAstNode parent, PrintAstNode child) {
+  isChildOf(parent, child) and isChildOf*(child, parent)
+}

swift/ql/lib/codeql/swift/printast/PrintAstNode.qll

@@ -135,3 +135,20 @@ class PrintFunction extends PrintLocatable {
     key = "InterfaceType" and result = ast.getInterfaceType().toString()
   }
 }
+
+/**
+ * A specialization for `PatternBindingDecl`s that are backing a property wrapper, required as the
+ * `getInit` in these cases points to the same `Expr` that the `PatternBindingDecl` of the original
+ * variable. We therefore need to skip it to avoid double parents.
+ */
+class PrintPropertyWrapperPatternBindingDecl extends PrintLocatable {
+  override PatternBindingDecl ast;
+
+  PrintPropertyWrapperPatternBindingDecl() {
+    ast = any(VarDecl d).getPropertyWrapperBackingVarBinding()
+  }
+
+  override predicate hasChild(PrintAstNode child, int index, string label) {
+    PrintLocatable.super.hasChild(child, index, label) and label != "getInit(0)"
+  }
+}

swift/ql/test/extractor-tests/generated/expr/OpenExistentialExpr/OpenExistentialExpr.expected

@@ -0,0 +1 @@
+| open_existential.swift:14:5:14:19 | OpenExistentialExpr | hasType: | yes | getSubExpr: | open_existential.swift:14:5:14:19 | call to foo() | getExistential: | open_existential.swift:14:5:14:13 | call to createP() | getOpaqueExpr: | open_existential.swift:14:5:14:13 | OpaqueValueExpr |

swift/ql/test/extractor-tests/generated/expr/OpenExistentialExpr/OpenExistentialExpr_getType.expected

@@ -0,0 +1 @@
+| open_existential.swift:14:5:14:19 | OpenExistentialExpr | () |

swift/ql/test/extractor-tests/generated/expr/OpenExistentialExpr/open_existential.swift

@@ -0,0 +1,15 @@
+protocol P {
+    func foo() -> ()
+}
+
+class C : P {
+    func foo() {}
+}
+
+func createP() -> P {
+    return C()
+}
+
+func test() {
+    createP().foo()
+}