Unified: loosen AST schema - #22498
Open
asgerf wants to merge 15 commits into
Open
Conversation
| final override F::AstNode getAFieldOrChild() { unified_expr_equality_pattern_def(this, result) } | ||
| } | ||
| /** Gets the node corresponding to the field `modifier`. */ | ||
| final F::Modifier getModifier(int i) { unified_expr_pattern_modifier(this, i, result) } |
|
|
||
| /** Gets the node corresponding to the field `type_argument`. */ | ||
| final F::TypeExpr getTypeArgument(int i) { | ||
| final F::Expr getTypeArgument(int i) { |
|
|
||
| /** Gets the node corresponding to the field `modifier`. */ | ||
| final F::Modifier getModifier(int i) { unified_name_pattern_modifier(this, i, result) } | ||
| final F::Modifier getModifier(int i) { unified_named_pattern_modifier(this, i, result) } |
Contributor
Author
|
@copilot investigate the failures in 'unified language tests' fix the underlying problems. (Ignore CI checks about QLdoc) |
unified: Rename back to Identifier
asgerf
force-pushed
the
unified/loosen-schema
branch
from
September 4, 2026 11:17
358fe1a to
99ffe04
Compare
asgerf
marked this pull request as ready for review
September 7, 2026 10:45
asgerf
marked this pull request as draft
September 7, 2026 10:47
asgerf
marked this pull request as ready for review
September 7, 2026 10:49
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Pattern traversal currently creates spurious local bindings for wildcards and syntax-only call children.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll — Pattern traversal now follows every nested Expr, so syntax-only children also become… |
|
unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll — A Swift wildcard is not a declaration, but it is now represented as an Identifier. In declaration… |
|
unified/extractor/ast_types.yml — This documentation still says simple parameters use named_pattern, but the updated Swift mapping… |
What changed in this PR
Refactors Unified AST expressions, patterns, and types into a looser shared expression hierarchy to support ambiguous Swift syntax.
Changes:
- Unifies expression, pattern, and type nodes and standardizes name-node getters.
- Updates Swift extraction and name-binding behavior.
- Regenerates schema fixtures and expands regression coverage.
| File | Description |
|---|---|
unified/ql/test/library-tests/local-name-binding/test.swift |
Tests guard pattern references. |
unified/ql/test/library-tests/BasicTest/test.ql |
Queries identifiers and named patterns. |
unified/ql/test/library-tests/BasicTest/test.expected |
Updates expected query output. |
unified/ql/lib/utils/test/TestUtils.qll |
Uses string name getters. |
unified/ql/lib/unified.dbscheme |
Regenerates the Unified schema. |
unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll |
Adapts static name binding. |
unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll |
Classifies Swift pattern identifiers. |
unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll |
Adds pattern-classification extension point. |
unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll |
Traverses unified expression patterns. |
unified/ql/lib/codeql/unified/internal/FacadeAst.qll |
Adds string and enclosing-expression helpers. |
unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll |
Adapts label accessors. |
unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll |
Updates terminology. |
unified/extractor/tests/corpus/swift/variables/var-without-initialiser.output |
Regenerates Swift AST snapshot. |
unified/extractor/tests/corpus/swift/variables/var-binding.output |
Regenerates Swift AST snapshot. |
unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output |
Regenerates tuple-pattern snapshot. |
unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output |
Regenerates observer snapshot. |
unified/extractor/tests/corpus/swift/variables/multiple-bindings-on-one-line.output |
Regenerates binding snapshot. |
unified/extractor/tests/corpus/swift/variables/let-with-type-annotation.output |
Regenerates typed-binding snapshot. |
unified/extractor/tests/corpus/swift/variables/let-binding.output |
Regenerates binding snapshot. |
unified/extractor/tests/corpus/swift/variables/compound-assignment.output |
Regenerates assignment snapshot. |
unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output |
Regenerates pattern-context snapshot. |
unified/extractor/tests/corpus/swift/variables/assignment.output |
Regenerates assignment snapshot. |
unified/extractor/tests/corpus/swift/types/struct.output |
Regenerates struct snapshot. |
unified/extractor/tests/corpus/swift/types/qualified-type.output |
Regenerates qualified-type snapshot. |
unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.swift |
Adds a protocol property fixture. |
unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output |
Regenerates protocol snapshot. |
unified/extractor/tests/corpus/swift/types/protocol-declaration.output |
Regenerates protocol snapshot. |
unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output |
Regenerates property snapshot. |
unified/extractor/tests/corpus/swift/types/noncopyable-type.output |
Regenerates noncopyable-type snapshot. |
unified/extractor/tests/corpus/swift/types/inline-array-type.output |
Regenerates inline-array snapshot. |
unified/extractor/tests/corpus/swift/types/generic-type-arguments.output |
Regenerates generic-type snapshot. |
unified/extractor/tests/corpus/swift/types/generic-class-parameters-and-constraints.output |
Regenerates generic-class snapshot. |
unified/extractor/tests/corpus/swift/types/function-type-with-sendable-attribute.output |
Regenerates function-type snapshot. |
unified/extractor/tests/corpus/swift/types/function-type-with-convention-attribute.output |
Regenerates function-type snapshot. |
unified/extractor/tests/corpus/swift/types/extension.output |
Regenerates extension snapshot. |
unified/extractor/tests/corpus/swift/types/enum-with-comma-separated-cases-chained-declaration.output |
Regenerates enum snapshot. |
unified/extractor/tests/corpus/swift/types/enum-with-cases.output |
Regenerates enum snapshot. |
unified/extractor/tests/corpus/swift/types/enum-with-associated-values.output |
Regenerates associated-value snapshot. |
unified/extractor/tests/corpus/swift/types/empty-class.output |
Regenerates class snapshot. |
unified/extractor/tests/corpus/swift/types/constructor-with-parameters.output |
Regenerates constructor snapshot. |
unified/extractor/tests/corpus/swift/types/conditional-compilation-in-class-body.output |
Regenerates conditional-compilation snapshot. |
unified/extractor/tests/corpus/swift/types/computed-property.output |
Regenerates computed-property snapshot. |
unified/extractor/tests/corpus/swift/types/class-with-stored-properties.output |
Regenerates stored-property snapshot. |
unified/extractor/tests/corpus/swift/types/class-with-multiple-base-types.output |
Regenerates inheritance snapshot. |
unified/extractor/tests/corpus/swift/types/class-with-method.output |
Regenerates method snapshot. |
unified/extractor/tests/corpus/swift/types/class-with-initializer.output |
Regenerates initializer snapshot. |
unified/extractor/tests/corpus/swift/types/class-inheritance.output |
Regenerates inheritance snapshot. |
unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output |
Regenerates accessor-context snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/try-expression.output |
Regenerates try-expression snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/try-expression-2.output |
Regenerates forced-try snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/throwing-function.output |
Regenerates throwing-function snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/optional-type-annotation.output |
Regenerates optional-type snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/optional-enum-case-binding.output |
Regenerates optional-pattern snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/optional-chaining.output |
Regenerates optional-chain snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/nil-coalescing.output |
Regenerates coalescing snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/force-unwrap.output |
Regenerates unwrap snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/do-catch.output |
Regenerates catch snapshot. |
unified/extractor/tests/corpus/swift/optionals-and-errors/catch-where-clauses.output |
Regenerates conditional-pattern snapshot. |
unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence-with-ternary.output |
Regenerates ternary operator snapshot. |
unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence-with-casts.output |
Regenerates cast operator snapshot. |
unified/extractor/tests/corpus/swift/operators/subtraction.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/partial-range-from.output |
Regenerates range snapshot. |
unified/extractor/tests/corpus/swift/operators/parenthesised-expression.output |
Regenerates parenthesized snapshot. |
unified/extractor/tests/corpus/swift/operators/operator-precedence-addition-and-multiplication.output |
Regenerates precedence snapshot. |
unified/extractor/tests/corpus/swift/operators/multiplication.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/logical-or.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/logical-not.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/logical-and.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/equality.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/division.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/custom-postfix-operator.output |
Regenerates custom-operator snapshot. |
unified/extractor/tests/corpus/swift/operators/comparison.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/operators/addition.output |
Regenerates operator snapshot. |
unified/extractor/tests/corpus/swift/loops/while-loop.output |
Regenerates loop snapshot. |
unified/extractor/tests/corpus/swift/loops/repeat-while-loop.output |
Regenerates loop snapshot. |
unified/extractor/tests/corpus/swift/loops/for-in-with-where-clause.output |
Regenerates for-loop snapshot. |
unified/extractor/tests/corpus/swift/loops/for-in-over-range.output |
Regenerates for-loop snapshot. |
unified/extractor/tests/corpus/swift/loops/for-in-over-array-literal.output |
Regenerates for-loop snapshot. |
unified/extractor/tests/corpus/swift/loops/break-and-continue.output |
Regenerates control-transfer snapshot. |
unified/extractor/tests/corpus/swift/literals/line-magic-literal.output |
Regenerates literal snapshot. |
unified/extractor/tests/corpus/swift/functions/variadic-function.output |
Regenerates function snapshot. |
unified/extractor/tests/corpus/swift/functions/nested-function-type.output |
Regenerates function-type snapshot. |
unified/extractor/tests/corpus/swift/functions/method-call.output |
Regenerates method-call snapshot. |
unified/extractor/tests/corpus/swift/functions/leading-dot-expression-value.output |
Regenerates leading-dot snapshot. |
unified/extractor/tests/corpus/swift/functions/leading-dot-expression-call.output |
Regenerates leading-dot call snapshot. |
unified/extractor/tests/corpus/swift/functions/generic-type-alias.output |
Regenerates type-alias snapshot. |
unified/extractor/tests/corpus/swift/functions/generic-function.output |
Regenerates generic-function snapshot. |
unified/extractor/tests/corpus/swift/functions/function-with-parameters-and-return-type.output |
Regenerates function snapshot. |
unified/extractor/tests/corpus/swift/functions/function-with-no-parameters.output |
Regenerates function snapshot. |
unified/extractor/tests/corpus/swift/functions/function-with-named-parameters.output |
Regenerates named-parameter snapshot. |
unified/extractor/tests/corpus/swift/functions/function-with-inout-parameter.output |
Regenerates inout-parameter snapshot. |
unified/extractor/tests/corpus/swift/functions/function-with-default-parameter-value.output |
Regenerates default-parameter snapshot. |
unified/extractor/tests/corpus/swift/functions/function-call.output |
Regenerates function-call snapshot. |
unified/extractor/tests/corpus/swift/functions/function-call-with-labelled-arguments.output |
Regenerates labeled-call snapshot. |
unified/extractor/tests/corpus/swift/functions/constructor-call-with-type-arguments.output |
Regenerates constructor-call snapshot. |
unified/extractor/tests/corpus/swift/functions/call-with-inout-argument.output |
Regenerates inout-call snapshot. |
unified/extractor/tests/corpus/swift/expressions/unsafe-expression.output |
Regenerates unsupported-expression snapshot. |
unified/extractor/tests/corpus/swift/expressions/key-path-expression.output |
Regenerates key-path snapshot. |
unified/extractor/tests/corpus/swift/expressions/generic-specialization-expression.output |
Regenerates specialization snapshot. |
unified/extractor/tests/corpus/swift/expressions/copy-expression.output |
Regenerates copy-expression snapshot. |
unified/extractor/tests/corpus/swift/expressions/consume-expression.output |
Regenerates consume-expression snapshot. |
unified/extractor/tests/corpus/swift/expressions/array-type-metatype.output |
Regenerates metatype snapshot. |
unified/extractor/tests/corpus/swift/expressions/array-type-constructor.output |
Regenerates array-constructor snapshot. |
unified/extractor/tests/corpus/swift/desugar/simple-import-with-single-name.output |
Regenerates import snapshot. |
unified/extractor/tests/corpus/swift/desugar/scoped-import-uses-name-pattern.output |
Regenerates scoped-import snapshot. |
unified/extractor/tests/corpus/swift/desugar/import-with-dotted-path-two-parts.output |
Regenerates dotted-import snapshot. |
unified/extractor/tests/corpus/swift/desugar/import-with-deeply-nested-path-three-parts.output |
Regenerates nested-import snapshot. |
unified/extractor/tests/corpus/swift/desugar/another-additive-expression-is-desugared.output |
Regenerates desugaring snapshot. |
unified/extractor/tests/corpus/swift/control-flow/ternary-expression.output |
Regenerates ternary snapshot. |
unified/extractor/tests/corpus/swift/control-flow/switch-with-labeled-case-pattern-arguments.output |
Regenerates labeled-pattern snapshot. |
unified/extractor/tests/corpus/swift/control-flow/switch-with-binding-pattern.output |
Regenerates binding-pattern snapshot. |
unified/extractor/tests/corpus/swift/control-flow/switch-statement.output |
Regenerates switch snapshot. |
unified/extractor/tests/corpus/swift/control-flow/switch-case-item-where-clauses.output |
Regenerates guarded-case snapshot. |
unified/extractor/tests/corpus/swift/control-flow/nested-enum-case-pattern.output |
Regenerates nested-pattern snapshot. |
unified/extractor/tests/corpus/swift/control-flow/if-statement.output |
Regenerates conditional snapshot. |
unified/extractor/tests/corpus/swift/control-flow/if-let-optional-binding.output |
Regenerates optional-binding snapshot. |
unified/extractor/tests/corpus/swift/control-flow/if-else.output |
Regenerates conditional snapshot. |
unified/extractor/tests/corpus/swift/control-flow/if-else-if-chain.output |
Regenerates conditional-chain snapshot. |
unified/extractor/tests/corpus/swift/control-flow/if-case-let-with-shadowing-in-condition-value.swift |
Adds mutable shadowing coverage. |
unified/extractor/tests/corpus/swift/control-flow/if-case-let-with-shadowing-in-condition-value.output |
Regenerates shadowing snapshot. |
unified/extractor/tests/corpus/swift/control-flow/guard-let.output |
Regenerates guard snapshot. |
unified/extractor/tests/corpus/swift/control-flow/fallthrough.output |
Regenerates fallthrough snapshot. |
unified/extractor/tests/corpus/swift/control-flow/discard-statement.output |
Regenerates discard snapshot. |
unified/extractor/tests/corpus/swift/control-flow/defer-statement.output |
Regenerates defer snapshot. |
unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output |
Regenerates pattern-context snapshot. |
unified/extractor/tests/corpus/swift/collections/tuple-member-access.output |
Regenerates tuple-access snapshot. |
unified/extractor/tests/corpus/swift/collections/tuple-literal.output |
Regenerates tuple snapshot. |
unified/extractor/tests/corpus/swift/collections/subscript-access.output |
Regenerates subscript snapshot. |
unified/extractor/tests/corpus/swift/collections/set-literal.output |
Regenerates set snapshot. |
unified/extractor/tests/corpus/swift/collections/empty-array-literal-with-type.output |
Regenerates array snapshot. |
unified/extractor/tests/corpus/swift/collections/dictionary-subscript.output |
Regenerates dictionary-access snapshot. |
unified/extractor/tests/corpus/swift/collections/dictionary-literal.output |
Regenerates dictionary snapshot. |
unified/extractor/tests/corpus/swift/collections/array-literal.output |
Regenerates array snapshot. |
unified/extractor/tests/corpus/swift/closures/trailing-closure.output |
Regenerates closure snapshot. |
unified/extractor/tests/corpus/swift/closures/multi-statement-closure.output |
Regenerates closure snapshot. |
unified/extractor/tests/corpus/swift/closures/closure-with-shorthand-parameters.output |
Regenerates shorthand-closure snapshot. |
unified/extractor/tests/corpus/swift/closures/closure-with-explicit-parameters.output |
Regenerates explicit-closure snapshot. |
unified/extractor/tests/corpus/swift/closures/closure-with-capture-list.output |
Regenerates capture-list snapshot. |
unified/extractor/src/languages/swift/swift.rs |
Emits the loosened Unified AST. |
unified/extractor/ast_types.yml |
Defines the merged AST schema. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+195
to
+197
| e instanceof MemberAccessExpr | ||
| or | ||
| exists(TypeAliasDeclaration decl | | ||
| getChild(scope, _) = decl and | ||
| pattern = decl.getName() and | ||
| declaration = decl | ||
| ) | ||
| or | ||
| exists(TypeParameter param | | ||
| scope = param.getParent() and | ||
| pattern = param.getName() and | ||
| declaration = param | ||
| ) | ||
| or | ||
| exists(AssociatedTypeDeclaration decl | | ||
| getChild(scope, _) = decl and | ||
| pattern = decl.getName() and | ||
| declaration = decl | ||
| ) | ||
| or | ||
| exists(AccessorDeclaration decl | | ||
| getChild(scope, _) = decl and | ||
| pattern = decl.getName() and | ||
| declaration = decl | ||
| ) | ||
| or | ||
| exists(ImportDeclaration imprt | | ||
| getChild(scope, _) = imprt and | ||
| pattern = imprt.getPattern() and | ||
| declaration = imprt | ||
| ) | ||
| or | ||
| exists(NamePattern p | | ||
| bindingContext(p, scope, declaration) and | ||
| pattern = p.getIdentifier() | ||
| any(NameBindingPlugin p).isNonPattern(e) |
|
|
||
| class NameBindingPluginSwift extends NameBindingPlugin { | ||
| bindingset[e] | ||
| override predicate isNonPattern(Expr e) { isUnboundPattern(e.(Identifier)) } |
Comment on lines
227
to
+232
| # `pattern` binds the parameter's internal name(s). For a simple parameter this is a | ||
| # `name_pattern`, but may be an arbitrary pattern for languages where patterns may appear | ||
| # in the parameter list. | ||
| # `named_pattern`, but may be an arbitrary expression where languages allow destructuring. | ||
| # | ||
| # `external_name` is the name by which to call sites refer to the parameter, if the parameter | ||
| # can be passed as a named parameter. For example, the Swift function `func greet(person id: String)` | ||
| # would have `person` as the external name and a `name_pattern` wrapping `id` is the parameter's pattern. | ||
| # would have `person` as the external name and a `named_pattern` wrapping `id` as the parameter's pattern. |
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.


Refactors the AST to be more loosely typed so that it becomes feasible to translate of various cases of ambiguous syntax into a valid AST. Previously we ended up with invalid ASTs in many cases, often manifesting as "holes" in the AST.
Changes
Expr,Pattern, andTypeExprare merged into a single type, calledExpr. The union types are merged and some of their concrete types likeTupleExprandTuplePatternare merged as well.Identifiersmay now appear in directly in expr/pattern/type context.identifier, giving languages the freedom to classify names where it happens to be easiest. But there isn't a strong need right now, and I see it as a relatively safe refactoring that we can do in the future.Identifierare also renamed to clarify the distinction between two kinds of getters we see in several contexts. We now use the convention:getName()->stringgetNameNode()->Identifier.Ambiguous syntax
As mentioned, the motivation for loosening the AST is to be able to handle ambiguous syntax. For example, in a pattern such as
case Foo.bar(3), we cannot distinguish between these interpretations at translation-time:Foo.barresolves to an enum case with data parameters: this is a constructor pattern, destructuring the incoming value.Foo.barresolves to a static method: this is an expr-equality pattern comparing the incoming value to the return value of the static call.With a more loosely-typed AST, it is mapped to a
CallExprin both cases, and we can recover the distinction by introducing subclasses after the name-binding pass (not done in this PR).See internal issue for more detailed write-up about such cases of ambiguous syntax in Swift.
ExprEqualityPattern
Note that we not longer insert
ExprEqualityPatternat the boundary between expressions and patterns. We previously made a best-effort attempt at this, but produced a malformed AST when we failed to insert it correctly. It might make sense to re-introduce this class and insert it heuristically where we can, but let's wait until we have a more pressing use-case.