update the check so that it's scope aware

Author: shoplando

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

@@ -473,4 +473,96 @@ describe('Module: UndefinedObject', () => {
 
     expect(offenses).toHaveLength(0);
   });
+
+  describe('Scope-aware checking for inline snippets', () => {
+    it('No doc tags anywhere - should skip all checks', async () => {
+      const sourceCode = `
+        {{ undefined_in_parent }}
+
+        {% snippet inline_one %}
+          {{ undefined_in_inline }}
+        {% endsnippet %}
+      `;
+
+      const filePath = 'snippets/file.liquid';
+      const offenses = await runLiquidCheck(UndefinedObject, sourceCode, filePath);
+
+      expect(offenses).toHaveLength(0);
+    });
+
+    it('Doc tag in parent only - should check parent, skip inline snippets', async () => {
+      const sourceCode = `
+        {% doc %}
+          @param {string} parent_param
+        {% enddoc %}
+
+        {{ parent_param }}
+        {{ undefined_in_parent }}
+
+        {% snippet inline_one %}
+          {{ undefined_in_inline }}
+        {% endsnippet %}
+      `;
+
+      const filePath = 'snippets/file.liquid';
+      const offenses = await runLiquidCheck(UndefinedObject, sourceCode, filePath);
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe("Unknown object 'undefined_in_parent' used.");
+    });
+
+    it('Doc tag in inline snippet only - should skip parent, check inline snippet', async () => {
+      const sourceCode = `
+        {{ undefined_in_parent }}
+
+        {% snippet inline_one %}
+          {% doc %}
+            @param {string} inline_param
+          {% enddoc %}
+          
+          {{ inline_param }}
+          {{ undefined_in_inline }}
+        {% endsnippet %}
+      `;
+
+      const filePath = 'snippets/file.liquid';
+      const offenses = await runLiquidCheck(UndefinedObject, sourceCode, filePath);
+
+      expect(offenses).toHaveLength(1);
+      expect(offenses[0].message).toBe("Unknown object 'undefined_in_inline' used.");
+    });
+
+    it('Doc tags in both - should check both, but skip inline snippets without docs', async () => {
+      const sourceCode = `
+        {% doc %}
+          @param {string} parent_param
+        {% enddoc %}
+
+        {{ parent_param }}
+        {{ undefined_in_parent }}
+
+        {% snippet inline_one %}
+          {% doc %}
+            @param {string} inline_param
+          {% enddoc %}
+          
+          {{ inline_param }}
+          {{ undefined_in_inline }}
+        {% endsnippet %}
+
+        {% snippet inline_two %}
+          {{ anything }}
+        {% endsnippet %}
+      `;
+
+      const filePath = 'snippets/file.liquid';
+      const offenses = await runLiquidCheck(UndefinedObject, sourceCode, filePath);
+
+      expect(offenses).toHaveLength(2);
+      expect(offenses.map((o) => o.message)).toEqual([
+        "Unknown object 'undefined_in_parent' used.",
+        "Unknown object 'undefined_in_inline' used.",
+      ]);
+    });
+  });
 });

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

