From efdb4a6d82845fe0ef1ccaaf6cda4ac2d1bcdcd1 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 27 Mar 2025 10:11:11 +0000
Subject: [PATCH 01/14] Use global dataflow for loop variable capture

---
 .../ql/src/Variables/LoopVariableCapture.ql   | 66 ++++++++++++-------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture.ql
index e4ccd73e5e1c..16130d5d50f5 100644
--- a/python/ql/src/Variables/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture.ql
@@ -10,38 +10,60 @@
  */
 
 import python
+import semmle.python.dataflow.new.DataFlow
 
-// Gets the scope of the iteration variable of the looping scope
-Scope iteration_variable_scope(AstNode loop) {
-  result = loop.(For).getScope()
-  or
-  result = loop.(Comp).getFunction()
+abstract class Loop extends AstNode {
+  abstract Variable getALoopVariable();
 }
 
-predicate capturing_looping_construct(CallableExpr capturing, AstNode loop, Variable var) {
-  var.getScope() = iteration_variable_scope(loop) and
+class ForLoop extends Loop, For {
+  override Variable getALoopVariable() {
+    this.getTarget() = result.getAnAccess().getParentNode*() and
+    result.getScope() = this.getScope()
+  }
+}
+
+predicate capturesLoopVariable(CallableExpr capturing, Loop loop, Variable var) {
   var.getAnAccess().getScope() = capturing.getInnerScope() and
   capturing.getParentNode+() = loop and
-  (
-    loop.(For).getTarget() = var.getAnAccess()
-    or
-    var = loop.(Comp).getAnIterationVariable()
-  )
+  var = loop.getALoopVariable()
 }
 
-predicate escaping_capturing_looping_construct(CallableExpr capturing, AstNode loop, Variable var) {
-  capturing_looping_construct(capturing, loop, var) and
-  // Escapes if used out side of for loop or is a lambda in a comprehension
-  (
-    loop instanceof For and
-    exists(Expr e | e.pointsTo(_, _, capturing) | not loop.contains(e))
+module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { capturesLoopVariable(node.asExpr(), _, _) }
+
+  predicate isSink(DataFlow::Node node) {
+    // Stored in a field.
+    exists(DataFlow::AttrWrite aw | aw.getObject() = node)
+    or
+    // Stored in a dict/list.
+    exists(Assign assign, Subscript sub |
+      sub = assign.getATarget() and node.asExpr() = assign.getValue()
+    )
     or
-    loop.(Comp).getElt() = capturing
+    // Stored in a list.
+    exists(DataFlow::MethodCallNode mc | mc.calls(_, "append") and node = mc.getArg(0))
     or
-    loop.(Comp).getElt().(Tuple).getAnElt() = capturing
-  )
+    // Used in a yeild statement, likely included in a collection.
+    // The element of comprehension expressions desugar to involve a yield statement internally.
+    exists(Yield y | node.asExpr() = y.getValue())
+  }
+
+  predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+    isSink(node) and
+    exists(cs)
+  }
+}
+
+module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowSig>;
+
+predicate escapingCapture(CallableExpr capturing, Loop loop, Variable var) {
+  capturesLoopVariable(capturing, loop, var) and
+  EscapingCaptureFlow::flow(DataFlow::exprNode(capturing), _)
 }
 
 from CallableExpr capturing, AstNode loop, Variable var
-where escaping_capturing_looping_construct(capturing, loop, var)
+where escapingCapture(capturing, loop, var)
 select capturing, "Capture of loop variable $@.", loop, var.getId()

From 08b428118790ee14875d4f8ea1d59e12142ae94c Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 31 Mar 2025 10:23:12 +0100
Subject: [PATCH 02/14] Update query message and remove field case

---
 python/ql/src/Variables/LoopVariableCapture.ql | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture.ql
index 16130d5d50f5..4682e682d815 100644
--- a/python/ql/src/Variables/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture.ql
@@ -34,8 +34,9 @@ module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node node) {
     // Stored in a field.
-    exists(DataFlow::AttrWrite aw | aw.getObject() = node)
-    or
+    // This appeared to lead to FPs through wrapper classes.
+    // exists(DataFlow::AttrWrite aw | aw.getObject() = node)
+    // or
     // Stored in a dict/list.
     exists(Assign assign, Subscript sub |
       sub = assign.getATarget() and node.asExpr() = assign.getValue()
@@ -44,7 +45,7 @@ module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
     // Stored in a list.
     exists(DataFlow::MethodCallNode mc | mc.calls(_, "append") and node = mc.getArg(0))
     or
-    // Used in a yeild statement, likely included in a collection.
+    // Used in a yield statement, likely included in a collection.
     // The element of comprehension expressions desugar to involve a yield statement internally.
     exists(Yield y | node.asExpr() = y.getValue())
   }
@@ -64,6 +65,8 @@ predicate escapingCapture(CallableExpr capturing, Loop loop, Variable var) {
   EscapingCaptureFlow::flow(DataFlow::exprNode(capturing), _)
 }
 
-from CallableExpr capturing, AstNode loop, Variable var
-where escapingCapture(capturing, loop, var)
-select capturing, "Capture of loop variable $@.", loop, var.getId()
+from CallableExpr capturing, AstNode loop, Variable var, string descr
+where
+  escapingCapture(capturing, loop, var) and
+  if capturing instanceof Lambda then descr = "lambda" else descr = "function"
+select capturing, "This " + descr + " captures the loop variable $@.", loop, var.getId()

From 5b7200a0418ca7a86a8ed863de755cd355c44634 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 31 Mar 2025 11:04:13 +0100
Subject: [PATCH 03/14] Use flow path in alerts

---
 .../ql/src/Variables/LoopVariableCapture.ql   | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture.ql
index 4682e682d815..e0a3d5211445 100644
--- a/python/ql/src/Variables/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture.ql
@@ -1,7 +1,7 @@
 /**
  * @name Loop variable capture
  * @description Capture of a loop variable is not the same as capturing the value of a loop variable, and may be erroneous.
- * @kind problem
+ * @kind path-problem
  * @tags correctness
  * @problem.severity error
  * @sub-severity low
@@ -60,13 +60,23 @@ module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
 
 module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowSig>;
 
-predicate escapingCapture(CallableExpr capturing, Loop loop, Variable var) {
+import EscapingCaptureFlow::PathGraph
+
+predicate escapingCapture(
+  CallableExpr capturing, Loop loop, Variable var, EscapingCaptureFlow::PathNode source,
+  EscapingCaptureFlow::PathNode sink
+) {
   capturesLoopVariable(capturing, loop, var) and
-  EscapingCaptureFlow::flow(DataFlow::exprNode(capturing), _)
+  capturing = source.getNode().asExpr() and
+  EscapingCaptureFlow::flowPath(source, sink)
 }
 
-from CallableExpr capturing, AstNode loop, Variable var, string descr
+from
+  CallableExpr capturing, AstNode loop, Variable var, string descr,
+  EscapingCaptureFlow::PathNode source, EscapingCaptureFlow::PathNode sink
 where
-  escapingCapture(capturing, loop, var) and
+  escapingCapture(capturing, loop, var, source, sink) and
   if capturing instanceof Lambda then descr = "lambda" else descr = "function"
-select capturing, "This " + descr + " captures the loop variable $@.", loop, var.getId()
+select capturing, source, sink,
+  "This " + descr + " captures the loop variable $@, and may escape the loop by being stored $@.",
+  loop, var.getId(), sink, "here"

From 11830bf66132f39c9df6567ef71193ac98213a4b Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 31 Mar 2025 14:33:55 +0100
Subject: [PATCH 04/14] Move to separate folder

---
 .../{ => LoopVariableCapture}/LoopVariableCapture.py      | 0
 .../{ => LoopVariableCapture}/LoopVariableCapture.qhelp   | 0
 .../{ => LoopVariableCapture}/LoopVariableCapture.ql      | 8 ++++++++
 .../{ => LoopVariableCapture}/LoopVariableCapture2.py     | 0
 4 files changed, 8 insertions(+)
 rename python/ql/src/Variables/{ => LoopVariableCapture}/LoopVariableCapture.py (100%)
 rename python/ql/src/Variables/{ => LoopVariableCapture}/LoopVariableCapture.qhelp (100%)
 rename python/ql/src/Variables/{ => LoopVariableCapture}/LoopVariableCapture.ql (91%)
 rename python/ql/src/Variables/{ => LoopVariableCapture}/LoopVariableCapture2.py (100%)

diff --git a/python/ql/src/Variables/LoopVariableCapture.py b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py
similarity index 100%
rename from python/ql/src/Variables/LoopVariableCapture.py
rename to python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py
diff --git a/python/ql/src/Variables/LoopVariableCapture.qhelp b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
similarity index 100%
rename from python/ql/src/Variables/LoopVariableCapture.qhelp
rename to python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
diff --git a/python/ql/src/Variables/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
similarity index 91%
rename from python/ql/src/Variables/LoopVariableCapture.ql
rename to python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index e0a3d5211445..b2ed802e9c39 100644
--- a/python/ql/src/Variables/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -52,6 +52,14 @@ module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
 
   predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
 
+  predicate isBarrier(DataFlow::Node node) {
+    // Incorrect virtual dispatch to __call__ methods is a source of FPs.
+    exists(Function call |
+      call.getName() = "__call__" and
+      call.getArg(0) = node.(DataFlow::ParameterNode).getParameter()
+    )
+  }
+
   predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
     isSink(node) and
     exists(cs)
diff --git a/python/ql/src/Variables/LoopVariableCapture2.py b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture2.py
similarity index 100%
rename from python/ql/src/Variables/LoopVariableCapture2.py
rename to python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture2.py

From 2d6476ad2138138c6fa1ac3b5fb6127254496aa4 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 1 Apr 2025 09:56:01 +0100
Subject: [PATCH 05/14] Update names and alert message

---
 .../Variables/LoopVariableCapture/LoopVariableCapture.ql  | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index b2ed802e9c39..6c979ffdf828 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -29,7 +29,7 @@ predicate capturesLoopVariable(CallableExpr capturing, Loop loop, Variable var)
   var = loop.getALoopVariable()
 }
 
-module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
+module EscapingCaptureFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node node) { capturesLoopVariable(node.asExpr(), _, _) }
 
   predicate isSink(DataFlow::Node node) {
@@ -66,7 +66,7 @@ module EscapingCaptureFlowSig implements DataFlow::ConfigSig {
   }
 }
 
-module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowSig>;
+module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowConfig>;
 
 import EscapingCaptureFlow::PathGraph
 
@@ -86,5 +86,5 @@ where
   escapingCapture(capturing, loop, var, source, sink) and
   if capturing instanceof Lambda then descr = "lambda" else descr = "function"
 select capturing, source, sink,
-  "This " + descr + " captures the loop variable $@, and may escape the loop by being stored $@.",
-  loop, var.getId(), sink, "here"
+  "This " + descr + " captures the loop variable $@, and may escape the loop by being stored at $@.",
+  loop, var.getId(), sink, "this location"

From c37809a187bf948186b23d548471931a398cc808 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 2 Apr 2025 09:33:30 +0100
Subject: [PATCH 06/14] Reduce scope of allowImplicitRead to avoid cartesian
 product.

---
 .../Variables/LoopVariableCapture/LoopVariableCapture.ql   | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index 6c979ffdf828..49392e3d01e7 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -62,7 +62,12 @@ module EscapingCaptureFlowConfig implements DataFlow::ConfigSig {
 
   predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
     isSink(node) and
-    exists(cs)
+    (
+      cs instanceof DataFlow::TupleElementContent or
+      cs instanceof DataFlow::ListElementContent or
+      cs instanceof DataFlow::SetElementContent or
+      cs instanceof DataFlow::DictionaryElementAnyContent
+    )
   }
 }
 

From adfe89fadc4b2f83a9d5c34ab8cd3e4cd3746ccc Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 4 Apr 2025 09:47:21 +0100
Subject: [PATCH 07/14] Update test output

---
 .../capture/LoopVariableCapture.expected      | 34 ++++++++++++++-----
 .../capture/LoopVariableCapture.qlref         |  2 +-
 .../query-tests/Variables/capture/test.py     | 19 ++++++-----
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
index 8fd40c120a62..d7e15f44e6c2 100644
--- a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
+++ b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
@@ -1,8 +1,26 @@
-| test.py:5:9:5:20 | FunctionExpr | Capture of loop variable $@. | test.py:4:5:4:23 | For | x |
-| test.py:10:6:10:14 | Lambda | Capture of loop variable $@. | test.py:10:5:10:36 | ListComp | i |
-| test.py:42:6:42:14 | Lambda | Capture of loop variable $@. | test.py:42:5:42:56 | ListComp | i |
-| test.py:43:6:43:14 | Lambda | Capture of loop variable $@. | test.py:43:5:43:56 | ListComp | j |
-| test.py:45:6:45:14 | Lambda | Capture of loop variable $@. | test.py:45:5:45:36 | SetComp | i |
-| test.py:49:8:49:16 | Lambda | Capture of loop variable $@. | test.py:49:5:49:38 | DictComp | i |
-| test.py:57:6:57:14 | Lambda | Capture of loop variable $@. | test.py:57:6:57:35 | GeneratorExp | i |
-| test.py:62:10:62:18 | Lambda | Capture of loop variable $@. | test.py:62:10:62:39 | GeneratorExp | i |
+edges
+| test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | test.py:5:13:5:17 | ControlFlowNode for inner | provenance |  |
+| test.py:5:13:5:17 | ControlFlowNode for inner | test.py:7:20:7:24 | ControlFlowNode for inner | provenance |  |
+| test.py:49:8:49:16 | ControlFlowNode for Lambda | test.py:49:6:49:16 | ControlFlowNode for Tuple | provenance |  |
+nodes
+| test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | semmle.label | ControlFlowNode for FunctionExpr |
+| test.py:5:13:5:17 | ControlFlowNode for inner | semmle.label | ControlFlowNode for inner |
+| test.py:7:20:7:24 | ControlFlowNode for inner | semmle.label | ControlFlowNode for inner |
+| test.py:10:6:10:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:42:6:42:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:43:6:43:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:45:6:45:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:49:6:49:16 | ControlFlowNode for Tuple | semmle.label | ControlFlowNode for Tuple |
+| test.py:49:8:49:16 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:57:6:57:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+| test.py:62:10:62:18 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
+subpaths
+#select
+| test.py:5:9:5:20 | FunctionExpr | test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | test.py:7:20:7:24 | ControlFlowNode for inner | This function captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:4:5:4:23 | For | x | test.py:7:20:7:24 | ControlFlowNode for inner | this location |
+| test.py:10:6:10:14 | Lambda | test.py:10:6:10:14 | ControlFlowNode for Lambda | test.py:10:6:10:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:10:5:10:36 | For | i | test.py:10:6:10:14 | ControlFlowNode for Lambda | this location |
+| test.py:42:6:42:14 | Lambda | test.py:42:6:42:14 | ControlFlowNode for Lambda | test.py:42:6:42:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:42:5:42:56 | For | i | test.py:42:6:42:14 | ControlFlowNode for Lambda | this location |
+| test.py:43:6:43:14 | Lambda | test.py:43:6:43:14 | ControlFlowNode for Lambda | test.py:43:6:43:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:43:5:43:56 | For | j | test.py:43:6:43:14 | ControlFlowNode for Lambda | this location |
+| test.py:45:6:45:14 | Lambda | test.py:45:6:45:14 | ControlFlowNode for Lambda | test.py:45:6:45:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:45:5:45:36 | For | i | test.py:45:6:45:14 | ControlFlowNode for Lambda | this location |
+| test.py:49:8:49:16 | Lambda | test.py:49:8:49:16 | ControlFlowNode for Lambda | test.py:49:6:49:16 | ControlFlowNode for Tuple | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:49:5:49:38 | For | i | test.py:49:6:49:16 | ControlFlowNode for Tuple | this location |
+| test.py:57:6:57:14 | Lambda | test.py:57:6:57:14 | ControlFlowNode for Lambda | test.py:57:6:57:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:57:6:57:35 | For | i | test.py:57:6:57:14 | ControlFlowNode for Lambda | this location |
+| test.py:62:10:62:18 | Lambda | test.py:62:10:62:18 | ControlFlowNode for Lambda | test.py:62:10:62:18 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:62:10:62:39 | For | i | test.py:62:10:62:18 | ControlFlowNode for Lambda | this location |
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
index 1e2a71cd6a71..05bfcbd94204 100644
--- a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
+++ b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
@@ -1 +1 @@
-Variables/LoopVariableCapture.ql
+Variables/LoopVariableCapture/LoopVariableCapture.ql
diff --git a/python/ql/test/query-tests/Variables/capture/test.py b/python/ql/test/query-tests/Variables/capture/test.py
index 480bc58862e0..5ffccc7a9e4d 100644
--- a/python/ql/test/query-tests/Variables/capture/test.py
+++ b/python/ql/test/query-tests/Variables/capture/test.py
@@ -2,12 +2,12 @@
 def bad1():
     results = []
     for x in range(10):
-        def inner():
+        def inner(): # $capturedVar=x
             return x
     results.append(inner)
     return results
 
-a = [lambda: i for i in range(1, 4)]
+a = [lambda: i for i in range(1, 4)] # $capturedVar=a
 for f in a:
     print(f())
 
@@ -39,14 +39,14 @@ def inner():
         result += inner()
     return result
 
-b = [lambda: i for i in range(1, 4) for j in range(1,5)]
-c = [lambda: j for i in range(1, 4) for j in range(1,5)]
+b = [lambda: i for i in range(1, 4) for j in range(1,5)] # $capturedVar=i
+c = [lambda: j for i in range(1, 4) for j in range(1,5)] # $capturedVar=j
 
-s = {lambda: i for i in range(1, 4)}
+s = {lambda: i for i in range(1, 4)} # $capturedVar=i
 for f in s:
     print(f())
 
-d = {i:lambda: i for i in range(1, 4)}
+d = {i:lambda: i for i in range(1, 4)} # $capturedVar=d
 for k, f in d.items():
     print(k, f())
 
@@ -54,14 +54,15 @@ def inner():
 #When the captured variable is used.
 #So technically this is a false positive, but it is extremely fragile
 #code, so I (Mark) think it is fine to report it as a violation.
-g = (lambda: i for i in range(1, 4))
+g = (lambda: i for i in range(1, 4)) # $capturedVar=i
 for f in g:
     print(f())
 
 #But not if evaluated eagerly
-l = list(lambda: i for i in range(1, 4))
+l = list(lambda: i for i in range(1, 4)) # $capturedVar=i
 for f in l:
     print(f())
 
+# This result is MISSING since the lambda is not detected to escape the loop
 def odasa4860(asset_ids):
-    return dict((asset_id, filter(lambda c : c.asset_id == asset_id, xxx)) for asset_id in asset_ids)
+    return dict((asset_id, filter(lambda c : c.asset_id == asset_id, xxx)) for asset_id in asset_ids) # $MISSING: capturedVar=asset_id

From 9fb1c312067e50c29010947d0b19de7178c08a3d Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 4 Apr 2025 10:13:39 +0100
Subject: [PATCH 08/14] Update tests to inline expectations

---
 .../LoopVariableCapture.ql                    | 72 +----------------
 .../LoopVariableCaptureQuery.qll              | 81 +++++++++++++++++++
 .../capture/LoopVariableCapture.expected      | 26 ------
 .../capture/LoopVariableCapture.qlref         |  1 -
 .../capture/LoopVariableCaptureTest.expected  |  0
 .../capture/LoopVariableCaptureTest.ql        | 19 +++++
 .../query-tests/Variables/capture/test.py     | 15 +++-
 7 files changed, 112 insertions(+), 102 deletions(-)
 create mode 100644 python/ql/src/Variables/LoopVariableCapture/LoopVariableCaptureQuery.qll
 delete mode 100644 python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
 delete mode 100644 python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
 create mode 100644 python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.expected
 create mode 100644 python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index 49392e3d01e7..9c8822a3f6b2 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -10,80 +10,10 @@
  */
 
 import python
+import LoopVariableCaptureQuery
 import semmle.python.dataflow.new.DataFlow
-
-abstract class Loop extends AstNode {
-  abstract Variable getALoopVariable();
-}
-
-class ForLoop extends Loop, For {
-  override Variable getALoopVariable() {
-    this.getTarget() = result.getAnAccess().getParentNode*() and
-    result.getScope() = this.getScope()
-  }
-}
-
-predicate capturesLoopVariable(CallableExpr capturing, Loop loop, Variable var) {
-  var.getAnAccess().getScope() = capturing.getInnerScope() and
-  capturing.getParentNode+() = loop and
-  var = loop.getALoopVariable()
-}
-
-module EscapingCaptureFlowConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node node) { capturesLoopVariable(node.asExpr(), _, _) }
-
-  predicate isSink(DataFlow::Node node) {
-    // Stored in a field.
-    // This appeared to lead to FPs through wrapper classes.
-    // exists(DataFlow::AttrWrite aw | aw.getObject() = node)
-    // or
-    // Stored in a dict/list.
-    exists(Assign assign, Subscript sub |
-      sub = assign.getATarget() and node.asExpr() = assign.getValue()
-    )
-    or
-    // Stored in a list.
-    exists(DataFlow::MethodCallNode mc | mc.calls(_, "append") and node = mc.getArg(0))
-    or
-    // Used in a yield statement, likely included in a collection.
-    // The element of comprehension expressions desugar to involve a yield statement internally.
-    exists(Yield y | node.asExpr() = y.getValue())
-  }
-
-  predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
-
-  predicate isBarrier(DataFlow::Node node) {
-    // Incorrect virtual dispatch to __call__ methods is a source of FPs.
-    exists(Function call |
-      call.getName() = "__call__" and
-      call.getArg(0) = node.(DataFlow::ParameterNode).getParameter()
-    )
-  }
-
-  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
-    isSink(node) and
-    (
-      cs instanceof DataFlow::TupleElementContent or
-      cs instanceof DataFlow::ListElementContent or
-      cs instanceof DataFlow::SetElementContent or
-      cs instanceof DataFlow::DictionaryElementAnyContent
-    )
-  }
-}
-
-module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowConfig>;
-
 import EscapingCaptureFlow::PathGraph
 
