From eda83ff091ea181092c3683f89ec69c5f354ccd1 Mon Sep 17 00:00:00 2001 From: Wojciech Grzeszczak Date: Thu, 26 Mar 2026 12:13:25 +0000 Subject: [PATCH 1/5] Modified checks --- .../extract-undefined-variables.spec.ts | 96 ++++++ .../extract-undefined-variables.ts | 286 ++++++++++++++++++ .../src/checks/metadata-params/index.spec.ts | 217 +++++++++++-- .../src/checks/metadata-params/index.ts | 76 ++--- .../src/checks/undefined-object/index.spec.ts | 120 +++++--- .../src/checks/undefined-object/index.ts | 10 + .../src/disabled-checks/index.spec.ts | 8 +- 7 files changed, 711 insertions(+), 102 deletions(-) create mode 100644 packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts create mode 100644 packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.ts diff --git a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts new file mode 100644 index 0000000..528e4be --- /dev/null +++ b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts @@ -0,0 +1,96 @@ +import { describe, it, expect } from 'vitest'; +import { extractUndefinedVariables } from './extract-undefined-variables'; + +describe('extractUndefinedVariables', () => { + it('should return variables used but not defined', () => { + const source = `{% liquid + assign b = a + %} + {{ b }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['a']); + }); + + it('should not include assigned variables', () => { + const source = `{% assign x = 1 %}{{ x }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should not include captured variables', () => { + const source = `{% capture x %}hello{% endcapture %}{{ x }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should not include for loop variables', () => { + const source = `{% for item in items %}{{ item }}{% endfor %}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['items']); + }); + + it('should not include forloop variable', () => { + const source = `{% for item in items %}{{ forloop.index }}{% endfor %}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['items']); + }); + + it('should handle function result variables', () => { + const source = `{% function res = 'my_partial' %}{{ res }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should handle graphql result variables', () => { + const source = `{% graphql res = 'my_query' %}{{ res }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should handle parse_json result variables', () => { + const source = `{% parse_json data %}{"a":1}{% endparse_json %}{{ data }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should not include global objects', () => { + const source = `{{ context.session }}`; + const result = extractUndefinedVariables(source, ['context', 'null', 'true', 'false', 'blank', 'empty']); + expect(result).to.deep.equal([]); + }); + + it('should deduplicate results', () => { + const source = `{{ a }}{{ a }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['a']); + }); + + it('should return empty array if source fails to parse', () => { + const source = `{% invalid unclosed`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should handle increment/decrement as definitions', () => { + const source = `{% increment counter %}{{ counter }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should handle hash_assign as definition', () => { + const source = `{% hash_assign h['key'] = 'val' %}{{ h }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + + it('should not include doc param names', () => { + const source = ` + {% doc %} + @param {String} name - a name + {% enddoc %} + {{ name }} + `; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['name']); + }); +}); diff --git a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.ts b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.ts new file mode 100644 index 0000000..c870512 --- /dev/null +++ b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.ts @@ -0,0 +1,286 @@ +import { + LiquidHtmlNode, + LiquidTag, + LiquidTagAssign, + LiquidTagCapture, + LiquidTagDecrement, + LiquidTagFor, + LiquidTagIncrement, + LiquidTagTablerow, + LiquidVariableLookup, + NamedTags, + NodeTypes, + Position, + FunctionMarkup, + LiquidTagHashAssign, + HashAssignMarkup, + LiquidTagGraphQL, + LiquidTagParseJson, + LiquidTagBackground, + BackgroundMarkup, + toLiquidHtmlAST, +} from '@platformos/liquid-html-parser'; + +type Scope = { start?: number; end?: number }; + +/** + * Parses a Liquid source string and returns a deduplicated list of variable names + * that are used but never defined. Returns [] on parse errors. + * + * This mirrors the variable tracking logic from the UndefinedObject check but + * packaged as a standalone synchronous function. + */ +export function extractUndefinedVariables( + source: string, + globalObjectNames: string[] = [], +): string[] { + let ast; + try { + ast = toLiquidHtmlAST(source); + } catch { + return []; + } + + const scopedVariables: Map = new Map(); + const fileScopedVariables: Set = new Set(globalObjectNames); + const variables: LiquidVariableLookup[] = []; + + function indexVariableScope(variableName: string | null, scope: Scope) { + if (!variableName) return; + const indexedScope = scopedVariables.get(variableName) ?? []; + scopedVariables.set(variableName, indexedScope.concat(scope)); + } + + function walk(node: LiquidHtmlNode, ancestors: LiquidHtmlNode[]) { + // Process definitions from LiquidTag nodes + if (node.type === NodeTypes.LiquidTag) { + handleLiquidTag(node, ancestors); + } + + // Process definitions from LiquidBranch nodes (catch) + if (node.type === NodeTypes.LiquidBranch) { + handleLiquidBranch(node); + } + + // Process variable usages + if (node.type === NodeTypes.VariableLookup) { + handleVariableLookup(node, ancestors); + } + + // Recurse into children + const newAncestors = ancestors.concat(node); + for (const value of Object.values(node)) { + if (Array.isArray(value)) { + for (const item of value) { + if (isNode(item)) { + walk(item, newAncestors); + } + } + } else if (isNode(value)) { + walk(value, newAncestors); + } + } + } + + function handleLiquidTag(node: LiquidTag, _ancestors: LiquidHtmlNode[]) { + if (isLiquidTagAssign(node) || isLiquidTagGraphQL(node) || isLiquidTagParseJson(node)) { + indexVariableScope(node.markup.name, { + start: node.blockStartPosition.end, + }); + } + + if (isLiquidTagHashAssign(node) && node.markup.target.name) { + indexVariableScope(node.markup.target.name, { + start: node.blockStartPosition.end, + }); + } + + if (isLiquidTagCapture(node)) { + indexVariableScope(node.markup.name, { + start: node.blockEndPosition?.end, + }); + } + + if (node.name === 'form') { + indexVariableScope(node.name, { + start: node.blockStartPosition.end, + end: node.blockEndPosition?.start, + }); + } + + if (node.name === 'function') { + const fnName = (node.markup as FunctionMarkup).name; + if (fnName.lookups.length === 0 && fnName.name !== null) { + indexVariableScope(fnName.name, { + start: node.position.end, + }); + } + } + + if ((isLiquidTagIncrement(node) || isLiquidTagDecrement(node)) && node.markup.name !== null) { + indexVariableScope(node.markup.name, { + start: node.position.start, + }); + } + + if (isLiquidForTag(node) || isLiquidTableRowTag(node)) { + indexVariableScope(node.markup.variableName, { + start: node.blockStartPosition.end, + end: node.blockEndPosition?.start, + }); + indexVariableScope(node.name === 'for' ? 'forloop' : 'tablerowloop', { + start: node.blockStartPosition.end, + end: node.blockEndPosition?.start, + }); + } + + if (isLiquidTagBackground(node)) { + indexVariableScope(node.markup.jobId, { + start: node.position.end, + }); + } + } + + function handleLiquidBranch(node: LiquidHtmlNode & { type: typeof NodeTypes.LiquidBranch }) { + if ( + node.name === NamedTags.catch && + node.markup && + typeof node.markup !== 'string' && + 'name' in node.markup && + (node.markup as any).name + ) { + indexVariableScope((node.markup as any).name, { + start: (node as any).blockStartPosition.end, + end: (node as any).blockEndPosition?.start, + }); + } + } + + function handleVariableLookup(node: LiquidVariableLookup, ancestors: LiquidHtmlNode[]) { + const parent = ancestors[ancestors.length - 1]; + + if (isLiquidTag(parent) && isLiquidTagCapture(parent)) return; + if (isLiquidTag(parent) && isLiquidTagParseJson(parent)) return; + if (isFunctionMarkup(parent) && parent.name === node) return; + if (isLiquidBranchCatch(parent) && parent.markup === node) return; + if (isHashAssignMarkup(parent) && parent.target === node) return; + + variables.push(node); + } + + walk(ast, []); + + // Determine undefined variables + const seen = new Set(); + const result: string[] = []; + + for (const variable of variables) { + if (!variable.name) continue; + if (seen.has(variable.name)) continue; + + const isVariableDefined = isDefined( + variable.name, + variable.position, + fileScopedVariables, + scopedVariables, + ); + + if (!isVariableDefined) { + seen.add(variable.name); + result.push(variable.name); + } + } + + return result; +} + +function isNode(x: any): x is LiquidHtmlNode { + return x !== null && typeof x === 'object' && typeof x.type === 'string'; +} + +function isDefined( + variableName: string, + variablePosition: Position, + fileScopedVariables: Set, + scopedVariables: Map, +): boolean { + if (fileScopedVariables.has(variableName)) { + return true; + } + + const scopes = scopedVariables.get(variableName); + if (!scopes) { + return false; + } + + return scopes.some((scope) => { + const start = variablePosition.start; + const isVariableAfterScopeStart = !scope.start || start > scope.start; + const isVariableBeforeScopeEnd = !scope.end || start < scope.end; + return isVariableAfterScopeStart && isVariableBeforeScopeEnd; + }); +} + +function isLiquidTag(node?: LiquidHtmlNode): node is LiquidTag { + return node?.type === NodeTypes.LiquidTag; +} + +function isLiquidTagCapture(node: LiquidTag): node is LiquidTagCapture { + return node.name === NamedTags.capture; +} + +function isLiquidTagAssign(node: LiquidTag): node is LiquidTagAssign { + return node.name === NamedTags.assign && typeof node.markup !== 'string'; +} + +function isLiquidTagHashAssign(node: LiquidTag): node is LiquidTagHashAssign { + return node.name === NamedTags.hash_assign && typeof node.markup !== 'string'; +} + +function isLiquidTagGraphQL(node: LiquidTag): node is LiquidTagGraphQL { + return node.name === NamedTags.graphql && typeof node.markup !== 'string'; +} + +function isLiquidTagParseJson(node: LiquidTag): node is LiquidTagParseJson { + return node.name === NamedTags.parse_json && typeof node.markup !== 'string'; +} + +function isLiquidForTag(node: LiquidTag): node is LiquidTagFor { + return node.name === NamedTags.for && typeof node.markup !== 'string'; +} + +function isLiquidTableRowTag(node: LiquidTag): node is LiquidTagTablerow { + return node.name === NamedTags.tablerow && typeof node.markup !== 'string'; +} + +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'; +} + +function isLiquidTagBackground( + node: LiquidTag, +): node is LiquidTagBackground & { markup: BackgroundMarkup } { + return ( + node.name === NamedTags.background && + typeof node.markup !== 'string' && + node.markup.type === NodeTypes.BackgroundMarkup + ); +} + +function isHashAssignMarkup(node?: LiquidHtmlNode): node is HashAssignMarkup { + return node?.type === NodeTypes.HashAssignMarkup; +} + +function isFunctionMarkup(node?: LiquidHtmlNode): node is FunctionMarkup { + return node?.type === NodeTypes.FunctionMarkup; +} + +function isLiquidBranchCatch( + node?: LiquidHtmlNode, +): node is LiquidHtmlNode & { type: typeof NodeTypes.LiquidBranch; name: 'catch'; markup: any } { + return node?.type === NodeTypes.LiquidBranch && (node as any).name === NamedTags.catch; +} diff --git a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts index 34e9d06..4eb47bf 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts @@ -3,14 +3,13 @@ import { MetadataParamsCheck } from '.'; import { check } from '../../test'; describe('Module: MetadataParamsCheck', () => { - it('should report the missing variable when not defined but passed', async () => { + it('should use doc tag as complete param list when present', async () => { const file = ` - --- - metadata: - params: - variable: {} - variable3: {} - --- + {% doc %} + @param {Number} variable - param with description + @param {Number} variable2 - param with description + {% enddoc %} + {% assign a = 5 | plus: variable | plus: variable2 %} {{ a }} `; @@ -25,23 +24,21 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); - expect(offenses).to.have.length(2); - expect(offenses).to.containOffense('Unknown parameter variable2 passed to function call'); - expect(offenses).to.containOffense( - 'Required parameter variable3 must be passed to function call', - ); + expect(offenses).to.have.length(0); }); - it('should ignore if metadata not defined', async () => { + it('should report missing required doc params', async () => { const file = ` - --- - metadata: - --- + {% doc %} + @param {Number} variable - param with description + @param {Number} variable2 - param with description + {% enddoc %} + {% assign a = 5 | plus: variable | plus: variable2 %} {{ a }} `; const file2 = ` - {% function a = 'commands/call/fileToCall', variable: 2, variable2: 12 %} + {% function a = 'commands/call/fileToCall', variable: 2 %} {{ a }} `; const files = { @@ -51,21 +48,23 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); - expect(offenses).to.have.length(0); + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Required parameter variable2 must be passed to function call', + ); }); - it('should accept doc tag if metadata not defined', async () => { + it('should report unknown params not in doc', async () => { const file = ` {% doc %} @param {Number} variable - param with description - @param {Number} variable2 - param with description {% enddoc %} - {% assign a = 5 | plus: variable | plus: variable2 %} + {% assign a = 5 | plus: variable %} {{ a }} `; const file2 = ` - {% function a = 'commands/call/fileToCall', variable: 2, variable2: 12 %} + {% function a = 'commands/call/fileToCall', variable: 2, extra: 12 %} {{ a }} `; const files = { @@ -75,22 +74,153 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Unknown parameter extra passed to function call', + ); + }); + + it('should allow doc-optional params without requiring them', async () => { + const file = ` + {% doc %} + @param {String} a - required + @param {String} [b] - optional + {% enddoc %} + {{ a }}{{ b }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall', a: 'hello' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + expect(offenses).to.have.length(0); }); - it('should reject if doc tag is missing params', async () => { + it('should allow passing doc-optional params without reporting unknown', async () => { const file = ` {% doc %} - @param {Number} variable - param with description + @param {String} a - required + @param {String} [b] - optional {% enddoc %} + {{ a }}{{ b }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall', a: 'hello', b: 'world' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; - {% assign a = 5 | plus: variable | plus: variable2 %} + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(0); + }); + + it('should require doc params even if not used in source', async () => { + const file = ` + {% doc %} + @param {String} a - required param + @param {String} unused - required but not used in source + {% enddoc %} {{ a }} `; const file2 = ` - {% function a = 'commands/call/fileToCall', variable: 2, variable2: 12 %} + {% function res = 'commands/call/fileToCall', a: 'hello' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Required parameter unused must be passed to function call', + ); + }); + + it('should infer required params from undefined variables when no doc', async () => { + const file = ` + {% assign b = a %} + {{ b }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall', a: 'hello' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(0); + }); + + it('should report missing inferred params when no doc', async () => { + const file = ` + {% assign b = a %} + {{ b }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Required parameter a must be passed to function call', + ); + }); + + it('should report unknown params when passing args not in inferred set', async () => { + const file = ` + {% assign b = a %} + {{ b }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall', a: 'hello', extra: 'world' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Unknown parameter extra passed to function call', + ); + }); + + it('should not include global objects like context in inferred params', async () => { + const file = ` + {{ context.session }} {{ a }} `; + const file2 = ` + {% function res = 'commands/call/fileToCall', a: 'hello' %} + {{ res }} + `; const files = { 'app/lib/commands/call/fileToCall.liquid': file, 'app/lib/caller.liquid': file2, @@ -98,6 +228,41 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); + expect(offenses).to.have.length(0); + }); + + it('should work with render tags too', async () => { + const file = `{{ a }}`; + const file2 = `{% render 'fileToRender' %}`; + const files = { + 'app/views/partials/fileToRender.liquid': file, + 'app/views/pages/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense( + 'Required parameter a must be passed to render call', + ); + }); + + it('should skip validation when no doc and no undefined vars', async () => { + const file = ` + {% assign a = 5 %} + {{ a }} + `; + const file2 = ` + {% function res = 'commands/call/fileToCall', extra: 'hello' %} + {{ res }} + `; + const files = { + 'app/lib/commands/call/fileToCall.liquid': file, + 'app/lib/caller.liquid': file2, + }; + + const offenses = await check(files, [MetadataParamsCheck]); + + expect(offenses).to.have.length(0); }); }); diff --git a/packages/platformos-check-common/src/checks/metadata-params/index.ts b/packages/platformos-check-common/src/checks/metadata-params/index.ts index 6a33eda..78c014a 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/index.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/index.ts @@ -1,31 +1,9 @@ import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; -import yaml from 'js-yaml'; import { DocumentsLocator, DocumentType } from '@platformos/platformos-common'; import { URI } from 'vscode-uri'; import { LiquidNamedArgument, Position } from '@platformos/liquid-html-parser'; import { relative } from '../../path'; - -type Metadata = { - metadata: { - params: Record; - }; -}; - -function extractMetadataParams(source: string): string[] | null { - source = source.trim(); - if (!source.startsWith('---')) return null; - - const end = source.indexOf('---', 3); - if (end === -1) return null; - - const yamlBlock = source.slice(3, end).trim(); - try { - const result = yaml.load(yamlBlock) as Metadata; - return Object.keys(result.metadata.params); - } catch (e) { - return null; - } -} +import { extractUndefinedVariables } from './extract-undefined-variables'; export const MetadataParamsCheck: LiquidCheckDefinition = { meta: { @@ -33,7 +11,7 @@ export const MetadataParamsCheck: LiquidCheckDefinition = { name: 'Metadata Params Check', docs: { description: - 'Ensures that parameters referenced in the document exist in metadata.params or in the doc tag.', + 'Ensures that parameters referenced in the document exist in the doc tag or are inferred from undefined variables.', recommended: true, url: undefined, }, @@ -61,21 +39,47 @@ export const MetadataParamsCheck: LiquidCheckDefinition = { if (!locatedFile) { return; } - let params = extractMetadataParams(await context.fs.readFile(locatedFile)); - if (!params) { - if (!context.getDocDefinition) return; - const relativePath = relative(locatedFile, context.config.rootUri); - const docDef = await context.getDocDefinition(relativePath); - if (!docDef?.liquidDoc?.parameters) return; - const liquidDocParameters = new Map(docDef.liquidDoc.parameters.map((p) => [p.name, p])); - params = Array.from(liquidDocParameters.values()) - .filter((p) => p.required) - .map((p) => p.name); + const source = await context.fs.readFile(locatedFile); + const relativePath = relative(locatedFile, context.config.rootUri); + + let requiredParams: string[]; + let allowedParams: string[]; + + // Check for @doc tag first — if present, it's the complete param list + const docDef = context.getDocDefinition + ? await context.getDocDefinition(relativePath) + : undefined; + + if (docDef?.liquidDoc?.parameters) { + requiredParams = docDef.liquidDoc.parameters.filter((p) => p.required).map((p) => p.name); + allowedParams = docDef.liquidDoc.parameters.map((p) => p.name); + } else { + // No @doc — scan for undefined variables, treat all as required + const globalObjectNames: string[] = []; + if (context.platformosDocset) { + const objects = await context.platformosDocset.objects(); + for (const obj of objects) { + if (!obj.access || obj.access.global === true || obj.access.template.length > 0) { + globalObjectNames.push(obj.name); + } + } + } + if (relativePath.includes('views/partials/') || relativePath.includes('/lib/')) { + if (!globalObjectNames.includes('app')) { + globalObjectNames.push('app'); + } + } + + const undefinedVars = extractUndefinedVariables(source, globalObjectNames); + if (undefinedVars.length === 0) return; + + requiredParams = undefinedVars; + allowedParams = undefinedVars; } args - .filter((arg) => !params.includes(arg.name)) + .filter((arg) => !allowedParams.includes(arg.name)) .forEach((arg) => { context.report({ message: `Unknown parameter ${arg.name} passed to ${nodeType} call`, @@ -84,7 +88,7 @@ export const MetadataParamsCheck: LiquidCheckDefinition = { }); }); - params + requiredParams .filter((param) => !args.find((arg) => arg.name === param)) .forEach((param) => { context.report({ diff --git a/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts b/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts index ebd4e8f..2c7a019 100644 --- a/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts +++ b/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts @@ -4,13 +4,25 @@ import { runLiquidCheck, highlightedOffenses } from '../../test'; import { Offense } from '../../types'; describe('Module: UndefinedObject', () => { - it('should report an offense when object is undefined', async () => { + it('should not report offenses when no doc tag is present', async () => { const sourceCode = ` {{ my_var }} `; const offenses = await runLiquidCheck(UndefinedObject, sourceCode); + expect(offenses).toHaveLength(0); + }); + + it('should report an offense when object is undefined and doc tag is present', async () => { + const sourceCode = ` + {% doc %} + {% enddoc %} + {{ my_var }} + `; + + const offenses = await runLiquidCheck(UndefinedObject, sourceCode); + expect(offenses).toHaveLength(1); expect(offenses.map((e) => e.message)).toEqual(["Unknown object 'my_var' used."]); @@ -18,8 +30,10 @@ describe('Module: UndefinedObject', () => { expect(highlights).toEqual(['my_var']); }); - it('should report an offense when object with an attribute is undefined', async () => { + it('should report an offense when object with an attribute is undefined and doc tag is present', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ my_var.my_attr }} `; @@ -29,8 +43,10 @@ describe('Module: UndefinedObject', () => { expect(offenses.map((e) => e.message)).toEqual(["Unknown object 'my_var' used."]); }); - it('should report an offense when undefined object is used as an argument', async () => { + it('should report an offense when undefined object is used as an argument with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ product[my_object] }} {{ product[my_object] }} @@ -47,12 +63,14 @@ describe('Module: UndefinedObject', () => { ]); }); - it('should report an offense when object is undefined in a Liquid tag', async () => { + it('should report an offense when object is undefined in a Liquid tag with doc tag', async () => { const sourceCode = ` - {% liquid - echo my_var - %} - `; + {% doc %} + {% enddoc %} + {% liquid + echo my_var + %} + `; const offenses = await runLiquidCheck(UndefinedObject, sourceCode); @@ -121,8 +139,11 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when object is defined in a for loop but used outside of the scope', async () => { + it('should report an offense when object is defined in a for loop but used outside of the scope with doc tag', async () => { const sourceCode = ` + {% doc %} + @param {Array} collections + {% enddoc %} {% for c in collections %} {{ c }} {% endfor %}{{ c }} @@ -145,8 +166,10 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when function result variable is used before its definition', async () => { + it('should report an offense when function result variable is used before its definition with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ a }} {% function a = 'test' %} `; @@ -183,15 +206,14 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when a variable partial in include is undefined', async () => { + it('should not report offenses for undefined partials without doc tag', async () => { const sourceCode = ` {% include undefined_partial %} `; const offenses = await runLiquidCheck(UndefinedObject, sourceCode); - expect(offenses).toHaveLength(1); - expect(offenses[0].message).toBe("Unknown object 'undefined_partial' used."); + expect(offenses).toHaveLength(0); }); it('should not report an offense when a variable partial in include is defined', async () => { @@ -205,19 +227,20 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when a variable partial in function is undefined', async () => { + it('should not report offenses for undefined function partials without doc tag', async () => { const sourceCode = ` {% function result = undefined_partial %} `; const offenses = await runLiquidCheck(UndefinedObject, sourceCode); - expect(offenses).toHaveLength(1); - expect(offenses[0].message).toBe("Unknown object 'undefined_partial' used."); + expect(offenses).toHaveLength(0); }); it('should not report an offense for the result variable itself in function tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% function result = undefined_partial %} `; @@ -227,8 +250,10 @@ describe('Module: UndefinedObject', () => { expect(offenses.every((o) => o.message !== "Unknown object 'result' used.")).toBe(true); }); - it('should report offenses for lookup key variables in function result target and partial', async () => { + it('should report offenses for lookup key variables in function result target and partial with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% parse_json my_hash %}{}{% endparse_json %} {% function my_hash[lookup_key] = my_hash[path_var] %} `; @@ -242,8 +267,10 @@ describe('Module: UndefinedObject', () => { expect(messages).not.toContain("Unknown object 'my_hash' used."); }); - it('should check the partial variable in function but not the hash-access result target base', async () => { + it('should check the partial variable in function but not the hash-access result target base with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% parse_json my_hash %}{}{% endparse_json %} {% function my_hash['key'] = undefined_partial %} `; @@ -255,8 +282,11 @@ describe('Module: UndefinedObject', () => { expect(messages).not.toContain("Unknown object 'my_hash' used."); }); - it('should report an offense when object is defined in a for loop but used outside of the scope (in scenarios where the same variable has multiple scopes in the file)', async () => { + it('should report an offense when object is defined in a for loop but used outside of the scope (multiple scopes) with doc tag', async () => { const sourceCode = ` + {% doc %} + @param {Array} collections + {% enddoc %} {% for c in collections %} {% comment %} -- Scope 1 -- {% endcomment %} {{ c }} @@ -278,8 +308,10 @@ describe('Module: UndefinedObject', () => { ]); }); - it('should report an offense when undefined object defines another object', async () => { + it('should report an offense when undefined object defines another object with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% assign my_object = my_var %} {{ my_object }} `; @@ -302,8 +334,11 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when object is defined in a tablerow loop but used outside of the scope', async () => { + it('should report an offense when object is defined in a tablerow loop but used outside of the scope with doc tag', async () => { const sourceCode = ` + {% doc %} + @param {Array} collections + {% enddoc %} {% tablerow c in collections %} {{ c }} {% endtablerow %}{{ c }} @@ -315,8 +350,10 @@ describe('Module: UndefinedObject', () => { expect(offenses.map((e) => e.message)).toEqual(["Unknown object 'c' used."]); }); - it('should contextually report on the undefined nature of the form object (defined in form tag, undefined outside)', async () => { + it('should contextually report on the undefined nature of the form object with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% form "cart" %} {{ form }} {% endform %}{{ form }} @@ -328,8 +365,10 @@ describe('Module: UndefinedObject', () => { expect(offenses.map((e) => e.message)).toEqual(["Unknown object 'form' used."]); }); - it('should support {% layout none %}', async () => { + it('should support {% layout none %} with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% layout none %} {{ none }} `; @@ -352,7 +391,7 @@ describe('Module: UndefinedObject', () => { } }); - it('should report an offense when object is undefined in a "partial" file with doc tags that are missing the associated param', async () => { + it('should report an offense when object is undefined in a partial file with empty doc tag', async () => { const sourceCode = ` {% doc %} {% enddoc %} @@ -384,8 +423,10 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when object is not global', async () => { + it('should report an offense when object is not global with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ image }} `; @@ -409,19 +450,21 @@ describe('Module: UndefinedObject', () => { } }); - it('should support contextual exceptions for partials', async () => { + it('should not report offenses for contextual objects without doc tag', async () => { let offenses: Offense[]; const contexts: [string, string][] = [['app', 'app/views/partials/theme-app-extension.liquid']]; for (const [object, goodPath] of contexts) { offenses = await runLiquidCheck(UndefinedObject, `{{ ${object} }}`, goodPath); expect(offenses).toHaveLength(0); offenses = await runLiquidCheck(UndefinedObject, `{{ ${object} }}`, 'file.liquid'); - expect(offenses).toHaveLength(1); + expect(offenses).toHaveLength(0); } }); - it('should report an offense for forloop/tablerowloop used outside of context', async () => { + it('should report an offense for forloop/tablerowloop used outside of context with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ forloop }} {{ tablerowloop }} `; @@ -458,8 +501,10 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when assigning an undefined variable to itself', async () => { + it('should report an offense when assigning an undefined variable to itself with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% assign my_var = my_var | default: "fallback" %} `; @@ -469,7 +514,7 @@ describe('Module: UndefinedObject', () => { expect(offenses[0].message).toBe("Unknown object 'my_var' used."); }); - it('should report an offense when undefined variable is used inside background block', async () => { + it('should not report offenses for undefined variables inside background block without doc tag', async () => { const sourceCode = ` {% background source_type: 'some form' %} {{ undefined_var }} @@ -478,7 +523,7 @@ describe('Module: UndefinedObject', () => { const offenses = await runLiquidCheck(UndefinedObject, sourceCode); - expect(offenses).toHaveLength(1); + expect(offenses).toHaveLength(0); }); it('should not report an offense when job_id is used after background file-based tag', async () => { @@ -503,7 +548,7 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when job_id is used before background file-based tag', async () => { + it('should not report offenses for job_id used before background tag without doc tag', async () => { const sourceCode = ` {{ my_job }} {% background my_job = 'some_partial' %} @@ -511,8 +556,7 @@ describe('Module: UndefinedObject', () => { const offenses = await runLiquidCheck(UndefinedObject, sourceCode); - expect(offenses).toHaveLength(1); - expect(offenses.map((e) => e.message)).toEqual(["Unknown object 'my_job' used."]); + expect(offenses).toHaveLength(0); }); it('should not report an offense when object is defined with a parse_json tag', async () => { @@ -528,8 +572,10 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense when parse_json variable is used before the tag', async () => { + it('should report an offense when parse_json variable is used before the tag with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {{ groups_data }} {% parse_json groups_data %} { "hello": "world" } @@ -556,8 +602,10 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); - it('should report an offense for catch variable used outside catch block', async () => { + it('should report an offense for catch variable used outside catch block with doc tag', async () => { const sourceCode = ` + {% doc %} + {% enddoc %} {% try %} {{ "something" }} {% catch error %} diff --git a/packages/platformos-check-common/src/checks/undefined-object/index.ts b/packages/platformos-check-common/src/checks/undefined-object/index.ts index f151b31..64b53d0 100644 --- a/packages/platformos-check-common/src/checks/undefined-object/index.ts +++ b/packages/platformos-check-common/src/checks/undefined-object/index.ts @@ -58,6 +58,7 @@ export const UndefinedObject: LiquidCheckDefinition = { const scopedVariables: Map = new Map(); const fileScopedVariables: Set = new Set(); const variables: LiquidVariableLookup[] = []; + let hasDocTag = false; function indexVariableScope(variableName: string | null, scope: Scope) { if (!variableName) return; @@ -74,6 +75,12 @@ export const UndefinedObject: LiquidCheckDefinition = { } }, + async LiquidRawTag(node) { + if (node.name === 'doc') { + hasDocTag = true; + } + }, + async LiquidTag(node, ancestors) { if (isWithinRawTagThatDoesNotParseItsContents(ancestors)) return; @@ -191,6 +198,9 @@ export const UndefinedObject: LiquidCheckDefinition = { }, async onCodePathEnd() { + // If no @doc tag, assume undefined variables are params from caller + if (!hasDocTag) return; + const objects = await globalObjects(platformosDocset, relativePath); objects.forEach((obj) => fileScopedVariables.add(obj.name)); diff --git a/packages/platformos-check-common/src/disabled-checks/index.spec.ts b/packages/platformos-check-common/src/disabled-checks/index.spec.ts index b8cb4e0..c2ba6b9 100644 --- a/packages/platformos-check-common/src/disabled-checks/index.spec.ts +++ b/packages/platformos-check-common/src/disabled-checks/index.spec.ts @@ -194,7 +194,7 @@ ${buildComment('platformos-check-enable')} }); it("should disable the parent node's next node if platformos-check is disabled as the last child node", async () => { - const file = `{% liquid + const file = `{% doc %}{% enddoc %}{% liquid if condition # platformos-check-disable-next-line elsif other_condition @@ -211,7 +211,7 @@ ${buildComment('platformos-check-enable')} }); it('should not disable any checks if platformos-check is disabled at the end', async () => { - const file = `{% liquid + const file = `{% doc %}{% enddoc %}{% liquid echo hello echo everyone # platformos-check-disable-next-line @@ -232,7 +232,7 @@ ${buildComment('platformos-check-enable')} }); it('should disable the next line if the content is an HTML tag with liquid', async () => { - const file = `{% # platformos-check-disable-next-line %} + const file = `{% doc %}{% enddoc %}{% # platformos-check-disable-next-line %}
`; @@ -246,7 +246,7 @@ ${buildComment('platformos-check-enable')} }); it('should not disable the next line if the specified rule does not exist', async () => { - const file = `{% # platformos-check-disable-next-line FAKE_RULE %} + const file = `{% doc %}{% enddoc %}{% # platformos-check-disable-next-line FAKE_RULE %}
`; const offenses = await check({ 'code.liquid': file }, [UndefinedObject]); From a3941a69f6a901ee45d2677b1500184bb0ca816a Mon Sep 17 00:00:00 2001 From: Wojciech Grzeszczak Date: Fri, 27 Mar 2026 09:03:28 +0000 Subject: [PATCH 2/5] Pages --- .../src/checks/undefined-object/index.spec.ts | 81 +++++++++++++++++++ .../src/checks/undefined-object/index.ts | 7 +- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts b/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts index 2c7a019..72d5108 100644 --- a/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts +++ b/packages/platformos-check-common/src/checks/undefined-object/index.spec.ts @@ -602,6 +602,87 @@ describe('Module: UndefinedObject', () => { expect(offenses).toHaveLength(0); }); + it('should report an offense for undefined variables in a page file even without doc tag', async () => { + const sourceCode = ` + {{ my_var }} + `; + + const offenses = await runLiquidCheck( + UndefinedObject, + sourceCode, + 'app/views/pages/home.liquid', + ); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe("Unknown object 'my_var' used."); + }); + + it('should report an offense for undefined variables in a module page file even without doc tag', async () => { + const sourceCode = ` + {{ my_var }} + `; + + const modulePaths = [ + 'modules/my_module/public/views/pages/home.liquid', + 'modules/my_module/private/views/pages/home.liquid', + 'app/modules/my_module/public/views/pages/home.liquid', + 'app/modules/my_module/private/views/pages/home.liquid', + ]; + + for (const pagePath of modulePaths) { + const offenses = await runLiquidCheck(UndefinedObject, sourceCode, pagePath); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe("Unknown object 'my_var' used."); + } + }); + + it('should not report offenses for global objects in a page file without doc tag', async () => { + const sourceCode = ` + {{ collections }} + `; + + const offenses = await runLiquidCheck( + UndefinedObject, + sourceCode, + 'app/views/pages/home.liquid', + ); + + expect(offenses).toHaveLength(0); + }); + + it('should not report offenses for assigned variables in a page file without doc tag', async () => { + const sourceCode = ` + {% assign my_var = "hello" %} + {{ my_var }} + `; + + const offenses = await runLiquidCheck( + UndefinedObject, + sourceCode, + 'app/views/pages/home.liquid', + ); + + expect(offenses).toHaveLength(0); + }); + + it('should respect @param in a page file with doc tag', async () => { + const sourceCode = ` + {% doc %} + @param {string} text + {% enddoc %} + {{ text }} + `; + + const offenses = await runLiquidCheck( + UndefinedObject, + sourceCode, + 'app/views/pages/home.liquid', + ); + + expect(offenses).toHaveLength(0); + }); + it('should report an offense for catch variable used outside catch block with doc tag', async () => { const sourceCode = ` {% doc %} diff --git a/packages/platformos-check-common/src/checks/undefined-object/index.ts b/packages/platformos-check-common/src/checks/undefined-object/index.ts index 64b53d0..a12ac49 100644 --- a/packages/platformos-check-common/src/checks/undefined-object/index.ts +++ b/packages/platformos-check-common/src/checks/undefined-object/index.ts @@ -22,6 +22,7 @@ import { import { LiquidCheckDefinition, Severity, SourceCodeType, PlatformOSDocset } from '../../types'; import { isError, last } from '../../utils'; import { isWithinRawTagThatDoesNotParseItsContents } from '../utils'; +import { isPage } from '../../path'; import yaml from 'js-yaml'; type Scope = { start?: number; end?: number }; @@ -198,8 +199,10 @@ export const UndefinedObject: LiquidCheckDefinition = { }, async onCodePathEnd() { - // If no @doc tag, assume undefined variables are params from caller - if (!hasDocTag) return; + const fileIsPage = isPage(context.file.uri); + + // If no @doc tag and not a page, assume undefined variables are params from caller + if (!hasDocTag && !fileIsPage) return; const objects = await globalObjects(platformosDocset, relativePath); From 932ddc0ab8fc60930c22e7b9d4aca2267fce4cbd Mon Sep 17 00:00:00 2001 From: Wojciech Grzeszczak Date: Mon, 30 Mar 2026 07:30:49 +0000 Subject: [PATCH 3/5] Formatting --- .../extract-undefined-variables.spec.ts | 9 ++++++++- .../src/checks/metadata-params/index.spec.ts | 20 +++++-------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts index 528e4be..80705af 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts @@ -55,7 +55,14 @@ describe('extractUndefinedVariables', () => { it('should not include global objects', () => { const source = `{{ context.session }}`; - const result = extractUndefinedVariables(source, ['context', 'null', 'true', 'false', 'blank', 'empty']); + const result = extractUndefinedVariables(source, [ + 'context', + 'null', + 'true', + 'false', + 'blank', + 'empty', + ]); expect(result).to.deep.equal([]); }); diff --git a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts index 4eb47bf..d446037 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts @@ -75,9 +75,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); expect(offenses).to.have.length(1); - expect(offenses).to.containOffense( - 'Unknown parameter extra passed to function call', - ); + expect(offenses).to.containOffense('Unknown parameter extra passed to function call'); }); it('should allow doc-optional params without requiring them', async () => { @@ -144,9 +142,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); expect(offenses).to.have.length(1); - expect(offenses).to.containOffense( - 'Required parameter unused must be passed to function call', - ); + expect(offenses).to.containOffense('Required parameter unused must be passed to function call'); }); it('should infer required params from undefined variables when no doc', async () => { @@ -185,9 +181,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); expect(offenses).to.have.length(1); - expect(offenses).to.containOffense( - 'Required parameter a must be passed to function call', - ); + expect(offenses).to.containOffense('Required parameter a must be passed to function call'); }); it('should report unknown params when passing args not in inferred set', async () => { @@ -207,9 +201,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); expect(offenses).to.have.length(1); - expect(offenses).to.containOffense( - 'Unknown parameter extra passed to function call', - ); + expect(offenses).to.containOffense('Unknown parameter extra passed to function call'); }); it('should not include global objects like context in inferred params', async () => { @@ -242,9 +234,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); expect(offenses).to.have.length(1); - expect(offenses).to.containOffense( - 'Required parameter a must be passed to render call', - ); + expect(offenses).to.containOffense('Required parameter a must be passed to render call'); }); it('should skip validation when no doc and no undefined vars', async () => { From 53db4b5c768b0f73aa5d7f1509f408720502f3cd Mon Sep 17 00:00:00 2001 From: Wojciech Grzeszczak Date: Wed, 1 Apr 2026 07:03:34 +0000 Subject: [PATCH 4/5] Cleanup implementation --- .../extract-undefined-variables.spec.ts | 16 ++++- .../src/checks/metadata-params/index.spec.ts | 5 +- .../src/checks/metadata-params/index.ts | 15 ++++- .../src/checks/unknown-property/index.spec.ts | 62 +++++++++++++++++ .../src/checks/unknown-property/index.ts | 67 +++++++++++++++++++ 5 files changed, 159 insertions(+), 6 deletions(-) diff --git a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts index 80705af..46798f3 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/extract-undefined-variables.spec.ts @@ -47,6 +47,12 @@ describe('extractUndefinedVariables', () => { expect(result).to.deep.equal([]); }); + it('should handle inline graphql result variables', () => { + const source = `{% graphql res %}{ users { id } }{% endgraphql %}{{ res }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal([]); + }); + it('should handle parse_json result variables', () => { const source = `{% parse_json data %}{"a":1}{% endparse_json %}{{ data }}`; const result = extractUndefinedVariables(source); @@ -84,12 +90,18 @@ describe('extractUndefinedVariables', () => { expect(result).to.deep.equal([]); }); - it('should handle hash_assign as definition', () => { - const source = `{% hash_assign h['key'] = 'val' %}{{ h }}`; + it('should handle background file-based result variables', () => { + const source = `{% background my_job = 'some_partial' %}{{ my_job }}`; const result = extractUndefinedVariables(source); expect(result).to.deep.equal([]); }); + it('should handle inline background tag without job_id', () => { + const source = `{% background source_name: 'my_task' %}echo "hello"{% endbackground %}{{ my_job }}`; + const result = extractUndefinedVariables(source); + expect(result).to.deep.equal(['my_job']); + }); + it('should not include doc param names', () => { const source = ` {% doc %} diff --git a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts index d446037..f05ddfa 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/index.spec.ts @@ -122,7 +122,7 @@ describe('Module: MetadataParamsCheck', () => { expect(offenses).to.have.length(0); }); - it('should require doc params even if not used in source', async () => { + it('should not require doc params that are not used in source', async () => { const file = ` {% doc %} @param {String} a - required param @@ -141,8 +141,7 @@ describe('Module: MetadataParamsCheck', () => { const offenses = await check(files, [MetadataParamsCheck]); - expect(offenses).to.have.length(1); - expect(offenses).to.containOffense('Required parameter unused must be passed to function call'); + expect(offenses).to.have.length(0); }); it('should infer required params from undefined variables when no doc', async () => { diff --git a/packages/platformos-check-common/src/checks/metadata-params/index.ts b/packages/platformos-check-common/src/checks/metadata-params/index.ts index 78c014a..15f7c68 100644 --- a/packages/platformos-check-common/src/checks/metadata-params/index.ts +++ b/packages/platformos-check-common/src/checks/metadata-params/index.ts @@ -52,7 +52,20 @@ export const MetadataParamsCheck: LiquidCheckDefinition = { : undefined; if (docDef?.liquidDoc?.parameters) { - requiredParams = docDef.liquidDoc.parameters.filter((p) => p.required).map((p) => p.name); + const globalObjectNames: string[] = []; + if (context.platformosDocset) { + const objects = await context.platformosDocset.objects(); + for (const obj of objects) { + if (!obj.access || obj.access.global === true || obj.access.template.length > 0) { + globalObjectNames.push(obj.name); + } + } + } + const undefinedVars = extractUndefinedVariables(source, globalObjectNames); + const docRequiredNames = docDef.liquidDoc.parameters + .filter((p) => p.required) + .map((p) => p.name); + requiredParams = docRequiredNames.filter((name) => undefinedVars.includes(name)); allowedParams = docDef.liquidDoc.parameters.map((p) => p.name); } else { // No @doc — scan for undefined variables, treat all as required diff --git a/packages/platformos-check-common/src/checks/unknown-property/index.spec.ts b/packages/platformos-check-common/src/checks/unknown-property/index.spec.ts index 042e435..de82728 100644 --- a/packages/platformos-check-common/src/checks/unknown-property/index.spec.ts +++ b/packages/platformos-check-common/src/checks/unknown-property/index.spec.ts @@ -347,6 +347,68 @@ query { }); }); + describe('JSON literal validation', () => { + it('should report unknown property on hash literal', async () => { + const sourceCode = `{% assign a = {x: 5} %}{{ a.b }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'b' on 'a'."); + }); + + it('should not report for valid property on hash literal', async () => { + const sourceCode = `{% assign a = {x: 5, y: 10} %}{{ a.x }}{{ a.y }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(0); + }); + + it('should handle nested hash literals', async () => { + const sourceCode = `{% assign a = {x: {y: 1}} %}{{ a.x.y }}{{ a.x.z }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'z' on 'a.x'."); + }); + + it('should report unknown property access on array literal', async () => { + const sourceCode = `{% assign a = [2, 3] %}{{ a.asd }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'asd' on 'a'."); + }); + + it('should allow first/last/size on array literals', async () => { + const sourceCode = `{% assign a = [2, 3] %}{{ a.first }}{{ a.last }}{{ a.size }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(0); + }); + + it('should allow numeric index on array literals', async () => { + const sourceCode = `{% assign a = [2, 3] %}{{ a[0] }}{{ a[1] }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(0); + }); + + it('should report primitive access on array item from hash literal items', async () => { + const sourceCode = `{% assign a = [{x: 1}, {x: 2}] %}{{ a.first.x }}{{ a.first.y }}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'y' on 'a.first'."); + }); + + it('should report unknown property when assigning from known-shape variable', async () => { + const sourceCode = `{% assign a = [2, 3] %}{% assign b = a.asd %}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'asd' on 'a'."); + }); + + it('should report unknown property on hash literal assigned via another variable', async () => { + const sourceCode = `{% assign a = {a: 5} %}{% assign b = a.b %}`; + const offenses = await runLiquidCheck(UnknownProperty, sourceCode); + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toEqual("Unknown property 'b' on 'a'."); + }); + }); + describe('error message formatting', () => { it('should include variable name in error message', async () => { const sourceCode = `{% assign myVar = '{"a": 1}' | parse_json %} diff --git a/packages/platformos-check-common/src/checks/unknown-property/index.ts b/packages/platformos-check-common/src/checks/unknown-property/index.ts index dfd3802..8ff4dc7 100644 --- a/packages/platformos-check-common/src/checks/unknown-property/index.ts +++ b/packages/platformos-check-common/src/checks/unknown-property/index.ts @@ -12,6 +12,8 @@ import { GraphQLMarkup, GraphQLInlineMarkup, HashAssignMarkup, + JsonHashLiteral, + JsonArrayLiteral, } from '@platformos/liquid-html-parser'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; import { isError } from '../../utils'; @@ -127,6 +129,19 @@ export const UnknownProperty: LiquidCheckDefinition = { } } + // {% assign x = {a: 5, b: {c: 1}} %} or {% assign x = [1, 2, 3] %} + const exprType = markup.value.expression.type; + if (exprType === NodeTypes.JsonHashLiteral || exprType === NodeTypes.JsonArrayLiteral) { + const shape = inferShapeFromLiteralNode(markup.value.expression); + if (shape) { + variableShapes.push({ + name: markup.name, + shape, + range: [node.position.end], + }); + } + } + // {% assign x = y | dig: "key1" | dig: "key2" %} // Follow the dig path through the source variable's known shape. const digFilters = @@ -540,3 +555,55 @@ function isGraphQLInlineMarkup( function isLiquidString(expr: LiquidString | LiquidVariableLookup): expr is LiquidString { return expr.type === NodeTypes.String; } + +/** + * Infer a PropertyShape from a JsonHashLiteral or JsonArrayLiteral AST node. + */ +function inferShapeFromLiteralNode(node: JsonHashLiteral | JsonArrayLiteral): PropertyShape | undefined { + if (node.type === NodeTypes.JsonArrayLiteral) { + let itemShape: PropertyShape | undefined; + for (const element of node.elements) { + const elShape = inferShapeFromExpressionNode(element); + if (elShape) { + itemShape = itemShape ? itemShape : elShape; + } + } + return { kind: 'array', itemShape }; + } + + if (node.type === NodeTypes.JsonHashLiteral) { + const properties = new Map(); + for (const entry of node.entries) { + // Keys are VariableLookup nodes where the name is the key string + if (entry.key.type === NodeTypes.VariableLookup && entry.key.name) { + const valueShape = inferShapeFromExpressionNode(entry.value); + properties.set(entry.key.name, valueShape ?? { kind: 'primitive' }); + } + } + return { kind: 'object', properties }; + } + + return undefined; +} + +/** + * Infer a PropertyShape from a Liquid expression or variable node. + */ +function inferShapeFromExpressionNode(node: LiquidExpression | LiquidVariable): PropertyShape { + if (node.type === NodeTypes.JsonHashLiteral) { + return inferShapeFromLiteralNode(node as JsonHashLiteral) ?? { kind: 'primitive' }; + } + if (node.type === NodeTypes.JsonArrayLiteral) { + return inferShapeFromLiteralNode(node as JsonArrayLiteral) ?? { kind: 'primitive' }; + } + if (node.type === NodeTypes.String) { + return { kind: 'primitive', primitiveType: 'string' }; + } + if (node.type === NodeTypes.Number) { + return { kind: 'primitive', primitiveType: 'number' }; + } + if (node.type === NodeTypes.LiquidLiteral) { + return { kind: 'primitive' }; + } + return { kind: 'primitive' }; +} From 1a9ad749c0f3daba45dc25f101846e12eb3cb574 Mon Sep 17 00:00:00 2001 From: Wojciech Grzeszczak Date: Wed, 1 Apr 2026 07:24:33 +0000 Subject: [PATCH 5/5] Formatting --- .../src/checks/unknown-property/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/platformos-check-common/src/checks/unknown-property/index.ts b/packages/platformos-check-common/src/checks/unknown-property/index.ts index 8ff4dc7..1041406 100644 --- a/packages/platformos-check-common/src/checks/unknown-property/index.ts +++ b/packages/platformos-check-common/src/checks/unknown-property/index.ts @@ -559,7 +559,9 @@ function isLiquidString(expr: LiquidString | LiquidVariableLookup): expr is Liqu /** * Infer a PropertyShape from a JsonHashLiteral or JsonArrayLiteral AST node. */ -function inferShapeFromLiteralNode(node: JsonHashLiteral | JsonArrayLiteral): PropertyShape | undefined { +function inferShapeFromLiteralNode( + node: JsonHashLiteral | JsonArrayLiteral, +): PropertyShape | undefined { if (node.type === NodeTypes.JsonArrayLiteral) { let itemShape: PropertyShape | undefined; for (const element of node.elements) {