Fix nested curly braces in template strings - #1539
Merged
Merged
Conversation
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Bug with nested curly braces in template strings
Fix nested curly braces in template strings
Jul 18, 2025
TwitchBronBron
requested changes
Jul 18, 2025
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
Replace the earlier lexer-state approach with a local brace-depth
counter scoped to each `${...}`. Token scanning stays entirely in
`scanToken`, and nested template strings recurse naturally, so no
shared lexer state is needed to support arbitrary nesting depth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cover the cases where a `}` or `{` appears somewhere the depth counter
must not see it: inside a string literal, inside a comment, and inside
an inline function body. Also cover a multi-line AA literal and an
empty expression.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TwitchBronBron
marked this pull request as ready for review
September 4, 2026 17:53
TwitchBronBron
approved these changes
Sep 4, 2026
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.
An AA literal inside a template string expression didn't lex, because the first
}was taken as the end of the expression:Fixes #1538.
What changed
Pulled the
${...}loop body out into atemplateStringExpressionmethod. It counts the braces opened inside the expression and only stops at a}seen at depth zero. Token scanning still goes throughscanToken, so nothing is duplicated.Nesting comes for free —
scanTokensends a backtick back intotemplateString, which calls this again for its own expressions, so each level gets its own count off the call stack:Braces inside a string or comment are safe for the same reason:
scanTokeneats the whole string or comment in one go, so the count never sees them.Verified
npm run test:nocover(3072 passing) andnpm run lintclean. No existing tests changed.New tests cover nested and deeply nested braces, the nested template strings above, braces inside strings and comments, a multi-line AA literal, an inline function body, an empty expression, and the unterminated-expression diagnostic.