packages/theme-check-browser/CHANGELOG.md

@@ -9,6 +9,7 @@
 - Updated dependencies [21354237]
 - Updated dependencies [dafba833]
 - Updated dependencies [c55077e7]
+- Updated dependencies [48d4d79e]
   - @shopify/theme-check-common@3.11.0
 
 ## 3.10.0

packages/theme-check-common/CHANGELOG.md

@@ -12,6 +12,7 @@
 - 0d96194e: Update MissingRenderSnippetParams to report when alias variable are provided using `with/for` syntax.
 - 21354237: Update UnrecognizedRenderSnippetParams to report when alias variable are provided using `with/for` syntax.
 - c55077e7: Split ValidRenderSnippetParams theme check into MissingRenderSnippetParams and UnrecognizedRenderSnippetParams. This allows them to be disabled independantly.
+- 48d4d79e: Update ValidRenderSnippetParamTypes to report when alias variable are provided using `with/for` syntax.
 
 ### Patch Changes
 

packages/theme-check-common/src/checks/valid-render-snippet-param-types/index.spec.ts

@@ -176,9 +176,49 @@ describe('Module: ValidRenderSnippetParamTypes', () => {
       });
       expect(offenses).toHaveLength(0);
     });
+
+    it('should report when `with` alias is used', async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': `
+          {% doc %}
+            @param {String} title - The title
+          {% enddoc %}
+          <div>{{ title }}</div>
+        `,
+      });
+
+      const sourceCode = `{% render 'card' with 12 as title %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe(
+        "Type mismatch for parameter 'title': expected string, got number",
+      );
+    });
+
+    it('should report when `for` alias is used', async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': `
+          {% doc %}
+            @param {String} title - The title
+          {% enddoc %}
+          <div>{{ title }}</div>
+        `,
+      });
+
+      const sourceCode = `{% render 'card' for 123 as title %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe(
+        "Type mismatch for parameter 'title': expected string, got number",
+      );
+    });
   });
 