@@ -7,6 +7,7 @@ import {
   LiquidTagDecrement,
   LiquidTagFor,
   LiquidTagIncrement,
+  LiquidTagSnippet,
   LiquidTagTablerow,
   LiquidVariableLookup,
   NamedTags,
@@ -16,7 +17,7 @@ import {
 import { LiquidCheckDefinition, Severity, SourceCodeType, ThemeDocset } from '../../types';
 import { isError, last } from '../../utils';
 import { hasLiquidDoc } from '../../liquid-doc/liquidDoc';
-import { isWithinRawTagThatDoesNotParseItsContents } from '../utils';
+import { isWithinRawTagThatDoesNotParseItsContents, findInlineSnippetAncestor } from '../utils';
 
 type Scope = { start?: number; end?: number };
 
@@ -38,13 +39,14 @@ export const UndefinedObject: LiquidCheckDefinition = {
   create(context) {
     const relativePath = context.toRelativePath(context.file.uri);
     const ast = context.file.ast;
+    const isSnippetFile = relativePath.startsWith('snippets/');
 
     if (isError(ast)) return {};
 
     /**
      * Skip this check when a snippet does not have the presence of doc tags.
      */
-    if (relativePath.startsWith('snippets/') && !hasLiquidDoc(ast)) return {};
+    if (isSnippetFile && !hasLiquidDoc(ast)) return {};
 
     /**
      * Skip this check when definitions for global objects are unavailable.
@@ -56,7 +58,9 @@ export const UndefinedObject: LiquidCheckDefinition = {
     const themeDocset = context.themeDocset;
     const scopedVariables: Map<string, Scope[]> = new Map();
     const fileScopedVariables: Set<string> = new Set();
-    const variables: LiquidVariableLookup[] = [];
+    const variables: Array<{ node: LiquidVariableLookup; inlineSnippet: LiquidTag | null }> = [];
+    const inlineSnippetsWithDocTags: Set<number> = new Set();
+    let snippetFileHasDocTag = false;
 
     function indexVariableScope(variableName: string | null, scope: Scope) {
       if (!variableName) return;
@@ -66,9 +70,27 @@ export const UndefinedObject: LiquidCheckDefinition = {
     }
 
     return {
-      async LiquidDocParamNode(node: LiquidDocParamNode) {
+      async LiquidRawTag(node, ancestors) {
+        if (!isSnippetFile || node.name !== 'doc') return;
+
+        const parent = last(ancestors);
+        if (isLiquidTag(parent) && isLiquidTagSnippet(parent)) {
+          inlineSnippetsWithDocTags.add(parent.position.start);
+        } else {
+          snippetFileHasDocTag = true;
+        }
+      },
+
+      async LiquidDocParamNode(node: LiquidDocParamNode, ancestors: LiquidHtmlNode[]) {
         const paramName = node.paramName?.value;
-        if (paramName) {
+        if (!paramName) return;
+        const snippetAncestor = findInlineSnippetAncestor(ancestors);
+        if (snippetAncestor) {
+          indexVariableScope(paramName, {
+            start: snippetAncestor.blockStartPosition.end,
+            end: snippetAncestor.blockEndPosition?.start,
+          });
+        } else {
           fileScopedVariables.add(paramName);
         }
       },
@@ -134,6 +156,10 @@ export const UndefinedObject: LiquidCheckDefinition = {
             end: node.blockEndPosition?.start,
           });
         }
+
+        if (isLiquidTagSnippet(node) && node.markup.name) {
+          fileScopedVariables.add(node.markup.name);
+        }
       },
 
       async VariableLookup(node, ancestors) {
@@ -142,21 +168,33 @@ export const UndefinedObject: LiquidCheckDefinition = {
         const parent = last(ancestors);
         if (isLiquidTag(parent) && isLiquidTagCapture(parent)) return;
 
-        if (parent?.type === NodeTypes.RenderMarkup && parent.snippet === node) return;
-
-        if (isLiquidTag(parent) && parent.name === 'snippet' && parent.markup === node) return;
+        if (isLiquidTag(parent) && isLiquidTagSnippet(parent)) return;
 
-        variables.push(node);
+        const inlineSnippet = findInlineSnippetAncestor(ancestors);
+        variables.push({ node, inlineSnippet });
       },
 
       async onCodePathEnd() {
         const objects = await globalObjects(themeDocset, relativePath);
 
         objects.forEach((obj) => fileScopedVariables.add(obj.name));
 
-        variables.forEach((variable) => {
+        variables.forEach(({ node: variable, inlineSnippet }) => {
           if (!variable.name) return;
 
+          // For snippet files: only check variables if they're in a scope with a doc tag.
+          if (isSnippetFile) {
+            if (inlineSnippet) {
+              if (!inlineSnippetsWithDocTags.has(inlineSnippet.position.start)) {
+                return;
+              }
+            } else {
+              if (!snippetFileHasDocTag) {
+                return;
+              }
+            }
+          }
+
           const isVariableDefined = isDefined(
             variable.name,
             variable.position,
@@ -272,6 +310,10 @@ function isLiquidTagCapture(node: LiquidTag): node is LiquidTagCapture {
   return node.name === NamedTags.capture;
 }
 
+function isLiquidTagSnippet(node: LiquidTag): node is LiquidTagSnippet {
+  return node.name === NamedTags.snippet;
+}
+
 function isLiquidTagAssign(node: LiquidTag): node is LiquidTagAssign {
   return node.name === NamedTags.assign && typeof node.markup !== 'string';
 }

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

@@ -13,6 +13,7 @@ import {
   LiquidTagTablerow,
   LiquidTag,
   LoopNamedTags,
+  NamedTags,
 } from '@shopify/liquid-html-parser';
 import { LiquidHtmlNodeOfType as NodeOfType } from '../types';
 
@@ -104,6 +105,16 @@ export function isLoopLiquidTag(tag: LiquidTag): tag is LiquidTagFor | LiquidTag
   return LoopNamedTags.includes(tag.name as any);
 }
 
+export function findInlineSnippetAncestor(ancestors: LiquidHtmlNode[]) {
+  for (let i = ancestors.length - 1; i >= 0; i--) {
+    const ancestor = ancestors[i];
+    if (ancestor.type === NodeTypes.LiquidTag && ancestor.name === NamedTags.snippet) {
+      return ancestor;
+    }
+  }
+  return null;
+}
+
 const RawTagsThatDoNotParseTheirContents = ['raw', 'stylesheet', 'javascript', 'schema'];
 
 function isRawTagThatDoesNotParseItsContent(node: LiquidHtmlNode) {