From 34dae62be87bf160e7cc6963315f2e733962e702 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 8 Oct 2026 16:39:11 -0400 Subject: [PATCH 01/20] Tokenize plain text runs in one step in liquid-html-parser The document tokenizer advanced one character at a time and ran every mode's startsWith checks on each one, even inside plain text. Most characters cannot start a token in the current mode, so jump straight to the next one that can and let the main loop re-check it there. Liquid tag and output bodies use indexOf for their closing delimiter. The `<` tag-start check uses char codes instead of a regex. Tokens and ASTs are unchanged. --- .changeset/tokenizer-text-fast-path.md | 7 ++ .../src/document/tokenizer.test.ts | 52 ++++++++ .../src/document/tokenizer.ts | 111 ++++++++++++++++-- 3 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 .changeset/tokenizer-text-fast-path.md diff --git a/.changeset/tokenizer-text-fast-path.md b/.changeset/tokenizer-text-fast-path.md new file mode 100644 index 000000000..45a3e8dcd --- /dev/null +++ b/.changeset/tokenizer-text-fast-path.md @@ -0,0 +1,7 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Tokenize runs of plain text in one step instead of one character at a time. Most characters can't start a token, so the tokenizer now jumps to the next one that can. + +Tokens and ASTs are unchanged. On Dawn, Horizon and the base theme, `tokenize` is about 6× faster and `toLiquidHtmlAST`/`toLiquidAST` about 2× faster. diff --git a/packages/liquid-html-parser/src/document/tokenizer.test.ts b/packages/liquid-html-parser/src/document/tokenizer.test.ts index b2f7594d6..41356b04b 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.test.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.test.ts @@ -543,6 +543,58 @@ describe('Unit: document-tokenizer', () => { }); }); + describe('text runs end at the next token', () => { + it('ends a Liquid tag body at a -%} preceded by another -', () => { + const source = '{% if a--%}'; + expect(tokens(source)).toMatchObject([ + { type: TokenType.LiquidTagOpen, start: 0, end: 2 }, + { type: TokenType.Text, start: 2, end: 8 }, + { type: TokenType.LiquidTagClose, start: 8, end: 11 }, + ]); + assertTokenInvariants(source); + }); + + it('keeps %} as text inside a Liquid drop and ends it at -}}', () => { + const source = '{{ "%}" -}}'; + expect(tokens(source)).toMatchObject([ + { type: TokenType.LiquidVariableOutputOpen, start: 0, end: 2 }, + { type: TokenType.Text, start: 2, end: 8 }, + { type: TokenType.LiquidVariableOutputClose, start: 8, end: 11 }, + ]); + assertTokenInvariants(source); + }); + + it('ends a curly-quoted value on its partner, not on a straight quote', () => { + const source = ''; + expect(tokens(source)).toMatchObject([ + { type: TokenType.HtmlTagOpen, start: 0, end: 1 }, + { type: TokenType.Text, start: 1, end: 2 }, + { type: TokenType.Text, start: 2, end: 3 }, + { type: TokenType.Text, start: 3, end: 4 }, + { type: TokenType.HtmlEquals, start: 4, end: 5 }, + { type: TokenType.HtmlQuoteOpen, start: 5, end: 6 }, + { type: TokenType.Text, start: 6, end: 9 }, + { type: TokenType.HtmlQuoteClose, start: 9, end: 10 }, + { type: TokenType.Text, start: 10, end: 11 }, + { type: TokenType.Text, start: 11, end: 12 }, + { type: TokenType.HtmlTagClose, start: 12, end: 13 }, + ]); + assertTokenInvariants(source); + }); + + it('finds --> and c`). */ +function nextDefaultCandidate(source: string, from: number): number { + for (let i = from; i < source.length; i++) { + const c = source.charCodeAt(i); + if (c === CHAR_OPEN_BRACE || c === CHAR_LESS_THAN || c === CHAR_DASH) return i; + } + return source.length; +} + +/** + * HtmlTag mode tokens start with `{`, `/`, `>`, `=`, or a straight/curly quote; + * a Text run also ends where whitespace starts or stops. + */ +function nextHtmlTagCandidate(source: string, from: number, textIsSpace: boolean): number { + for (let i = from; i < source.length; i++) { + const c = source.charCodeAt(i); + if ( + c === CHAR_OPEN_BRACE || + c === CHAR_SLASH || + c === CHAR_GREATER_THAN || + c === CHAR_EQUALS || + c === CHAR_DOUBLE_QUOTE || + c === CHAR_SINGLE_QUOTE || + (c >= CHAR_LEFT_SINGLE_CURLY_QUOTE && c <= CHAR_RIGHT_DOUBLE_CURLY_QUOTE) || + isWhitespace(c) !== textIsSpace + ) { + return i; + } + } + return source.length; +} + +/** QuotedValue mode tokens start with `{` or either quote of the open pair. */ +function nextQuotedValueCandidate(source: string, from: number, quote: string): number { + const open = quote.charCodeAt(0); + const close = closingQuoteFor(quote).charCodeAt(0); + for (let i = from; i < source.length; i++) { + const c = source.charCodeAt(i); + if (c === CHAR_OPEN_BRACE || c === open || c === close) return i; + } + return source.length; +} + +/** + * Liquid tag bodies only end at `%}`, optionally preceded by `-`. Any `-%}` + * match contains a `%}` one character later, so the earliest close is at the + * first `%}` or the `-` immediately before it. + */ +function nextLiquidCloseCandidate(source: string, from: number, close: '%}'): number { + const i = source.indexOf(close, from); + if (i === -1) return source.length; + return i > from && source.charCodeAt(i - 1) === CHAR_DASH ? i - 1 : i; +} + +/** `[a-zA-Z{]`: what may follow `<` or `= 0x61 && c <= 0x7a) || (c >= 0x41 && c <= 0x5a) || c === CHAR_OPEN_BRACE; +} + +/** Exactly the UTF-16 code units JavaScript's `\s` matches. */ +function isWhitespace(c: number): boolean { + return ( + (c >= 0x09 && c <= 0x0d) || + c === 0x20 || + c === 0xa0 || + c === 0x1680 || + (c >= 0x2000 && c <= 0x200a) || + c === 0x2028 || + c === 0x2029 || + c === 0x202f || + c === 0x205f || + c === 0x3000 || + c === 0xfeff + ); +} + enum Mode { Default = 'Default', HtmlTag = 'HtmlTag', From f11f8023393d856878f9c9e7f32ab74817c7cfc7 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 8 Oct 2026 17:36:33 -0400 Subject: [PATCH 02/20] Test that the tokenizer fast path stops at every token start The existing tests only had quotes directly after `=`, where the main loop checks them without the fast path, so a helper missing a stop character could pass CI. The oracle suites that might catch it are skipped there because the theme fixtures are gitignored. Route the five text branches through one `nextTextCandidate` dispatcher, so a test-only `tokenizeWithoutFastPath` can run the same loop one character at a time. Tests now: - put each token start in the middle of plain text in every mode - compare `tokenize` with `tokenizeWithoutFastPath` on every string of up to 4 token-relevant characters, in every entry state --- .../src/document/tokenizer.test.ts | 108 +++++++++++++++++- .../src/document/tokenizer.ts | 69 +++++++++-- 2 files changed, 164 insertions(+), 13 deletions(-) diff --git a/packages/liquid-html-parser/src/document/tokenizer.test.ts b/packages/liquid-html-parser/src/document/tokenizer.test.ts index 41356b04b..623392521 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.test.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { tokenize, TokenType } from './tokenizer'; -import type { Token } from './tokenizer'; +import { tokenize, tokenizeWithoutFastPath, TokenType } from './tokenizer'; +import type { Token, TokenizeOptions } from './tokenizer'; /** Strip the trailing EndOfInput token for cleaner assertions. */ function tokens(source: string): Token[] { @@ -595,6 +595,110 @@ describe('Unit: document-tokenizer', () => { }); }); + describe('text fast path stops at every token start mid-text', () => { + // No character in `pad` can start a token in any mode, so the fast path + // skips the whole pad and must stop exactly where the token begins. + const pad = 'abc 1 '; + const quotePairs = [ + ['"', '"'], + ["'", "'"], + ['\u201c', '\u201d'], + ['\u201d', '\u201c'], + ['\u2018', '\u2019'], + ['\u2019', '\u2018'], + ]; + const cases: Array<[string, string, TokenizeOptions, string[]]> = [ + [ + 'Default', + '', + {}, + ['{{', '{{-', '{%', '{%-', '', '', + '>', + '=', + '"', + "'", + '\u201c', + '\u201d', + '\u2018', + '\u2019', + ], + ], + ...quotePairs.map(([open, close]): [string, string, TokenizeOptions, string[]] => [ + `QuotedValue ${open}`, + '', + { insideQuotedAttribute: open }, + [...new Set(['{{', '{%', open, close])], + ]), + ['LiquidTag', '{% ', {}, ['%}', '-%}']], + ['LiquidVariableOutput', '{{ ', {}, ['}}', '-}}']], + ]; + + for (const [mode, prefix, options, starts] of cases) { + it.each(starts)(`${mode}: %j`, (start) => { + const offset = prefix.length + pad.length; + const result = tokenize(prefix + pad + start + pad, options); + expect(result.some((t) => t.type !== TokenType.Text && t.start === offset)).toBe(true); + }); + } + }); + + describe('matches tokenizeWithoutFastPath', () => { + // Every string of up to 4 of these characters, in every entry state. The + // set covers each character a mode's token can start with, plus near + // misses (`!`, `\u201a`) and plain text. + const alphabet = [...'{}%-<>/=!"\'\u201c\u201d\u2018\u2019\u201a a\n']; + let sources = ['']; + for (let length = 1, previous = ['']; length <= 4; length++) { + previous = previous.flatMap((s) => alphabet.map((c) => s + c)); + sources = sources.concat(previous); + } + + const entryStates: Array<[string, string, TokenizeOptions]> = [ + ['document start', '', {}], + ['skipFrontmatter', '', { skipFrontmatter: true }], + ['insideHtmlTag', '', { insideHtmlTag: true }], + ...['"', "'", '\u201c', '\u201d', '\u2018', '\u2019'].map( + (quote): [string, string, TokenizeOptions] => [ + `insideQuotedAttribute ${quote}`, + '', + { insideQuotedAttribute: quote }, + ], + ), + ['inside a Liquid tag', '{% ', {}], + ['inside a Liquid output', '{{ ', {}], + ]; + + function sameTokens(a: Token[], b: Token[]): boolean { + return ( + a.length === b.length && + a.every((t, i) => t.type === b[i].type && t.start === b[i].start && t.end === b[i].end) + ); + } + + it.each(entryStates)('%s', (_name, prefix, options) => { + const mismatches: string[] = []; + for (const source of sources) { + const input = prefix + source; + if (!sameTokens(tokenize(input, options), tokenizeWithoutFastPath(input, options))) { + mismatches.push(input); + if (mismatches.length === 10) break; + } + } + expect(mismatches).toEqual([]); + }); + }); + describe('structural invariants', () => { const cases = [ '{{ x }}', diff --git a/packages/liquid-html-parser/src/document/tokenizer.ts b/packages/liquid-html-parser/src/document/tokenizer.ts index 2b240c92b..21fc1b2a9 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.ts @@ -62,8 +62,21 @@ export interface TokenizeOptions { } export function tokenize(source: string, options: TokenizeOptions = {}): Token[] { + return tokenizeToEnd(source, options, true); +} + +/** + * `tokenize` without the text fast path: plain text advances one character at + * a time. Test-only reference for checking that the fast path never skips over + * a token start. Not exported from the package. + */ +export function tokenizeWithoutFastPath(source: string, options: TokenizeOptions = {}): Token[] { + return tokenizeToEnd(source, options, false); +} + +function tokenizeToEnd(source: string, options: TokenizeOptions, fastPath: boolean): Token[] { const tokens: Token[] = []; - const tokenizer = createTokenizer(source, tokens); + const tokenizer = createTokenizer(source, tokens, fastPath); tokenizer.restart(0, options); tokenizer.fill(Infinity); return tokens; @@ -87,7 +100,7 @@ export interface Tokenizer { fill(index: number): boolean; } -export function createTokenizer(source: string, tokens: Token[]): Tokenizer { +export function createTokenizer(source: string, tokens: Token[], fastPath = true): Tokenizer { const modeStack: Mode[] = []; let mode = Mode.Default as Mode; let pos = 0; @@ -137,6 +150,17 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { if (textStart === -1) textStart = pos; } + /** Moves past plain text, to where the current mode's next token could start. */ + function skipText() { + if (!fastPath) { + pos++; + } else if (mode === Mode.LiquidVariableOutput) { + pos = nextOutputClose(); + } else { + pos = nextTextCandidate(source, pos + 1, mode, quoteChar, textIsSpace); + } + } + /** * Where the `-}}` or `}}` that closes Liquid output at `pos` begins, or the * end of the input. Neither begins at `pos` itself. @@ -224,7 +248,7 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { popMode(); } else { startText(); - pos = nextLiquidCloseCandidate(source, pos + 1, '%}'); + skipText(); } break; } @@ -237,9 +261,8 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { emit(TokenType.LiquidVariableOutputClose, 2); popMode(); } else { - // Jump to the close, rather than step to it a character at a time. startText(); - pos = nextOutputClose(); + skipText(); } break; } @@ -280,7 +303,7 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { } startText(); - pos = nextDefaultCandidate(source, pos + 1); + skipText(); break; } @@ -330,7 +353,7 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { if (textStart !== -1 && isSpace !== textIsSpace) flushText(); if (textStart === -1) textIsSpace = isSpace; startText(); - pos = nextHtmlTagCandidate(source, pos + 1, textIsSpace); + skipText(); break; } @@ -347,7 +370,7 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { } startText(); - pos = nextQuotedValueCandidate(source, pos + 1, quoteChar); + skipText(); break; } @@ -374,9 +397,33 @@ export function createTokenizer(source: string, tokens: Token[]): Tokenizer { * consumed in one step. Inside an HTML tag a run also ends where text switches * between whitespace and non-whitespace, because each is its own Text token. * Returning a superset of real token starts is safe: the main loop re-checks - * that position and treats a non-match as text. + * that position and treats a non-match as text. Missing a token start is not, + * so a new token type needs its first character added to its mode's helper + * (tokenizer.test.ts compares against `tokenizeWithoutFastPath` to catch this). + * Liquid output jumps with `nextOutputClose` instead, which caches its search. */ +function nextTextCandidate( + source: string, + from: number, + mode: Exclude, + quoteChar: string, + textIsSpace: boolean, +): number { + switch (mode) { + case Mode.Default: + return nextDefaultCandidate(source, from); + case Mode.HtmlTag: + return nextHtmlTagCandidate(source, from, textIsSpace); + case Mode.QuotedValue: + return nextQuotedValueCandidate(source, from, quoteChar); + case Mode.LiquidTag: + return nextLiquidTagCloseCandidate(source, from); + default: + return assertNever(mode); + } +} + const CHAR_DOUBLE_QUOTE = 0x22; // " const CHAR_SINGLE_QUOTE = 0x27; // ' const CHAR_DASH = 0x2d; // - @@ -436,8 +483,8 @@ function nextQuotedValueCandidate(source: string, from: number, quote: string): * match contains a `%}` one character later, so the earliest close is at the * first `%}` or the `-` immediately before it. */ -function nextLiquidCloseCandidate(source: string, from: number, close: '%}'): number { - const i = source.indexOf(close, from); +function nextLiquidTagCloseCandidate(source: string, from: number): number { + const i = source.indexOf('%}', from); if (i === -1) return source.length; return i > from && source.charCodeAt(i - 1) === CHAR_DASH ? i - 1 : i; } From 5b634d936cc3f73968e62b1001404463cf07e250 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 8 Oct 2026 19:14:41 -0400 Subject: [PATCH 03/20] Check character codes before regexes in liquid-html-parser tokenizers - `scanLiquidOpen` returns early unless the character is `{`, instead of four failed `startsWith` calls at every `<`, `-`, quote, `/`, `>` and `=` the text fast path stops at. - The Liquid expression tokenizer tests digits with char codes instead of evaluating `/\d/` literals per token, and only runs the whitespace regex when the character could be whitespace. Each check is a pre-filter in front of the existing `startsWith` or regex, so letting extra characters through can't change tokens; only rejecting a real `{` or digit could, and the suite catches that. --- .changeset/markup-tokenizer-fast-checks.md | 5 ++++ .../src/document/tokenizer.ts | 1 + .../src/markup/tokenizer.test.ts | 17 +++++++++++++ .../src/markup/tokenizer.ts | 24 ++++++++++++++----- 4 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 .changeset/markup-tokenizer-fast-checks.md diff --git a/.changeset/markup-tokenizer-fast-checks.md b/.changeset/markup-tokenizer-fast-checks.md new file mode 100644 index 000000000..1bbfed464 --- /dev/null +++ b/.changeset/markup-tokenizer-fast-checks.md @@ -0,0 +1,5 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Check character codes before running regular expressions or `startsWith` in the Liquid expression tokenizer and when looking for `{{`/`{%`. Tokens and ASTs are unchanged; parsing is 5–7% faster. diff --git a/packages/liquid-html-parser/src/document/tokenizer.ts b/packages/liquid-html-parser/src/document/tokenizer.ts index 21fc1b2a9..a077ff180 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.ts @@ -213,6 +213,7 @@ export function createTokenizer(source: string, tokens: Token[], fastPath = true // Liquid open check — reused in Default, HtmlTag, and QuotedValue modes function scanLiquidOpen(): boolean { + if (source.charCodeAt(pos) !== CHAR_OPEN_BRACE) return false; if (match('{{-')) { emit(TokenType.LiquidVariableOutputOpen, 3); pushMode(Mode.LiquidVariableOutput); diff --git a/packages/liquid-html-parser/src/markup/tokenizer.test.ts b/packages/liquid-html-parser/src/markup/tokenizer.test.ts index a83dd7bcf..e898e0ea2 100644 --- a/packages/liquid-html-parser/src/markup/tokenizer.test.ts +++ b/packages/liquid-html-parser/src/markup/tokenizer.test.ts @@ -96,6 +96,23 @@ describe('Unit: markup-tokenizer', () => { expect(result).toMatchObject([{ type: MarkupTokenType.Number, value: '-100_000' }]); expect(result).toHaveLength(1); }); + + it('treats only ASCII 0-9 as digits', () => { + // U+0661 and U+FF15 are digits in other scripts; `\d` without `u` excludes them. + expect(tokens('\u0661 -\uff15 0 9 -0')).toMatchObject([ + { type: MarkupTokenType.Dash, value: '-' }, + { type: MarkupTokenType.Number, value: '0' }, + { type: MarkupTokenType.Number, value: '9' }, + { type: MarkupTokenType.Number, value: '-0' }, + ]); + }); + + it('tokenizes a trailing dash as Dash', () => { + expect(tokens('a -')).toMatchObject([ + { type: MarkupTokenType.Id, value: 'a' }, + { type: MarkupTokenType.Dash, value: '-' }, + ]); + }); }); describe('operators', () => { diff --git a/packages/liquid-html-parser/src/markup/tokenizer.ts b/packages/liquid-html-parser/src/markup/tokenizer.ts index b4aa9da40..744b56b7a 100644 --- a/packages/liquid-html-parser/src/markup/tokenizer.ts +++ b/packages/liquid-html-parser/src/markup/tokenizer.ts @@ -31,16 +31,28 @@ const WHITESPACE_RE = /\s+/y; const SINGLE_STRING_RE = /'[^']*'/y; const DOUBLE_STRING_RE = /"[^"]*"/y; +/** `\d` (no `u` flag): ASCII 0-9 only. NaN past the end is not a digit. */ +function isDigit(c: number): boolean { + return c >= 0x30 && c <= 0x39; +} + +/** Pre-check for WHITESPACE_RE: every `\s` character is <= U+0020 or >= U+00A0. */ +function mayBeWhitespace(c: number): boolean { + return c <= 0x20 || c >= 0xa0; +} + export function tokenizeMarkup(markup: string, startOffset = 0): MarkupToken[] { const tokens: MarkupToken[] = []; let pos = 0; let lastTokenType: MarkupTokenType | undefined; while (pos < markup.length) { - WHITESPACE_RE.lastIndex = pos; - if (WHITESPACE_RE.test(markup)) { - pos = WHITESPACE_RE.lastIndex; - continue; + if (mayBeWhitespace(markup.charCodeAt(pos))) { + WHITESPACE_RE.lastIndex = pos; + if (WHITESPACE_RE.test(markup)) { + pos = WHITESPACE_RE.lastIndex; + continue; + } } const ch = markup[pos]; @@ -63,7 +75,7 @@ export function tokenizeMarkup(markup: string, startOffset = 0): MarkupToken[] { } } - if (ch === '-' && /\d/.test(next)) { + if (ch === '-' && isDigit(markup.charCodeAt(pos + 1))) { NUMBER_RE.lastIndex = pos + 1; if (NUMBER_RE.test(markup)) { const value = markup.slice(pos, NUMBER_RE.lastIndex); @@ -79,7 +91,7 @@ export function tokenizeMarkup(markup: string, startOffset = 0): MarkupToken[] { } } - if (/\d/.test(ch)) { + if (isDigit(markup.charCodeAt(pos))) { NUMBER_RE.lastIndex = pos; if (NUMBER_RE.test(markup)) { const matchEnd = NUMBER_RE.lastIndex; From 320fa1feb643b986cd8b35ee9c8a0d4feeca0742 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 8 Oct 2026 19:01:32 -0400 Subject: [PATCH 04/20] Trim text around Liquid tags with trimStart/trimEnd `value.replace(/\s+$/, '')` retries the match at every position of each whitespace run, so its cost grows with the square of the run length. It was the most expensive single step when parsing Dawn and Horizon. `trimStart()`/`trimEnd()` strip exactly the characters `\s` matches (both use the spec's WhiteSpace and LineTerminator sets), in linear time. A test pins that set and the long-run case. --- .changeset/linear-text-trim.md | 5 ++++ .../src/document/tree-builder.test.ts | 28 +++++++++++++++++++ .../src/document/tree-builder.ts | 4 +-- 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 .changeset/linear-text-trim.md diff --git a/.changeset/linear-text-trim.md b/.changeset/linear-text-trim.md new file mode 100644 index 000000000..95738e0b8 --- /dev/null +++ b/.changeset/linear-text-trim.md @@ -0,0 +1,5 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Trim whitespace around Liquid tags with `trimStart()`/`trimEnd()` instead of regular expressions. The trailing-whitespace regex was quadratic in the length of a whitespace run and was the single most expensive step of parsing a typical theme. diff --git a/packages/liquid-html-parser/src/document/tree-builder.test.ts b/packages/liquid-html-parser/src/document/tree-builder.test.ts index 9193bfd68..4560888d4 100644 --- a/packages/liquid-html-parser/src/document/tree-builder.test.ts +++ b/packages/liquid-html-parser/src/document/tree-builder.test.ts @@ -124,6 +124,34 @@ describe('Unit: tree-builder', () => { }); }); + describe('whitespace trimming around Liquid tags', () => { + function ifBranchChildren(body: string): LiquidHtmlNode[] { + const [tag] = toLiquidHtmlAST(`{% if a %}${body}{% endif %}`).children; + if (tag.type !== NodeTypes.LiquidTag || !tag.children) throw new Error('expected {% if %}'); + const [branch] = tag.children; + if (branch.type !== NodeTypes.LiquidBranch) throw new Error('expected a branch'); + return branch.children; + } + + it('trims every character that \\s matches, and keeps others', () => { + const whitespace = + '\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008' + + '\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff'; + const text = 'x\u200by'; // zero-width space is not whitespace + const start = '{% if a %}'.length + whitespace.length; + expect(ifBranchChildren(whitespace + text + whitespace)).toMatchObject([ + { type: NodeTypes.TextNode, value: text, position: { start, end: start + text.length } }, + ]); + }); + + it('trims text containing a long whitespace run in linear time', () => { + const text = 'x' + ' '.repeat(100_000) + 'y'; + expect(ifBranchChildren(` ${text} `)).toMatchObject([ + { type: NodeTypes.TextNode, value: text }, + ]); + }, 1000); + }); + describe('compoundNamesMatch', () => { it('returns true for two empty arrays', () => { expect(compoundNamesMatch([], [])).toBe(true); diff --git a/packages/liquid-html-parser/src/document/tree-builder.ts b/packages/liquid-html-parser/src/document/tree-builder.ts index a2ec91789..cb6f1875d 100644 --- a/packages/liquid-html-parser/src/document/tree-builder.ts +++ b/packages/liquid-html-parser/src/document/tree-builder.ts @@ -107,7 +107,7 @@ export function mergeAdjacentTextNodesTrimmed( // Trim leading whitespace if preceded by a non-text node or at start const prev = merged[i - 1]; if (!prev || !isTextNode(prev)) { - const trimmed = value.replace(/^\s+/, ''); + const trimmed = value.trimStart(); const trimLen = value.length - trimmed.length; value = trimmed; start += trimLen; @@ -116,7 +116,7 @@ export function mergeAdjacentTextNodesTrimmed( // Trim trailing whitespace if followed by a non-text node or at end const next = merged[i + 1]; if (!next || !isTextNode(next)) { - const trimmed = value.replace(/\s+$/, ''); + const trimmed = value.trimEnd(); const trimLen = value.length - trimmed.length; value = trimmed; end -= trimLen; From a73b174fa9bb8224babc6e6aaac3f2c0138d7a40 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 8 Oct 2026 19:08:58 -0400 Subject: [PATCH 05/20] Parse Object.prototype names as ordinary names in liquid-html-parser `isLiteralKeyword` used `s in LiquidLiteralValues`, and `tagForName` read `this.builtins[name]` from a plain object. Both also see inherited properties, so `{{ constructor }}` became a LiquidLiteral whose value was the Object function, and `{% constructor %}` / `{% __proto__ %}` resolved to a non-tag "definition" and threw. Recognize literal keywords with a switch (also faster than `in` on this hot path) and store builtin tags in a Map. A test checks every LiquidLiteralValues key is still recognized. --- .changeset/prototype-names-not-keywords.md | 5 + .../src/environment.test.ts | 7 + .../liquid-html-parser/src/environment.ts | 7 +- .../src/markup/parser.test.ts | 23 ++ .../liquid-html-parser/src/markup/parser.ts | 14 +- specs/parser/liquid-tags.yml | 248 ++++++++++++++++++ 6 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 .changeset/prototype-names-not-keywords.md diff --git a/.changeset/prototype-names-not-keywords.md b/.changeset/prototype-names-not-keywords.md new file mode 100644 index 000000000..db605cabd --- /dev/null +++ b/.changeset/prototype-names-not-keywords.md @@ -0,0 +1,5 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Parse names inherited from `Object.prototype` like any other name. `{{ constructor }}` and `{{ toString }}` were parsed as Liquid literals whose value was a JavaScript function, and `{% constructor %}` or `{% __proto__ %}` made the parser throw instead of producing an unknown tag. Recognizing literal keywords with a `switch` is also slightly faster. diff --git a/packages/liquid-html-parser/src/environment.test.ts b/packages/liquid-html-parser/src/environment.test.ts index 5b53161eb..91d1dd3e4 100644 --- a/packages/liquid-html-parser/src/environment.test.ts +++ b/packages/liquid-html-parser/src/environment.test.ts @@ -49,6 +49,13 @@ describe('Unit: Environment', () => { expect(env.tagForName('nonexistent')).toBeUndefined(); }); + it('returns undefined for Object.prototype names', () => { + const env = new Environment({ if: makeBlockDef(['elsif', 'else']) }); + for (const name of ['constructor', 'toString', 'hasOwnProperty', '__proto__']) { + expect(env.tagForName(name)).toBeUndefined(); + } + }); + it('returns the builtin definition when constructed with builtins', () => { const ifDef = makeBlockDef(['elsif', 'else']); const env = new Environment({ if: ifDef }); diff --git a/packages/liquid-html-parser/src/environment.ts b/packages/liquid-html-parser/src/environment.ts index 8b2aca21c..18c6addc6 100644 --- a/packages/liquid-html-parser/src/environment.ts +++ b/packages/liquid-html-parser/src/environment.ts @@ -16,17 +16,18 @@ import type { TagDefinition } from './tag-definitions'; import { builtinTags } from './tags/index'; export class Environment { - private builtins: Record; + private builtins: Map; private custom: Map; private static _default: Environment | undefined; constructor(builtins: Record = {}) { - this.builtins = builtins; + // A Map so tag names like `constructor` don't resolve to Object.prototype members. + this.builtins = new Map(Object.entries(builtins)); this.custom = new Map(); } tagForName(name: string): TagDefinition | undefined { - return this.custom.get(name) ?? this.builtins[name]; + return this.custom.get(name) ?? this.builtins.get(name); } registerTag(name: string, definition: TagDefinition): void { diff --git a/packages/liquid-html-parser/src/markup/parser.test.ts b/packages/liquid-html-parser/src/markup/parser.test.ts index c8c9b6e9a..9065aa79a 100644 --- a/packages/liquid-html-parser/src/markup/parser.test.ts +++ b/packages/liquid-html-parser/src/markup/parser.test.ts @@ -11,6 +11,7 @@ import type { LiquidBooleanExpression, LiquidNamedArgument, } from '../ast'; +import { LiquidLiteralValues } from '../ast'; function parser(markup: string): MarkupParser { return new MarkupParser(tokenizeMarkup(markup), markup); @@ -268,6 +269,28 @@ describe('Unit: MarkupParser expression parsing', () => { value: '', }); }); + + it.each(Object.entries(LiquidLiteralValues))( + 'recognizes %s as a literal', + (keyword, value) => { + expect(parser(keyword).valueExpression()).toMatchObject({ + type: NodeTypes.LiquidLiteral, + keyword, + value, + }); + }, + ); + + it.each(['constructor', 'toString', 'hasOwnProperty', '__proto__', 'valueOf'])( + 'parses Object.prototype name %s as a variable, not a literal', + (name) => { + expect(parser(name).valueExpression()).toMatchObject({ + type: NodeTypes.VariableLookup, + name, + lookups: [], + }); + }, + ); }); describe('variable lookups', () => { diff --git a/packages/liquid-html-parser/src/markup/parser.ts b/packages/liquid-html-parser/src/markup/parser.ts index b5fa0b6e5..fc7b7eb4f 100644 --- a/packages/liquid-html-parser/src/markup/parser.ts +++ b/packages/liquid-html-parser/src/markup/parser.ts @@ -1438,6 +1438,18 @@ function isLogicalOp(s: string): s is LogicalOperator { return LOGICAL_OPERATORS.has(s); } +// A switch rather than `s in LiquidLiteralValues`: `in` also matches inherited +// names like `constructor`, and the switch is faster on this hot path. function isLiteralKeyword(s: string): s is keyof typeof LiquidLiteralValues { - return s in LiquidLiteralValues; + switch (s) { + case 'nil': + case 'null': + case 'true': + case 'false': + case 'blank': + case 'empty': + return true; + default: + return false; + } } diff --git a/specs/parser/liquid-tags.yml b/specs/parser/liquid-tags.yml index 55885f1ae..05c3e4fc8 100644 --- a/specs/parser/liquid-tags.yml +++ b/specs/parser/liquid-tags.yml @@ -1911,3 +1911,251 @@ cases: value: x position: { start: [0, 8], text: '"x"' } filters: [] + - name: parses the Object.prototype name constructor as an unknown tag + source: "{% constructor a %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% constructor a %}" } + children: + - type: LiquidTag + name: constructor + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% constructor a %}" } + markupPosition: { start: [0, 15], text: "a " } + position: { start: [0, 0], text: "{% constructor a %}" } + - name: parses the Object.prototype name constructor as an unknown tag in {% liquid %} + source: |- + {% liquid + constructor a + %} + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + constructor a + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + constructor a + %} + markupPosition: + start: [0, 9] + text: |2 + + constructor a + position: + start: [0, 0] + text: |- + {% liquid + constructor a + %} + markup: + - type: LiquidTag + name: constructor + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [1, 2], text: constructor a } + markupPosition: { start: [1, 14], text: a } + position: { start: [1, 2], text: constructor a } + - name: parses the Object.prototype name toString as an unknown tag + source: "{% toString a %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% toString a %}" } + children: + - type: LiquidTag + name: toString + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% toString a %}" } + markupPosition: { start: [0, 12], text: "a " } + position: { start: [0, 0], text: "{% toString a %}" } + - name: parses the Object.prototype name toString as an unknown tag in {% liquid %} + source: |- + {% liquid + toString a + %} + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + toString a + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + toString a + %} + markupPosition: + start: [0, 9] + text: |2 + + toString a + position: + start: [0, 0] + text: |- + {% liquid + toString a + %} + markup: + - type: LiquidTag + name: toString + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [1, 2], text: toString a } + markupPosition: { start: [1, 11], text: a } + position: { start: [1, 2], text: toString a } + - name: parses the Object.prototype name hasOwnProperty as an unknown tag + source: "{% hasOwnProperty a %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% hasOwnProperty a %}" } + children: + - type: LiquidTag + name: hasOwnProperty + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% hasOwnProperty a %}" } + markupPosition: { start: [0, 18], text: "a " } + position: { start: [0, 0], text: "{% hasOwnProperty a %}" } + - name: parses the Object.prototype name hasOwnProperty as an unknown tag in {% liquid %} + source: |- + {% liquid + hasOwnProperty a + %} + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + hasOwnProperty a + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + hasOwnProperty a + %} + markupPosition: + start: [0, 9] + text: |2 + + hasOwnProperty a + position: + start: [0, 0] + text: |- + {% liquid + hasOwnProperty a + %} + markup: + - type: LiquidTag + name: hasOwnProperty + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [1, 2], text: hasOwnProperty a } + markupPosition: { start: [1, 17], text: a } + position: { start: [1, 2], text: hasOwnProperty a } + - name: parses the Object.prototype name __proto__ as an unknown tag + source: "{% __proto__ a %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% __proto__ a %}" } + children: + - type: LiquidTag + name: __proto__ + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% __proto__ a %}" } + markupPosition: { start: [0, 13], text: "a " } + position: { start: [0, 0], text: "{% __proto__ a %}" } + - name: parses the Object.prototype name __proto__ as an unknown tag in {% liquid %} + source: |- + {% liquid + __proto__ a + %} + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + __proto__ a + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + __proto__ a + %} + markupPosition: + start: [0, 9] + text: |2 + + __proto__ a + position: + start: [0, 0] + text: |- + {% liquid + __proto__ a + %} + markup: + - type: LiquidTag + name: __proto__ + markup: a + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [1, 2], text: __proto__ a } + markupPosition: { start: [1, 12], text: a } + position: { start: [1, 2], text: __proto__ a } From 1d06444db8b42009d945f54a36577f52cb1f1650 Mon Sep 17 00:00:00 2001 From: Natacha Levesque Date: Thu, 10 Sep 2026 15:54:42 -0400 Subject: [PATCH 06/20] Type block and section arguments as one BlockArgument Block and section markup now share a single BlockArgument type, whose value is a scalar or a single-level array literal, instead of a LiquidNamedArgument | BlockArrayArgument union. Filters in these arguments are still rejected, and the tag falls back to string markup. Co-Authored-By: Charles-P. Clermont Co-Authored-By: Claude Opus 5.5 --- packages/liquid-html-parser/src/ast.ts | 28 +++++++------ .../liquid-html-parser/src/markup/parser.ts | 39 +++++++------------ .../liquid-html-parser/src/tags/block.test.ts | 10 +++++ packages/liquid-html-parser/src/tags/block.ts | 4 +- .../src/tags/section.test.ts | 11 +++++- specs/parser/liquid-tags.yml | 38 ++++++++++++++++++ 6 files changed, 92 insertions(+), 38 deletions(-) diff --git a/packages/liquid-html-parser/src/ast.ts b/packages/liquid-html-parser/src/ast.ts index cc1051e66..c0451ea17 100644 --- a/packages/liquid-html-parser/src/ast.ts +++ b/packages/liquid-html-parser/src/ast.ts @@ -358,11 +358,8 @@ export interface LiquidTagBlock extends LiquidTagNode { /** The name of the block template */ name: LiquidString; - // TODO: BlockArrayArgument exists as a separate type because - // LiquidNamedArgument.value is typed as LiquidExpression, which doesn't - // include BlockArrayLiteral. Once LiquidExpression supports array literals, - // unify BlockArrayArgument into LiquidNamedArgument. - args: (LiquidNamedArgument | BlockArrayArgument)[]; + /** Optional named arguments, including block-local arrays. */ + args: BlockArgument[]; } /** https://shopify.dev/docs/api/liquid/tags#section */ @@ -373,7 +370,7 @@ export interface SectionMarkup extends ASTNode { /** The name of the section template */ name: LiquidString; /** Optional named arguments */ - args: (LiquidNamedArgument | BlockArrayArgument)[]; + args: BlockArgument[]; } /** https://shopify.dev/docs/api/liquid/tags#sections */ @@ -591,20 +588,29 @@ export interface LiquidRange extends ASTNode { * A single-level array literal, e.g. `['a', 'b']` or `[1, 2, x]`. * Supported as a block or section tag argument value. Nested arrays are rejected. * - * Reachable only via `BlockArrayArgument.value` and the block/section parser path. + * Reachable only via `BlockArgument.value` and the block/section parser path. */ export interface BlockArrayLiteral extends ASTNode<'BlockArrayLiteral'> { /** The parsed element expressions, in source order */ elements: LiquidExpression[]; } -/** - * Tag arg whose value is a literal array. Lives on block and section markup. - */ -export interface BlockArrayArgument extends ASTNode { +/** Value accepted by named arguments on block and section tags. */ +export type BlockArgumentValue = LiquidExpression | BlockArrayLiteral; + +/** Named argument accepted by block and section tags. */ +export interface BlockArgument extends ASTNode { /** name: value */ name: string; + /** Scalar or single-level array value. */ + value: BlockArgumentValue; +} + +/** + * Tag arg whose value is a literal array. Lives on block and section markup. + */ +export interface BlockArrayArgument extends BlockArgument { /** name: value (always an array literal) */ value: BlockArrayLiteral; } diff --git a/packages/liquid-html-parser/src/markup/parser.ts b/packages/liquid-html-parser/src/markup/parser.ts index fc7b7eb4f..889ac2f9d 100644 --- a/packages/liquid-html-parser/src/markup/parser.ts +++ b/packages/liquid-html-parser/src/markup/parser.ts @@ -3,7 +3,8 @@ import type { LiquidExpression, LiquidVariableLookup, BlockArrayLiteral, - BlockArrayArgument, + BlockArgument, + BlockArgumentValue, LiquidVariableLookupMode, LiquidConditionalExpression, ComplexLiquidExpression, @@ -1316,8 +1317,8 @@ export class MarkupParser { * section tags use this path because both accept scalar values and * single-level array literals. */ - blockNamedArguments(): (LiquidNamedArgument | BlockArrayArgument)[] { - const args: (LiquidNamedArgument | BlockArrayArgument)[] = []; + blockNamedArguments(): BlockArgument[] { + const args: BlockArgument[] = []; this.pushBlockNamedArgument(args); while (this.consumeOptional(MarkupTokenType.Comma)) { if (!this.look(MarkupTokenType.Id)) break; /* trailing comma */ @@ -1328,10 +1329,10 @@ export class MarkupParser { /* * Parses one block named argument and appends it to the unified list. - * The if/else is required for TypeScript to narrow the value type into - * the correct branch of the LiquidNamedArgument | BlockArrayArgument union. + * The block-specific argument type keeps filtered and array values out of + * the shared LiquidNamedArgument contract used by other tags. */ - private pushBlockNamedArgument(args: (LiquidNamedArgument | BlockArrayArgument)[]): void { + private pushBlockNamedArgument(args: BlockArgument[]): void { const nameToken = this.consume(MarkupTokenType.Id); let name = nameToken.value; while (this.consumeOptional(MarkupTokenType.Dot)) { @@ -1342,30 +1343,20 @@ export class MarkupParser { const value = this.blockValueExpression(); const position = { start: nameToken.start, end: value.position.end }; - if (value.type === 'BlockArrayLiteral') { - args.push({ - type: NodeTypes.NamedArgument, - name, - value, - position, - source: this.source, - }); - } else { - args.push({ - type: NodeTypes.NamedArgument, - name, - value, - position, - source: this.source, - }); - } + args.push({ + type: NodeTypes.NamedArgument, + name, + value, + position, + source: this.source, + }); } /* * Parses a block or section argument value: a leading `[` opens an array * literal, and everything else delegates to the shared `valueExpression()`. */ - private blockValueExpression(): LiquidExpression | BlockArrayLiteral { + private blockValueExpression(): BlockArgumentValue { if (this.look(MarkupTokenType.OpenSquare)) { return this.blockArrayLiteral(); } diff --git a/packages/liquid-html-parser/src/tags/block.test.ts b/packages/liquid-html-parser/src/tags/block.test.ts index b8922ddae..9b0308fd7 100644 --- a/packages/liquid-html-parser/src/tags/block.test.ts +++ b/packages/liquid-html-parser/src/tags/block.test.ts @@ -58,6 +58,16 @@ describe('blockTag', () => { ]); }); + it('rejects filtered named arguments', () => { + expect(() => + blockTag.parse( + 'block', + parser("'card', product: collections.frontpage.products | first"), + stubParser, + ), + ).toThrow("Unexpected token in 'block' tag: |"); + }); + describe('array-literal named args (block-only)', () => { it('parses an array of strings', () => { const result = blockTag.parse('block', parser("'name', items: ['a', 'b']"), stubParser); diff --git a/packages/liquid-html-parser/src/tags/block.ts b/packages/liquid-html-parser/src/tags/block.ts index e9232d026..f633bdff4 100644 --- a/packages/liquid-html-parser/src/tags/block.ts +++ b/packages/liquid-html-parser/src/tags/block.ts @@ -1,4 +1,4 @@ -import type { BlockMarkup, BlockArrayArgument, LiquidNamedArgument } from '../ast'; +import type { BlockArgument, BlockMarkup } from '../ast'; import type { MarkupParser } from '../markup/parser'; import { MarkupTokenType } from '../markup/tokenizer'; import { NodeTypes } from '../types'; @@ -25,7 +25,7 @@ export const blockTag: TagDefinitionBlock = { throw new Error(`in 'block' - '${name.value}' is not a valid block type`); } - let args: (LiquidNamedArgument | BlockArrayArgument)[] = []; + let args: BlockArgument[] = []; if (markup.consumeOptional(MarkupTokenType.Comma) && !markup.isAtEnd()) { args = markup.blockNamedArguments(); } diff --git a/packages/liquid-html-parser/src/tags/section.test.ts b/packages/liquid-html-parser/src/tags/section.test.ts index cef45abf4..8f7917c1a 100644 --- a/packages/liquid-html-parser/src/tags/section.test.ts +++ b/packages/liquid-html-parser/src/tags/section.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import { sectionTag } from './section'; import { TagKind, type Parser } from '../environment'; -import { tokenizeMarkup } from '../markup/tokenizer'; +import { MarkupTokenType, tokenizeMarkup } from '../markup/tokenizer'; import { MarkupParser } from '../markup/parser'; import { NodeTypes } from '../types'; @@ -56,6 +56,15 @@ describe('sectionTag', () => { }); }); + it('leaves filtered named arguments unconsumed', () => { + const markup = parser("'featured', product: collections.frontpage.products | first"); + + sectionTag.parse('section', markup, stubParser); + + expect(markup.peek().type).toBe(MarkupTokenType.Pipe); + expect(markup.isAtEnd()).toBe(false); + }); + it('parses section setting args with array values', () => { const result = sectionTag.parse( 'section', diff --git a/specs/parser/liquid-tags.yml b/specs/parser/liquid-tags.yml index 05c3e4fc8..34fc11e9f 100644 --- a/specs/parser/liquid-tags.yml +++ b/specs/parser/liquid-tags.yml @@ -2159,3 +2159,41 @@ cases: blockStartPosition: { start: [1, 2], text: __proto__ a } markupPosition: { start: [1, 12], text: a } position: { start: [1, 2], text: __proto__ a } + - name: should keep filtered block arguments as string markup + source: "{% block 'card', product: products | first %}{% endblock %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% block 'card', product: products | first %}{% endblock %}" } + children: + - type: LiquidTag + name: block + markup: "'card', product: products | first" + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% block 'card', product: products | first %}" } + blockEndPosition: { start: [0, 45], text: "{% endblock %}" } + markupPosition: { start: [0, 9], text: "'card', product: products | first " } + position: { start: [0, 0], text: "{% block 'card', product: products | first %}{% endblock %}" } + reason: "Unexpected token in 'block' tag: |" + children: [] + - name: should keep filtered section arguments as string markup + source: "{% section 'featured', product: products | first %}" + parse: [html, liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% section 'featured', product: products | first %}" } + children: + - type: LiquidTag + name: section + markup: "'featured', product: products | first" + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% section 'featured', product: products | first %}" } + markupPosition: { start: [0, 11], text: "'featured', product: products | first " } + position: { start: [0, 0], text: "{% section 'featured', product: products | first %}" } + reason: unexpected tokens after markup From b8414be8a7d53b87d3bc780f06d3fe7a959afa46 Mon Sep 17 00:00:00 2001 From: Iain Wilson Date: Mon, 31 Aug 2026 14:03:04 +0200 Subject: [PATCH 07/20] Test parse error locations at the end of input liquid-html-parser already clamps out-of-range indices when it builds a LiquidHTMLASTParsingError; these tests pin that an unclosed `{{` at the end of input is reported as a parse error with a real location. Co-Authored-By: Claude Opus 5.5 --- .../liquid-html-parser/src/errors.test.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 packages/liquid-html-parser/src/errors.test.ts diff --git a/packages/liquid-html-parser/src/errors.test.ts b/packages/liquid-html-parser/src/errors.test.ts new file mode 100644 index 000000000..227d71848 --- /dev/null +++ b/packages/liquid-html-parser/src/errors.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest'; +import { toLiquidHtmlAST } from './ast'; +import { LiquidHTMLASTParsingError } from './errors'; + +/** + * A parse error has to arrive as a parse error. `line-column` answers `null` for an + * index outside the source, and the token that fails at the end of input starts at + * `source.length` — one past the last character — so building the error used to throw + * `TypeError: Cannot read properties of null (reading 'line')` from its own + * constructor. A caller that wanted to catch a syntax error instead got an unrelated + * TypeError with no position on it. + */ +describe('Unit: LiquidHTMLASTParsingError', () => { + describe('errors at the end of input', () => { + it('reports an unclosed variable output as a parsing error', () => { + const source = '{{ unclosed '; + + expect(() => toLiquidHtmlAST(source)).toThrowError(LiquidHTMLASTParsingError); + }); + + it('points at the last position in the source', () => { + const source = '{{ unclosed '; + let thrown: LiquidHTMLASTParsingError | undefined; + + try { + toLiquidHtmlAST(source); + } catch (error) { + thrown = error as LiquidHTMLASTParsingError; + } + + expect(thrown?.name).to.eql('LiquidHTMLParsingError'); + // `col` is 1-based, so the last character of the source is `source.length`. + expect(thrown?.loc).to.eql({ + start: { line: 1, column: source.length }, + end: { line: 1, column: source.length }, + }); + }); + + it('reports the line an unclosed output starts on, not the first line', () => { + const source = 'hello\nworld\n{{ unclosed '; + let thrown: LiquidHTMLASTParsingError | undefined; + + try { + toLiquidHtmlAST(source); + } catch (error) { + thrown = error as LiquidHTMLASTParsingError; + } + + expect(thrown?.loc?.start.line).to.eql(3); + }); + }); + + describe('indices outside the source', () => { + it('falls back to the document start for an empty source', () => { + const error = new LiquidHTMLASTParsingError('boom', '', 0, 0); + + expect(error.loc).to.eql({ start: { line: 1, column: 1 }, end: { line: 1, column: 1 } }); + }); + + it('clamps an index past the end of the source', () => { + const error = new LiquidHTMLASTParsingError('boom', 'ab', 99, 99); + + expect(error.loc).to.eql({ start: { line: 1, column: 2 }, end: { line: 1, column: 2 } }); + }); + + it('clamps a negative index', () => { + const error = new LiquidHTMLASTParsingError('boom', 'ab', -5, -5); + + expect(error.loc).to.eql({ start: { line: 1, column: 1 }, end: { line: 1, column: 1 } }); + }); + }); + + describe('errors inside the source', () => { + it('keeps the reported position of an error away from the boundaries', () => { + const source = 'line one\nline two\nline three'; + const index = source.indexOf('two'); + const error = new LiquidHTMLASTParsingError('boom', source, index, index + 2); + + expect(error.loc).to.eql({ start: { line: 2, column: 6 }, end: { line: 2, column: 8 } }); + }); + }); +}); From e391d5ca2bec6a47212630b98270195b54a07f5a Mon Sep 17 00:00:00 2001 From: River Date: Thu, 8 Oct 2026 13:44:31 +0000 Subject: [PATCH 08/20] Let each raw tag end its body by its own Liquid rule Each raw tag now carries the rule of the Liquid class that parses it: - comment nests, skips raw bodies, and closes on a tag named endcomment whatever follows the name, as Comment#parse_body does; - raw, javascript, schema and stylesheet close at the last tag of their first end token, as Liquid::Raw does; - doc reads like raw but rejects a nested doc, as Liquid::Doc does; - style is a Liquid::Block, so it nests and skips raw bodies inside it. The same rules apply to raw tags inside {% liquid %}, whose lines are named only after ASCII indentation. Templates that Liquid reports as never closed now fail to parse here too. Co-Authored-By: Martin Morissette Co-Authored-By: Mbarak Bujra Co-Authored-By: Claude Opus 5.5 --- .../liquid-html-parser/specs/architecture.md | 2 +- .../specs/recursive-descent.md | 20 +- .../src/document/liquid-lines.ts | 45 +- .../src/document/liquid-raw.ts | 112 +-- .../src/document/raw-body.ts | 169 ++++ .../src/environment.test.ts | 7 +- .../liquid-html-parser/src/tag-definitions.ts | 29 + .../liquid-html-parser/src/tags/raw.test.ts | 28 + packages/liquid-html-parser/src/tags/raw.ts | 39 +- specs/parser/liquid-lines.yml | 422 +++++++- specs/parser/liquid-raw.yml | 948 +++++++++++++++++- 11 files changed, 1647 insertions(+), 174 deletions(-) create mode 100644 packages/liquid-html-parser/src/document/raw-body.ts diff --git a/packages/liquid-html-parser/specs/architecture.md b/packages/liquid-html-parser/specs/architecture.md index 428c1d92e..f91427f56 100644 --- a/packages/liquid-html-parser/specs/architecture.md +++ b/packages/liquid-html-parser/specs/architecture.md @@ -745,7 +745,7 @@ liquid tag parse(name, markupParser, parser): **Block nesting:** Block tags inside `{% liquid %}` consume subsequent lines until their matching `end*` line, reusing `finalizeBranch()`. -**Raw tags inside `{% liquid %}`:** When the line is a raw tag (e.g., `comment`), subsequent lines are consumed until `endcomment`. +**Raw tags inside `{% liquid %}`:** When the line is a raw tag (e.g., `comment`), subsequent lines are consumed until the line that ends its body under the tag's `body` rule (`findRawLineBodyEnd` in `raw-body.ts`); a `comment` nests, and a `raw` or `doc` body never closes there. **Nested `{% liquid %}`:** Supported. Inner `liquid` tag processes its own body independently. diff --git a/packages/liquid-html-parser/specs/recursive-descent.md b/packages/liquid-html-parser/specs/recursive-descent.md index 7265b55f9..98583b9f4 100644 --- a/packages/liquid-html-parser/specs/recursive-descent.md +++ b/packages/liquid-html-parser/specs/recursive-descent.md @@ -199,6 +199,24 @@ Block tags (`LiquidTag` with children) have: - `for`/`tablerow` can contain `else` branch - The **first** branch of any branched tag is an unnamed branch (`LiquidBranchUnnamed` with `name: null`) +**Raw tag bodies:** + +Every raw tag definition carries a `body` rule (`RawBodyRule` in `tag-definitions.ts`), and one scanner (`document/raw-body.ts`) ends every raw body by it. The rule mirrors the Liquid class that parses the tag: + +| Tag | Liquid class | `delimiter` | `sameName` | `opaque` | Closes in `{% liquid %}` | +| ------------------------------------------- | ------------------------------------ | ----------- | ---------- | ---------------------- | ------------------------ | +| `raw`, `javascript`, `schema`, `stylesheet` | `Liquid::Raw` | `trailing` | `ignored` | none | no | +| `doc` | `Liquid::Doc` | `trailing` | `rejected` | none | no | +| `comment` | `Liquid::Comment` | `leading` | `nests` | `raw` | yes | +| `style` | `Liquid::Block` (Shopify's `style`) | `leading` | `nests` | the other six raw tags | yes | + +- The body is read as the tokens `Liquid::Tokenizer` produces: a `{%` tag runs to the first `%}`, and a `{{` output runs to its first `}` (taking a second `}` with it), or through the `%}` of a `{%` that opens inside it +- `leading` names a `{% %}` token by the `#` or word that opens it (`BlockBody::FullToken`); `trailing` names it by its last tag and keeps any text before that tag in the body (`FullTokenPossiblyInvalid`). Either way markup after the name does not matter, so `{% endcomment xyz %}` closes and `{% endcommentXXXXX %}` does not +- `sameName` says what an inner tag with the tag's own name does: `nests` opens another level that its own end tag closes, `ignored` does nothing, and `rejected` throws, as Liquid raises `Nested doc tags are not allowed` +- An `opaque` inner tag is skipped whole, ending by its own rule, so a `{% endcomment %}` inside a `{% raw %}` inside a comment does not count +- A body that runs out of tokens before it closes throws, as Liquid raises `'' tag was never closed` +- Inside `{% liquid %}` the same levels apply line by line, each line named by its leading `#` or word after ASCII-only indentation (a line indented with U+00A0 or other non-ASCII whitespace has no name, as in Liquid's `LiquidTagToken`). A `Liquid::Raw` or `Liquid::Doc` body never closes there, because a line cannot hold its `{% end %}` + **Inline comments `{% # %}`:** - `#` is a valid tag name, producing `LiquidTag` with `name: '#'` and `markup: string` (the rest of the line) @@ -213,7 +231,7 @@ Block tags (`LiquidTag` with children) have: - Nesting is fully supported: `for` inside `if` inside `liquid` is valid - Branch tags (`elsif`, `else`, `when`) work inside `{% liquid %}` blocks - Blocks open and close within the same `{% liquid %}` boundary -- Raw tags (`comment`/`endcomment`, `raw`/`endraw`) work inside `{% liquid %}`, consuming lines until their end tag +- Raw tags (`comment`/`endcomment`, `raw`/`endraw`) work inside `{% liquid %}`, consuming lines until their end tag (a `comment` nests; see Comment bodies) - Nested `{% liquid %}` inside `{% liquid %}` is supported (inner scope is per-line) - There is no `{{ }}` syntax inside `{% liquid %}` — use `echo` instead - `markup` is `LiquidStatement[]` diff --git a/packages/liquid-html-parser/src/document/liquid-lines.ts b/packages/liquid-html-parser/src/document/liquid-lines.ts index 46900845a..117ae101e 100644 --- a/packages/liquid-html-parser/src/document/liquid-lines.ts +++ b/packages/liquid-html-parser/src/document/liquid-lines.ts @@ -39,6 +39,7 @@ import { envelopeFromLine } from '../shared'; import { isBranchName, finalizeBranch } from './liquid-blocks'; import { ChildFilterMode, filterChildren } from './tree-builder'; import { rawMarkupKindForTag } from './liquid-raw'; +import { findRawLineBodyEnd } from './raw-body'; /** * Interface capturing what line-based parsing free functions need from @@ -171,7 +172,7 @@ export function parseLineBlockTag( // lineRawTag := tagName LF line* "end" tagName export function parseLineRawTag( parser: LineParserDelegate, - _def: TagDefinitionRaw, + def: TagDefinitionRaw, envelope: LiquidTagEnvelope, ctx: LiquidLineContext, ): LiquidRawTag { @@ -192,43 +193,11 @@ export function parseLineRawTag( ? ctx.lines[ctx.index - 1].lineEnd + 1 : envelope.blockStartPosition.end; - // `comment`/`doc` bodies balance nested opens of the same tag before - // matching their end line, mirroring the document-path scan in - // liquid-raw.ts (Ruby comment.rb v5.13.0 `comment_tag_depth`). A nested - // `raw` block is carved out: `raw` is first-match and does not nest, so an - // `endcomment`/`comment` word sitting on a line inside a raw body must not - // affect the depth. Every other raw tag (`raw`, `javascript`, `schema`, - // `style`) keeps the original first-match scan and stays byte-identical. - const balanced = tagName === 'comment' || tagName === 'doc'; - let endLineIndex = -1; - if (balanced) { - let depth = 0; - for (let i = ctx.index; i < ctx.lines.length; i++) { - const name = ctx.lines[i].tagName; - if (name === 'raw') { - // Skip past the nested raw block; its body lines never affect depth. - i++; - while (i < ctx.lines.length && ctx.lines[i].tagName !== 'endraw') i++; - continue; - } - if (name === tagName) { - depth++; - } else if (name === endTagName) { - if (depth === 0) { - endLineIndex = i; - break; - } - depth--; - } - } - } else { - for (let i = ctx.index; i < ctx.lines.length; i++) { - if (ctx.lines[i].tagName === endTagName) { - endLineIndex = i; - break; - } - } - } + const endLineIndex = + findRawLineBodyEnd(parser.getSource(), ctx.lines, ctx.index, tagName, def.body, (name) => { + const inner = parser.lineEnv.tagForName(name); + return inner?.kind === TagKind.Raw ? inner.body : undefined; + }) ?? -1; if (endLineIndex === -1) { throw new LiquidHTMLASTParsingError( diff --git a/packages/liquid-html-parser/src/document/liquid-raw.ts b/packages/liquid-html-parser/src/document/liquid-raw.ts index 5e8d67d1e..e4ebda75b 100644 --- a/packages/liquid-html-parser/src/document/liquid-raw.ts +++ b/packages/liquid-html-parser/src/document/liquid-raw.ts @@ -2,12 +2,14 @@ import { TokenType } from './tokenizer'; import type { Token } from './tokenizer'; import { ParserBase } from './base'; import { makeTextNode, makeRawMarkup, makeLiquidRawTag } from './factories'; -import type { LiquidTagEnvelope, LiquidOpenWhitespace, LiquidCloseWhitespace } from './factories'; +import type { LiquidTagEnvelope } from './factories'; import { ChildFilterMode, filterChildren } from './tree-builder'; import type { Position } from '../types'; import { RawMarkupKinds } from '../ast'; import type { LiquidRawTag, LiquidNode, TextNode, LiquidVariableOutput } from '../ast'; -import type { TagDefinitionRaw } from '../environment'; +import { TagKind } from '../environment'; +import type { TagDefinition, TagDefinitionRaw } from '../environment'; +import { findRawBodyEnd } from './raw-body'; import { LiquidHTMLASTParsingError } from '../errors'; import { parseLiquidDoc } from '../liquid-doc/parser'; @@ -18,6 +20,7 @@ import { parseLiquidDoc } from '../liquid-doc/parser'; */ export interface RawParserDelegate extends ParserBase { rawParseHtml: boolean; + tagForName(name: string): TagDefinition | undefined; parseLiquidVariableOutput(): LiquidVariableOutput; parseLiquidTag(): LiquidRawTag | import('../ast').LiquidTag; } @@ -30,9 +33,17 @@ export function parseRawTag( closeToken: Token, ): LiquidRawTag { const tagName = envelope.tagName; - const endTagName = `end${tagName}`; - const endTag = scanForEndTag(parser, endTagName); + const endTag = findRawBodyEnd( + parser.getSource(), + parser.tokenAt(parser.getPosition()).start, + tagName, + def.body, + (name) => { + const inner = parser.tagForName(name); + return inner?.kind === TagKind.Raw ? inner.body : undefined; + }, + ); if (!endTag) { throw new LiquidHTMLASTParsingError( `Attempting to end parsing before LiquidRawTag '${tagName}' was closed`, @@ -165,99 +176,6 @@ export function parseRawTagBody( return filterChildren(ChildFilterMode.Syntactic, [textNode], source) as (LiquidNode | TextNode)[]; } -/** - * Result of scanning for a raw tag's end tag in the source string. - * - * We scan the source directly (rather than the token stream) because the - * tokenizer doesn't know about raw-tag semantics. Content inside a raw - * tag body may contain `{%` sequences that the tokenizer greedily enters - * as LiquidTag mode, mangling the tokens around the real end tag. - */ -export interface EndTagScanResult { - /** Source offset where the end tag starts (the `{`). */ - tagStart: number; - /** Source offset where the end tag ends (after `%}`). */ - tagEnd: number; - /** Opening whitespace-strip marker: `'-'` or `''`. */ - wsStart: LiquidOpenWhitespace; - /** Closing whitespace-strip marker: `'-'` or `''`. */ - wsEnd: LiquidCloseWhitespace; -} - -export function scanForEndTag(parser: ParserBase, endTagName: string): EndTagScanResult | null { - const source = parser.getSource(); - const searchStart = parser.tokenAt(parser.getPosition()).start; - - // `comment` and `doc` bodies balance nested opens of the same tag before - // matching their end tag, mirroring Ruby comment.rb v5.13.0's - // `comment_tag_depth`. `raw` (and every other raw tag) keeps first-match - // semantics — Ruby's raw does not balance. - if (endTagName === 'endcomment' || endTagName === 'enddoc') { - return scanForBalancedEndTag(source, searchStart, endTagName); - } - - const pattern = new RegExp(`\\{%(-?)\\s*${endTagName}\\s*(-?)%\\}`); - const match = pattern.exec(source.slice(searchStart)); - if (!match) return null; - - const tagStart = searchStart + match.index; - const tagEnd = tagStart + match[0].length; - const wsStart: LiquidOpenWhitespace = match[1] === '-' ? '-' : ''; - const wsEnd: LiquidCloseWhitespace = match[2] === '-' ? '-' : ''; - - return { tagStart, tagEnd, wsStart, wsEnd }; -} - -/** - * Depth-balancing end-tag scan for `comment`/`doc`. Counts nested opens of the - * same tag so a `{% comment %}` inside the body is paired with its own - * `{% endcomment %}` rather than closing the outer block early. - * - * `searchStart` is already positioned past the outer open tag, so depth starts - * at 0 and every `{% comment %}` encountered is a nested open. A nested - * `{% raw %}`…`{% endraw %}` is carved out — its literal contents (which may - * contain `{% comment %}`/`{% endcomment %}` text) do not affect the depth — - * mirroring Ruby's `parse_raw_tag_body`. - */ -function scanForBalancedEndTag( - source: string, - searchStart: number, - endTagName: string, -): EndTagScanResult | null { - const openTagName = endTagName.slice(3); // "endcomment" -> "comment" - const scanner = new RegExp( - `\\{%(-?)\\s*(${openTagName}|${endTagName}|raw|endraw)\\b\\s*(-?)%\\}`, - 'g', - ); - scanner.lastIndex = searchStart; - - let depth = 0; - let inRaw = false; - let match: RegExpExecArray | null; - while ((match = scanner.exec(source)) !== null) { - const name = match[2]; - if (inRaw) { - if (name === 'endraw') inRaw = false; - continue; - } - if (name === 'raw') { - inRaw = true; - } else if (name === openTagName) { - depth++; - } else if (name === endTagName) { - if (depth === 0) { - const tagStart = match.index; - const tagEnd = tagStart + match[0].length; - const wsStart: LiquidOpenWhitespace = match[1] === '-' ? '-' : ''; - const wsEnd: LiquidCloseWhitespace = match[3] === '-' ? '-' : ''; - return { tagStart, tagEnd, wsStart, wsEnd }; - } - depth--; - } - } - return null; -} - export function rawMarkupKindForTag(tagName: string, bodySource: string = ''): RawMarkupKinds { switch (tagName) { case 'javascript': diff --git a/packages/liquid-html-parser/src/document/raw-body.ts b/packages/liquid-html-parser/src/document/raw-body.ts new file mode 100644 index 000000000..5b1000ffa --- /dev/null +++ b/packages/liquid-html-parser/src/document/raw-body.ts @@ -0,0 +1,169 @@ +import type { LiquidLine, RawBodyEnd, RawBodyRule } from '../tag-definitions'; + +// Liquid's `BlockBody::FullToken`, `FullTokenPossiblyInvalid` and `LiquidTagToken`, +// with Ruby's ASCII `\s`. +const RUBY_WHITESPACE = '[ \\t\\n\\v\\f\\r]'; +const FULL_TOKEN = new RegExp( + `^\\{%-?${RUBY_WHITESPACE}*(#|\\w+)${RUBY_WHITESPACE}*[\\s\\S]*?-?%\\}$`, +); +const FULL_TOKEN_POSSIBLY_INVALID = new RegExp( + `^([\\s\\S]*)\\{%-?${RUBY_WHITESPACE}*(\\w+)${RUBY_WHITESPACE}*[\\s\\S]*-?%\\}$`, +); +const LIQUID_TAG_NAME = /^(#|\w+)/; +const RUBY_LINE_INDENT = /^[ \t\v\f\r]*$/; +const TAG_OR_VARIABLE_START = /\{[{%]/g; + +/** Looks up the body rule of another raw tag, for the tags a rule skips as opaque. */ +export type RawBodyRuleFor = (tagName: string) => RawBodyRule | undefined; + +/** + * Where the body of `tagName` that starts at `bodyStart` ends under `rule`, + * reading the source as `Liquid::Tokenizer` tokens. Returns null where Liquid + * reports the tag as never closed, or rejects a tag nested inside it. + */ +export function findRawBodyEnd( + source: string, + bodyStart: number, + tagName: string, + rule: RawBodyRule, + ruleFor: RawBodyRuleFor, +): RawBodyEnd | null { + const endName = `end${tagName}`; + let tokenStart = bodyStart; + let depth = 1; + + while (tokenStart < source.length) { + const tokenEnd = liquidTokenEnd(source, tokenStart); + const token = source.slice(tokenStart, tokenEnd); + const tag = tokenTag(token, rule.delimiter); + + if (tag?.name === endName) { + depth -= 1; + if (depth === 0) { + const tagStart = tokenStart + tag.offset; + return { + tagStart, + tagEnd: tokenEnd, + wsStart: source[tagStart + 2] === '-' ? '-' : '', + wsEnd: token[token.length - 3] === '-' ? '-' : '', + }; + } + } else if (tag?.name === tagName) { + if (rule.sameName === 'rejected') return null; + if (rule.sameName === 'nests') depth += 1; + } else if (tag && rule.opaque.includes(tag.name)) { + const innerRule = ruleFor(tag.name); + if (innerRule) { + const innerEnd = findRawBodyEnd(source, tokenEnd, tag.name, innerRule, ruleFor); + if (!innerEnd) return null; + tokenStart = innerEnd.tagEnd; + continue; + } + } + tokenStart = tokenEnd; + } + + return null; +} + +/** + * The index of the line that ends the body of `tagName` starting at `lines[from]` + * inside `{% liquid %}`, where each line is a token named by its leading `#` or + * word. Returns null where Liquid reports the tag as never closed there. + */ +export function findRawLineBodyEnd( + source: string, + lines: readonly LiquidLine[], + from: number, + tagName: string, + rule: RawBodyRule, + ruleFor: RawBodyRuleFor, +): number | null { + if (!rule.closesInLiquidTag) return null; + + const endName = `end${tagName}`; + let depth = 1; + + for (let index = from; index < lines.length; index++) { + const name = lineTagName(source, lines[index]); + + if (name === endName) { + depth -= 1; + if (depth === 0) return index; + } else if (name === tagName) { + if (rule.sameName === 'rejected') return null; + if (rule.sameName === 'nests') depth += 1; + } else if (name !== undefined && rule.opaque.includes(name)) { + const innerRule = ruleFor(name); + if (innerRule) { + const innerEnd = findRawLineBodyEnd(source, lines, index + 1, name, innerRule, ruleFor); + if (innerEnd === null) return null; + index = innerEnd; + } + } + } + + return null; +} + +// The tag a token names under `delimiter`, and where that tag starts in the token. +function tokenTag( + token: string, + delimiter: RawBodyRule['delimiter'], +): { name: string; offset: number } | undefined { + if (delimiter === 'leading') { + const name = FULL_TOKEN.exec(token)?.[1]; + return name === undefined ? undefined : { name, offset: 0 }; + } + const match = FULL_TOKEN_POSSIBLY_INVALID.exec(token); + return match ? { name: match[2], offset: match[1].length } : undefined; +} + +// `BlockBody::LiquidTagToken` indents only with ASCII whitespace, so a line +// indented with a character such as U+00A0 has no name. +function lineTagName(source: string, line: LiquidLine): string | undefined { + const lineStart = source.lastIndexOf('\n', line.nameOffset - 1) + 1; + if (!RUBY_LINE_INDENT.test(source.slice(lineStart, line.nameOffset))) return undefined; + return LIQUID_TAG_NAME.exec(line.tagName)?.[1]; +} + +/** + * The end offset of the Liquid token that starts at `start`, split the way + * `Liquid::Tokenizer#next_token` splits it: a tag runs to the first `%}`, and + * text runs to the next `{{` or `{%`. + */ +function liquidTokenEnd(source: string, start: number): number { + if (source.startsWith('{%', start)) return tagTokenEnd(source, start + 2); + if (source.startsWith('{{', start)) return variableTokenEnd(source, start + 2); + + TAG_OR_VARIABLE_START.lastIndex = start; + const next = TAG_OR_VARIABLE_START.exec(source); + return next ? next.index : source.length; +} + +// `Tokenizer#next_tag_token`: an unterminated `{%` is a token of its own. +function tagTokenEnd(source: string, afterOpen: number): number { + const close = source.indexOf('%}', afterOpen); + return close === -1 ? afterOpen : close + 2; +} + +// `Tokenizer#next_variable_token`: a `{%` inside an unfinished `{{` extends it to that tag's `%}`. +function variableTokenEnd(source: string, afterOpen: number): number { + let position = afterOpen; + let current = source[position++]; + let next = current; + + while (next !== undefined) { + while (current !== undefined && current !== '}' && current !== '{') { + current = source[position++]; + } + if (current === undefined || position >= source.length) return source.length; + + next = source[position++]; + if (current === '}') return next === '}' ? position : position - 1; + if (current === '{' && next === '%') return tagTokenEnd(source, position); + current = next; + } + + return source.length; +} diff --git a/packages/liquid-html-parser/src/environment.test.ts b/packages/liquid-html-parser/src/environment.test.ts index 91d1dd3e4..991b78f3a 100644 --- a/packages/liquid-html-parser/src/environment.test.ts +++ b/packages/liquid-html-parser/src/environment.test.ts @@ -20,7 +20,12 @@ function makeStandaloneDef(): TagDefinitionTag { } function makeRawDef(parseLiquidInBody?: boolean): TagDefinitionRaw { - return { kind: TagKind.Raw, parse: stubParse, parseLiquidInBody }; + return { + kind: TagKind.Raw, + parse: stubParse, + parseLiquidInBody, + body: { delimiter: 'trailing', sameName: 'ignored', opaque: [], closesInLiquidTag: false }, + }; } describe('Unit: Environment', () => { diff --git a/packages/liquid-html-parser/src/tag-definitions.ts b/packages/liquid-html-parser/src/tag-definitions.ts index 90d3eb03b..d1feb4fe1 100644 --- a/packages/liquid-html-parser/src/tag-definitions.ts +++ b/packages/liquid-html-parser/src/tag-definitions.ts @@ -34,6 +34,35 @@ export interface TagDefinitionRaw { /** true for javascript/style/stylesheet, false for raw/comment/schema. doc uses liquid-doc-parser. */ parseLiquidInBody?: boolean; parse(name: string, markup: MarkupParser, parser: Parser): M; + /** Where this tag's body ends, following the Liquid tag class that parses it. */ + body: RawBodyRule; +} + +/** + * How a raw tag's body ends. Each Liquid tag class finds the end of its own body, + * so the rule belongs to the tag, not to the parser. + */ +export interface RawBodyRule { + /** + * How a tag is named inside the body: `leading` reads the name at the start of a + * `{% %}` token (`BlockBody::FullToken`); `trailing` reads the last tag in the + * token and leaves any text before it in the body (`FullTokenPossiblyInvalid`). + */ + delimiter: 'leading' | 'trailing'; + /** What an inner tag with the same name does: opens another level, is ignored, or is a syntax error. */ + sameName: 'nests' | 'ignored' | 'rejected'; + /** Inner tags whose bodies are skipped whole, each ending by its own rule. */ + opaque: readonly string[]; + /** Whether the body can close inside `{% liquid %}`, whose lines cannot hold a `{% %}` delimiter. */ + closesInLiquidTag: boolean; +} + +/** The closing tag of a raw tag's body, as source offsets. */ +export interface RawBodyEnd { + tagStart: number; + tagEnd: number; + wsStart: '-' | ''; + wsEnd: '-' | ''; } export type TagDefinition = diff --git a/packages/liquid-html-parser/src/tags/raw.test.ts b/packages/liquid-html-parser/src/tags/raw.test.ts index 1ef665666..78e993cad 100644 --- a/packages/liquid-html-parser/src/tags/raw.test.ts +++ b/packages/liquid-html-parser/src/tags/raw.test.ts @@ -41,6 +41,34 @@ describe('commentRaw', () => { }); }); +describe('raw tag body rules', () => { + it('follows the Liquid class that parses each tag', () => { + const liquidRaw = { + delimiter: 'trailing', + sameName: 'ignored', + opaque: [], + closesInLiquidTag: false, + }; + expect(rawRaw.body).toEqual(liquidRaw); + expect(javascriptRaw.body).toEqual(liquidRaw); + expect(schemaRaw.body).toEqual(liquidRaw); + expect(stylesheetRaw.body).toEqual(liquidRaw); + expect(docRaw.body).toEqual({ ...liquidRaw, sameName: 'rejected' }); + expect(commentRaw.body).toEqual({ + delimiter: 'leading', + sameName: 'nests', + opaque: ['raw'], + closesInLiquidTag: true, + }); + expect(styleRaw.body).toEqual({ + delimiter: 'leading', + sameName: 'nests', + opaque: ['comment', 'doc', 'javascript', 'raw', 'schema', 'stylesheet'], + closesInLiquidTag: true, + }); + }); +}); + describe('docRaw', () => { it('has raw kind', () => { expect(docRaw.kind).toBe(TagKind.Raw); diff --git a/packages/liquid-html-parser/src/tags/raw.ts b/packages/liquid-html-parser/src/tags/raw.ts index e82d16798..a54cae728 100644 --- a/packages/liquid-html-parser/src/tags/raw.ts +++ b/packages/liquid-html-parser/src/tags/raw.ts @@ -1,5 +1,35 @@ import type { MarkupParser } from '../markup/parser'; -import { TagKind, type TagDefinitionRaw, type Parser } from '../tag-definitions'; +import { TagKind, type Parser, type RawBodyRule, type TagDefinitionRaw } from '../tag-definitions'; + +// `Liquid::Raw` (raw, and liquid_extensions' javascript, schema and stylesheet) +// closes at the last tag of its first `end` token, and cannot close inside +// `{% liquid %}`. +const LIQUID_RAW_BODY: RawBodyRule = { + delimiter: 'trailing', + sameName: 'ignored', + opaque: [], + closesInLiquidTag: false, +}; + +// `Liquid::Doc` reads tags like `Liquid::Raw` but rejects a nested `doc`. +const LIQUID_DOC_BODY: RawBodyRule = { ...LIQUID_RAW_BODY, sameName: 'rejected' }; + +// `Liquid::Comment#parse_body` counts nested comments and skips `raw` bodies. +const LIQUID_COMMENT_BODY: RawBodyRule = { + delimiter: 'leading', + sameName: 'nests', + opaque: ['raw'], + closesInLiquidTag: true, +}; + +// Shopify's `style` tag is a `Liquid::Block`, so its body nests like any block and +// the raw tags inside it end by their own rules. +const LIQUID_BLOCK_BODY: RawBodyRule = { + delimiter: 'leading', + sameName: 'nests', + opaque: ['comment', 'doc', 'javascript', 'raw', 'schema', 'stylesheet'], + closesInLiquidTag: true, +}; function parseNoMarkup(name: string, markup: MarkupParser, _parser: Parser): void { if (!markup.isAtEnd()) { @@ -9,6 +39,7 @@ function parseNoMarkup(name: string, markup: MarkupParser, _parser: Parser): voi export const commentRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_COMMENT_BODY, parseLiquidInBody: false, parse: parseNoMarkup, }; @@ -18,35 +49,41 @@ export const commentRaw: TagDefinitionRaw = { // the body off to the doc-specific parser rather than the standard raw pipeline. export const docRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_DOC_BODY, parse: parseNoMarkup, }; export const rawRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_RAW_BODY, parseLiquidInBody: false, parse: parseNoMarkup, }; export const javascriptRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_RAW_BODY, parseLiquidInBody: true, parse: parseNoMarkup, }; export const schemaRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_RAW_BODY, parseLiquidInBody: false, parse: parseNoMarkup, }; export const styleRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_BLOCK_BODY, parseLiquidInBody: true, parse: parseNoMarkup, }; export const stylesheetRaw: TagDefinitionRaw = { kind: TagKind.Raw, + body: LIQUID_RAW_BODY, parseLiquidInBody: true, parse: parseNoMarkup, }; diff --git a/specs/parser/liquid-lines.yml b/specs/parser/liquid-lines.yml index 6abbf566a..830b7de80 100644 --- a/specs/parser/liquid-lines.yml +++ b/specs/parser/liquid-lines.yml @@ -1931,7 +1931,7 @@ cases: comment inner endcomment - - name: should carve out a nested raw so a literal endcomment inside it does not close early + - name: should leave a comment unclosed when a raw inside it cannot close in {% liquid %} source: |- {% liquid comment @@ -1961,6 +1961,14 @@ cases: children: - type: LiquidTag name: liquid + markup: |- + comment + before + raw + endcomment + endraw + after + endcomment whitespaceStart: "" whitespaceEnd: "" blockStartPosition: @@ -1998,6 +2006,69 @@ cases: after endcomment %} + reason: Unclosed raw tag 'comment' in {% liquid %} block + - name: should close a nested comment at the endcomment of its outer level + source: |- + {% liquid + comment + comment + endcomment + echo 'X' + endcomment + echo 'Y' + %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + comment + comment + endcomment + echo 'X' + endcomment + echo 'Y' + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + comment + comment + endcomment + echo 'X' + endcomment + echo 'Y' + %} + markupPosition: + start: [0, 9] + text: | + + comment + comment + endcomment + echo 'X' + endcomment + echo 'Y' + position: + start: [0, 0] + text: |- + {% liquid + comment + comment + endcomment + echo 'X' + endcomment + echo 'Y' + %} markup: - type: LiquidRawTag name: comment @@ -2008,48 +2079,355 @@ cases: delimiterWhitespaceStart: "" delimiterWhitespaceEnd: "" blockStartPosition: { start: [1, 0], text: comment } - blockEndPosition: { start: [7, 0], text: endcomment } + blockEndPosition: { start: [5, 0], text: endcomment } position: start: [1, 0] text: |- comment - before - raw + comment endcomment - endraw - after + echo 'X' endcomment body: type: RawMarkup kind: text value: | - before - raw + comment endcomment - endraw - after + echo 'X' position: start: [1, 7] text: | - before - raw + comment endcomment - endraw - after + echo 'X' nodes: - type: TextNode value: |- - before - raw + comment endcomment - endraw - after + echo 'X' position: start: [2, 0] text: |- - before - raw + comment endcomment - endraw - after + echo 'X' + - type: LiquidTag + name: echo + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [6, 0], text: echo 'Y' } + markupPosition: { start: [6, 5], text: "'Y'" } + position: { start: [6, 0], text: echo 'Y' } + markup: + type: LiquidVariable + rawSource: "'Y'" + position: { start: [6, 5], text: "'Y'" } + expression: + type: String + single: true + value: Y + position: { start: [6, 5], text: "'Y'" } + filters: [] + - name: should not count a comment line indented with non-ASCII whitespace + source: |- + {% liquid + comment +  comment + endcomment + echo 'Y' + %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + comment +  comment + endcomment + echo 'Y' + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + comment +  comment + endcomment + echo 'Y' + %} + markupPosition: + start: [0, 9] + text: | + + comment +  comment + endcomment + echo 'Y' + position: + start: [0, 0] + text: |- + {% liquid + comment +  comment + endcomment + echo 'Y' + %} + markup: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [1, 7], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [1, 0], text: comment } + blockEndPosition: { start: [3, 0], text: endcomment } + position: + start: [1, 0] + text: |- + comment +  comment + endcomment + body: + type: RawMarkup + kind: text + value: | +  comment + position: + start: [1, 7] + text: | + +  comment + nodes: + - type: TextNode + value: comment + position: { start: [2, 1], text: comment } + - type: LiquidTag + name: echo + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [4, 0], text: echo 'Y' } + markupPosition: { start: [4, 5], text: "'Y'" } + position: { start: [4, 0], text: echo 'Y' } + markup: + type: LiquidVariable + rawSource: "'Y'" + position: { start: [4, 5], text: "'Y'" } + expression: + type: String + single: true + value: Y + position: { start: [4, 5], text: "'Y'" } + filters: [] + - name: should not count a raw line indented with non-ASCII whitespace + source: |- + {% liquid + comment +  raw + endcomment + echo 'Y' + %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + comment +  raw + endcomment + echo 'Y' + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + comment +  raw + endcomment + echo 'Y' + %} + markupPosition: + start: [0, 9] + text: | + + comment +  raw + endcomment + echo 'Y' + position: + start: [0, 0] + text: |- + {% liquid + comment +  raw + endcomment + echo 'Y' + %} + markup: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [1, 7], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [1, 0], text: comment } + blockEndPosition: { start: [3, 0], text: endcomment } + position: + start: [1, 0] + text: |- + comment +  raw + endcomment + body: + type: RawMarkup + kind: text + value: | +  raw + position: + start: [1, 7] + text: | + +  raw + nodes: + - type: TextNode + value: raw + position: { start: [2, 1], text: raw } + - type: LiquidTag + name: echo + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [4, 0], text: echo 'Y' } + markupPosition: { start: [4, 5], text: "'Y'" } + position: { start: [4, 0], text: echo 'Y' } + markup: + type: LiquidVariable + rawSource: "'Y'" + position: { start: [4, 5], text: "'Y'" } + expression: + type: String + single: true + value: Y + position: { start: [4, 5], text: "'Y'" } + filters: [] + - name: should not count an endcomment line indented with non-ASCII whitespace + source: |- + {% liquid + comment +  endcomment + endcomment + echo 'Y' + %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% liquid + comment +  endcomment + endcomment + echo 'Y' + %} + children: + - type: LiquidTag + name: liquid + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: + start: [0, 0] + text: |- + {% liquid + comment +  endcomment + endcomment + echo 'Y' + %} + markupPosition: + start: [0, 9] + text: | + + comment +  endcomment + endcomment + echo 'Y' + position: + start: [0, 0] + text: |- + {% liquid + comment +  endcomment + endcomment + echo 'Y' + %} + markup: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [1, 7], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [1, 0], text: comment } + blockEndPosition: { start: [3, 0], text: endcomment } + position: + start: [1, 0] + text: |- + comment +  endcomment + endcomment + body: + type: RawMarkup + kind: text + value: | +  endcomment + position: + start: [1, 7] + text: | + +  endcomment + nodes: + - type: TextNode + value: endcomment + position: { start: [2, 1], text: endcomment } + - type: LiquidTag + name: echo + whitespaceStart: "" + whitespaceEnd: "" + blockStartPosition: { start: [4, 0], text: echo 'Y' } + markupPosition: { start: [4, 5], text: "'Y'" } + position: { start: [4, 0], text: echo 'Y' } + markup: + type: LiquidVariable + rawSource: "'Y'" + position: { start: [4, 5], text: "'Y'" } + expression: + type: String + single: true + value: Y + position: { start: [4, 5], text: "'Y'" } + filters: [] diff --git a/specs/parser/liquid-raw.yml b/specs/parser/liquid-raw.yml index 31febb4d9..ce2b72584 100644 --- a/specs/parser/liquid-raw.yml +++ b/specs/parser/liquid-raw.yml @@ -1725,46 +1725,46 @@ cases: value: ok position: { start: [0, 56], text: ok } - name: does not duplicate a straddling {{ into both a Liquid-in-body raw body and a sibling - source: "{% style %}{{ invalid {% endstyle %}{{ 1 }}" + source: "{% stylesheet %}{{ invalid {% endstylesheet %}{{ 1 }}" parse: [html] ast: type: Document name: "#document" - position: { start: [0, 0], text: "{% style %}{{ invalid {% endstyle %}{{ 1 }}" } + position: { start: [0, 0], text: "{% stylesheet %}{{ invalid {% endstylesheet %}{{ 1 }}" } children: - type: LiquidRawTag - name: style + name: stylesheet markup: "" - markupPosition: { start: [0, 9], text: "" } + markupPosition: { start: [0, 14], text: "" } whitespaceStart: "" whitespaceEnd: "" delimiterWhitespaceStart: "" delimiterWhitespaceEnd: "" - blockStartPosition: { start: [0, 0], text: "{% style %}" } - blockEndPosition: { start: [0, 22], text: "{% endstyle %}" } - position: { start: [0, 0], text: "{% style %}{{ invalid {% endstyle %}" } + blockStartPosition: { start: [0, 0], text: "{% stylesheet %}" } + blockEndPosition: { start: [0, 27], text: "{% endstylesheet %}" } + position: { start: [0, 0], text: "{% stylesheet %}{{ invalid {% endstylesheet %}" } body: type: RawMarkup kind: text value: "{{ invalid " - position: { start: [0, 11], text: "{{ invalid " } + position: { start: [0, 16], text: "{{ invalid " } nodes: - type: TextNode value: "{{ invalid" - position: { start: [0, 11], text: "{{ invalid" } + position: { start: [0, 16], text: "{{ invalid" } - type: LiquidVariableOutput whitespaceStart: "" whitespaceEnd: "" - markupPosition: { start: [0, 38], text: " 1 " } - position: { start: [0, 36], text: "{{ 1 }}" } + markupPosition: { start: [0, 48], text: " 1 " } + position: { start: [0, 46], text: "{{ 1 }}" } markup: type: LiquidVariable rawSource: "1" - position: { start: [0, 39], text: "1 " } + position: { start: [0, 49], text: "1 " } expression: type: Number value: "1" - position: { start: [0, 39], text: "1" } + position: { start: [0, 49], text: "1" } filters: [] - name: does not let a decoy endcommentXXXXX close the comment; only the real endcomment does source: |- @@ -1952,3 +1952,925 @@ cases: - type: TextNode value: < position: { start: [0, 34], text: < } + - name: closes a raw tag on an end tag with extra strings, as Liquid::Raw does + source: "{% raw %}x{% endraw xyz %}Y" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% raw %}x{% endraw xyz %}Y" } + children: + - type: LiquidRawTag + name: raw + markup: "" + markupPosition: { start: [0, 7], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% raw %}" } + blockEndPosition: { start: [0, 10], text: "{% endraw xyz %}" } + position: { start: [0, 0], text: "{% raw %}x{% endraw xyz %}" } + body: + type: RawMarkup + kind: text + value: x + position: { start: [0, 9], text: x } + nodes: + - type: TextNode + value: x + position: { start: [0, 9], text: x } + - type: TextNode + value: Y + position: { start: [0, 26], text: Y } + - name: keeps text before the end tag in the same token in a raw body + source: "{% raw %}x{{ a {% endraw %}Y" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% raw %}x{{ a {% endraw %}Y" } + children: + - type: LiquidRawTag + name: raw + markup: "" + markupPosition: { start: [0, 7], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% raw %}" } + blockEndPosition: { start: [0, 15], text: "{% endraw %}" } + position: { start: [0, 0], text: "{% raw %}x{{ a {% endraw %}" } + body: + type: RawMarkup + kind: text + value: "x{{ a " + position: { start: [0, 9], text: "x{{ a " } + nodes: + - type: TextNode + value: x{{ a + position: { start: [0, 9], text: "x{{ a" } + - type: TextNode + value: Y + position: { start: [0, 27], text: Y } + - name: does not nest a raw tag inside a raw tag + source: "{% raw %}{% raw %}x{% endraw %}Y{% endraw %}" + parse: [html] + error: + message: Attempting to close LiquidTag 'raw' before it was opened without a matching 'raw' + start: [0, 32] + text: "{% endraw %" + - name: rejects a doc tag nested in a doc tag, as Liquid::Doc does + source: "{% doc %}{% doc %}{% enddoc %}{% enddoc %}" + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'doc' was closed + start: [0, 0] + text: "{% doc %}" + - name: nests a style tag and skips raw bodies inside it, as Liquid::Block does + source: "{% style %}a{% style %}b{% endstyle %}{% comment %}{% endstyle %}{% endcomment %}c{% endstyle %}Y" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% style %}a{% style %}b{% endstyle %}{% comment %}{% endstyle %}{% endcomment %}c{% endstyle %}Y" } + children: + - type: LiquidRawTag + name: style + markup: "" + markupPosition: { start: [0, 9], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% style %}" } + blockEndPosition: { start: [0, 82], text: "{% endstyle %}" } + position: { start: [0, 0], text: "{% style %}a{% style %}b{% endstyle %}{% comment %}{% endstyle %}{% endcomment %}c{% endstyle %}" } + body: + type: RawMarkup + kind: text + value: a{% style %}b{% endstyle %}{% comment %}{% endstyle %}{% endcomment %}c + position: { start: [0, 11], text: "a{% style %}b{% endstyle %}{% comment %}{% endstyle %}{% endcomment %}c" } + nodes: + - type: TextNode + value: a + position: { start: [0, 11], text: a } + - type: LiquidRawTag + name: style + markup: "" + markupPosition: { start: [0, 21], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 12], text: "{% style %}" } + blockEndPosition: { start: [0, 24], text: "{% endstyle %}" } + position: { start: [0, 12], text: "{% style %}b{% endstyle %}" } + body: + type: RawMarkup + kind: css + value: b + position: { start: [0, 23], text: b } + nodes: + - type: TextNode + value: b + position: { start: [0, 23], text: b } + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 49], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 38], text: "{% comment %}" } + blockEndPosition: { start: [0, 65], text: "{% endcomment %}" } + position: { start: [0, 38], text: "{% comment %}{% endstyle %}{% endcomment %}" } + body: + type: RawMarkup + kind: text + value: "{% endstyle %}" + position: { start: [0, 51], text: "{% endstyle %}" } + nodes: + - type: TextNode + value: "{% endstyle %}" + position: { start: [0, 51], text: "{% endstyle %}" } + - type: TextNode + value: c + position: { start: [0, 81], text: c } + - type: TextNode + value: Y + position: { start: [0, 96], text: Y } + - name: does not parse nodes inside a comment + source: |- + {% comment %} + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [9, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + position: + start: [0, 13] + text: |2 + + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + nodes: + - type: TextNode + value: |- + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + position: + start: [1, 2] + text: |- + {% if true %} + {% if ... %} + {%- for ? -%} + {% while true %} + {% + unless if + %} + {% endcase %} + - name: allows unclosed tags in a comment + source: |- + {% comment %} + {% if true %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% if true %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [2, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% if true %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% if true %} + position: + start: [0, 13] + text: |2 + + {% if true %} + nodes: + - type: TextNode + value: "{% if true %}" + position: { start: [1, 2], text: "{% if true %}" } + - name: allows an open tag that runs into a nested comment + source: |- + {% comment %} + {% assign a = 123 {% comment %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% assign a = 123 {% comment %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [2, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% assign a = 123 {% comment %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% assign a = 123 {% comment %} + position: + start: [0, 13] + text: |2 + + {% assign a = 123 {% comment %} + nodes: + - type: TextNode + value: "{% assign a = 123 {% comment %}" + position: { start: [1, 2], text: "{% assign a = 123 {% comment %}" } + - name: rejects an open tag that runs into the endcomment + source: |- + {% comment %} + {% assign foo = "1" + {% endcomment %} + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'comment' was closed + start: [0, 0] + text: "{% comment %}" + - name: rejects an open tag that runs into a nested endcomment + source: |- + {% comment %} + {% comment %} + {% invalid + {% endcomment %} + {% endcomment %} + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'comment' was closed + start: [0, 0] + text: "{% comment %}" + - name: rejects an open variable that runs into the endcomment + source: |- + {% comment %} + {% {{ {%- endcomment %} + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'comment' was closed + start: [0, 0] + text: "{% comment %}" + - name: closes child comment tags + source: |- + {% comment %} + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [4, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + position: + start: [0, 13] + text: |2 + + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + nodes: + - type: TextNode + value: |- + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + position: + start: [1, 2] + text: |- + {% comment %} + {% comment %}{% endcomment %} + {% endcomment %} + - name: rejects an unclosed child comment tag + source: |- + {% comment %} + {% comment %} + {% comment %} + {% endcomment %} + {% endcomment %} + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'comment' was closed + start: [0, 0] + text: "{% comment %}" + - name: skips a closed child raw tag in a comment + source: |- + {% comment %} + {% raw %} + {% endcomment %} + {% endraw %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% raw %} + {% endcomment %} + {% endraw %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [4, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% raw %} + {% endcomment %} + {% endraw %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% raw %} + {% endcomment %} + {% endraw %} + position: + start: [0, 13] + text: |2 + + {% raw %} + {% endcomment %} + {% endraw %} + nodes: + - type: TextNode + value: |- + {% raw %} + {% endcomment %} + {% endraw %} + position: + start: [1, 2] + text: |- + {% raw %} + {% endcomment %} + {% endraw %} + - name: rejects an unclosed child raw tag in a comment + source: |- + {% comment %} + {% raw %} + {% endcomment %} + {% endcomment %} + parse: [html] + error: + message: Attempting to end parsing before LiquidRawTag 'comment' was closed + start: [0, 0] + text: "{% comment %}" + - name: closes a nested comment on an endcomment with extra strings + source: |- + {% comment %} + {% comment %} + {% endcomment + {% if true %} + {% endif %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% comment %} + {% endcomment + {% if true %} + {% endif %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [5, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% comment %} + {% endcomment + {% if true %} + {% endif %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% comment %} + {% endcomment + {% if true %} + {% endif %} + position: + start: [0, 13] + text: |2 + + {% comment %} + {% endcomment + {% if true %} + {% endif %} + nodes: + - type: TextNode + value: |- + {% comment %} + {% endcomment + {% if true %} + {% endif %} + position: + start: [1, 2] + text: |- + {% comment %} + {% endcomment + {% if true %} + {% endif %} + - name: opens and closes a nested comment with extra strings + source: |- + {% comment %} + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + {% endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %} + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + {% endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [5, 0], text: "{% endcomment %}" } + position: + start: [0, 0] + text: |- + {% comment %} + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + {% endcomment %} + body: + type: RawMarkup + kind: text + value: |2 + + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + position: + start: [0, 13] + text: |2 + + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + nodes: + - type: TextNode + value: |- + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + position: + start: [1, 2] + text: |- + {% comment + {% assign foo = 1 %} + {% endcomment + {% assign foo = 1 %} + - name: closes a comment on an endcomment followed by a space and extra strings + source: "{% comment %}123{% endcomment xyz %}" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %}123{% endcomment xyz %}" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [0, 16], text: "{% endcomment xyz %}" } + position: { start: [0, 0], text: "{% comment %}123{% endcomment xyz %}" } + body: + type: RawMarkup + kind: text + value: "123" + position: { start: [0, 13], text: "123" } + nodes: + - type: TextNode + value: "123" + position: { start: [0, 13], text: "123" } + - name: closes a comment on an endcomment followed by a tab and extra strings + source: "{% comment %}123{% endcomment\txyz %}" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %}123{% endcomment\txyz %}" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [0, 16], text: "{% endcomment\txyz %}" } + position: { start: [0, 0], text: "{% comment %}123{% endcomment\txyz %}" } + body: + type: RawMarkup + kind: text + value: "123" + position: { start: [0, 13], text: "123" } + nodes: + - type: TextNode + value: "123" + position: { start: [0, 13], text: "123" } + - name: closes a comment on an endcomment followed by a newline and extra strings + source: |- + {% comment %}123{% endcomment + xyz %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %}123{% endcomment + xyz %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: + start: [0, 16] + text: |- + {% endcomment + xyz %} + position: + start: [0, 0] + text: |- + {% comment %}123{% endcomment + xyz %} + body: + type: RawMarkup + kind: text + value: "123" + position: { start: [0, 13], text: "123" } + nodes: + - type: TextNode + value: "123" + position: { start: [0, 13], text: "123" } + - name: closes a comment on an endcomment whose extra strings repeat endcomment + source: |- + {% comment %}123{% endcomment + xyz endcomment %} + parse: [html] + ast: + type: Document + name: "#document" + position: + start: [0, 0] + text: |- + {% comment %}123{% endcomment + xyz endcomment %} + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: + start: [0, 16] + text: |- + {% endcomment + xyz endcomment %} + position: + start: [0, 0] + text: |- + {% comment %}123{% endcomment + xyz endcomment %} + body: + type: RawMarkup + kind: text + value: "123" + position: { start: [0, 13], text: "123" } + nodes: + - type: TextNode + value: "123" + position: { start: [0, 13], text: "123" } + - name: keeps whitespace control on a comment + source: "{%- comment -%}123{%- endcomment -%}" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{%- comment -%}123{%- endcomment -%}" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 12], text: "" } + whitespaceStart: "-" + whitespaceEnd: "-" + delimiterWhitespaceStart: "-" + delimiterWhitespaceEnd: "-" + blockStartPosition: { start: [0, 0], text: "{%- comment -%}" } + blockEndPosition: { start: [0, 18], text: "{%- endcomment -%}" } + position: { start: [0, 0], text: "{%- comment -%}123{%- endcomment -%}" } + body: + type: RawMarkup + kind: text + value: "123" + position: { start: [0, 15], text: "123" } + nodes: + - type: TextNode + value: "123" + position: { start: [0, 15], text: "123" } + - name: keeps the outer whitespace control on a nested comment + source: "{% comment %}{%- comment -%}inner{%- endcomment -%}{% endcomment -%}" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %}{%- comment -%}inner{%- endcomment -%}{% endcomment -%}" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "-" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [0, 51], text: "{% endcomment -%}" } + position: { start: [0, 0], text: "{% comment %}{%- comment -%}inner{%- endcomment -%}{% endcomment -%}" } + body: + type: RawMarkup + kind: text + value: "{%- comment -%}inner{%- endcomment -%}" + position: { start: [0, 13], text: "{%- comment -%}inner{%- endcomment -%}" } + nodes: + - type: TextNode + value: "{%- comment -%}inner{%- endcomment -%}" + position: { start: [0, 13], text: "{%- comment -%}inner{%- endcomment -%}" } + - name: parses the markup that follows a nested comment + source: "{% comment %}a{% comment %}b{% endcomment %}c{% endcomment %}{{ x }}" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %}a{% comment %}b{% endcomment %}c{% endcomment %}{{ x }}" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [0, 45], text: "{% endcomment %}" } + position: { start: [0, 0], text: "{% comment %}a{% comment %}b{% endcomment %}c{% endcomment %}" } + body: + type: RawMarkup + kind: text + value: a{% comment %}b{% endcomment %}c + position: { start: [0, 13], text: "a{% comment %}b{% endcomment %}c" } + nodes: + - type: TextNode + value: a{% comment %}b{% endcomment %}c + position: { start: [0, 13], text: "a{% comment %}b{% endcomment %}c" } + - type: HtmlElement + blockStartPosition: { start: [0, 61], text: } + blockEndPosition: { start: [0, 75], text: } + position: { start: [0, 61], text: "{{ x }}" } + name: + - type: TextNode + value: title + position: { start: [0, 62], text: title } + attributes: [] + children: + - type: LiquidVariableOutput + whitespaceStart: "" + whitespaceEnd: "" + markupPosition: { start: [0, 70], text: " x " } + position: { start: [0, 68], text: "{{ x }}" } + markup: + type: LiquidVariable + rawSource: x + position: { start: [0, 71], text: "x " } + expression: + type: VariableLookup + name: x + position: { start: [0, 71], text: x } + lookups: [] + lookupModes: [] + filters: [] From 81bf98bf06ce8ba096359f4bb85398704182a0e4 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Fri, 30 Oct 2026 11:13:27 -0400 Subject: [PATCH 09/20] Complete block arguments through the BlockArgument type Block and section arguments are now one BlockArgument type, so the completion params narrow on the argument's value rather than on the argument. Co-Authored-By: Claude Opus 5.5 --- .../completions/params/LiquidCompletionParams.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts b/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts index 2b6b9372e..e0bcc5f3c 100644 --- a/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts +++ b/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts @@ -1,7 +1,9 @@ import { + BlockArgument, BlockMarkup, findErrorNodeAtOffset, LiquidHtmlNode, + LiquidNamedArgument, LiquidTag, MarkupToken, MarkupTokenType, @@ -746,7 +748,7 @@ function findCompletionNode( case NodeTypes.SectionMarkup: { if (isNotEmpty(current.args)) { const arg = last(current.args); - finder.current = isBlockArrayArgument(arg) ? arg.value.elements.at(-1) : arg; + finder.current = blockArgumentNode(arg); } else if (isCovered(cursor, current.name.position)) { finder.current = current.name; } @@ -1835,7 +1837,7 @@ function blockMarkupChild( } if (coveredArg) { - return isBlockArrayArgument(coveredArg) ? coveredArg.value.elements.at(-1) : coveredArg; + return blockArgumentNode(coveredArg); } if (isCovered(cursor, markup.name.position)) return markup.name; @@ -2401,8 +2403,12 @@ function isDefined(x: T | undefined): x is T { return x !== undefined; } -function isBlockArrayArgument(arg: any): arg is { value: { elements: LiquidHtmlNode[] } } { - return arg.value?.type === 'BlockArrayLiteral'; +// The node to complete in a block argument: its last array element, or the argument itself, +// which is a LiquidNamedArgument once its value isn't an array literal. +function blockArgumentNode(arg: BlockArgument): LiquidHtmlNode | undefined { + return arg.value.type === 'BlockArrayLiteral' + ? arg.value.elements.at(-1) + : (arg as LiquidNamedArgument); } function last(x: NonEmptyArray): T { From ca054cfec3a2f756fe562140c520e352012547e5 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Fri, 30 Oct 2026 11:15:25 -0400 Subject: [PATCH 10/20] Expect raw tag syntax errors where the parser now reports them The parser now closes {% raw %} and {% doc %} on an end tag with extra strings, as Liquid does, so a later unclosed tag is the one reported. It also rejects a comment whose body Liquid's tokenizer runs past the endcomment, so LiquidSyntaxError reports the parser's error at the comment's open tag. Co-Authored-By: Claude Opus 5.5 --- specs/checks/liquid-syntax-error.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/specs/checks/liquid-syntax-error.yml b/specs/checks/liquid-syntax-error.yml index 7685f9c3a..0257d29b7 100644 --- a/specs/checks/liquid-syntax-error.yml +++ b/specs/checks/liquid-syntax-error.yml @@ -1037,8 +1037,8 @@ cases: offenses: - file: snippets/test.liquid message: "Attempting to end parsing before LiquidRawTag 'doc' was closed" - start: [0, 0] - highlight: "{% doc %}" + start: [0, 32] + highlight: "{% doc %" - name: accepts later balanced doc blocks after accepted doc closing markup files: snippets/test.liquid: "{% doc %}content{% enddoc foo %}{% doc %}later{% enddoc %}" @@ -1090,8 +1090,8 @@ cases: offenses: - file: snippets/test.liquid message: "Attempting to end parsing before LiquidRawTag 'raw' was closed" - start: [0, 0] - highlight: "{% raw %}" + start: [0, 26] + highlight: "{% raw %" - name: accepts later balanced raw blocks after accepted raw closing markup files: snippets/test.liquid: "{% raw %}c{% endraw foo %}{% raw %}later{% endraw %}" @@ -1278,17 +1278,17 @@ cases: snippets/test.liquid: "{% comment %}this is {{{ not }}} valid{{ liquid{% endcomment %}" offenses: - file: snippets/test.liquid - message: "Liquid syntax error: 'comment' tag was never closed" - start: [0, 13] - highlight: this is {{{ not }}} valid{{ liquid + message: "Attempting to end parsing before LiquidRawTag 'comment' was closed" + start: [0, 0] + highlight: "{% comment %}" - name: "reports LiquidSyntaxError for Ruby-stricter comment raw bodies: {% comment %}this is {% if liquid{% endcomment %}" files: snippets/test.liquid: "{% comment %}this is {% if liquid{% endcomment %}" offenses: - file: snippets/test.liquid - message: "Liquid syntax error: 'comment' tag was never closed" - start: [0, 13] - highlight: this is {% if liquid + message: "Attempting to end parsing before LiquidRawTag 'comment' was closed" + start: [0, 0] + highlight: "{% comment %}" - name: produces no diagnostics for inert closed Liquid tags inside comment bodies files: snippets/test.liquid: "{% comment %}{% if %}{% endcomment %}" From 3b427704d12835e65a31efb5d4014cb2062ad680 Mon Sep 17 00:00:00 2001 From: River Date: Wed, 30 Sep 2026 16:31:13 +0000 Subject: [PATCH 11/20] Add a pairDanglingCloseTags option for branch-opened elements Themes often open an element in each branch of a conditional and close it once after the conditional: {% if a %}
{% else %}
{% endif %} {{ content_for_layout }}
Liquid writes this as text, so Shopify renders it, but the parser matched the close tag against the enclosing element and threw a few tags later. With the new pairDanglingCloseTags option, off by default, such a close tag becomes an HtmlDanglingMarkerClose when an earlier if, unless or case in the same scope leaves an element of that name open on every path. Co-Authored-By: Claude Opus 5.5 --- .../liquid-html-parser/specs/architecture.md | 2 + packages/liquid-html-parser/src/ast.ts | 14 ++ .../document/branch-opened-elements.test.ts | 178 ++++++++++++++++++ .../src/document/branch-opened-elements.ts | 76 ++++++++ .../liquid-html-parser/src/document/html.ts | 27 +++ .../src/document/liquid-blocks.ts | 5 +- .../liquid-html-parser/src/document/parser.ts | 22 ++- 7 files changed, 321 insertions(+), 3 deletions(-) create mode 100644 packages/liquid-html-parser/src/document/branch-opened-elements.test.ts create mode 100644 packages/liquid-html-parser/src/document/branch-opened-elements.ts diff --git a/packages/liquid-html-parser/specs/architecture.md b/packages/liquid-html-parser/specs/architecture.md index f91427f56..88a44245b 100644 --- a/packages/liquid-html-parser/specs/architecture.md +++ b/packages/liquid-html-parser/specs/architecture.md @@ -594,6 +594,8 @@ All except `AttrEmpty` extend `AttributeNodeBase` with `name: (TextNode | Liq Defined in frozen `ast.ts`. Produced when a `` close tag does not match any open element AND the current context has `allowUnclosedHtml: true` (i.e., inside `if`/`unless`/`case` branches). Has `name: (TextNode | LiquidVariableOutput)[]` and `blockStartPosition: Position`. +With the `pairDanglingCloseTags` option, it is also produced for a close tag that ends an element an earlier `if`/`unless`/`case` in the same scope left open on every path (`document/branch-opened-elements.ts`). That case is checked before the close tag is matched against the enclosing element. + --- ## 5. Decisions Log diff --git a/packages/liquid-html-parser/src/ast.ts b/packages/liquid-html-parser/src/ast.ts index c0451ea17..211d08473 100644 --- a/packages/liquid-html-parser/src/ast.ts +++ b/packages/liquid-html-parser/src/ast.ts @@ -938,6 +938,19 @@ export interface ASTBuildOptions { /** Optional environment for tag registration. Uses Environment.default() if omitted. */ environment?: Environment; + + /** + * Parse a close tag as an `HtmlDanglingMarkerClose` when it ends an element + * that an earlier `if` / `unless` / `case` in the same scope left open on + * every branch, instead of matching it against the enclosing element: + * + * {% if a %}
{% else %}
{% endif %}...
+ * + * Without it such a template throws. Liquid treats the markup as text, so + * Shopify renders it, and a renderer can pair the marker with the open + * element. Off by default: it accepts templates the strict grammar rejects. + */ + pairDanglingCloseTags?: boolean; } export function isBranchedTag(node: LiquidHtmlNode) { @@ -967,6 +980,7 @@ export function toLiquidHtmlAST( const env = options.environment ?? Environment.default(); const tokens = tokenize(source); const parser = new DocumentParser(tokens, source, env, true, options.allowUnclosedDocumentNode); + parser.pairDanglingCloseTags = options.pairDanglingCloseTags ?? false; return parser.parseDocument(); } diff --git a/packages/liquid-html-parser/src/document/branch-opened-elements.test.ts b/packages/liquid-html-parser/src/document/branch-opened-elements.test.ts new file mode 100644 index 000000000..30cef99f1 --- /dev/null +++ b/packages/liquid-html-parser/src/document/branch-opened-elements.test.ts @@ -0,0 +1,178 @@ +import { describe, it, expect } from 'vitest'; +import { toLiquidHtmlAST } from '../ast'; +import type { DocumentNode, LiquidHtmlNode } from '../ast'; +import { NodeTypes } from '../types'; + +const PAIRING = { + mode: 'strict', + allowUnclosedDocumentNode: false, + pairDanglingCloseTags: true, +} as const; +const STRICT = { mode: 'strict', allowUnclosedDocumentNode: false } as const; + +/** One line per node: its type, its plain name, and whether it is left open. */ +function outline(nodes: LiquidHtmlNode[], depth = 0): string[] { + const lines: string[] = []; + for (const node of nodes) { + if (node.type === NodeTypes.TextNode) continue; + const pad = ' '.repeat(depth); + if (node.type === NodeTypes.HtmlElement) { + const name = node.name.map((s) => ('value' in s ? s.value : '?')).join(''); + const open = node.blockEndPosition.start === node.blockEndPosition.end ? ' (open)' : ''; + lines.push(`${pad}<${name}>${open}`); + lines.push(...outline(node.children, depth + 1)); + } else if (node.type === NodeTypes.HtmlDanglingMarkerClose) { + const name = node.name.map((s) => ('value' in s ? s.value : '?')).join(''); + lines.push(`${pad} (dangling)`); + } else if (node.type === NodeTypes.LiquidTag) { + lines.push(`${pad}{% ${node.name} %}`); + if (Array.isArray(node.children)) lines.push(...outline(node.children, depth + 1)); + } else if (node.type === NodeTypes.LiquidBranch) { + lines.push(`${pad}branch ${node.name ?? '(first)'}`); + lines.push(...outline(node.children, depth + 1)); + } else if (node.type === NodeTypes.LiquidVariableOutput) { + lines.push(`${pad}{{ }}`); + } + } + return lines; +} + +function parseOutline(source: string): string[] { + const ast: DocumentNode = toLiquidHtmlAST(source, PAIRING); + return outline(ast.children); +} + +describe('pairDanglingCloseTags', () => { + const LAYOUT = [ + '', + '
', + '{% if a %}', + '
', + '{% elsif b %}', + '
', + '{% else %}', + '
', + '{% endif %}', + '{{ content_for_layout }}', + '
', + '
', + '', + '', + ].join('\n'); + + it('throws on an element opened in every branch and closed after endif, without the option', () => { + expect(() => toLiquidHtmlAST(LAYOUT, STRICT)).toThrow( + "Attempting to close HtmlElement 'div' before HtmlElement 'body' was closed", + ); + }); + + it('parses the close as a dangling marker inside the enclosing element', () => { + expect(parseOutline(LAYOUT)).toEqual([ + '', + '
', + ' {% if %}', + ' branch (first)', + '
(open)', + ' branch elsif', + '
(open)', + ' branch else', + '
(open)', + ' {{ }}', + '
(dangling)', + '
', + ]); + }); + + it('parses a close at the document level the same way', () => { + const source = '{% if a %}
{% else %}
{% endif %}hi
'; + expect(() => toLiquidHtmlAST(source, STRICT)).toThrow( + "Attempting to close HtmlElement 'div' before it was opened", + ); + expect(parseOutline(source)).toEqual([ + '{% if %}', + ' branch (first)', + '
(open)', + ' branch else', + '
(open)', + '
(dangling)', + ]); + }); + + it('parses a close in a loop body the same way', () => { + const source = + '
    {% for i in (1..2) %}{% if s %}
    {% else %}
    {% endif %}{{ i }}
    {% endfor %}
'; + expect(() => toLiquidHtmlAST(source, STRICT)).toThrow(); + expect(parseOutline(source)).toEqual([ + '
+ position: { start: [0, 46], text: } + - type: HtmlElement + blockStartPosition: { start: [0, 61], text:

} + blockEndPosition: { start: [0, 69], text:

} + position: { start: [0, 61], text:

after

} + name: + - type: TextNode + value: p + position: { start: [0, 62], text: p } + attributes: [] + children: + - type: TextNode + value: after + position: { start: [0, 64], text: after } + - name: parses an HTML comment after a style whose body holds an unpaired /* b {}

after

+ parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "

after

" } + children: + - type: HtmlRawNode + name: style + blockStartPosition: { start: [0, 0], text: } + position: { start: [0, 0], text: "" } + attributes: [] + body: + type: RawMarkup + kind: css + value: /* b {} + position: { start: [0, 7], text: "/* b {}" } + nodes: + - type: TextNode + value: /* b {} + position: { start: [0, 7], text: "/* b {}" } + - type: HtmlComment + body:
+ position: { start: [0, 39], text: } + - type: HtmlElement + blockStartPosition: { start: [0, 54], text:

} + blockEndPosition: { start: [0, 62], text:

} + position: { start: [0, 54], text:

after

} + name: + - type: TextNode + value: p + position: { start: [0, 55], text: p } + attributes: [] + children: + - type: TextNode + value: after + position: { start: [0, 57], text: after } + - name: closes an svg whose open tag has a glued Liquid attribute + source: + parse: [liquid] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: '' } + children: + - type: TextNode + value: + position: { start: [0, 23], text: --> } + - name: closes a nested svg at its outer when CDATA after the inner one holds a

after

+ parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text:

after

} + children: + - type: HtmlRawNode + name: svg + blockStartPosition: { start: [0, 0], text: } + blockEndPosition: { start: [0, 16], text: } + position: { start: [0, 0], text: } + attributes: [] + body: + type: RawMarkup + kind: text + value: + position: { start: [0, 5], text: } + nodes: + - type: TextNode + value: + position: { start: [0, 5], text: } + - type: HtmlComment + body:
+ position: { start: [0, 22], text: } + - type: HtmlElement + blockStartPosition: { start: [0, 37], text:

} + blockEndPosition: { start: [0, 45], text:

} + position: { start: [0, 37], text:

after

} + name: + - type: TextNode + value: p + position: { start: [0, 38], text: p } + attributes: [] + children: + - type: TextNode + value: after + position: { start: [0, 40], text: after } diff --git a/specs/parser/liquid-raw.yml b/specs/parser/liquid-raw.yml index e2d29988f..33acdff5c 100644 --- a/specs/parser/liquid-raw.yml +++ b/specs/parser/liquid-raw.yml @@ -2858,3 +2858,545 @@ cases: lookups: [] lookupModes: [] filters: [] + - name: parses the markup after a comment whose body opens a tag + source: "{% comment %} ax
" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %} ax
" } + children: + - type: LiquidRawTag + name: comment + markup: "" + markupPosition: { start: [0, 11], text: "" } + whitespaceStart: "" + whitespaceEnd: "" + delimiterWhitespaceStart: "" + delimiterWhitespaceEnd: "" + blockStartPosition: { start: [0, 0], text: "{% comment %}" } + blockEndPosition: { start: [0, 18], text: "{% endcomment %}" } + position: { start: [0, 0], text: "{% comment %} a } + blockEndPosition: { start: [0, 40], text:
} + position: { start: [0, 34], text:
x
} + name: + - type: TextNode + value: div + position: { start: [0, 35], text: div } + attributes: [] + children: + - type: TextNode + value: x + position: { start: [0, 39], text: x } + - name: parses the markup after a comment whose body opens a quoted attribute + source: '{% comment %}
x
" + parse: [html] + ast: + type: Document + name: "#document" + position: { start: [0, 0], text: "{% comment %} as text, so the Liquid inside an HTML comment renders, and its tags still open, branch and close blocks. The parser read a comment's body as opaque, so {% if a %} failed with an unclosed if, and checks never saw the Liquid in a comment. HtmlComment now has children (text and Liquid, never markup) and blockStartPosition/blockEndPosition for its delimiters: - A comment that crosses a Liquid block boundary is split there, and the part on the split side has an empty delimiter range. - A comment whose --> sits in a block opened inside it ends with that block. - A comment that never closes ends at the end of the source, as in a browser. - An end tag in a comment that matches no open block fails, as in Liquid. - opaqueHtmlComments keeps comment bodies opaque, for HTML that is data rather than a template. Consumers: - Prettier prints a split or unclosed comment as written, trimming the whitespace on its delimiter-less side so formatting stays idempotent. - Completions descend into a comment's children. - After an error inside a comment, tolerant recovery resumes at the comment's --> instead of skipping it, so the markup after it stays HTML. Co-Authored-By: River Co-Authored-By: Martin Morissette Co-Authored-By: Mbarak Bujra Co-Authored-By: Claude Opus 5.5 --- .../liquid-html-parser/specs/architecture.md | 2 +- .../specs/recursive-descent.md | 6 +- packages/liquid-html-parser/src/ast.test.ts | 22 + packages/liquid-html-parser/src/ast.ts | 38 +- .../src/document/factories.ts | 13 +- .../liquid-html-parser/src/document/html.ts | 90 ++- .../src/document/node-dispatch.ts | 18 +- .../liquid-html-parser/src/document/parser.ts | 24 + .../src/document/tolerant-parser.ts | 10 +- .../liquid-html-parser/src/tolerant.test.ts | 13 + packages/liquid-html-parser/src/tolerant.ts | 1 + .../src/printer/printer-liquid-html.ts | 14 +- .../src/test/html-comment/fixed.liquid | 15 + .../src/test/html-comment/index.liquid | 9 + .../params/LiquidCompletionParams.ts | 6 +- .../ObjectCompletionProvider.spec.ts | 7 + .../src/css/CSSLanguageService.ts | 5 +- specs/checks/liquid-html-syntax-error.yml | 13 + specs/checks/undefined-object.yml | 9 + specs/parser/html.yml | 612 ++++++++++++++++++ 20 files changed, 894 insertions(+), 33 deletions(-) diff --git a/packages/liquid-html-parser/specs/architecture.md b/packages/liquid-html-parser/specs/architecture.md index 2a956d3fa..b68d85af6 100644 --- a/packages/liquid-html-parser/specs/architecture.md +++ b/packages/liquid-html-parser/specs/architecture.md @@ -432,7 +432,7 @@ function makeHtmlRawNode( blockEndPosition, source, ): HtmlRawNode; -function makeHtmlComment(body, start, end, source): HtmlComment; +function makeHtmlComment(children, blockStartPosition, blockEndPosition, source): HtmlComment; function makeHtmlDoctype(legacyDoctypeString, start, end, source): HtmlDoctype; function makeAttrDoubleQuoted(name, value, attributePosition, position, source): AttrDoubleQuoted; function makeAttrSingleQuoted(name, value, attributePosition, position, source): AttrSingleQuoted; diff --git a/packages/liquid-html-parser/specs/recursive-descent.md b/packages/liquid-html-parser/specs/recursive-descent.md index de3e0537d..e83e3c066 100644 --- a/packages/liquid-html-parser/specs/recursive-descent.md +++ b/packages/liquid-html-parser/specs/recursive-descent.md @@ -78,7 +78,7 @@ LiquidHtmlNode (discriminated union of all traversable nodes) ├── TextNode { value: string } ├── YAMLFrontmatter { body: string } ├── HtmlDoctype { legacyDoctypeString: string | null } - ├── HtmlComment { body: string } + ├── HtmlComment { body: string, children: (LiquidNode|TextNode)[] } │ ├── LiquidVariableOutput { markup: string | LiquidVariable, whitespace } │ @@ -307,9 +307,11 @@ htmlVoid := '<' voidTagName attributes '>' htmlSelfClosing := '<' tagName attributes '/>' htmlRawNode := '<' rawTagName attributes '>' rawBody '' htmlDoctype := '' -htmlComment := '' | '--!>') | '' | '' +htmlComment := '' | '--!>') | '' | '' ``` +**HTML comments hold Liquid.** Liquid renders the Liquid inside ``, so a comment's body is parsed into `children` as text and Liquid, never markup. A comment that crosses a Liquid block boundary is split there: one `HtmlComment` ends before the tag that ends or branches the enclosing block, and the next starts after it, with an empty `blockEndPosition` or `blockStartPosition` on the split side. When a block opened inside a comment holds the `-->`, the comment ends with that block. A comment that never closes ends at the end of the source, as in a browser. An end or branch tag inside a comment that matches no open block fails, as in Liquid. `opaqueHtmlComments` keeps comment bodies opaque, for HTML that is data rather than a template. + **Tag names can be compound** — they may contain Liquid variable outputs: `<{{ header_type }}--header>`. **Void elements:** `area`, `base`, `br`, `col`, `command`, `embed`, `hr`, `img`, `input`, `keygen`, `link`, `meta`, `param`, `source`, `track`, `wbr` (case-insensitive). diff --git a/packages/liquid-html-parser/src/ast.test.ts b/packages/liquid-html-parser/src/ast.test.ts index 1aac72c37..32452e0f6 100644 --- a/packages/liquid-html-parser/src/ast.test.ts +++ b/packages/liquid-html-parser/src/ast.test.ts @@ -65,6 +65,28 @@ describe('Unit: Stage 2 (AST)', () => { }); }); + describe('toLiquidHtmlAST(source, { opaqueHtmlComments: true })', () => { + const options = { + mode: 'strict', + allowUnclosedDocumentNode: false, + opaqueHtmlComments: true, + } as const; + const expectPath = makeExpectPath('opaqueHtmlComments'); + + it('should keep an HTML comment body opaque', () => { + ast = toLiquidHtmlAST('

a

', options); + expectPath(ast, 'children.1.type').to.eql('HtmlComment'); + expectPath(ast, 'children.1.children').to.eql([]); + expectPath(ast, 'children.1.body').to.eql('{% endif %} {% raw %} {{ b }}'); + }); + + it('should reject an unclosed HTML comment', () => { + expect(() => toLiquidHtmlAST('` */ +/** + * Represents ``. + * + * Liquid treats `` as plain text, so the Liquid inside a comment + * still renders and can close a block opened before the comment. + * + * This node describes the source, not the rendered page. The parser knows where + * each delimiter is written, not which `-->` closes which `` sits inside a block it opened ends with + * that block. Liquid outputs every delimiter as written, so only the rendered + * output shows which comments are open. + */ export interface HtmlComment extends ASTNode { + /** The source between the delimiters, trimmed. */ body: string; + /** The text and Liquid between the delimiters. */ + children: (LiquidNode | TextNode)[]; + /** The range covered by this node's ``, empty when the node ends at a block boundary. */ + blockEndPosition: Position; } export interface HtmlNodeBase extends ASTNode { @@ -953,6 +973,13 @@ export interface ASTBuildOptions { * element. Off by default: it accepts templates the strict grammar rejects. */ pairDanglingCloseTags?: boolean; + + /** + * Keep each HTML comment's body as opaque text instead of parsing the Liquid + * inside it. For HTML that is data rather than a template, such as a rendered + * setting value or a translation, where `{% ... %}` inside a comment is not a tag. + */ + opaqueHtmlComments?: boolean; } export function isBranchedTag(node: LiquidHtmlNode) { @@ -985,7 +1012,14 @@ export function toLiquidHtmlAST( const tokens = tokenize(source, { rawBodyRule: rawBodyRuleLookup((name) => env.tagForName(name)), }); - const parser = new DocumentParser(tokens, source, env, true, options.allowUnclosedDocumentNode); + const parser = new DocumentParser( + tokens, + source, + env, + true, + options.allowUnclosedDocumentNode, + options.opaqueHtmlComments, + ); parser.pairDanglingCloseTags = options.pairDanglingCloseTags ?? false; return parser.parseDocument(); } diff --git a/packages/liquid-html-parser/src/document/factories.ts b/packages/liquid-html-parser/src/document/factories.ts index 1640e6f5b..9e4c00a7c 100644 --- a/packages/liquid-html-parser/src/document/factories.ts +++ b/packages/liquid-html-parser/src/document/factories.ts @@ -353,15 +353,18 @@ export function makeHtmlRawNode( } export function makeHtmlComment( - body: string, - start: number, - end: number, + children: (LiquidNode | TextNode)[], + blockStartPosition: Position, + blockEndPosition: Position, source: string, ): HtmlComment { return { type: NodeTypes.HtmlComment, - body, - position: { start, end }, + body: source.slice(blockStartPosition.end, blockEndPosition.start).trim(), + children, + blockStartPosition, + blockEndPosition, + position: { start: blockStartPosition.start, end: blockEndPosition.end }, source, }; } diff --git a/packages/liquid-html-parser/src/document/html.ts b/packages/liquid-html-parser/src/document/html.ts index 6d06459d0..0c43b9ad9 100644 --- a/packages/liquid-html-parser/src/document/html.ts +++ b/packages/liquid-html-parser/src/document/html.ts @@ -53,12 +53,16 @@ import type { TagDefinition } from '../environment'; * the coupling explicit and narrow. */ export interface HtmlParserDelegate extends ParserBase { - readonly htmlParseHtml: boolean; + htmlParseHtml: boolean; htmlAllowUnclosedHtml: boolean; /** See `ASTBuildOptions.pairDanglingCloseTags`. */ readonly htmlPairDanglingCloseTags: boolean; htmlInAttributeContext: boolean; htmlInAttributeValueContext: boolean; + /** The `" +// htmlComment := ""? +// +// The body is parsed as text and Liquid, never as markup. When the body reaches +// a tag that ends or branches an enclosing block, the comment stops before that +// tag and `parseNode` resumes it after the tag. When a block opened inside the +// comment consumes the `-->`, the comment ends with that block. export function parseHtmlComment(parser: HtmlParserDelegate): HtmlComment { - const source = parser.getSource(); - const openToken = parser.consume(TokenType.HtmlCommentOpen); - const bodyStart = openToken.end; + if (parser.htmlOpaqueComments) return parseOpaqueHtmlComment(parser); - while (!parser.isAtEnd() && !parser.check(TokenType.HtmlCommentClose)) { - parser.advance(); + const source = parser.getSource(); + const start = parser.peek().start; + let blockStartPosition: Position = { start, end: start }; + if (parser.check(TokenType.HtmlCommentOpen)) { + const openToken = parser.consume(TokenType.HtmlCommentOpen); + blockStartPosition = { start: openToken.start, end: openToken.end }; + parser.htmlOpenComment = blockStartPosition; } - if (parser.isAtEnd()) { - throw new LiquidHTMLASTParsingError( - `Attempting to end parsing before HtmlComment '" +function parseOpaqueHtmlComment(parser: HtmlParserDelegate): HtmlComment { + const source = parser.getSource(); + const openToken = parser.consume(TokenType.HtmlCommentOpen); + while (!parser.isAtEnd() && !parser.check(TokenType.HtmlCommentClose)) parser.advance(); + if (parser.isAtEnd()) throw unclosedHtmlCommentError(source, openToken); const closeToken = parser.consume(TokenType.HtmlCommentClose); - const body = source.slice(bodyStart, bodyEnd).trim(); + return makeHtmlComment( + [], + { start: openToken.start, end: openToken.end }, + { start: closeToken.start, end: closeToken.end }, + source, + ); +} - return makeHtmlComment(body, openToken.start, closeToken.end, source); +function unclosedHtmlCommentError( + source: string, + openComment: Position, +): LiquidHTMLASTParsingError { + return new LiquidHTMLASTParsingError( + `Attempting to end parsing before HtmlComment '`. + p.dispatchOpenHtmlComment = null; + return advanceAsText(p); + case TokenType.HtmlTagClose: case TokenType.HtmlSelfClose: - case TokenType.HtmlCommentClose: case TokenType.HtmlEquals: case TokenType.HtmlQuoteOpen: case TokenType.HtmlQuoteClose: diff --git a/packages/liquid-html-parser/src/document/parser.ts b/packages/liquid-html-parser/src/document/parser.ts index dac39383e..5a950b5c5 100644 --- a/packages/liquid-html-parser/src/document/parser.ts +++ b/packages/liquid-html-parser/src/document/parser.ts @@ -69,6 +69,8 @@ export class DocumentParser private _pairDanglingCloseTags: boolean = false; private _inAttributeContext: boolean = false; private _inAttributeValueContext: boolean = false; + private _openHtmlComment: Position | null = null; + private readonly _opaqueHtmlComments: boolean; constructor( tokens: Token[], @@ -76,6 +78,7 @@ export class DocumentParser env: Environment, parseHtml: boolean, allowUnclosedDocumentNode: boolean = false, + opaqueHtmlComments: boolean = false, ) { super(tokens, source); this.env = env; @@ -83,6 +86,7 @@ export class DocumentParser this._parseHtml = parseHtml; this._allowUnclosedHtml = allowUnclosedDocumentNode; this._allowUnclosedDocumentNode = allowUnclosedDocumentNode; + this._opaqueHtmlComments = opaqueHtmlComments; } get rawParseHtml(): boolean { @@ -118,6 +122,18 @@ export class DocumentParser get htmlParseHtml(): boolean { return this._parseHtml; } + set htmlParseHtml(v: boolean) { + this._parseHtml = v; + } + get htmlOpenComment(): Position | null { + return this._openHtmlComment; + } + set htmlOpenComment(v: Position | null) { + this._openHtmlComment = v; + } + get htmlOpaqueComments(): boolean { + return this._opaqueHtmlComments; + } get htmlAllowUnclosedHtml(): boolean { return this._allowUnclosedHtml; } @@ -158,6 +174,12 @@ export class DocumentParser get dispatchAllowUnclosedHtml(): boolean { return this._allowUnclosedHtml; } + get dispatchOpenHtmlComment(): Position | null { + return this._openHtmlComment; + } + set dispatchOpenHtmlComment(v: Position | null) { + this._openHtmlComment = v; + } tagForName(name: string) { return this.env.tagForName(name); @@ -168,6 +190,8 @@ export class DocumentParser const children: LiquidHtmlNode[] = []; if (this.check(TokenType.YamlFrontmatter)) children.push(parseNodeFn(this)); while (!this.isAtEnd()) children.push(this.parseNodeInScope(children)); + // A comment still open at the end of the input ends there, as in a browser. + this._openHtmlComment = null; return makeDocumentNode( filterChildren(ChildFilterMode.Syntactic, children, this.source), this.source, diff --git a/packages/liquid-html-parser/src/document/tolerant-parser.ts b/packages/liquid-html-parser/src/document/tolerant-parser.ts index f942cd61f..58175864f 100644 --- a/packages/liquid-html-parser/src/document/tolerant-parser.ts +++ b/packages/liquid-html-parser/src/document/tolerant-parser.ts @@ -103,9 +103,17 @@ export class TolerantDocumentParser extends DocumentParser { * construct. isResyncToken includes EndOfInput, so this also stops * cleanly at the end of the source. */ - while (!this.isAtEnd() && !isResyncToken(this.peek().type)) this.advance(); + while (!this.isAtEnd() && !this.isResyncPoint()) this.advance(); const end = this.peek().start; return makeLiquidErrorNode(startTok.start, end, source, e.message, found); } } + + private isResyncPoint(): boolean { + const type = this.peek().type; + // Skipping the open comment's `-->` would make the rest of the document comment text. + return ( + isResyncToken(type) || (type === TokenType.HtmlCommentClose && this.htmlOpenComment !== null) + ); + } } diff --git a/packages/liquid-html-parser/src/tolerant.test.ts b/packages/liquid-html-parser/src/tolerant.test.ts index 9a0f42750..62d104587 100644 --- a/packages/liquid-html-parser/src/tolerant.test.ts +++ b/packages/liquid-html-parser/src/tolerant.test.ts @@ -189,6 +189,19 @@ describe('tolerant mode resynchronizes after an error (panic mode)', () => { expect(ast.children[0].position).toEqual({ start: 0, end: 12 }); expect(ast.children[2].position).toEqual({ start: 22, end: 33 }); }); + + it('resumes an interrupted HTML comment at its close, so the markup after it is HTML', () => { + const source = '

a

'; + const ast = toTolerantLiquidHtmlAST(source); + + expect(ast.children.map((child) => child.type)).toEqual([ + NodeTypes.HtmlComment, + NodeTypes.LiquidErrorNode, + NodeTypes.HtmlComment, + NodeTypes.HtmlElement, + ]); + expect(source.slice(ast.children[2].position.start, ast.children[2].position.end)).toBe('-->'); + }); }); describe('findErrorNodeAtOffset', () => { diff --git a/packages/liquid-html-parser/src/tolerant.ts b/packages/liquid-html-parser/src/tolerant.ts index 8ad7170a1..885e803bc 100644 --- a/packages/liquid-html-parser/src/tolerant.ts +++ b/packages/liquid-html-parser/src/tolerant.ts @@ -66,6 +66,7 @@ export function toTolerantLiquidHtmlAST( env, true, options.allowUnclosedDocumentNode, + options.opaqueHtmlComments, ); return parser.parseDocument(); } diff --git a/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts b/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts index 1eabbaa38..ee7c83b59 100644 --- a/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts +++ b/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts @@ -253,7 +253,7 @@ function printNode( } case NodeTypes.RawMarkup: { - if (node.parentNode?.name === 'doc') { + if (node.parentNode?.type === NodeTypes.LiquidRawTag && node.parentNode.name === 'doc') { return printLiquidDoc(path as AstPath, options, print, args); } @@ -332,6 +332,18 @@ function printNode( } case NodeTypes.HtmlComment: { + const { blockStartPosition: open, blockEndPosition: close } = node; + // Part of a comment split at a Liquid block boundary, or one that never + // closes: adding the missing delimiter would change the output. On a side + // without one, the next parse reads the line break printed there as part + // of this node, so trim it to keep formatting idempotent. + if (open.start === open.end || close.start === close.end) { + let text = node.source.slice(node.position.start, node.position.end); + if (open.start === open.end) text = text.trimStart(); + if (close.start === close.end) text = text.trimEnd(); + return text; + } + const conditionalComment = getConditionalComment( node.source.slice(node.position.start, node.position.end), ); diff --git a/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid b/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid index c69c34c87..89db02420 100644 --- a/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid @@ -69,3 +69,18 @@ It should support and indend the bodies of all IE comment variations + +It should print the parts of a comment split at a Liquid block boundary as written +{% if a %} + +{% endif %} + +It should print a comment as written when a block opened inside it holds its close + a {% endif %} +c + +It should print a comment that never closes as written +

a

+ + +It should print the parts of a comment split at a Liquid block boundary as written +{% if a %}{% endif %} + +It should print a comment as written when a block opened inside it holds its close + a {% endif %}c + +It should print a comment that never closes as written +

a

`, ``, `

" + docset: default + - name: should report an end tag inside an HTML comment that closes no block + files: + file.liquid: "" + docset: default + offenses: + - file: file.liquid + message: "Attempting to close LiquidTag 'if' before it was opened without a matching 'if'" + start: [0, 5] + highlight: "{% endif %}" diff --git a/specs/checks/undefined-object.yml b/specs/checks/undefined-object.yml index 4122443ec..480c67271 100644 --- a/specs/checks/undefined-object.yml +++ b/specs/checks/undefined-object.yml @@ -756,3 +756,12 @@ cases: } {% endschema %} docset: default + - name: reports an undefined object inside an HTML comment, which Liquid renders + files: + file.liquid: "" + docset: default + offenses: + - file: file.liquid + message: "Unknown object 'my_var' used." + start: [0, 8] + highlight: my_var diff --git a/specs/parser/html.yml b/specs/parser/html.yml index 3df32852c..817f73af4 100644 --- a/specs/parser/html.yml +++ b/specs/parser/html.yml @@ -2558,12 +2558,37 @@ cases: children: - type: HtmlComment body: hello {{ product.name }} + blockStartPosition: { start: [0, 0], text: } position: start: [0, 0] text: |- + children: + - type: TextNode + value: hello + position: { start: [1, 2], text: hello } + - type: LiquidVariableOutput + whitespaceStart: "" + whitespaceEnd: "" + markupPosition: { start: [1, 10], text: " product.name " } + position: { start: [1, 8], text: "{{ product.name }}" } + markup: + type: LiquidVariable + rawSource: product.name + position: { start: [1, 11], text: "product.name " } + expression: + type: VariableLookup + name: product + position: { start: [1, 11], text: product.name } + lookups: + - type: String + value: name + position: { start: [1, 19], text: name } + lookupModes: [bareword] + filters: [] - name: should parse script tags as raw source: |- ` now ends the script at the first `` and reports the second as an unmatched close tag. `` still pairs nested `` elements. From 2fd0edf53ba41b9d04dfd73a0dec41f4499c3cb1 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Tue, 3 Nov 2026 15:33:28 -0500 Subject: [PATCH 18/20] Test that a trailing reversed doesn't reverse a for loop Liquid only accepts `reversed` right after the collection, so `{% for x in c limit: 5 reversed %}` is a syntax error. The parser leaves the tag as string markup and prettier prints it as written. Reading it as reversed would make prettier move it before `limit:`, which reverses the loop. Co-Authored-By: Claude Opus 5.5 --- packages/liquid-html-parser/src/tags/for.test.ts | 6 ++++++ packages/liquid-html-parser/src/tags/for.ts | 1 + .../src/test/liquid-tag-for/fixed.liquid | 6 ++++++ .../src/test/liquid-tag-for/index.liquid | 6 ++++++ 4 files changed, 19 insertions(+) diff --git a/packages/liquid-html-parser/src/tags/for.test.ts b/packages/liquid-html-parser/src/tags/for.test.ts index 2b894aabf..4522de628 100644 --- a/packages/liquid-html-parser/src/tags/for.test.ts +++ b/packages/liquid-html-parser/src/tags/for.test.ts @@ -85,6 +85,12 @@ describe('forTag', () => { expect(result.args).toHaveLength(2); }); + it('rejects reversed after named args', () => { + expect(() => + forTag.parse('for', parser('item in collection limit:5 reversed'), stubParser), + ).toThrow(); + }); + it('parses for loop with comma-separated named args', () => { const result = forTag.parse('for', parser('item in collection limit:5, offset:2'), stubParser); expect(result).toMatchObject({ diff --git a/packages/liquid-html-parser/src/tags/for.ts b/packages/liquid-html-parser/src/tags/for.ts index 32d8cf65f..86ee7f58f 100644 --- a/packages/liquid-html-parser/src/tags/for.ts +++ b/packages/liquid-html-parser/src/tags/for.ts @@ -10,6 +10,7 @@ function parseForMarkup(_name: string, markup: MarkupParser, _parser: Parser): F throw new Error("Expected 'in'"); } const collection = markup.valueExpression(); + // Liquid only accepts `reversed` here, before any named argument. const reversed = markup.id('reversed'); // Named args in for/tablerow accept optional commas between args diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-for/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-for/fixed.liquid index 8f0306c88..a9182d34a 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-for/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-for/fixed.liquid @@ -39,3 +39,9 @@ printWidth: 80 {% for element in collection reversed limit: 10 %} {{ element }} {% endfor %} + +It keeps a reversed after tag arguments where it is, since Liquid ignores it there +printWidth: 1 +{% for element in collection limit: 10 reversed %} + {{ element }} +{% endfor %} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-for/index.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-for/index.liquid index 9035b17b7..24d3cc50f 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-for/index.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-for/index.liquid @@ -30,3 +30,9 @@ printWidth: 80 {% for element in collection reversed limit: 10 %} {{ element }} {% endfor %} + +It keeps a reversed after tag arguments where it is, since Liquid ignores it there +printWidth: 1 +{% for element in collection limit: 10 reversed %} + {{ element }} +{% endfor %} From 713066d77eae37a68d4c65cd52f94eddd35bf621 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Wed, 4 Nov 2026 17:31:30 -0500 Subject: [PATCH 19/20] Release the HTML comment and script/style changes as major Liquid in HTML comments, the browser-style comment tokenizer and raw `

x

` is one comment whose `if` holds `

x

` as text, not two comments around a `

` element. +- `` with no open `if` no longer parses. An unclosed `` now parse. +- Code that creates `HtmlComment` nodes must set `children`, `blockStartPosition` and `blockEndPosition`. + +Pass `opaqueHtmlComments: true` to get the 2.x tree for these templates. + +Checks now see the Liquid in HTML comments, and completions work inside them. Prettier prints an unclosed comment as written, and can move whitespace and line breaks inside a comment that holds a Liquid block or is split by one. diff --git a/.changeset/html-comment-mode.md b/.changeset/html-comment-mode.md index d47f699a5..2a3d9b4e6 100644 --- a/.changeset/html-comment-mode.md +++ b/.changeset/html-comment-mode.md @@ -1,5 +1,12 @@ --- -'@shopify/liquid-html-parser': patch +'@shopify/liquid-html-parser': major --- Tokenize HTML comments as a browser does. In a comment only Liquid and the comment's close count, so a ``. A comment closes on `-->` or `--!>`, and `` and `` are empty comments. Outside a comment, `-->` is text. The bodies of Liquid raw tags like `{% comment %}` are opaque to the tokenizer too, so a `

shown
-->` is an empty comment, a `
` element and the text `-->`. It used to be one comment. +- `

b

` closes the comment at `--!>` instead of failing to parse. + +`opaqueHtmlComments` doesn't change these. diff --git a/.changeset/script-style-raw-text.md b/.changeset/script-style-raw-text.md index 97b7cf5ca..48f77166f 100644 --- a/.changeset/script-style-raw-text.md +++ b/.changeset/script-style-raw-text.md @@ -1,5 +1,9 @@ --- -'@shopify/liquid-html-parser': patch +'@shopify/liquid-html-parser': major --- Read `c` now ends the script at the first `` and reports the second as an unmatched close tag. `` still pairs nested `` elements. + +**Breaking changes** + +- A ``. `` and `` now fail to parse in `strict` mode with "Attempting to close HtmlElement 'script' before it was opened". From 1438b50c7e0aab94b9cb50c0e3eb22a8d6b62f09 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Thu, 5 Nov 2026 11:08:32 -0500 Subject: [PATCH 20/20] Release the HTML comment and script/style changes as minor This undoes the major bump from 713066d7. The three changesets keep their lists of what changes for existing templates, now headed "Behavior changes". `opaqueHtmlComments` is described as giving the previous tree rather than the 2.x tree, since the release stays on 2.x. Co-Authored-By: Claude Opus 5.5 --- .changeset/html-comment-children.md | 6 +++--- .changeset/html-comment-mode.md | 4 ++-- .changeset/script-style-raw-text.md | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.changeset/html-comment-children.md b/.changeset/html-comment-children.md index e36f398f9..e9ebe2782 100644 --- a/.changeset/html-comment-children.md +++ b/.changeset/html-comment-children.md @@ -1,5 +1,5 @@ --- -'@shopify/liquid-html-parser': major +'@shopify/liquid-html-parser': minor '@shopify/prettier-plugin-liquid': patch '@shopify/theme-check-common': patch '@shopify/theme-language-server-common': patch @@ -13,13 +13,13 @@ Parse the Liquid inside HTML comments. Liquid renders it, and its tags can open, - An end tag in a comment that closes no block is a parse error, as in Liquid. - The `opaqueHtmlComments` option keeps comment bodies opaque. -**Breaking changes** +**Behavior changes** - Code that walks `children` now visits the Liquid and text inside HTML comments. - `

x

` is one comment whose `if` holds `

x

` as text, not two comments around a `

` element. - `` with no open `if` no longer parses. An unclosed `` now parse. - Code that creates `HtmlComment` nodes must set `children`, `blockStartPosition` and `blockEndPosition`. -Pass `opaqueHtmlComments: true` to get the 2.x tree for these templates. +Pass `opaqueHtmlComments: true` to get the previous tree for these templates. Checks now see the Liquid in HTML comments, and completions work inside them. Prettier prints an unclosed comment as written, and can move whitespace and line breaks inside a comment that holds a Liquid block or is split by one. diff --git a/.changeset/html-comment-mode.md b/.changeset/html-comment-mode.md index 2a3d9b4e6..45689b679 100644 --- a/.changeset/html-comment-mode.md +++ b/.changeset/html-comment-mode.md @@ -1,10 +1,10 @@ --- -'@shopify/liquid-html-parser': major +'@shopify/liquid-html-parser': minor --- Tokenize HTML comments as a browser does. In a comment only Liquid and the comment's close count, so a ``. A comment closes on `-->` or `--!>`, and `` and `` are empty comments. Outside a comment, `-->` is text. The bodies of Liquid raw tags like `{% comment %}` are opaque to the tokenizer too, so a `

shown
-->` is an empty comment, a `
` element and the text `-->`. It used to be one comment. - `

b

` closes the comment at `--!>` instead of failing to parse. diff --git a/.changeset/script-style-raw-text.md b/.changeset/script-style-raw-text.md index 48f77166f..f79c97b31 100644 --- a/.changeset/script-style-raw-text.md +++ b/.changeset/script-style-raw-text.md @@ -1,9 +1,9 @@ --- -'@shopify/liquid-html-parser': major +'@shopify/liquid-html-parser': minor --- Read `c` now ends the script at the first `` and reports the second as an unmatched close tag. `` still pairs nested `` elements. -**Breaking changes** +**Behavior changes** - A ``. `` and `` now fail to parse in `strict` mode with "Attempting to close HtmlElement 'script' before it was opened".