Add implicit block content tooling

.changeset/block-content-implicit-argument-checks.md

@@ -0,0 +1,12 @@
+---
+'@shopify/theme-check-common': minor
+'@shopify/theme-check-node': minor
+---
+
+Guide theme blocks to the implicit `content` parameter.
+
+Add the recommended `BlockContentUsage` error for `block.content` in `blocks/*.liquid`. Its message is "Use the implicit 'content' parameter directly instead of 'block.content'."
+
+Add the recommended `ValidBlockContentSettingType` error for a theme block schema setting named `content` whose Liquid type is not `string`. This error replaces the `ValidBlockArgumentTypes` warning for the same schema declaration.
+
+`UndefinedObject` accepts bare `content` in `blocks/*.liquid`.

.changeset/block-content-implicit-argument-language-server.md

@@ -0,0 +1,7 @@
+---
+'@shopify/theme-language-server-common': patch
+---
+
+Complete and hover the implicit `content` parameter inside `blocks/*.liquid`.
+
+Bare `content` is a `string` variable in every theme block file, and `block.content` is a `string` property there. A later assign or capture changes the variable's type from that point. A schema setting or LiquidDoc parameter named `content` keeps the `string` type.

packages/theme-check-common/src/checks/block-content-usage/index.spec.ts

@@ -0,0 +1,63 @@
+import { describe, expect, it } from 'vitest';
+import { recommended } from '../index';
+import { highlightedOffenses, runLiquidCheck } from '../../test';
+import { Severity } from '../../types';
+import { BlockContentUsage } from './index';
+
+const MESSAGE = "Use the implicit 'content' parameter directly instead of 'block.content'.";
+
+describe('BlockContentUsage', () => {
+  it('is a recommended error', () => {
+    expect(recommended).toContain(BlockContentUsage);
+    expect(BlockContentUsage.meta.severity).toBe(Severity.ERROR);
+  });
+
+  it.each([
+    ['{{ block.content }}', 'block.content'],
+    ["{{ block['content'] | upcase }}", "block['content']"],
+    ['{{ block.content.size }}', 'block.content.size'],
+    ['{% if block.content != blank %}{{ content }}{% endif %}', 'block.content'],
+    ["{% render 'card', body: block.content %}", 'block.content'],
+    ['{% liquid\n  echo block.content\n%}', 'block.content'],
+  ])('reports the block.content lookup in %j', async (source, highlight) => {
+    const offenses = await runLiquidCheck(BlockContentUsage, source, 'blocks/card.liquid');
+
+    expect(offenses).toMatchObject([{ message: MESSAGE, severity: Severity.ERROR }]);
+    expect(highlightedOffenses({ 'blocks/card.liquid': source }, offenses)).toEqual([highlight]);
+  });
+
+  it('accepts bare content and other block properties', async () => {
+    const source = [
+      '{{ content }}',
+      '{{ block.id }}',
+      '{{ block.settings.content }}',
+      '{{ block.shopify_attributes }}',
+      '{{ product.content }}',
+    ].join('\n');
+
+    const offenses = await runLiquidCheck(BlockContentUsage, source, 'blocks/card.liquid');
+
+    expect(offenses).toEqual([]);
+  });
+
+  it.each([
+    'sections/main.liquid',
+    'snippets/card.liquid',
+    'templates/index.liquid',
+    'layout/theme.liquid',
+  ])('does not report the ordinary block object in %s', async (fileName) => {
+    const source = '{% for block in section.blocks %}{{ block.content }}{% endfor %}';
+
+    const offenses = await runLiquidCheck(BlockContentUsage, source, fileName);
+
+    expect(offenses).toEqual([]);
+  });
+
+  it('does not report block.content inside a tag that does not render Liquid', async () => {
+    const source = '{% javascript %}console.log("{{ block.content }}");{% endjavascript %}';
+
+    const offenses = await runLiquidCheck(BlockContentUsage, source, 'blocks/card.liquid');
+
+    expect(offenses).toEqual([]);
+  });
+});

packages/theme-check-common/src/checks/block-content-usage/index.ts

@@ -0,0 +1,53 @@
+import { LiquidVariableLookup, NodeTypes } from '@shopify/liquid-html-parser';
+import { BLOCK_CONTENT_PARAMETER } from '../../block-parameters';
+import { isBlock } from '../../to-schema';
+import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
+import { isWithinRawTagThatDoesNotParseItsContents } from '../utils';
+
+const MESSAGE = "Use the implicit 'content' parameter directly instead of 'block.content'.";
+
+export const BlockContentUsage: LiquidCheckDefinition = {
+  meta: {
+    code: 'BlockContentUsage',
+    name: 'Use `content` instead of `block.content`',
+    docs: {
+      description:
+        "Reports 'block.content' in theme block files, where the built-in 'content' parameter is available as the 'content' variable.",
+      recommended: true,
+      url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/block-content-usage',
+    },
+    type: SourceCodeType.LiquidHtml,
+    severity: Severity.ERROR,
+    schema: {},
+    targets: [],
+  },
+
+  create(context) {
+    if (!isBlock(context.file.uri)) return {};
+
+    return {
+      // BAD: {{ block.content }}
+      // BAD: {{ block['content'] | upcase }}
+      // GOOD: {{ content }}
+      async VariableLookup(node, ancestors) {
+        if (isWithinRawTagThatDoesNotParseItsContents(ancestors)) return;
+        if (!isBlockContentLookup(node)) return;
+
+        context.report({
+          message: MESSAGE,
+          startIndex: node.position.start,
+          endIndex: node.position.end,
+        });
+      },
+    };
+  },
+};
+
+function isBlockContentLookup(node: LiquidVariableLookup): boolean {
+  const [firstLookup] = node.lookups;
+  return (
+    node.name === 'block' &&
+    firstLookup?.type === NodeTypes.String &&
+    firstLookup.value === BLOCK_CONTENT_PARAMETER
+  );
+}

