diff --git a/.changeset/real-walls-kiss.md b/.changeset/real-walls-kiss.md new file mode 100644 index 000000000..849c295d0 --- /dev/null +++ b/.changeset/real-walls-kiss.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': minor +--- + +Updates the UnusedDocParam theme check so that it also takes into accounts for inline snippets and its scoping diff --git a/packages/theme-check-common/src/checks/unused-doc-param/index.spec.ts b/packages/theme-check-common/src/checks/unused-doc-param/index.spec.ts index dc94f2dba..eed64ba84 100644 --- a/packages/theme-check-common/src/checks/unused-doc-param/index.spec.ts +++ b/packages/theme-check-common/src/checks/unused-doc-param/index.spec.ts @@ -86,4 +86,44 @@ describe('Module: UnusedDocParam', () => { expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param2'"); }); }); + + it('should report a warning when a variable is defined but not used in an inline snippet', async () => { + const sourceCode = ` + {% snippet %} + {% doc %} + @param param1 - Example param + {% enddoc %} + {% endsnippet %} + `; + + const offenses = await runLiquidCheck(UnusedDocParam, sourceCode); + + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "The parameter 'param1' is defined but not used in this inline snippet.", + ); + expect(offenses[0].suggest).to.have.length(1); + expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param1'"); + }); + + it('should report a warning when a variable is defined but used outside of the snippet tag', async () => { + const sourceCode = ` + {% snippet %} + {% doc %} + @param param1 - Example param + {% enddoc %} + {% endsnippet %} + + {{ param1 }} + `; + + const offenses = await runLiquidCheck(UnusedDocParam, sourceCode); + + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "The parameter 'param1' is defined but not used in this inline snippet.", + ); + expect(offenses[0].suggest).to.have.length(1); + expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param1'"); + }); }); diff --git a/packages/theme-check-common/src/checks/unused-doc-param/index.ts b/packages/theme-check-common/src/checks/unused-doc-param/index.ts index 28270cf46..44ab29588 100644 --- a/packages/theme-check-common/src/checks/unused-doc-param/index.ts +++ b/packages/theme-check-common/src/checks/unused-doc-param/index.ts @@ -1,6 +1,11 @@ -import { LiquidDocParamNode, NodeTypes } from '@shopify/liquid-html-parser'; +import { + LiquidDocParamNode, + LiquidHtmlNode, + NamedTags, + NodeTypes, +} from '@shopify/liquid-html-parser'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; -import { isLoopScopedVariable } from '../utils'; +import { findInlineSnippetAncestor, isLoopScopedVariable } from '../utils'; export const UnusedDocParam: LiquidCheckDefinition = { meta: { @@ -19,12 +24,31 @@ export const UnusedDocParam: LiquidCheckDefinition = { }, create(context) { - const definedLiquidDocParams: Map = new Map(); - const usedVariables: Set = new Set(); + const fileScopedLiquidDocParams: Map = new Map(); + const inlineSnippetScopedLiquidDocParams: Map< + LiquidHtmlNode, + Map + > = new Map(); + + const fileScopedUsedVariables: Set = new Set(); + const inlineSnippetScopedUsedVariables: Map> = new Map(); return { - async LiquidDocParamNode(node) { - definedLiquidDocParams.set(node.paramName.value, node); + async LiquidTag(node) { + if (node.name === NamedTags.snippet) { + inlineSnippetScopedLiquidDocParams.set(node, new Map()); + inlineSnippetScopedUsedVariables.set(node, new Set()); + } + }, + + async LiquidDocParamNode(node, ancestors) { + const snippetAncestor = findInlineSnippetAncestor(ancestors); + + if (snippetAncestor) { + inlineSnippetScopedLiquidDocParams.get(snippetAncestor)!.set(node.paramName.value, node); + } else { + fileScopedLiquidDocParams.set(node.paramName.value, node); + } }, async VariableLookup(node, ancestors) { @@ -33,13 +57,19 @@ export const UnusedDocParam: LiquidCheckDefinition = { node.name && !isLoopScopedVariable(node.name, ancestors) ) { - usedVariables.add(node.name); + const snippetAncestor = findInlineSnippetAncestor(ancestors); + + if (snippetAncestor) { + inlineSnippetScopedUsedVariables.get(snippetAncestor)!.add(node.name); + } else { + fileScopedUsedVariables.add(node.name); + } } }, async onCodePathEnd() { - for (const [variable, node] of definedLiquidDocParams.entries()) { - if (!usedVariables.has(variable)) { + for (const [variable, node] of fileScopedLiquidDocParams.entries()) { + if (!fileScopedUsedVariables.has(variable)) { context.report({ message: `The parameter '${variable}' is defined but not used in this file.`, startIndex: node.position.start, @@ -53,6 +83,23 @@ export const UnusedDocParam: LiquidCheckDefinition = { }); } } + for (const [snippet, docParams] of inlineSnippetScopedLiquidDocParams.entries()) { + for (const [variable, node] of docParams.entries()) { + if (!inlineSnippetScopedUsedVariables.get(snippet)?.has(variable)) { + context.report({ + message: `The parameter '${variable}' is defined but not used in this inline snippet.`, + startIndex: node.position.start, + endIndex: node.position.end, + suggest: [ + { + message: `Remove unused parameter '${variable}'`, + fix: (corrector) => corrector.remove(node.position.start, node.position.end), + }, + ], + }); + } + } + } }, }; },