@shopify/theme-check-common - npm Package Compare versions

Comparing version 1.18.0 to 1.18.1

CHANGELOG.md

 # @shopify/theme-check-common
 
+## 1.18.1
+
+### Patch Changes
+
+- aa33c5f: Fix hover, completion and `UndefinedObject` reporting of `{% increment var %}` and `{% decrement var %}`
+- 0d71145: Fix UnusedAssign false positives in raw-like nodes
+- Updated dependencies [0d71145]
+  - @shopify/liquid-html-parser@1.1.0
+
 ## 1.18.0
@@ -4,0 +13,0 @@

dist/checks/undefined-object/index.js

@@ -77,2 +77,9 @@ "use strict";
                 }
+                /* {% increment var %} */
+                if ((isLiquidTagIncrement(node) || isLiquidTagDecrement(node)) &&
+                    node.markup.name !== null) {
+                    indexVariableScope(node.markup.name, {
+                        start: node.position.start,
+                    });
+                }
                 /**
@@ -178,2 +185,8 @@ * {% for x in y %}
 }
+function isLiquidTagIncrement(node) {
+    return node.name === liquid_html_parser_1.NamedTags.increment && typeof node.markup !== 'string';
+}
+function isLiquidTagDecrement(node) {
+    return node.name === liquid_html_parser_1.NamedTags.decrement && typeof node.markup !== 'string';
+}
 //# sourceMappingURL=index.js.map

dist/checks/undefined-object/index.spec.js

@@ -183,2 +183,12 @@ "use strict";
     });
+    (0, vitest_1.it)('should support {% increment var %} and {% decrement var %}', async () => {
+        for (const tag of ['increment', 'decrement']) {
+            const sourceCode = `
+        {% ${tag} var %}
+        {{ var }}
+      `;
+            const offenses = await (0, test_1.runLiquidCheck)(index_1.UndefinedObject, sourceCode);
+            (0, vitest_1.expect)(offenses).toHaveLength(0);
+        }
+    });
     (0, vitest_1.it)('should not report an offense when object is undefined in a "snippet" file', async () => {
@@ -185,0 +195,0 @@ const sourceCode = `

dist/checks/unused-assign/index.js

@@ -30,7 +30,14 @@ "use strict";
             async LiquidTag(node) {
-                if (!isLiquidTagAssign(node))
+                if (isLiquidTagAssign(node)) {
+                    assignedVariables.set(node.markup.name, node);
+                }
+                else if (isLiquidTagCapture(node) && node.markup.name) {
+                    assignedVariables.set(node.markup.name, node);
+                }
+            },
+            async VariableLookup(node, ancestors) {
+                const parentNode = ancestors.at(-1);
+                if (parentNode && isLiquidTagCapture(parentNode)) {
                     return;
-                assignedVariables.set(node.markup.name, node);
-            },
-            async VariableLookup(node) {
+                }
                 checkVariableUsage(node);
@@ -61,2 +68,5 @@ },
 }
+function isLiquidTagCapture(node) {
+    return (node.type == liquid_html_parser_1.NodeTypes.LiquidTag && node.name === 'capture' && typeof node.markup !== 'string');
+}
 //# sourceMappingURL=index.js.map

dist/checks/unused-assign/index.spec.js

@@ -50,3 +50,61 @@ "use strict";
     });
+    (0, vitest_1.it)('should not report unused assigns for things used in a capture tag', async () => {
+        const sourceCode = `
+      {% assign usedVar = "anotherValue" %}
+      {% capture foo %}
+        {{ usedVar }}
+      {% endcapture %}
+      {{ foo }}
+    `;
+        const offenses = await (0, test_1.runLiquidCheck)(index_1.UnusedAssign, sourceCode);
+        (0, vitest_1.expect)(offenses).to.be.empty;
+    });
+    (0, vitest_1.it)('should report unused capture', async () => {
+        const sourceCode = `
+      {% assign usedVar = "anotherValue" %}
+      {% capture foo %}
+        {{ usedVar }}
+      {% endcapture %}
+    `;
+        const offenses = await (0, test_1.runLiquidCheck)(index_1.UnusedAssign, sourceCode);
+        (0, vitest_1.expect)(offenses).not.to.be.empty;
+    });
+    (0, vitest_1.it)('should not report unused assigns for things used in a raw-like tag', async () => {
+        const tags = ['style', 'javascript', 'stylesheet'];
+        for (const tag of tags) {
+            const sourceCode = `
+        {% assign usedVar = 1 %}
+        {% ${tag} %}
+          {{ usedVar }}
+        {% end${tag} %}
+      `;
+            const offenses = await (0, test_1.runLiquidCheck)(index_1.UnusedAssign, sourceCode);
+            (0, vitest_1.expect)(offenses).to.be.empty;
+        }
+    });
+    (0, vitest_1.it)('should not report unused assigns for things used in a HTML raw-like tag', async () => {
+        const tags = ['style', 'script'];
+        for (const tag of tags) {
+            const sourceCode = `
+        {% assign usedVar = 1 %}
+        <${tag}>
+          {{ usedVar }}
+        </${tag}>
+      `;
+            const offenses = await (0, test_1.runLiquidCheck)(index_1.UnusedAssign, sourceCode);
+            (0, vitest_1.expect)(offenses).to.be.empty;
+        }
+    });
+    (0, vitest_1.it)('should report unused assign for things used in a {% raw %} tag', async () => {
+        const sourceCode = `
+        {% assign unusedVar = 1 %}
+        {% # It's a trap. It's not really used %}
+        {% raw %}
+          {{ unusedVar }}
+        {% endraw %}
+      `;
+        const offenses = await (0, test_1.runLiquidCheck)(index_1.UnusedAssign, sourceCode);
+        (0, vitest_1.expect)(offenses).to.have.lengthOf(1);
+    });
 });
 //# sourceMappingURL=index.spec.js.map

dist/disabled-checks/index.spec.js

@@ -24,5 +24,5 @@ "use strict";
     const checks = [test_checks_1.LiquidFilter, test_checks_1.RenderMarkup];
-    commentTypes.forEach((buildComment, index) => {
-        (0, vitest_1.describe)(`Comment variant ${index + 1}`, () => {
-            (0, vitest_1.it)('should disable checks for the entire document if comment is placed on the first line', async () => {
+    (0, vitest_1.describe)(`Comment variant`, () => {
+        (0, vitest_1.it)('should disable checks for the entire document if comment is placed on the first line', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `${buildComment('theme-check-disable LiquidFilter')}
@@ -38,4 +38,6 @@ {% comment %}
                 expectRenderMarkupOffense(offenses, 'something.liquid');
-            });
-            (0, vitest_1.it)('should disable all checks even if comment has additional spaces', async () => {
+            }
+        });
+        (0, vitest_1.it)('should disable all checks even if comment has additional spaces', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `${buildComment('  theme-check-disable     ')}
@@ -46,4 +48,6 @@ {{ 'asset' | random_filter }}
                 (0, vitest_1.expect)(offenses).to.have.length(0);
-            });
-            (0, vitest_1.it)('should disable all checks for a section of the template', async () => {
+            }
+        });
+        (0, vitest_1.it)('should disable all checks for a section of the template', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `{{ 'asset-1' | random_filter }}
@@ -61,4 +65,6 @@ {% render 'something-1' %}
                 expectRenderMarkupOffense(offenses, 'something-1.liquid');
-            });
-            (0, vitest_1.it)('should disable a specific check if check is included in the comment', async () => {
+            }
+        });
+        (0, vitest_1.it)('should disable a specific check if check is included in the comment', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `${buildComment('theme-check-disable LiquidFilter')}
@@ -73,4 +79,6 @@ {{ 'asset-1' | random_filter }}
                 expectRenderMarkupOffense(offenses, 'something.liquid');
-            });
-            (0, vitest_1.it)('should disable multiple checks if checks are separated by a comma (and maybe some spaces)', async () => {
+            }
+        });
+        (0, vitest_1.it)('should disable multiple checks if checks are separated by a comma (and maybe some spaces)', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -84,4 +92,6 @@ {{ 'asset-1' | random_filter }}
                 expectLiquidFilterOffense(offenses, file, 'asset-2');
-            });
-            (0, vitest_1.it)('should enable specific checks individually', async () => {
+            }
+        });
+        (0, vitest_1.it)('should enable specific checks individually', async () => {
+            for (const buildComment of commentTypes) {
                 const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -101,5 +111,7 @@ {{ 'asset-1' | random_filter }}
                 expectRenderMarkupOffense(offenses, 'something-3.liquid');
-            });
-            (0, vitest_1.describe)('Mix of general and specific commands', () => {
-                (0, vitest_1.it)('should not reenable specific check when all checks have been disabled before', async () => {
+            }
+        });
+        (0, vitest_1.describe)('Mix of general and specific commands', () => {
+            (0, vitest_1.it)('should not reenable specific check when all checks have been disabled before', async () => {
+                for (const buildComment of commentTypes) {
                     const file = `${buildComment('theme-check-disable')}
@@ -113,4 +125,6 @@ {{ 'asset-1' | random_filter }}
                     (0, vitest_1.expect)(offenses).to.have.length(0);
-                });
-                (0, vitest_1.it)('should reenable all checks when specific ones have been disabled before', async () => {
+                }
+            });
+            (0, vitest_1.it)('should reenable all checks when specific ones have been disabled before', async () => {
+                for (const buildComment of commentTypes) {
                     const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -126,3 +140,3 @@ {{ 'asset-3' | random_filter }}
                     expectRenderMarkupOffense(offenses, 'something-4.liquid');
-                });
+                }
             });
@@ -129,0 +143,0 @@ });

package.json

 {
   "name": "@shopify/theme-check-common",
-  "version": "1.18.0",
+  "version": "1.18.1",
   "license": "MIT",
@@ -23,3 +23,3 @@ "homepage": "https://github.com/Shopify/theme-tools/blob/main/packages/theme-check-common/README.md",
   "dependencies": {
-    "@shopify/liquid-html-parser": "1.0.0",
+    "@shopify/liquid-html-parser": "1.1.0",
     "cross-fetch": "^4.0.0",
@@ -26,0 +26,0 @@ "json-to-ast": "^2.1.0",

src/checks/undefined-object/index.spec.ts

@@ -236,2 +236,14 @@ import { expect, describe, it } from 'vitest';
 
+  it('should support {% increment var %} and {% decrement var %}', async () => {
+    for (const tag of ['increment', 'decrement']) {
+      const sourceCode = `
+        {% ${tag} var %}
+        {{ var }}
+      `;
+
+      const offenses = await runLiquidCheck(UndefinedObject, sourceCode);
+      expect(offenses).toHaveLength(0);
+    }
+  });
+
   it('should not report an offense when object is undefined in a "snippet" file', async () => {
@@ -238,0 +250,0 @@ const sourceCode = `

src/checks/undefined-object/index.ts

@@ -6,3 +6,5 @@ import {
   LiquidTagCapture,
+  LiquidTagDecrement,
   LiquidTagFor,
+  LiquidTagIncrement,
   LiquidTagTablerow,
@@ -96,2 +98,12 @@ LiquidVariableLookup,
 
+        /* {% increment var %} */
+        if (
+          (isLiquidTagIncrement(node) || isLiquidTagDecrement(node)) &&
+          node.markup.name !== null
+        ) {
+          indexVariableScope(node.markup.name, {
+            start: node.position.start,
+          });
+        }
+
         /**
@@ -222,1 +234,9 @@ * {% for x in y %}
 }
+
+function isLiquidTagIncrement(node: LiquidTag): node is LiquidTagIncrement {
+  return node.name === NamedTags.increment && typeof node.markup !== 'string';
+}
+
+function isLiquidTagDecrement(node: LiquidTag): node is LiquidTagDecrement {
+  return node.name === NamedTags.decrement && typeof node.markup !== 'string';
+}

src/checks/unused-assign/index.spec.ts

@@ -61,2 +61,70 @@ import { expect, describe, it } from 'vitest';
   });
+
+  it('should not report unused assigns for things used in a capture tag', async () => {
+    const sourceCode = `
+      {% assign usedVar = "anotherValue" %}
+      {% capture foo %}
+        {{ usedVar }}
+      {% endcapture %}
+      {{ foo }}
+    `;
+
+    const offenses = await runLiquidCheck(UnusedAssign, sourceCode);
+    expect(offenses).to.be.empty;
+  });
+
+  it('should report unused capture', async () => {
+    const sourceCode = `
+      {% assign usedVar = "anotherValue" %}
+      {% capture foo %}
+        {{ usedVar }}
+      {% endcapture %}
+    `;
+
+    const offenses = await runLiquidCheck(UnusedAssign, sourceCode);
+    expect(offenses).not.to.be.empty;
+  });
+
+  it('should not report unused assigns for things used in a raw-like tag', async () => {
+    const tags = ['style', 'javascript', 'stylesheet'];
+    for (const tag of tags) {
+      const sourceCode = `
+        {% assign usedVar = 1 %}
+        {% ${tag} %}
+          {{ usedVar }}
+        {% end${tag} %}
+      `;
+
+      const offenses = await runLiquidCheck(UnusedAssign, sourceCode);
+      expect(offenses).to.be.empty;
+    }
+  });
+
+  it('should not report unused assigns for things used in a HTML raw-like tag', async () => {
+    const tags = ['style', 'script'];
+    for (const tag of tags) {
+      const sourceCode = `
+        {% assign usedVar = 1 %}
+        <${tag}>
+          {{ usedVar }}
+        </${tag}>
+      `;
+
+      const offenses = await runLiquidCheck(UnusedAssign, sourceCode);
+      expect(offenses).to.be.empty;
+    }
+  });
+
+  it('should report unused assign for things used in a {% raw %} tag', async () => {
+    const sourceCode = `
+        {% assign unusedVar = 1 %}
+        {% # It's a trap. It's not really used %}
+        {% raw %}
+          {{ unusedVar }}
+        {% endraw %}
+      `;
+
+    const offenses = await runLiquidCheck(UnusedAssign, sourceCode);
+    expect(offenses).to.have.lengthOf(1);
+  });
 });

src/checks/unused-assign/index.ts

@@ -1,2 +0,8 @@
-import { LiquidTag, LiquidTagAssign, NodeTypes } from '@shopify/liquid-html-parser';
+import {
+  LiquidHtmlNode,
+  LiquidTag,
+  LiquidTagAssign,
+  LiquidTagCapture,
+  NodeTypes,
+} from '@shopify/liquid-html-parser';
 import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
@@ -21,3 +27,3 @@
   create(context) {
-    const assignedVariables: Map<string, LiquidTagAssign> = new Map();
+    const assignedVariables: Map<string, LiquidTagAssign | LiquidTagCapture> = new Map();
     const usedVariables: Set<string> = new Set();
@@ -33,7 +39,14 @@
       async LiquidTag(node) {
-        if (!isLiquidTagAssign(node)) return;
-        assignedVariables.set(node.markup.name, node);
+        if (isLiquidTagAssign(node)) {
+          assignedVariables.set(node.markup.name, node);
+        } else if (isLiquidTagCapture(node) && node.markup.name) {
+          assignedVariables.set(node.markup.name, node);
+        }
       },
 
-      async VariableLookup(node) {
+      async VariableLookup(node, ancestors) {
+        const parentNode = ancestors.at(-1);
+        if (parentNode && isLiquidTagCapture(parentNode)) {
+          return;
+        }
         checkVariableUsage(node);
@@ -66,1 +79,7 @@ },
 }
+
+function isLiquidTagCapture(node: LiquidHtmlNode): node is LiquidTagCapture {
+  return (
+    node.type == NodeTypes.LiquidTag && node.name === 'capture' && typeof node.markup !== 'string'
+  );
+}

src/disabled-checks/index.spec.ts

@@ -33,5 +33,5 @@ import { check } from '../test';
 
-  commentTypes.forEach((buildComment, index) => {
-    describe(`Comment variant ${index + 1}`, () => {
-      it('should disable checks for the entire document if comment is placed on the first line', async () => {
+  describe(`Comment variant`, () => {
+    it('should disable checks for the entire document if comment is placed on the first line', async () => {
+      for (const buildComment of commentTypes) {
         const file = `${buildComment('theme-check-disable LiquidFilter')}
@@ -48,5 +48,7 @@ {% comment %}
         expectRenderMarkupOffense(offenses, 'something.liquid');
-      });
+      }
+    });
 
-      it('should disable all checks even if comment has additional spaces', async () => {
+    it('should disable all checks even if comment has additional spaces', async () => {
+      for (const buildComment of commentTypes) {
         const file = `${buildComment('  theme-check-disable     ')}
@@ -58,5 +60,7 @@ {{ 'asset' | random_filter }}
         expect(offenses).to.have.length(0);
-      });
+      }
+    });
 
-      it('should disable all checks for a section of the template', async () => {
+    it('should disable all checks for a section of the template', async () => {
+      for (const buildComment of commentTypes) {
         const file = `{{ 'asset-1' | random_filter }}
@@ -75,5 +79,7 @@ {% render 'something-1' %}
         expectRenderMarkupOffense(offenses, 'something-1.liquid');
-      });
+      }
+    });
 
-      it('should disable a specific check if check is included in the comment', async () => {
+    it('should disable a specific check if check is included in the comment', async () => {
+      for (const buildComment of commentTypes) {
         const file = `${buildComment('theme-check-disable LiquidFilter')}
@@ -89,5 +95,7 @@ {{ 'asset-1' | random_filter }}
         expectRenderMarkupOffense(offenses, 'something.liquid');
-      });
+      }
+    });
 
-      it('should disable multiple checks if checks are separated by a comma (and maybe some spaces)', async () => {
+    it('should disable multiple checks if checks are separated by a comma (and maybe some spaces)', async () => {
+      for (const buildComment of commentTypes) {
         const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -102,5 +110,7 @@ {{ 'asset-1' | random_filter }}
         expectLiquidFilterOffense(offenses, file, 'asset-2');
-      });
+      }
+    });
 
-      it('should enable specific checks individually', async () => {
+    it('should enable specific checks individually', async () => {
+      for (const buildComment of commentTypes) {
         const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -121,6 +131,8 @@ {{ 'asset-1' | random_filter }}
         expectRenderMarkupOffense(offenses, 'something-3.liquid');
-      });
+      }
+    });
 
-      describe('Mix of general and specific commands', () => {
-        it('should not reenable specific check when all checks have been disabled before', async () => {
+    describe('Mix of general and specific commands', () => {
+      it('should not reenable specific check when all checks have been disabled before', async () => {
+        for (const buildComment of commentTypes) {
           const file = `${buildComment('theme-check-disable')}
@@ -134,5 +146,7 @@ {{ 'asset-1' | random_filter }}
           expect(offenses).to.have.length(0);
-        });
+        }
+      });
 
-        it('should reenable all checks when specific ones have been disabled before', async () => {
+      it('should reenable all checks when specific ones have been disabled before', async () => {
+        for (const buildComment of commentTypes) {
           const file = `${buildComment('theme-check-disable LiquidFilter, RenderMarkup')}
@@ -149,3 +163,3 @@ {{ 'asset-3' | random_filter }}
           expectRenderMarkupOffense(offenses, 'something-4.liquid');
-        });
+        }
       });
@@ -152,0 +166,0 @@ });

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

1168443

increased by0.96%

Lines of code

19654

increased by1.14%

Dependency changes

• Updated@shopify/liquid-html-parser@1.1.0