-  describe('autofix tests', () => {
+  describe('suggestions', () => {
     const makeSnippet = (type: string) => `
       {% doc %}
         @param {${type}} param - Description
@@ -326,5 +366,74 @@ describe('Module: ValidRenderSnippetParamTypes', () => {
         '' 
       %}`);
     });
+
+    it('should suggest removal and replacement if expected type has a default value', async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': makeSnippet('string'),
+      });
+
+      const sourceCode = `{% render 'card', param: 123 %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].suggest).toHaveLength(2);
+      expect(offenses[0].suggest?.[0]?.message).toBe("Replace with default value '''' for string");
+      expect(offenses[0].suggest?.[1]?.message).toBe('Remove value');
+    });
+
+    it("should only suggest removal if expected type default value is ''", async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': makeSnippet('object'),
+      });
+
+      const sourceCode = `{% render 'card', param: 123 %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].suggest).toHaveLength(1);
+      expect(offenses[0].suggest?.[0]?.message).toBe('Remove value');
+    });
+
+    it('should handle when aliases `with` syntax is used', async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': makeSnippet('string'),
+      });
+
+      const sourceCode = `{% render 'card' with 123 as param %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe(
+        "Type mismatch for parameter 'param': expected string, got number",
+      );
+
+      const result = applySuggestions(sourceCode, offenses[0]);
+      expect(result?.[0]).toEqual(`{% render 'card' with '' as param %}`);
+    });
+
+    it('should handle when aliases `for` syntax is used', async () => {
+      const fs = new MockFileSystem({
+        'snippets/card.liquid': makeSnippet('string'),
+      });
+
+      const sourceCode = `{% render 'card' for 123 as param %}`;
+      const offenses = await runLiquidCheck(ValidRenderSnippetParamTypes, sourceCode, undefined, {
+        fs,
+      });
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe(
+        "Type mismatch for parameter 'param': expected string, got number",
+      );
+
+      const result = applySuggestions(sourceCode, offenses[0]);
+      expect(result?.[0]).toEqual(`{% render 'card' for '' as param %}`);
+    });
   });
 });

packages/theme-check-common/src/checks/valid-render-snippet-param-types/index.ts

@@ -9,6 +9,7 @@ import {
   SupportedParamTypes,
   isTypeCompatible,
 } from '../../liquid-doc/utils';
+import { StringCorrector } from '../../fixes';
 
 export const ValidRenderSnippetParamTypes: LiquidCheckDefinition = {
   meta: {
@@ -28,6 +29,38 @@ export const ValidRenderSnippetParamTypes: LiquidCheckDefinition = {
   },
 
   create(context) {
+    /**
+     * Generates suggestions for type mismatches based on the expected type and node positions
+     */
+    function generateTypeMismatchSuggestions(
+      expectedType: string,
+      startPosition: number,
+      endPosition: number,
+    ) {
+      const defaultValue = getDefaultValueForType(expectedType);
+      const suggestions = [];
+
+      // Only add the "replace with default" suggestion if the default is not an empty string
+      if (defaultValue !== '') {
+        suggestions.push({
+          message: `Replace with default value '${defaultValue}' for ${expectedType}`,
+          fix: (fixer: StringCorrector) => {
+            return fixer.replace(startPosition, endPosition, defaultValue);
+          },
+        });
+      }
+
+      // Always include the "remove value" suggestion
+      suggestions.push({
+        message: `Remove value`,
+        fix: (fixer: StringCorrector) => {
+          return fixer.remove(startPosition, endPosition);
+        },
+      });
+
+      return suggestions;
+    }
+
     function findTypeMismatchParams(
       liquidDocParameters: Map<string, LiquidDocParameter>,
       providedParams: LiquidNamedArgument[],
@@ -47,7 +80,7 @@ export const ValidRenderSnippetParamTypes: LiquidCheckDefinition = {
             continue;
           }
 
-          if (!isTypeCompatible(paramType, inferArgumentType(arg))) {
+          if (!isTypeCompatible(paramType, inferArgumentType(arg.value))) {
             typeMismatchParams.push(arg);
           }
         }
@@ -65,39 +98,62 @@ export const ValidRenderSnippetParamTypes: LiquidCheckDefinition = {
         if (!paramDef || !paramDef.type) continue;
 
         const expectedType = paramDef.type.toLowerCase();
-        const actualType = inferArgumentType(arg);
+        const actualType = inferArgumentType(arg.value);
+
+        const suggestions = generateTypeMismatchSuggestions(
+          expectedType,
+          arg.value.position.start,
+          arg.value.position.end,
+        );
 
         context.report({
           message: `Type mismatch for parameter '${arg.name}': expected ${expectedType}, got ${actualType}`,
           startIndex: arg.value.position.start,
           endIndex: arg.value.position.end,
-          suggest: [
-            {
-              message: `Replace with default value '${getDefaultValueForType(
-                expectedType,
-              )}' for ${expectedType}`,
-              fix: (fixer) => {
-                return fixer.replace(
-                  arg.value.position.start,
-                  arg.value.position.end,
-                  getDefaultValueForType(expectedType),
-                );
-              },
-            },
-            {
-              message: `Remove value`,
-              fix: (fixer) => {
-                return fixer.remove(arg.value.position.start, arg.value.position.end);
-              },
-            },
-          ],
+          suggest: suggestions,
         });
       }
     }
 
