diff --git a/.changeset/undefined-object-offset-continue.md b/.changeset/undefined-object-offset-continue.md new file mode 100644 index 000000000..a1ae7494c --- /dev/null +++ b/.changeset/undefined-object-offset-continue.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Stop `UndefinedObject` from reporting `continue` in `{% for ... offset: continue %}`. It is a Liquid keyword there, not a variable. diff --git a/packages/theme-check-common/src/checks/undefined-object/index.spec.ts b/packages/theme-check-common/src/checks/undefined-object/index.spec.ts index 0f2ea6ebc..92704a8d9 100644 --- a/packages/theme-check-common/src/checks/undefined-object/index.spec.ts +++ b/packages/theme-check-common/src/checks/undefined-object/index.spec.ts @@ -121,6 +121,41 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); + it('should not report an offense for the continue keyword in a for loop offset', async () => { + const sourceCode = ` + {% for product in collections.all.products limit: 3 %}{{ product.title }}{% endfor %} + {% for product in collections.all.products limit: 3 offset: continue %}{{ product.title }}{% endfor %} + {%- for product in collections.all.products + limit: 3 + offset: continue + -%} + {{ product.title }} + {%- endfor -%} + `; + + const offenses = await runLiquidCheck(UndefinedObject, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('should report an offense when continue is used as a variable outside of a for loop offset', async () => { + const sourceCode = ` + {{ continue }} + {% for product in collections.all.products limit: continue %}{% endfor %} + {% for product in collections.all.products offset: continue.size %}{% endfor %} + {% tablerow product in collections.all.products offset: continue %}{% endtablerow %} + `; + + const offenses = await runLiquidCheck(UndefinedObject, sourceCode); + + expect(offenses.map((e) => e.message)).toEqual([ + "Unknown object 'continue' used.", + "Unknown object 'continue' used.", + "Unknown object 'continue' used.", + "Unknown object 'continue' used.", + ]); + }); + it('should report an offense when object is defined in a for loop but used outside of the scope', async () => { const sourceCode = ` {% for c in collections %} diff --git a/packages/theme-check-common/src/checks/undefined-object/index.ts b/packages/theme-check-common/src/checks/undefined-object/index.ts index 4c04c3cc3..0b5218ca5 100644 --- a/packages/theme-check-common/src/checks/undefined-object/index.ts +++ b/packages/theme-check-common/src/checks/undefined-object/index.ts @@ -151,6 +151,7 @@ export const UndefinedObject: LiquidCheckDefinition = { const parent = last(ancestors); if (isLiquidTag(parent) && isLiquidTagCapture(parent)) return; + if (isForOffsetContinue(node, ancestors)) return; variables.push(node); }, @@ -298,6 +299,26 @@ function isDefinedInScope(variablePosition: Position, scope: Scope) { return isVariableAfterScopeStart && isVariableBeforeScopeEnd; } +/** + * {% for x in y offset: continue %} + * + * `continue` is a keyword here, not a variable: it resumes the loop where the + * last loop over the same collection stopped. Only `for` supports it, not + * `tablerow`. + */ +function isForOffsetContinue(node: LiquidVariableLookup, ancestors: LiquidHtmlNode[]): boolean { + if (node.name !== 'continue' || node.lookups.length > 0) return false; + + const [tag, markup, argument] = ancestors.slice(-3); + return ( + argument?.type === NodeTypes.NamedArgument && + argument.name === 'offset' && + markup?.type === NodeTypes.ForMarkup && + isLiquidTag(tag) && + tag.name === NamedTags.for + ); +} + function isLiquidTag(node?: LiquidHtmlNode): node is LiquidTag { return node?.type === NodeTypes.LiquidTag; }