packages/theme-check-common/src/checks/index.ts

@@ -79,6 +79,7 @@ import {
   StylesheetOncePerFile,
   StylesheetTagInWrongFile,
 } from './raw-tags';
+import { BlockContentUsage } from './block-content-usage';
 import { DuplicateBlockArguments } from './duplicate-block-arguments';
 import { ExcessiveSettingsCount } from './excessive-settings-count';
 import { LiquidComplexity } from './liquid-complexity';
@@ -88,6 +89,7 @@ import { MaxFileSize, MaxFileSizeJSON } from './max-file-size';
 import { MissingBlockArguments } from './missing-block-arguments';
 import { UnrecognizedBlockArguments } from './unrecognized-block-arguments';
 import { ValidBlockArgumentTypes } from './valid-block-argument-types';
+import { ValidBlockContentSettingType } from './valid-block-content-setting-type';
 import { ValidBlockTagPlacement } from './valid-block-tag-placement';
 
 export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
@@ -169,6 +171,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
   SchemaSectionOrBlockOnly,
   StylesheetOncePerFile,
   StylesheetTagInWrongFile,
+  BlockContentUsage,
   DuplicateBlockArguments,
   ExcessiveSettingsCount,
   LiquidComplexity,
@@ -179,6 +182,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
   MissingBlockArguments,
   UnrecognizedBlockArguments,
   ValidBlockArgumentTypes,
+  ValidBlockContentSettingType,
   ValidBlockTagPlacement,
 ];
 

packages/theme-check-common/src/checks/undefined-object/index.spec.ts

@@ -449,6 +449,23 @@ describe('Module: UndefinedObject', () => {
     expect(offenses[0].message).toBe("Unknown object 'undefined_variable' used.");
   });
 
