From f071bebd7a30c78d4c1341358f1f819d68f83368 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 14 Jul 2026 16:55:30 +0200 Subject: [PATCH 1/4] Update theme-check-common, prettier-plugin-liquid, and theme-graph for new parser --- packages/prettier-plugin-liquid/src/parser.ts | 4 +- .../preprocess/augment-with-css-properties.ts | 6 + .../src/printer/print/liquid.ts | 60 ++++-- .../src/printer/print/tag.ts | 15 +- .../src/printer/printer-liquid-html.ts | 50 ++++- .../src/printer/utils/node.ts | 13 +- .../src/test/embed-script-tag/fixed.liquid | 13 +- .../src/test/html-attributes/fixed.liquid | 10 +- .../src/test/html-comment/fixed.liquid | 46 ++--- .../src/test/issue-154/fixed.liquid | 3 +- .../src/test/issue-156/fixed.liquid | 4 +- .../src/test/liquid-comment/fixed.liquid | 12 +- .../src/test/liquid-doc/fixed.liquid | 9 +- .../src/test/liquid-drop-filters/fixed.liquid | 6 +- .../src/test/liquid-liquid-tag/fixed.liquid | 27 +-- .../test/liquid-tag-ifchanged/fixed.liquid | 5 + .../test/liquid-tag-ifchanged/index.liquid | 5 + .../test/liquid-tag-ifchanged/index.spec.ts | 7 + .../src/test/liquid-tag-paginate/fixed.liquid | 7 +- .../src/test/liquid-tag-tablerow/fixed.liquid | 14 +- .../prettier-plugin-liquid/vitest.config.mjs | 1 - packages/theme-check-common/package.json | 2 +- .../src/checks/cdn-preconnect/index.ts | 48 +++-- .../src/checks/deprecate-lazysizes/index.ts | 57 +++--- .../checks/InvalidBooleanExpression.spec.ts | 2 +- .../checks/InvalidConditionalNode.ts | 10 + .../checks/InvalidFilterName.ts | 94 ++++++++-- .../liquid-html-syntax-error/index.spec.ts | 16 +- .../src/checks/missing-template/index.ts | 16 +- .../src/checks/remote-asset/index.ts | 35 +++- .../index.ts | 4 +- .../theme-check-common/src/checks/utils.ts | 22 +++ .../valid-visible-if/visible-if-utils.ts | 6 +- .../src/disabled-checks/test-checks.ts | 5 +- .../src/liquid-doc/arguments.ts | 46 +++-- .../theme-check-common/src/visitor.spec.ts | 5 +- packages/theme-check-common/src/visitor.ts | 4 +- packages/theme-graph/src/graph/traverse.ts | 4 +- pnpm-lock.yaml | 177 +++++++++--------- 39 files changed, 577 insertions(+), 293 deletions(-) create mode 100644 packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/fixed.liquid create mode 100644 packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.liquid create mode 100644 packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.spec.ts diff --git a/packages/prettier-plugin-liquid/src/parser.ts b/packages/prettier-plugin-liquid/src/parser.ts index 143889a7a..51bcb66d8 100644 --- a/packages/prettier-plugin-liquid/src/parser.ts +++ b/packages/prettier-plugin-liquid/src/parser.ts @@ -1,9 +1,9 @@ -import { toLiquidHtmlAST, LiquidHtmlNode } from '@shopify/liquid-html-parser'; +import { toTolerantLiquidHtmlAST, LiquidHtmlNode } from '@shopify/liquid-html-parser'; import { locEnd, locStart } from './utils'; export function parse(text: string): LiquidHtmlNode { - return toLiquidHtmlAST(text); + return toTolerantLiquidHtmlAST(text); } export const liquidHtmlAstFormat = 'liquid-html-ast'; diff --git a/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts b/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts index f5d204b09..9aac64c54 100644 --- a/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts +++ b/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts @@ -121,6 +121,8 @@ function getCssDisplay(node: AugmentedNode, options: LiquidParserO case NodeTypes.Range: case NodeTypes.VariableLookup: case NodeTypes.AssignMarkup: + case NodeTypes.BlockMarkup: + case NodeTypes.SectionMarkup: case NodeTypes.CycleMarkup: case NodeTypes.ContentForMarkup: case NodeTypes.ForMarkup: @@ -134,6 +136,7 @@ function getCssDisplay(node: AugmentedNode, options: LiquidParserO case NodeTypes.LiquidDocExampleNode: case NodeTypes.LiquidDocDescriptionNode: case NodeTypes.LiquidDocPromptNode: + case NodeTypes.LiquidErrorNode: return 'should not be relevant'; default: @@ -232,6 +235,8 @@ function getNodeCssStyleWhiteSpace( case NodeTypes.Range: case NodeTypes.VariableLookup: case NodeTypes.AssignMarkup: + case NodeTypes.BlockMarkup: + case NodeTypes.SectionMarkup: case NodeTypes.CycleMarkup: case NodeTypes.ContentForMarkup: case NodeTypes.ForMarkup: @@ -245,6 +250,7 @@ function getNodeCssStyleWhiteSpace( case NodeTypes.LiquidDocExampleNode: case NodeTypes.LiquidDocDescriptionNode: case NodeTypes.LiquidDocPromptNode: + case NodeTypes.LiquidErrorNode: return 'should not be relevant'; default: diff --git a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts index 7e8628311..ae147fc98 100644 --- a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts +++ b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts @@ -7,6 +7,7 @@ import { LiquidDocExampleNode, LiquidDocDescriptionNode, LiquidDocPromptNode, + TAGS_WITHOUT_MARKUP, } from '@shopify/liquid-html-parser'; import { Doc, doc } from 'prettier'; @@ -192,13 +193,28 @@ function printNamedLiquidBlockStart( case NamedTags.increment: case NamedTags.decrement: case NamedTags.layout: - case NamedTags.section: { + case NamedTags.section: + /* + * `block` prints like `section` (both carry a name plus optional + * named arguments) and `partial` prints like `sections` (a bare + * string markup). Both delegate markup rendering to `printNode`. + */ + case NamedTags.block: + case NamedTags.partial: { return tag(' '); } case NamedTags.sections: { return tag(' '); } + /* + * `ifchanged` has no markup (`markup: null`), so we print just the + * tag name — there is nothing to render between the name and `%}`. + */ + case NamedTags.ifchanged: { + return wrapper([...prefix, node.name, ...suffix(' ')]); + } + case NamedTags.form: { const trailingWhitespace = node.markup.length > 1 ? line : ' '; return tagWithArrayMarkup(trailingWhitespace); @@ -342,16 +358,16 @@ export function printLiquidBlockStart( } const markup = node.markup; - return group([ - '{%', - whitespaceStart, - ' ', - node.name, - markup ? ` ${markup}` : '', - ' ', - whitespaceEnd, - '%}', - ]); + /* + * A few tags — `break`, `continue`, `else`, and friends listed in + * TAGS_WITHOUT_MARKUP — accept no markup at all. When one of them is + * written with stray arguments, e.g. `{% break huh?? %}`, we drop the + * markup rather than echo the invalid text back out. This restores the + * pre-port behaviour, where the parser blanked the markup before the + * printer ever saw it. + */ + const printedMarkup = markup && !TAGS_WITHOUT_MARKUP.includes(node.name) ? ` ${markup}` : ''; + return group(['{%', whitespaceStart, ' ', node.name, printedMarkup, ' ', whitespaceEnd, '%}']); } export function printLiquidBlockEnd( @@ -480,7 +496,13 @@ export function printLiquidRawTag( ' ', node.name, ' ', - node.markup ? `${node.markup} ` : '', + /* + * Argument-less raw tags such as `style` keep no markup. When one + * carries stray arguments, e.g. `{% style what %}`, we strip them + * instead of printing the invalid text; other raw tags keep their + * markup as before. + */ + node.markup && !TAGS_WITHOUT_MARKUP.includes(node.name) ? `${node.markup} ` : '', node.whitespaceEnd, '%}', ]); @@ -530,6 +552,20 @@ export function printLiquidDocParam( _args: LiquidPrinterArgs, ): Doc { const node = path.getValue(); + + /* + * A malformed `@param` line (e.g. an unclosed `[missingTail`) parses to a + * degenerate node with an empty `paramName`. Synthesizing the parts below + * would emit a spurious `@param - ...`, so emit the raw source span + * verbatim instead to preserve a faithful round-trip. `@param` is kept as + * the first part so `printLiquidDoc`'s tag grouping still treats this as a + * `@param` node and does not insert a blank line before the next param. + */ + if (node.paramName.value === '') { + const raw = node.source.slice(node.position.start, node.position.end); + return ['@param', raw.slice('@param'.length)]; + } + const parts: Doc[] = ['@param']; if (node.paramType?.value) { diff --git a/packages/prettier-plugin-liquid/src/printer/print/tag.ts b/packages/prettier-plugin-liquid/src/printer/print/tag.ts index 3c28506ee..675ce5bda 100644 --- a/packages/prettier-plugin-liquid/src/printer/print/tag.ts +++ b/packages/prettier-plugin-liquid/src/printer/print/tag.ts @@ -437,11 +437,18 @@ function getCompoundName( .map((part) => { if (part.type === NodeTypes.TextNode) { return part.value; - } else if (typeof part.markup === 'string') { - return `{{ ${part.markup.trim()} }}`; - } else { - return `{{ ${part.markup.rawSource} }}`; } + if (part.type === NodeTypes.LiquidVariableOutput) { + return typeof part.markup === 'string' + ? `{{ ${part.markup.trim()} }}` + : `{{ ${part.markup.rawSource} }}`; + } + /* + * Remaining compound-name arms are LiquidTag | LiquidRawTag + * (e.g. `<{% if c %}a{% endif %}>`). Neither carries a `.rawSource`, + * so reproduce the original source span verbatim. + */ + return part.source.slice(part.position.start, part.position.end); }) .join(''); } 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 57f4f91bf..1f61701e0 100644 --- a/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts +++ b/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts @@ -194,6 +194,18 @@ function printNode( args: LiquidPrinterArgs = {}, ): Doc { const node = path.getValue(); + + if ((node as any).type === 'BlockArrayLiteral') { + return [ + '[', + join( + [',', line], + (path as any).map((p: any) => print(p), 'elements'), + ), + ']', + ]; + } + switch (node.type) { case NodeTypes.Document: { return [printChildren(path as AstPath, options, print, args), hardline]; @@ -343,7 +355,7 @@ function printNode( whitespace, join( [',', whitespace], - path.map((p) => print(p), 'args'), + path.map((p: any) => print(p), 'args'), ), ); @@ -362,7 +374,7 @@ function printNode( line, join( line, - path.map((p) => print(p), 'args'), + path.map((p: any) => print(p), 'args'), ), ]); } @@ -384,7 +396,7 @@ function printNode( line, join( [',', line], - path.map((p) => print(p), 'args'), + path.map((p: any) => print(p), 'args'), ), ]); } @@ -401,7 +413,29 @@ function printNode( line, join( [',', line], - path.map((p) => print(p), 'args'), + path.map((p: any) => print(p), 'args'), + ), + ); + } + + return doc; + } + + /* + * `block` and `section` markup share the same shape: a name (a + * LiquidString) followed by optional named arguments. We print them + * the same way `content_for` prints its markup. + */ + case NodeTypes.BlockMarkup: + case NodeTypes.SectionMarkup: { + const doc: Doc = [path.call((p: any) => print(p), 'name')]; + if (node.args.length > 0) { + doc.push( + ',', + line, + join( + [',', line], + path.map((p: any) => print(p), 'args'), ), ); } @@ -428,7 +462,7 @@ function printNode( line, join( [',', line], - path.map((p) => print(p), 'args'), + path.map((p: any) => print(p), 'args'), ), ); } @@ -479,7 +513,7 @@ function printNode( let args: Doc[] = []; if (node.args.length > 0) { - const printed = path.map((p) => print(p), 'args'); + const printed = path.map((p: any) => print(p), 'args'); const shouldPrintFirstArgumentSameLine = node.args[0].type !== NodeTypes.NamedArgument; if (shouldPrintFirstArgumentSameLine) { @@ -590,6 +624,10 @@ function printNode( return printLiquidDocPrompt(path as AstPath, options, print, args); } + case NodeTypes.LiquidErrorNode: { + return node.source.slice(node.position.start, node.position.end); + } + default: { return assertNever(node); } diff --git a/packages/prettier-plugin-liquid/src/printer/utils/node.ts b/packages/prettier-plugin-liquid/src/printer/utils/node.ts index 42c723bc1..5341de3e8 100644 --- a/packages/prettier-plugin-liquid/src/printer/utils/node.ts +++ b/packages/prettier-plugin-liquid/src/printer/utils/node.ts @@ -1,4 +1,10 @@ -import { NodeTypes, LiquidNodeTypes, HtmlNodeTypes, Position } from '@shopify/liquid-html-parser'; +import { + NodeTypes, + LiquidNodeTypes, + HtmlNodeTypes, + Position, + CompoundNameSegment, +} from '@shopify/liquid-html-parser'; import { HtmlSelfClosingElement, LiquidHtmlNode, @@ -330,10 +336,7 @@ export function getLastDescendant(node: LiquidHtmlNode): LiquidHtmlNode { return node.lastChild ? getLastDescendant(node.lastChild) : node; } -function isTagNameIncluded( - collection: string[], - name: (TextNode | LiquidVariableOutput)[], -): boolean { +function isTagNameIncluded(collection: string[], name: CompoundNameSegment[]): boolean { if (name.length !== 1 || name[0].type !== NodeTypes.TextNode) return false; return collection.includes(name[0].value); } diff --git a/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid b/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid index 8d2ad5a9d..aff739a0f 100644 --- a/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid @@ -1,4 +1,4 @@ -It should pretty pring pure JS inside a script tag +It should pretty print pure JS inside a script tag printWidth: 20 diff --git a/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid b/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid index 1dced10b4..79708ea87 100644 --- a/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid @@ -70,7 +70,7 @@ It should break and indent attributes that are long af If there's only one attribute and self-closing, don't break when the line is too long. -If there's only one attribute and not self-closing, break the attr and close the tag on a different line. +If there's only one attribute, not self-closing and no children, break the attr and close the tag on a different line.
@@ -81,17 +81,19 @@ If there's only one attribute and not self-closing, but with children, do not br If there's only one attribute, not self-closing, with children, do not break the LiquidTag attr and close the tag on a different line. +printWidth: 30
Hello
If there's only one attribute, not self-closing, with children, do not break the LiquidNode attr and close the tag on a different line. +printWidth: 30
Hello
If there's only one attribute, not self closing, with children, but the attribute is a liquid tag that has children, break it normally. -printWidth: 30 +printWidth: 40
If there's only one attribute, not self closing, without children, but the attribute is a liquid tag that has children, break it normally. -printWidth: 30 +printWidth: 40
It should pretty print smart quotes into dumb quotes to prevent copy pasting errors -

+

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..2237d93a6 100644 --- a/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid @@ -1,15 +1,18 @@ +it should format as expected (chunk 0) +it should format as expected (chunk 1) +it should format as expected (chunk 2) It should change the css display of the next node @@ -21,51 +24,52 @@ printWidth: 1 It should preserve MSO conditional comments -It should support the IE Comment '&' operator +It should support the IE Comment '&' operators -It should support the IE Comment '|' operator +It should support the IE Comment '|' operators It should support and indend the bodies of all IE comment variations +printWidth: 60 diff --git a/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid b/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid index 24d5e5f48..f834b0dac 100644 --- a/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid @@ -1,3 +1,4 @@ +it should format as expected (chunk 0) {%- liquid layout none @@ -5,7 +6,7 @@ render 'product-item', product: item endpaginate - paginate search.results by 28, window_size: 1 + paginate search.results by 28 window_size:1 render 'product-item', product: item endpaginate -%} diff --git a/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid b/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid index 718cbb472..1c8c895f2 100644 --- a/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid @@ -1,7 +1,9 @@ +it should format as expected (chunk 0) diff --git a/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid index 3cff9b772..dfc861243 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid @@ -1,3 +1,4 @@ +it should format as expected (chunk 0) {% comment %} This gets reindented, but we don't parse the content. So stuff can be unclosed and it'll be fine. @@ -11,22 +12,23 @@ It should support nested comments ho {% comment %} hi - {% endcomment %} - ha {% endcomment %} +ha +{%endcomment%} It should support empty comments {% comment %}{% endcomment %} It should support nested empty comments -{% comment %}{% comment %}{% endcomment %}{% endcomment %} +{% comment %}{% comment %}{% endcomment %} +{% endcomment %} It should support nested empty comments (pt 2) {% comment %} hi {% comment %} - {% endcomment %} - ho +{% endcomment %} +ho {% endcomment %} It should support inline comments diff --git a/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid index 2adb04aa5..35b6caa39 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid @@ -10,7 +10,7 @@ It should trim whitespace between nodes It should normalize the param type {% doc %} - @param {string} paramName - param with description + @param { string } paramName - param with description {% enddoc %} It should format the param description with a dash separator @@ -19,7 +19,6 @@ It should format the param description with a dash separator {% enddoc %} It should respect the liquidDocParamDash option liquidDocParamDash: false -liquidDocParamDash: false {% doc %} @param paramName param with description {% enddoc %} @@ -31,10 +30,10 @@ It should normalize the param description It should strip whitespace between optional param delimiters {% doc %} - @param [paramName] - param with description + @param [ paramName ] - param with description {% enddoc %} -It should not break params with malformed optional delimiters +It should handle params with malformed optional delimiters {% doc %} @param [missingTail - param with description @param missingHead - ] - param with description @@ -99,7 +98,7 @@ It should push multi-line description content to the next line It should add a space between a description tag and content {% doc %} - @description This is a description + @descriptionThis is a description {% enddoc %} It should handle param, example, and description nodes diff --git a/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid index 5e087b890..c77e0827a 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid @@ -48,7 +48,7 @@ printWidth: 1 key2: val2 }} -It should support a mix of positional and named arguments as expected (p40) +It should support a mix of positional and named arguments as expected (p50) printWidth: 50 {{ x @@ -59,7 +59,7 @@ It should support a mix of positional and named arguments as expected (p80) printWidth: 80 {{ x | filter1: pos1, pos2, key1: val1, key2: val2 }} -It should support all types of arguments +It should support all types of arguments, and add spaces {{ x | f: 'string' }} {{ x | f: 'string' }} {{ x | f: 0.0 }} @@ -81,4 +81,4 @@ It should support spaces or lack of thereof erywhere {{ x | f: a: b, range: (a..b) }} It should strip trailing commas by default -{{ x | split: '' }} +{{ x|split:'', }} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid index a63c8c919..b2be286fe 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid @@ -1,3 +1,4 @@ +it should format as expected (chunk 0)
{% liquid assign x = 1 @@ -36,19 +37,19 @@ It should handle comments correctly It should handle nested comments correctly {%- liquid comment - how u doin? - comment - ho - endcomment + how u doin? + comment + ho + endcomment endcomment -%} It should handle empty nested comments correctly {%- liquid comment - how u doin? - comment - endcomment + how u doin? + comment + endcomment endcomment -%} @@ -63,7 +64,7 @@ It should handle inline comments correctly It should pretty print #### lines without a space {%- liquid ############# - # fancy + # fancy ############# -%} @@ -117,7 +118,7 @@ printWidth: 1 section 'string' layout 'string' - paginate col by 10, window_size: 50 + paginate col by 10 window_size:50 render 'icon' with x as bar, foo: bar endpaginate %} @@ -137,10 +138,10 @@ It should keep newlines between tags It should keep leading whitespace in comments {% liquid # This list should not be reindented: - # - a - # - b - # - d - # - c + # - a + # - b + # - d + # - c %} It should not freak out on variable output characters inside conditions diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/fixed.liquid new file mode 100644 index 000000000..a6b10d22f --- /dev/null +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/fixed.liquid @@ -0,0 +1,5 @@ +It should format ifchanged block tags +{% ifchanged %}content{% endifchanged %} + +It should normalize whitespace control on ifchanged tags +{%- ifchanged -%}content{%- endifchanged -%} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.liquid new file mode 100644 index 000000000..f8130f74a --- /dev/null +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.liquid @@ -0,0 +1,5 @@ +It should format ifchanged block tags +{% ifchanged %}content{% endifchanged %} + +It should normalize whitespace control on ifchanged tags +{%- ifchanged -%}content{%- endifchanged -%} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.spec.ts b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.spec.ts new file mode 100644 index 000000000..0a3e8d617 --- /dev/null +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-ifchanged/index.spec.ts @@ -0,0 +1,7 @@ +import { test } from 'vitest'; +import { assertFormattedEqualsFixed } from '../test-helpers'; +import * as path from 'path'; + +test('Unit: liquid-tag-ifchanged', async () => { + await assertFormattedEqualsFixed(__dirname); +}); diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid index f016b0b3a..bff3aba68 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid @@ -11,16 +11,13 @@ printWidth: 1 It is undocumented, but it also accepts named attributes (window_size). Those are comma separated. printWidth: 80 -{% paginate collection by pageSize, window_size: 50, attr: (0..2) %} +{% paginate collection by pageSize window_size:50, attr:(0.. 2) %} {{ collection }} {% endpaginate %} It is undocumented, but it also accepts named attributes (window_size). We print those comma separated and on a new line when it breaks. printWidth: 1 -{% paginate collection - by pageSize, - window_size: 50 -%} +{% paginate collection by pageSize window_size: 50 %} {{ collection }} {% endpaginate %} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid index 997a3d9c4..7ab48959c 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid @@ -4,7 +4,8 @@ It takes an element and a collection It does not breaks on in collection printWidth: 1 {% tablerow element in collection %} - {{ collection }} + + {{ collection }} {% endtablerow %} It breaks on reversed @@ -12,7 +13,8 @@ printWidth: 1 {% tablerow element in collection reversed %} - {{ element }} + + {{ element }} {% endtablerow %} It breaks on tag arguments @@ -21,16 +23,18 @@ printWidth: 1 limit: 10 offset: 10 %} - {{ element }} + + {{ element }} {% endtablerow %} -It breaks on both reversed and tag argumentsc +It breaks on both reversed and tag arguments printWidth: 1 {% tablerow element in collection reversed limit: 10 %} - {{ element }} + + {{ element }} {% endtablerow %} It only breaks if the printWidth is hit diff --git a/packages/prettier-plugin-liquid/vitest.config.mjs b/packages/prettier-plugin-liquid/vitest.config.mjs index 8c27203ac..488dbfd86 100644 --- a/packages/prettier-plugin-liquid/vitest.config.mjs +++ b/packages/prettier-plugin-liquid/vitest.config.mjs @@ -9,6 +9,5 @@ export default defineConfig({ maxWorkers: 1, isolate: true, globalSetup: ['./src/test/test-setup.js'], - setupFiles: ['../liquid-html-parser/build/shims.js'], }, }); diff --git a/packages/theme-check-common/package.json b/packages/theme-check-common/package.json index eaa58726f..0600c4947 100644 --- a/packages/theme-check-common/package.json +++ b/packages/theme-check-common/package.json @@ -22,7 +22,7 @@ "build": "pnpm build:ts", "build:ci": "pnpm build", "build:ts": "tsc -b tsconfig.build.json", - "test": "vitest", + "test": "vitest --root ../.. --run packages/theme-check-common/src --exclude \"dist/**\"", "type-check": "tsc --noEmit" }, "dependencies": { diff --git a/packages/theme-check-common/src/checks/cdn-preconnect/index.ts b/packages/theme-check-common/src/checks/cdn-preconnect/index.ts index 0bb04972a..12b4b60e1 100644 --- a/packages/theme-check-common/src/checks/cdn-preconnect/index.ts +++ b/packages/theme-check-common/src/checks/cdn-preconnect/index.ts @@ -1,5 +1,6 @@ +import { HtmlSelfClosingElement, HtmlVoidElement } from '@shopify/liquid-html-parser'; import { Severity, SourceCodeType, LiquidCheckDefinition } from '../../types'; -import { isAttr, isValuedHtmlAttribute, valueIncludes } from '../utils'; +import { getHtmlNodeName, isAttr, isValuedHtmlAttribute, valueIncludes } from '../utils'; export const CdnPreconnect: LiquidCheckDefinition = { meta: { @@ -17,26 +18,37 @@ export const CdnPreconnect: LiquidCheckDefinition = { }, create(context) { - return { - async HtmlVoidElement(node) { - if (node.name !== 'link') return; + function checkNode(node: HtmlVoidElement | HtmlSelfClosingElement) { + if (getHtmlNodeName(node) !== 'link') return; + + const isPreconnect = node.attributes + .filter(isValuedHtmlAttribute) + .some((attr) => isAttr(attr, 'rel') && valueIncludes(attr, 'preconnect')); + if (!isPreconnect) return; - const isPreconnect = node.attributes - .filter(isValuedHtmlAttribute) - .some((attr) => isAttr(attr, 'rel') && valueIncludes(attr, 'preconnect')); - if (!isPreconnect) return; + const isShopifyCdn = node.attributes + .filter(isValuedHtmlAttribute) + .some((attr) => isAttr(attr, 'href') && valueIncludes(attr, '.+cdn.shopify.com.+')); + if (!isShopifyCdn) return; - const isShopifyCdn = node.attributes - .filter(isValuedHtmlAttribute) - .some((attr) => isAttr(attr, 'href') && valueIncludes(attr, '.+cdn.shopify.com.+')); - if (!isShopifyCdn) return; + context.report({ + message: + 'Preconnecting to cdn.shopify.com is unnecessary and can lead to worse performance', + startIndex: node.position.start, + endIndex: node.position.end, + }); + } - context.report({ - message: - 'Preconnecting to cdn.shopify.com is unnecessary and can lead to worse performance', - startIndex: node.position.start, - endIndex: node.position.end, - }); + return { + async HtmlVoidElement(node) { + checkNode(node); + }, + // The ported parser emits `HtmlSelfClosingElement` for self-closed + // void tags such as ``, whereas the previous parser emitted + // `HtmlVoidElement` regardless of the trailing slash. Visit both so the + // check still fires on self-closing markup. + async HtmlSelfClosingElement(node) { + checkNode(node); }, }; }, diff --git a/packages/theme-check-common/src/checks/deprecate-lazysizes/index.ts b/packages/theme-check-common/src/checks/deprecate-lazysizes/index.ts index 783582d32..2986faa48 100644 --- a/packages/theme-check-common/src/checks/deprecate-lazysizes/index.ts +++ b/packages/theme-check-common/src/checks/deprecate-lazysizes/index.ts @@ -1,6 +1,8 @@ +import { HtmlSelfClosingElement, HtmlVoidElement } from '@shopify/liquid-html-parser'; import { Severity, SourceCodeType, LiquidCheckDefinition } from '../../types'; import { ValuedHtmlAttribute, + getHtmlNodeName, isAttr, isValuedHtmlAttribute, isHtmlAttribute, @@ -28,30 +30,41 @@ export const DeprecateLazysizes: LiquidCheckDefinition = { }, create(context) { + function checkNode(node: HtmlVoidElement | HtmlSelfClosingElement) { + if (getHtmlNodeName(node) !== 'img') return; + + const attributes = node.attributes.filter(isHtmlAttribute); + const hasSrc = attributes.some((attr) => isAttr(attr, 'src')); + const hasNativeLoading = attributes.some((attr) => isAttr(attr, 'loading')); + if (hasSrc && hasNativeLoading) return; + + const hasLazyloadClass = node.attributes + .filter(isValuedHtmlAttribute) + .some((attr) => isAttr(attr, 'class') && valueIncludes(attr, 'lazyload')); + if (!hasLazyloadClass) return; + + const hasLazysizesAttribute = node.attributes + .filter(isValuedHtmlAttribute) + .some(showsLazysizesUsage); + if (!hasLazysizesAttribute) return; + + context.report({ + message: 'Use the native loading="lazy" attribute instead of lazysizes', + startIndex: node.position.start, + endIndex: node.position.end, + }); + } + return { async HtmlVoidElement(node) { - if (node.name !== 'img') return; - - const attributes = node.attributes.filter(isHtmlAttribute); - const hasSrc = attributes.some((attr) => isAttr(attr, 'src')); - const hasNativeLoading = attributes.some((attr) => isAttr(attr, 'loading')); - if (hasSrc && hasNativeLoading) return; - - const hasLazyloadClass = node.attributes - .filter(isValuedHtmlAttribute) - .some((attr) => isAttr(attr, 'class') && valueIncludes(attr, 'lazyload')); - if (!hasLazyloadClass) return; - - const hasLazysizesAttribute = node.attributes - .filter(isValuedHtmlAttribute) - .some(showsLazysizesUsage); - if (!hasLazysizesAttribute) return; - - context.report({ - message: 'Use the native loading="lazy" attribute instead of lazysizes', - startIndex: node.position.start, - endIndex: node.position.end, - }); + checkNode(node); + }, + // The ported parser emits `HtmlSelfClosingElement` for self-closed + // void tags such as ``, whereas the previous parser emitted + // `HtmlVoidElement` regardless of the trailing slash. Visit both so the + // check still fires on self-closing markup. + async HtmlSelfClosingElement(node) { + checkNode(node); }, }; }, diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidBooleanExpression.spec.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidBooleanExpression.spec.ts index 88af441fc..fc0afd32a 100644 --- a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidBooleanExpression.spec.ts +++ b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidBooleanExpression.spec.ts @@ -28,7 +28,7 @@ describe('detectTrailingAssignValue', async () => { [`{% assign foo = something == else %}`, '{% assign foo = something %}'], [`{% echo foo != bar %}`, '{% echo foo %}'], [`{{ this > that }}`, '{{ this }}'], - [`{{ bool and cond }}`, '{{ bool}}'], + [`{{ bool and cond }}`, '{{ bool }}'], ]; for (const [sourceCode, expected] of testCases) { diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidConditionalNode.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidConditionalNode.ts index a1a62ac83..ed20aa5ef 100644 --- a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidConditionalNode.ts +++ b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidConditionalNode.ts @@ -70,6 +70,16 @@ function isOperatorToken(token: Token): boolean { function checkInvalidStartingToken(tokens: Token[]): ExpressionIssue | null { const firstToken = tokens[0]; + // `contains` is the only word-operator in the comparison pattern that is + // also a valid identifier (the symbolic operators == != >= <= > < never + // are). When it stands alone it is a bare variable named `contains`, not + // the comparison operator, so it must not be flagged as an invalid + // starting token. This mirrors the previous parser, which produced a + // structured VariableLookup here; the ported parser falls back to string + // markup instead. + if (tokens.length === 1 && firstToken.value === 'contains') { + return null; + } if (firstToken.type === 'invalid' || firstToken.type === 'comparison') { return { message: `Conditional cannot start with '${firstToken.value}'. Use a variable or value instead`, diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidFilterName.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidFilterName.ts index ee98a086d..66458117c 100644 --- a/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidFilterName.ts +++ b/packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidFilterName.ts @@ -1,4 +1,10 @@ -import { LiquidVariableOutput, LiquidTag, NodeTypes, NamedTags } from '@shopify/liquid-html-parser'; +import { + LiquidVariableOutput, + LiquidTag, + LiquidVariable, + NodeTypes, + NamedTags, +} from '@shopify/liquid-html-parser'; import { Problem, SourceCodeType, Context, FilterEntry } from '../../..'; import { INVALID_SYNTAX_MESSAGE } from './utils'; @@ -11,30 +17,90 @@ export async function detectInvalidFilterName( } if (node.type === NodeTypes.LiquidVariableOutput) { - if (typeof node.markup !== 'string') { - return []; + // When the markup parses cleanly the parser hands us a structured + // `LiquidVariable`; only a bail-out leaves the raw markup as a string. + // Both carry the same offenses, they just live in different places. + if (typeof node.markup === 'string') { + return detectInvalidFilterNameInMarkup(node, node.markup, filters); } - return detectInvalidFilterNameInMarkup(node, node.markup, filters); + return detectInvalidFilterNameInFilters(node, node.markup, filters); } if (node.type === NodeTypes.LiquidTag) { - if (node.name === NamedTags.echo && typeof node.markup !== 'string') { - return []; - } - if (node.name === NamedTags.assign && typeof node.markup !== 'string') { - return []; + if (node.name === NamedTags.echo) { + if (typeof node.markup === 'string') { + return detectInvalidFilterNameInMarkup(node, node.markup, filters); + } + if (node.markup.type === NodeTypes.LiquidVariable) { + return detectInvalidFilterNameInFilters(node, node.markup, filters); + } } - if ( - typeof node.markup === 'string' && - (node.name === NamedTags.echo || node.name === NamedTags.assign) - ) { - return detectInvalidFilterNameInMarkup(node, node.markup, filters); + + if (node.name === NamedTags.assign) { + if (typeof node.markup === 'string') { + return detectInvalidFilterNameInMarkup(node, node.markup, filters); + } + if (node.markup.type === NodeTypes.AssignMarkup) { + return detectInvalidFilterNameInFilters(node, node.markup.value, filters); + } } } return []; } +// When the tokenizer meets an invalid trailing character (`@`, `!`, `#`, ...) +// it silently drops the byte, so the filter parses cleanly and the markup ends +// up as a structured `LiquidVariable` — the raw-markup regex above never sees +// it. The dropped byte still lives in the document `source` right after the +// filter name, so we recover the offense from the source span instead. +async function detectInvalidFilterNameInFilters( + node: LiquidVariableOutput | LiquidTag, + variable: LiquidVariable, + filters: FilterEntry[], +): Promise[]> { + const knownFilters = filters; + const source = node.source; + const markupEnd = variable.position.end; + const problems: Problem[] = []; + + for (const filter of variable.filters) { + if (!knownFilters.some((known) => known.name === filter.name)) { + continue; + } + + // `filter.position.start` sits before this filter's pipe, so the first + // occurrence of the name from there is this filter's name. + const nameStartInSource = source.indexOf(filter.name, filter.position.start); + if (nameStartInSource === -1) { + continue; + } + + const trailingStartInSource = nameStartInSource + filter.name.length; + + // Capture the run of characters wedged between the filter name and its + // next valid boundary (whitespace, `:` before arguments, `|` before the + // next filter, or the end of the markup). Anything there is junk. + const trailing = source.slice(trailingStartInSource, markupEnd).match(/^([^\s:|]+)/)?.[1]; + if (!trailing) { + continue; + } + + const trailingEndInSource = trailingStartInSource + trailing.length; + + problems.push({ + message: `${INVALID_SYNTAX_MESSAGE} Filter '${filter.name}' has trailing characters '${trailing}' that should be removed.`, + startIndex: trailingStartInSource, + endIndex: trailingEndInSource, + fix: (corrector) => { + corrector.replace(trailingStartInSource, trailingEndInSource, ''); + }, + }); + } + + return problems; +} + async function detectInvalidFilterNameInMarkup( node: LiquidVariableOutput | LiquidTag, markup: string, diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts index 566b273c8..0858f1b35 100644 --- a/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts +++ b/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.spec.ts @@ -54,7 +54,7 @@ describe('Module: LiquidHTMLSyntaxError', () => { const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( - `Attempting to close LiquidTag 'if' before HtmlElement 'a' was closed`, + `Attempting to close LiquidTag 'endif' before HtmlElement 'a' was closed`, ); }); @@ -65,7 +65,7 @@ describe('Module: LiquidHTMLSyntaxError', () => { const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode); expect(offenses).to.have.length(1); - expect(offenses[0].message).to.equal(`SyntaxError: expected "%}"`); + expect(offenses[0].message).to.equal(`Expected LiquidTagClose but got EndOfInput`); }); it('should report unexpected tokens (2)', async () => { @@ -75,7 +75,7 @@ describe('Module: LiquidHTMLSyntaxError', () => { const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode); expect(offenses).to.have.length(1); - expect(offenses[0].message).to.equal(`SyntaxError: expected ">", not """`); + expect(offenses[0].message).to.equal(`Expected HtmlTagClose but got EndOfInput`); }); it('should report unexpected tokens (3)', async () => { @@ -85,9 +85,7 @@ describe('Module: LiquidHTMLSyntaxError', () => { const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode); expect(offenses).to.have.length(1); - expect(offenses[0].message).to.equal( - `SyntaxError: expected "#", a letter, "when", "sections", "section", "render", "liquid", "layout", "increment", "include", "elsif", "else", "echo", "decrement", "content_for", "cycle", "continue", "break", "assign", "tablerow", "unless", "if", "ifchanged", "for", "case", "capture", "paginate", "form", "end", "style", "stylesheet", "schema", "javascript", "raw", "comment", or "doc"`, - ); + expect(offenses[0].message).to.equal(`Expected LiquidTagClose but got EndOfInput`); }); it('should not report syntax error in valid Liquid code', async () => { @@ -114,16 +112,16 @@ describe('Module: LiquidHTMLSyntaxError', () => { source = `
`; offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source); highlights = highlightedOffenses({ 'file.liquid': source }, offenses); - expect(highlights).to.include(''); + expect(highlights).to.include(''); source = ``; offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source); highlights = highlightedOffenses({ 'file.liquid': source }, offenses); - expect(highlights).to.include('{% endif %}'); + expect(highlights).to.include(''); source = ``; offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source); highlights = highlightedOffenses({ 'file.liquid': source }, offenses); - expect(highlights).to.include('"'); + expect(highlights).to.include('>'); }); }); diff --git a/packages/theme-check-common/src/checks/missing-template/index.ts b/packages/theme-check-common/src/checks/missing-template/index.ts index f1cda6a07..d2b3c4b67 100644 --- a/packages/theme-check-common/src/checks/missing-template/index.ts +++ b/packages/theme-check-common/src/checks/missing-template/index.ts @@ -58,7 +58,12 @@ export const MissingTemplate: LiquidCheckDefinition = { return { async RenderMarkup(node) { - if (node.snippet.type === NodeTypes.VariableLookup) return; + if ( + node.snippet.type === NodeTypes.VariableLookup || + node.snippet.type === NodeTypes.Range + ) { + return; + } const snippet = node.snippet; const relativePath = `snippets/${snippet.value}.liquid`; @@ -71,9 +76,14 @@ export const MissingTemplate: LiquidCheckDefinition = { if (node.name !== NamedTags.section) return; const markup = node.markup; - const relativePath = `sections/${markup.value}.liquid`; + // The ported parser wraps the section name in a `SectionMarkup` node + // whose name lives at `markup.name` (a String node); the previous + // parser exposed the name directly as `markup.value`. Read the name and + // report against the String node so the offense covers the quoted + // string rather than the whole markup. + const relativePath = `sections/${markup.name.value}.liquid`; - await maybeReportMissing(relativePath, markup); + await maybeReportMissing(relativePath, markup.name); }, }; }, diff --git a/packages/theme-check-common/src/checks/remote-asset/index.ts b/packages/theme-check-common/src/checks/remote-asset/index.ts index 31ff07da3..3baefd36d 100644 --- a/packages/theme-check-common/src/checks/remote-asset/index.ts +++ b/packages/theme-check-common/src/checks/remote-asset/index.ts @@ -1,5 +1,6 @@ import { HtmlRawNode, + HtmlSelfClosingElement, HtmlVoidElement, TextNode, LiquidVariable, @@ -14,7 +15,13 @@ import { LiquidHtmlNode, SchemaProp, } from '../../types'; -import { isAttr, isValuedHtmlAttribute, isNodeOfType, ValuedHtmlAttribute } from '../utils'; +import { + getHtmlNodeName, + isAttr, + isValuedHtmlAttribute, + isNodeOfType, + ValuedHtmlAttribute, +} from '../utils'; import { last } from '../../utils'; const RESOURCE_TAGS = ['img', 'link', 'source', 'script']; @@ -51,6 +58,10 @@ function isDataUri(url: string): boolean { return /^data:/i.test(url); } +function cleanUrlText(url: string): string { + return url.replace(/[“”‘’]/g, ''); +} + /** * Checks if the attribute value starts with a variable lookup. * When a value starts with a VariableLookup (e.g., {{ source.url }}, {{ image }}), @@ -89,8 +100,8 @@ function valueIsDefinitelyNotShopifyHosted( allowedDomains: string[] = [], ): boolean { return attr.value.some((node) => { - if (node.type === NodeTypes.TextNode && /^(https?:)?\/\//.test(node.value)) { - if (!isUrlHostedbyShopify(node.value, allowedDomains)) { + if (node.type === NodeTypes.TextNode && /^(https?:)?\/\//.test(cleanUrlText(node.value))) { + if (!isUrlHostedbyShopify(cleanUrlText(node.value), allowedDomains)) { return true; } } @@ -175,8 +186,9 @@ export const RemoteAsset: LiquidCheckDefinition = { create(context) { const allowedDomains = normaliseAllowedDomains(context.settings.allowedDomains || []); - function checkHtmlNode(node: HtmlVoidElement | HtmlRawNode) { - if (!RESOURCE_TAGS.includes(node.name)) return; + function checkHtmlNode(node: HtmlVoidElement | HtmlSelfClosingElement | HtmlRawNode) { + const nodeName = getHtmlNodeName(node); + if (!nodeName || !RESOURCE_TAGS.includes(nodeName)) return; const urlAttribute: ValuedHtmlAttribute | undefined = node.attributes .filter(isValuedHtmlAttribute) @@ -187,14 +199,14 @@ export const RemoteAsset: LiquidCheckDefinition = { const firstTextNode = urlAttribute.value.find( (node): node is TextNode => node.type === NodeTypes.TextNode, ); - if (firstTextNode && isHashUrl(firstTextNode.value)) return; - if (firstTextNode && isDataUri(firstTextNode.value)) return; + if (firstTextNode && isHashUrl(cleanUrlText(firstTextNode.value))) return; + if (firstTextNode && isDataUri(cleanUrlText(firstTextNode.value))) return; if (startsWithVariableLookup(urlAttribute)) return; const isShopifyUrl = urlAttribute.value .filter((node): node is TextNode => node.type === NodeTypes.TextNode) - .some((textNode) => isUrlHostedbyShopify(textNode.value, allowedDomains)); + .some((textNode) => isUrlHostedbyShopify(cleanUrlText(textNode.value), allowedDomains)); if (isShopifyUrl) return; @@ -265,6 +277,13 @@ export const RemoteAsset: LiquidCheckDefinition = { async HtmlVoidElement(node) { checkHtmlNode(node); }, + // The ported parser emits `HtmlSelfClosingElement` for self-closed + // void tags such as `` and ``, whereas the previous + // parser emitted `HtmlVoidElement` regardless of the trailing slash. + // Visit both so the check still fires on self-closing markup. + async HtmlSelfClosingElement(node) { + checkHtmlNode(node); + }, async HtmlRawNode(node) { checkHtmlNode(node); }, diff --git a/packages/theme-check-common/src/checks/unrecognized-render-snippet-arguments/index.ts b/packages/theme-check-common/src/checks/unrecognized-render-snippet-arguments/index.ts index e7588831d..16eef77e5 100644 --- a/packages/theme-check-common/src/checks/unrecognized-render-snippet-arguments/index.ts +++ b/packages/theme-check-common/src/checks/unrecognized-render-snippet-arguments/index.ts @@ -34,7 +34,7 @@ export const UnrecognizedRenderSnippetArguments: LiquidCheckDefinition = { const variable = node.variable; if (alias && !liquidDocParameters.has(alias.value) && variable) { - const startIndex = variable.position.start + 1; + const startIndex = variable.position.start; context.report({ message: `Unknown argument '${alias.value}' in render tag for snippet '${snippetName}'.`, @@ -45,7 +45,7 @@ export const UnrecognizedRenderSnippetArguments: LiquidCheckDefinition = { message: `Remove '${alias.value}'`, fix: (fixer: any) => { if (variable) { - return fixer.remove(variable.position.start, alias.position.end); + return fixer.remove(variable.position.start - 1, alias.position.end); } }, }, diff --git a/packages/theme-check-common/src/checks/utils.ts b/packages/theme-check-common/src/checks/utils.ts index 73a89500c..bb7a483c1 100644 --- a/packages/theme-check-common/src/checks/utils.ts +++ b/packages/theme-check-common/src/checks/utils.ts @@ -2,6 +2,9 @@ import { Position, NodeTypes, HtmlElement, + HtmlSelfClosingElement, + HtmlVoidElement, + HtmlRawNode, TextNode, AttrEmpty, AttrSingleQuoted, @@ -31,6 +34,25 @@ export function isLiquidBranch(node: LiquidHtmlNode): node is LiquidBranch { return isNodeOfType(NodeTypes.LiquidBranch, node); } +/** + * Returns the static tag name of an HTML node as a string. + * + * Void and raw nodes (``, `c'); + expectPath(ast, 'children').to.have.lengthOf(1); + expectPath(ast, 'children.0.type').to.eql('HtmlRawNode'); + expectPath(ast, 'children.0.name').to.eql('script'); + expectPath(ast, 'children.0.body.value').to.eql('ac'); + }); + it(`should parse a basic text node into a TextNode`, () => { for (const { toAST, expectPath, expectPosition } of testCases) { ast = toAST('Hello world!'); diff --git a/packages/liquid-html-parser/src/document/base.ts b/packages/liquid-html-parser/src/document/base.ts index e362275a7..6d9125017 100644 --- a/packages/liquid-html-parser/src/document/base.ts +++ b/packages/liquid-html-parser/src/document/base.ts @@ -54,6 +54,17 @@ export class ParserBase { return this.source; } + /** + * Whether this document parse is tolerant. The strict/default parser returns + * false; `TolerantDocumentParser` overrides it to true. Markup-construction + * sites consult this to enable the markup parser's tolerant recovery axis + * (`enableTolerant()`), leaving strict (and theme-check) parses untouched. + * Disjoint from the render-tree lax axis. + */ + isTolerant(): boolean { + return false; + } + tokenAt(index: number): Token { return this.tokens[index]; } diff --git a/packages/liquid-html-parser/src/document/factories.ts b/packages/liquid-html-parser/src/document/factories.ts index 38aba5ff7..1640e6f5b 100644 --- a/packages/liquid-html-parser/src/document/factories.ts +++ b/packages/liquid-html-parser/src/document/factories.ts @@ -105,12 +105,15 @@ export function makeLiquidTagBaseCase( blockEndPosition?: Position, delimiterWhitespace?: { start: LiquidOpenWhitespace; end: LiquidCloseWhitespace }, reason?: string, + // `#`-comment lines in `{% liquid %}` keep their inner indentation verbatim; + // every other base-case tag trims (the default). + preserveMarkup: boolean = false, ): LiquidTagBaseCase { const posEnd = blockEndPosition ? blockEndPosition.end : envelope.blockStartPosition.end; return { type: NodeTypes.LiquidTag, name: envelope.tagName, - markup: envelope.markupString.trim(), + markup: preserveMarkup ? envelope.markupString : envelope.markupString.trim(), children, whitespaceStart: envelope.whitespaceStart, whitespaceEnd: envelope.whitespaceEnd, diff --git a/packages/liquid-html-parser/src/document/html.ts b/packages/liquid-html-parser/src/document/html.ts index 18ad20c67..a296519f4 100644 --- a/packages/liquid-html-parser/src/document/html.ts +++ b/packages/liquid-html-parser/src/document/html.ts @@ -217,11 +217,15 @@ export function parseHtmlComment(parser: HtmlParserDelegate): HtmlComment { const openToken = parser.consume(TokenType.HtmlCommentOpen); const bodyStart = openToken.end; - while (!parser.isAtEnd() && !parser.check(TokenType.HtmlCommentClose)) { - parser.advance(); - } - - if (parser.isAtEnd()) { + // Find the closing `-->` by scanning the source rather than walking tokens. + // A conditional comment body (``) contains + // `` as an HtmlTagClose, so no + // HtmlCommentClose token is ever emitted and the token walk would run to EOF. + // A source scan pins us to the real comment close, so the (unchanged) + // HtmlComment node feeds getConditionalComment correctly in the printer. + const closeIdx = source.indexOf('-->', bodyStart); + if (closeIdx === -1) { throw new LiquidHTMLASTParsingError( `Attempting to end parsing before HtmlComment '` const body = source.slice(bodyStart, bodyEnd).trim(); - return makeHtmlComment(body, openToken.start, closeToken.end, source); + parser.seekToSourceOffset(commentEnd); + + return makeHtmlComment(body, openToken.start, commentEnd, source); } // htmlDoctype := "" @@ -499,7 +505,11 @@ function parseAttributeList(parser: HtmlParserDelegate): AttributeNode[] { const attrEnd = closeQuote.end; const attributePosition: Position = { start: valueStart, end: valueEnd }; - if (quoteChar === '"') { + // Double straight quote and double curly quotes (“ ”) map to a + // double-quoted attr; single straight quote and single curly quotes + // (‘ ’) map to a single-quoted attr. The printer normalizes the curly + // variants to straight quotes. + if (quoteChar === '"' || quoteChar === '“' || quoteChar === '”') { attrs.push( makeAttrDoubleQuoted(name, value, attributePosition, attrStart, attrEnd, source), ); @@ -738,13 +748,39 @@ export function scanForHtmlCloseTag(parser: ParserBase, tagName: string): number const tokenCount = parser.tokenCount(); const pos = parser.getPosition(); + // Depth-balance nested same-name elements so the OUTER close tag is + // returned, not the first inner one (e.g. `………`). A + // nested open tag is an `HtmlTagOpen` followed by a `Text` token whose first + // word matches the tag name — the tokenizer folds the tag name and any + // trailing attributes into a single text token, so we take the first word. + // The scan begins after the outer open tag has been consumed, so the outer + // open is never counted. Mirrors `scanForEndTagNested`. + let depth = 0; for (let i = pos; i < tokenCount; i++) { - if (parser.tokenAt(i).type !== TokenType.HtmlCloseTagOpen) continue; + const token = parser.tokenAt(i); + + if (token.type === TokenType.HtmlTagOpen) { + const textIdx = i + 1; + if (textIdx >= tokenCount) continue; + if (parser.tokenAt(textIdx).type !== TokenType.Text) continue; + const trimmed = source + .slice(parser.tokenAt(textIdx).start, parser.tokenAt(textIdx).end) + .trimStart(); + const firstWs = trimmed.search(/\s/); + const name = firstWs === -1 ? trimmed.trim() : trimmed.slice(0, firstWs); + if (name.toLowerCase() === lowerName) depth++; + continue; + } + + if (token.type !== TokenType.HtmlCloseTagOpen) continue; const textIdx = i + 1; if (textIdx >= tokenCount) continue; if (parser.tokenAt(textIdx).type !== TokenType.Text) continue; const text = source.slice(parser.tokenAt(textIdx).start, parser.tokenAt(textIdx).end); - if (text.trim().toLowerCase() === lowerName) return i; + if (text.trim().toLowerCase() === lowerName) { + if (depth === 0) return i; + depth--; + } } return -1; } @@ -768,6 +804,7 @@ function scriptKindFromAttributes(attributes: AttributeNode[]): RawMarkupKinds { const typeValue = extractPlainAttributeValue(attributes, 'type'); if (typeValue === null) return RawMarkupKinds.javascript; if (typeValue === 'text/html') return RawMarkupKinds.html; + if (typeValue === 'text/markdown') return RawMarkupKinds.markdown; if ( typeValue.endsWith('json') || typeValue.endsWith('importmap') || diff --git a/packages/liquid-html-parser/src/document/liquid-blocks.ts b/packages/liquid-html-parser/src/document/liquid-blocks.ts index 470c4ac37..ba08d651e 100644 --- a/packages/liquid-html-parser/src/document/liquid-blocks.ts +++ b/packages/liquid-html-parser/src/document/liquid-blocks.ts @@ -68,6 +68,7 @@ export function parseBlockTag( const markupStringStart = closeToken.start - envelope.markupString.length; const tokens = tokenizeMarkup(envelope.markupString, markupStringStart); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); markup = def.parse(envelope.tagName, markupParser, parser); if (!markupParser.isAtEnd()) { markup = undefined; @@ -341,6 +342,7 @@ export function parseBranchMarkup( try { const tokens = tokenizeMarkup(envelope.markupString, markupStringStart); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); const result = elsifBranchParse(branchName, markupParser); if (!markupParser.isAtEnd()) return envelope.markupString.trim(); return result; @@ -352,6 +354,7 @@ export function parseBranchMarkup( try { const tokens = tokenizeMarkup(envelope.markupString, markupStringStart); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); const result = whenBranchParse(branchName, markupParser); if (!markupParser.isAtEnd()) return envelope.markupString.trim(); return result; diff --git a/packages/liquid-html-parser/src/document/liquid-hybrid.ts b/packages/liquid-html-parser/src/document/liquid-hybrid.ts index 4262f2dc3..f855efa19 100644 --- a/packages/liquid-html-parser/src/document/liquid-hybrid.ts +++ b/packages/liquid-html-parser/src/document/liquid-hybrid.ts @@ -31,6 +31,7 @@ export function parseHybridTag( const markupStringStart = closeToken.start - envelope.markupString.length; const tokens = tokenizeMarkup(envelope.markupString, markupStringStart); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); markup = def.parse(envelope.tagName, markupParser, parser); if (!markupParser.isAtEnd()) { markup = undefined; diff --git a/packages/liquid-html-parser/src/document/liquid-lines.test.ts b/packages/liquid-html-parser/src/document/liquid-lines.test.ts index 0741ef38a..e5c1729cb 100644 --- a/packages/liquid-html-parser/src/document/liquid-lines.test.ts +++ b/packages/liquid-html-parser/src/document/liquid-lines.test.ts @@ -112,6 +112,14 @@ describe('Unit: liquid-lines', () => { expectPath(ast, 'children.0.markup.1.name').to.eql('echo'); }); + it('should preserve inner indentation after `#`, stripping only one separator space (Category J)', () => { + // `# fancy` must keep four spaces of the ASCII-art indent — only a + // single separator space after `#` is stripped (ohm `"#" space?`). + const ast = toLiquidHtmlAST('{% liquid\n# fancy\necho "hi"\n%}'); + expectPath(ast, 'children.0.markup.0.name').to.eql('#'); + expectPath(ast, 'children.0.markup.0.markup').to.eql(' fancy'); + }); + it('should skip empty lines', () => { const ast = toLiquidHtmlAST('{% liquid\n\necho "hi"\n\n%}'); expectPath(ast, 'children.0.markup').to.have.lengthOf(1); @@ -226,4 +234,33 @@ describe('Unit: liquid-lines', () => { expectPath(ast, `${branch}.blockEndPosition.end`).to.eql(source.indexOf('endif')); }); }); + + describe('nested comment/doc balancing (Category B)', () => { + it('should balance nested comments so the outer endcomment closes the block', () => { + // The inner `comment`/`endcomment` pair must not close the outer + // block early; the outer comment ends at the final `endcomment`. + const source = '{% liquid\ncomment\nouter\ncomment\ninner\nendcomment\nendcomment\n%}'; + const ast = toLiquidHtmlAST(source); + expectPath(ast, 'children.0.markup').to.have.lengthOf(1); + expectPath(ast, 'children.0.markup.0.type').to.eql('LiquidRawTag'); + expectPath(ast, 'children.0.markup.0.name').to.eql('comment'); + expectPath(ast, 'children.0.markup.0.body.value').to.eql( + 'outer\ncomment\ninner\nendcomment\n', + ); + }); + + it('should carve out a nested raw so a literal endcomment inside it does not close early', () => { + // The `endcomment` line sits inside a nested `raw`…`endraw` block, so it + // must be ignored by the depth scan; the outer comment closes at the + // real trailing `endcomment`. + const source = '{% liquid\ncomment\nbefore\nraw\nendcomment\nendraw\nafter\nendcomment\n%}'; + const ast = toLiquidHtmlAST(source); + expectPath(ast, 'children.0.markup').to.have.lengthOf(1); + expectPath(ast, 'children.0.markup.0.type').to.eql('LiquidRawTag'); + expectPath(ast, 'children.0.markup.0.name').to.eql('comment'); + expectPath(ast, 'children.0.markup.0.body.value').to.eql( + 'before\nraw\nendcomment\nendraw\nafter\n', + ); + }); + }); }); diff --git a/packages/liquid-html-parser/src/document/liquid-lines.ts b/packages/liquid-html-parser/src/document/liquid-lines.ts index 92499f105..f00cf46a1 100644 --- a/packages/liquid-html-parser/src/document/liquid-lines.ts +++ b/packages/liquid-html-parser/src/document/liquid-lines.ts @@ -64,7 +64,9 @@ export function parseLiquidStatement( const envelope = envelopeFromLine(line, parser.getSource()); if (tagName === '#') { - return makeLiquidTagBaseCase(envelope); + // Preserve the inline comment's inner indentation (`preserveMarkup`); only + // the single separator space after `#` was stripped upstream. + return makeLiquidTagBaseCase(envelope, undefined, undefined, undefined, undefined, true); } if (tagName.startsWith('end')) { @@ -93,6 +95,7 @@ export function parseLiquidStatement( markupOffset, envelope.markupEnd, ); + if (parser.isTolerant()) markupParser.enableTolerant(); const markup = def.parse(tagName, markupParser, parser); if (!markupParser.isAtEnd()) { @@ -140,6 +143,7 @@ export function parseLineBlockTag( try { const tokens = tokenizeMarkup(markupString, markupOffset); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); markup = def.parse(envelope.tagName, markupParser, parser); if (!markupParser.isAtEnd()) { @@ -196,11 +200,41 @@ 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; - for (let i = ctx.index; i < ctx.lines.length; i++) { - if (ctx.lines[i].tagName === endTagName) { - endLineIndex = i; - break; + 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; + } } } @@ -262,6 +296,7 @@ export function parseLineHybridTag( try { const tokens = tokenizeMarkup(markupString, markupOffset); const markupParser = new MarkupParser(tokens, parser.getSource()); + if (parser.isTolerant()) markupParser.enableTolerant(); markup = def.parse(envelope.tagName, markupParser, parser); if (!markupParser.isAtEnd()) { @@ -385,6 +420,7 @@ export function parseLineBranchedBody( line.tagName as BranchName, branchEnvelope, parser.getSource(), + parser.isTolerant(), ); currentBranch = makeLiquidBranchNamed(branchEnvelope, branchMarkup); currentChildren = []; @@ -423,12 +459,14 @@ export function parseLineBranchMarkup( branchName: BranchName, envelope: LiquidTagEnvelope, source: string, + tolerant: boolean = false, ): unknown { switch (branchName) { case 'elsif': { try { const tokens = tokenizeMarkup(envelope.markupString, envelope.markupOffset); const markupParser = new MarkupParser(tokens, source); + if (tolerant) markupParser.enableTolerant(); const result = elsifBranchParse(branchName, markupParser); if (!markupParser.isAtEnd()) return envelope.markupString.trim(); @@ -441,6 +479,7 @@ export function parseLineBranchMarkup( try { const tokens = tokenizeMarkup(envelope.markupString, envelope.markupOffset); const markupParser = new MarkupParser(tokens, source); + if (tolerant) markupParser.enableTolerant(); const result = whenBranchParse(branchName, markupParser); if (!markupParser.isAtEnd()) return envelope.markupString.trim(); diff --git a/packages/liquid-html-parser/src/document/liquid-raw.ts b/packages/liquid-html-parser/src/document/liquid-raw.ts index dd136716a..56690aad5 100644 --- a/packages/liquid-html-parser/src/document/liquid-raw.ts +++ b/packages/liquid-html-parser/src/document/liquid-raw.ts @@ -188,6 +188,14 @@ export function scanForEndTag(parser: ParserBase, endTagName: string): EndTagSca 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; @@ -200,6 +208,56 @@ export function scanForEndTag(parser: ParserBase, endTagName: string): EndTagSca 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/liquid-tags.ts b/packages/liquid-html-parser/src/document/liquid-tags.ts index e23cfff21..604761a90 100644 --- a/packages/liquid-html-parser/src/document/liquid-tags.ts +++ b/packages/liquid-html-parser/src/document/liquid-tags.ts @@ -98,6 +98,7 @@ function parseStandaloneTag( markupStringStart, markupStringEnd, ); + if (parser.isTolerant()) markupParser.enableTolerant(); const markup = def.parse(envelope.tagName, markupParser, parser); if (!markupParser.isAtEnd()) { return makeLiquidTagBaseCase( diff --git a/packages/liquid-html-parser/src/document/liquid-variable-output.ts b/packages/liquid-html-parser/src/document/liquid-variable-output.ts index d7af5452a..e3ad2f7f2 100644 --- a/packages/liquid-html-parser/src/document/liquid-variable-output.ts +++ b/packages/liquid-html-parser/src/document/liquid-variable-output.ts @@ -17,6 +17,7 @@ export function parseLiquidVariableOutput(parser: ParserBase): LiquidVariableOut try { const tokens = tokenizeMarkup(rawMarkup, openToken.end); const markupParser = new MarkupParser(tokens, source); + if (parser.isTolerant()) markupParser.enableTolerant(); const liquidVariable = markupParser.liquidVariable(); if (!markupParser.isAtEnd()) { diff --git a/packages/liquid-html-parser/src/document/tokenizer.ts b/packages/liquid-html-parser/src/document/tokenizer.ts index f32b2ac1d..dcef56ec4 100644 --- a/packages/liquid-html-parser/src/document/tokenizer.ts +++ b/packages/liquid-html-parser/src/document/tokenizer.ts @@ -250,7 +250,18 @@ export function tokenize(source: string, options: TokenizeOptions = {}): Token[] continue; } - if (ch(0) === '"' || ch(0) === "'") { + // Accept straight quotes and curly (smart) quotes as attribute-value + // openers. Curly quotes get normalized to straight quotes downstream; + // recognizing them here (HTML scope only — Liquid tokenization is + // untouched) stops the value from splitting at interior spaces. + if ( + ch(0) === '"' || + ch(0) === "'" || + ch(0) === '“' || + ch(0) === '”' || + ch(0) === '‘' || + ch(0) === '’' + ) { quoteChar = ch(0); emit(TokenType.HtmlQuoteOpen, 1); pushMode(Mode.QuotedValue); @@ -265,7 +276,10 @@ export function tokenize(source: string, options: TokenizeOptions = {}): Token[] case Mode.QuotedValue: { if (scanLiquidOpen()) continue; - if (ch(0) === quoteChar) { + // Curly quotes are directional, so a value opened with a left curly + // quote closes on its right partner (and vice-versa); straight quotes + // close on themselves. + if (ch(0) === closingQuoteFor(quoteChar) || ch(0) === quoteChar) { emit(TokenType.HtmlQuoteClose, 1); popMode(); continue; @@ -293,3 +307,21 @@ enum Mode { LiquidTag = 'LiquidTag', LiquidVariableOutput = 'LiquidVariableOutput', } + +// Curly (smart) quotes come in directional pairs: a value opened with a left +// curly quote closes on its right partner and vice-versa. Straight quotes are +// their own partner, so they close on an identical character. +function closingQuoteFor(open: string): string { + switch (open) { + case '“': + return '”'; + case '”': + return '“'; + case '‘': + return '’'; + case '’': + return '‘'; + default: + return open; + } +} diff --git a/packages/liquid-html-parser/src/document/tolerant-parser.ts b/packages/liquid-html-parser/src/document/tolerant-parser.ts index 465c1452e..f942cd61f 100644 --- a/packages/liquid-html-parser/src/document/tolerant-parser.ts +++ b/packages/liquid-html-parser/src/document/tolerant-parser.ts @@ -62,6 +62,14 @@ export function makeLiquidErrorNode( * different entry point and is left byte-identical. */ export class TolerantDocumentParser extends DocumentParser { + /** + * Marks this parse as tolerant so the markup-construction sites enable the + * markup parser's tolerant recovery axis. Disjoint from lax. + */ + isTolerant(): boolean { + return true; + } + /* * Wraps the polymorphic node parse. On a LiquidHTMLASTParsingError it emits a * LiquidErrorNode covering the skipped region and resynchronizes onto the next diff --git a/packages/liquid-html-parser/src/liquid-doc/parser.ts b/packages/liquid-html-parser/src/liquid-doc/parser.ts index 364a418a9..b0bb4bec0 100644 --- a/packages/liquid-html-parser/src/liquid-doc/parser.ts +++ b/packages/liquid-html-parser/src/liquid-doc/parser.ts @@ -119,6 +119,18 @@ export class LiquidDocParser { const annotationToken = this.consume(LiquidDocTokenType.Annotation); const name = annotationToken.value; + // `@descriptionText` — no space between `@description` and its content. The + // level-1 tokenizer greedily folds the leading text into the annotation + // name (`descriptionText`), so recognize the `description` prefix here and + // treat the glued remainder as the start of the inline description content. + // This matches the pre-swap parser, which printed `@description Text`. + if ( + name !== LiquidDocAnnotation.Description && + name.startsWith(LiquidDocAnnotation.Description) + ) { + return this.parseGluedDescription(annotationToken); + } + if (!isKnownAnnotation(name)) { return this.parseUnsupportedAnnotation(annotationToken); } @@ -282,6 +294,36 @@ export class LiquidDocParser { ); } + // gluedDescription := "@description" gluedContent (no separating space) + // + // The glued suffix and any same-line text that follows are one inline + // description. Content is read straight from source (not by concatenating + // token values) so the original spacing between the suffix and the trailing + // text is preserved — the suffix's own token boundary is mid-word. + private parseGluedDescription(annotationToken: LiquidDocToken): LiquidDocDescriptionNode { + const start = annotationToken.start; + // Source offset immediately past the literal `@description`; the glued + // remainder (e.g. `This` in `@descriptionThis`) begins here. + const contentStart = start + 1 + LiquidDocAnnotation.Description.length; + + const inlineToken = this.accept(LiquidDocTokenType.TextLine); + const { value: rest, endPos } = this.collectContentLines(); + const hasRest = rest.trim().length > 0; + + let end: number; + if (hasRest) end = endPos; + else if (inlineToken) end = inlineToken.end; + else end = annotationToken.end; + + const content = makeTextNode( + this.source.slice(contentStart, end), + contentStart, + end, + this.source, + ); + return makeLiquidDocDescriptionNode(content, false, true, start, end, this.source); + } + // example := "@example" content private parseExample(annotationToken: LiquidDocToken): LiquidDocExampleNode { const start = annotationToken.start; diff --git a/packages/liquid-html-parser/src/markup/parser.ts b/packages/liquid-html-parser/src/markup/parser.ts index a5a1c5961..de78f751b 100644 --- a/packages/liquid-html-parser/src/markup/parser.ts +++ b/packages/liquid-html-parser/src/markup/parser.ts @@ -52,6 +52,14 @@ export class MarkupParser { * `liquid-render-tree/src/lax-recover.ts`. */ private lax: boolean; + /** + * Tolerant recovery axis, DISJOINT from `lax`. Enabled exclusively by the + * tolerant document parser (`TolerantDocumentParser`) so the formatter can + * build a best-effort tag/markup node instead of discarding markup. It never + * affects strict parsing (`toLiquidHtmlAST` / `theme-check`) and it does NOT + * change lax (Ruby-render-parity) behavior. + */ + private tolerant: boolean; /** * True while parsing a condition (`if`/`unless`/`elsif`). In lax mode this * permits stripping meaningless grouping parens (`(false || true)`), which is @@ -69,6 +77,7 @@ export class MarkupParser { this.markupStart = markupStart ?? tokens[0]?.start ?? 0; this.markupEnd = markupEnd ?? this.computeLastTokenEnd(); this.lax = false; + this.tolerant = false; this.inCondition = false; } @@ -86,6 +95,21 @@ export class MarkupParser { return this.lax; } + /** Enables tolerant recovery for subsequent parse calls. Returns `this` for + * chaining. Only the tolerant document parser calls this. Disjoint from + * lax — enabling one does not enable the other. */ + enableTolerant(): this { + this.tolerant = true; + return this; + } + + /** True when tolerant recovery is enabled. Tag/markup parse callbacks consult + * this to build a best-effort node instead of discarding markup, without + * affecting strict or lax parsing. */ + isTolerant(): boolean { + return this.tolerant; + } + /** Returns the raw source text from the current token up to (but excluding) * the token whose type is `stop` (or end of markup), trimmed. Advances the * cursor past everything consumed. Used by lax recovery to capture an @@ -1173,7 +1197,7 @@ export class MarkupParser { const result: LiquidFilter[] = []; while (this.consumeOptional(MarkupTokenType.Pipe)) { // Lax: a `|` not followed by a filter name (e.g. trailing `|`) is dropped. - if (this.lax && !this.look(MarkupTokenType.Id)) { + if ((this.lax || this.tolerant) && !this.look(MarkupTokenType.Id)) { this.skipToNextPipe(); continue; } @@ -1199,7 +1223,7 @@ export class MarkupParser { if (this.consumeOptional(MarkupTokenType.Colon)) { // Lax: a colon with no following argument (`upcase:`) is tolerated; only // parse arguments when something argument-like actually follows. - if (!this.lax || this.atArgumentStart()) { + if (!(this.lax || this.tolerant) || this.atArgumentStart()) { args = this.arguments(); } if (args.length > 0) { @@ -1259,7 +1283,7 @@ export class MarkupParser { while (this.consumeOptional(MarkupTokenType.Comma)) { // Lax: a trailing or empty comma (`append: "x",`) ends the argument list // rather than forcing another argument parse. - if (this.lax && !this.atArgumentStart()) { + if ((this.lax || this.tolerant) && !this.atArgumentStart()) { break; } args.push(this.argument()); diff --git a/packages/liquid-html-parser/src/tags/liquid.ts b/packages/liquid-html-parser/src/tags/liquid.ts index c2b1e8554..a0d2c4592 100644 --- a/packages/liquid-html-parser/src/tags/liquid.ts +++ b/packages/liquid-html-parser/src/tags/liquid.ts @@ -78,7 +78,11 @@ function parseLines(body: string, bodyStart: number): LiquidLine[] { if (firstLineTrimmed.startsWith('#')) { tagName = '#'; - markup = trimmed.slice(1).trimStart(); + // Strip at most one separator whitespace after `#`, mirroring the + // pre-swap ohm grammar `"#" space?`. Trimming all leading whitespace + // would collapse intentional indentation in `#`-comment art such as + // `# fancy`, which the printer re-pads with a single space. + markup = trimmed.slice(1).replace(/^[ \t]/, ''); nameOffset = startIndex + leadingWs; markupOffset = startIndex + leadingWs + 1 + (trimmed.length - 1 - markup.length); } else { diff --git a/packages/liquid-html-parser/src/tags/paginate.ts b/packages/liquid-html-parser/src/tags/paginate.ts index 8bd707721..9d8422bcb 100644 --- a/packages/liquid-html-parser/src/tags/paginate.ts +++ b/packages/liquid-html-parser/src/tags/paginate.ts @@ -19,6 +19,12 @@ export const paginateTag: TagDefinitionBlock = { const args: LiquidNamedArgument[] = []; if (markup.consumeOptional(MarkupTokenType.Comma)) { args.push(...markup.namedArguments()); + } else if (markup.isTolerant() && markup.peek().type === MarkupTokenType.Id) { + // Tolerant: paginate accepts whitespace-separated named attrs with no + // leading comma (`... by N window_size: 50`). Consume them so the tag + // builds as PaginateMarkup instead of leaving leftover tokens. Strict is + // unchanged. + args.push(...markup.namedArguments()); } return { diff --git a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts index ae147fc98..3e750ea66 100644 --- a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts +++ b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts @@ -442,7 +442,15 @@ export function printLiquidTag( let body: Doc = []; - if (isBranchedTag(node)) { + /* + * `tablerow` is not a branched tag, but like `for` it wraps its body in a + * single default `LiquidBranch`. Routing it through `printChildren` (the + * non-branched path below) would indent that branch twice and prepend a + * blank line; the branched-path `path.map` prints the default branch with + * `for`'s single-indent, no-leading-blank shape. This is a printer-only + * special-case — `tablerow` is deliberately kept out of `isBranchedTag`. + */ + if (isBranchedTag(node) || node.name === 'tablerow') { body = cleanDoc( path.map( (p) => @@ -569,13 +577,13 @@ export function printLiquidDocParam( const parts: Doc[] = ['@param']; if (node.paramType?.value) { - parts.push(' ', `{${node.paramType.value}}`); + parts.push(' ', `{${node.paramType.value.trim()}}`); } if (node.required) { - parts.push(' ', node.paramName.value); + parts.push(' ', node.paramName.value.trim()); } else { - parts.push(' ', `[${node.paramName.value}]`); + parts.push(' ', `[${node.paramName.value.trim()}]`); } if (node.paramDescription?.value) { diff --git a/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid b/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid index aff739a0f..8d2ad5a9d 100644 --- a/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/embed-script-tag/fixed.liquid @@ -1,4 +1,4 @@ -It should pretty print pure JS inside a script tag +It should pretty pring pure JS inside a script tag printWidth: 20
diff --git a/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid b/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid index 79708ea87..1dced10b4 100644 --- a/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/html-attributes/fixed.liquid @@ -70,7 +70,7 @@ It should break and indent attributes that are long af If there's only one attribute and self-closing, don't break when the line is too long. -If there's only one attribute, not self-closing and no children, break the attr and close the tag on a different line. +If there's only one attribute and not self-closing, break the attr and close the tag on a different line.
@@ -81,19 +81,17 @@ If there's only one attribute and not self-closing, but with children, do not br
If there's only one attribute, not self-closing, with children, do not break the LiquidTag attr and close the tag on a different line. -printWidth: 30
Hello
If there's only one attribute, not self-closing, with children, do not break the LiquidNode attr and close the tag on a different line. -printWidth: 30
Hello
If there's only one attribute, not self closing, with children, but the attribute is a liquid tag that has children, break it normally. -printWidth: 40 +printWidth: 30
If there's only one attribute, not self closing, without children, but the attribute is a liquid tag that has children, break it normally. -printWidth: 40 +printWidth: 30
It should pretty print smart quotes into dumb quotes to prevent copy pasting errors -

+

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 2237d93a6..c69c34c87 100644 --- a/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/html-comment/fixed.liquid @@ -1,18 +1,15 @@ -it should format as expected (chunk 0) -it should format as expected (chunk 1) -it should format as expected (chunk 2) It should change the css display of the next node @@ -24,52 +21,51 @@ printWidth: 1 It should preserve MSO conditional comments -It should support the IE Comment '&' operators +It should support the IE Comment '&' operator -It should support the IE Comment '|' operators +It should support the IE Comment '|' operator It should support and indend the bodies of all IE comment variations -printWidth: 60 diff --git a/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid b/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid index f834b0dac..24d5e5f48 100644 --- a/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/issue-154/fixed.liquid @@ -1,4 +1,3 @@ -it should format as expected (chunk 0) {%- liquid layout none @@ -6,7 +5,7 @@ it should format as expected (chunk 0) render 'product-item', product: item endpaginate - paginate search.results by 28 window_size:1 + paginate search.results by 28, window_size: 1 render 'product-item', product: item endpaginate -%} diff --git a/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid b/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid index 1c8c895f2..718cbb472 100644 --- a/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/issue-156/fixed.liquid @@ -1,9 +1,7 @@ -it should format as expected (chunk 0) diff --git a/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid index dfc861243..3cff9b772 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-comment/fixed.liquid @@ -1,4 +1,3 @@ -it should format as expected (chunk 0) {% comment %} This gets reindented, but we don't parse the content. So stuff can be unclosed and it'll be fine. @@ -12,23 +11,22 @@ It should support nested comments ho {% comment %} hi + {% endcomment %} + ha {% endcomment %} -ha -{%endcomment%} It should support empty comments {% comment %}{% endcomment %} It should support nested empty comments -{% comment %}{% comment %}{% endcomment %} -{% endcomment %} +{% comment %}{% comment %}{% endcomment %}{% endcomment %} It should support nested empty comments (pt 2) {% comment %} hi {% comment %} -{% endcomment %} -ho + {% endcomment %} + ho {% endcomment %} It should support inline comments diff --git a/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid index 35b6caa39..2adb04aa5 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid @@ -10,7 +10,7 @@ It should trim whitespace between nodes It should normalize the param type {% doc %} - @param { string } paramName - param with description + @param {string} paramName - param with description {% enddoc %} It should format the param description with a dash separator @@ -19,6 +19,7 @@ It should format the param description with a dash separator {% enddoc %} It should respect the liquidDocParamDash option liquidDocParamDash: false +liquidDocParamDash: false {% doc %} @param paramName param with description {% enddoc %} @@ -30,10 +31,10 @@ It should normalize the param description It should strip whitespace between optional param delimiters {% doc %} - @param [ paramName ] - param with description + @param [paramName] - param with description {% enddoc %} -It should handle params with malformed optional delimiters +It should not break params with malformed optional delimiters {% doc %} @param [missingTail - param with description @param missingHead - ] - param with description @@ -98,7 +99,7 @@ It should push multi-line description content to the next line It should add a space between a description tag and content {% doc %} - @descriptionThis is a description + @description This is a description {% enddoc %} It should handle param, example, and description nodes diff --git a/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid index c77e0827a..5e087b890 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-drop-filters/fixed.liquid @@ -48,7 +48,7 @@ printWidth: 1 key2: val2 }} -It should support a mix of positional and named arguments as expected (p50) +It should support a mix of positional and named arguments as expected (p40) printWidth: 50 {{ x @@ -59,7 +59,7 @@ It should support a mix of positional and named arguments as expected (p80) printWidth: 80 {{ x | filter1: pos1, pos2, key1: val1, key2: val2 }} -It should support all types of arguments, and add spaces +It should support all types of arguments {{ x | f: 'string' }} {{ x | f: 'string' }} {{ x | f: 0.0 }} @@ -81,4 +81,4 @@ It should support spaces or lack of thereof erywhere {{ x | f: a: b, range: (a..b) }} It should strip trailing commas by default -{{ x|split:'', }} +{{ x | split: '' }} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid index b2be286fe..a63c8c919 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-liquid-tag/fixed.liquid @@ -1,4 +1,3 @@ -it should format as expected (chunk 0)
{% liquid assign x = 1 @@ -37,19 +36,19 @@ It should handle comments correctly It should handle nested comments correctly {%- liquid comment - how u doin? - comment - ho - endcomment + how u doin? + comment + ho + endcomment endcomment -%} It should handle empty nested comments correctly {%- liquid comment - how u doin? - comment - endcomment + how u doin? + comment + endcomment endcomment -%} @@ -64,7 +63,7 @@ It should handle inline comments correctly It should pretty print #### lines without a space {%- liquid ############# - # fancy + # fancy ############# -%} @@ -118,7 +117,7 @@ printWidth: 1 section 'string' layout 'string' - paginate col by 10 window_size:50 + paginate col by 10, window_size: 50 render 'icon' with x as bar, foo: bar endpaginate %} @@ -138,10 +137,10 @@ It should keep newlines between tags It should keep leading whitespace in comments {% liquid # This list should not be reindented: - # - a - # - b - # - d - # - c + # - a + # - b + # - d + # - c %} It should not freak out on variable output characters inside conditions diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid index bff3aba68..f016b0b3a 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-paginate/fixed.liquid @@ -11,13 +11,16 @@ printWidth: 1 It is undocumented, but it also accepts named attributes (window_size). Those are comma separated. printWidth: 80 -{% paginate collection by pageSize window_size:50, attr:(0.. 2) %} +{% paginate collection by pageSize, window_size: 50, attr: (0..2) %} {{ collection }} {% endpaginate %} It is undocumented, but it also accepts named attributes (window_size). We print those comma separated and on a new line when it breaks. printWidth: 1 -{% paginate collection by pageSize window_size: 50 %} +{% paginate collection + by pageSize, + window_size: 50 +%} {{ collection }} {% endpaginate %} diff --git a/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid b/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid index 7ab48959c..997a3d9c4 100644 --- a/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid +++ b/packages/prettier-plugin-liquid/src/test/liquid-tag-tablerow/fixed.liquid @@ -4,8 +4,7 @@ It takes an element and a collection It does not breaks on in collection printWidth: 1 {% tablerow element in collection %} - - {{ collection }} + {{ collection }} {% endtablerow %} It breaks on reversed @@ -13,8 +12,7 @@ printWidth: 1 {% tablerow element in collection reversed %} - - {{ element }} + {{ element }} {% endtablerow %} It breaks on tag arguments @@ -23,18 +21,16 @@ printWidth: 1 limit: 10 offset: 10 %} - - {{ element }} + {{ element }} {% endtablerow %} -It breaks on both reversed and tag arguments +It breaks on both reversed and tag argumentsc printWidth: 1 {% tablerow element in collection reversed limit: 10 %} - - {{ element }} + {{ element }} {% endtablerow %} It only breaks if the printWidth is hit From 6ff25c64de9e708dbfcfe1b0737cf73b207bdfb6 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 17 Jul 2026 13:19:49 +0200 Subject: [PATCH 3/4] Add changeset for hand-written liquid-html-parser (#1258) --- .changeset/hand-written-liquid-html-parser.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/hand-written-liquid-html-parser.md diff --git a/.changeset/hand-written-liquid-html-parser.md b/.changeset/hand-written-liquid-html-parser.md new file mode 100644 index 000000000..0ee06def3 --- /dev/null +++ b/.changeset/hand-written-liquid-html-parser.md @@ -0,0 +1,16 @@ +--- +'@shopify/liquid-html-parser': minor +'@shopify/theme-language-server-common': minor +'@shopify/theme-check-common': minor +'@shopify/prettier-plugin-liquid': minor +'@shopify/theme-graph': minor +--- + +Adopt the hand-written `liquid-html-parser` for performance + +Replace the parser-combinator-based Liquid/HTML parser with a hand-written +recursive-descent parser. The new parser is significantly faster, adds +resilient parsing (it recovers from malformed input instead of bailing), and is +adapted to the theme-tools source model. `theme-language-server-common`, +`theme-check-common`, `prettier-plugin-liquid`, and `theme-graph` are updated to +consume the new parser. From 10ca6d02e6bcd7e484a694dc07258d33d867c262 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 17 Jul 2026 13:40:49 +0200 Subject: [PATCH 4/4] Remove unnecessary change --- packages/theme-check-common/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/theme-check-common/package.json b/packages/theme-check-common/package.json index 0600c4947..eaa58726f 100644 --- a/packages/theme-check-common/package.json +++ b/packages/theme-check-common/package.json @@ -22,7 +22,7 @@ "build": "pnpm build:ts", "build:ci": "pnpm build", "build:ts": "tsc -b tsconfig.build.json", - "test": "vitest --root ../.. --run packages/theme-check-common/src --exclude \"dist/**\"", + "test": "vitest", "type-check": "tsc --noEmit" }, "dependencies": {