From 27bcaf99e1e9122146cfaa96b45ce06df17f5693 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Thu, 5 Dec 2024 11:51:15 -0500 Subject: [PATCH] Improved --- .../checks/json-missing-block/index.spec.ts | 494 ++++++++++++------ .../src/checks/json-missing-block/index.ts | 127 +---- .../json-missing-block/missing-block-utils.ts | 131 +++++ 3 files changed, 487 insertions(+), 265 deletions(-) create mode 100644 packages/theme-check-common/src/checks/json-missing-block/missing-block-utils.ts 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 709e55a47..ad8fd2790 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,178 +1,378 @@ import { expect, describe, it } from 'vitest'; import { JsonMissingBlock } from './index'; -import { check, highlightedOffenses, MockTheme } from '../../test'; +import { check, MockTheme } from '../../test'; describe('Module: JsonMissingBlock', () => { - 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": { + 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" } - }, - "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"] } - ] - } - {% 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 highlights = highlightedOffenses(theme, offenses); - expect(highlights).to.have.length(1); - expect(highlights[0]).to.equal('"missing_block"'); - }); + }, + "order": ["custom-section"] + }`, + 'sections/custom-section.liquid': ` + {% schema %} + { + "name": "Custom Section", + "blocks": [ + { + "type": "text" + } + ] + } + {% endschema %} + `, + 'blocks/text.liquid': '', + }; - 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": { + 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" } - }, - "block_order": ["text_block"] + ] } - }, - "order": ["custom-section"] - }`, - 'sections/custom-section.liquid': ` + {% endschema %} + `, + 'blocks/text.liquid': ` {% schema %} - { - "name": "Custom Section", - "blocks": [ - { - "type": "text" - } - ] - } - {% endschema %} - `, - 'blocks/text.liquid': '', - }; - - const offenses = await check(theme, [JsonMissingBlock]); - expect(offenses).to.be.empty; + { + "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"'); + }); }); - 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": { + 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" } - }, - "block_order": ["text_block"] + ] } - }, - "order": ["custom-section"] - }`, - 'sections/custom-section.liquid': ` + {% endschema %} + `, + 'blocks/text.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'.", - ); - }); + { + "name": "Text Block", + "blocks": [ + { + "type": "image" + } + ] + } + {% endschema %} + `, + 'blocks/image.liquid': '', + 'blocks/missing_nested.liquid': '', + }; - it('should report an offense when a block is not in root level', async () => { - const theme: MockTheme = { - 'templates/nested.json': `{ - "sections": { - "custom-section": { - "type": "custom-section", - "blocks": { - "parent_block": { - "type": "text", - "blocks": { - "child_block": { - "type": "missing_nested" + 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" } - }, - "block_order": ["parent_block"] + ] } - }, - "order": ["custom-section"] - }`, - 'sections/custom-section.liquid': ` + {% endschema %} + `, + 'blocks/text.liquid': ` {% schema %} - { - "name": "Custom Section", - "blocks": [ - { - "type": "text" - } - ] - } - {% endschema %} - `, - 'blocks/text.liquid': '', - }; - - 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.", - ); + { + "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"'); + }); }); - it('should ignore non-template JSON files', async () => { - const theme: MockTheme = { - 'config/settings.json': `{ - "blocks": { - "text_block": { - "type": "nonexistent" + 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; + 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 08c40d2e8..e64ebd1cf 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 @@ -1,15 +1,6 @@ import { getSchemaFromJSON } from '../../to-schema'; -import { - JSONCheckDefinition, - JSONNode, - Preset, - Section, - Severity, - SourceCodeType, -} from '../../types'; -import { doesFileExist } from '../../utils/file-utils'; -import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; -import { LiteralNode } from 'json-to-ast'; +import { JSONCheckDefinition, Severity, SourceCodeType } from '../../types'; +import { getAllBlocks, isPropertyNode } from './missing-block-utils'; export const JsonMissingBlock: JSONCheckDefinition = { meta: { @@ -30,107 +21,6 @@ export const JsonMissingBlock: JSONCheckDefinition = { const relativePath = context.toRelativePath(context.file.uri); if (!relativePath.startsWith('templates/')) return {}; - async function validateBlockFileExistence(blockType: string): Promise { - if (blockType === '@theme' || blockType === '@app') { - return true; - } - - const blockPath = `blocks/${blockType}.liquid`; - return await doesFileExist(context, blockPath); - } - - async function isThemeBlock(sectionType: string): Promise { - let themeBlocks: string[] = []; - if (!sectionType) return themeBlocks; - - const parentSchema = await context.getSectionSchema?.(sectionType); - if (!parentSchema || parentSchema instanceof Error) return themeBlocks; - - const { validSchema } = parentSchema; - if (!validSchema || validSchema instanceof Error) return themeBlocks; - - if (Array.isArray(validSchema.blocks)) { - validSchema.blocks.forEach((block) => { - const hasName = 'name' in block; - if (!hasName) { - themeBlocks.push(block.type); - } - }); - } - - return themeBlocks; - } - - async function getAllBlocks( - ast: JSONNode, - offset: number, - sectionType: string, - blocks: Preset.PresetBlockHash | Preset.PresetBlockForArray[], - currentPath: string[], - nested: boolean = false, - ): 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) { - if (nested) { - console.log('check nested', block.type); - const exists = await validateBlockFileExistence(block.type); - if (!exists) { - context.report({ - message: `Theme block 'blocks/${block.type}.liquid' does not exist.`, - startIndex: offset + getLocStart(blockPath), - endIndex: offset + getLocEnd(blockPath), - }); - } - } else { - const themeBlocks = await isThemeBlock(sectionType); - const isPrivateBlock = block.type.startsWith('_'); - const isThemeInRootLevel = themeBlocks.some((block) => block === '@theme'); - const isPresetInRootLevel = themeBlocks.some( - (blockType) => blockType === block.type, - ); - - if ( - !isPrivateBlock ? isPresetInRootLevel || isThemeInRootLevel : isPresetInRootLevel - ) { - const exists = await validateBlockFileExistence(block.type); - if (!exists) { - context.report({ - message: `Theme block 'blocks/${block.type}.liquid' does not exist.`, - startIndex: offset + getLocStart(blockPath), - endIndex: offset + getLocEnd(blockPath), - }); - } - } else { - context.report({ - message: `Block type '${block.type}' is not allowed in 'sections/${sectionType}.liquid'.`, - startIndex: offset + getLocStart(blockPath), - endIndex: offset + getLocEnd(blockPath), - }); - } - } - } - } - - // Recursively get nested blocks - if ('blocks' in block) { - await getAllBlocks( - ast, - offset, - block.type, - block.blocks, - currentPath.concat(blockKey, 'blocks'), - true, - ); - } - }), - ); - } - return { async onCodePathEnd() { const schema = await getSchemaFromJSON(context); @@ -144,17 +34,18 @@ export const JsonMissingBlock: JSONCheckDefinition = { await Promise.all( Object.entries(sections).map(async ([sectionKey, section]) => { if ( - section && - typeof section === 'object' && - 'type' in section && - 'blocks' in section + isPropertyNode(section) && + 'blocks' in section && + isPropertyNode(section.blocks) && + 'type' in section ) { await getAllBlocks( ast, offset, - section.type as string, - section.blocks as Preset.PresetBlockHash, + 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, + ); + } + }), + ); +}