From bced7bb6d2577d57846cea318636271a6ec92f4c Mon Sep 17 00:00:00 2001
From: Asger F <asgerf@github.com>
Date: Wed, 1 Jul 2026 21:51:21 +0200
Subject: [PATCH] unified: Add local name binding and tests

As documented in the test, 'guard if' statements are still not properly supported.
---
 .../lib/codeql/unified/internal/Variables.qll | 244 ++++++++++++++
 unified/ql/lib/qlpack.yml                     |   1 +
 unified/ql/lib/unified.qll                    |   1 +
 .../test/library-tests/variables/test.swift   | 309 ++++++++++++++++++
 .../variables/variables.expected              |   0
 .../test/library-tests/variables/variables.ql |  51 +++
 6 files changed, 606 insertions(+)
 create mode 100644 unified/ql/lib/codeql/unified/internal/Variables.qll
 create mode 100644 unified/ql/test/library-tests/variables/test.swift
 create mode 100644 unified/ql/test/library-tests/variables/variables.expected
 create mode 100644 unified/ql/test/library-tests/variables/variables.ql

diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll
new file mode 100644
index 00000000000..51ff5a5ab7f
--- /dev/null
+++ b/unified/ql/lib/codeql/unified/internal/Variables.qll
@@ -0,0 +1,244 @@
+/**
+ * Provides classes for reasoning about lexically scoped variables and references to these.
+ */
+
+private import unified
+private import unified as U
+private import codeql.namebinding.LocalNameBinding
+
+private module LocalNameBindingInput implements LocalNameBindingInputSig<Location> {
+  class AstNode = U::AstNode;
+
+  private class LogicalAndRoot extends LogicalAndExpr {
+    LogicalAndRoot() { not this = any(LogicalAndExpr e).getAnOperand() }
+
+    private Expr getDescendent(string path) {
+      path = "" and result = this
+      or
+      exists(LogicalAndExpr mid, string midPath | mid = this.getDescendent(midPath) |
+        result = mid.getLeft() and path = midPath + "A"
+        or
+        result = mid.getRight() and path = midPath + "B"
+      )
+    }
+
+    Expr getNthLeaf(int n) {
+      result =
+        rank[n](Expr e, string path |
+          e = this.getDescendent(path) and not e instanceof LogicalAndExpr
+        |
+          e order by path
+        )
+    }
+
+    Expr getLastLeaf() { result = max(int n | | this.getNthLeaf(n) order by n) }
+  }
+
+  private AstNode getChild1(AstNode n, int index) {
+    result = n.(Block).getStmt(index)
+    or
+    result = n.(LogicalAndRoot).getNthLeaf(index)
+    or
+    exists(PatternGuardExpr guard | n = guard |
+      index = 0 and result = guard.getPattern()
+      or
+      index = 1 and result = guard.getValue()
+    )
+    or
+    exists(IfExpr expr | n = expr |
+      index = 0 and result = expr.getCondition()
+      or
+      index = 1 and result = expr.getThen()
+      or
+      index = 2 and result = expr.getElse()
+    )
+    or
+    exists(VariableDeclaration decl | n = decl |
+      index = 0 and result = decl.getPattern()
+      or
+      index = 1 and result = decl.getType()
+      or
+      index = 2 and result = decl.getValue()
+    )
+  }
+
+  AstNode getChild(AstNode n, int index) {
+    result = getChild1(n, index)
+    or
+    not exists(getChild1(n, _)) and
+    not n instanceof LogicalAndExpr and // also ignore intermediate nodes within a 'logical and' tree
+    index = 0 and
+    result = n.getAFieldOrChild()
+  }
+
+  abstract class Conditional extends AstNode {
+    /** Gets the condition of this conditional. */
+    abstract AstNode getCondition();
+
+    /** Gets the then-branch of this conditional. */
+    abstract AstNode getThen();
+
+    /** Gets the else-branch of this conditional. */
+    abstract AstNode getElse();
+  }
+
+  private class IfExprConditional extends Conditional instanceof IfExpr {
+    override AstNode getCondition() { result = IfExpr.super.getCondition() }
+
+    override AstNode getThen() { result = IfExpr.super.getThen() }
+
+    override AstNode getElse() { result = IfExpr.super.getElse() }
+  }
+
+  abstract class SiblingShadowingDecl extends AstNode {
+    /** Gets the left-hand side of this declaration. */
+    abstract AstNode getLhs();
+
+    /**
+     * Gets the right-hand side of this declaration.
+     *
+     * Any local declared in the left-hand side of this declaration is _not_ in scope
+     * in the right-hand side.
+     */
+    abstract AstNode getRhs();
+
+    /**
+     * Gets the else-branch of this declaration, if any.
+     *
+     * Any local declared in the left-hand side of this declaration is _not_ in scope
+     * in the else-branch.
+     */
+    abstract AstNode getElse();
+  }
+
+  private class LocalVariableDeclarationSiblingShadowingDecl extends SiblingShadowingDecl instanceof LocalVariableDeclaration
+  {
+    override AstNode getLhs() { result = LocalVariableDeclaration.super.getPattern() }
+
+    override AstNode getRhs() { result = LocalVariableDeclaration.super.getValue() }
+
+    override AstNode getElse() { none() }
+  }
+
+  private class PatternGuardExprSiblingShadowingDecl extends SiblingShadowingDecl instanceof PatternGuardExpr
+  {
+    override AstNode getLhs() { result = PatternGuardExpr.super.getPattern() }
+
+    override AstNode getRhs() { result = PatternGuardExpr.super.getValue() }
+
+    override AstNode getElse() { none() }
+  }
+
+  private class GuardIfStmtSiblingShadowingDecl extends SiblingShadowingDecl instanceof GuardIfStmt {
+    override AstNode getLhs() { result = GuardIfStmt.super.getCondition() }
+
+    override AstNode getRhs() { none() }
+
+    override AstNode getElse() { result = GuardIfStmt.super.getElse() }
+  }
+
+  private predicate bindingContext(AstNode pattern, AstNode scope) {
+    exists(LocalVariableDeclaration decl |
+      scope = decl and // LocalVariableDeclaration is a ShadowingSiblingDecl, it must use itself as the scope
+      pattern = decl.getPattern()
+    )
+    or
+    exists(LocalFunctionDeclaration func |
+      scope = func.getDeclaringBlock() and
+      pattern = func.getName()
+    )
+    or
+    exists(Parameter param |
+      scope = param.getParent() and // TODO: add SourceCallable and use .getParameter() instead
+      pattern = param.getPattern()
+    )
+    or
+    exists(CatchClause catch |
+      scope = catch and // ensure both 'body' and 'guard' clause are in scope
+      pattern = catch.getPattern()
+    )
+    or
+    exists(SwitchCase case |
+      scope = case and // ensure both 'body' and 'guard' clause are in scope (TODO: merge CatchClause and SwitchCase?)
+      pattern = case.getPattern()
+    )
+    or
+    exists(ForEachStmt stmt |
+      scope = stmt and // ensure both 'body' and 'guard' are in scope
+      pattern = stmt.getPattern()
+    )
+    or
+    exists(TuplePattern pat |
+      bindingContext(pat, scope) and
+      pattern = pat.getElement(_).getPattern()
+    )
+    or
+    exists(ConstructorPattern pat |
+      bindingContext(pat, scope) and
+      pattern = pat.getElement(_).getPattern()
+    )
+    or
+    exists(OrPattern pat |
+      bindingContext(pat, scope) and
+      pattern = pat.getPattern(_)
+    )
+    or
+    exists(PatternGuardExpr expr |
+      pattern = expr.getPattern() and
+      scope = expr
+    )
+  }
+
+  predicate declInScope(AstNode definingNode, string name, AstNode scope) {
+    bindingContext(definingNode, scope) and
+    (
+      definingNode.(NamePattern).getIdentifier().getValue() = name
+      or
+      definingNode.(Identifier).getValue() = name
+    )
+  }
+
+  predicate implicitDeclInScope(string name, AstNode scope) {
+    none()
+    // TODO: self
+  }
+
+  predicate accessCand(AstNode n, string name) {
+    n.(NameExpr).getIdentifier().getValue() = name
+    or
+    n.(NamePattern).getIdentifier().getValue() = name
+    or
+    n = any(LocalFunctionDeclaration f).getName() and
+    n.(Identifier).getValue() = name
+  }
+
+  predicate lookupStartsAt(AstNode n, AstNode scope) { none() }
+}
+
+module LocalNameBindingOutput = LocalNameBinding<Location, LocalNameBindingInput>;
+
+module Public {
+  /**
+   * A local variable.
+   */
+  class Variable extends LocalNameBindingOutput::Local {
+    VariableAccess getAnAccess() { result.getVariable() = this }
+  }
+
+  /**
+   * An AST node that is a reference to a local variable.
+   */
+  class VariableAccess extends AstNode instanceof LocalNameBindingOutput::LocalAccess {
+    Variable getVariable() { result = super.getLocal() }
+
+    Identifier getIdentifier() {
+      result = this.(NameExpr).getIdentifier()
+      or
+      result = this.(NamePattern).getIdentifier()
+      or
+      result = this
+    }
+
+    string getName() { result = this.getIdentifier().getValue() }
+  }
+}
diff --git a/unified/ql/lib/qlpack.yml b/unified/ql/lib/qlpack.yml
index 896bf37ac5e..d67af16271b 100644
--- a/unified/ql/lib/qlpack.yml
+++ b/unified/ql/lib/qlpack.yml
@@ -7,5 +7,6 @@ library: true
 upgrades: upgrades
 dependencies:
   codeql/util: ${workspace}
