Add isTerminal and previousInChain getters to AstNode - #1788
Merged
Conversation
Expression chains like `alpha.beta.charlie` are stored inverted in the AST, and walking `.parent` can't tell when you've left one chain and landed in an unrelated enclosing expression: in `doSomething(alpha.beta)`, walking up from `alpha` reaches the CallExpression even though `alpha.beta` is only an argument. Chain membership is determined by which parent property holds the child, so each chain-forming node now declares its single chain link via `chainChild`. `chainParent` returns the parent only when this node is that parent's chain link, which gives the boundary-aware walks (`getChainEnd`, `getChainStart`, `getChain`) and the `isTerminal`/`isChainStart` predicates. Boundaries need no code - the base returns undefined, so groupings, binary operands, literals, call args and indexes stop a chain by default. That also replaces the hardcoded `isTerminal` overrides, including the one on Statement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/parser/AstNode.spec.ts # src/parser/AstNode.ts
The chain traversal helpers (chainParent, isChainStart, getChainEnd, getChainStart, getChain) had no callers outside their own tests. Drop them and keep the two members that answer the isTerminal question: `chainChild`, which each wrapping node declares, and `isTerminal()`, which reads it off the parent. Also rewrite the doc comments to lead with a concrete example rather than prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge silently kept the stale side of several auto-resolved files, which reverted upstream's findAncestor inference cleanup (#1785): the type-guard overload signatures on AstNode.findAncestor, its test suite, and the now- redundant explicit type arguments across 13 other files. Rebuild AstNode.ts and AstNode.spec.ts from origin/master and re-apply only the chain additions, so the branch is now purely additive against master. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Behavior is unchanged; `parent?.chainChild !== this` already returned true for a parentless node. Naming that branch explicitly makes it clear the case is deliberate rather than a side effect of optional chaining. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`chainChild` described the AST direction but read backwards: in `a.b.c`, the "child" is `a.b`, which sits to the left in source and reads like a parent. `previousInChain` is unambiguous, since each node spans a whole prefix and this walks to the next-shorter one. Also reword the docs around that framing, and note explicitly that this is not `parent` reversed - args and index values have a parent but are never a previousInChain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`print a.b.c(1)` made the argument row hard to read, since the `1` in the listing and the `(1)` in the source line look like the same thing. Use `arg` for arguments and `i` for index values so each row names one distinct role, and show that a longer argument is terminal too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This test timed out at 20s on the macos CI runner. It spawns a worker thread and talks to it over a socket, which is the same cold-boot cost the other worker-thread tests in this file already budget 60s for, so match them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isTerminal and previousInChain getters to AstNode
TwitchBronBron
enabled auto-merge (squash)
September 8, 2026 17:56
Merged
TwitchBronBron
added a commit
that referenced
this pull request
Sep 9, 2026
Ports every master commit since 0.73.1 (through 0.73.3). Where master's code collided with v1's rewrites, v1's architecture wins and the change was re-implemented against it rather than taken verbatim. Features ported: - `continue` transpiles to a goto label for firmware below 11.5 (#489) - go-to-definition for file path strings in BRS/BS/XML (#1648) - `isTerminal`/`previousInChain` on AstNode (#1788) - nested curly braces in template strings (#1539) - regex literals after `${` and `,` (#1789) - wrong-cased XML tag diagnostic (#1793) - duplicate/crashing "find all references" fix (#1791) - duplicate sourceMappingURL fix (#1786) - findAncestor type-guard inference (#1787) - lexer token-text interning (#1712) - memory-aware default for max worker threads (#1798) - js-yaml override bumped to ^4.3.2 (#1796) Notable adaptations: - master's whitespace fast-path skipped `addToken`, which in v1 is also what routes a token into `leadingTrivia`. Kept the Token allocation (trivia depends on it) and took only the interning half of that optimization. - #1798 rewrote WorkerPool around master's simpler worker tracking. v1 has an `isDead` crashed-worker feature master lacks, so only the `getDefaultMaxWorkerThreads` logic was ported, on top of v1's tracking. - `no-unsafe-argument` is a warning, not an error: master enabled it after cleaning up v0's call sites, and v1's rewrites carry ~76 more that were never part of that cleanup. - three tests from master assert v0 behavior v1 changed on purpose (Comment tokens are trivia, `getReferences` returns `[]` not null, and the NamespacedVariableNameExpression chain step is gone). Updated to v1's contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
Expression chains are stored inverted in the AST, so walking
.parentcan't tell when you've left one expression and landed in an unrelated enclosing one:Walking
.parentfromagivesa.b, then theCallExpression— but that call isn't part ofa.b, it's the thinga.bsits inside.Each node in a chain spans a whole prefix of the source, so a node's previous step is the next-shorter prefix:
Which parent property holds the child is what decides membership, not node type. So each chaining node declares its previous step:
and
isTerminal()walks up then back down — if it doesn't land on itself, nothing chains onto it:This is not
parentreversed: args and index values have aparent, but are never anything'spreviousInChain, which is exactly what makes them boundaries.Two members total. Nodes that chain onto nothing (literals, binary expressions, groupings) inherit
previousInChain === undefinedand are terminal by default, so boundaries need no code.What changed
AstNode:previousInChaingetter andisTerminal()previousInChainoverrides onCallExpression,CallfuncExpression,DottedGetExpression,IndexedGetExpression,XmlAttributeGetExpression,NamespacedVariableNameExpression,NewExpressionVerified — 3108 passing (upstream master is 3084; +24 tests covering each boundary type: call args, index values, groupings, binary/unary operands, array & AA literals, ternary, null-coalescing, template interpolations, set-statement operands).
tsc --noEmitandnpm run lintclean. Purely additive against master.