diff --git a/.changeset/liquid-unclosed-block-end.md b/.changeset/liquid-unclosed-block-end.md new file mode 100644 index 000000000..8611c9e67 --- /dev/null +++ b/.changeset/liquid-unclosed-block-end.md @@ -0,0 +1,5 @@ +--- +'@shopify/liquid-html-parser': patch +--- + +Fix the end of an unclosed block in `{% liquid %}`. It ended past the end of the template, as if its end tag came after the last line. It now ends where the last line does, as an unclosed block outside `{% liquid %}` does. diff --git a/packages/liquid-html-parser/src/document/liquid-lines.ts b/packages/liquid-html-parser/src/document/liquid-lines.ts index 3a30864bf..46900845a 100644 --- a/packages/liquid-html-parser/src/document/liquid-lines.ts +++ b/packages/liquid-html-parser/src/document/liquid-lines.ts @@ -153,19 +153,15 @@ export function parseLineBlockTag( reason = e instanceof Error ? e.message : 'unknown error'; } - const endTagLength = 3 + envelope.tagName.length; // 'end' + tagName - if (def.branches.length === 0) { - const { children, endNameOffset } = parseLineBlockBody(parser, envelope.tagName, ctx); - const endPosition: Position = { start: endNameOffset, end: endNameOffset + endTagLength }; + const { children, endPosition } = parseLineBlockBody(parser, envelope.tagName, ctx); if (markupParsed) { return makeLiquidTagNamed(envelope, markup, children, endPosition, { start: '', end: '' }); } return makeLiquidTagBaseCase(envelope, children, endPosition, { start: '', end: '' }, reason); } - const { branches, endNameOffset } = parseLineBranchedBody(parser, envelope.tagName, def, ctx); - const endPosition: Position = { start: endNameOffset, end: endNameOffset + endTagLength }; + const { branches, endPosition } = parseLineBranchedBody(parser, envelope.tagName, def, ctx); if (markupParsed) { return makeLiquidTagNamed(envelope, markup, branches, endPosition, { start: '', end: '' }); } @@ -282,7 +278,7 @@ export function parseLineBlockBody( parser: LineParserDelegate, parentName: string, ctx: LiquidLineContext, -): { children: LiquidStatement[]; endNameOffset: number } { +): { children: LiquidStatement[]; endPosition: Position } { const children: LiquidStatement[] = []; while (ctx.index < ctx.lines.length) { @@ -290,7 +286,8 @@ export function parseLineBlockBody( if (line.tagName === `end${parentName}`) { ctx.index++; - return { children, endNameOffset: line.nameOffset }; + const endPosition = { start: line.nameOffset, end: line.nameOffset + line.tagName.length }; + return { children, endPosition }; } ctx.index++; @@ -298,8 +295,9 @@ export function parseLineBlockBody( } if (parser.lineAllowUnclosedDocumentNode) { - const endNameOffset = ctx.lines.length > 0 ? ctx.lines[ctx.lines.length - 1].lineEnd : 0; - return { children, endNameOffset }; + // An unclosed block ends where its last line does, as at the top level. + const end = ctx.lines.length > 0 ? ctx.lines[ctx.lines.length - 1].lineEnd : 0; + return { children, endPosition: { start: end, end } }; } throw new LiquidHTMLASTParsingError( @@ -316,7 +314,7 @@ export function parseLineBranchedBody( parentName: string, def: TagDefinitionBlock, ctx: LiquidLineContext, -): { branches: LiquidBranch[]; endNameOffset: number } { +): { branches: LiquidBranch[]; endPosition: Position } { const branches: LiquidBranch[] = []; const bodyStart = ctx.index > 0 ? ctx.lines[ctx.index - 1].lineEnd : 0; let currentBranch: LiquidBranchUnnamed | LiquidBranchNamed = makeLiquidBranchUnnamed( @@ -338,7 +336,8 @@ export function parseLineBranchedBody( ); branches.push(currentBranch); ctx.index++; - return { branches, endNameOffset: line.nameOffset }; + const endPosition = { start: line.nameOffset, end: line.nameOffset + line.tagName.length }; + return { branches, endPosition }; } if (isBranchName(line.tagName) && def.branches.includes(line.tagName)) { @@ -371,16 +370,16 @@ export function parseLineBranchedBody( } if (parser.lineAllowUnclosedDocumentNode) { - const endNameOffset = ctx.lines.length > 0 ? ctx.lines[ctx.lines.length - 1].lineEnd : 0; + const end = ctx.lines.length > 0 ? ctx.lines[ctx.lines.length - 1].lineEnd : 0; finalizeBranch( currentBranch, currentChildren, - endNameOffset, + end, parser.getSource(), parser.lineParseHtml ? ChildFilterMode.StripEdges : ChildFilterMode.Syntactic, ); branches.push(currentBranch); - return { branches, endNameOffset }; + return { branches, endPosition: { start: end, end } }; } throw new LiquidHTMLASTParsingError( diff --git a/packages/liquid-html-parser/src/document/parser.test.ts b/packages/liquid-html-parser/src/document/parser.test.ts index a68ca47ae..89e322d3e 100644 --- a/packages/liquid-html-parser/src/document/parser.test.ts +++ b/packages/liquid-html-parser/src/document/parser.test.ts @@ -183,6 +183,9 @@ describe('Unit: document-parser', () => { // Verify inner structure is properly parsed (not base case fallback) expectPath(ast, 'children.0.markup.0.name').to.eql('assign'); expectPath(ast, 'children.0.markup.1.name').to.eql('if'); + // It ends where its last line does, since there's no endif. + expectPosition(ast, 'children.0.markup.1').toEqual('if use_variant\n assign var2 = var1'); + expectPath(ast, 'children.0.markup.1.blockEndPosition').to.eql({ start: 74, end: 74 }); }); it('should parse raw Liquid tags normally (doc, comment, raw)', () => {