Skip to content

Commit

Permalink
Merge pull request #598 from Shopify/navdeep-warn-liquid-setting-value
Browse files Browse the repository at this point in the history
Add LiquidFreeSettings Check
  • Loading branch information
navdeep5 authored Nov 26, 2024
2 parents e0cfd31 + 81d63a7 commit 611fc3b
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 0 deletions.
6 changes: 6 additions & 0 deletions .changeset/large-geese-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/theme-check-common': minor
'@shopify/theme-check-node': minor
---

Add LiquidFreeSettings check
2 changes: 2 additions & 0 deletions packages/theme-check-common/src/checks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { DeprecatedFilter } from './deprecated-filter';
import { DeprecatedTag } from './deprecated-tag';
import { ImgWidthAndHeight } from './img-width-and-height';
import { JSONSyntaxError } from './json-syntax-error';
import { LiquidFreeSettings } from './liquid-free-settings';
import { LiquidHTMLSyntaxError } from './liquid-html-syntax-error';
import { MatchingTranslations } from './matching-translations';
import { MissingAsset } from './missing-asset';
Expand Down Expand Up @@ -59,6 +60,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
DeprecateLazysizes,
ImgWidthAndHeight,
JSONSyntaxError,
LiquidFreeSettings,
LiquidHTMLSyntaxError,
MatchingTranslations,
MissingAsset,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
import { expect, describe, it } from 'vitest';
import { LiquidFreeSettings } from './index';
import { check, MockTheme } from '../../test';

describe('LiquidFreeSettings validation', () => {
const paths = ['sections', 'blocks'];
paths.forEach((path) => {
it(`should not report errors for valid settings without liquid logic in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "Section name",
"settings": [
{
"id": "text_value",
"type": "text",
"label": "Text Value",
"default": "Some text without liquid logic."
}
]
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(0);
});

it(`should not report errors for valid settings with dynamic source liquid logic in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "Section name",
"settings": [
{
"id": "text_value",
"type": "text",
"label": "Text Value",
"default": "Title {{ product.title }}"
}
]
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(0);
});

it(`should report an error when settings value contains Liquid logic in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "Section name",
"settings": [
{
"id": "input_with_logic",
"type": "text",
"label": "Input with Logic",
"default": "Hello {% if user %} User {% endif %}!"
}
]
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(1);
expect(offenses[0].message).to.equal('Settings values cannot contain liquid logic.');
});

it(`should report an error with correct indices when settings value contains Liquid logic in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "Section name",
"settings": [
{
"id": "input_with_logic",
"type": "text",
"label": "Input with Logic",
"default": "Hello {% if user %} User {% endif %}!"
}
]
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(1);
const content = theme[`${path}/test-section.liquid`];
const errorContent = content.slice(offenses[0].start.index, offenses[0].end.index);
expect(errorContent).to.equal('"Hello {% if user %} User {% endif %}!"');
});

it(`should report an error when settings value contains Liquid logic in object format in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "test",
"settings": {
"text_block": {
"type": "text",
"id": "text_block",
"label": "Text Block",
"default": "Hello {% if user %} User {% endif %}!"
}
}
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(1);
expect(offenses[0].message).to.equal('Settings values cannot contain liquid logic.');
});

it(`should report errors when preset block values contain Liquid logic in ${path} bucket`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "Section name",
"presets": [
{
"name": "Default",
"block": [
{
"type": "text",
"settings": {
"text": "Hello World!{% random liquid logic %}"
}
}
]
}
]
}
{% endschema %}
`,
};

const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(1);
expect(offenses[0].message).to.equal('Settings values cannot contain liquid logic.');
});

it(`should not report errors when settings value is available_if and it contains liquid logic`, async () => {
const theme: MockTheme = {
[`${path}/test-section.liquid`]: `
{% schema %}
{
"name": "test",
"settings": {
"text_block": {
"type": "text",
"id": "text_block",
"label": "Text Block",
"default": "Hello World!",
"available_if": "{% if user %} true {% endif %}"
}
}
}
{% endschema %}
`,
};
const offenses = await check(theme, [LiquidFreeSettings]);
expect(offenses).to.have.length(0);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { JSONNode, LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
import { toJSONAST } from '../../to-source-code';
import { visit } from '../../visitor';
import { LiteralNode } from 'json-to-ast';

export const LiquidFreeSettings: LiquidCheckDefinition = {
meta: {
code: 'LiquidFreeSettings',
name: 'Check for liquid free settings values',
docs: {
description: 'Ensures settings values are liquid free.',
recommended: true,
url: 'https://shopify.dev/docs/themes/tools/theme-check/checks/liquid-free-settings',
},
type: SourceCodeType.LiquidHtml,
severity: Severity.WARNING,
schema: {},
targets: [],
},

create(context) {
return {
async LiquidRawTag(node) {
if (node.name !== 'schema' || node.body.kind !== 'json') {
return;
}

const jsonString = node.source.slice(
node.blockStartPosition.end,
node.blockEndPosition.start,
);

const jsonFile = toJSONAST(jsonString);
if (jsonFile instanceof Error) return;

visit<SourceCodeType.JSON, void>(jsonFile, {
Property(schemaNode, ancestors) {
if (isInArrayWithParentKey(ancestors, 'settings') && isLiteralNode(schemaNode.value)) {
const { value, loc } = schemaNode.value;
const propertyValue = schemaNode.key.value;
if (
typeof value === 'string' &&
propertyValue !== 'available_if' &&
value.includes('{%') &&
value.includes('%}')
) {
context.report({
message: 'Settings values cannot contain liquid logic.',
startIndex: node.blockStartPosition.end + loc!.start.offset,
endIndex: node.blockStartPosition.end + loc!.end.offset,
});
}
}
},
});
},
};
},
};

function isLiteralNode(node: JSONNode): node is LiteralNode {
return node.type === 'Literal';
}

function isInArrayWithParentKey(ancestors: JSONNode[], parentKey: string): boolean {
return ancestors.some((ancestor, index) => {
const parent = ancestors[index - 1];
return (
(ancestor.type === 'Array' || ancestor.type === 'Object') &&
parent?.type === 'Property' &&
parent.key?.value === parentKey
);
});
}
3 changes: 3 additions & 0 deletions packages/theme-check-node/configs/all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ ImgWidthAndHeight:
JSONSyntaxError:
enabled: true
severity: 0
LiquidFreeSettings:
enabled: true
severity: 1
LiquidHTMLSyntaxError:
enabled: true
severity: 0
Expand Down
3 changes: 3 additions & 0 deletions packages/theme-check-node/configs/recommended.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ ImgWidthAndHeight:
JSONSyntaxError:
enabled: true
severity: 0
LiquidFreeSettings:
enabled: true
severity: 1
LiquidHTMLSyntaxError:
enabled: true
severity: 0
Expand Down

0 comments on commit 611fc3b

Please sign in to comment.