Blank out backticks/dollars in placeholder JS instead of escaping - #69
Merged
NullVoxPopuli merged 1 commit intoJul 3, 2026
Conversation
NullVoxPopuli
approved these changes
Jul 3, 2026
Backslash-escaping ` and $ in template content grew the placeholder by one character per occurrence. Once the growth exceeded the padding slack (11 chars for class-member templates, 19 for expression templates), the placeholder no longer lined up with the original <template> region: matchPlaceholder's end-range check failed, the raw StaticBlock / TemplateLiteral leaked into the AST, and every offset after the template shifted. Downstream this surfaced as a false no-unused-expressions error on gts files with backtick-heavy template comments (ember-tooling/ember-eslint-parser#230). Placeholder content is discarded when the Glimmer AST is spliced in, so only its length and line structure matter — replace each ` and $ with a space to keep the placeholder the exact same length as the source region. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NullVoxPopuli-ai-agent
force-pushed
the
fix-placeholder-backtick-overflow-pr
branch
from
July 3, 2026 15:27
ca7ece8 to
3fc9513
Compare
NullVoxPopuli
approved these changes
Jul 3, 2026
Merged
NullVoxPopuli
added a commit
to ember-tooling/ember-eslint-parser
that referenced
this pull request
Jul 3, 2026
ember-estree < 0.6.10 backslash-escaped backticks and dollar signs when building its same-length placeholder JS. Each escape grew the placeholder by one character, and once the growth exceeded the padding slack (11 chars for class-member templates) the placeholder no longer matched the original <template> region: the raw static block leaked into the AST and no-unused-expressions fired on it, with all offsets after the template shifted. Fixed upstream in NullVoxPopuli/ember-estree#69 by blanking the characters instead of escaping. Adds an end-to-end regression test with the exact snippet from #230. The refreshed lockfile pulls newer oxc-parser/content-tag within ember-estree's ranges, which rewords one syntax-error diagnostic; the inline snapshot is updated to match. Fixes #230 Co-Authored-By: Claude Fable 5 <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
2 participants
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.
Fixes the root cause of ember-tooling/ember-eslint-parser#230.
The bug
toPlaceholderJSbackslash-escapes`and$in template content, growing the placeholder by one character per occurrence. The padding slack is fixed:<template></template>is 21 chars, thestatic{`...`}wrapper is 10, so class-member templates have 11 chars of slack (expression templates have 19).Math.max(0, spaces)silently clamps once the escapes exceed that, so the placeholder ends up longer than the original region:matchPlaceholder's end-range check fails, and the rawStaticBlock/TemplateLiteralplaceholder leaks into the final AST — downstream,no-unused-expressionsfires on it (that's exactly the #230 report: its two comments contain 12 backticks, one over the limit; 11 lints clean).$alone triggers it as well (e.g.{{! $$$$$$$$$$$$ }}), not just backticks.The fix
Replace each
`and$with a space instead of escaping. The placeholder content is discarded once the real Glimmer AST is spliced in — the placeholder node itself is only used as a WeakMap key for the TS node map in ember-eslint-parser, and its type (string/void) doesn't depend on the quasi content — so only the placeholder's length and line structure matter. Blanking keeps it the exact same length as the source region in all cases.Verification
$variant, the expression-template form, and offset accuracy for code after a hostile template (that last one failed withexpected '' to be 'const after = 1;'before the fix).node_modules, the #230 file lints clean and ember-eslint-parser's own 86 tests still pass.Note: now rebased onto current
main.🤖 Generated with Claude Code