Narrow dotted block argument diagnostics

.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.

packages/theme-check-common/src/checks/liquid-syntax-error/block.ts

@@ -1,4 +1,10 @@
-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 { Problem, SourceCodeType } from '../../types';
 import type { Context } from '.';
 import {
   hasBareArrayAccess,
@@ -21,27 +27,36 @@ const BLOCK_PARSER_ERROR_MESSAGES = new Set([
 ]);
 
 export function checkBlockTag(node: LiquidTag, context: Context): void {
-  const message = blockTagSyntaxError(node);
-  if (message) report(node, context, message);
+  const problem = blockTagSyntaxError(node);
+  if (problem) context.report(problem);
 }
 
-export function blockTagSyntaxError(node: LiquidTag): string | undefined {
-  if (typeof node.markup === 'string') return SYNTAX_ERROR;
+export function blockTagSyntaxError(
+  node: LiquidTag,
+): Problem<SourceCodeType.LiquidHtml> | undefined {
+  if (typeof node.markup === 'string') return syntaxProblem(node.position, 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 syntaxProblem(
+      node.position,
+      "Liquid syntax error: in 'block' - Valid syntax: block '[file_name]'",
+    );
   }
 
   /*
    * Ruby Liquid still accepts the unsupported experimental block.settings.<id>
    * 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 syntaxProblem(dottedArgument.position, DOTTED_ARGUMENT);
 
-  if (markup.args.some(isInvalidBlockNameArgument)) return 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
@@ -54,10 +69,10 @@ export function blockTagSyntaxError(node: LiquidTag): string | undefined {
       (arg) => arg.value.type !== 'BlockArrayLiteral' && hasBareArrayAccess(arg.value),
     )
   ) {
-    return BARE_ARRAY_ACCESS;
+    return syntaxProblem(node.position, BARE_ARRAY_ACCESS);
   }
 
-  if (hasSkippedCharacters(rawMarkup(node))) return SYNTAX_ERROR;
+  if (hasSkippedCharacters(rawMarkup(node))) return syntaxProblem(node.position, SYNTAX_ERROR);
 }
 
 export function checkBlockParserError(error: Error, context: Context, source: string): void {
@@ -79,6 +94,10 @@ export function checkBlockParserError(error: Error, context: Context, source: st
   });
 }
 
+function syntaxProblem(position: Position, message: string): Problem<SourceCodeType.LiquidHtml> {
+  return { message, startIndex: position.start, endIndex: position.end };
+}
+
 function hasInvalidBlockName(value: string): boolean {
   return value.includes('/') || value.includes('.');
 }
@@ -90,11 +109,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,
-  });
-}

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,16 +885,22 @@ 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,
         'templates/test.liquid',
         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'`);