Include elided tokens in yeast AST node locations

[Copilot]

Yeast rewrites can elide matched tokens when producing the target AST, causing synthesized node locations to shrink to only their retained children. For cases like return x, this made the rewritten return_expr span only x instead of the full return statement.

• Location synthesis

  • Union synthesized node child ranges with the matched source node range.
  • Preserve existing child-derived behavior while allowing elided matched tokens to contribute to the replacement range.

• Regression coverage

  • Added a yeast regression test where a rule drops part of a matched node but the replacement node still spans the original match.

Example:

rule!(
    (call
        method: (identifier) @name
        receiver: (identifier) @recv
    )
    =>
    (call
        method: {name}
    )
)

The synthesized call now keeps the source range of the full matched foo.bar() expression, not just the retained bar child.

[Copilot]

🟢 Approval recommended

The implementation matches the stated behavior and includes focused regression coverage.

Pull request overview

Extends Yeast synthesized-node locations to include elided portions of matched source nodes.

Changes:

• Unions child-derived and inherited match ranges.
• Adds regression coverage for elided tokens.

File summaries

File	Description
shared/yeast/src/lib.rs	Implements source-range unioning.
shared/yeast/tests/test.rs	Verifies the full matched call range is retained.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

---

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

[tausbn]

Looks good to me! 👍

While I'm not sure if this is the solution we want to use long term, I think this a perfectly good solution right now, especially if it unblocks other work.

[tausbn]

On second thought, looking at the test failure:

--- expected
+++ actual
@@ -1,6 +1,6 @@
 nameExpr
 | name_expr.swift:1:9:1:9 | NameExpr | y |
-| test.swift:1:8:1:17 | NameExpr | Foundation |
+| test.swift:1:1:1:17 | NameExpr | Foundation |
 | test.swift:8:9:8:13 | NameExpr | items |
 | test.swift:8:22:8:25 | NameExpr | item |
 | test.swift:12:16:12:20 | NameExpr | items |
Error: [7/10 comp 2.6s eval 44ms] FAILED(RESULT) /home/runner/work/semmle-code/semmle-code/ql/unified/ql/test/library-tests/BasicTest/test.ql

The code in question is import Foundation, and I'm not sure it's sensible to have the location of the NameExpr be the entire import statement.

[aschackmull]

The code in question is import Foundation, and I'm not sure it's sensible to have the location of the NameExpr be the entire import statement.

That's certainly true - I've no idea why that happens. I'll dig a bit.

[tausbn]

I think we need something that treats the matched "top-level" node of a rule specially.

So, something like:

• The top-level node of the output of a rule gets the location of the outermost node that was matched.
• A child node containing a single capture gets the location of that capture.
• If there are multiple captures, we union their ranges.

I'm not quite sure what to do about childless synthesised nodes (if we have any). Perhaps these should just inherit the range surrounding them.

[aschackmull]

Copilot claims to have fixed it now. I admit I don't have a good enough overview of the code to have complete confidence, but testing locally it appears to work.