-predicate escapingCapture(
-  CallableExpr capturing, Loop loop, Variable var, EscapingCaptureFlow::PathNode source,
-  EscapingCaptureFlow::PathNode sink
-) {
-  capturesLoopVariable(capturing, loop, var) and
-  capturing = source.getNode().asExpr() and
-  EscapingCaptureFlow::flowPath(source, sink)
-}
-
 from
   CallableExpr capturing, AstNode loop, Variable var, string descr,
   EscapingCaptureFlow::PathNode source, EscapingCaptureFlow::PathNode sink
diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCaptureQuery.qll b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCaptureQuery.qll
new file mode 100644
index 000000000000..ed0c64961413
--- /dev/null
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCaptureQuery.qll
@@ -0,0 +1,81 @@
+/** Definitions for reasoning about loop variable capture issues. */
+
+import python
+import semmle.python.dataflow.new.DataFlow
+
+/** A looping construct. */
+abstract class Loop extends AstNode {
+  /**
+   * Gets a loop variable of this loop.
+   * For example, `x` and `y` in `for x,y in pairs: print(x+y)`
+   */
+  abstract Variable getALoopVariable();
+}
+
+/** A `for` loop. */
+private class ForLoop extends Loop, For {
+  override Variable getALoopVariable() {
+    this.getTarget() = result.getAnAccess().getParentNode*() and
+    result.getScope() = this.getScope()
+  }
+}
+
+/** Holds if the callable `capturing` captures the variable `var` from the loop `loop`. */
+predicate capturesLoopVariable(CallableExpr capturing, Loop loop, Variable var) {
+  var.getAnAccess().getScope() = capturing.getInnerScope() and
+  capturing.getParentNode+() = loop and
+  var = loop.getALoopVariable()
+}
+
+/** Dataflow configuration for reasoning about callables that capture a loop variable and then may escape from the loop. */
+module EscapingCaptureFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { capturesLoopVariable(node.asExpr(), _, _) }
+
+  predicate isSink(DataFlow::Node node) {
+    // Stored in a dict/list.
+    exists(Assign assign, Subscript sub |
+      sub = assign.getATarget() and node.asExpr() = assign.getValue()
+    )
+    or
+    // Stored in a list.
+    exists(DataFlow::MethodCallNode mc | mc.calls(_, "append") and node = mc.getArg(0))
+    or
+    // Used in a yield statement, likely included in a collection.
+    // The element of comprehension expressions desugar to involve a yield statement internally.
+    exists(Yield y | node.asExpr() = y.getValue())
+    // Checks for storing in a field leads to false positives, so are omitted.
+  }
+
+  predicate isBarrierOut(DataFlow::Node node) { isSink(node) }
+
+  predicate isBarrier(DataFlow::Node node) {
+    // Incorrect virtual dispatch to __call__ methods is a source of FPs.
+    exists(Function call |
+      call.getName() = "__call__" and
+      call.getArg(0) = node.(DataFlow::ParameterNode).getParameter()
+    )
+  }
+
+  predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
+    isSink(node) and
+    (
+      cs instanceof DataFlow::TupleElementContent or
+      cs instanceof DataFlow::ListElementContent or
+      cs instanceof DataFlow::SetElementContent or
+      cs instanceof DataFlow::DictionaryElementAnyContent
+    )
+  }
+}
+
+/** Dataflow for reasoning about callables that capture a loop variable and then escape from the loop. */
+module EscapingCaptureFlow = DataFlow::Global<EscapingCaptureFlowConfig>;
+
+/** Holds if `capturing` is a callable that captures the variable `var` of the loop `loop`, and then may escape the loop via a flow path from `source` to `sink`. */
+predicate escapingCapture(
+  CallableExpr capturing, Loop loop, Variable var, EscapingCaptureFlow::PathNode source,
+  EscapingCaptureFlow::PathNode sink
+) {
+  capturesLoopVariable(capturing, loop, var) and
+  capturing = source.getNode().asExpr() and
+  EscapingCaptureFlow::flowPath(source, sink)
+}
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
deleted file mode 100644
index d7e15f44e6c2..000000000000
--- a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.expected
+++ /dev/null
@@ -1,26 +0,0 @@
-edges
-| test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | test.py:5:13:5:17 | ControlFlowNode for inner | provenance |  |
-| test.py:5:13:5:17 | ControlFlowNode for inner | test.py:7:20:7:24 | ControlFlowNode for inner | provenance |  |
-| test.py:49:8:49:16 | ControlFlowNode for Lambda | test.py:49:6:49:16 | ControlFlowNode for Tuple | provenance |  |
-nodes
-| test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | semmle.label | ControlFlowNode for FunctionExpr |
-| test.py:5:13:5:17 | ControlFlowNode for inner | semmle.label | ControlFlowNode for inner |
-| test.py:7:20:7:24 | ControlFlowNode for inner | semmle.label | ControlFlowNode for inner |
-| test.py:10:6:10:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:42:6:42:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:43:6:43:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:45:6:45:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:49:6:49:16 | ControlFlowNode for Tuple | semmle.label | ControlFlowNode for Tuple |
-| test.py:49:8:49:16 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:57:6:57:14 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-| test.py:62:10:62:18 | ControlFlowNode for Lambda | semmle.label | ControlFlowNode for Lambda |
-subpaths
-#select
-| test.py:5:9:5:20 | FunctionExpr | test.py:5:9:5:20 | ControlFlowNode for FunctionExpr | test.py:7:20:7:24 | ControlFlowNode for inner | This function captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:4:5:4:23 | For | x | test.py:7:20:7:24 | ControlFlowNode for inner | this location |
-| test.py:10:6:10:14 | Lambda | test.py:10:6:10:14 | ControlFlowNode for Lambda | test.py:10:6:10:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:10:5:10:36 | For | i | test.py:10:6:10:14 | ControlFlowNode for Lambda | this location |
-| test.py:42:6:42:14 | Lambda | test.py:42:6:42:14 | ControlFlowNode for Lambda | test.py:42:6:42:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:42:5:42:56 | For | i | test.py:42:6:42:14 | ControlFlowNode for Lambda | this location |
-| test.py:43:6:43:14 | Lambda | test.py:43:6:43:14 | ControlFlowNode for Lambda | test.py:43:6:43:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:43:5:43:56 | For | j | test.py:43:6:43:14 | ControlFlowNode for Lambda | this location |
-| test.py:45:6:45:14 | Lambda | test.py:45:6:45:14 | ControlFlowNode for Lambda | test.py:45:6:45:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:45:5:45:36 | For | i | test.py:45:6:45:14 | ControlFlowNode for Lambda | this location |
-| test.py:49:8:49:16 | Lambda | test.py:49:8:49:16 | ControlFlowNode for Lambda | test.py:49:6:49:16 | ControlFlowNode for Tuple | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:49:5:49:38 | For | i | test.py:49:6:49:16 | ControlFlowNode for Tuple | this location |
-| test.py:57:6:57:14 | Lambda | test.py:57:6:57:14 | ControlFlowNode for Lambda | test.py:57:6:57:14 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:57:6:57:35 | For | i | test.py:57:6:57:14 | ControlFlowNode for Lambda | this location |
-| test.py:62:10:62:18 | Lambda | test.py:62:10:62:18 | ControlFlowNode for Lambda | test.py:62:10:62:18 | ControlFlowNode for Lambda | This lambda captures the loop variable $@, and may escape the loop by being stored at $@. | test.py:62:10:62:39 | For | i | test.py:62:10:62:18 | ControlFlowNode for Lambda | this location |
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref b/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
deleted file mode 100644
index 05bfcbd94204..000000000000
--- a/python/ql/test/query-tests/Variables/capture/LoopVariableCapture.qlref
+++ /dev/null
@@ -1 +0,0 @@
-Variables/LoopVariableCapture/LoopVariableCapture.ql
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.expected b/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.expected
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql b/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql
new file mode 100644
index 000000000000..792fa2159cbe
--- /dev/null
+++ b/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql
@@ -0,0 +1,19 @@
+import python
+import Variables.LoopVariableCapture.LoopVariableCaptureQuery
+import utils.test.InlineExpectationsTest
+
+module MethodArgTest implements TestSig {
+  string getARelevantTag() { result = ["capturedVar"] }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(CallableExpr capturing, AstNode loop, Variable var |
+      escapingCapture(capturing, loop, var, _, _) and
+      element = capturing.toString() and
+      location = capturing.getLocation() and
+      tag = "capturedVar" and
+      value = var.getId()
+    )
+  }
+}
+
+import MakeTest<MethodArgTest>
diff --git a/python/ql/test/query-tests/Variables/capture/test.py b/python/ql/test/query-tests/Variables/capture/test.py
index 5ffccc7a9e4d..186a19b75705 100644
--- a/python/ql/test/query-tests/Variables/capture/test.py
+++ b/python/ql/test/query-tests/Variables/capture/test.py
@@ -4,10 +4,10 @@ def bad1():
     for x in range(10):
         def inner(): # $capturedVar=x
             return x
