Unified: implement local scoping#22120
Open
asgerf wants to merge 13 commits into
Open
Conversation
Also move into 'internal' and expose through a Public module
As documented in the test, 'guard if' statements are still not properly supported.
The original test was not valid Swift syntax
- Context is fully reset when stepping into an expr/stmt/body. - Binding modifier is rolled into outer_modifiers.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+17
to
+19
| /** | ||
| * Declaration of a local or top-level variable. | ||
| */ |
Comment on lines
+29
to
+31
| /** | ||
| * Declaration of a local or top-level function. | ||
| */ |
| // TODO: self | ||
| } | ||
|
|
||
| predicate accessCand(AstNode n, string name) { |
| */ | ||
| predicate relevantNode(AstNode node) { | ||
| // Match an ancestor node by location so its whole subtree is shown. | ||
| node.getParent*().getLocation().toString().matches("%test.swift@227:%") |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces lexical (local) scoping support to the Unified libraries by adding a name-binding–based Variables library, restructuring Unified’s public surface to re-export internal Public modules via unified.qll, and updating the Swift Yeast translation to correctly distinguish binding patterns from expression-equality patterns while preventing modifier/context leakage into unrelated subtrees.
Changes:
- Add a Unified local-variable model (
Variables) backed bycodeql/namebinding, plus a developer debug query for the scope graph. - Update Swift extractor translation to use context-passing + context resets so patterns are translated correctly and binding modifiers don’t leak into initializers/bodies.
- Add/expand Swift corpus and Unified library-tests covering variable binding, shadowing, and guard/conditional scoping behaviors.
Show a summary per file
| File | Description |
|---|---|
| unified/ql/test/library-tests/variables/variables.ql | Adds an InlineExpectationsTest query to validate VariableAccess→Variable resolution via inline comments. |
| unified/ql/test/library-tests/variables/variables.expected | Inline-expectations .expected placeholder for the new test (expected to be empty). |
| unified/ql/test/library-tests/variables/test.swift | Adds a comprehensive Swift fixture exercising shadowing and scoping scenarios. |
| unified/ql/lib/unified.qll | Switches Unified’s public imports to re-export internal Public modules (Variables/Ast extras). |
| unified/ql/lib/qlpack.yml | Adds dependency on codeql/namebinding to support local name binding. |
| unified/ql/lib/codeql/unified/internal/Variables.qll | Implements local variable declarations/accesses and scope lookup wiring for Unified. |
| unified/ql/lib/codeql/unified/internal/dev/debugScopeGraph.ql | Adds a dev-only graph query to render/debug the local scope graph. |
| unified/ql/lib/codeql/unified/internal/AstExtra.qll | Adds non-generated AST helper classes (including Comment) under an internal module. |
| unified/ql/lib/codeql/unified/Comments.qll | Removes the old comments helper (moved under internal AstExtra). |
| unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output | Updates expected AST output for tuple destructuring bindings. |
| unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.swift | Adds corpus fixture ensuring binding modifiers don’t leak into initializer translation. |
| unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output | Adds expected output for the initializer leakage fixture. |
| unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.swift | Adds corpus fixture ensuring binding modifiers don’t leak into accessor bodies. |
| unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output | Adds expected output for the accessor-body leakage fixture. |
| unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.swift | Adds corpus fixture ensuring binding modifiers don’t leak to sibling statements. |
| unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output | Adds expected output for the sibling leakage fixture. |
| unified/extractor/src/languages/swift/swift.rs | Updates Swift Yeast rules to publish binding context, translate/reset subtrees, and pattern-translate identifiers appropriately. |
| shared/yeast/src/build.rs | Adds translate_reset helper to translate captures under a fresh user context. |
| shared/namebinding/codeql/namebinding/LocalNameBinding.qll | Adjusts the namebinding interface and adds a debug scope graph helper module. |
| ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll | Updates Ruby implementation to match the adjusted namebinding signature. |
Review details
- Files reviewed: 19/20 changed files
- Comments generated: 7
- Review effort level: Low
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds local scoping and does a few other things in order to get that working:
codeql.unified.internaland imports theirPublicmodules intounified.qll. Makes it easy to share internal code between the libraries.name_patternorexpr_equality_patternwrapping aname_expr. In the future we might want to move this logic into QL but I wanted to make sure that Yeast can handle this kind of problem if we need it to. The commit also fixes a bunch of potential context-leaking issues where modifiers would leak into expr/stmt subtrees.guardstatements could not be natively handled by the shared library, but it turns out can handle it by flattening their children into the enclosingBlock.