Merge pull request #14633 from github/redsun82/capture-list-vars

MathiasVP · web-flow · commit e75562e5084b · 2023-10-31T12:48:14.000+01:00
Swift: add `VarDecl` children to `CaptureListExpr`
diff --git a/swift/ql/.generated.list b/swift/ql/.generated.list
diff --git a/swift/ql/lib/change-notes/2023-10-31-capture-list-expr-variables.md b/swift/ql/lib/change-notes/2023-10-31-capture-list-expr-variables.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added indexed `getVariable` to `CaptureListExpr`, improving its AST printing and data flow.
diff --git a/swift/ql/lib/codeql/swift/elements/expr/CaptureListExpr.qll b/swift/ql/lib/codeql/swift/elements/expr/CaptureListExpr.qll
@@ -1,5 +1,11 @@
 private import codeql.swift.generated.expr.CaptureListExpr
+private import codeql.swift.elements.pattern.NamedPattern
 
 class CaptureListExpr extends Generated::CaptureListExpr {
   override string toString() { result = this.getClosureBody().toString() }
+
+  override VarDecl getVariable(int index) {
+    // all capture binding declarations consist of a single named pattern
+    result = this.getBindingDecl(index).getPattern(0).(NamedPattern).getVarDecl()
+  }
 }
diff --git a/swift/ql/lib/codeql/swift/generated/ParentChild.qll b/swift/ql/lib/codeql/swift/generated/ParentChild.qll
diff --git a/swift/ql/lib/codeql/swift/generated/expr/CaptureListExpr.qll b/swift/ql/lib/codeql/swift/generated/expr/CaptureListExpr.qll
diff --git a/swift/ql/test/extractor-tests/declarations/all.expected b/swift/ql/test/extractor-tests/declarations/all.expected
@@ -274,3 +274,29 @@
 | declarations.swift:182:7:182:7 | self |  |
 | declarations.swift:183:1:183:15 | A | getAliasedType:B |
 | declarations.swift:184:1:184:18 | C | getAliasedType:Int? |
