Skip to content

Commit 6e2754b

Browse files
NullVoxPopuliclaude
andcommitted
fix: literals shouldn't emit original, block inner strip, UndefinedLiteral.value
Three related AST shape fixes to match the reference builder: 1. Literals (String/Number/Boolean/Null/Undefined) no longer emit `original` from Rust. The JS wrapper adds it as a non-enumerable getter that returns value — matching the reference builder's defineProperty pattern. 2. UndefinedLiteral needs a real `value: undefined` own property on its node, not a missing key. The JS wrapper assigns this after conversion since JSON can't serialize undefined. 3. BlockStatement's inner strip flags now apply whitespace stripping to program/inverse bodies: openStrip.close → trim leading ws in program body first text inverseStrip.open → trim trailing ws in program body last text inverseStrip.close → trim leading ws in inverse body first text closeStrip.open → trim trailing ws in inverse (or program) body last text Local test count: 326 → 190 → (expected further drop) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 710f3cb commit 6e2754b

5 files changed

Lines changed: 68 additions & 9 deletions

File tree

packages/@glimmer/syntax/lib/parser/tokenizer-event-handlers.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,33 @@ function stripBodyWhitespace(body: Stripable[]): void {
343343
next.chars = next.chars.replace(/^[ \t\r\n]+/u, '');
344344
}
345345
}
346+
347+
// BlockStatement has additional inner stripping:
348+
// openStrip.close → trim leading ws on program body's first text
349+
// inverseStrip.open → trim trailing ws on program body's last text
350+
// inverseStrip.close → trim leading ws on inverse body's first text
351+
// closeStrip.open → trim trailing ws on inverse (or program) last text
352+
if (stmt.type === 'BlockStatement') {
353+
const program = stmt.program?.body;
354+
const inverse = stmt.inverse?.body;
355+
356+
if (stmt.openStrip?.close && program && program.length > 0) {
357+
stripFirstTextLeading(program);
358+
}
359+
if (stmt.inverseStrip?.open && program && program.length > 0) {
360+
stripLastTextTrailing(program);
361+
}
362+
if (stmt.inverseStrip?.close && inverse && inverse.length > 0) {
363+
stripFirstTextLeading(inverse);
364+
}
365+
if (stmt.closeStrip?.open) {
366+
if (inverse && inverse.length > 0) {
367+
stripLastTextTrailing(inverse);
368+
} else if (program && program.length > 0) {
369+
stripLastTextTrailing(program);
370+
}
371+
}
372+
}
346373
}
347374

348375
// Drop any text nodes that are now empty after stripping.
@@ -354,6 +381,20 @@ function stripBodyWhitespace(body: Stripable[]): void {
354381
}
355382
}
356383

384+
function stripFirstTextLeading(body: Stripable[]): void {
385+
const first = body[0];
386+
if (first?.type === 'TextNode' && typeof first.chars === 'string') {
387+
first.chars = first.chars.replace(/^[ \t\r\n]+/u, '');
388+
}
389+
}
390+
391+
function stripLastTextTrailing(body: Stripable[]): void {
392+
const last = body[body.length - 1];
393+
if (last?.type === 'TextNode' && typeof last.chars === 'string') {
394+
last.chars = last.chars.replace(/[ \t\r\n]+$/u, '');
395+
}
396+
}
397+
357398
function getOpenStrip(stmt: Stripable): boolean {
358399
if (stmt.type === 'MustacheStatement') return !!stmt.strip?.open;
359400
if (stmt.type === 'MustacheCommentStatement') return !!stmt.__strip?.open;
@@ -432,6 +473,32 @@ function convertLocations(node: unknown, source: src.Source): void {
432473
});
433474
}
434475

476+
// Add deprecated `original` as a non-enumerable getter on literal nodes
477+
// (matches the reference builder's defineProperty pattern).
478+
if (
479+
(obj['type'] === 'StringLiteral' ||
480+
obj['type'] === 'NumberLiteral' ||
481+
obj['type'] === 'BooleanLiteral' ||
482+
obj['type'] === 'NullLiteral' ||
483+
obj['type'] === 'UndefinedLiteral') &&
484+
!Object.getOwnPropertyDescriptor(obj, 'original')
485+
) {
486+
Object.defineProperty(obj, 'original', {
487+
enumerable: false,
488+
configurable: true,
489+
get(this: { value: unknown }): unknown {
490+
return this.value;
491+
},
492+
});
493+
}
494+
495+
// UndefinedLiteral needs a real `value: undefined` property (the reference
496+
// builder creates it this way). We can't emit undefined from JSON so we
497+
// assign it here.
498+
if (obj['type'] === 'UndefinedLiteral' && !('value' in obj)) {
499+
obj['value'] = undefined;
500+
}
501+
435502
// Decode HTML entities in text node content (precompile mode only).
436503
// The old parser used simple-html-tokenizer's EntityParser; we inline
437504
// a minimal decoder here to avoid re-adding that dependency.
Binary file not shown.

packages/@glimmer/syntax/pkg/wasm-bytes.mjs

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

packages/@glimmer/syntax/src/ast.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ pub struct StringLiteral {
259259
#[serde(rename = "type")]
260260
pub node_type: &'static str, // "StringLiteral"
261261
pub value: String,
262-
pub original: String,
263262
pub loc: SourceLocation,
264263
}
265264

@@ -268,7 +267,6 @@ pub struct BooleanLiteral {
268267
#[serde(rename = "type")]
269268
pub node_type: &'static str, // "BooleanLiteral"
270269
pub value: bool,
271-
pub original: bool,
272270
pub loc: SourceLocation,
273271
}
274272

@@ -277,7 +275,6 @@ pub struct NumberLiteral {
277275
#[serde(rename = "type")]
278276
pub node_type: &'static str, // "NumberLiteral"
279277
pub value: f64,
280-
pub original: f64,
281278
pub loc: SourceLocation,
282279
}
283280

@@ -294,7 +291,6 @@ pub struct NullLiteral {
294291
pub node_type: &'static str, // "NullLiteral"
295292
// value and original are both `null` in ASTv1
296293
pub value: Option<()>,
297-
pub original: Option<()>,
298294
pub loc: SourceLocation,
299295
}
300296

packages/@glimmer/syntax/src/builder.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,6 @@ fn build_string_literal(pair: Pair<'_, Rule>, source: &str) -> Expression {
589589
Expression::StringLiteral(StringLiteral {
590590
node_type: "StringLiteral",
591591
value: unescaped.clone(),
592-
original: unescaped,
593592
loc,
594593
})
595594
}
@@ -601,7 +600,6 @@ fn build_number_literal(pair: Pair<'_, Rule>, source: &str) -> Expression {
601600
Expression::NumberLiteral(NumberLiteral {
602601
node_type: "NumberLiteral",
603602
value,
604-
original: value,
605603
loc,
606604
})
607605
}
@@ -613,7 +611,6 @@ fn build_boolean_literal(pair: Pair<'_, Rule>, source: &str) -> Expression {
613611
Expression::BooleanLiteral(BooleanLiteral {
614612
node_type: "BooleanLiteral",
615613
value,
616-
original: value,
617614
loc,
618615
})
619616
}
@@ -631,7 +628,6 @@ fn build_null_literal(pair: Pair<'_, Rule>, source: &str) -> Expression {
631628
Expression::NullLiteral(NullLiteral {
632629
node_type: "NullLiteral",
633630
value: None,
634-
original: None,
635631
loc,
636632
})
637633
}

0 commit comments

Comments
 (0)