From fb6970a92f39cefc43de19b114999dd3ae9503ab Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 29 Sep 2026 16:22:17 -0400 Subject: [PATCH 1/2] Narrow dotted block argument diagnostics --- .changeset/block-argument-diagnostic-range.md | 5 ++ .../src/checks/liquid-syntax-error/block.ts | 86 +++++++++++-------- .../checks/liquid-syntax-error/index.spec.ts | 71 ++++++++++++--- 3 files changed, 115 insertions(+), 47 deletions(-) create mode 100644 .changeset/block-argument-diagnostic-range.md diff --git a/.changeset/block-argument-diagnostic-range.md b/.changeset/block-argument-diagnostic-range.md new file mode 100644 index 000000000..53fe6a69d --- /dev/null +++ b/.changeset/block-argument-diagnostic-range.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Report unsupported dotted `block` arguments on the first offending argument instead of the entire `block` tag and its body. diff --git a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts index 981cbf4af..340b019db 100644 --- a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts +++ b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts @@ -1,4 +1,9 @@ -import { NodeTypes, type BlockMarkup, type LiquidTag } from '@shopify/liquid-html-parser'; +import { + NodeTypes, + type BlockMarkup, + type LiquidTag, + type Position, +} from '@shopify/liquid-html-parser'; import type { Context } from '.'; import { hasBareArrayAccess, @@ -20,28 +25,64 @@ const BLOCK_PARSER_ERROR_MESSAGES = new Set([ "Attempting to close LiquidTag 'block' before it was opened without a matching 'block'", ]); +interface BlockTagSyntaxOffense { + message: string; + position: Position; +} + export function checkBlockTag(node: LiquidTag, context: Context): void { - const message = blockTagSyntaxError(node); - if (message) report(node, context, message); + const offense = blockTagSyntaxOffense(node); + if (!offense) return; + + context.report({ + message: offense.message, + startIndex: offense.position.start, + endIndex: offense.position.end, + }); } export function blockTagSyntaxError(node: LiquidTag): string | undefined { - if (typeof node.markup === 'string') return SYNTAX_ERROR; + return blockTagSyntaxOffense(node)?.message; +} + +export function checkBlockParserError(error: Error, context: Context, source: string): void { + if (!BLOCK_PARSER_ERROR_MESSAGES.has(error.message)) return; + + const [startIndex, endIndex] = error.message.includes( + "Unclosed block tag 'block' in {% liquid %} block", + ) + ? (liquidLineTagLocation(source, 'block') ?? resolveErrorLocation(error, source)) + : resolveErrorLocation(error, source); + + context.report({ + message: + error.message === UNCLOSED_BLOCK_PARSER_ERROR + ? "Liquid syntax error: 'block' tag was never closed" + : error.message, + startIndex, + endIndex, + }); +} + +function blockTagSyntaxOffense(node: LiquidTag): BlockTagSyntaxOffense | undefined { + if (typeof node.markup === 'string') return tagOffense(node, SYNTAX_ERROR); const markup = node.markup as BlockMarkup; if (hasInvalidBlockName(markup.name.value)) { - return "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'"; + return tagOffense(node, "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'"); } /* * Ruby Liquid still accepts the unsupported experimental block.settings. * and block.content caller forms. Theme Check intentionally rejects every * dotted argument except block.name so authors move to plain arguments. + * Report the first one so body-form children stay outside the offense. */ - if (markup.args.some(isDottedArgument)) return DOTTED_ARGUMENT; + const dottedArgument = markup.args.find(isDottedArgument); + if (dottedArgument) return { message: DOTTED_ARGUMENT, position: dottedArgument.position }; - if (markup.args.some(isInvalidBlockNameArgument)) return SYNTAX_ERROR; + if (markup.args.some(isInvalidBlockNameArgument)) return tagOffense(node, SYNTAX_ERROR); /* * A +BlockArrayLiteral+ value (e.g. +size: [1, 2]+) is a first-class array @@ -54,29 +95,14 @@ export function blockTagSyntaxError(node: LiquidTag): string | undefined { (arg) => arg.value.type !== 'BlockArrayLiteral' && hasBareArrayAccess(arg.value), ) ) { - return BARE_ARRAY_ACCESS; + return tagOffense(node, BARE_ARRAY_ACCESS); } - if (hasSkippedCharacters(rawMarkup(node))) return SYNTAX_ERROR; + if (hasSkippedCharacters(rawMarkup(node))) return tagOffense(node, SYNTAX_ERROR); } -export function checkBlockParserError(error: Error, context: Context, source: string): void { - if (!BLOCK_PARSER_ERROR_MESSAGES.has(error.message)) return; - - const [startIndex, endIndex] = error.message.includes( - "Unclosed block tag 'block' in {% liquid %} block", - ) - ? (liquidLineTagLocation(source, 'block') ?? resolveErrorLocation(error, source)) - : resolveErrorLocation(error, source); - - context.report({ - message: - error.message === UNCLOSED_BLOCK_PARSER_ERROR - ? "Liquid syntax error: 'block' tag was never closed" - : error.message, - startIndex, - endIndex, - }); +function tagOffense(node: LiquidTag, message: string): BlockTagSyntaxOffense { + return { message, position: node.position }; } function hasInvalidBlockName(value: string): boolean { @@ -90,11 +116,3 @@ function isInvalidBlockNameArgument(argument: BlockMarkup['args'][number]): bool function isDottedArgument(argument: BlockMarkup['args'][number]): boolean { return argument.name !== 'block.name' && argument.name.includes('.'); } - -function report(node: LiquidTag, context: Context, message: string): void { - context.report({ - message, - startIndex: node.position.start, - endIndex: node.position.end, - }); -} diff --git a/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts b/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts index f2fbd8184..5c5abee28 100644 --- a/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts +++ b/packages/theme-check-common/src/checks/liquid-syntax-error/index.spec.ts @@ -811,12 +811,13 @@ describe('LiquidSyntaxError', () => { describe('block caller arguments', () => { it.each([ - "{% block 'card', block.settings.heading: 'Heading' %}{% endblock %}", - "{% block 'card', block.content: body %}{% endblock %}", - "{% block 'card', block.settings.heading.label: 'Heading' %}{% endblock %}", - "{% block 'card', block.unknown: 'value' %}{% endblock %}", - "{% block 'card', heading.label: 'Heading' %}{% endblock %}", - ])('rejects unsupported experimental dotted arguments in %s', async (template) => { + "block.settings.heading: 'Heading'", + 'block.content: body', + "block.settings.heading.label: 'Heading'", + "block.unknown: 'value'", + "heading.label: 'Heading'", + ])('rejects the unsupported dotted argument %s', async (argument) => { + const template = `{% block 'card', ${argument} %}{% endblock %}`; const offenses = await runLiquidCheck( LiquidSyntaxError, template, @@ -826,10 +827,47 @@ describe('LiquidSyntaxError', () => { expect(offenses).toMatchObject([ { - message: - "Liquid syntax error: in 'block' - Use plain named arguments, for example: block 'name', heading: value", - start: { index: 0 }, - end: { index: template.length }, + message: DOTTED_ARGUMENT, + start: { index: template.indexOf(argument) }, + end: { index: template.indexOf(argument) + argument.length }, + }, + ]); + }); + + it('reports only the dotted argument in a body-form block', async () => { + const argument = "block.settings.heading: 'Heading'"; + const template = `{% block 'card', ${argument} %}\n Child content\n{% endblock %}`; + const offenses = await runLiquidCheck( + LiquidSyntaxError, + template, + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toMatchObject([ + { + message: DOTTED_ARGUMENT, + start: { index: template.indexOf(argument) }, + end: { index: template.indexOf(argument) + argument.length }, + }, + ]); + }); + + it('reports only the first of several dotted arguments', async () => { + const first = "block.settings.heading: 'Heading'"; + const template = `{% block 'card', title: 'Title', ${first}, block.content: body, heading.label: 'Label' %}Child{% endblock %}`; + const offenses = await runLiquidCheck( + LiquidSyntaxError, + template, + 'templates/test.liquid', + NO_DOCSET, + ); + + expect(offenses).toMatchObject([ + { + message: DOTTED_ARGUMENT, + start: { index: template.indexOf(first) }, + end: { index: template.indexOf(first) + first.length }, }, ]); }); @@ -847,8 +885,8 @@ describe('LiquidSyntaxError', () => { it.each([ "{% block 'card', block.settings %}{% endblock %}", - "{% block 'card', block.name: value %}{% endblock %}", - ])('reports other malformed block tags in %s', async (template) => { + "{% block 'card', block.name: value %}Child content{% endblock %}", + ])('reports other malformed block tags on the whole tag in %s', async (template) => { const offenses = await runLiquidCheck( LiquidSyntaxError, template, @@ -856,7 +894,13 @@ describe('LiquidSyntaxError', () => { NO_DOCSET, ); - expect(offenses).toMatchObject([{ message: "Syntax error in 'block' tag" }]); + expect(offenses).toMatchObject([ + { + message: "Syntax error in 'block' tag", + start: { index: 0 }, + end: { index: template.length }, + }, + ]); }); describe('with the block parameter checks', () => { @@ -902,6 +946,7 @@ describe('LiquidSyntaxError', () => { ["heading.label: 'Hello'", DOTTED_ARGUMENT], ['block.name: value', "Syntax error in 'block' tag"], ["block.name: value, heading.label: 'Hello'", DOTTED_ARGUMENT], + ["block.settings.heading: 'Hello', block.content: body", DOTTED_ARGUMENT], ])('reports only one syntax error for %s', async (argument, message) => { const offenses = await checkBlockCall(`${argument}, count: 'many', count: 'more'`); From 21aef9e7aa2882664a314c587fa8500c82d62a34 Mon Sep 17 00:00:00 2001 From: "Charles-P. Clermont" Date: Tue, 29 Sep 2026 16:36:42 -0400 Subject: [PATCH 2/2] Use the standard syntax problem shape --- .../src/checks/liquid-syntax-error/block.ts | 83 +++++++++---------- 1 file changed, 38 insertions(+), 45 deletions(-) diff --git a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts index 340b019db..38ad7ba67 100644 --- a/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts +++ b/packages/theme-check-common/src/checks/liquid-syntax-error/block.ts @@ -4,6 +4,7 @@ import { type LiquidTag, type Position, } from '@shopify/liquid-html-parser'; +import { Problem, SourceCodeType } from '../../types'; import type { Context } from '.'; import { hasBareArrayAccess, @@ -25,52 +26,23 @@ const BLOCK_PARSER_ERROR_MESSAGES = new Set([ "Attempting to close LiquidTag 'block' before it was opened without a matching 'block'", ]); -interface BlockTagSyntaxOffense { - message: string; - position: Position; -} - export function checkBlockTag(node: LiquidTag, context: Context): void { - const offense = blockTagSyntaxOffense(node); - if (!offense) return; - - context.report({ - message: offense.message, - startIndex: offense.position.start, - endIndex: offense.position.end, - }); -} - -export function blockTagSyntaxError(node: LiquidTag): string | undefined { - return blockTagSyntaxOffense(node)?.message; -} - -export function checkBlockParserError(error: Error, context: Context, source: string): void { - if (!BLOCK_PARSER_ERROR_MESSAGES.has(error.message)) return; - - const [startIndex, endIndex] = error.message.includes( - "Unclosed block tag 'block' in {% liquid %} block", - ) - ? (liquidLineTagLocation(source, 'block') ?? resolveErrorLocation(error, source)) - : resolveErrorLocation(error, source); - - context.report({ - message: - error.message === UNCLOSED_BLOCK_PARSER_ERROR - ? "Liquid syntax error: 'block' tag was never closed" - : error.message, - startIndex, - endIndex, - }); + const problem = blockTagSyntaxError(node); + if (problem) context.report(problem); } -function blockTagSyntaxOffense(node: LiquidTag): BlockTagSyntaxOffense | undefined { - if (typeof node.markup === 'string') return tagOffense(node, SYNTAX_ERROR); +export function blockTagSyntaxError( + node: LiquidTag, +): Problem | undefined { + if (typeof node.markup === 'string') return syntaxProblem(node.position, SYNTAX_ERROR); const markup = node.markup as BlockMarkup; if (hasInvalidBlockName(markup.name.value)) { - return tagOffense(node, "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'"); + return syntaxProblem( + node.position, + "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'", + ); } /* @@ -80,9 +52,11 @@ function blockTagSyntaxOffense(node: LiquidTag): BlockTagSyntaxOffense | undefin * Report the first one so body-form children stay outside the offense. */ const dottedArgument = markup.args.find(isDottedArgument); - if (dottedArgument) return { message: DOTTED_ARGUMENT, position: dottedArgument.position }; + if (dottedArgument) return syntaxProblem(dottedArgument.position, DOTTED_ARGUMENT); - if (markup.args.some(isInvalidBlockNameArgument)) return tagOffense(node, SYNTAX_ERROR); + if (markup.args.some(isInvalidBlockNameArgument)) { + return syntaxProblem(node.position, SYNTAX_ERROR); + } /* * A +BlockArrayLiteral+ value (e.g. +size: [1, 2]+) is a first-class array @@ -95,14 +69,33 @@ function blockTagSyntaxOffense(node: LiquidTag): BlockTagSyntaxOffense | undefin (arg) => arg.value.type !== 'BlockArrayLiteral' && hasBareArrayAccess(arg.value), ) ) { - return tagOffense(node, BARE_ARRAY_ACCESS); + return syntaxProblem(node.position, BARE_ARRAY_ACCESS); } - if (hasSkippedCharacters(rawMarkup(node))) return tagOffense(node, SYNTAX_ERROR); + if (hasSkippedCharacters(rawMarkup(node))) return syntaxProblem(node.position, SYNTAX_ERROR); +} + +export function checkBlockParserError(error: Error, context: Context, source: string): void { + if (!BLOCK_PARSER_ERROR_MESSAGES.has(error.message)) return; + + const [startIndex, endIndex] = error.message.includes( + "Unclosed block tag 'block' in {% liquid %} block", + ) + ? (liquidLineTagLocation(source, 'block') ?? resolveErrorLocation(error, source)) + : resolveErrorLocation(error, source); + + context.report({ + message: + error.message === UNCLOSED_BLOCK_PARSER_ERROR + ? "Liquid syntax error: 'block' tag was never closed" + : error.message, + startIndex, + endIndex, + }); } -function tagOffense(node: LiquidTag, message: string): BlockTagSyntaxOffense { - return { message, position: node.position }; +function syntaxProblem(position: Position, message: string): Problem { + return { message, startIndex: position.start, endIndex: position.end }; } function hasInvalidBlockName(value: string): boolean {