+    /**
+     * Checks for type mismatches when alias is used with `for` or `with` syntax.
+     * This can be refactored at a later date to share more code with regular named arguments as they are both backed by LiquidExpression nodes.
+     *
+     * E.g. {% render 'card' with 123 as title %}
+     */
+    function findAndReportAliasType(
+      node: RenderMarkup,
+      liquidDocParameters: Map<string, LiquidDocParameter>,
+    ) {
+      if (
+        node.alias &&
+        node.variable?.name &&
+        node.variable.name.type !== NodeTypes.VariableLookup
+      ) {
+        const paramIsDefinedWithType = liquidDocParameters.get(node.alias)?.type?.toLowerCase();
+        if (paramIsDefinedWithType) {
+          const providedParamType = inferArgumentType(node.variable.name);
+          if (!isTypeCompatible(paramIsDefinedWithType, providedParamType)) {
+            const suggestions = generateTypeMismatchSuggestions(
+              paramIsDefinedWithType,
+              node.variable.name.position.start,
+              node.variable.name.position.end,
+            );
+
+            context.report({
+              message: `Type mismatch for parameter '${node.alias}': expected ${paramIsDefinedWithType}, got ${providedParamType}`,
+              startIndex: node.variable.name.position.start,
+              endIndex: node.variable.name.position.end,
+              suggest: suggestions,
+            });
+          }
+        }
+      }
+    }
+
     return {
       async RenderMarkup(node: RenderMarkup) {
-        if (!isLiquidString(node.snippet) || node.variable) {
+        if (!isLiquidString(node.snippet)) {
           return;
         }
 
@@ -117,6 +173,8 @@ export const ValidRenderSnippetParamTypes: LiquidCheckDefinition = {
           snippetDef.liquidDoc.parameters.map((p) => [p.name, p]),
         );
 
+        findAndReportAliasType(node, liquidDocParameters);
+
         const typeMismatchParams = findTypeMismatchParams(liquidDocParameters, node.args);
         reportTypeMismatches(typeMismatchParams, liquidDocParameters);
       },

packages/theme-check-common/src/liquid-doc/utils.ts

@@ -1,4 +1,4 @@
-import { LiquidNamedArgument, NodeTypes } from '@shopify/liquid-html-parser';
+import { LiquidExpression, LiquidNamedArgument, NodeTypes } from '@shopify/liquid-html-parser';
 import { assertNever } from '../utils';
 
 export enum SupportedParamTypes {
@@ -34,8 +34,8 @@ export function getDefaultValueForType(type: string | null) {
 /**
  * Casts the value of a LiquidNamedArgument to a string representing the type of the value.
  */
-export function inferArgumentType(arg: LiquidNamedArgument): SupportedParamTypes {
-  switch (arg.value.type) {
+export function inferArgumentType(arg: LiquidExpression): SupportedParamTypes {
+  switch (arg.type) {
     case NodeTypes.String:
       return SupportedParamTypes.String;
     case NodeTypes.Number:
@@ -47,7 +47,7 @@ export function inferArgumentType(arg: LiquidNamedArgument): SupportedParamTypes
       return SupportedParamTypes.Object;
     default:
       // This ensures that we have a case for every possible type for arg.value
-      return assertNever(arg.value);
+      return assertNever(arg);
   }
 }
 

packages/theme-check-docs-updater/CHANGELOG.md

@@ -9,6 +9,7 @@
 - Updated dependencies [21354237]
 - Updated dependencies [dafba833]
 - Updated dependencies [c55077e7]
+- Updated dependencies [48d4d79e]
   - @shopify/theme-check-common@3.11.0
 
 ## 3.10.0

packages/theme-check-node/CHANGELOG.md

@@ -18,6 +18,7 @@
 - Updated dependencies [21354237]
 - Updated dependencies [dafba833]
 - Updated dependencies [c55077e7]
+- Updated dependencies [48d4d79e]
   - @shopify/theme-check-common@3.11.0
   - @shopify/theme-check-docs-updater@3.11.0
 

packages/theme-language-server-common/CHANGELOG.md

@@ -22,6 +22,7 @@
 - Updated dependencies [21354237]
 - Updated dependencies [dafba833]
 - Updated dependencies [c55077e7]
+- Updated dependencies [48d4d79e]
 - Updated dependencies [de877551]
   - @shopify/theme-check-common@3.11.0
   - @shopify/liquid-html-parser@2.7.0

packages/vscode-extension/CHANGELOG.md

@@ -16,6 +16,7 @@
 - Updated dependencies [21354237]
 - Updated dependencies [dafba833]
 - Updated dependencies [c55077e7]
+- Updated dependencies [48d4d79e]
 - Updated dependencies [de877551]
   - @shopify/prettier-plugin-liquid@1.9.0
   - @shopify/theme-check-common@3.11.0