+  codeql/namebinding: ${workspace}
 warnOnImplicitThis: true
 compileForOverlayEval: true
diff --git a/unified/ql/lib/unified.qll b/unified/ql/lib/unified.qll
index 73af928aa4e..477ac22f366 100644
--- a/unified/ql/lib/unified.qll
+++ b/unified/ql/lib/unified.qll
@@ -6,3 +6,4 @@ import codeql.Locations
 import codeql.files.FileSystem
 import codeql.unified.Ast::Unified
 import codeql.unified.internal.AstExtra::Public
+import codeql.unified.internal.Variables::Public
diff --git a/unified/ql/test/library-tests/variables/test.swift b/unified/ql/test/library-tests/variables/test.swift
new file mode 100644
index 00000000000..2f942605029
--- /dev/null
+++ b/unified/ql/test/library-tests/variables/test.swift
@@ -0,0 +1,309 @@
+func t1() -> Int {
+    let x = 42 // name=x1
+    return x // $ access=x1
+}
+
+func t2() -> Int {
+    let x = 42 // name=x1
+
+    if case let x = x + 10 { // $ access=x1 // name=x2
+        print(x) // $ access=x2
+    }
+    print(x) // $ access=x1
+}
+
+func t3() -> Int {
+    guard let x = foo() else { // name=x1
+        return 0
+    }
+    print(x) // $ MISSING: access=x1
+}
+
+// Function parameters
+func t4(x: Int) -> Int { // name=x1
+    return x // $ access=x1
+}
+
+// Multiple parameters
+func t5(x: Int, // name=x1
+        y: Int) -> Int { // name=y1
+    let z = x + // $ access=x1 // name=z1
+            y // $ access=y1
+    return z // $ access=z1
+}
+
+// Parameter shadowed by local variable
+func t6(x: Int) -> Int { // name=x1
+    let x = x * 2 // $ access=x1 // name=x2
+    return x // $ access=x2
+}
+
+// Nested blocks
+func t7() {
+    let x = 1 // name=x1
+    {
+        print(x) // $ MISSING: access=x1
+        let x = 2 // name=x2
+        print(x) // $ access=x2
+    }
+    print(x) // $ access=x1
+}
+
+// For-in loop
+func t8() {
+    let array = [1, 2, 3] // name=array1
+    for x in array { // $ access=array1 // name=x1
+        print(x) // $ access=x1
+    }
+}
+
+// For-in loop with shadowing
+func t9() {
+    let x = 0 // name=x1
+    let array = [1, 2, 3] // name=array1
+    for x in array { // $ access=array1 // name=x2
+        print(x) // $ access=x2
+    }
+    print(x) // $ access=x1
+}
+
+// Switch statement with case bindings
+func t10(value: Int) { // name=value1
+    switch value { // $ access=value1
+    case let x: // name=x1
+        print(x) // $ access=x1
+    default:
+        break
+    }
+}
+
+// Switch with multiple cases
+func t11(value: Int) { // name=value1
+    switch value { // $ access=value1
+    case let x where x > 0: // name=x1
+        print(x) // $ access=x1
+    case let x: // name=x2
+        print(x) // $ access=x2
+    }
+}
+
+// Tuple unpacking
+func t12() {
+    let tuple = (1, 2) // name=tuple1
+    let (x, // name=x1
+         y) = tuple // $ access=tuple1 // name=y1
+    print(x) // $ MISSING: access=x1 // because x,y are incorrectly mapped to expr_equality_pattern
+    print(y) // $ MISSING: access=y1
+}
+
+// Tuple unpacking with underscore
+func t13() {
+    let tuple = (1, 2, 3) // name=tuple1
+    let (x, // name=x1
+         _,
+         y) = tuple // $ access=tuple1 // name=y1
+    print(x) // $ MISSING: access=x1
+    print(y) // $ MISSING: access=y1
+}
+
+// Optional binding (if let)
+func t14(optional: Int?) { // name=optional1
+    if let x = optional { // $ access=optional1 // name=x1
+        print(x) // $ access=x1
+    }
+}
+
+// Multiple optional bindings
+func t15(opt1: Int?, // name=opt11
+         opt2: String?) { // name=opt21
+    if let x = opt1, // $ access=opt11 // name=x1
+       let y = opt2 { // $ access=opt21 // name=y1
+        print(x) // $ access=x1
+        print(y) // $ access=y1
+    }
+}
+
+// Do-catch blocks
+func t16() throws {
+    do {
+        let x = try foo() // name=x1
+        print(x) // $ access=x1
+    } catch let error { // name=error1
+        print(error) // $ access=error1
+    }
+}
+
+// Closure captures
+func t17() {
+    let x = 1 // name=x1
+    let closure = { // name=closure1
+        print(x) // $ access=x1
+    }
+    closure() // $ access=closure1
+}
+
+// Closure with parameter shadowing
+func t18() {
+    let x = 1 // name=x1
+    let closure =
+        { (x: Int) -> Void in // name=x2
+            print(x) // $ access=x2
+        }
+    closure(2) // $ access=closure
+    print(x) // $ access=x1
+}
+
+// While loop
+func t19() {
+    var x = 0 // name=x1
+    while x < 10 { // $ access=x1
+        x = x + 1 // $ access=x1
+        print(x) // $ access=x1
+    }
+}
+
+// Repeat-while loop
+func t20() {
+    var x = 0 // name=x1
+    repeat {
+        x = x + 1 // $ access=x1
+        print(x) // $ access=x1
+    } while x < 10 // $ access=x1
+}
+
+// Property shadowing (var)
+func t21() {
+    var x = 1 // name=x1
+    var x = 2 // name=x2
+    print(x) // $ access=x2
+}
+
+// Nested functions
+func t22() {
+    let x = 1 // name=x1
+    func inner() { // name=inner1
+        let x = 2 // name=x2
+        print(x) // $ access=x2
+    }
+    inner() // $ access=inner1
+    print(x) // $ access=x1
+}
+
+// Three levels of shadowing
+func t23() {
+    let x = 1 // name=x1
+    {
+        let x = 2 // name=x2
+        {
+            let x = 3 // name=x3
+            print(x) // $ access=x3
+        }
+        print(x) // $ access=x2
+    }
+    print(x) // $ access=x1
+}
+
+// If-let followed by regular if
+func t24(optional: Int?) { // name=optional1
+    if let x = optional { // $ access=optional1 // name=x1
+        print(x) // $ access=x1
+    }
+    if true {
+        let x = 5 // name=x2
+        print(x) // $ access=x2
+    }
+}
+
+// Switch with same variable name in different cases
+func t25(value: Int) { // name=value1
+    switch value { // $ access=value1
+    case let x: // name=x1
+        print(x) // $ access=x1
+        let x = 10 // name=x2
+        print(x) // $ access=x2
+    }
+}
+
+func t26() -> Int {
+    let x = 42 // name=x1
+    guard let x = foo() else { // name=x2
+        return x // $ access=x1
+    }
+    print(x) // $ MISSING: access=x2 SPURIOUS: access=x1
+}
+
+// if with multiple conditions, mixing boolean and optional binding
+func t27(opt: Int?) { // name=opt1
+    if opt != nil, // $ access=opt1
+       let x = opt { // $ access=opt1
+        print(x) // $ access=x
+    }
+}
+
+// if with multiple let bindings and a boolean condition
+func t28(a: Int?, b: Int?) { // name=a1 // name=b1
+    if let x = a, // $ access=a1 SPURIOUS: access=b1
+       let y = b, // $ access=b1 SPURIOUS: access=a1
+       x < y { // $ $ access=x access=y
+        print(x) // $ access=x
+        print(y) // $ access=y
+    }
+}
+
+// if with multiple conditions where a later binding shadows an outer variable
+func t29(opt: Int?) { // name=opt1
+    let x = 0 // name=x1
+    if opt != nil, // $ access=opt1
+       let x = opt { // $ access=opt1
+        print(x) // $ access=x
+    }
+    print(x) // $ access=x1
+}
+
+// guard with multiple conditions, mixing boolean and optional binding
+func t30(opt: Int?) { // name=opt1
+    guard opt != nil, // $ access=opt1
+          let x = opt else { // $ access=opt1
+        return
+    }
+    print(x) // $ MISSING: access=x
+}
+
+// guard with multiple let bindings and a boolean condition
+func t31(a: Int?, b: Int?) { // name=a1 // name=b1
+    guard let x = a, // $ access=a1 SPURIOUS: access=b1
+          let y = b, // $ access=b1 SPURIOUS: access=a1
+          x < y else { // $ $ access=x access=y
+        return
+    }
+    print(x) // $ MISSING: access=x
+    print(y) // $ MISSING: access=y
+}
+
+// guard with multiple conditions where bound variable is used in later condition
+func t32(opt: Int?) { // name=opt1
+    guard let x = opt, // $ access=opt1
+          x > 0 else { // $ access=x
+        return
+    }
+    print(x) // $ MISSING: access=x
+}
+
+func t33() {
+    let x = 1 // name=x1
+    guard x > 0, // $ access=x1
+          let x = x, // $ MISSING: access=x2 SPURIOUS: access=x1 // name=x2
+          x > 0 else { // $ access=x2
+        return
+    }
+    print(x) // $ MISSING: access=x2 SPURIOUS: access=x1
+}
+
+func t34() {
+    let x = 1 // name=x1
+    if x > 0, // $ access=x1
+       let x = x, // $ access=x1 // name=x2
+       x > 0 else { // $ access=x2
+        print(x) // $ access=x2
+    }
+}
diff --git a/unified/ql/test/library-tests/variables/variables.expected b/unified/ql/test/library-tests/variables/variables.expected
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/unified/ql/test/library-tests/variables/variables.ql b/unified/ql/test/library-tests/variables/variables.ql
new file mode 100644
index 00000000000..146da296ae7
--- /dev/null
+++ b/unified/ql/test/library-tests/variables/variables.ql
@@ -0,0 +1,51 @@
+import unified
+import utils.test.InlineExpectationsTest
+
+/** Holds if a comment with `text` appears at `filepath:line`, excluding the text in a `$` section. */
+predicate plainCommentAt(string filepath, int line, string text) {
+  exists(Comment comment |
+    comment.getLocation().hasLocationInfo(filepath, line, _, _, _) and
+    text = comment.getCommentText().regexpReplaceAll("\\$([^/]|/[^/])*", "")
+  )
+}
+
+/** Holds if a `key=value` comment appears on `filepath:line` (not in the `$` section). */
+predicate keyValueCommentAt(string filepath, int line, string key, string value) {
+  exists(string text, string regexp, string match |
+    plainCommentAt(filepath, line, text) and
+    regexp = "(\\w+)=(\\w+)" and
+    match = text.regexpFind(regexp, _, _) and
+    key = match.regexpCapture(regexp, 1) and
+    value = match.regexpCapture(regexp, 2)
+  )
+}
+
+module VariableAccessTest implements TestSig {
+  string getARelevantTag() { result = "access" }
+
+  private predicate declAt(Variable v, string filepath, int line) {
+    v.getLocation().hasLocationInfo(filepath, _, _, line, _)
+  }
+
+  private predicate decl(Variable v, string alias) {
+    exists(string filepath, int line | declAt(v, filepath, line) |
+      keyValueCommentAt(filepath, line, "name", alias)
+      or
+      not keyValueCommentAt(filepath, line, "name", _) and
+      alias = v.getName()
+    )
+  }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(VariableAccess va, Variable v |
+      v = va.getVariable() and
+      not va = v.getDefiningNode() and
+      location = va.getLocation() and
+      element = va.toString() and
+      decl(v, value) and
+      tag = "access"
+    )
+  }
+}
+
+import MakeTest<VariableAccessTest>