-    results.append(inner)
+        results.append(inner)
     return results
 
-a = [lambda: i for i in range(1, 4)] # $capturedVar=a
+a = [lambda: i for i in range(1, 4)] # $capturedVar=i
 for f in a:
     print(f())
 
@@ -18,7 +18,14 @@ def good1():
     for y in range(10):
         def inner(y=y):
             return y
-    results.append(inner)
+        results.append(inner)
+    return results
+
+# OK: Using default argument.
+def good2():
+    results = []
+    for y in range(10):
+        results.append(lambda y=y: y)
     return results
 
 #Factory function
@@ -46,7 +53,7 @@ def inner():
 for f in s:
     print(f())
 
-d = {i:lambda: i for i in range(1, 4)} # $capturedVar=d
+d = {i:lambda: i for i in range(1, 4)} # $capturedVar=i
 for k, f in d.items():
     print(k, f())
 

From b5805503fe34f76a6e31ec0fd7ad53d437488ad1 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 4 Apr 2025 11:56:07 +0100
Subject: [PATCH 09/14] Cleanups

---
 .../Variables/LoopVariableCapture/LoopVariableCapture.ql    | 1 -
 .../Variables/capture/LoopVariableCaptureTest.ql            | 6 +++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index 9c8822a3f6b2..9c41e52cc45c 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -11,7 +11,6 @@
 
 import python
 import LoopVariableCaptureQuery
