unified: improve node locations by tausbn · Pull Request #22608 · github/codeql

tausbn · 2026-09-17T12:37:58Z

Changes the location-assigning heuristics to implement roughly the following principles:

A node that is translated recursively gets its location assigned first, and this location is not modified by the outer rule. (However, it can affect the outer location.)
If an intermediate node is synthesised (e.g. middle in (outer foo: (middle bar: {baz}))), then it gets a location that spans all of its children (that is, all of the captures that have already been translated, that are assigned as descendants of this node).
If an intermediate node has no children that give it a location (and the location is not otherwise set), it gets an empty location at the start of the node that led to this rule match in the first place. (Thus, it's generally in the correct area, even if there isn't a specific bit of source code we can point to as the appropriate "location".)
The outermost node of the rule body (i.e. outer above) gets the location of the entire node that matched that rule (unless that location has been assigned already, e.g. by recursive translation or explicit assignment).
Interpolated node IDs carry their location. If you write (identifier #{name}), where name is some captured node ID, then the identifier node gets the location of that capture, whereas the string value is what is actually interpolated.
If a node is constructed with an explicit location, that location is respected and is not subsequently widened to the entire rule match.

One consequence of the above rules is that if you have something like

(foo bar: @baz) => expr {baz}

then the location of the node emitted by the right hand side is not that of foo, but rather whatever is assigned to baz when it is recursively translated.

In cases where this heuristic is insufficient, newly added tree_at! and tree_spanning! macros may be employed. These take a second argument that specifies the node (or iterable of nodes) from which to take the location of the tree being constructed.

Should be reviewed commit-by-commit. I checked a bunch of the location changes in the last commit manually (and instructed Copilot to check the rest). In all cases, the locations are an improvement on what was there before (e.g. a call inside a tuple accidentally including the following , in its range). This does produce a fair amount of churn, but I think it's somewhat unavoidable.

NB: This temporarily changes the test output for bulk imports, blocks, and things like synthesised Array and Option literals. Later commits will fix these up again.

The location 0..0 was being treated as "no useable location", which at the beginning of a file could cause problems. We now represent this as None instead, making Some(0..0) a valid empty location.

Adds two new macros, both variants of the existing `tree!` macro. First, `tree_at!` takes an extra argument, and sets the location of the root of the constructed tree to be that of the argument in question. Secondly, `tree_spanning!` does the same construction, but accepts an iterable of nodes instead. It then makes it so that the location of the root node of the constructed tree is the smallest span that contains all of the locations given by the iterable.

For such nodes, we assign them the zero-length location at the beginning of the matched input node. Note: This does mean some `expr_pattern`s now appear to have a fixed `let ` prefix in their source range (when really that `let` should belong to the modifier). This will be fixed in a later commit.

Blocks are slightly awkward, since we destructure them (as codeBlock) in the query, and then reconstruct them (as `block`) in the rule body, meaning the location of the block is (by default) assigned to the wrong place by the heuristic. To get around this, I added a helper function that updates the location appropriately, including handling cases where we only optionally match a block. (Also, in some cases we can fix this by just not destructuring `codeBlock`s in the first place -- there's already a rule that maps `codeBlock` to `block`.)

Handles things like `try!` (which is represented as two separate tokens -- we explicitly union their ranges) and "let" binding modifiers (where we reuse the bindingSpecifier, getting its location and string value for free).

Anchor synthesized generic type expressions for Swift array syntax to the original array node. This keeps the closing bracket in the type range without leaking it into surrounding calls or member accesses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Trailing commas appear in a lot of places in the swift-syntax AST, and are all attached to the individual values separated by said commas, not the parent node. This means that the default heuristics thinks a call like `foo(1, 2)` has arguments `1,` and `2`, which is undesirable. Since this applies uniformly for a lot of different comma-separated lists, I decided that adding a generic mechanism for ignoring certain fields was the least invasive change. Thus, we now add `trailingComma` to that list, and this means it's automatically excluded from the location calculations, without need a bunch of specific changes to the existing rules.

In some cases (I'm looking at you, access_declaration) we link up things like the modifier of an accessor to a node that lives _outside_ of the declaration itself. Previously this was treated as an error (and the "fix" was to extend the source ranges), but this lead to some weird source skeletons in the output. Instead, we now allow the source of a node to be anywhere (within reason -- it has to be inside the file, and properly UTF-8 aligned), and simply add a trailing ` (external)` to the source annotation. Thus, the fields containing these values do not appear in the source skeleton for the node in question.

Keep inherited property metadata at its original source while limiting each accessor declaration to its own keyword and optional body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Boundary exclusion can create invalid ranges, and bodyless Swift initializers lose their name node.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

Return immediately when ignored range equals matched range · New
Preserve initializer name for bodyless protocol constructors · New

What changed in this PR

Improves Yeast source-range propagation and applies the new location semantics to Swift extraction.

Changes:

Adds child-derived ranges, explicit location macros, boundary exclusions, and external-child diagnostics.
Updates Swift desugaring to produce more precise node locations.
Adds focused tests and regenerates Swift corpus and QL expectations.

File	Description
`unified/ql/test/library-tests/dataflow/test.expected`	Updates data-flow locations.
`unified/ql/test/library-tests/controlflow/cfg.swift`	Updates inline CFG assertions.
`unified/ql/test/library-tests/controlflow/cfg.expected`	Updates CFG expectations.
`unified/ql/test/library-tests/controlflow/basicblock-slices.expected`	Updates block-range expectations.
`unified/ql/test/library-tests/BasicTest/test.expected`	Updates identifier locations.
`unified/extractor/tests/location_tests.rs`	Adds focused location tests.
`unified/extractor/tests/corpus/swift/variables/tuple-destructuring-binding.output`	Regenerates tuple-binding output.
`unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output`	Regenerates observer output.
`unified/extractor/tests/corpus/swift/variables/multiple-bindings-on-one-line.output`	Regenerates binding locations.
`unified/extractor/tests/corpus/swift/variables/binding-modifier-does-not-leak-into-initializer.output`	Regenerates switch locations.
`unified/extractor/tests/corpus/swift/types/static-function.output`	Regenerates function locations.
`unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output`	Regenerates accessor locations.
`unified/extractor/tests/corpus/swift/types/protocol-declaration.output`	Regenerates protocol output.
`unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output`	Regenerates property output.
`unified/extractor/tests/corpus/swift/types/noncopyable-type.output`	Regenerates base-type locations.
`unified/extractor/tests/corpus/swift/types/generic-class-parameters-and-constraints.output`	Regenerates generic locations.
`unified/extractor/tests/corpus/swift/types/extension.output`	Regenerates extension output.
`unified/extractor/tests/corpus/swift/types/enum-with-comma-separated-cases-chained-declaration.output`	Regenerates enum-case output.
`unified/extractor/tests/corpus/swift/types/enum-with-cases.output`	Regenerates enum locations.
`unified/extractor/tests/corpus/swift/types/enum-with-associated-values.output`	Regenerates enum constructors.
`unified/extractor/tests/corpus/swift/types/constructor-with-parameters.output`	Regenerates constructor locations.
`unified/extractor/tests/corpus/swift/types/computed-property.output`	Regenerates computed-property output.
`unified/extractor/tests/corpus/swift/types/class-with-multiple-base-types.output`	Regenerates base-type ranges.
`unified/extractor/tests/corpus/swift/types/class-with-method.output`	Regenerates method locations.
`unified/extractor/tests/corpus/swift/types/class-with-initializer.output`	Regenerates initializer locations.
`unified/extractor/tests/corpus/swift/types/class-inheritance.output`	Regenerates inheritance output.
`unified/extractor/tests/corpus/swift/types/class-function.output`	Regenerates class-function output.
`unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output`	Regenerates accessor-body output.
`unified/extractor/tests/corpus/swift/optionals-and-errors/try-expression.output`	Regenerates `try?` locations.
`unified/extractor/tests/corpus/swift/optionals-and-errors/try-expression-2.output`	Regenerates `try!` locations.
`unified/extractor/tests/corpus/swift/optionals-and-errors/throwing-function.output`	Regenerates throwing-function output.
`unified/extractor/tests/corpus/swift/optionals-and-errors/optional-type-annotation.output`	Regenerates optional-type output.
`unified/extractor/tests/corpus/swift/optionals-and-errors/optional-enum-case-binding.output`	Regenerates optional-pattern output.
`unified/extractor/tests/corpus/swift/optionals-and-errors/force-unwrap.output`	Regenerates unwrap locations.
`unified/extractor/tests/corpus/swift/optionals-and-errors/do-catch.output`	Regenerates `do-catch` output.
`unified/extractor/tests/corpus/swift/optionals-and-errors/catch-where-clauses.output`	Regenerates catch-pattern locations.
`unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence.output`	Regenerates operator output.
`unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence-with-ternary.output`	Regenerates ternary-sequence output.
`unified/extractor/tests/corpus/swift/operators/unresolved-operator-sequence-with-casts.output`	Regenerates cast-sequence output.
`unified/extractor/tests/corpus/swift/literals/string-with-interpolation.output`	Regenerates interpolation locations.
`unified/extractor/tests/corpus/swift/functions/variadic-function.output`	Regenerates variadic-function output.
`unified/extractor/tests/corpus/swift/functions/nested-function-type.output`	Regenerates nested-function-type output.
`unified/extractor/tests/corpus/swift/functions/generic-type-alias.output`	Regenerates type-alias locations.
`unified/extractor/tests/corpus/swift/functions/generic-function.output`	Regenerates generic-function output.
`unified/extractor/tests/corpus/swift/functions/function-with-parameters-and-return-type.output`	Regenerates function signature output.
`unified/extractor/tests/corpus/swift/functions/function-with-no-parameters.output`	Regenerates function-body locations.
`unified/extractor/tests/corpus/swift/functions/function-with-named-parameters.output`	Regenerates named-parameter output.
`unified/extractor/tests/corpus/swift/functions/function-with-inout-parameter.output`	Regenerates inout-function output.
`unified/extractor/tests/corpus/swift/functions/function-with-default-parameter-value.output`	Regenerates default-parameter output.
`unified/extractor/tests/corpus/swift/functions/function-call.output`	Regenerates call locations.
`unified/extractor/tests/corpus/swift/functions/call-with-inout-argument.output`	Regenerates inout-call output.
`unified/extractor/tests/corpus/swift/expressions/unsafe-expression.output`	Regenerates unsafe-expression output.
`unified/extractor/tests/corpus/swift/expressions/super-expression.output`	Regenerates superclass locations.
`unified/extractor/tests/corpus/swift/expressions/array-type-metatype.output`	Regenerates array metatype output.
`unified/extractor/tests/corpus/swift/expressions/array-type-constructor.output`	Regenerates array-constructor output.
`unified/extractor/tests/corpus/swift/control-flow/switch-with-labeled-case-pattern-arguments.output`	Regenerates switch-pattern output.
`unified/extractor/tests/corpus/swift/control-flow/switch-with-binding-pattern.output`	Regenerates binding-pattern output.
`unified/extractor/tests/corpus/swift/control-flow/switch-statement.output`	Regenerates switch locations.
`unified/extractor/tests/corpus/swift/control-flow/switch-expression-pattern.output`	Regenerates expression-pattern output.
`unified/extractor/tests/corpus/swift/control-flow/switch-case-item-where-clauses.output`	Regenerates conditional-pattern output.
`unified/extractor/tests/corpus/swift/control-flow/nested-enum-case-pattern.output`	Regenerates nested-pattern output.
`unified/extractor/tests/corpus/swift/control-flow/if-let-optional-binding.output`	Regenerates optional-binding output.
`unified/extractor/tests/corpus/swift/control-flow/guard-let.output`	Regenerates guard-binding output.
`unified/extractor/tests/corpus/swift/control-flow/fallthrough.output`	Regenerates fallthrough output.
`unified/extractor/tests/corpus/swift/control-flow/discard-statement.output`	Regenerates discard-statement output.
`unified/extractor/tests/corpus/swift/control-flow/defer-statement.output`	Regenerates defer-statement output.
`unified/extractor/tests/corpus/swift/control-flow/binding-modifier-does-not-leak-to-sibling.output`	Regenerates sibling-binding output.
`unified/extractor/tests/corpus/swift/collections/tuple-literal.output`	Regenerates tuple locations.
`unified/extractor/tests/corpus/swift/collections/empty-array-literal-with-type.output`	Regenerates array-type output.
`unified/extractor/tests/corpus/swift/closures/trailing-closure.output`	Regenerates trailing-closure output.
`unified/extractor/src/languages/swift/swift.rs`	Applies precise Swift location construction.
`unified/extractor/BUILD.bazel`	Registers location tests.
`shared/yeast/tests/test.rs`	Tests new range semantics.
`shared/yeast/src/range.rs`	Adds range operations and exclusions.
`shared/yeast/src/lib.rs`	Adds location APIs and configuration.
`shared/yeast/src/dump.rs`	Marks external child locations.
`shared/yeast/src/build.rs`	Tracks and finalizes constructed nodes.
`shared/yeast/doc/yeast.md`	Documents location behavior.
`shared/yeast-macros/src/parse.rs`	Finalizes rule results.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tausbn · 2026-09-22T14:47:37Z

Rerun has been triggered: 6 restarted 🚀

tausbn added the no-change-note-required This PR does not need a change note label Sep 17, 2026

github-actions Bot added the documentation label Sep 17, 2026

tausbn force-pushed the tausbn/unified-improve-node-locations branch 7 times, most recently from f662105 to 1a9354d Compare September 22, 2026 11:30

tausbn and others added 11 commits September 22, 2026 11:40

yeast: Track child source ranges in separate contexts

bc1b012

NB: This temporarily changes the test output for bulk imports, blocks, and things like synthesised Array and Option literals. Later commits will fix these up again.

yeast: Handle empty locations correctly

15e4a20

The location 0..0 was being treated as "no useable location", which at the beginning of a file could cause problems. We now represent this as None instead, making Some(0..0) a valid empty location.

Unified: Fix locations of various tokens

c953ebd

Handles things like `try!` (which is represented as two separate tokens -- we explicitly union their ranges) and "let" binding modifiers (where we reuse the bindingSpecifier, getting its location and string value for free).

Unified: Format Swift translation rules

04a6768

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Unified: Give accessors their own source ranges

a6ffe1f

Keep inherited property metadata at its original source while limiting each accessor declaration to its own keyword and optional body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tausbn force-pushed the tausbn/unified-improve-node-locations branch from 1a9354d to 985155a Compare September 22, 2026 11:42

Unified: Update location test expectations

364d59c

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tausbn force-pushed the tausbn/unified-improve-node-locations branch from 985155a to 364d59c Compare September 22, 2026 13:18

tausbn requested a balanced review from Copilot September 22, 2026 13:44

Copilot started reviewing on behalf of tausbn September 22, 2026 13:44 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread shared/yeast/src/range.rs Outdated

Comment thread unified/extractor/src/languages/swift/swift.rs

tausbn and others added 2 commits September 22, 2026 16:05

yeast: Fix condition to return empty_at_start for range match

d9211f7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Unified: Update Swift initializer declaration rule

624cdfe

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

tausbn marked this pull request as ready for review September 22, 2026 14:47

tausbn requested review from a team as code owners September 22, 2026 14:47

Navigation Menu

unified: improve node locations - #22608

unified: improve node locations#22608
tausbn wants to merge 14 commits into
mainfrom
tausbn/unified-improve-node-locations

tausbn commented Sep 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

tausbn commented Sep 22, 2026 •

edited by codeql-ci-app Bot

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tausbn commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

tausbn commented Sep 22, 2026 • edited by codeql-ci-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tausbn commented Sep 17, 2026 •

edited

Loading

tausbn commented Sep 22, 2026 •

edited by codeql-ci-app Bot

Loading