Skip to content

Commit a313b96

Browse files
committed
Updated the UnusedDocParam theme check for inline snippets
1 parent b4049b2 commit a313b96

3 files changed

Lines changed: 101 additions & 9 deletions

File tree

.changeset/real-walls-kiss.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme-check-common': minor
3+
---
4+
5+
Updates the UnusedDocParam theme check so that it also takes into accounts for inline snippets and its scoping

packages/theme-check-common/src/checks/unused-doc-param/index.spec.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,44 @@ describe('Module: UnusedDocParam', () => {
8686
expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param2'");
8787
});
8888
});
89+
90+
it('should report a warning when a variable is defined but not used in an inline snippet', async () => {
91+
const sourceCode = `
92+
{% snippet %}
93+
{% doc %}
94+
@param param1 - Example param
95+
{% enddoc %}
96+
{% endsnippet %}
97+
`;
98+
99+
const offenses = await runLiquidCheck(UnusedDocParam, sourceCode);
100+
101+
expect(offenses).to.have.length(1);
102+
expect(offenses[0].message).to.equal(
103+
"The parameter 'param1' is defined but not used in this inline snippet.",
104+
);
105+
expect(offenses[0].suggest).to.have.length(1);
106+
expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param1'");
107+
});
108+
109+
it('should report a warning when a variable is defined but used outside of the snippet tag', async () => {
110+
const sourceCode = `
111+
{% snippet %}
112+
{% doc %}
113+
@param param1 - Example param
114+
{% enddoc %}
115+
{% endsnippet %}
116+
117+
{{ param1 }}
118+
`;
119+
120+
const offenses = await runLiquidCheck(UnusedDocParam, sourceCode);
121+
122+
expect(offenses).to.have.length(1);
123+
expect(offenses[0].message).to.equal(
124+
"The parameter 'param1' is defined but not used in this inline snippet.",
125+
);
126+
expect(offenses[0].suggest).to.have.length(1);
127+
expect(offenses[0]!.suggest![0].message).to.equal("Remove unused parameter 'param1'");
128+
});
89129
});

packages/theme-check-common/src/checks/unused-doc-param/index.ts

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
import { LiquidDocParamNode, NodeTypes } from '@shopify/liquid-html-parser';
1+
import {
2+
LiquidDocParamNode,
3+
LiquidHtmlNode,
4+
NamedTags,
5+
NodeTypes,
6+
} from '@shopify/liquid-html-parser';
27
import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
3-
import { isLoopScopedVariable } from '../utils';
8+
import { findInlineSnippetAncestor, isLoopScopedVariable } from '../utils';
49

510
export const UnusedDocParam: LiquidCheckDefinition = {
611
meta: {
@@ -19,12 +24,31 @@ export const UnusedDocParam: LiquidCheckDefinition = {
1924
},
2025

2126
create(context) {
22-
const definedLiquidDocParams: Map<string, LiquidDocParamNode> = new Map();
23-
const usedVariables: Set<string> = new Set();
27+
const fileScopedLiquidDocParams: Map<string, LiquidDocParamNode> = new Map();
28+
const inlineSnippetScopedLiquidDocParams: Map<
29+
LiquidHtmlNode,
30+
Map<string, LiquidDocParamNode>
31+
> = new Map();
32+
33+
const fileScopedUsedVariables: Set<string> = new Set();
34+
const inlineSnippetScopedUsedVariables: Map<LiquidHtmlNode, Set<string>> = new Map();
2435

2536
return {
26-
async LiquidDocParamNode(node) {
27-
definedLiquidDocParams.set(node.paramName.value, node);
37+
async LiquidTag(node) {
38+
if (node.name === NamedTags.snippet) {
39+
inlineSnippetScopedLiquidDocParams.set(node, new Map());
40+
inlineSnippetScopedUsedVariables.set(node, new Set());
41+
}
42+
},
43+
44+
async LiquidDocParamNode(node, ancestors) {
45+
const snippetAncestor = findInlineSnippetAncestor(ancestors);
46+
47+
if (snippetAncestor) {
48+
inlineSnippetScopedLiquidDocParams.get(snippetAncestor)!.set(node.paramName.value, node);
49+
} else {
50+
fileScopedLiquidDocParams.set(node.paramName.value, node);
51+
}
2852
},
2953

3054
async VariableLookup(node, ancestors) {
@@ -33,13 +57,19 @@ export const UnusedDocParam: LiquidCheckDefinition = {
3357
node.name &&
3458
!isLoopScopedVariable(node.name, ancestors)
3559
) {
36-
usedVariables.add(node.name);
60+
const snippetAncestor = findInlineSnippetAncestor(ancestors);
61+
62+
if (snippetAncestor) {
63+
inlineSnippetScopedUsedVariables.get(snippetAncestor)!.add(node.name);
64+
} else {
65+
fileScopedUsedVariables.add(node.name);
66+
}
3767
}
3868
},
3969

4070
async onCodePathEnd() {
41-
for (const [variable, node] of definedLiquidDocParams.entries()) {
42-
if (!usedVariables.has(variable)) {
71+
for (const [variable, node] of fileScopedLiquidDocParams.entries()) {
72+
if (!fileScopedUsedVariables.has(variable)) {
4373
context.report({
4474
message: `The parameter '${variable}' is defined but not used in this file.`,
4575
startIndex: node.position.start,
@@ -53,6 +83,23 @@ export const UnusedDocParam: LiquidCheckDefinition = {
5383
});
5484
}
5585
}
86+
for (const [snippet, docParams] of inlineSnippetScopedLiquidDocParams.entries()) {
87+
for (const [variable, node] of docParams.entries()) {
88+
if (!inlineSnippetScopedUsedVariables.get(snippet)?.has(variable)) {
89+
context.report({
90+
message: `The parameter '${variable}' is defined but not used in this inline snippet.`,
91+
startIndex: node.position.start,
92+
endIndex: node.position.end,
93+
suggest: [
94+
{
95+
message: `Remove unused parameter '${variable}'`,
96+
fix: (corrector) => corrector.remove(node.position.start, node.position.end),
97+
},
98+
],
99+
});
100+
}
101+
}
102+
}
56103
},
57104
};
58105
},

0 commit comments

Comments
 (0)