+  it('does not report the built-in content variable in a block file', async () => {
+    const sourceCode = `
+      {{ content }}
+      {% if content != blank %}{{ content | upcase }}{% endif %}
+    `;
+
+    const offenses = await runLiquidCheck(UndefinedObject, sourceCode, 'blocks/card.liquid');
+
+    expect(offenses).toEqual([]);
+  });
+
+  it('reports content outside block files', async () => {
+    const offenses = await runLiquidCheck(UndefinedObject, '{{ content }}', 'sections/main.liquid');
+
+    expect(offenses).toMatchObject([{ message: "Unknown object 'content' used." }]);
+  });
+
   it('should not report an offense when a self defined variable is defined with a @param tag', async () => {
     const sourceCode = `
       {% doc %}

packages/theme-check-common/src/checks/undefined-object/index.ts

@@ -13,6 +13,7 @@ import {
   NodeTypes,
   Position,
 } from '@shopify/liquid-html-parser';
+import { BLOCK_CONTENT_PARAMETER } from '../../block-parameters';
 import { LiquidCheckDefinition, Mode, Severity, SourceCodeType, ThemeDocset } from '../../types';
 import { isError, last } from '../../utils';
 import { hasLiquidDoc } from '../../liquid-doc/liquidDoc';
@@ -55,7 +56,7 @@ export const UndefinedObject: LiquidCheckDefinition = {
 
     const themeDocset = context.themeDocset;
     const scopedVariables: Map<string, Scope[]> = new Map();
-    const fileScopedVariables: Set<string> = new Set();
+    const fileScopedVariables: Set<string> = new Set(builtInVariables(relativePath));
     const variables: LiquidVariableLookup[] = [];
 
     function indexVariableScope(variableName: string | null, scope: Scope) {
@@ -188,6 +189,11 @@ async function globalObjects(themeDocset: ThemeDocset, relativePath: string, mod
   return globalObjects;
 }
 
+/** Theme blocks always receive the built-in `content` parameter as a variable. */
+function builtInVariables(relativePath: string): string[] {
+  return relativePath.startsWith('blocks/') ? [BLOCK_CONTENT_PARAMETER] : [];
+}
+
 const BLOCK_CONTEXTUAL_OBJECTS = ['app', 'section', 'recommendations', 'block'];
 
 function getContextualObjects(relativePath: string, mode: Mode = 'theme'): string[] {

packages/theme-check-common/src/checks/valid-block-argument-types/index.spec.ts

@@ -264,28 +264,19 @@ describe('ValidBlockArgumentTypes', () => {
       ]);
     });
 
-    it('reports a schema content setting that is not string-compatible', async () => {
-      const source = blockSource([{ id: 'content', type: 'number' }]);
-      const offenses = await definitions(source);
-
-      expect(offenses).toMatchObject([
-        {
-          message:
-            "Schema setting 'content' has Liquid type 'number', but the built-in 'content' parameter has type 'string'.",
-        },
-      ]);
-      expect(highlightedOffenses({ 'blocks/card.liquid': source }, offenses)).toEqual(['"number"']);
-    });
-
-    it('uses the built-in string type for a LiquidDoc echo of schema content', async () => {
-      const offenses = await definitions(
-        blockSource([{ id: 'content', type: 'number' }], ['@param {string} [content] - Body']),
-      );
-
-      expect(offenses.map((offense) => offense.message)).toEqual([
-        "Schema setting 'content' has Liquid type 'number', but the built-in 'content' parameter has type 'string'.",
-      ]);
-    });
+    it.each([
+      ['without LiquidDoc', []],
+      ['with a LiquidDoc string echo', ['@param {string} [content] - Body']],
+    ])(
+      'leaves a non-string schema content setting %s to ValidBlockContentSettingType',
+      async (_name, params) => {
+        const offenses = await definitions(
+          blockSource([{ id: 'content', type: 'number' }], params),
+        );
+
+        expect(offenses).toEqual([]);
+      },
+    );
 
     it('accepts a string-compatible schema content setting', async () => {
       const offenses = await definitions(blockSource([{ id: 'content', type: 'text' }]));

packages/theme-check-common/src/checks/valid-block-argument-types/index.ts

@@ -4,19 +4,10 @@ import {
   type BlockParameters,
   liquidDocType,
 } from '../../block-parameters';
-import { nodeAtPath } from '../../json';
 import { BasicParamTypes, inferArgumentType, isTypeCompatible } from '../../liquid-doc/utils';
 import * as path from '../../path';
-import { schemaSettingLiquidType } from '../../schema-settings';
 import { isBlock } from '../../to-schema';
-import {
-  Severity,
-  SourceCodeType,
-  type Context,
-  type LiquidCheckDefinition,
-  type ThemeBlockSchema,
-} from '../../types';
-import { reportWarning } from '../../utils';
+import { Severity, SourceCodeType, type Context, type LiquidCheckDefinition } from '../../types';
 import { blockTagSyntaxError } from '../liquid-syntax-error/block';
 import {
   NodeTypes,
@@ -81,13 +72,6 @@ export const ValidBlockArgumentTypes: LiquidCheckDefinition = {
 
         reportLiquidDocTypeMismatch(context, node, parameter);
       },
-
-      async LiquidRawTag(node) {
-        if (node.name !== 'schema' || node.body.kind !== 'json') return;
-        if (!currentBlockName || !context.getBlockSchema) return;
-
-        reportContentSettingTypeMismatch(context, await context.getBlockSchema(currentBlockName));
-      },
     };
   },
 };
@@ -129,7 +113,7 @@ function reportLiquidDocTypeMismatch(
   if (!node.paramType || !declaredType || !authoritativeType) return;
   if (declaredType === BasicParamTypes.Object || declaredType === authoritativeType) return;
 
-  const source = parameter.schemaSetting ? 'schema setting' : 'built-in parameter';
+  const source = declarationSource(parameter);
   context.report({
     message:
       `The ${source} '${parameter.name}' has Liquid type '${authoritativeType}', ` +
@@ -139,30 +123,17 @@ function reportLiquidDocTypeMismatch(
   });
 }
 
+/**
+ * The built-in `content` parameter decides the type of `content`, even when a
+ * schema setting also declares it.
+ */
+function declarationSource(parameter: BlockParameter): string {
+  if (parameter.name === BLOCK_CONTENT_PARAMETER) return 'built-in parameter';
+  return 'schema setting';
+}
+
 /** Returns the merged type that a schema or built-in declaration must fit. */
 function declarationType(parameter: BlockParameter): string | undefined {
   if (!parameter.schemaSetting && parameter.name !== BLOCK_CONTENT_PARAMETER) return undefined;
   return parameter.type;
 }
-
-function reportContentSettingTypeMismatch(
-  context: Context<SourceCodeType.LiquidHtml>,
-  schema: ThemeBlockSchema | undefined,
-): void {
-  if (!schema || schema.validSchema instanceof Error || schema.ast instanceof Error) return;
-
-  const settings = schema.validSchema.settings ?? [];
-  const index = settings.findIndex((setting) => setting.id === BLOCK_CONTENT_PARAMETER);
-  if (index < 0) return;
-
-  const settingType = schemaSettingLiquidType(settings[index].type);
-  const typeNode = nodeAtPath(schema.ast, ['settings', index, 'type']);
-  if (!settingType || settingType === 'string' || !typeNode) return;
-
-  reportWarning(
-    `Schema setting 'content' has Liquid type '${settingType}', but the built-in 'content' parameter has type 'string'.`,
-    schema.offset,
-    typeNode,
-    context,
-  );
-}