+| declarations.swift:186:1:194:1 | S |  |
+| declarations.swift:186:7:186:7 | S.deinit() |  |
+| declarations.swift:186:7:186:7 | S.init() |  |
+| declarations.swift:186:7:186:7 | self |  |
+| declarations.swift:186:7:186:7 | self |  |
+| declarations.swift:187:3:187:13 | var ... = ... |  |
+| declarations.swift:187:7:187:7 | _modify |  |
+| declarations.swift:187:7:187:7 | bf1 |  |
+| declarations.swift:187:7:187:7 | get |  |
+| declarations.swift:187:7:187:7 | self |  |
+| declarations.swift:187:7:187:7 | self |  |
+| declarations.swift:187:7:187:7 | self |  |
+| declarations.swift:187:7:187:7 | set |  |
+| declarations.swift:187:7:187:7 | value |  |
+| declarations.swift:188:3:193:3 | captureThis() |  |
+| declarations.swift:188:8:188:8 | self |  |
+| declarations.swift:189:5:189:13 | var ... = ... |  |
+| declarations.swift:189:9:189:9 | x |  |
+| declarations.swift:190:5:192:5 | var ... = ... |  |
+| declarations.swift:190:9:190:9 | f |  |
+| declarations.swift:190:16:190:16 | self |  |
+| declarations.swift:190:16:190:16 | var ... = ... |  |
+| declarations.swift:190:22:190:22 | var ... = ... |  |
+| declarations.swift:190:22:190:22 | x |  |
+| declarations.swift:191:7:191:7 | self |  |
+| declarations.swift:191:18:191:18 | x |  |
diff --git a/swift/ql/test/extractor-tests/declarations/declarations.swift b/swift/ql/test/extractor-tests/declarations/declarations.swift
@@ -181,4 +181,14 @@ func ifConfig() {
 
 class B {}
 typealias A = B
-typealias C = Int?
+typealias C = Int?
+
+class S {
+  var bf1 = 0
+  func captureThis() {
+    var x = 0
+    var f = { [self, x] () in
+      self.bf1 = x
+    };
+  }
+}
diff --git a/swift/ql/test/extractor-tests/generated/decl/CapturedDecl/PrintAst.expected b/swift/ql/test/extractor-tests/generated/decl/CapturedDecl/PrintAst.expected
@@ -18,6 +18,8 @@ closures.swift:
 #    7|           getInit(0): [CallExpr] call to hello()
 #    7|             getFunction(): [DeclRefExpr] hello()
 #    7|           getPattern(0): [NamedPattern] x
+#    7|         getVariable(0): [ConcreteVarDecl] x
+#    7|             Type = String
 #    7|         getClosureBody(): [ExplicitClosureExpr] { ... }
 #    7|           getBody(): [BraceStmt] { ... }
 #    8|             getElement(0): [CallExpr] call to print(_:separator:terminator:)
@@ -44,8 +46,6 @@ closures.swift:
 #    9|                 getExpr(): [DefaultArgumentExpr] default terminator
 #    8|           getCapture(0): [CapturedDecl] x
 #    9|           getCapture(1): [CapturedDecl] y
-#    7| [ConcreteVarDecl] x
-#    7|     Type = String
 #   12| [TopLevelCodeDecl] { ... }
 #   12|   getBody(): [BraceStmt] { ... }
 #   12|     getElement(0): [PatternBindingDecl] var ... = ...
diff --git a/swift/ql/test/extractor-tests/updates/PrintAst.expected b/swift/ql/test/extractor-tests/updates/PrintAst.expected
@@ -214,6 +214,8 @@ v5.8.swift:
 #   37|             getInit(0): [DeclRefExpr] self
 #   37|             getInit(0).getFullyConverted(): [InjectIntoOptionalExpr] (ViewController?) ...
 #   37|             getPattern(0): [NamedPattern] self
+#   37|           getVariable(0): [ConcreteVarDecl] self
+#   37|               Type = ViewController?
 #   37|           getClosureBody(): [ExplicitClosureExpr] { ... }
 #   37|             getBody(): [BraceStmt] { ... }
 #   38|               getElement(0): [GuardStmt] guard ... else { ... }
@@ -248,8 +250,6 @@ v5.8.swift:
 #   33|         Type = ViewController
 #   33|     getBody(): [BraceStmt] { ... }
 #   33|       getElement(0): [ReturnStmt] return
-#   37| [ConcreteVarDecl] self
-#   37|     Type = ViewController?
 #   38| [ConcreteVarDecl] self
 #   38|     Type = ViewController
 #   39| [Comment] // refers to `self.dismiss()`
diff --git a/swift/ql/test/library-tests/ast/PrintAst.expected b/swift/ql/test/library-tests/ast/PrintAst.expected
@@ -2444,15 +2444,15 @@ cfg.swift:
 #  368|         getBindingDecl(1): [PatternBindingDecl] var ... = ...
 #  368|           getInit(0): [StringLiteralExpr] literal
 #  368|           getPattern(0): [NamedPattern] t
+#  368|         getVariable(0): [ConcreteVarDecl] z
+#  368|             Type = Int
+#  368|         getVariable(1): [ConcreteVarDecl] t
+#  368|             Type = String
 #  368|         getClosureBody(): [ExplicitClosureExpr] { ... }
 #  368|           getBody(): [BraceStmt] { ... }
 #  369|             getElement(0): [ReturnStmt] return ...
 #  369|               getResult(): [DeclRefExpr] z
 #  369|           getCapture(0): [CapturedDecl] z
-#  368| [ConcreteVarDecl] z
-#  368|     Type = Int
-#  368| [ConcreteVarDecl] t
-#  368|     Type = String
 #  373| [NamedFunction] testTupleElement(t:)
 #  373|     InterfaceType = ((a: Int, Int, c: Int)) -> Int
 #  373|   getParam(0): [ParamDecl] t
@@ -4543,6 +4543,86 @@ declarations.swift:
 #  182|       getElement(0): [ReturnStmt] return
 #  183| [TypeAliasDecl] A
 #  184| [TypeAliasDecl] C
+#  186| [ClassDecl] S
+#  187|   getMember(0): [PatternBindingDecl] var ... = ...
+#  187|     getInit(0): [IntegerLiteralExpr] 0
+#  187|     getPattern(0): [NamedPattern] bf1
+#  187|   getMember(1): [ConcreteVarDecl] bf1
+#  187|       Type = Int
+#  187|     getAccessor(0): [Accessor] get
+#  187|         InterfaceType = (S) -> () -> Int
+#  187|       getSelfParam(): [ParamDecl] self
+#  187|           Type = S
+#  187|       getBody(): [BraceStmt] { ... }
+#-----|         getElement(0): [ReturnStmt] return ...
+#-----|           getResult(): [MemberRefExpr] .bf1
+#-----|             getBase(): [DeclRefExpr] self
+#  187|     getAccessor(1): [Accessor] set
+#  187|         InterfaceType = (S) -> (Int) -> ()
+#  187|       getSelfParam(): [ParamDecl] self
+#  187|           Type = S
+#  187|       getParam(0): [ParamDecl] value
+#  187|           Type = Int
+#  187|       getBody(): [BraceStmt] { ... }
+#-----|         getElement(0): [AssignExpr]  ... = ...
+#-----|           getDest(): [MemberRefExpr] .bf1
+#-----|             getBase(): [DeclRefExpr] self
+#-----|           getSource(): [DeclRefExpr] value
+#  187|     getAccessor(2): [Accessor] _modify
+#  187|         InterfaceType = (S) -> () -> ()
+#  187|       getSelfParam(): [ParamDecl] self
+#  187|           Type = S
+#  187|       getBody(): [BraceStmt] { ... }
+#  187|         getElement(0): [YieldStmt] yield ...
+#-----|           getResult(0): [MemberRefExpr] .bf1
+#-----|             getBase(): [DeclRefExpr] self
+#-----|           getResult(0).getFullyConverted(): [InOutExpr] &...
+#  188|   getMember(2): [NamedFunction] captureThis()
+#  188|       InterfaceType = (S) -> () -> ()
+#  188|     getSelfParam(): [ParamDecl] self
+#  188|         Type = S
+#  188|     getBody(): [BraceStmt] { ... }
+#  189|       getVariable(0): [ConcreteVarDecl] x
+#  189|           Type = Int
+#  190|       getVariable(1): [ConcreteVarDecl] f
+#  190|           Type = () -> ()
+#  189|       getElement(0): [PatternBindingDecl] var ... = ...
+#  189|         getInit(0): [IntegerLiteralExpr] 0
+#  189|         getPattern(0): [NamedPattern] x
+#  190|       getElement(1): [PatternBindingDecl] var ... = ...
+#  190|         getInit(0): [CaptureListExpr] { ... }
+#  190|           getBindingDecl(0): [PatternBindingDecl] var ... = ...
+#  190|             getInit(0): [DeclRefExpr] self
+#  190|             getPattern(0): [NamedPattern] self
+#  190|           getBindingDecl(1): [PatternBindingDecl] var ... = ...
+#  190|             getInit(0): [DeclRefExpr] x
+#  190|             getInit(0).getFullyConverted(): [LoadExpr] (Int) ...
+#  190|             getPattern(0): [NamedPattern] x
+#  190|           getVariable(0): [ConcreteVarDecl] self
+#  190|               Type = S
+#  190|           getVariable(1): [ConcreteVarDecl] x
+#  190|               Type = Int
+#  190|           getClosureBody(): [ExplicitClosureExpr] { ... }
+#  190|             getBody(): [BraceStmt] { ... }
+#  191|               getElement(0): [ReturnStmt] return ...
+#  191|                 getResult(): [AssignExpr]  ... = ...
+#  191|                   getDest(): [MemberRefExpr] .bf1
+#  191|                     getBase(): [DeclRefExpr] self
+#  191|                   getSource(): [DeclRefExpr] x
+#  191|             getCapture(0): [CapturedDecl] self
+#  191|             getCapture(1): [CapturedDecl] x
+#  190|         getPattern(0): [NamedPattern] f
+#  186|   getMember(3): [Deinitializer] S.deinit()
+#  186|       InterfaceType = (S) -> () -> ()
+#  186|     getSelfParam(): [ParamDecl] self
+#  186|         Type = S
+#  186|     getBody(): [BraceStmt] { ... }
+#  186|   getMember(4): [Initializer] S.init()
+#  186|       InterfaceType = (S.Type) -> () -> S
+#  186|     getSelfParam(): [ParamDecl] self
+#  186|         Type = S
+#  186|     getBody(): [BraceStmt] { ... }
+#  186|       getElement(0): [ReturnStmt] return
 expressions.swift:
 #    1| [TopLevelCodeDecl] { ... }
 #    1|   getBody(): [BraceStmt] { ... }
diff --git a/swift/ql/test/library-tests/ast/declarations.swift b/swift/ql/test/library-tests/ast/declarations.swift
@@ -181,4 +181,14 @@ func ifConfig() {
 
 class B {}
 typealias A = B
-typealias C = Int?
+typealias C = Int?
+
+class S {
+  var bf1 = 0
+  func captureThis() {
+    var x = 0
+    var f = { [self, x] () in
+      self.bf1 = x
+    };
+  }
+}
diff --git a/swift/ql/test/library-tests/dataflow/capture/closures.swift b/swift/ql/test/library-tests/dataflow/capture/closures.swift
@@ -177,8 +177,8 @@ class S {
 
     f(source("captureThis", 2));
 
-    sink(bf1); // $ MISSING: hasValueFlow
-    sink(self.bf2); // $ MISSING: hasValueFlow
+    sink(bf1); // $ MISSING: hasValueFlow=captureThis
+    sink(self.bf2); // $ MISSING: hasValueFlow=captureThis
   }
 }
 
@@ -188,4 +188,4 @@ func multi() {
   var f = { () in x = y }
   f()
   sink(x) // $ hasValueFlow=multi
-}
+}
diff --git a/swift/schema.py b/swift/schema.py
@@ -411,6 +411,9 @@ class CapturedDecl(Decl):
 
 class CaptureListExpr(Expr):
     binding_decls: list[PatternBindingDecl] | child
+    variables: list[VarDecl] | child | synth | desc("""
+        These are the variables introduced by this capture in the closure's scope, not the captured ones.
+    """)
     closure_body: "ClosureExpr" | child
 
 class CollectionExpr(Expr):

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* Added indexed `getVariable` to `CaptureListExpr`, improving its AST printing and data flow.