From d7436b4a96235b9313056c462382038cab5e3ec2 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Wed, 4 Dec 2024 16:50:48 -0500 Subject: [PATCH 1/5] Add `JsonMissingBlock` check --- .changeset/mighty-planets-dream.md | 6 + .../theme-check-common/src/checks/index.ts | 2 + .../checks/json-missing-block/index.spec.ts | 378 ++++++++++++++++++ .../src/checks/json-missing-block/index.ts | 56 +++ .../json-missing-block/missing-block-utils.ts | 131 ++++++ packages/theme-check-common/src/to-schema.ts | 16 + packages/theme-check-node/configs/all.yml | 3 + .../theme-check-node/configs/recommended.yml | 3 + 8 files changed, 595 insertions(+) create mode 100644 .changeset/mighty-planets-dream.md create mode 100644 packages/theme-check-common/src/checks/json-missing-block/index.spec.ts create mode 100644 packages/theme-check-common/src/checks/json-missing-block/index.ts create mode 100644 packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts diff --git a/.changeset/mighty-planets-dream.md b/.changeset/mighty-planets-dream.md new file mode 100644 index 000000000..089d59632 --- /dev/null +++ b/.changeset/mighty-planets-dream.md @@ -0,0 +1,6 @@ +--- +'@shopify/theme-check-common': minor +'@shopify/theme-check-node': minor +--- + +Add `JsonMissingBlock` check diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index 6adc0e633..1fb8568f6 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -17,6 +17,7 @@ import { DeprecatedFilter } from './deprecated-filter'; import { DeprecatedTag } from './deprecated-tag'; import { EmptyBlockContent } from './empty-block-content'; import { ImgWidthAndHeight } from './img-width-and-height'; +import { JsonMissingBlock } from './json-missing-block'; import { JSONSyntaxError } from './json-syntax-error'; import { LiquidFreeSettings } from './liquid-free-settings'; import { LiquidHTMLSyntaxError } from './liquid-html-syntax-error'; @@ -62,6 +63,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ DeprecateLazysizes, EmptyBlockContent, ImgWidthAndHeight, + JsonMissingBlock, JSONSyntaxError, LiquidFreeSettings, LiquidHTMLSyntaxError, diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts new file mode 100644 index 000000000..ad8fd2790 --- /dev/null +++ b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts @@ -0,0 +1,378 @@ +import { expect, describe, it } from 'vitest'; +import { JsonMissingBlock } from './index'; +import { check, MockTheme } from '../../test'; + +describe('Module: JsonMissingBlock', () => { + describe('File existence validation', () => { + it('should report an offense when a block does not exist', async () => { + const theme: MockTheme = { + 'templates/failing.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "text_tqQTNE": { + "type": "missing_block" + } + }, + "block_order": ["text_tqQTNE"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "missing_block" + } + ] + } + {% endschema %} + `, + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "Theme block 'blocks/missing_block.liquid' does not exist.", + ); + + const content = theme['templates/failing.json']; + const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); + expect(erroredContent).to.equal('"missing_block"'); + }); + + it('should not report an offense when block exists', async () => { + const theme: MockTheme = { + 'templates/valid.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "text_block": { + "type": "text" + } + }, + "block_order": ["text_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': '', + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.be.empty; + }); + + it('should report an offense when a nested block does not exist', async () => { + const theme: MockTheme = { + 'templates/nested.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "parent_block": { + "type": "text", + "blocks": { + "child_block": { + "type": "missing_nested" + } + } + } + }, + "block_order": ["parent_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': ` + {% schema %} + { + "name": "Text Block", + "blocks": [ + { + "type": "missing_nested" + } + ] + } + {% endschema %} + `, + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "Theme block 'blocks/missing_nested.liquid' does not exist.", + ); + + const content = theme['templates/nested.json']; + const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); + expect(erroredContent).to.equal('"missing_nested"'); + }); + }); + + describe('Allowed block type validation', () => { + it('should report an offense when block exists but is not in the section liquid schema', async () => { + const theme: MockTheme = { + 'templates/valid.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "text_block": { + "type": "text" + } + }, + "block_order": ["text_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "image" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': '', + 'blocks/image.liquid': '', + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "Block type 'text' is not allowed in 'sections/custom-section.liquid'.", + ); + + const content = theme['templates/valid.json']; + const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); + expect(erroredContent).to.equal('"text"'); + }); + + it('should report an offense when a nested block exists but is not in the block liquid schema', async () => { + const theme: MockTheme = { + 'templates/nested.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "parent_block": { + "type": "text", + "blocks": { + "child_block": { + "type": "missing_nested" + } + } + } + }, + "block_order": ["parent_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': ` + {% schema %} + { + "name": "Text Block", + "blocks": [ + { + "type": "image" + } + ] + } + {% endschema %} + `, + 'blocks/image.liquid': '', + 'blocks/missing_nested.liquid': '', + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "Block type 'missing_nested' is not allowed in 'blocks/text.liquid'.", + ); + + const content = theme['templates/nested.json']; + const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); + expect(erroredContent).to.equal('"missing_nested"'); + }); + + it('should report an offense when a nested private block exists but is not in the block liquid schema', async () => { + const theme: MockTheme = { + 'templates/nested.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "parent_block": { + "type": "text", + "blocks": { + "child_block": { + "type": "_private_block" + } + } + } + }, + "block_order": ["parent_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': ` + {% schema %} + { + "name": "Text Block", + "blocks": [ + { + "type": "@theme" + } + ] + } + {% endschema %} + `, + 'blocks/image.liquid': '', + 'blocks/_private_block.liquid': '', + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.have.length(1); + expect(offenses[0].message).to.equal( + "Block type '_private_block' is not allowed in 'blocks/text.liquid'.", + ); + + const content = theme['templates/nested.json']; + const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); + expect(erroredContent).to.equal('"_private_block"'); + }); + }); + + describe('Edge case validation', () => { + it('should ignore non-template JSON files', async () => { + const theme: MockTheme = { + 'config/settings.json': `{ + "blocks": { + "text_block": { + "type": "nonexistent" + } + } + }`, + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.be.empty; + }); + + it('should ingore templates with local blocks', async () => { + const theme: MockTheme = { + 'templates/nested.json': `{ + "sections": { + "custom-section": { + "type": "custom-section", + "blocks": { + "parent_block": { + "type": "text", + "blocks": { + "child_block": { + "type": "_private_block" + } + } + } + }, + "block_order": ["parent_block"] + } + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text", + "name": "Text Block" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': ` + {% schema %} + { + "name": "Text Block", + "blocks": [ + { + "type": "image", + "name": "Image Block" + } + ] + } + {% endschema %} + `, + 'blocks/image.liquid': '', + 'blocks/_private_block.liquid': '', + }; + + const offenses = await check(theme, [JsonMissingBlock]); + expect(offenses).to.be.empty; + }); + }); +}); diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.ts b/packages/theme-check-common/src/checks/json-missing-block/index.ts new file mode 100644 index 000000000..e64ebd1cf --- /dev/null +++ b/packages/theme-check-common/src/checks/json-missing-block/index.ts @@ -0,0 +1,56 @@ +import { getSchemaFromJSON } from '../../to-schema'; +import { JSONCheckDefinition, Severity, SourceCodeType } from '../../types'; +import { getAllBlocks, isPropertyNode } from './missing-block-utils'; + +export const JsonMissingBlock: JSONCheckDefinition = { + meta: { + code: 'JsonMissingBlock', + name: 'Check for missing blocks types in JSON templates', + docs: { + description: 'This check ensures that JSON templates contain valid block types.', + recommended: true, + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/json-missing-block', + }, + type: SourceCodeType.JSON, + severity: Severity.ERROR, + schema: {}, + targets: [], + }, + + create(context) { + const relativePath = context.toRelativePath(context.file.uri); + if (!relativePath.startsWith('templates/')) return {}; + + return { + async onCodePathEnd() { + const schema = await getSchemaFromJSON(context); + const { ast, offset } = schema ?? {}; + if (!ast || ast instanceof Error) return; + if (!schema) return; + + const sections = schema.parsed.sections; + if (!sections) return; + + await Promise.all( + Object.entries(sections).map(async ([sectionKey, section]) => { + if ( + isPropertyNode(section) && + 'blocks' in section && + isPropertyNode(section.blocks) && + 'type' in section + ) { + await getAllBlocks( + ast, + offset, + section.type, + section.blocks, + ['sections', sectionKey, 'blocks'], + context, + ); + } + }), + ); + }, + }; + }, +}; diff --git a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts new file mode 100644 index 000000000..451835ee2 --- /dev/null +++ b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts @@ -0,0 +1,131 @@ +import { Context, SourceCodeType, JSONNode, Preset } from '../../types'; +import { LiteralNode, PropertyNode } from 'json-to-ast'; +import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; +import { doesFileExist } from '../../utils/file-utils'; + +export function isPropertyNode(node: unknown): node is PropertyNode { + return typeof node === 'object' && node !== null; +} + +function isNestedBlock(currentPath: string[]): boolean { + return currentPath.filter((segment) => segment === 'blocks').length > 1; +} + +function reportWarning( + message: string, + offset: number, + astNode: LiteralNode, + context: Context, +) { + context.report({ + message, + startIndex: offset + getLocStart(astNode), + endIndex: offset + getLocEnd(astNode), + }); +} + +async function validateBlockFileExistence( + blockType: string, + context: Context, +): Promise { + if (blockType === '@theme' || blockType === '@app') { + return true; + } + const blockPath = `blocks/${blockType}.liquid`; + return await doesFileExist(context, blockPath); +} + +async function getThemeBlocks( + sectionType: string, + currentPath: string[], + context: Context, +): Promise { + const themeBlocks: string[] = []; + if (!sectionType) return themeBlocks; + + const schema = isNestedBlock(currentPath) + ? await context.getBlockSchema?.(sectionType) + : await context.getSectionSchema?.(sectionType); + if (!schema || schema instanceof Error) return themeBlocks; + + const { validSchema } = schema; + if (!validSchema || validSchema instanceof Error) return themeBlocks; + + if (Array.isArray(validSchema.blocks)) { + validSchema.blocks.forEach((block) => { + if (!('name' in block) && block.type !== '@app') { + themeBlocks.push(block.type); + } + }); + } + return themeBlocks; +} + +async function validateBlock( + blockType: string, + blockPath: LiteralNode, + ancestorType: string, + currentPath: string[], + offset: number, + context: Context, +) { + const themeBlocks = await getThemeBlocks(ancestorType, currentPath, context); + if (themeBlocks.length === 0) return; + + const isPrivateBlock = blockType.startsWith('_'); + const isThemeInRootLevel = themeBlocks.includes('@theme'); + const isPresetInRootLevel = themeBlocks.includes(blockType); + + if (!isPrivateBlock ? isPresetInRootLevel || isThemeInRootLevel : isPresetInRootLevel) { + const exists = await validateBlockFileExistence(blockType, context); + if (!exists) { + reportWarning( + `Theme block 'blocks/${blockType}.liquid' does not exist.`, + offset, + blockPath, + context, + ); + } + } else { + const location = isNestedBlock(currentPath) ? 'blocks' : 'sections'; + reportWarning( + `Block type '${blockType}' is not allowed in '${location}/${ancestorType}.liquid'.`, + offset, + blockPath, + context, + ); + } +} + +export async function getAllBlocks( + ast: JSONNode, + offset: number, + ancestorType: string, + blocks: PropertyNode, + currentPath: string[], + context: Context, +): Promise { + await Promise.all( + Object.entries(blocks).map(async ([blockKey, block]) => { + if (block.type) { + const typePath = currentPath.concat(blockKey, 'type'); + const blockPath = nodeAtPath(ast, typePath)! as LiteralNode; + + if (blockPath) { + await validateBlock(block.type, blockPath, ancestorType, currentPath, offset, context); + } + } + + if ('blocks' in block) { + await getAllBlocks( + ast, + offset, + block.type, + block.blocks, + currentPath.concat(blockKey, 'blocks'), + context, + ); + } + }), + ); +} diff --git a/packages/theme-check-common/src/to-schema.ts b/packages/theme-check-common/src/to-schema.ts index 691752c4f..54a5b7308 100644 --- a/packages/theme-check-common/src/to-schema.ts +++ b/packages/theme-check-common/src/to-schema.ts @@ -153,6 +153,22 @@ export function getSchema(context: Context) { } } +export async function getSchemaFromJSON(context: Context) { + const originalSource = context.file.source; + const strippedSource = originalSource.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, '').trim(); + + const offset = originalSource.indexOf(strippedSource); + + const parsed = parseJSON(strippedSource); + const ast = toJSONAST(strippedSource); + + return { + parsed, + ast, + offset, + }; +} + function toParsed(schemaNode: LiquidRawTag | Error): any | Error { if (schemaNode instanceof Error) return schemaNode; return parseJSON(schemaNode.body.value); diff --git a/packages/theme-check-node/configs/all.yml b/packages/theme-check-node/configs/all.yml index 62b11565e..aa18271d2 100644 --- a/packages/theme-check-node/configs/all.yml +++ b/packages/theme-check-node/configs/all.yml @@ -58,6 +58,9 @@ ImgWidthAndHeight: JSONSyntaxError: enabled: true severity: 0 +JsonMissingBlock: + enabled: true + severity: 0 LiquidFreeSettings: enabled: true severity: 1 diff --git a/packages/theme-check-node/configs/recommended.yml b/packages/theme-check-node/configs/recommended.yml index e2de00b9c..f87433b37 100644 --- a/packages/theme-check-node/configs/recommended.yml +++ b/packages/theme-check-node/configs/recommended.yml @@ -39,6 +39,9 @@ ImgWidthAndHeight: JSONSyntaxError: enabled: true severity: 0 +JsonMissingBlock: + enabled: true + severity: 0 LiquidFreeSettings: enabled: true severity: 1 From 4831ddc5290505fde4648341605f91b1724b5e40 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Thu, 5 Dec 2024 15:43:04 -0500 Subject: [PATCH 2/5] Renaming to `JSONMissingBlock` --- .../theme-check-common/src/checks/index.ts | 4 ++-- .../checks/json-missing-block/index.spec.ts | 18 +++++++++--------- .../src/checks/json-missing-block/index.ts | 4 ++-- .../json-missing-block/missing-block-utils.ts | 2 +- packages/theme-check-node/configs/all.yml | 4 ++-- .../theme-check-node/configs/recommended.yml | 4 ++-- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index 1fb8568f6..fbb892b64 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -17,7 +17,7 @@ import { DeprecatedFilter } from './deprecated-filter'; import { DeprecatedTag } from './deprecated-tag'; import { EmptyBlockContent } from './empty-block-content'; import { ImgWidthAndHeight } from './img-width-and-height'; -import { JsonMissingBlock } from './json-missing-block'; +import { JSONMissingBlock } from './json-missing-block'; import { JSONSyntaxError } from './json-syntax-error'; import { LiquidFreeSettings } from './liquid-free-settings'; import { LiquidHTMLSyntaxError } from './liquid-html-syntax-error'; @@ -63,7 +63,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ DeprecateLazysizes, EmptyBlockContent, ImgWidthAndHeight, - JsonMissingBlock, + JSONMissingBlock, JSONSyntaxError, LiquidFreeSettings, LiquidHTMLSyntaxError, diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts index ad8fd2790..7f36fcf6a 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts @@ -1,5 +1,5 @@ import { expect, describe, it } from 'vitest'; -import { JsonMissingBlock } from './index'; +import { JSONMissingBlock } from './index'; import { check, MockTheme } from '../../test'; describe('Module: JsonMissingBlock', () => { @@ -34,7 +34,7 @@ describe('Module: JsonMissingBlock', () => { `, }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( "Theme block 'blocks/missing_block.liquid' does not exist.", @@ -76,7 +76,7 @@ describe('Module: JsonMissingBlock', () => { 'blocks/text.liquid': '', }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.be.empty; }); @@ -127,7 +127,7 @@ describe('Module: JsonMissingBlock', () => { `, }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( "Theme block 'blocks/missing_nested.liquid' does not exist.", @@ -172,7 +172,7 @@ describe('Module: JsonMissingBlock', () => { 'blocks/image.liquid': '', }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( "Block type 'text' is not allowed in 'sections/custom-section.liquid'.", @@ -232,7 +232,7 @@ describe('Module: JsonMissingBlock', () => { 'blocks/missing_nested.liquid': '', }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( "Block type 'missing_nested' is not allowed in 'blocks/text.liquid'.", @@ -292,7 +292,7 @@ describe('Module: JsonMissingBlock', () => { 'blocks/_private_block.liquid': '', }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.have.length(1); expect(offenses[0].message).to.equal( "Block type '_private_block' is not allowed in 'blocks/text.liquid'.", @@ -316,7 +316,7 @@ describe('Module: JsonMissingBlock', () => { }`, }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.be.empty; }); @@ -371,7 +371,7 @@ describe('Module: JsonMissingBlock', () => { 'blocks/_private_block.liquid': '', }; - const offenses = await check(theme, [JsonMissingBlock]); + const offenses = await check(theme, [JSONMissingBlock]); expect(offenses).to.be.empty; }); }); diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.ts b/packages/theme-check-common/src/checks/json-missing-block/index.ts index e64ebd1cf..7daa4fd0b 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/index.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/index.ts @@ -2,9 +2,9 @@ import { getSchemaFromJSON } from '../../to-schema'; import { JSONCheckDefinition, Severity, SourceCodeType } from '../../types'; import { getAllBlocks, isPropertyNode } from './missing-block-utils'; -export const JsonMissingBlock: JSONCheckDefinition = { +export const JSONMissingBlock: JSONCheckDefinition = { meta: { - code: 'JsonMissingBlock', + code: 'JSONMissingBlock', name: 'Check for missing blocks types in JSON templates', docs: { description: 'This check ensures that JSON templates contain valid block types.', diff --git a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts index 451835ee2..8d7c450ea 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts @@ -1,4 +1,4 @@ -import { Context, SourceCodeType, JSONNode, Preset } from '../../types'; +import { Context, SourceCodeType, JSONNode } from '../../types'; import { LiteralNode, PropertyNode } from 'json-to-ast'; import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; import { doesFileExist } from '../../utils/file-utils'; diff --git a/packages/theme-check-node/configs/all.yml b/packages/theme-check-node/configs/all.yml index aa18271d2..ab19f0993 100644 --- a/packages/theme-check-node/configs/all.yml +++ b/packages/theme-check-node/configs/all.yml @@ -55,10 +55,10 @@ EmptyBlockContent: ImgWidthAndHeight: enabled: true severity: 0 -JSONSyntaxError: +JSONMissingBlock: enabled: true severity: 0 -JsonMissingBlock: +JSONSyntaxError: enabled: true severity: 0 LiquidFreeSettings: diff --git a/packages/theme-check-node/configs/recommended.yml b/packages/theme-check-node/configs/recommended.yml index f87433b37..34b57a8a0 100644 --- a/packages/theme-check-node/configs/recommended.yml +++ b/packages/theme-check-node/configs/recommended.yml @@ -36,10 +36,10 @@ EmptyBlockContent: ImgWidthAndHeight: enabled: true severity: 0 -JSONSyntaxError: +JSONMissingBlock: enabled: true severity: 0 -JsonMissingBlock: +JSONSyntaxError: enabled: true severity: 0 LiquidFreeSettings: From 4182c1f8571c6ed49328c9f74083049893034801 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Fri, 6 Dec 2024 15:05:21 -0500 Subject: [PATCH 3/5] Renaming and type conversion --- .../checks/json-missing-block/index.spec.ts | 26 +++++++++---------- .../json-missing-block/missing-block-utils.ts | 8 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts index 7f36fcf6a..8046bbcea 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts @@ -6,7 +6,7 @@ describe('Module: JsonMissingBlock', () => { describe('File existence validation', () => { it('should report an offense when a block does not exist', async () => { const theme: MockTheme = { - 'templates/failing.json': `{ + 'templates/product.failing.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -40,14 +40,14 @@ describe('Module: JsonMissingBlock', () => { "Theme block 'blocks/missing_block.liquid' does not exist.", ); - const content = theme['templates/failing.json']; + const content = theme['templates/product.failing.json']; const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); expect(erroredContent).to.equal('"missing_block"'); }); it('should not report an offense when block exists', async () => { const theme: MockTheme = { - 'templates/valid.json': `{ + 'templates/product.valid.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -82,7 +82,7 @@ describe('Module: JsonMissingBlock', () => { it('should report an offense when a nested block does not exist', async () => { const theme: MockTheme = { - 'templates/nested.json': `{ + 'templates/product.nested.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -133,7 +133,7 @@ describe('Module: JsonMissingBlock', () => { "Theme block 'blocks/missing_nested.liquid' does not exist.", ); - const content = theme['templates/nested.json']; + const content = theme['templates/product.nested.json']; const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); expect(erroredContent).to.equal('"missing_nested"'); }); @@ -142,7 +142,7 @@ describe('Module: JsonMissingBlock', () => { describe('Allowed block type validation', () => { it('should report an offense when block exists but is not in the section liquid schema', async () => { const theme: MockTheme = { - 'templates/valid.json': `{ + 'templates/product.valid.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -178,14 +178,14 @@ describe('Module: JsonMissingBlock', () => { "Block type 'text' is not allowed in 'sections/custom-section.liquid'.", ); - const content = theme['templates/valid.json']; + const content = theme['templates/product.valid.json']; const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); expect(erroredContent).to.equal('"text"'); }); it('should report an offense when a nested block exists but is not in the block liquid schema', async () => { const theme: MockTheme = { - 'templates/nested.json': `{ + 'templates/product.nested.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -238,14 +238,14 @@ describe('Module: JsonMissingBlock', () => { "Block type 'missing_nested' is not allowed in 'blocks/text.liquid'.", ); - const content = theme['templates/nested.json']; + const content = theme['templates/product.nested.json']; const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); expect(erroredContent).to.equal('"missing_nested"'); }); it('should report an offense when a nested private block exists but is not in the block liquid schema', async () => { const theme: MockTheme = { - 'templates/nested.json': `{ + 'templates/product.nested.json': `{ "sections": { "custom-section": { "type": "custom-section", @@ -298,7 +298,7 @@ describe('Module: JsonMissingBlock', () => { "Block type '_private_block' is not allowed in 'blocks/text.liquid'.", ); - const content = theme['templates/nested.json']; + const content = theme['templates/product.nested.json']; const erroredContent = content.slice(offenses[0].start.index, offenses[0].end.index); expect(erroredContent).to.equal('"_private_block"'); }); @@ -307,7 +307,7 @@ describe('Module: JsonMissingBlock', () => { describe('Edge case validation', () => { it('should ignore non-template JSON files', async () => { const theme: MockTheme = { - 'config/settings.json': `{ + 'config/index.json': `{ "blocks": { "text_block": { "type": "nonexistent" @@ -322,7 +322,7 @@ describe('Module: JsonMissingBlock', () => { it('should ingore templates with local blocks', async () => { const theme: MockTheme = { - 'templates/nested.json': `{ + 'templates/product.nested.json': `{ "sections": { "custom-section": { "type": "custom-section", diff --git a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts index 8d7c450ea..6aa609025 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts @@ -1,5 +1,5 @@ import { Context, SourceCodeType, JSONNode } from '../../types'; -import { LiteralNode, PropertyNode } from 'json-to-ast'; +import { PropertyNode } from 'json-to-ast'; import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; import { doesFileExist } from '../../utils/file-utils'; @@ -14,7 +14,7 @@ function isNestedBlock(currentPath: string[]): boolean { function reportWarning( message: string, offset: number, - astNode: LiteralNode, + astNode: JSONNode, context: Context, ) { context.report({ @@ -63,7 +63,7 @@ async function getThemeBlocks( async function validateBlock( blockType: string, - blockPath: LiteralNode, + blockPath: JSONNode, ancestorType: string, currentPath: string[], offset: number, @@ -109,7 +109,7 @@ export async function getAllBlocks( Object.entries(blocks).map(async ([blockKey, block]) => { if (block.type) { const typePath = currentPath.concat(blockKey, 'type'); - const blockPath = nodeAtPath(ast, typePath)! as LiteralNode; + const blockPath = nodeAtPath(ast, typePath)! as JSONNode; if (blockPath) { await validateBlock(block.type, blockPath, ancestorType, currentPath, offset, context); From 49e0db8d157ffa877b47572d7b38be220ad49c46 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Fri, 6 Dec 2024 16:36:38 -0500 Subject: [PATCH 4/5] Design update --- .../json-missing-block/missing-block-utils.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts index 6aa609025..119dd9738 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts @@ -72,28 +72,30 @@ async function validateBlock( const themeBlocks = await getThemeBlocks(ancestorType, currentPath, context); if (themeBlocks.length === 0) return; - const isPrivateBlock = blockType.startsWith('_'); - const isThemeInRootLevel = themeBlocks.includes('@theme'); - const isPresetInRootLevel = themeBlocks.includes(blockType); + const exists = await validateBlockFileExistence(blockType, context); + if (!exists) { + reportWarning( + `Theme block 'blocks/${blockType}.liquid' does not exist.`, + offset, + blockPath, + context, + ); + } else { + const isPrivateBlock = blockType.startsWith('_'); + const isThemeInRootLevel = themeBlocks.includes('@theme'); + const isPresetInRootLevel = themeBlocks.includes(blockType); - if (!isPrivateBlock ? isPresetInRootLevel || isThemeInRootLevel : isPresetInRootLevel) { - const exists = await validateBlockFileExistence(blockType, context); - if (!exists) { + if (!isPrivateBlock ? isPresetInRootLevel || isThemeInRootLevel : isPresetInRootLevel) { + return; + } else { + const location = isNestedBlock(currentPath) ? 'blocks' : 'sections'; reportWarning( - `Theme block 'blocks/${blockType}.liquid' does not exist.`, + `Block type '${blockType}' is not allowed in '${location}/${ancestorType}.liquid'.`, offset, blockPath, context, ); } - } else { - const location = isNestedBlock(currentPath) ? 'blocks' : 'sections'; - reportWarning( - `Block type '${blockType}' is not allowed in '${location}/${ancestorType}.liquid'.`, - offset, - blockPath, - context, - ); } } From 06c48d89b663e0a8619db109736fcbdef1dcfd4e Mon Sep 17 00:00:00 2001 From: Navdeep Date: Mon, 9 Dec 2024 16:31:06 -0500 Subject: [PATCH 5/5] Nit fix --- .../src/checks/json-missing-block/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts index 8046bbcea..b0c62b0e0 100644 --- a/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts +++ b/packages/theme-check-common/src/checks/json-missing-block/index.spec.ts @@ -320,7 +320,7 @@ describe('Module: JsonMissingBlock', () => { expect(offenses).to.be.empty; }); - it('should ingore templates with local blocks', async () => { + it('should ignore templates with local blocks', async () => { const theme: MockTheme = { 'templates/product.nested.json': `{ "sections": {