diff --git a/.changeset/forty-moose-pay.md b/.changeset/forty-moose-pay.md new file mode 100644 index 000000000..ab17da469 --- /dev/null +++ b/.changeset/forty-moose-pay.md @@ -0,0 +1,9 @@ +--- +'@shopify/theme-check-common': minor +'@shopify/theme-check-node': minor +--- + +Theme check verifies if setting key exists in schema + +- Check if the keys inside `presets.[].settings` and `default.settings` exist as `settings.[].id` in the same file +- Check if the keys inside `presets.[](recursive .blocks.[]).settings` and `default.blocks.[].settings` exist as `settings.[].id` inside the referenced block's file \ No newline at end of file diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index 120388636..e4058c453 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -42,6 +42,7 @@ import { ValidJSON } from './valid-json'; import { ValidLocalBlocks } from './valid-local-blocks'; import { ValidSchema } from './valid-schema'; import { ValidSchemaName } from './valid-schema-name'; +import { ValidSettingsKey } from './valid-settings-key'; import { ValidStaticBlockType } from './valid-static-block-type'; import { VariableName } from './variable-name'; import { AppBlockMissingSchema } from './app-block-missing-schema'; @@ -91,6 +92,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ ValidJSON, ValidLocalBlocks, ValidSchema, + ValidSettingsKey, ValidStaticBlockType, VariableName, ValidSchemaName, diff --git a/packages/theme-check-common/src/checks/valid-block-target/index.spec.ts b/packages/theme-check-common/src/checks/valid-block-target/index.spec.ts index 5312e0096..1d251204e 100644 --- a/packages/theme-check-common/src/checks/valid-block-target/index.spec.ts +++ b/packages/theme-check-common/src/checks/valid-block-target/index.spec.ts @@ -1,7 +1,6 @@ import { expect, describe, it } from 'vitest'; import { ValidBlockTarget } from './index'; import { check, MockTheme } from '../../test'; -import { Dependencies } from '../../types'; describe('Module: ValidBlockTarget', () => { const paths = ['sections', 'blocks']; diff --git a/packages/theme-check-common/src/checks/valid-block-target/index.ts b/packages/theme-check-common/src/checks/valid-block-target/index.ts index 5f3aeb15e..ed98095e4 100644 --- a/packages/theme-check-common/src/checks/valid-block-target/index.ts +++ b/packages/theme-check-common/src/checks/valid-block-target/index.ts @@ -15,7 +15,7 @@ import { validateNestedBlocks, validateBlockFileExistence, reportWarning, -} from './block-utils'; +} from '../../utils'; type BlockNodeWithPath = { node: Section.Block | ThemeBlock.Block | Preset.Block; path: string[]; diff --git a/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts b/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts new file mode 100644 index 000000000..2e02c9d4a --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-settings-key/index.spec.ts @@ -0,0 +1,317 @@ +import { expect, describe, it } from 'vitest'; +import { ValidSettingsKey } from './index'; +import { check } from '../../test'; + +describe('Moduel: ValidSettingsKey', () => { + const schemaTemplate = { + name: 'Example', + settings: [ + { + id: 'example-text-setting', + type: 'text', + label: 'Example Text Setting', + }, + ], + }; + + describe('default settings', () => { + it('does not report an error when default setting exists', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + default: { + settings: { + 'example-text-setting': 'value', + }, + }, + }), + }; + + const offenses = await check(theme, [ValidSettingsKey]); + expect(offenses).to.have.length(0); + }); + + it('reports an error when default setting does not exist', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + default: { + settings: { + 'non-existent-setting': 'value', + }, + }, + }), + }; + + const offenses = await check(theme, [ValidSettingsKey]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'non-existent-setting' does not exist in schema.`, + ); + }); + + it('does not report an error when default settings are defined in a block schema', async () => { + const theme = { + 'blocks/example.liquid': toLiquidFile({ + ...schemaTemplate, + default: { + settings: { + 'non-existent-setting': 'value', + }, + }, + }), + }; + + const offenses = await check(theme, [ValidSettingsKey]); + expect(offenses).to.have.length(0); + }); + }); + + describe('presets settings', () => { + it('does not report an error when presets setting exists', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + presets: [ + { + settings: { + 'example-text-setting': 'value', + }, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey]); + expect(offenses).to.have.length(0); + }); + + it('reports an error when presets setting does not exist', async () => { + const theme = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + presets: [ + { + settings: { + 'non-existent-setting-1': 'value', + }, + }, + { + settings: { + 'non-existent-setting-2': 'value', + }, + }, + ], + }), + }; + + const offenses = await check(theme, [ValidSettingsKey]); + expect(offenses).to.have.length(2); + expect(offenses[0].message).to.equal( + `Setting 'non-existent-setting-1' does not exist in schema.`, + ); + expect(offenses[1].message).to.equal( + `Setting 'non-existent-setting-2' does not exist in schema.`, + ); + }); + }); + + const tests = [ + { + label: 'default', + blockTemplate: (blocks: any[]) => { + return { + default: { + blocks, + }, + }; + }, + }, + { + label: 'presets', + blockTemplate: (blocks: any[]) => { + return { + presets: [ + { + blocks, + }, + ], + }; + }, + }, + ]; + + tests.forEach(({ label, blockTemplate }) => { + describe(`${label} block settings`, () => { + describe('referenced blocks', () => { + const referencedBlock = { + 'blocks/referenced.liquid': toLiquidFile(schemaTemplate), + }; + + it(`does not report an error when ${label} block setting exists in referenced file`, async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + ...blockTemplate([ + { + type: 'referenced', + settings: { + 'example-text-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check( + { + ...referencedBlock, + ...fileToTest, + }, + [ValidSettingsKey], + ); + expect(offenses).to.have.length(0); + }); + + it('does not report an error when referenced file does not exist', async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + ...blockTemplate([ + { + type: 'non-existent-file', + settings: { + 'non-existent-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check( + { + ...referencedBlock, + ...fileToTest, + }, + [ValidSettingsKey], + ); + expect(offenses).to.have.length(0); + }); + + it(`reports an error when ${label} block setting does not exist in referenced file`, async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...schemaTemplate, + ...blockTemplate([ + { + type: 'referenced', + settings: { + 'non-existent-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check( + { + ...referencedBlock, + ...fileToTest, + }, + [ValidSettingsKey], + ); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'non-existent-setting' does not exist in 'blocks/referenced.liquid'.`, + ); + }); + }); + + describe('local blocks', () => { + const localBlocksTemplate = { + blocks: [ + { + type: 'local-block', + name: 'Local block', + settings: [ + { + id: 'local-setting', + type: 'text', + label: 'Local Setting', + }, + ], + }, + ], + }; + + it(`reports an error when ${label} block setting does not exist in existing local block`, async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...localBlocksTemplate, + ...blockTemplate([ + { + type: 'local-block', + settings: { + 'non-existent-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check(fileToTest, [ValidSettingsKey]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + `Setting 'non-existent-setting' does not exist in schema.`, + ); + }); + + it(`does not report an error when ${label} block setting does not exist in non-existent local block`, async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...localBlocksTemplate, + ...blockTemplate([ + { + type: 'non-existent-local-block', + settings: { + 'non-existent-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check(fileToTest, [ValidSettingsKey]); + expect(offenses).to.have.length(0); + }); + + it(`does not report an error when ${label} block setting exists in local block`, async () => { + const fileToTest = { + 'sections/example.liquid': toLiquidFile({ + ...localBlocksTemplate, + ...blockTemplate([ + { + type: 'local-block', + settings: { + 'local-setting': 'value', + }, + }, + ]), + }), + }; + + const offenses = await check(fileToTest, [ValidSettingsKey]); + expect(offenses).to.have.length(0); + }); + }); + }); + }); +}); + +function toLiquidFile(content: any) { + return ` + {% schema %} + ${JSON.stringify(content)} + {% endschema %} + `; +} diff --git a/packages/theme-check-common/src/checks/valid-settings-key/index.ts b/packages/theme-check-common/src/checks/valid-settings-key/index.ts new file mode 100644 index 000000000..e31d90e8b --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-settings-key/index.ts @@ -0,0 +1,129 @@ +import { + LiquidCheckDefinition, + Severity, + SourceCodeType, + Preset, + Section, + ThemeBlock, + JSONNode, + Setting, +} from '../../types'; +import { nodeAtPath } from '../../json'; +import { getSchema, isSectionSchema } from '../../to-schema'; +import { BlockNodeWithPath, getBlocks, reportWarning } from '../../utils'; + +export const ValidSettingsKey: LiquidCheckDefinition = { + meta: { + code: 'ValidSettingsKey', + name: 'Validate settings key in presets', + docs: { + description: + "Ensures settings key only references valid settings defined in it's respective schema", + recommended: true, + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-settings-key', + }, + type: SourceCodeType.LiquidHtml, + severity: Severity.ERROR, + schema: {}, + targets: [], + }, + + create(context) { + return { + async LiquidRawTag(node) { + if (node.name !== 'schema' || node.body.kind !== 'json') return; + + const offset = node.blockStartPosition.end; + const schema = await getSchema(context); + + const { validSchema, ast } = schema ?? {}; + if (!validSchema || validSchema instanceof Error) return; + if (!ast || ast instanceof Error) return; + + const { rootLevelLocalBlocks, presetLevelBlocks } = getBlocks(validSchema); + + // Check if presets settings match schema-level settings + if (validSchema.presets) { + for (let i = 0; i < validSchema.presets.length; i++) { + const settingsNode = nodeAtPath(ast, ['presets', i, 'settings']); + + validateSettingsKey(context, offset, settingsNode, validSchema.settings); + } + } + + if (isSectionSchema(schema) && 'default' in validSchema && validSchema.default) { + // Check if default settings match schema-level settings + const settingsNode = nodeAtPath(ast, ['default', 'settings']); + + validateSettingsKey(context, offset, settingsNode, validSchema.settings); + + // Check if default block settings match the settings defined in the block file's schema + validSchema.default.blocks?.forEach(async (block, i) => { + const settingsNode = nodeAtPath(ast, ['default', 'blocks', i, 'settings']); + + validateReferencedBlock(context, offset, settingsNode, rootLevelLocalBlocks, block); + }); + } + + // Check if preset block settings match the settings defined in the block file's schema + for (let [_depthStr, blocks] of Object.entries(presetLevelBlocks)) { + blocks.forEach(async ({ node: blockNode, path }) => { + const settingsNode = nodeAtPath(ast, path.slice(0, -1).concat('settings')); + + validateReferencedBlock(context, offset, settingsNode, rootLevelLocalBlocks, blockNode); + }); + } + }, + }; + }, +}; + +async function validateReferencedBlock( + context: any, + offset: number, + settingsNode: JSONNode | undefined, + localBlocks: BlockNodeWithPath[], + referencedBlock: Preset.Block | Section.Block | ThemeBlock.Block, +) { + if (localBlocks.length > 0) { + const localBlock = localBlocks.find( + (localBlock) => localBlock.node.type === referencedBlock.type, + ); + + if (!localBlock) return; + + const localBlockNode = localBlock.node as Section.LocalBlock; + + validateSettingsKey(context, offset, settingsNode, localBlockNode.settings); + } else { + const blockSchema = await context.getBlockSchema?.(referencedBlock.type); + const { validSchema: validBlockSchema } = blockSchema ?? {}; + if (!validBlockSchema || validBlockSchema instanceof Error) return; + + validateSettingsKey(context, offset, settingsNode, validBlockSchema.settings, referencedBlock); + } +} + +function validateSettingsKey( + context: any, + offset: number, + settingsNode: JSONNode | undefined, + validSettings: Setting.Any[] | undefined, + blockNode?: Section.Block | ThemeBlock.Block | Preset.Block, +) { + if (!settingsNode || settingsNode.type !== 'Object') return; + + for (const setting of settingsNode.children) { + const settingExists = validSettings?.find( + (validSetting) => validSetting?.id === setting.key.value, + ); + + if (!settingExists) { + const errorMessage = blockNode + ? `Setting '${setting.key.value}' does not exist in 'blocks/${blockNode.type}.liquid'.` + : `Setting '${setting.key.value}' does not exist in schema.`; + + reportWarning(errorMessage, offset, setting.key, context); + } + } +} diff --git a/packages/theme-check-common/src/checks/valid-block-target/block-utils.ts b/packages/theme-check-common/src/utils/block.ts similarity index 95% rename from packages/theme-check-common/src/checks/valid-block-target/block-utils.ts rename to packages/theme-check-common/src/utils/block.ts index 8826aa41f..7704681f1 100644 --- a/packages/theme-check-common/src/checks/valid-block-target/block-utils.ts +++ b/packages/theme-check-common/src/utils/block.ts @@ -1,15 +1,7 @@ -import { - JSONNode, - LiteralNode, - Preset, - Section, - SourceCodeType, - Theme, - ThemeBlock, -} from '../../types'; -import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; -import { Context } from '../../types'; -import { doesFileExist } from '../../utils/file-utils'; +import { JSONNode, LiteralNode, Preset, Section, SourceCodeType, ThemeBlock } from '../types'; +import { getLocEnd, getLocStart, nodeAtPath } from '../json'; +import { Context } from '../types'; +import { doesFileExist } from './file-utils'; export type BlockNodeWithPath = { node: Section.Block | ThemeBlock.Block | Preset.Block; @@ -213,7 +205,7 @@ export async function validateNestedBlocks( export function reportWarning( message: string, offset: number, - astNode: LiteralNode, + astNode: JSONNode, context: Context, ) { context.report({ diff --git a/packages/theme-check-common/src/utils/index.ts b/packages/theme-check-common/src/utils/index.ts index 0a2062a53..0b1f4c83e 100644 --- a/packages/theme-check-common/src/utils/index.ts +++ b/packages/theme-check-common/src/utils/index.ts @@ -5,3 +5,4 @@ export * from './error'; export * from './types'; export * from './memo'; export * from './indexBy'; +export * from './block'; diff --git a/packages/theme-check-node/configs/all.yml b/packages/theme-check-node/configs/all.yml index 61fef574d..da20b6ab9 100644 --- a/packages/theme-check-node/configs/all.yml +++ b/packages/theme-check-node/configs/all.yml @@ -142,6 +142,9 @@ ValidSchema: ValidSchemaName: enabled: true severity: 0 +ValidSettingsKey: + enabled: true + severity: 0 ValidStaticBlockType: enabled: true severity: 0 diff --git a/packages/theme-check-node/configs/recommended.yml b/packages/theme-check-node/configs/recommended.yml index 5c63a4481..b7b996472 100644 --- a/packages/theme-check-node/configs/recommended.yml +++ b/packages/theme-check-node/configs/recommended.yml @@ -120,6 +120,9 @@ ValidSchema: ValidSchemaName: enabled: true severity: 0 +ValidSettingsKey: + enabled: true + severity: 0 ValidStaticBlockType: enabled: true severity: 0