-import semmle.python.dataflow.new.DataFlow
 import EscapingCaptureFlow::PathGraph
 
 from
diff --git a/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql b/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql
index 792fa2159cbe..dc9fbed27dce 100644
--- a/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql
+++ b/python/ql/test/query-tests/Variables/capture/LoopVariableCaptureTest.ql
@@ -3,11 +3,11 @@ import Variables.LoopVariableCapture.LoopVariableCaptureQuery
 import utils.test.InlineExpectationsTest
 
 module MethodArgTest implements TestSig {
-  string getARelevantTag() { result = ["capturedVar"] }
+  string getARelevantTag() { result = "capturedVar" }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(CallableExpr capturing, AstNode loop, Variable var |
-      escapingCapture(capturing, loop, var, _, _) and
+    exists(CallableExpr capturing, Variable var |
+      escapingCapture(capturing, _, var, _, _) and
       element = capturing.toString() and
       location = capturing.getLocation() and
       tag = "capturedVar" and

From de7e6119624c99e6e1c990904162a3b3c78c2619 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 4 Apr 2025 12:36:13 +0100
Subject: [PATCH 10/14] Rewrite documentation

---
 .../LoopVariableCapture.py                    | 18 --------
 .../LoopVariableCapture.qhelp                 | 46 ++++++-------------
 .../LoopVariableCapture.ql                    |  1 +
 .../LoopVariableCapture/examples/bad.py       |  8 ++++
 .../LoopVariableCapture/examples/good.py      |  8 ++++
 .../LoopVariableCapture/examples/good2.py     |  9 ++++
 6 files changed, 41 insertions(+), 49 deletions(-)
 delete mode 100644 python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py
 create mode 100644 python/ql/src/Variables/LoopVariableCapture/examples/bad.py
 create mode 100644 python/ql/src/Variables/LoopVariableCapture/examples/good.py
 create mode 100644 python/ql/src/Variables/LoopVariableCapture/examples/good2.py

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py
deleted file mode 100644
index 4a6abcb88946..000000000000
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.py
+++ /dev/null
@@ -1,18 +0,0 @@
-
-#Make a list of functions to increment their arguments by 0 to 9.
-def make_incrementers():
-    result = []
-    for i in range(10):
-        def incrementer(x):
-            return x + i
-        result.append(incrementer)
-    return result
-
-#This will fail
-def test():
-    incs = make_incrementers()
-    for x in range(10):
-        for y in range(10):
-            assert incs[x](y) == x+y
-
-test()
\ No newline at end of file
diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
index 15f2b185eb9d..ae23584344c8 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
@@ -5,56 +5,40 @@
 
 <overview>
 <p> 
-Nested functions are a useful feature of Python as it allows a function to access the variables of its enclosing function.
-However, the programmer needs to be aware that when an inner function accesses a variable in an outer scope, 
-it is the variable that is captured, not the value of that variable.
+In Python, a nested function or lambda expression that captures a variable from its surrounding scope is a <em>late-binding</em> closure, 
+meaning that the value of the variable is determined when the closure is called, not when it is created.
 </p>
 <p>
-Therefore, care must be taken when the captured variable is a loop variable, since it is the loop <em>variable</em> and
-<em>not</em> the <em>value</em> of that variable that is captured.
-This will mean that by the time that the inner function executes, 
-the loop variable will have its final value, not the value when the inner function was created.
+Care must be taken when the captured variable is a loop variable. If the closure is called after the loop ends, it will use the value of the variable on the last iteration of the loop, rather than the value at the iteration at which it was created.
 </p>
 
 </overview>
 <recommendation>
 <p>
-The simplest way to fix this problem is to add a local variable of the same name as the outer variable and initialize that 
-using the outer variable as a default.
-<code>
-for var in seq:
-    ...
-    def inner_func(arg):
-        ...
-        use(var)
-</code>
-becomes
-<code>
-for var in seq:
-    ...
-    def inner_func(arg, var=var):
-        ...
-        use(var)
-</code>
+Ensure that closures that capture loop variables aren't used outside of a single iteration of the loop. 
+To capture the value of a loop variable at the time the closure is created, use a default parameter, or <code>functools.partial</code>. 
 </p>
 
 </recommendation>
 <example>
 <p>
-In this example, a list of functions is created which should each increment its argument by its index in the list.
-However, since <code>i</code> will be 9 when the functions execute, they will each increment their argument by 9.
+In the following (BAD) example, a `tasks` list is created, but each task captures the loop variable <code>i</code>, and reads the same value when run.
  </p>
-<sample src="LoopVariableCapture.py" />
+<sample src="examples/bad.py" />
 <p>
-This can be fixed by adding the default value as shown below. The default value is computed when the function is created, so the desired effect is achieved.
+In the following (GOOD) example, each closure has an `i` default parameter, shadowing the outer <code>i</code> variable, the default value of which is determined as the value of the loop variable <code>i</code> at the time the closure is created.
+<sample src="examples/good.py" />
+In the following (GOOD) example, <code>functools.partial</code> is used to partially evaluate the lambda expression with the value of <code>i</code>.
+<sample src="examples/good2.py" />
 </p>
 
-<sample src="LoopVariableCapture2.py" />
 
 </example>
 <references>
-<li>The Hitchhiker’s Guide to Python: <a href="http://docs.python-guide.org/en/latest/writing/gotchas/#late-binding-closures">Late Binding Closures</a></li>
-<li>Python Language Reference: <a href="https://docs.python.org/reference/executionmodel.html#naming-and-binding">Naming and binding</a></li>
+<li>The Hitchhiker's Guide to Python: <a href="http://docs.python-guide.org/en/latest/writing/gotchas/#late-binding-closures">Late Binding Closures</a>.</li>
+<li>Python Language Reference: <a href="https://docs.python.org/reference/executionmodel.html#naming-and-binding">Naming and binding</a>.</li>
+<li>Stack Overflow: <a href="https://stackoverflow.com/questions/3431676/creating-functions-or-lambdas-in-a-loop-or-comprehension">Creating functions (or lambdas) in a loop (or comprehension)</a>.</li>
+<li>Python Language Reference: <a href="https://docs.python.org/3/library/functools.html#functools.partial">functools.partial</a>.</li>
 
 </references>
 </qhelp>
diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index 9c41e52cc45c..514a6790ea08 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -3,6 +3,7 @@
  * @description Capture of a loop variable is not the same as capturing the value of a loop variable, and may be erroneous.
  * @kind path-problem
  * @tags correctness
+ *       quality
  * @problem.severity error
  * @sub-severity low
  * @precision high
diff --git a/python/ql/src/Variables/LoopVariableCapture/examples/bad.py b/python/ql/src/Variables/LoopVariableCapture/examples/bad.py
new file mode 100644
index 000000000000..1e70d20fd1db
--- /dev/null
+++ b/python/ql/src/Variables/LoopVariableCapture/examples/bad.py
@@ -0,0 +1,8 @@
+# BAD: The loop variable `i` is captured.
+tasks = []
+for i in range(5):
+    tasks.append(lambda: print(i))
+
+# This will print `4,4,4,4,4`, rather than `0,1,2,3,4` as likely intended.
+for t in tasks:
+    t() 
\ No newline at end of file
diff --git a/python/ql/src/Variables/LoopVariableCapture/examples/good.py b/python/ql/src/Variables/LoopVariableCapture/examples/good.py
new file mode 100644
index 000000000000..67ed2624f0a8
--- /dev/null
+++ b/python/ql/src/Variables/LoopVariableCapture/examples/good.py
@@ -0,0 +1,8 @@
+# GOOD: A default parameter is used, so the variable `i` is not being captured.
+tasks = []
+for i in range(5):
+    tasks.append(lambda i=i: print(i))
+
+# This will print `0,1,2,3,4``.
+for t in tasks:
+    t() 
\ No newline at end of file
diff --git a/python/ql/src/Variables/LoopVariableCapture/examples/good2.py b/python/ql/src/Variables/LoopVariableCapture/examples/good2.py
new file mode 100644
index 000000000000..1a2469b42202
--- /dev/null
+++ b/python/ql/src/Variables/LoopVariableCapture/examples/good2.py
@@ -0,0 +1,9 @@
+import functools
+# GOOD: A default parameter is used, so the variable `i` is not being captured.
+tasks = []
+for i in range(5):
+    tasks.append(functools.partial(lambda i: print(i), i))
+
+# This will print `0,1,2,3,4``.
+for t in tasks:
+    t() 
\ No newline at end of file

From e08072d77b7bc99c9af1c525de419c6736754412 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 4 Apr 2025 12:43:23 +0100
Subject: [PATCH 11/14] Fix qhelp formatting

---
 .../Variables/LoopVariableCapture/LoopVariableCapture.qhelp  | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
index ae23584344c8..b902f976a537 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
@@ -27,10 +27,13 @@ In the following (BAD) example, a `tasks` list is created, but each task capture
 <sample src="examples/bad.py" />
 <p>
 In the following (GOOD) example, each closure has an `i` default parameter, shadowing the outer <code>i</code> variable, the default value of which is determined as the value of the loop variable <code>i</code> at the time the closure is created.
+</p>
 <sample src="examples/good.py" />
+<p>
 In the following (GOOD) example, <code>functools.partial</code> is used to partially evaluate the lambda expression with the value of <code>i</code>.
-<sample src="examples/good2.py" />
 </p>
+<sample src="examples/good2.py" />
+
 
 
 </example>

From 84aa2e8627abcf3ca1dceb1d4ebfaaf16033b8e6 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 9 Apr 2025 14:07:38 +0100
Subject: [PATCH 12/14] Apply review suggestion - Tweak wording of example
 comment

Co-authored-by: Taus <tausbn@github.com>
---
 python/ql/src/Variables/LoopVariableCapture/examples/good2.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/examples/good2.py b/python/ql/src/Variables/LoopVariableCapture/examples/good2.py
index 1a2469b42202..7047e11d0b69 100644
--- a/python/ql/src/Variables/LoopVariableCapture/examples/good2.py
+++ b/python/ql/src/Variables/LoopVariableCapture/examples/good2.py
@@ -1,5 +1,5 @@
 import functools
-# GOOD: A default parameter is used, so the variable `i` is not being captured.
+# GOOD: `functools.partial` takes care of capturing the _value_ of `i`.
 tasks = []
 for i in range(5):
     tasks.append(functools.partial(lambda i: print(i), i))

From 00999baf9ae4f7ecad7c2bb627ab6f715a79dfba Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 10 Apr 2025 09:06:01 +0100
Subject: [PATCH 13/14] Apply docs review suggestion - Reword query
 description.

Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
---
 .../ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
index 514a6790ea08..034ac05ee946 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql
@@ -1,6 +1,6 @@
 /**
  * @name Loop variable capture
- * @description Capture of a loop variable is not the same as capturing the value of a loop variable, and may be erroneous.
+ * @description Capturing a loop variable is not the same as capturing its value, and can lead to unexpected behavior or bugs.
  * @kind path-problem
  * @tags correctness
  *       quality

From 6802037c892b51ecb1758d0d4d884135e71eb6e2 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 10 Apr 2025 09:52:18 +0100
Subject: [PATCH 14/14] Update qhelp formatting

---
 .../Variables/LoopVariableCapture/LoopVariableCapture.qhelp   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
index b902f976a537..2f3beeb2e9fc 100644
--- a/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
+++ b/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.qhelp
@@ -22,11 +22,11 @@ To capture the value of a loop variable at the time the closure is created, use
 </recommendation>
 <example>
 <p>
-In the following (BAD) example, a `tasks` list is created, but each task captures the loop variable <code>i</code>, and reads the same value when run.
+In the following (BAD) example, a <code>tasks</code> list is created, but each task captures the loop variable <code>i</code>, and reads the same value when run.
  </p>
 <sample src="examples/bad.py" />
 <p>
-In the following (GOOD) example, each closure has an `i` default parameter, shadowing the outer <code>i</code> variable, the default value of which is determined as the value of the loop variable <code>i</code> at the time the closure is created.
+In the following (GOOD) example, each closure has an <code>i</code> default parameter, shadowing the outer <code>i</code> variable, the default value of which is determined as the value of the loop variable <code>i</code> at the time the closure is created.
 </p>
 <sample src="examples/good.py" />
 <p>