From 982f6a5e8ab30df3f39b7b636582ea34c18c07fa Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 1 Jul 2026 21:49:48 +0200 Subject: [PATCH 01/13] unified: Move Comments.ql into AstExtra.qll Also move into 'internal' and expose through a Public module --- unified/ql/lib/codeql/unified/Comments.qll | 18 --------------- .../lib/codeql/unified/internal/AstExtra.qll | 22 +++++++++++++++++++ unified/ql/lib/unified.qll | 2 +- 3 files changed, 23 insertions(+), 19 deletions(-) delete mode 100644 unified/ql/lib/codeql/unified/Comments.qll create mode 100644 unified/ql/lib/codeql/unified/internal/AstExtra.qll diff --git a/unified/ql/lib/codeql/unified/Comments.qll b/unified/ql/lib/codeql/unified/Comments.qll deleted file mode 100644 index e839af2dbee2..000000000000 --- a/unified/ql/lib/codeql/unified/Comments.qll +++ /dev/null @@ -1,18 +0,0 @@ -/** Provides classes for working with comments. */ - -private import unified - -/** - * A comment appearing in the source code. - */ -class Comment extends TriviaToken { - // At the moment, comments are the only type trivia token we extract - /** - * Gets the text inside this comment, not counting the delimeters. - */ - string getCommentText() { - result = this.getValue().regexpCapture("//(.*)", 1) - or - result = this.getValue().regexpCapture("(?s)/\\*(.*)\\*/", 1) - } -} diff --git a/unified/ql/lib/codeql/unified/internal/AstExtra.qll b/unified/ql/lib/codeql/unified/internal/AstExtra.qll new file mode 100644 index 000000000000..df98711c9750 --- /dev/null +++ b/unified/ql/lib/codeql/unified/internal/AstExtra.qll @@ -0,0 +1,22 @@ +/** + * Provides additional AST-like classes outside the generated tree-sitter classes. + */ + +private import unified + +module Public { + /** + * A comment appearing in the source code. + */ + class Comment extends TriviaToken { + // At the moment, comments are the only type trivia token we extract + /** + * Gets the text inside this comment, not counting the delimeters. + */ + string getCommentText() { + result = this.getValue().regexpCapture("//(.*)", 1) + or + result = this.getValue().regexpCapture("(?s)/\\*(.*)\\*/", 1) + } + } +} diff --git a/unified/ql/lib/unified.qll b/unified/ql/lib/unified.qll index 4f7387ef8f1c..73af928aa4e4 100644 --- a/unified/ql/lib/unified.qll +++ b/unified/ql/lib/unified.qll @@ -5,4 +5,4 @@ import codeql.Locations import codeql.files.FileSystem import codeql.unified.Ast::Unified -import codeql.unified.Comments +import codeql.unified.internal.AstExtra::Public From 970e991b2c690c046d2879f8fd609cb2b40d43e8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 1 Jul 2026 21:50:04 +0200 Subject: [PATCH 02/13] unified: Add a few more AST helper classes --- .../lib/codeql/unified/internal/AstExtra.qll | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/unified/ql/lib/codeql/unified/internal/AstExtra.qll b/unified/ql/lib/codeql/unified/internal/AstExtra.qll index df98711c9750..ba625aad5fe7 100644 --- a/unified/ql/lib/codeql/unified/internal/AstExtra.qll +++ b/unified/ql/lib/codeql/unified/internal/AstExtra.qll @@ -5,6 +5,39 @@ private import unified module Public { + /** + * A logical 'and' expression with short-circuiting. + */ + class LogicalAndExpr extends BinaryExpr { + LogicalAndExpr() { this.getOperator().getValue() = "&&" } + + Expr getAnOperand() { result = [this.getLeft(), this.getRight()] } + } + + /** + * Declaration of a local or top-level variable. + */ + class LocalVariableDeclaration extends VariableDeclaration { + private Block block; + + LocalVariableDeclaration() { this = block.getStmt(_) } + + /** Gets the block in which this variable is declared. */ + Block getDeclaringBlock() { result = block } + } + + /** + * Declaration of a local or top-level function. + */ + class LocalFunctionDeclaration extends FunctionDeclaration { + private Block block; + + LocalFunctionDeclaration() { this = block.getStmt(_) } + + /** Gets the block in which this function is declared. */ + Block getDeclaringBlock() { result = block } + } + /** * A comment appearing in the source code. */ From bced7bb6d2577d57846cea318636271a6ec92f4c Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 1 Jul 2026 21:51:21 +0200 Subject: [PATCH 03/13] 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 000000000000..51ff5a5ab7f1 --- /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 { + 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; + +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 896bf37ac5e5..d67af16271b6 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 73af928aa4e4..477ac22f3661 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 000000000000..2f942605029c --- /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 000000000000..e69de29bb2d1 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 000000000000..146da296ae74 --- /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 From 36b31a855c714492d6dd1493d1c277f28f3e62fc Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 1 Jul 2026 21:51:41 +0200 Subject: [PATCH 04/13] unified: Add debug query for local scoping graph --- .../codeql/namebinding/LocalNameBinding.qll | 25 +++++++++++++++++++ .../unified/internal/dev/debugScopeGraph.ql | 19 ++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 unified/ql/lib/codeql/unified/internal/dev/debugScopeGraph.ql diff --git a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll index a467568f9832..9aa265f161a4 100644 --- a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll +++ b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll @@ -296,6 +296,31 @@ module LocalNameBinding ) } + /** Holds if `node` should be included in the debug tree. */ + private signature predicate relevantNodeSig(AstNode node); + + module DebugScopeGraph { + private string getANodeAnnotation(AstNode node) { + result = + "[scope=" + + strictconcat(string name | + declInScope(_, name, node) or implicitDeclInScope(name, node) + | + name, "," + ) + "]" + } + + query predicate nodes(AstNode node, string key, string value) { + relevantNode(node) and + key = "semmle.label" and + value = node.toString() + concat(getANodeAnnotation(node)) + } + + query predicate edges(AstNode node1, AstNode node2) { + relevantNode(node2) and node2 = getParentForScoping(node1) + } + } + /** Gets the immediately enclosing variable scope of `n`. */ private Scope getEnclosingScope(AstNode n) { result = getParentForScoping(n) diff --git a/unified/ql/lib/codeql/unified/internal/dev/debugScopeGraph.ql b/unified/ql/lib/codeql/unified/internal/dev/debugScopeGraph.ql new file mode 100644 index 000000000000..2f9376b1732e --- /dev/null +++ b/unified/ql/lib/codeql/unified/internal/dev/debugScopeGraph.ql @@ -0,0 +1,19 @@ +/** + * @name Debug scope graph + * @description Renders the graph used to perform local variable lookups + * @kind graph + * @id unified/debug-scope-graph + */ + +private import unified +private import codeql.unified.internal.Variables + +/** + * Holds if `node` should be shown in the graph. + */ +predicate relevantNode(AstNode node) { + // Match an ancestor node by location so its whole subtree is shown. + node.getParent*().getLocation().toString().matches("%test.swift@227:%") +} + +import LocalNameBindingOutput::DebugScopeGraph From d0cd4ac536915534eb3ae899c26904e4b34442e2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 2 Jul 2026 15:39:54 +0200 Subject: [PATCH 05/13] Shared: remove unused getLhs() predicate --- ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll | 2 -- .../namebinding/codeql/namebinding/LocalNameBinding.qll | 3 --- unified/ql/lib/codeql/unified/internal/Variables.qll | 9 --------- 3 files changed, 14 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll b/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll index 5ff48191534b..8eed5ee6f702 100644 --- a/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll +++ b/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll @@ -170,8 +170,6 @@ private module Input implements LocalNameBindingInputSig { class SiblingShadowingDecl extends AstNode { SiblingShadowingDecl() { none() } - AstNode getLhs() { none() } - AstNode getRhs() { none() } AstNode getElse() { none() } diff --git a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll index 9aa265f161a4..57c792027ba8 100644 --- a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll +++ b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll @@ -82,9 +82,6 @@ signature module LocalNameBindingInputSig { * ``` */ class SiblingShadowingDecl extends AstNode { - /** Gets the left-hand side of this declaration. */ - AstNode getLhs(); - /** * Gets the right-hand side of this declaration. * diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll index 51ff5a5ab7f1..2203eaf5a68e 100644 --- a/unified/ql/lib/codeql/unified/internal/Variables.qll +++ b/unified/ql/lib/codeql/unified/internal/Variables.qll @@ -91,9 +91,6 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig Date: Thu, 2 Jul 2026 16:57:52 +0200 Subject: [PATCH 06/13] unified: Flatten GuardIfStmt into the enclosing block --- .../lib/codeql/unified/internal/Variables.qll | 39 +++++++++++++++---- .../test/library-tests/variables/test.swift | 14 +++---- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll index 2203eaf5a68e..46eee90e7e29 100644 --- a/unified/ql/lib/codeql/unified/internal/Variables.qll +++ b/unified/ql/lib/codeql/unified/internal/Variables.qll @@ -34,8 +34,38 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig Int { guard let x = foo() else { // name=x1 return 0 } - print(x) // $ MISSING: access=x1 + print(x) // $ access=x1 } // Function parameters @@ -229,7 +229,7 @@ func t26() -> Int { guard let x = foo() else { // name=x2 return x // $ access=x1 } - print(x) // $ MISSING: access=x2 SPURIOUS: access=x1 + print(x) // $ access=x2 } // if with multiple conditions, mixing boolean and optional binding @@ -266,7 +266,7 @@ func t30(opt: Int?) { // name=opt1 let x = opt else { // $ access=opt1 return } - print(x) // $ MISSING: access=x + print(x) // $ access=x } // guard with multiple let bindings and a boolean condition @@ -276,8 +276,8 @@ func t31(a: Int?, b: Int?) { // name=a1 // name=b1 x < y else { // $ $ access=x access=y return } - print(x) // $ MISSING: access=x - print(y) // $ MISSING: access=y + print(x) // $ access=x + print(y) // $ access=y } // guard with multiple conditions where bound variable is used in later condition @@ -286,7 +286,7 @@ func t32(opt: Int?) { // name=opt1 x > 0 else { // $ access=x return } - print(x) // $ MISSING: access=x + print(x) // $ access=x } func t33() { @@ -296,7 +296,7 @@ func t33() { x > 0 else { // $ access=x2 return } - print(x) // $ MISSING: access=x2 SPURIOUS: access=x1 + print(x) // $ access=x2 } func t34() { From 6ea146f65b26cb9c617d1fc5bbabe132a9e8cd08 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 2 Jul 2026 17:09:31 +0200 Subject: [PATCH 07/13] unified: Fix broken block syntax The original test was not valid Swift syntax --- unified/ql/test/library-tests/variables/test.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unified/ql/test/library-tests/variables/test.swift b/unified/ql/test/library-tests/variables/test.swift index 2d8a8f7a0b3b..c915303caede 100644 --- a/unified/ql/test/library-tests/variables/test.swift +++ b/unified/ql/test/library-tests/variables/test.swift @@ -40,9 +40,9 @@ func t6(x: Int) -> Int { // name=x1 // Nested blocks func t7() { - let x = 1 // name=x1 - { - print(x) // $ MISSING: access=x1 + let x = 1; // name=x1 + do { + print(x) // $ access=x1 let x = 2 // name=x2 print(x) // $ access=x2 } From e7bff859e8aa371b907441fad276ebfc4c716d48 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 2 Jul 2026 17:11:44 +0200 Subject: [PATCH 08/13] unified: More fixes in test cases --- unified/ql/test/library-tests/variables/test.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/unified/ql/test/library-tests/variables/test.swift b/unified/ql/test/library-tests/variables/test.swift index c915303caede..7860c801fbed 100644 --- a/unified/ql/test/library-tests/variables/test.swift +++ b/unified/ql/test/library-tests/variables/test.swift @@ -241,9 +241,9 @@ func t27(opt: Int?) { // name=opt1 } // 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 +func t28(a: Int?, b: Int?) { + if let x = a, // $ access=a + let y = b, // $ access=b x < y { // $ $ access=x access=y print(x) // $ access=x print(y) // $ access=y @@ -270,9 +270,9 @@ func t30(opt: Int?) { // name=opt1 } // 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 +func t31(a: Int?, b: Int?) { + guard let x = a, // $ access=a + let y = b, // $ access=b x < y else { // $ $ access=x access=y return } @@ -292,7 +292,7 @@ func t32(opt: Int?) { // name=opt1 func t33() { let x = 1 // name=x1 guard x > 0, // $ access=x1 - let x = x, // $ MISSING: access=x2 SPURIOUS: access=x1 // name=x2 + let x = x, // $ access=x1 // name=x2 x > 0 else { // $ access=x2 return } From c8eb2071d8c835fdc8bf990ea49f042e268c1034 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 2 Jul 2026 17:17:47 +0200 Subject: [PATCH 09/13] unified: Add tests with 'while let' --- .../test/library-tests/variables/test.swift | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/unified/ql/test/library-tests/variables/test.swift b/unified/ql/test/library-tests/variables/test.swift index 7860c801fbed..4b96101bd4ac 100644 --- a/unified/ql/test/library-tests/variables/test.swift +++ b/unified/ql/test/library-tests/variables/test.swift @@ -307,3 +307,30 @@ func t34() { print(x) // $ access=x2 } } + +// While-let optional binding +func t35(optional: Int?) { // name=optional1 + while let x = optional { // $ access=optional1 // name=x1 + print(x) // $ MISSING: access=x1 + } +} + +// While with a sequence of variable bindings in the condition +func t36(a: Int?, b: Int?) { + while let x = a, // $ access=a + let y = b, // $ access=b + x < y { // $ $ access=x access=y + print(x) // $ MISSING: access=x + print(y) // $ MISSING: access=y + } +} + +// While-let with a sequence of shadowing variable declarations +func t37() { + let x = 1 // name=x1 + while let x = x, // $ access=x1 // name=x2 + let x = x, // $ access=x2 // name=x3 + x > 0 { // $ access=x3 + print(x) // $ MISSING: access=x3 SPURIOUS: access=x1 + } +} From 891e2441165fe4b7ebbb9bff394d0c1646379d4b Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 2 Jul 2026 17:20:12 +0200 Subject: [PATCH 10/13] unified: Add WhileStmt as a Conditional --- unified/ql/lib/codeql/unified/internal/Variables.qll | 8 ++++++++ unified/ql/test/library-tests/variables/test.swift | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll index 46eee90e7e29..97dc25382c7b 100644 --- a/unified/ql/lib/codeql/unified/internal/Variables.qll +++ b/unified/ql/lib/codeql/unified/internal/Variables.qll @@ -121,6 +121,14 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig 0 { // $ access=x3 - print(x) // $ MISSING: access=x3 SPURIOUS: access=x1 + print(x) // $ access=x3 } } From 036ed04eee203addbe1982ef7b00d44aa872cfb6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 3 Jul 2026 09:21:21 +0200 Subject: [PATCH 11/13] unified: Support OrPattern bindings --- .../lib/codeql/unified/internal/Variables.qll | 37 ++++++++++++++++--- .../test/library-tests/variables/test.swift | 15 ++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll index 97dc25382c7b..a7e8160deb86 100644 --- a/unified/ql/lib/codeql/unified/internal/Variables.qll +++ b/unified/ql/lib/codeql/unified/internal/Variables.qll @@ -213,12 +213,39 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig Date: Fri, 3 Jul 2026 11:23:26 +0200 Subject: [PATCH 12/13] unified: Translate bare names in patterns correctly - Context is fully reset when stepping into an expr/stmt/body. - Binding modifier is rolled into outer_modifiers. --- shared/yeast/src/build.rs | 31 +++++ .../extractor/src/languages/swift/swift.rs | 128 ++++++++++++------ ...g-modifier-does-not-leak-to-sibling.output | 86 ++++++++++++ ...ng-modifier-does-not-leak-to-sibling.swift | 7 + ...er-does-not-leak-into-accessor-body.output | 100 ++++++++++++++ ...ier-does-not-leak-into-accessor-body.swift | 10 ++ ...fier-does-not-leak-into-initializer.output | 63 +++++++++ ...ifier-does-not-leak-into-initializer.swift | 4 + .../tuple-destructuring-binding.output | 12 +- .../test/library-tests/variables/test.swift | 8 +- 10 files changed, 394 insertions(+), 55 deletions(-) create mode 100644 unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output create mode 100644 unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.swift create mode 100644 unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output create mode 100644 unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.swift create mode 100644 unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output create mode 100644 unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.swift diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index a5f454a5f544..7136e2332f8f 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -177,6 +177,37 @@ impl BuildCtx<'_, C> { None => Err("translate() called on a BuildCtx without a translator handle".into()), } } + + /// Translate every node in an iterator with a **fresh** user context + /// (reset to `C::default()`), restoring the previous context afterwards. + /// + /// Use when descending into a subtree — a body, expression, or statement + /// list — that must not inherit any of the surrounding translation + /// context (for example an enclosing binding modifier). Accepts single + /// (`Id`), optional (`Option`), and repeated (`Vec`) captures, + /// since all of them are `IntoIterator`. + pub fn translate_reset>( + &mut self, + ids: impl IntoIterator, + ) -> Result, String> + where + C: Default, + { + let saved = std::mem::take(&mut *self.user_ctx); + let mut out = Vec::new(); + let mut result = Ok(()); + for id in ids { + match self.translate(id) { + Ok(v) => out.extend(v), + Err(e) => { + result = Err(e); + break; + } + } + } + *self.user_ctx = saved; + result.map(|()| out) + } } impl std::ops::Deref for BuildCtx<'_, C> { diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index 5689d930bff3..0f4f80a8f019 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -25,16 +25,13 @@ struct SwiftContext { /// by the outer `function_parameter` rule; read by the `parameter` /// rules. default_value: Option, - /// Translated outer modifiers (e.g. visibility, attributes) to - /// attach to each child of a flattening outer rule. Set by - /// `property_declaration`, `enum_entry`, and - /// `protocol_property_declaration`. + /// Translated outer modifiers to attach to each child of a flattening + /// outer rule. Set by `property_declaration`, `binding_pattern`, + /// `enum_entry`, and `protocol_property_declaration`. For `let`/`var` + /// declarations and `binding_pattern`s the list is led by the binding + /// modifier, which also serves as the "this is a binding" signal for + /// pattern translation (see `in_binding_pattern`). outer_modifiers: Vec, - /// The `let`/`var` binding modifier for a `property_declaration`. - /// Set by `property_declaration`; read by the inner declaration - /// rules (`property_binding` variants, accessor rules) so they - /// emit it as part of the output node's `modifier:` field. - binding_modifier: Option, /// True when the current child of a flattening outer rule is not /// the first one — its inner rule should emit a /// `chained_declaration` modifier so the original grouping can be @@ -42,6 +39,20 @@ struct SwiftContext { is_chained: bool, } +impl SwiftContext { + /// Whether the pattern currently being translated is a binding + /// (the LHS of a `let`/`var` declaration or a `binding_pattern`). + /// + /// True exactly when an enclosing binding has published its modifier into + /// `outer_modifiers`. This is reliable because non-binding subtrees + /// (bodies, initializer values, ...) are translated with a reset context + /// (see `BuildCtx::translate_reset`), so a bare identifier only sees a + /// non-empty `outer_modifiers` when it really is a binding. + fn in_binding_pattern(&self) -> bool { + !self.outer_modifiers.is_empty() + } +} + /// Build a freshly-created `chained_declaration` modifier node if /// `ctx.is_chained`, else `None`. Used by inner declaration rules to /// emit the chained tag for non-first children of a flattening outer @@ -220,16 +231,15 @@ fn translation_rules() -> Vec> { (property_binding name: (pattern bound_identifier: @name) type: _? @ty - computed_value: (computed_property statement: _* @body)) + computed_value: (computed_property statement: _* @@body)) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: (identifier #{name}) type: {ty} accessor_kind: (accessor_kind "get") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // Stored property with willSet/didSet observers (initializer // optional) → a `variable_declaration` followed by one @@ -246,13 +256,19 @@ fn translation_rules() -> Vec> { (property_binding name: (pattern bound_identifier: @name) type: _? @ty - value: _? @val + value: _? @@val observers: (willset_didset_block willset: _? @@ws didset: _? @@ds)) => {{ + // The initializer value must not inherit the binding context + // (it may contain patterns, e.g. a switch expression), so + // translate it with a reset context. The observers keep the + // context: each willSet/didSet accessor emits the binding + // modifier and resets its own body. + let val = ctx.translate_reset(val)?; + let var_decl = tree!( (variable_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} pattern: (name_pattern identifier: (identifier #{name})) @@ -275,19 +291,23 @@ fn translation_rules() -> Vec> { ), // property_binding with any pattern name (identifier or // destructuring). Reads outer modifiers / chained tag from `ctx`. + // + // The enclosing `property_declaration` leads `ctx.outer_modifiers` + // with the `let`/`var` binding modifier, so the auto-translated name + // pattern (the LHS) becomes a binding, while the initializer value is + // translated with a reset context (see `translate_reset`). rule!( (property_binding name: @pattern type: _? @ty - value: _? @val) + value: _? @@val) => (variable_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} pattern: {pattern} type: {ty} - value: {val}) + value: {ctx.translate_reset(val)?}) // reset context: the initializer must not see the binding ), // property_declaration: flatten declarators (each may translate // to multiple nodes — variable_declaration and/or @@ -307,8 +327,11 @@ fn translation_rules() -> Vec> { => {{ let binding_text = ctx.ast.source_text(binding_kind); - ctx.binding_modifier = Some(ctx.literal("modifier", &binding_text)); - ctx.outer_modifiers = mods; + let binding = ctx.literal("modifier", &binding_text); + // The `let`/`var` binding modifier leads the declaration's + // modifier list and doubles as the "this is a binding" signal + // for pattern translation (see `in_binding_pattern`). + ctx.outer_modifiers = std::iter::once(binding).chain(mods).collect(); let mut result = Vec::new(); for (i, decl) in decls.into_iter().enumerate() { @@ -403,14 +426,23 @@ fn translation_rules() -> Vec> { rule!((type name: @inner) => {inner}), // `directly_assignable_expression` is just a wrapper; unwrap it rule!((directly_assignable_expression expr: @inner) => {inner}), - // Pattern with bound_identifier → name_pattern - rule!((pattern bound_identifier: @name) => (name_pattern identifier: (identifier #{name}))), - // Pattern with 'let' or 'var' binding: extract the inner pattern - // TODO: Names in a pattern need to be translated to expr_equality_pattern if not under a 'var/let' but we lack a way to pass down context to do this. + // Pattern with bound_identifier → name_pattern. + rule!( + (pattern bound_identifier: @name) + => + (name_pattern identifier: (identifier #{name})) + ), + // Pattern with 'let' or 'var' binding: publish the binding modifier + // into `ctx` and translate the inner pattern under it. rule!( - (pattern kind: (binding_pattern binding: _? pattern: @pattern)) + (pattern kind: (binding_pattern binding: (value_binding_pattern mutability: @@binding_kind) pattern: @@pattern)) => - {pattern} + {{ + let binding_text = ctx.ast.source_text(binding_kind); + let binding = ctx.literal("modifier", &binding_text); + ctx.outer_modifiers = vec![binding]; + ctx.translate(pattern)? + }} ), // case T.foo(x,y) pattern rule!( @@ -436,6 +468,22 @@ fn translation_rules() -> Vec> { rule!((pattern kind: (type_casting_pattern)) => (unsupported_node)), // Wildcard pattern rule!((pattern kind: (wildcard_pattern)) => (ignore_pattern)), + // A bare identifier used as an expression-pattern. Under a `var`/`let` + // binding it introduces a new variable and becomes a `name_pattern`; + // otherwise it matches by equality and is left as an `expr_equality_pattern` + // over the name expression. + rule!( + (pattern kind: (simple_identifier) @name) + => + { + if ctx.in_binding_pattern() { + tree!((name_pattern identifier: (identifier #{name}))) + } else { + let expr = tree!((name_expr identifier: (identifier #{name}))); + tree!((expr_equality_pattern expr: {expr})) + } + } + ), // Expression pattern // We lack a way to check if 'expr' is actually an expression, but due to rule ordering // the 'expression' case is the only remaining possibility when this rule tries to match. @@ -1062,56 +1110,52 @@ fn translation_rules() -> Vec> { // and binding/outer modifiers + chained tag from the outer // property_declaration rule. rule!( - (computed_getter body: (block statement: _* @body)?) + (computed_getter body: (block statement: _* @@body)?) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("computed_getter outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "get") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // Computed setter with explicit parameter name. rule!( - (computed_setter parameter: @param body: (block statement: _* @body)) + (computed_setter parameter: @param body: (block statement: _* @@body)) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("computed_setter outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "set") parameter: (parameter pattern: (name_pattern identifier: (identifier #{param}))) - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // Computed setter without explicit parameter name; body optional. rule!( - (computed_setter body: (block statement: _* @body)?) + (computed_setter body: (block statement: _* @@body)?) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("computed_setter outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "set") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // Computed modify → accessor_declaration rule!( - (computed_modify body: (block statement: _* @body)) + (computed_modify body: (block statement: _* @@body)) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("computed_modify outside property_binding context")?} type: {ctx.property_type} accessor_kind: (accessor_kind "modify") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // willset/didset block — spread to children (only reachable as a // fallback; the outer property_binding manual rule normally @@ -1122,27 +1166,25 @@ fn translation_rules() -> Vec> { // binding/outer modifiers + chained tag from the outer // property_declaration rule. rule!( - (willset_clause body: (block statement: _* @body)?) + (willset_clause body: (block statement: _* @@body)?) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("willset_clause outside property_binding context")?} accessor_kind: (accessor_kind "willSet") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // didset clause → accessor_declaration (body optional). rule!( - (didset_clause body: (block statement: _* @body)?) + (didset_clause body: (block statement: _* @@body)?) => (accessor_declaration - modifier: {ctx.binding_modifier} modifier: {ctx.outer_modifiers.clone()} modifier: {chained_modifier(&mut ctx)} name: {ctx.property_name.ok_or("didset_clause outside property_binding context")?} accessor_kind: (accessor_kind "didSet") - body: (block stmt: {body})) + body: (block stmt: {ctx.translate_reset(body)?})) ), // Preprocessor conditionals — unsupported rule!((diagnostic) => (unsupported_node)), diff --git a/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output b/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output new file mode 100644 index 000000000000..3919b875b96a --- /dev/null +++ b/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output @@ -0,0 +1,86 @@ +let x = 1 +switch y { +case someConstant: + print("matched") +default: + break +} + +--- + +source_file + statement: + property_declaration + binding: + value_binding_pattern + mutability: let + declarator: + property_binding + name: + pattern + bound_identifier: simple_identifier "x" + value: integer_literal "1" + switch_statement + entry: + switch_entry + pattern: + switch_pattern + pattern: + pattern + kind: simple_identifier "someConstant" + statement: + call_expression + function: simple_identifier "print" + suffix: + call_suffix + arguments: + value_arguments + argument: + value_argument + value: + line_string_literal + text: line_str_text "matched" + switch_entry + default: default_keyword "default" + statement: + control_transfer_statement + kind: break + expr: simple_identifier "y" + +--- + +top_level + body: + block + stmt: + variable_declaration + modifier: modifier "let" + pattern: + name_pattern + identifier: identifier "x" + value: int_literal "1" + switch_expr + case: + switch_case + body: + block + stmt: + call_expr + argument: + argument + value: string_literal "\"matched\"" + callee: + name_expr + identifier: identifier "print" + pattern: + expr_equality_pattern + expr: + name_expr + identifier: identifier "someConstant" + switch_case + body: + block + stmt: break_expr "break" + value: + name_expr + identifier: identifier "y" diff --git a/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.swift b/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.swift new file mode 100644 index 000000000000..1338097e8280 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.swift @@ -0,0 +1,7 @@ +let x = 1 +switch y { +case someConstant: + print("matched") +default: + break +} diff --git a/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output new file mode 100644 index 000000000000..4ee195672bb2 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output @@ -0,0 +1,100 @@ +var p: Int { + get { + switch y { + case someConstant: + return 1 + default: + return 2 + } + } +} + +--- + +source_file + statement: + property_declaration + binding: + value_binding_pattern + mutability: var + declarator: + property_binding + computed_value: + computed_property + accessor: + computed_getter + body: + block + statement: + switch_statement + entry: + switch_entry + pattern: + switch_pattern + pattern: + pattern + kind: simple_identifier "someConstant" + statement: + control_transfer_statement + kind: return + result: integer_literal "1" + switch_entry + default: default_keyword "default" + statement: + control_transfer_statement + kind: return + result: integer_literal "2" + expr: simple_identifier "y" + specifier: + getter_specifier + name: + pattern + bound_identifier: simple_identifier "p" + type: + type_annotation + type: + type + name: + user_type + part: + simple_user_type + name: type_identifier "Int" + +--- + +top_level + body: + block + stmt: + accessor_declaration + body: + block + stmt: + switch_expr + case: + switch_case + body: + block + stmt: + return_expr + value: int_literal "1" + pattern: + expr_equality_pattern + expr: + name_expr + identifier: identifier "someConstant" + switch_case + body: + block + stmt: + return_expr + value: int_literal "2" + value: + name_expr + identifier: identifier "y" + modifier: modifier "var" + name: identifier "p" + type: + named_type_expr + name: identifier "Int" + accessor_kind: accessor_kind "get" diff --git a/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.swift b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.swift new file mode 100644 index 000000000000..74430bb282f5 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.swift @@ -0,0 +1,10 @@ +var p: Int { + get { + switch y { + case someConstant: + return 1 + default: + return 2 + } + } +} diff --git a/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output b/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output new file mode 100644 index 000000000000..65364c1ef918 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output @@ -0,0 +1,63 @@ +let x = switch y { +case someConstant: 1 +default: 2 +} + +--- + +source_file + statement: + property_declaration + binding: + value_binding_pattern + mutability: let + declarator: + property_binding + name: + pattern + bound_identifier: simple_identifier "x" + value: + switch_statement + entry: + switch_entry + pattern: + switch_pattern + pattern: + pattern + kind: simple_identifier "someConstant" + statement: integer_literal "1" + switch_entry + default: default_keyword "default" + statement: integer_literal "2" + expr: simple_identifier "y" + +--- + +top_level + body: + block + stmt: + variable_declaration + modifier: modifier "let" + pattern: + name_pattern + identifier: identifier "x" + value: + switch_expr + case: + switch_case + body: + block + stmt: int_literal "1" + pattern: + expr_equality_pattern + expr: + name_expr + identifier: identifier "someConstant" + switch_case + body: + block + stmt: int_literal "2" + value: + name_expr + identifier: identifier "y" diff --git a/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.swift b/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.swift new file mode 100644 index 000000000000..2e6224b95019 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.swift @@ -0,0 +1,4 @@ +let x = switch y { +case someConstant: 1 +default: 2 +} diff --git a/unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output b/unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output index a0a45eb6d22a..5709911171c1 100644 --- a/unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output +++ b/unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output @@ -38,16 +38,12 @@ top_level element: pattern_element pattern: - expr_equality_pattern - expr: - name_expr - identifier: identifier "a" + name_pattern + identifier: identifier "a" pattern_element pattern: - expr_equality_pattern - expr: - name_expr - identifier: identifier "b" + name_pattern + identifier: identifier "b" value: name_expr identifier: identifier "pair" diff --git a/unified/ql/test/library-tests/variables/test.swift b/unified/ql/test/library-tests/variables/test.swift index 07929a0f0382..3a43ffeb7bbf 100644 --- a/unified/ql/test/library-tests/variables/test.swift +++ b/unified/ql/test/library-tests/variables/test.swift @@ -92,8 +92,8 @@ 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 + print(x) // $ access=x1 + print(y) // $ access=y1 } // Tuple unpacking with underscore @@ -102,8 +102,8 @@ func t13() { let (x, // name=x1 _, y) = tuple // $ access=tuple1 // name=y1 - print(x) // $ MISSING: access=x1 - print(y) // $ MISSING: access=y1 + print(x) // $ access=x1 + print(y) // $ access=y1 } // Optional binding (if let) From bfb3ead31409041d683116eb6c1170b4e34386ae Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 3 Jul 2026 14:29:54 +0200 Subject: [PATCH 13/13] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- shared/yeast/src/build.rs | 6 +++--- unified/ql/lib/codeql/unified/internal/AstExtra.qll | 2 +- unified/ql/lib/codeql/unified/internal/Variables.qll | 6 +++--- unified/ql/test/library-tests/variables/test.swift | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/shared/yeast/src/build.rs b/shared/yeast/src/build.rs index 7136e2332f8f..6c2a9d4181d7 100644 --- a/shared/yeast/src/build.rs +++ b/shared/yeast/src/build.rs @@ -183,9 +183,9 @@ impl BuildCtx<'_, C> { /// /// Use when descending into a subtree — a body, expression, or statement /// list — that must not inherit any of the surrounding translation - /// context (for example an enclosing binding modifier). Accepts single - /// (`Id`), optional (`Option`), and repeated (`Vec`) captures, - /// since all of them are `IntoIterator`. + /// context (for example an enclosing binding modifier). Accepts optional + /// (`Option`) and repeated (`Vec`) captures (both `IntoIterator`); + /// for a single `Id`, wrap it in `std::iter::once(id)`. pub fn translate_reset>( &mut self, ids: impl IntoIterator, diff --git a/unified/ql/lib/codeql/unified/internal/AstExtra.qll b/unified/ql/lib/codeql/unified/internal/AstExtra.qll index ba625aad5fe7..e14043dc46d2 100644 --- a/unified/ql/lib/codeql/unified/internal/AstExtra.qll +++ b/unified/ql/lib/codeql/unified/internal/AstExtra.qll @@ -44,7 +44,7 @@ module Public { class Comment extends TriviaToken { // At the moment, comments are the only type trivia token we extract /** - * Gets the text inside this comment, not counting the delimeters. + * Gets the text inside this comment, not counting the delimiters. */ string getCommentText() { result = this.getValue().regexpCapture("//(.*)", 1) diff --git a/unified/ql/lib/codeql/unified/internal/Variables.qll b/unified/ql/lib/codeql/unified/internal/Variables.qll index a7e8160deb86..97629f2c57ff 100644 --- a/unified/ql/lib/codeql/unified/internal/Variables.qll +++ b/unified/ql/lib/codeql/unified/internal/Variables.qll @@ -12,10 +12,10 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig 0, // $ access=x1 let x = x, // $ access=x1 // name=x2 - x > 0 else { // $ access=x2 + x > 0 { // $ access=x2 print(x) // $ access=x2 } } @@ -319,7 +319,7 @@ func t35(optional: Int?) { // name=optional1 func t36(a: Int?, b: Int?) { while let x = a, // $ access=a let y = b, // $ access=b - x < y { // $ $ access=x access=y + x < y { // $ access=x access=y print(x) // $ access=x print(y) // $ access=y }