From ec44bc7a4f167256b3adac1684a89bee9ab4a1c3 Mon Sep 17 00:00:00 2001 From: Maciej Krajowski-Kukiel Date: Sun, 15 Mar 2026 13:09:52 +0100 Subject: [PATCH 1/2] add support for theme render rc --- .../src/stage-1-cst.spec.ts | 91 ++++- .../src/stage-2-ast.spec.ts | 105 ++++- .../src/checks/missing-page/index.ts | 9 +- .../src/checks/missing-partial/index.spec.ts | 360 ++++++++++++++++++ .../src/checks/missing-partial/index.ts | 90 +++-- .../src/url-helpers.ts | 28 +- .../DocumentsLocator.spec.ts | 304 ++++++++++++++- .../src/documents-locator/DocumentsLocator.ts | 145 ++++++- .../src/route-table/RouteTable.ts | 10 +- .../src/TypeSystem.ts | 5 +- .../src/definitions/DefinitionProvider.ts | 25 +- .../RenderPartialDefinitionProvider.spec.ts | 140 +++++++ .../RenderPartialDefinitionProvider.ts | 70 ++++ .../DocumentLinksProvider.spec.ts | 141 ++++++- .../documentLinks/DocumentLinksProvider.ts | 55 ++- .../src/server/CachedFileSystem.ts | 13 +- .../src/server/startServer.ts | 24 +- .../src/utils/searchPaths.spec.ts | 59 +++ .../src/utils/searchPaths.ts | 16 + .../src/printer/print/liquid.ts | 6 +- 20 files changed, 1561 insertions(+), 135 deletions(-) create mode 100644 packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts create mode 100644 packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts create mode 100644 packages/platformos-language-server-common/src/utils/searchPaths.spec.ts create mode 100644 packages/platformos-language-server-common/src/utils/searchPaths.ts diff --git a/packages/liquid-html-parser/src/stage-1-cst.spec.ts b/packages/liquid-html-parser/src/stage-1-cst.spec.ts index 9c55ddd..96f6ba0 100644 --- a/packages/liquid-html-parser/src/stage-1-cst.spec.ts +++ b/packages/liquid-html-parser/src/stage-1-cst.spec.ts @@ -2182,13 +2182,90 @@ describe('Unit: Stage 1 (CST)', () => { }); it('should parse the theme_render_rc tag', () => { - for (const { toCST, expectPath } of testCases) { - cst = toCST(`{% theme_render_rc 'partial' %}`); - expectPath(cst, '0.type').to.equal('LiquidTag'); - expectPath(cst, '0.name').to.equal('theme_render_rc'); - expectPath(cst, '0.markup.type').to.equal('RenderMarkup'); - expectPath(cst, '0.markup.partial.type').to.equal('String'); - } + [ + { + expression: `'partial'`, + partialType: 'String', + alias: null, + renderVariableExpression: null, + namedArguments: [], + }, + { + expression: `'partial' as foo`, + partialType: 'String', + alias: { + value: 'foo', + }, + renderVariableExpression: null, + namedArguments: [], + }, + { + expression: `'partial' with product as item`, + partialType: 'String', + alias: { + value: 'item', + }, + renderVariableExpression: { + kind: 'with', + name: { + type: 'VariableLookup', + }, + }, + namedArguments: [], + }, + { + expression: `'partial' for products as product`, + partialType: 'String', + alias: { + value: 'product', + }, + renderVariableExpression: { + kind: 'for', + name: { + type: 'VariableLookup', + }, + }, + namedArguments: [], + }, + { + expression: `'my/partial', key1: val1, key2: "hi"`, + partialType: 'String', + alias: null, + renderVariableExpression: null, + namedArguments: [ + { name: 'key1', valueType: 'VariableLookup' }, + { name: 'key2', valueType: 'String' }, + ], + }, + ].forEach( + ({ expression, partialType, renderVariableExpression, alias, namedArguments }) => { + for (const { toCST, expectPath } of testCases) { + cst = toCST(`{% theme_render_rc ${expression} -%}`); + expectPath(cst, '0.type').to.equal('LiquidTag'); + expectPath(cst, '0.name').to.equal('theme_render_rc'); + expectPath(cst, '0.markup.type').to.equal('RenderMarkup'); + expectPath(cst, '0.markup.partial.type').to.equal(partialType); + if (renderVariableExpression) { + expectPath(cst, '0.markup.variable.type').to.equal('RenderVariableExpression'); + expectPath(cst, '0.markup.variable.kind').to.equal(renderVariableExpression.kind); + expectPath(cst, '0.markup.variable.name.type').to.equal( + renderVariableExpression.name.type, + ); + } else { + expectPath(cst, '0.markup.variable').to.equal(null); + } + expectPath(cst, '0.markup.alias.value').to.equal(alias?.value); + expectPath(cst, '0.markup.renderArguments').to.have.lengthOf(namedArguments.length); + namedArguments.forEach(({ name, valueType }, i) => { + expectPath(cst, `0.markup.renderArguments.${i}.type`).to.equal('NamedArgument'); + expectPath(cst, `0.markup.renderArguments.${i}.name`).to.equal(name); + expectPath(cst, `0.markup.renderArguments.${i}.value.type`).to.equal(valueType); + }); + expectPath(cst, '0.whitespaceStart').to.equal(null); + expectPath(cst, '0.whitespaceEnd').to.equal('-'); + } + }, + ); }); it('should parse the response_status tag', () => { diff --git a/packages/liquid-html-parser/src/stage-2-ast.spec.ts b/packages/liquid-html-parser/src/stage-2-ast.spec.ts index 2844b0f..fd182f2 100644 --- a/packages/liquid-html-parser/src/stage-2-ast.spec.ts +++ b/packages/liquid-html-parser/src/stage-2-ast.spec.ts @@ -1543,15 +1543,102 @@ describe('Unit: Stage 2 (AST)', () => { }); it('should parse the theme_render_rc tag', () => { - for (const { toAST, expectPath, expectPosition } of testCases) { - ast = toAST(`{% theme_render_rc 'partial' %}`); - expectPath(ast, 'children.0.type').to.equal('LiquidTag'); - expectPath(ast, 'children.0.name').to.equal('theme_render_rc'); - expectPath(ast, 'children.0.markup.type').to.equal('RenderMarkup'); - expectPath(ast, 'children.0.markup.partial.type').to.equal('String'); - expectPosition(ast, 'children.0'); - expectPosition(ast, 'children.0.markup'); - } + [ + { + expression: `'partial'`, + partialType: 'String', + alias: null, + renderVariableExpression: null, + namedArguments: [], + }, + { + expression: `'partial' as foo`, + partialType: 'String', + alias: { + value: 'foo', + }, + renderVariableExpression: null, + namedArguments: [], + }, + { + expression: `'partial' with product as item`, + partialType: 'String', + alias: { + value: 'item', + }, + renderVariableExpression: { + kind: 'with', + name: { + type: 'VariableLookup', + }, + }, + namedArguments: [], + }, + { + expression: `'partial' for products as product`, + partialType: 'String', + alias: { + value: 'product', + }, + renderVariableExpression: { + kind: 'for', + name: { + type: 'VariableLookup', + }, + }, + namedArguments: [], + }, + { + expression: `'my/partial', key1: val1, key2: "hi"`, + partialType: 'String', + alias: null, + renderVariableExpression: null, + namedArguments: [ + { name: 'key1', valueType: 'VariableLookup' }, + { name: 'key2', valueType: 'String' }, + ], + }, + ].forEach( + ({ expression, partialType, renderVariableExpression, alias, namedArguments }) => { + for (const { toAST, expectPath, expectPosition } of testCases) { + ast = toAST(`{% theme_render_rc ${expression} -%}`); + expectPath(ast, 'children.0.type').to.equal('LiquidTag'); + expectPath(ast, 'children.0.name').to.equal('theme_render_rc'); + expectPath(ast, 'children.0.markup.type').to.equal('RenderMarkup'); + expectPath(ast, 'children.0.markup.partial.type').to.equal(partialType); + if (renderVariableExpression) { + expectPath(ast, 'children.0.markup.variable.type').to.equal( + 'RenderVariableExpression', + ); + expectPath(ast, 'children.0.markup.variable.kind').to.equal( + renderVariableExpression.kind, + ); + expectPath(ast, 'children.0.markup.variable.name.type').to.equal( + renderVariableExpression.name.type, + ); + } else { + expectPath(ast, 'children.0.markup.variable').to.equal(null); + } + if (alias) { + expectPath(ast, 'children.0.markup.alias.value').to.equal(alias.value); + } else { + expectPath(ast, 'children.0.markup.alias').to.equal(null); + } + expectPath(ast, 'children.0.markup.args').to.have.lengthOf(namedArguments.length); + namedArguments.forEach(({ name, valueType }, i) => { + expectPath(ast, `children.0.markup.args.${i}.type`).to.equal('NamedArgument'); + expectPath(ast, `children.0.markup.args.${i}.name`).to.equal(name); + expectPath(ast, `children.0.markup.args.${i}.value.type`).to.equal(valueType); + expectPosition(ast, `children.0.markup.args.${i}`); + expectPosition(ast, `children.0.markup.args.${i}.value`); + }); + expectPath(ast, 'children.0.whitespaceStart').to.equal(''); + expectPath(ast, 'children.0.whitespaceEnd').to.equal('-'); + expectPosition(ast, 'children.0'); + expectPosition(ast, 'children.0.markup'); + } + }, + ); }); it('should parse the response_status tag', () => { diff --git a/packages/platformos-check-common/src/checks/missing-page/index.ts b/packages/platformos-check-common/src/checks/missing-page/index.ts index 3c60c54..5454aab 100644 --- a/packages/platformos-check-common/src/checks/missing-page/index.ts +++ b/packages/platformos-check-common/src/checks/missing-page/index.ts @@ -56,6 +56,11 @@ export const MissingPage: LiquidCheckDefinition = { } return { + async onCodePathStart() { + // Front-load the route table build so individual HtmlElement visits don't wait. + routeTable = await context.getRouteTable(); + }, + async LiquidTag(node: LiquidTag) { if (node.name !== NamedTags.assign) return; const markup = (node as LiquidTagAssign).markup as AssignMarkup; @@ -68,10 +73,6 @@ export const MissingPage: LiquidCheckDefinition = { }, async HtmlElement(node) { - if (!routeTable) { - routeTable = await context.getRouteTable(); - } - if (isHtmlTag(node, 'a')) { const hrefAttr = node.attributes.find( (a) => isValuedAttrNode(a) && getAttrName(a) === 'href', diff --git a/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts b/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts index 0fc0b4a..329b0c2 100644 --- a/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts +++ b/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts @@ -47,6 +47,19 @@ describe('Module: MissingPartial', () => { 'app/views/partials/partial.liquid': file, }), }, + { + testCase: 'should report the missing partial to be rendered with "theme_render_rc"', + file: "{% theme_render_rc 'missing' %}", + expected: { + message: "'missing' does not exist", + uri: 'file:///app/views/partials/partial.liquid', + start: { index: 19, line: 0, character: 19 }, + end: { index: 28, line: 0, character: 28 }, + }, + filesWith: (file: string) => ({ + 'app/views/partials/partial.liquid': file, + }), + }, ]; for (const { testCase, file, expected, filesWith } of testCases) { const offenses = await check(filesWith(file), [MissingPartial]); @@ -58,4 +71,351 @@ describe('Module: MissingPartial', () => { }); } }); + + it('should not report when the partial exists for theme_render_rc', async () => { + const offenses = await check( + { + 'app/views/partials/partial.liquid': + "{% theme_render_rc 'my_product', class: 'featured' %}", + 'app/views/partials/my_product.liquid': '
Product
', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should not report theme_render_rc with variable lookup', async () => { + const offenses = await check( + { + 'app/views/partials/partial.liquid': + "{% theme_render_rc 'existing' for products as product %}", + 'app/views/partials/existing.liquid': '{{ product }}', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + describe('theme_render_rc with theme_search_paths', () => { + it('should find partial via first search path (highest priority)', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'another/super/partial' %}", + 'app/views/partials/theme/dress/another/super/partial.liquid': 'dress partial', + 'app/views/partials/theme/simple/another/super/partial.liquid': 'simple partial', + 'app/views/partials/another/super/partial.liquid': 'default partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should find partial via second search path when first does not have it', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'my/partial' %}", + 'app/views/partials/theme/simple/my/partial.liquid': 'simple partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should fallback to default path when no search path matches', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'default' %}", + 'app/views/partials/default.liquid': 'default partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should report missing when partial is not found in any search path or fallback', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'my/missing' %}", + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense({ + check: 'MissingPartial', + message: "'my/missing' does not exist", + }); + }); + + it('should respect empty string in search paths as default path position', async () => { + // With ['theme/dress', '', 'theme/simple'], the empty string means "default path" + // appears between dress and simple in priority order. + // So if dress doesn't have it but default path does, use default (skip simple). + const offenses = await check( + { + 'app/config.yml': "theme_search_paths:\n - theme/dress\n - ''\n - theme/simple", + 'app/views/partials/page.liquid': "{% theme_render_rc 'my/partial' %}", + 'app/views/partials/my/partial.liquid': 'default partial', + 'app/views/partials/theme/simple/my/partial.liquid': 'simple partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should expand Liquid expressions as wildcards and find matching partial', async () => { + // {{ context.constants.MY_THEME }} acts as a wildcard matching any subdirectory. + // With theme/custom_theme/my/partial.liquid existing, the wildcard should match it. + const offenses = await check( + { + 'app/config.yml': + 'theme_search_paths:\n - theme/{{ context.constants.MY_THEME }}\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'my/partial' %}", + 'app/views/partials/theme/custom_theme/my/partial.liquid': 'custom theme partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should find partial via static path when dynamic path has no match', async () => { + const offenses = await check( + { + 'app/config.yml': + 'theme_search_paths:\n - theme/{{ context.constants.MY_THEME }}\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'product' %}", + 'app/views/partials/theme/simple/product.liquid': 'simple partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should report missing when wildcard expansion finds no matching partial', async () => { + // Dynamic path expands but the partial doesn't exist in any expanded directory + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/{{ context.constants.MY_THEME }}', + 'app/views/partials/page.liquid': "{% theme_render_rc 'missing' %}", + 'app/views/partials/theme/custom/other.liquid': 'wrong partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense({ + check: 'MissingPartial', + message: "'missing' does not exist", + }); + }); + + it('should handle multiple Liquid expressions in a path', async () => { + // E.g. "{{ context.constants.BRAND }}/{{ context.constants.TIER }}" - both segments are wildcards + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - "{{ context.constants.BRAND }}/{{ context.constants.TIER }}"', + 'app/views/partials/page.liquid': "{% theme_render_rc 'card' %}", + 'app/views/partials/acme/premium/card.liquid': 'acme premium card', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should render partial which includes path in its name', async () => { + // When the partial name itself includes the search path prefix + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'theme/simple/my/partial' %}", + 'app/views/partials/theme/simple/my/partial.liquid': 'simple partial', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should work with nested partial directories', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% theme_render_rc 'components/card' %}", + 'app/views/partials/theme/dress/components/card.liquid': 'dress card', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should not affect regular render tags even when config exists', async () => { + // 'card' exists under the search path prefix, so theme_render_rc would find it, + // but render ignores search paths entirely and looks only in the default locations. + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': "{% render 'card' %}", + 'app/views/partials/theme/simple/card.liquid': 'simple card', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense({ + check: 'MissingPartial', + message: "'card' does not exist", + }); + }); + + it('should also search in app/lib with search paths', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress', + 'app/views/partials/page.liquid': "{% theme_render_rc 'my_helper' %}", + 'app/lib/theme/dress/my_helper.liquid': 'helper from lib', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should fallback to standard render resolution when no config.yml exists', async () => { + const offenses = await check( + { + 'app/views/partials/page.liquid': "{% theme_render_rc 'existing' %}", + 'app/views/partials/existing.liquid': 'found it', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should handle theme_render_rc with named arguments and search paths', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress', + 'app/views/partials/page.liquid': + "{% theme_render_rc 'product', class: 'featured', size: 'large' %}", + 'app/views/partials/theme/dress/product.liquid': '{{ class }} {{ size }}', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should handle theme_render_rc with for/with and search paths', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress', + 'app/views/partials/page.liquid': "{% theme_render_rc 'item' for products as product %}", + 'app/views/partials/theme/dress/item.liquid': '{{ product }}', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should treat empty theme_search_paths array same as absent', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths: []', + 'app/views/partials/page.liquid': "{% theme_render_rc 'existing' %}", + 'app/views/partials/existing.liquid': 'found it', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should handle malformed theme_search_paths (not an array)', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths: some_string', + 'app/views/partials/page.liquid': "{% theme_render_rc 'existing' %}", + 'app/views/partials/existing.liquid': 'found it', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should handle multiple theme_render_rc tags in one file', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'app/views/partials/page.liquid': + "{% theme_render_rc 'header' %} {% theme_render_rc 'footer' %} {% theme_render_rc 'missing' %}", + 'app/views/partials/theme/dress/header.liquid': 'header', + 'app/views/partials/theme/simple/footer.liquid': 'footer', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(1); + expect(offenses).to.containOffense({ + check: 'MissingPartial', + message: "'missing' does not exist", + }); + }); + + it('should handle theme_render_rc inside {% liquid %} block', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress', + 'app/views/partials/page.liquid': "{% liquid\n theme_render_rc 'card'\n%}", + 'app/views/partials/theme/dress/card.liquid': 'card', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should resolve module-prefixed partials via fallback', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - theme/dress', + 'app/views/partials/page.liquid': "{% theme_render_rc 'modules/shop/card' %}", + 'modules/shop/public/views/partials/card.liquid': 'module card', + }, + [MissingPartial], + ); + + expect(offenses).to.have.length(0); + }); + + it('should handle non-string entries in theme_search_paths gracefully', async () => { + const offenses = await check( + { + 'app/config.yml': 'theme_search_paths:\n - 123\n - true', + 'app/views/partials/page.liquid': "{% theme_render_rc 'card' %}", + 'app/views/partials/card.liquid': 'default card', + }, + [MissingPartial], + ); + + // 123/card and true/card won't exist, but fallback to unprefixed finds it + expect(offenses).to.have.length(0); + }); + }); }); diff --git a/packages/platformos-check-common/src/checks/missing-partial/index.ts b/packages/platformos-check-common/src/checks/missing-partial/index.ts index fb7f7d0..10cb4b7 100644 --- a/packages/platformos-check-common/src/checks/missing-partial/index.ts +++ b/packages/platformos-check-common/src/checks/missing-partial/index.ts @@ -1,8 +1,20 @@ -import { NodeTypes } from '@platformos/liquid-html-parser'; +import { LiquidHtmlNode, NamedTags, NodeTypes } from '@platformos/liquid-html-parser'; import { LiquidCheckDefinition, SchemaProp, Severity, SourceCodeType } from '../../types'; -import { DocumentsLocator } from '@platformos/platformos-common'; +import { + DocumentsLocator, + DocumentType, + loadSearchPaths, +} from '@platformos/platformos-common'; import { URI } from 'vscode-uri'; +function getTagName(ancestors: LiquidHtmlNode[]): DocumentType { + const parent = ancestors.at(-1); + if (parent?.type === NodeTypes.LiquidTag && parent.name === NamedTags.theme_render_rc) { + return 'theme_render_rc'; + } + return 'render'; +} + const schema = { ignoreMissing: SchemaProp.array(SchemaProp.string(), []), }; @@ -24,62 +36,46 @@ export const MissingPartial: LiquidCheckDefinition = { create(context) { const locator = new DocumentsLocator(context.fs); + const rootUri = URI.parse(context.config.rootUri); + let searchPathsPromise: Promise | undefined; - return { - async RenderMarkup(node) { - if (node.partial.type === NodeTypes.VariableLookup) return; + function getSearchPaths(): Promise { + searchPathsPromise ??= loadSearchPaths(context.fs, rootUri); + return searchPathsPromise; + } - const partial = node.partial; - const location = await locator.locate( - URI.parse(context.config.rootUri), - 'render', - partial.value, - ); + async function reportIfMissing( + docType: DocumentType, + name: string, + position: LiquidHtmlNode['position'], + ) { + const searchPaths = docType === 'theme_render_rc' ? await getSearchPaths() : null; + const location = await locator.locate(rootUri, docType, name, searchPaths); + if (!location) { + context.report({ + message: `'${name}' does not exist`, + startIndex: position.start, + endIndex: position.end, + }); + } + } - if (!location) { - context.report({ - message: `'${partial.value}' does not exist`, - startIndex: node.partial.position.start, - endIndex: node.partial.position.end, - }); + return { + async RenderMarkup(node, ancestors) { + if (node.partial.type !== NodeTypes.VariableLookup) { + await reportIfMissing(getTagName(ancestors), node.partial.value, node.partial.position); } }, async FunctionMarkup(node) { - if (node.partial.type === NodeTypes.VariableLookup) return; - - const partial = node.partial; - const location = await locator.locate( - URI.parse(context.config.rootUri), - 'function', - partial.value, - ); - - if (!location) { - context.report({ - message: `'${partial.value}' does not exist`, - startIndex: node.partial.position.start, - endIndex: node.partial.position.end, - }); + if (node.partial.type !== NodeTypes.VariableLookup) { + await reportIfMissing('function', node.partial.value, node.partial.position); } }, async GraphQLMarkup(node) { - if (node.graphql.type === NodeTypes.VariableLookup) return; - - const graphql = node.graphql; - const location = await locator.locate( - URI.parse(context.config.rootUri), - 'graphql', - graphql.value, - ); - - if (!location) { - context.report({ - message: `'${graphql.value}' does not exist`, - startIndex: node.graphql.position.start, - endIndex: node.graphql.position.end, - }); + if (node.graphql.type !== NodeTypes.VariableLookup) { + await reportIfMissing('graphql', node.graphql.value, node.graphql.position); } }, }; diff --git a/packages/platformos-check-common/src/url-helpers.ts b/packages/platformos-check-common/src/url-helpers.ts index 61e86ff..68abb9b 100644 --- a/packages/platformos-check-common/src/url-helpers.ts +++ b/packages/platformos-check-common/src/url-helpers.ts @@ -28,6 +28,24 @@ import { * remains in platformos-common. */ +/** Extract the `children` array from a node if it has one (block tags, elements). */ +function getTraversableChildren(node: LiquidHtmlNode): LiquidHtmlNode[] | null { + if ('children' in node) { + const children = (node as { children: unknown }).children; + if (Array.isArray(children)) return children as LiquidHtmlNode[]; + } + return null; +} + +/** Extract the `markup` array from a node if it is an array ({% liquid %} tags). */ +function getTraversableMarkup(node: LiquidHtmlNode): LiquidHtmlNode[] | null { + if ('markup' in node) { + const markup = (node as { markup: unknown }).markup; + if (Array.isArray(markup)) return markup as LiquidHtmlNode[]; + } + return null; +} + const SKIP_PREFIXES = ['http://', 'https://', '//', 'mailto:', 'tel:', 'javascript:', 'data:', '#']; export function shouldSkipUrl(url: string): boolean { @@ -311,14 +329,12 @@ export function buildVariableMap( } // Recurse into children (block tags like {% if %}, {% for %}) - if ('children' in node && Array.isArray((node as any).children)) { - walk((node as any).children); - } + const children = getTraversableChildren(node); + if (children) walk(children); // Recurse into markup arrays ({% liquid %} block contains assigns in markup) - if ('markup' in node && Array.isArray((node as any).markup)) { - walk((node as any).markup); - } + const markupArray = getTraversableMarkup(node); + if (markupArray) walk(markupArray); } } diff --git a/packages/platformos-common/src/documents-locator/DocumentsLocator.spec.ts b/packages/platformos-common/src/documents-locator/DocumentsLocator.spec.ts index 62081a5..af59b76 100644 --- a/packages/platformos-common/src/documents-locator/DocumentsLocator.spec.ts +++ b/packages/platformos-common/src/documents-locator/DocumentsLocator.spec.ts @@ -1,16 +1,28 @@ import { describe, it, expect, vi } from 'vitest'; import { URI } from 'vscode-uri'; -import { DocumentsLocator } from './DocumentsLocator'; +import { DocumentsLocator, loadSearchPaths } from './DocumentsLocator'; import { AbstractFileSystem, FileType, FileStat, FileTuple } from '../AbstractFileSystem'; function createMockFileSystem(files: Record): AbstractFileSystem { const fileSet = new Set(Object.keys(files)); + // Build directory tree from file paths + const dirs = new Set(); + for (const filePath of fileSet) { + const parts = filePath.split('/'); + for (let i = 1; i < parts.length; i++) { + dirs.add(parts.slice(0, i).join('/')); + } + } + return { stat: vi.fn(async (uri: string): Promise => { if (fileSet.has(uri)) { return { type: FileType.File, size: files[uri].length }; } + if (dirs.has(uri)) { + return { type: FileType.Directory, size: 0 }; + } throw new Error(`File not found: ${uri}`); }), readFile: vi.fn(async (uri: string): Promise => { @@ -21,10 +33,19 @@ function createMockFileSystem(files: Record): AbstractFileSystem }), readDirectory: vi.fn(async (uri: string): Promise => { const results: FileTuple[] = []; - for (const filePath of fileSet) { - if (filePath.startsWith(uri)) { - results.push([filePath, FileType.File]); - } + const seen = new Set(); + const prefix = uri.endsWith('/') ? uri : uri + '/'; + + for (const path of [...fileSet, ...dirs]) { + if (!path.startsWith(prefix)) continue; + const rest = path.slice(prefix.length); + const firstSegment = rest.split('/')[0]; + if (!firstSegment || seen.has(firstSegment)) continue; + seen.add(firstSegment); + + const fullPath = prefix + firstSegment; + const isDir = dirs.has(fullPath) && !fileSet.has(fullPath); + results.push([fullPath, isDir ? FileType.Directory : FileType.File]); } return results; }), @@ -137,4 +158,277 @@ describe('DocumentsLocator', () => { expect(result).toEqual([]); }); }); + + describe('locateWithSearchPaths', () => { + it('should find partial via first search path', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/dress/card.liquid': 'dress', + 'file:///project/app/views/partials/theme/simple/card.liquid': 'simple', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', [ + 'theme/dress', + 'theme/simple', + ]); + + expect(result).toBe('file:///project/app/views/partials/theme/dress/card.liquid'); + }); + + it('should fall through to second search path', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/simple/card.liquid': 'simple', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', [ + 'theme/dress', + 'theme/simple', + ]); + + expect(result).toBe('file:///project/app/views/partials/theme/simple/card.liquid'); + }); + + it('should fallback to unprefixed path when no search path matches', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/card.liquid': 'default', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', [ + 'theme/dress', + 'theme/simple', + ]); + + expect(result).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should not fallback when empty string is in search paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/card.liquid': 'default', + }); + const locator = new DocumentsLocator(fs); + + // '' is at position 0, so it tries default path first, finds it + const result = await locator.locateWithSearchPaths(rootUri, 'card', ['', 'theme/dress']); + expect(result).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should return undefined when nothing matches', async () => { + const fs = createMockFileSystem({}); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', ['theme/dress']); + expect(result).toBeUndefined(); + }); + + it('should handle nested partial names with search paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/dress/components/hero.liquid': 'hero', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'components/hero', [ + 'theme/dress', + ]); + + expect(result).toBe('file:///project/app/views/partials/theme/dress/components/hero.liquid'); + }); + + it('should also search app/lib with search paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/lib/theme/dress/helper.liquid': 'helper', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'helper', ['theme/dress']); + + expect(result).toBe('file:///project/app/lib/theme/dress/helper.liquid'); + }); + + it('should expand dynamic Liquid expressions as wildcards', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/custom/card.liquid': 'custom card', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', [ + 'theme/{{ context.constants.THEME }}', + ]); + + expect(result).toBe('file:///project/app/views/partials/theme/custom/card.liquid'); + }); + + it('should expand multiple wildcards in a single path', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/acme/premium/card.liquid': 'card', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'card', [ + '{{ context.constants.BRAND }}/{{ context.constants.TIER }}', + ]); + + expect(result).toBe('file:///project/app/views/partials/acme/premium/card.liquid'); + }); + + it('should return undefined when wildcard expands but partial not found', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/custom/other.liquid': 'other', + }); + const locator = new DocumentsLocator(fs); + + const result = await locator.locateWithSearchPaths(rootUri, 'missing', [ + 'theme/{{ context.constants.THEME }}', + ]); + + // Fallback to unprefixed — also not found + expect(result).toBeUndefined(); + }); + + it('should cache expanded paths across calls', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/custom/a.liquid': 'a', + 'file:///project/app/views/partials/theme/custom/b.liquid': 'b', + }); + const locator = new DocumentsLocator(fs); + const searchPaths = ['theme/{{ x }}']; + + await locator.locateWithSearchPaths(rootUri, 'a', searchPaths); + const readDirSpy = fs.readDirectory as ReturnType; + const callCountAfterFirst = readDirSpy.mock.calls.length; + + await locator.locateWithSearchPaths(rootUri, 'b', searchPaths); + // readDirectory should not be called again for wildcard expansion + // (only for locateFile stat calls, not for listSubdirectories) + const expansionCalls = readDirSpy.mock.calls.filter( + (call: string[]) => + call[0].includes('app/views/partials/theme') && !call[0].includes('.liquid'), + ); + // All expansion readDirectory calls should come from the first invocation + const expansionCallsAfterFirst = readDirSpy.mock.calls + .slice(callCountAfterFirst) + .filter( + (call: string[]) => + call[0].includes('app/views/partials/theme') && !call[0].includes('.liquid'), + ); + expect(expansionCallsAfterFirst).toHaveLength(0); + }); + + it('should clear expanded paths cache', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/views/partials/theme/v1/card.liquid': 'v1', + }); + const locator = new DocumentsLocator(fs); + + const result1 = await locator.locateWithSearchPaths(rootUri, 'card', ['theme/{{ version }}']); + expect(result1).toBe('file:///project/app/views/partials/theme/v1/card.liquid'); + + locator.clearExpandedPathsCache(); + + // After clearing, a fresh expansion should work (same result since fs unchanged) + const result2 = await locator.locateWithSearchPaths(rootUri, 'card', ['theme/{{ version }}']); + expect(result2).toBe('file:///project/app/views/partials/theme/v1/card.liquid'); + }); + + it('should handle module-prefixed partials with search paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/modules/shop/public/views/partials/card.liquid': 'module card', + }); + const locator = new DocumentsLocator(fs); + + // module path in fallback (search paths don't apply to module prefix) + const result = await locator.locateWithSearchPaths(rootUri, 'modules/shop/card', [ + 'theme/dress', + ]); + + expect(result).toBe('file:///project/app/modules/shop/public/views/partials/card.liquid'); + }); + }); + + describe('loadSearchPaths', () => { + it('should load valid theme_search_paths from config', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toEqual(['theme/dress', 'theme/simple']); + }); + + it('should return null when config file does not exist', async () => { + const fs = createMockFileSystem({}); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toBeNull(); + }); + + it('should return null for empty array', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': 'theme_search_paths: []', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toBeNull(); + }); + + it('should return null when theme_search_paths is not an array', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': 'theme_search_paths: some_string', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toBeNull(); + }); + + it('should return null when config has no theme_search_paths key', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': 'some_other_key: value', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toBeNull(); + }); + + it('should coerce non-string entries to strings', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': 'theme_search_paths:\n - 123\n - true\n - null', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toEqual(['123', 'true', 'null']); + }); + + it('should handle config with Liquid expressions in paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': + 'theme_search_paths:\n - "theme/{{ context.constants.MY_THEME | default: \'custom\' }}"\n - theme/simple', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toEqual([ + "theme/{{ context.constants.MY_THEME | default: 'custom' }}", + 'theme/simple', + ]); + }); + + it('should handle malformed YAML gracefully', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': '{{invalid yaml', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toBeNull(); + }); + + it('should handle config with other properties alongside theme_search_paths', async () => { + const fs = createMockFileSystem({ + 'file:///project/app/config.yml': + 'some_setting: true\ntheme_search_paths:\n - theme/dress\nanother_setting: 42', + }); + + const result = await loadSearchPaths(fs, rootUri); + expect(result).toEqual(['theme/dress']); + }); + }); }); diff --git a/packages/platformos-common/src/documents-locator/DocumentsLocator.ts b/packages/platformos-common/src/documents-locator/DocumentsLocator.ts index e9ae461..f130d5c 100644 --- a/packages/platformos-common/src/documents-locator/DocumentsLocator.ts +++ b/packages/platformos-common/src/documents-locator/DocumentsLocator.ts @@ -1,8 +1,38 @@ +import yaml from 'js-yaml'; import { AbstractFileSystem, FileType } from '../AbstractFileSystem'; import { getAppPaths, getModulePaths, PlatformOSFileType } from '../path-utils'; import { URI, Utils } from 'vscode-uri'; -export type DocumentType = 'function' | 'render' | 'include' | 'graphql' | 'asset'; +export type DocumentType = + | 'function' + | 'render' + | 'include' + | 'graphql' + | 'asset' + | 'theme_render_rc'; + +/** + * Load theme_search_paths from app/config.yml. + * Returns null if the file doesn't exist, is malformed, or has no valid theme_search_paths. + * Results should be cached per root URI. + */ +export async function loadSearchPaths( + fs: { readFile(uri: string): Promise }, + rootUri: URI, +): Promise { + try { + const configUri = Utils.joinPath(rootUri, 'app/config.yml').toString(); + const content = await fs.readFile(configUri); + const config = yaml.load(content) as Record | null; + const paths = config?.theme_search_paths; + if (Array.isArray(paths) && paths.length > 0) { + return paths.map(String); + } + return null; + } catch { + return null; + } +} type ModulePathInfo = | { isModule: false; key: string } @@ -127,10 +157,117 @@ export class DocumentsLocator { return Array.from(results).sort((a, b) => a.localeCompare(b)); } + private static readonly LIQUID_EXPRESSION_RE = /\{\{.*?\}\}/; + + private async listSubdirectoryNames(dirUri: string): Promise { + try { + const entries = await this.fs.readDirectory(dirUri); + return entries + .filter(([, type]) => type === FileType.Directory) + .map(([name]) => { + const lastSlash = name.lastIndexOf('/'); + return lastSlash === -1 ? name : name.slice(lastSlash + 1); + }) + .filter((name) => name.length > 0); + } catch { + return []; + } + } + + private expandedPathsCache = new Map>(); + + clearExpandedPathsCache(): void { + this.expandedPathsCache.clear(); + } + + /** + * Expand a search path that may contain {{ ... }} Liquid expressions into + * concrete directory prefixes by enumerating subdirectories at each dynamic + * segment. Static segments pass through unchanged. + * + * Results are cached per (rootUri, searchPath) and capped at 100 entries. + */ + private async expandDynamicPath(rootUri: URI, searchPath: string): Promise { + const segments = searchPath.split('/'); + const basePaths = this.getSearchPaths('partial'); + let prefixes = ['']; + + for (const segment of segments) { + if (!DocumentsLocator.LIQUID_EXPRESSION_RE.test(segment)) { + prefixes = prefixes.map((p) => (p ? `${p}/${segment}` : segment)); + continue; + } + + const nextPrefixes: string[] = []; + for (const prefix of prefixes) { + const subdirs = new Set(); + for (const base of basePaths) { + const dirUri = prefix + ? Utils.joinPath(rootUri, base, prefix).toString() + : Utils.joinPath(rootUri, base).toString(); + for (const name of await this.listSubdirectoryNames(dirUri)) { + subdirs.add(name); + } + } + for (const sub of subdirs) { + nextPrefixes.push(prefix ? `${prefix}/${sub}` : sub); + if (nextPrefixes.length >= 100) break; + } + if (nextPrefixes.length >= 100) break; + } + prefixes = nextPrefixes; + } + + return prefixes; + } + + /** + * Resolve a search path (static, dynamic, or empty) into concrete prefix + * strings. Cached for dynamic paths. + */ + private async resolveSearchPath(rootUri: URI, searchPath: string): Promise { + if (searchPath === '') return ['']; + if (!DocumentsLocator.LIQUID_EXPRESSION_RE.test(searchPath)) return [searchPath]; + + const cacheKey = `${rootUri.toString()}:${searchPath}`; + if (!this.expandedPathsCache.has(cacheKey)) { + this.expandedPathsCache.set(cacheKey, this.expandDynamicPath(rootUri, searchPath)); + } + return this.expandedPathsCache.get(cacheKey)!; + } + + /** + * Locate a partial using theme search paths (for theme_render_rc). + * + * Tries each search path prefix in priority order, then falls back to the + * unprefixed name (unless '' was already in the list, meaning the default + * position was explicitly placed). + */ + async locateWithSearchPaths( + rootUri: URI, + fileName: string, + themeSearchPaths: string[], + ): Promise { + for (const searchPath of themeSearchPaths) { + for (const prefix of await this.resolveSearchPath(rootUri, searchPath)) { + const candidate = prefix ? `${prefix}/${fileName}` : fileName; + const result = await this.locateFile(rootUri, candidate, 'partial'); + if (result) return result; + } + } + + if (!themeSearchPaths.includes('')) { + return this.locateFile(rootUri, fileName, 'partial'); + } + + return undefined; + } + async locate( rootUri: URI, nodeName: DocumentType, fileName: string, + themeSearchPaths?: string[] | null, ): Promise { switch (nodeName) { case 'render': @@ -138,6 +275,11 @@ export class DocumentsLocator { case 'function': return this.locateFile(rootUri, fileName, 'partial'); + case 'theme_render_rc': + return themeSearchPaths + ? this.locateWithSearchPaths(rootUri, fileName, themeSearchPaths) + : this.locateFile(rootUri, fileName, 'partial'); + case 'graphql': return this.locateFile(rootUri, fileName, 'graphql'); @@ -154,6 +296,7 @@ export class DocumentsLocator { case 'function': case 'render': case 'include': + case 'theme_render_rc': return this.listFiles(rootUri, filePrefix, 'partial'); case 'graphql': diff --git a/packages/platformos-common/src/route-table/RouteTable.ts b/packages/platformos-common/src/route-table/RouteTable.ts index c1412f2..84e1176 100644 --- a/packages/platformos-common/src/route-table/RouteTable.ts +++ b/packages/platformos-common/src/route-table/RouteTable.ts @@ -50,8 +50,10 @@ function extractFrontmatter(source: string): PageFrontmatter | null { */ function extractRelativePagePath(uri: string): string | null { const patterns = [ + // App-level pages: app/views/pages/ or marketplace_builder/pages/ /\/(app|marketplace_builder)\/(views\/pages|pages)\//, - /\/(public|private)\/(views\/pages|pages)\//, + // Module pages: must be under modules//(public|private)/(views/pages|pages)/ + /\/modules\/[^/]+\/(public|private)\/(views\/pages|pages)\//, ]; for (const pattern of patterns) { @@ -361,15 +363,17 @@ export class RouteTable { await this.walkDirectory(baseUri.toString(), uris); } - // Module pages — discover module names first + // Module pages — discover module names first, then walk all in parallel const moduleNames = await this.discoverModuleNames(rootUri); + const moduleWalks: Promise[] = []; for (const moduleName of moduleNames) { const modulePaths = getModulePaths(PlatformOSFileType.Page, moduleName); for (const basePath of modulePaths) { const baseUri = Utils.joinPath(rootUri, basePath); - await this.walkDirectory(baseUri.toString(), uris); + moduleWalks.push(this.walkDirectory(baseUri.toString(), uris)); } } + await Promise.all(moduleWalks); return uris; } diff --git a/packages/platformos-language-server-common/src/TypeSystem.ts b/packages/platformos-language-server-common/src/TypeSystem.ts index 58df72a..64f2743 100644 --- a/packages/platformos-language-server-common/src/TypeSystem.ts +++ b/packages/platformos-language-server-common/src/TypeSystem.ts @@ -1767,12 +1767,13 @@ function resolveExpressionShape( if (pseudoType !== undefined && objectMap) { if (lookup.type !== NodeTypes.String) return undefined; const propType = inferPseudoTypePropertyType(pseudoType, lookup, objectMap); + // inferPseudoTypePropertyType returns PseudoType | ArrayType (string or { kind: 'array' }) if (typeof propType === 'string') { pseudoType = propType; shape = resolvedTypeToShape(propType); - } else if (isShapeType(propType as PseudoType | ArrayType | ShapeType | UnionType)) { + } else if (isArrayType(propType)) { pseudoType = undefined; - shape = (propType as unknown as ShapeType).shape; + shape = resolvedTypeToShape(propType); } else { return undefined; } diff --git a/packages/platformos-language-server-common/src/definitions/DefinitionProvider.ts b/packages/platformos-language-server-common/src/definitions/DefinitionProvider.ts index bd00008..a24b118 100644 --- a/packages/platformos-language-server-common/src/definitions/DefinitionProvider.ts +++ b/packages/platformos-language-server-common/src/definitions/DefinitionProvider.ts @@ -1,10 +1,12 @@ import { findCurrentNode, SourceCodeType } from '@platformos/platformos-check-common'; -import { AbstractFileSystem } from '@platformos/platformos-common'; +import { AbstractFileSystem, DocumentsLocator, RouteTable } from '@platformos/platformos-common'; import { DefinitionLink, DefinitionParams } from 'vscode-languageserver'; import { AugmentedJsonSourceCode, DocumentManager } from '../documents'; +import { SearchPathsLoader } from '../utils/searchPaths'; import { BaseDefinitionProvider } from './BaseDefinitionProvider'; import { PageRouteDefinitionProvider } from './providers/PageRouteDefinitionProvider'; +import { RenderPartialDefinitionProvider } from './providers/RenderPartialDefinitionProvider'; import { TranslationStringDefinitionProvider } from './providers/TranslationStringDefinitionProvider'; export class DefinitionProvider { @@ -16,6 +18,8 @@ export class DefinitionProvider { getDefaultLocaleSourceCode: (uri: string) => Promise, fs?: AbstractFileSystem, findAppRootURI?: (uri: string) => Promise, + documentsLocator?: DocumentsLocator, + searchPathsCache?: SearchPathsLoader, ) { this.providers = [ new TranslationStringDefinitionProvider(documentManager, getDefaultLocaleSourceCode), @@ -24,6 +28,17 @@ export class DefinitionProvider { if (fs && findAppRootURI) { this.pageRouteProvider = new PageRouteDefinitionProvider(documentManager, fs, findAppRootURI); this.providers.push(this.pageRouteProvider); + + if (documentsLocator && searchPathsCache) { + this.providers.push( + new RenderPartialDefinitionProvider( + documentManager, + documentsLocator, + searchPathsCache, + findAppRootURI, + ), + ); + } } } @@ -45,8 +60,12 @@ export class DefinitionProvider { this.pageRouteProvider?.invalidate(); } - /** Returns the shared RouteTable, or undefined if route support is not configured. */ - getRouteTable(): import('@platformos/platformos-common').RouteTable | undefined { + /** + * Returns the shared RouteTable, or undefined if route support is not configured. + * When undefined (no fs/findAppRootURI), the check pipeline will build a fresh + * RouteTable per run via makeGetRouteTable in context-utils.ts. + */ + getRouteTable(): RouteTable | undefined { return this.pageRouteProvider?.getRouteTable(); } diff --git a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts new file mode 100644 index 0000000..18f0e33 --- /dev/null +++ b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts @@ -0,0 +1,140 @@ +import { describe, it, expect } from 'vitest'; +import { toLiquidHtmlAST } from '@platformos/liquid-html-parser'; +import { findCurrentNode } from '@platformos/platformos-check-common'; +import { MockFileSystem } from '@platformos/platformos-check-common/src/test'; +import { DocumentsLocator } from '@platformos/platformos-common'; +import { DefinitionParams, Position } from 'vscode-languageserver-protocol'; +import { DocumentManager } from '../../documents'; +import { SearchPathsLoader } from '../../utils/searchPaths'; +import { RenderPartialDefinitionProvider } from './RenderPartialDefinitionProvider'; + +const rootUri = 'file:///project'; +const uriString = 'file:///project/app/views/pages/index.liquid'; + +function setup(files: Record) { + const documentManager = new DocumentManager(); + const mockFs = new MockFileSystem(files); + const provider = new RenderPartialDefinitionProvider( + documentManager, + new DocumentsLocator(mockFs), + new SearchPathsLoader(mockFs), + async () => rootUri, + ); + return { documentManager, provider }; +} + +async function getDefinitions(source: string, cursorOffset: number, files: Record) { + const { documentManager, provider } = setup(files); + documentManager.open(uriString, source, 1); + + const ast = toLiquidHtmlAST(source); + const [node, ancestors] = findCurrentNode(ast, cursorOffset); + const params: DefinitionParams = { + textDocument: { uri: uriString }, + position: Position.create(0, cursorOffset), + }; + + return provider.definitions(params, node, ancestors); +} + +describe('RenderPartialDefinitionProvider', () => { + describe('render tag', () => { + it('should resolve render partials', async () => { + const result = await getDefinitions("{% render 'card' %}", 12, { + 'project/app/views/partials/card.liquid': 'card content', + }); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should NOT use search paths for regular render tags', async () => { + const result = await getDefinitions("{% render 'card' %}", 12, { + 'project/app/config.yml': 'theme_search_paths:\n - theme/dress', + 'project/app/views/partials/theme/dress/card.liquid': 'dress card', + }); + + expect(result).toHaveLength(0); + }); + }); + + describe('theme_render_rc tag', () => { + it('should resolve via theme_search_paths', async () => { + const result = await getDefinitions("{% theme_render_rc 'card' %}", 20, { + 'project/app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'project/app/views/partials/theme/dress/card.liquid': 'dress card', + }); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe( + 'file:///project/app/views/partials/theme/dress/card.liquid', + ); + }); + + it('should fallback to standard resolution when no config exists', async () => { + const result = await getDefinitions("{% theme_render_rc 'card' %}", 20, { + 'project/app/views/partials/card.liquid': 'default card', + }); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should resolve inside {% liquid %} blocks', async () => { + const source = `{% liquid + theme_render_rc 'components/atoms/heading', content: 'text' +%}`; + const offset = source.indexOf('components/atoms/heading'); + const { documentManager, provider } = setup({ + 'project/app/config.yml': "theme_search_paths:\n - ''\n - modules/components", + 'project/modules/components/public/views/partials/components/atoms/heading.liquid': + 'heading', + }); + documentManager.open(uriString, source, 1); + + const ast = toLiquidHtmlAST(source); + const [node, ancestors] = findCurrentNode(ast, offset); + const params: DefinitionParams = { + textDocument: { uri: uriString }, + position: Position.create(1, 20), + }; + + const result = await provider.definitions(params, node, ancestors); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe( + 'file:///project/modules/components/public/views/partials/components/atoms/heading.liquid', + ); + }); + }); + + describe('function tag', () => { + it('should resolve function partials', async () => { + const result = await getDefinitions("{% function result = 'commands/apply' %}", 24, { + 'project/app/lib/commands/apply.liquid': 'apply content', + }); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/lib/commands/apply.liquid'); + }); + }); + + describe('graphql tag', () => { + it('should resolve graphql references', async () => { + const result = await getDefinitions("{% graphql g = 'users/search' %}", 18, { + 'project/app/graphql/users/search.graphql': 'query { }', + }); + + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/graphql/users/search.graphql'); + }); + }); + + describe('non-matching nodes', () => { + it('should return empty for non-string nodes', async () => { + const result = await getDefinitions("{% assign x = 'hello' %}", 3, {}); + + expect(result).toHaveLength(0); + }); + }); +}); diff --git a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts new file mode 100644 index 0000000..df05c75 --- /dev/null +++ b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts @@ -0,0 +1,70 @@ +import { LiquidHtmlNode, LiquidString, LiquidTag, NodeTypes } from '@platformos/liquid-html-parser'; +import { DocumentsLocator, DocumentType } from '@platformos/platformos-common'; +import { + DefinitionParams, + DefinitionLink, + Range, + LocationLink, +} from 'vscode-languageserver-protocol'; +import { URI } from 'vscode-uri'; +import { DocumentManager } from '../../documents'; +import { BaseDefinitionProvider } from '../BaseDefinitionProvider'; +import { SearchPathsLoader } from '../../utils/searchPaths'; + +const TAG_MARKUP_TYPE: Record = { + render: NodeTypes.RenderMarkup, + include: NodeTypes.RenderMarkup, + theme_render_rc: NodeTypes.RenderMarkup, + function: NodeTypes.FunctionMarkup, + graphql: NodeTypes.GraphQLMarkup, +}; + +export class RenderPartialDefinitionProvider implements BaseDefinitionProvider { + constructor( + private documentManager: DocumentManager, + private documentsLocator: DocumentsLocator, + private searchPathsCache: SearchPathsLoader, + private findAppRootURI: (uri: string) => Promise, + ) {} + + async definitions( + params: DefinitionParams, + node: LiquidHtmlNode, + ancestors: LiquidHtmlNode[], + ): Promise { + if (node.type !== NodeTypes.String) return []; + + const markup = ancestors.at(-1); + const tag = ancestors.at(-2); + if (!markup || !tag || tag.type !== NodeTypes.LiquidTag) return []; + + const expectedMarkupType = TAG_MARKUP_TYPE[(tag as LiquidTag).name]; + if (expectedMarkupType === undefined || markup.type !== expectedMarkupType) return []; + + const rootUri = await this.findAppRootURI(params.textDocument.uri); + if (!rootUri) return []; + + const root = URI.parse(rootUri); + const searchPaths = await this.searchPathsCache.get(root); + const docType = (tag as LiquidTag).name as DocumentType; + const fileUri = await this.documentsLocator.locate( + root, + docType, + (node as LiquidString).value, + searchPaths, + ); + if (!fileUri) return []; + + const sourceCode = this.documentManager.get(params.textDocument.uri); + if (!sourceCode) return []; + + const doc = sourceCode.textDocument; + const originRange = Range.create( + doc.positionAt(node.position.start), + doc.positionAt(node.position.end), + ); + const targetRange = Range.create(0, 0, 0, 0); + + return [LocationLink.create(fileUri, targetRange, targetRange, originRange)]; + } +} diff --git a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts index 5d8be5b..597b6dc 100644 --- a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts +++ b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts @@ -3,28 +3,35 @@ import { DocumentManager } from '../documents'; import { DocumentLinksProvider } from './DocumentLinksProvider'; import { DocumentsLocator, TranslationProvider } from '@platformos/platformos-common'; import { MockFileSystem } from '@platformos/platformos-check-common/src/test'; +import { SearchPathsLoader } from '../utils/searchPaths'; + +function makeProvider( + documentManager: DocumentManager, + rootUri: string, + mockFs: MockFileSystem, +): DocumentLinksProvider { + return new DocumentLinksProvider( + documentManager, + async () => rootUri, + new DocumentsLocator(mockFs), + new TranslationProvider(mockFs), + new SearchPathsLoader(mockFs), + ); +} describe('DocumentLinksProvider', () => { let documentManager: DocumentManager; let documentLinksProvider: DocumentLinksProvider; - let documentsLocator: DocumentsLocator; - let fs: MockFileSystem; let rootUri: string; let uriString: string; beforeEach(() => { documentManager = new DocumentManager(); - fs = new MockFileSystem({ + const fs = new MockFileSystem({ 'path/to/project/app/lib/commands/apply.liquid': 'apply content', 'path/to/project/app/views/apply_view.liquid': 'apply view content', }); - documentsLocator = new DocumentsLocator(fs); - documentLinksProvider = new DocumentLinksProvider( - documentManager, - async () => rootUri, - documentsLocator, - new TranslationProvider(fs), - ); + documentLinksProvider = makeProvider(documentManager, 'file:///path/to/project', fs); }); it('should return an empty array for non-LiquidHtml documents', async () => { @@ -63,4 +70,118 @@ describe('DocumentLinksProvider', () => { expect(result[i].target).toBe(expectedUrls[i]); } }); + + describe('theme_render_rc', () => { + it('should resolve theme_render_rc partials via theme_search_paths', async () => { + rootUri = 'file:///project'; + uriString = 'file:///project/app/views/pages/index.liquid'; + + const mockFs = new MockFileSystem({ + 'project/app/config.yml': 'theme_search_paths:\n - theme/dress\n - theme/simple', + 'project/app/views/partials/theme/dress/card.liquid': 'dress card', + 'project/app/views/partials/theme/simple/footer.liquid': 'simple footer', + }); + + const provider = makeProvider(documentManager, rootUri, mockFs); + + documentManager.open( + uriString, + "{% theme_render_rc 'card' %} {% theme_render_rc 'footer' %}", + 1, + ); + + const result = await provider.documentLinks(uriString); + + expect(result).toHaveLength(2); + expect(result[0].target).toBe('file:///project/app/views/partials/theme/dress/card.liquid'); + expect(result[1].target).toBe( + 'file:///project/app/views/partials/theme/simple/footer.liquid', + ); + }); + + it('should fallback to standard resolution when no config exists', async () => { + rootUri = 'file:///project'; + uriString = 'file:///project/app/views/pages/index.liquid'; + + const mockFs = new MockFileSystem({ + 'project/app/views/partials/card.liquid': 'default card', + }); + + const provider = makeProvider(documentManager, rootUri, mockFs); + + documentManager.open(uriString, "{% theme_render_rc 'card' %}", 1); + + const result = await provider.documentLinks(uriString); + + expect(result).toHaveLength(1); + expect(result[0].target).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should resolve theme_render_rc with Liquid wildcard paths', async () => { + rootUri = 'file:///project'; + uriString = 'file:///project/app/views/pages/index.liquid'; + + const mockFs = new MockFileSystem({ + 'project/app/config.yml': 'theme_search_paths:\n - theme/{{ context.constants.THEME }}', + 'project/app/views/partials/theme/custom/hero.liquid': 'custom hero', + }); + + const provider = makeProvider(documentManager, rootUri, mockFs); + + documentManager.open(uriString, "{% theme_render_rc 'hero' %}", 1); + + const result = await provider.documentLinks(uriString); + + expect(result).toHaveLength(1); + expect(result[0].target).toBe('file:///project/app/views/partials/theme/custom/hero.liquid'); + }); + + it('should not use search paths for regular render tags', async () => { + rootUri = 'file:///project'; + uriString = 'file:///project/app/views/pages/index.liquid'; + + const mockFs = new MockFileSystem({ + 'project/app/config.yml': 'theme_search_paths:\n - theme/dress', + 'project/app/views/partials/card.liquid': 'default card', + }); + + const provider = makeProvider(documentManager, rootUri, mockFs); + + documentManager.open(uriString, "{% render 'card' %}", 1); + + const result = await provider.documentLinks(uriString); + + expect(result).toHaveLength(1); + expect(result[0].target).toBe('file:///project/app/views/partials/card.liquid'); + }); + + it('should pick up new config after cache invalidation', async () => { + rootUri = 'file:///project'; + uriString = 'file:///project/app/views/pages/index.liquid'; + + const initialFiles: Record = { + 'project/app/views/partials/card.liquid': 'default card', + 'project/app/views/partials/theme/new/card.liquid': 'new card', + }; + const mockFs = new MockFileSystem(initialFiles); + const provider = makeProvider(documentManager, rootUri, mockFs); + + documentManager.open(uriString, "{% theme_render_rc 'card' %}", 1); + + const result1 = await provider.documentLinks(uriString); + expect(result1).toHaveLength(1); + expect(result1[0].target).toBe('file:///project/app/views/partials/card.liquid'); + + // Simulate config.yml being created with search paths — new provider with new fs + const updatedFs = new MockFileSystem({ + ...initialFiles, + 'project/app/config.yml': 'theme_search_paths:\n - theme/new', + }); + const provider2 = makeProvider(documentManager, rootUri, updatedFs); + + const result2 = await provider2.documentLinks(uriString); + expect(result2).toHaveLength(1); + expect(result2[0].target).toBe('file:///project/app/views/partials/theme/new/card.liquid'); + }); + }); }); diff --git a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts index 292cda2..0f4348e 100644 --- a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts +++ b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts @@ -1,13 +1,14 @@ -import { LiquidHtmlNode, LiquidString, NamedTags, NodeTypes } from '@platformos/liquid-html-parser'; +import { LiquidHtmlNode, LiquidString, NodeTypes } from '@platformos/liquid-html-parser'; import { SourceCodeType } from '@platformos/platformos-check-common'; import { DocumentLink, Range } from 'vscode-languageserver'; import { TextDocument } from 'vscode-languageserver-textdocument'; -import { URI, Utils } from 'vscode-uri'; +import { URI } from 'vscode-uri'; import { visit, Visitor } from '@platformos/platformos-check-common'; import { DocumentManager } from '../documents'; import { FindAppRootURI } from '../internal-types'; -import { DocumentsLocator, TranslationProvider } from '@platformos/platformos-common'; +import { DocumentsLocator, DocumentType, TranslationProvider } from '@platformos/platformos-common'; +import { SearchPathsLoader } from '../utils/searchPaths'; export class DocumentLinksProvider { constructor( @@ -15,6 +16,7 @@ export class DocumentLinksProvider { private findAppRootURI: FindAppRootURI, private documentsLocator: DocumentsLocator, private translationProvider: TranslationProvider, + private searchPathsCache: SearchPathsLoader, ) {} async documentLinks(uriString: string): Promise { @@ -32,11 +34,15 @@ export class DocumentLinksProvider { return []; } + const root = URI.parse(rootUri); + const searchPaths = await this.searchPathsCache.get(root); + const visitor = documentLinksVisitor( sourceCode.textDocument, - URI.parse(rootUri), + root, this.documentsLocator, this.translationProvider, + searchPaths, ); return visit(sourceCode.ast, visitor); } @@ -47,43 +53,28 @@ function documentLinksVisitor( root: URI, documentsLocator: DocumentsLocator, translationProvider: TranslationProvider, + searchPaths: string[] | null, ): Visitor { return { async LiquidTag(node) { - if ( - (node.name === 'render' || node.name === 'include') && - typeof node.markup !== 'string' && - isLiquidString(node.markup.partial) - ) { - const partial = node.markup.partial; - return DocumentLink.create( - range(textDocument, partial), - await documentsLocator.locate(root, node.name, partial.value), - ); - } + const markup = node.markup; + if (typeof markup === 'string' || markup === null) return; + + const name = node.name as DocumentType; - if ( - node.name === 'function' && - typeof node.markup !== 'string' && - isLiquidString(node.markup.partial) - ) { - const partial = node.markup.partial; + // render, include, function, theme_render_rc all have a .partial field + if ('partial' in markup && isLiquidString(markup.partial)) { return DocumentLink.create( - range(textDocument, partial), - await documentsLocator.locate(root, node.name, partial.value), + range(textDocument, markup.partial), + await documentsLocator.locate(root, name, markup.partial.value, searchPaths), ); } - if ( - node.name === 'graphql' && - typeof node.markup !== 'string' && - 'graphql' in node.markup && - isLiquidString(node.markup.graphql) - ) { - const snippet = node.markup.graphql; + // graphql has a .graphql field + if ('graphql' in markup && isLiquidString(markup.graphql)) { return DocumentLink.create( - range(textDocument, snippet), - await documentsLocator.locate(root, node.name, snippet.value), + range(textDocument, markup.graphql), + await documentsLocator.locate(root, name, markup.graphql.value), ); } }, diff --git a/packages/platformos-language-server-common/src/server/CachedFileSystem.ts b/packages/platformos-language-server-common/src/server/CachedFileSystem.ts index 72f94d3..980113e 100644 --- a/packages/platformos-language-server-common/src/server/CachedFileSystem.ts +++ b/packages/platformos-language-server-common/src/server/CachedFileSystem.ts @@ -6,7 +6,12 @@ export class CachedFileSystem implements AbstractFileSystem { stat: Cached; constructor(fs: AbstractFileSystem) { - this.readFile = cachedByUri(fs.readFile.bind(fs)); + this.readFile = cachedByUri( + fs.readFile.bind(fs), + // app/config.yml can change externally (e.g. vim) without a file watcher + // notification. Always read it fresh — it's small and rarely accessed. + (uri) => uri.endsWith('/app/config.yml'), + ); this.readDirectory = cachedByUri(fs.readDirectory.bind(fs)); this.stat = cachedByUri(fs.stat.bind(fs)); } @@ -17,10 +22,14 @@ interface Cached Promise, T = ReturnType> { invalidate(uri: string): void; } -function cachedByUri(fn: (uri: string) => Promise): Cached { +function cachedByUri( + fn: (uri: string) => Promise, + skipCache?: (uri: string) => boolean, +): Cached { const cache = new Map>(); function cached(uri: string) { + if (skipCache?.(uri)) return fn(uri); if (!cache.has(uri)) { // I'm intentionally leaving this comment here for debugging purposes :) // console.error('cache miss', fn.name, uri); diff --git a/packages/platformos-language-server-common/src/server/startServer.ts b/packages/platformos-language-server-common/src/server/startServer.ts index fc5b535..4ceb2df 100644 --- a/packages/platformos-language-server-common/src/server/startServer.ts +++ b/packages/platformos-language-server-common/src/server/startServer.ts @@ -44,6 +44,7 @@ import { AppGraphRootRequest, } from '../types'; import { debounce } from '../utils'; +import { SearchPathsLoader } from '../utils/searchPaths'; import { VERSION } from '../version'; import { CachedFileSystem } from './CachedFileSystem'; import { Configuration, INCLUDE_FILES_FROM_DISK } from './Configuration'; @@ -108,11 +109,13 @@ export function startServer( const diagnosticsManager = new DiagnosticsManager(connection); const documentsLocator = new DocumentsLocator(fs); const translationProvider = new TranslationProvider(fs); + const searchPathsCache = new SearchPathsLoader(fs); const documentLinksProvider = new DocumentLinksProvider( documentManager, findAppRootURI, documentsLocator, translationProvider, + searchPathsCache, ); const codeActionsProvider = new CodeActionsProvider(documentManager, diagnosticsManager); const onTypeFormattingProvider = new OnTypeFormattingProvider( @@ -221,6 +224,8 @@ export function startServer( getDefaultLocaleSourceCode, fs, findAppRootURI, + documentsLocator, + searchPathsCache, ); const jsonLanguageService = new JSONLanguageService(documentManager, jsonValidationSet); const cssLanguageService = new CSSLanguageService(documentManager); @@ -348,6 +353,9 @@ export function startServer( { globPattern: '**/*.css', }, + { + globPattern: '**/app/config.yml', + }, ], }); }); @@ -550,7 +558,7 @@ export function startServer( // individual onDidChangeTextDocument events. VS Code reports branch-switch changes // as FileChangeType.Changed, so we count all change types. const bulkPageChanges = params.changes.filter((c) => isPage(c.uri)); - if (bulkPageChanges.length >= 3) { + if (bulkPageChanges.length >= 10) { definitionsProvider.invalidateRouteTable(); } @@ -562,6 +570,20 @@ export function startServer( continue; } + // app/config.yml changes should clear expanded search paths cache + // and force re-check of all open documents. + // Note: readFile for config.yml bypasses the CachedFileSystem cache, + // so we don't need to invalidate it — reads are always fresh. + if (change.uri.endsWith('/app/config.yml')) { + documentsLocator.clearExpandedPathsCache(); + + // Ensure open liquid files are re-checked with the new search paths + for (const doc of documentManager.openDocuments) { + triggerUris.push(doc.uri); + } + continue; + } + // Rename cache invalidation is handled by onDidRenameFiles if (documentManager.hasRecentRename(change.uri)) { documentManager.clearRecentRename(change.uri); diff --git a/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts b/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts new file mode 100644 index 0000000..6dfb837 --- /dev/null +++ b/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi } from 'vitest'; +import { URI } from 'vscode-uri'; +import { SearchPathsLoader } from './searchPaths'; + +const rootUri = URI.parse('file:///project'); +const configUri = 'file:///project/app/config.yml'; + +function createMockFs(configContent: string | null) { + return { + readFile: vi.fn(async (uri: string) => { + if (uri === configUri && configContent !== null) return configContent; + throw new Error(`File not found: ${uri}`); + }), + readDirectory: vi.fn(async () => []), + stat: vi.fn(async () => ({ type: 1, size: 0 })), + }; +} + +describe('SearchPathsLoader', () => { + it('should always read fresh (no internal caching)', async () => { + let configContent = 'theme_search_paths:\n - theme/dress'; + const fs = { + readFile: vi.fn(async (uri: string) => { + if (uri === configUri) return configContent; + throw new Error(`File not found: ${uri}`); + }), + readDirectory: vi.fn(async () => []), + stat: vi.fn(async () => ({ type: 1, size: 0 })), + }; + const cache = new SearchPathsLoader(fs); + + const result1 = await cache.get(rootUri); + expect(result1).toEqual(['theme/dress']); + + // Config changes — no invalidation needed, reads are always fresh + configContent = 'theme_search_paths:\n - theme/simple'; + + const result2 = await cache.get(rootUri); + expect(result2).toEqual(['theme/simple']); + expect(fs.readFile).toHaveBeenCalledTimes(2); + }); + + it('should return null when config has no search paths', async () => { + const fs = createMockFs('some_other_key: value'); + const cache = new SearchPathsLoader(fs); + + const result = await cache.get(rootUri); + expect(result).toBeNull(); + }); + + it('should return null when config.yml does not exist', async () => { + const fs = createMockFs(null); + const cache = new SearchPathsLoader(fs); + + const result = await cache.get(rootUri); + expect(result).toBeNull(); + }); + +}); diff --git a/packages/platformos-language-server-common/src/utils/searchPaths.ts b/packages/platformos-language-server-common/src/utils/searchPaths.ts new file mode 100644 index 0000000..71dc00b --- /dev/null +++ b/packages/platformos-language-server-common/src/utils/searchPaths.ts @@ -0,0 +1,16 @@ +import { AbstractFileSystem, loadSearchPaths } from '@platformos/platformos-common'; +import { URI } from 'vscode-uri'; + +/** + * Loader for theme_search_paths from app/config.yml. + * Always reads fresh — config.yml bypasses the CachedFileSystem cache + * so that external edits (e.g. vim, CLI) are picked up without relying + * on file watcher notifications. + */ +export class SearchPathsLoader { + constructor(private fs: AbstractFileSystem) {} + + get(rootUri: URI): Promise { + return loadSearchPaths(this.fs, rootUri); + } +} diff --git a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts index 7f15956..d0da267 100644 --- a/packages/prettier-plugin-liquid/src/printer/print/liquid.ts +++ b/packages/prettier-plugin-liquid/src/printer/print/liquid.ts @@ -188,7 +188,8 @@ function printNamedLiquidBlockStart( } case NamedTags.include: - case NamedTags.render: { + case NamedTags.render: + case NamedTags.theme_render_rc: { const markup = node.markup; const trailingWhitespace = markup.args.length > 0 || (markup.variable && markup.alias) ? line : ' '; @@ -287,8 +288,7 @@ function printNamedLiquidBlockStart( return tag(trailingWhitespace); } case NamedTags.include_form: - case NamedTags.spam_protection: - case NamedTags.theme_render_rc: { + case NamedTags.spam_protection: { const trailingWhitespace = node.markup.args.length > 0 ? line : ' '; return tag(trailingWhitespace); } From 97f8f5dd2a29d3403b118dbad44735858fdb72d4 Mon Sep 17 00:00:00 2001 From: Maciej Krajowski-Kukiel Date: Thu, 19 Mar 2026 09:57:49 +0100 Subject: [PATCH 2/2] refactor --- .../src/checks/missing-page/index.ts | 20 +-- .../src/checks/missing-partial/index.spec.ts | 3 +- .../src/checks/missing-partial/index.ts | 6 +- .../src/context-utils.ts | 8 +- .../src/url-helpers.ts | 36 +++-- .../src/route-table/RouteTable.spec.ts | 128 ++++++++++++++++++ .../src/route-table/RouteTable.ts | 9 ++ .../PageRouteDefinitionProvider.spec.ts | 23 ++++ .../RenderPartialDefinitionProvider.spec.ts | 53 +++++++- .../src/server/startServer.ts | 19 ++- .../src/utils/searchPaths.spec.ts | 62 +++++++-- .../src/utils/searchPaths.ts | 26 +++- 12 files changed, 346 insertions(+), 47 deletions(-) diff --git a/packages/platformos-check-common/src/checks/missing-page/index.ts b/packages/platformos-check-common/src/checks/missing-page/index.ts index 5454aab..d54538d 100644 --- a/packages/platformos-check-common/src/checks/missing-page/index.ts +++ b/packages/platformos-check-common/src/checks/missing-page/index.ts @@ -1,10 +1,4 @@ -import { - HtmlElement, - LiquidTag, - NamedTags, - LiquidTagAssign, - AssignMarkup, -} from '@platformos/liquid-html-parser'; +import { HtmlElement, LiquidTag } from '@platformos/liquid-html-parser'; import { RouteTable } from '@platformos/platformos-common'; import { shouldSkipUrl, @@ -12,7 +6,7 @@ import { getAttrName, extractUrlPattern, getEffectiveMethod, - resolveAssignToUrlPattern, + tryExtractAssignUrl, } from '../../url-helpers'; import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; import { isHtmlTag } from '../utils'; @@ -62,13 +56,9 @@ export const MissingPage: LiquidCheckDefinition = { }, async LiquidTag(node: LiquidTag) { - if (node.name !== NamedTags.assign) return; - const markup = (node as LiquidTagAssign).markup as AssignMarkup; - if (markup.lookups.length > 0) return; - - const urlPattern = resolveAssignToUrlPattern(markup); - if (urlPattern !== null) { - variableMap.set(markup.name, urlPattern); + const extracted = tryExtractAssignUrl(node); + if (extracted) { + variableMap.set(extracted.name, extracted.urlPattern); } }, diff --git a/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts b/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts index 329b0c2..c70ef7f 100644 --- a/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts +++ b/packages/platformos-check-common/src/checks/missing-partial/index.spec.ts @@ -225,7 +225,8 @@ describe('Module: MissingPartial', () => { // E.g. "{{ context.constants.BRAND }}/{{ context.constants.TIER }}" - both segments are wildcards const offenses = await check( { - 'app/config.yml': 'theme_search_paths:\n - "{{ context.constants.BRAND }}/{{ context.constants.TIER }}"', + 'app/config.yml': + 'theme_search_paths:\n - "{{ context.constants.BRAND }}/{{ context.constants.TIER }}"', 'app/views/partials/page.liquid': "{% theme_render_rc 'card' %}", 'app/views/partials/acme/premium/card.liquid': 'acme premium card', }, diff --git a/packages/platformos-check-common/src/checks/missing-partial/index.ts b/packages/platformos-check-common/src/checks/missing-partial/index.ts index 10cb4b7..9de45bf 100644 --- a/packages/platformos-check-common/src/checks/missing-partial/index.ts +++ b/packages/platformos-check-common/src/checks/missing-partial/index.ts @@ -1,10 +1,6 @@ import { LiquidHtmlNode, NamedTags, NodeTypes } from '@platformos/liquid-html-parser'; import { LiquidCheckDefinition, SchemaProp, Severity, SourceCodeType } from '../../types'; -import { - DocumentsLocator, - DocumentType, - loadSearchPaths, -} from '@platformos/platformos-common'; +import { DocumentsLocator, DocumentType, loadSearchPaths } from '@platformos/platformos-common'; import { URI } from 'vscode-uri'; function getTagName(ancestors: LiquidHtmlNode[]): DocumentType { diff --git a/packages/platformos-check-common/src/context-utils.ts b/packages/platformos-check-common/src/context-utils.ts index ea9b314..d3b2413 100644 --- a/packages/platformos-check-common/src/context-utils.ts +++ b/packages/platformos-check-common/src/context-utils.ts @@ -144,7 +144,13 @@ export function makeGetRouteTable( if (table.isBuilt()) { buildPromise = Promise.resolve(table); } else { - buildPromise = table.build(URI.parse(rootUri)).then(() => table); + buildPromise = table + .build(URI.parse(rootUri)) + .then(() => table) + .catch((err) => { + buildPromise = null; + throw err; + }); } } return buildPromise; diff --git a/packages/platformos-check-common/src/url-helpers.ts b/packages/platformos-check-common/src/url-helpers.ts index 68abb9b..d191bd3 100644 --- a/packages/platformos-check-common/src/url-helpers.ts +++ b/packages/platformos-check-common/src/url-helpers.ts @@ -297,6 +297,26 @@ export function getEffectiveMethod(formNode: HtmlElement): string | null { return formMethod; } +/** + * If the given node is a `{% assign %}` tag whose RHS resolves to a URL pattern, + * returns `{ name, urlPattern }`. Otherwise returns null. + * + * Shared by `buildVariableMap` (full AST walk) and `MissingPage` (incremental + * visitor), so the assign-to-URL resolution logic lives in one place. + */ +export function tryExtractAssignUrl( + node: LiquidHtmlNode, +): { name: string; urlPattern: string } | null { + if (node.type !== NodeTypes.LiquidTag || (node as LiquidTag).name !== NamedTags.assign) { + return null; + } + const markup = (node as LiquidTagAssign).markup as AssignMarkup; + if (markup.lookups.length > 0) return null; + const urlPattern = resolveAssignToUrlPattern(markup); + if (urlPattern === null) return null; + return { name: markup.name, urlPattern }; +} + /** * Walk an AST subtree and collect {% assign %} variable mappings that resolve to URL patterns. * Not scope-aware: assigns inside {% if %} / {% for %} blocks are tracked even though they @@ -317,14 +337,14 @@ export function buildVariableMap( function walk(nodes: LiquidHtmlNode[]): void { for (const node of nodes) { - if (node.type === NodeTypes.LiquidTag && (node as LiquidTag).name === NamedTags.assign) { - if (beforeOffset !== undefined && node.position.end > beforeOffset) continue; - const markup = (node as LiquidTagAssign).markup as AssignMarkup; - if (markup.lookups.length === 0) { - const urlPattern = resolveAssignToUrlPattern(markup); - if (urlPattern !== null) { - variableMap.set(markup.name, urlPattern); - } + // Apply the beforeOffset cutoff only to assign nodes themselves, not to block + // containers. A block tag ({% if %}, {% for %}, HTML element, etc.) may start + // before the cursor but end after it — we must still recurse into its children + // to find any assigns that precede the cursor within that block. + const extracted = tryExtractAssignUrl(node); + if (extracted) { + if (beforeOffset === undefined || node.position.end <= beforeOffset) { + variableMap.set(extracted.name, extracted.urlPattern); } } diff --git a/packages/platformos-common/src/route-table/RouteTable.spec.ts b/packages/platformos-common/src/route-table/RouteTable.spec.ts index 6cb71b8..8dda50e 100644 --- a/packages/platformos-common/src/route-table/RouteTable.spec.ts +++ b/packages/platformos-common/src/route-table/RouteTable.spec.ts @@ -577,4 +577,132 @@ describe('RouteTable', () => { expect(rt.hasMatch('/api/endpoint.json')).toBe(true); }); }); + + describe('routeCount', () => { + it('returns 0 for empty table', () => { + const rt = new RouteTable(createMockFileSystem({})); + expect(rt.routeCount()).toBe(0); + }); + + it('counts single route entry', async () => { + const fs = createMockFileSystem( + Object.fromEntries([page('app/views/pages/about.html.liquid')]), + ); + const rt = new RouteTable(fs); + await rt.build(ROOT); + + expect(rt.routeCount()).toBe(1); + }); + + it('counts index alias as two entries', async () => { + const fs = createMockFileSystem( + Object.fromEntries([page('app/views/pages/products/index.html.liquid')]), + ); + const rt = new RouteTable(fs); + await rt.build(ROOT); + + // products + products/index + expect(rt.routeCount()).toBe(2); + }); + + it('decrements after removeFile', async () => { + const fs = createMockFileSystem( + Object.fromEntries([ + page('app/views/pages/a.html.liquid'), + page('app/views/pages/b.html.liquid'), + ]), + ); + const rt = new RouteTable(fs); + await rt.build(ROOT); + + expect(rt.routeCount()).toBe(2); + rt.removeFile('file:///project/app/views/pages/a.html.liquid'); + expect(rt.routeCount()).toBe(1); + }); + }); + + describe('large route table', () => { + it('handles hundreds of routes and matches correctly', async () => { + const files: [string, string][] = []; + for (let i = 0; i < 200; i++) { + files.push(page(`app/views/pages/section-${i}/page.html.liquid`)); + } + // Add some dynamic routes + for (let i = 0; i < 50; i++) { + files.push( + page(`app/views/pages/api-${i}.html.liquid`, `---\nslug: api/${i}/items/:id\n---\n`), + ); + } + const fs = createMockFileSystem(Object.fromEntries(files)); + const rt = new RouteTable(fs); + await rt.build(ROOT); + + expect(rt.routeCount()).toBe(250); + + // Static matches + expect(rt.hasMatch('/section-0/page')).toBe(true); + expect(rt.hasMatch('/section-199/page')).toBe(true); + expect(rt.hasMatch('/section-200/page')).toBe(false); + + // Dynamic matches + expect(rt.hasMatch('/api/0/items/42')).toBe(true); + expect(rt.hasMatch('/api/49/items/abc')).toBe(true); + expect(rt.hasMatch('/api/50/items/1')).toBe(false); + + // Precedence: static more specific than dynamic + const matches = rt.match('/api/0/items/42'); + expect(matches.length).toBe(1); + expect(matches[0].slug).toBe('api/0/items/:id'); + }); + + it('updateFile on large table does not corrupt other routes', async () => { + const files: [string, string][] = []; + for (let i = 0; i < 100; i++) { + files.push(page(`app/views/pages/page-${i}.html.liquid`)); + } + const fs = createMockFileSystem(Object.fromEntries(files)); + const rt = new RouteTable(fs); + await rt.build(ROOT); + + expect(rt.routeCount()).toBe(100); + + // Update one page to change its slug + rt.updateFile( + 'file:///project/app/views/pages/page-50.html.liquid', + '---\nslug: new-slug\n---\n', + ); + + expect(rt.routeCount()).toBe(100); + expect(rt.hasMatch('/page-50')).toBe(false); + expect(rt.hasMatch('/new-slug')).toBe(true); + // Other routes unaffected + expect(rt.hasMatch('/page-0')).toBe(true); + expect(rt.hasMatch('/page-99')).toBe(true); + }); + + it('build clears all routes before repopulating', async () => { + const files1: [string, string][] = []; + for (let i = 0; i < 50; i++) { + files1.push(page(`app/views/pages/old-${i}.html.liquid`)); + } + const fs1 = createMockFileSystem(Object.fromEntries(files1)); + const rt = new RouteTable(fs1); + await rt.build(ROOT); + + expect(rt.routeCount()).toBe(50); + + // Rebuild with different files + const files2: [string, string][] = []; + for (let i = 0; i < 30; i++) { + files2.push(page(`app/views/pages/new-${i}.html.liquid`)); + } + const fs2 = createMockFileSystem(Object.fromEntries(files2)); + const rt2 = new RouteTable(fs2); + await rt2.build(ROOT); + + expect(rt2.routeCount()).toBe(30); + expect(rt2.hasMatch('/old-0')).toBe(false); + expect(rt2.hasMatch('/new-0')).toBe(true); + }); + }); }); diff --git a/packages/platformos-common/src/route-table/RouteTable.ts b/packages/platformos-common/src/route-table/RouteTable.ts index 84e1176..725c87d 100644 --- a/packages/platformos-common/src/route-table/RouteTable.ts +++ b/packages/platformos-common/src/route-table/RouteTable.ts @@ -257,6 +257,15 @@ export class RouteTable { return false; } + /** Returns the total number of route entries (including index aliases). */ + routeCount(): number { + let count = 0; + for (const entries of this.routes.values()) { + count += entries.length; + } + return count; + } + allRoutes(): RouteEntry[] { const all: RouteEntry[] = []; for (const entries of this.routes.values()) { diff --git a/packages/platformos-language-server-common/src/definitions/providers/PageRouteDefinitionProvider.spec.ts b/packages/platformos-language-server-common/src/definitions/providers/PageRouteDefinitionProvider.spec.ts index 3959c8d..7bf06c6 100644 --- a/packages/platformos-language-server-common/src/definitions/providers/PageRouteDefinitionProvider.spec.ts +++ b/packages/platformos-language-server-common/src/definitions/providers/PageRouteDefinitionProvider.spec.ts @@ -524,6 +524,29 @@ describe('Module: PageRouteDefinitionProvider', () => { expect(result).toHaveLength(1); expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid'); }); + + it('navigates when assign and are both inside the same block tag', async () => { + // Regression test: when buildVariableMap skips recursion into block containers + // that end after beforeOffset, assigns inside the same block as are missed. + setup({ + 'app/views/pages/about.html.liquid': '

About

', + }); + + const source = + '{% if true %}{% assign url = "/about" %}About{% endif %}'; + documentManager.open('file:///project/app/views/pages/home.html.liquid', source, 1); + + const urlOffset = source.indexOf('{{ url }}'); + const params: DefinitionParams = { + textDocument: { uri: 'file:///project/app/views/pages/home.html.liquid' }, + position: { line: 0, character: urlOffset + 3 }, // Inside {{ url }} + }; + + const result = await provider.definitions(params); + assert(result); + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid'); + }); }); describe('format-aware go-to-definition', () => { diff --git a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts index 18f0e33..8568486 100644 --- a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts +++ b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, assert } from 'vitest'; import { toLiquidHtmlAST } from '@platformos/liquid-html-parser'; import { findCurrentNode } from '@platformos/platformos-check-common'; import { MockFileSystem } from '@platformos/platformos-check-common/src/test'; @@ -137,4 +137,55 @@ describe('RenderPartialDefinitionProvider', () => { expect(result).toHaveLength(0); }); }); + + describe('search path cache invalidation', () => { + it('returns stale result when cache is not invalidated after config change', async () => { + // Regression test: SearchPathsLoader caches results across calls. + // Without explicit invalidate(), a config change is not picked up. + const files: Record = { + 'project/app/config.yml': 'theme_search_paths:\n - theme/dress', + 'project/app/views/partials/theme/dress/card.liquid': 'dress card', + 'project/app/views/partials/theme/simple/card.liquid': 'simple card', + }; + const mockFs = new MockFileSystem(files); + const searchPathsCache = new SearchPathsLoader(mockFs); + const documentManager = new DocumentManager(); + const provider = new RenderPartialDefinitionProvider( + documentManager, + new DocumentsLocator(mockFs), + searchPathsCache, + async () => rootUri, + ); + + const source = "{% theme_render_rc 'card' %}"; + const offset = source.indexOf("'card'") + 1; + documentManager.open(uriString, source, 1); + + const ast = toLiquidHtmlAST(source); + const [node, ancestors] = findCurrentNode(ast, offset); + const params: DefinitionParams = { + textDocument: { uri: uriString }, + position: Position.create(0, offset), + }; + + // First call: populates the cache with 'theme/dress' + const result1 = await provider.definitions(params, node, ancestors); + assert(result1.length === 1); + expect(result1[0].targetUri).toContain('theme/dress/card.liquid'); + + // Simulate config.yml change on disk (mutate mockApp in-place) + files['project/app/config.yml'] = 'theme_search_paths:\n - theme/simple'; + + // Without invalidation: cache still returns 'theme/dress' result + const result2 = await provider.definitions(params, node, ancestors); + assert(result2.length === 1); + expect(result2[0].targetUri).toContain('theme/dress/card.liquid'); + + // After invalidation: re-reads config and returns 'theme/simple' result + searchPathsCache.invalidate(); + const result3 = await provider.definitions(params, node, ancestors); + assert(result3.length === 1); + expect(result3[0].targetUri).toContain('theme/simple/card.liquid'); + }); + }); }); diff --git a/packages/platformos-language-server-common/src/server/startServer.ts b/packages/platformos-language-server-common/src/server/startServer.ts index 4ceb2df..a99280c 100644 --- a/packages/platformos-language-server-common/src/server/startServer.ts +++ b/packages/platformos-language-server-common/src/server/startServer.ts @@ -56,6 +56,13 @@ import { URI } from 'vscode-uri'; const defaultLogger = () => {}; +/** + * When a file-watcher batch contains this many page-file changes, we + * assume a bulk operation (git checkout, branch switch, stash pop) and + * fully rebuild the route table instead of applying incremental updates. + */ +const BULK_PAGE_CHANGE_THRESHOLD = 10; + /** * The `git:` VFS does not support the `fs.readDirectory` call and makes most things break. * `git` URIs are the ones you'd encounter when doing a git diff in VS Code. They're not @@ -418,6 +425,15 @@ export function startServer( connection.onDidSaveTextDocument(async (params) => { if (hasUnsupportedDocument(params)) return; const { uri } = params.textDocument; + + // onDidChangeWatchedFiles also fires for in-editor saves, but it arrives after + // onDidSaveTextDocument. Invalidate the search-paths cache here immediately so + // that go-to-definition requests triggered by the same save don't see stale data + // while waiting for the file-watcher notification. + if (uri.endsWith('/app/config.yml')) { + searchPathsCache.invalidate(); + } + if (await configuration.shouldCheckOnSave()) { runChecks([uri]); } @@ -558,7 +574,7 @@ export function startServer( // individual onDidChangeTextDocument events. VS Code reports branch-switch changes // as FileChangeType.Changed, so we count all change types. const bulkPageChanges = params.changes.filter((c) => isPage(c.uri)); - if (bulkPageChanges.length >= 10) { + if (bulkPageChanges.length >= BULK_PAGE_CHANGE_THRESHOLD) { definitionsProvider.invalidateRouteTable(); } @@ -576,6 +592,7 @@ export function startServer( // so we don't need to invalidate it — reads are always fresh. if (change.uri.endsWith('/app/config.yml')) { documentsLocator.clearExpandedPathsCache(); + searchPathsCache.invalidate(); // Ensure open liquid files are re-checked with the new search paths for (const doc of documentManager.openDocuments) { diff --git a/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts b/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts index 6dfb837..552f62e 100644 --- a/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts +++ b/packages/platformos-language-server-common/src/utils/searchPaths.spec.ts @@ -17,7 +17,21 @@ function createMockFs(configContent: string | null) { } describe('SearchPathsLoader', () => { - it('should always read fresh (no internal caching)', async () => { + it('should cache results across calls for the same root', async () => { + const fs = createMockFs('theme_search_paths:\n - theme/dress'); + const loader = new SearchPathsLoader(fs); + + const result1 = await loader.get(rootUri); + expect(result1).toEqual(['theme/dress']); + + const result2 = await loader.get(rootUri); + expect(result2).toEqual(['theme/dress']); + + // readFile should only be called once — second call served from cache + expect(fs.readFile).toHaveBeenCalledTimes(1); + }); + + it('should return fresh results after invalidate()', async () => { let configContent = 'theme_search_paths:\n - theme/dress'; const fs = { readFile: vi.fn(async (uri: string) => { @@ -27,33 +41,61 @@ describe('SearchPathsLoader', () => { readDirectory: vi.fn(async () => []), stat: vi.fn(async () => ({ type: 1, size: 0 })), }; - const cache = new SearchPathsLoader(fs); + const loader = new SearchPathsLoader(fs); - const result1 = await cache.get(rootUri); + const result1 = await loader.get(rootUri); expect(result1).toEqual(['theme/dress']); - // Config changes — no invalidation needed, reads are always fresh + // Config changes externally configContent = 'theme_search_paths:\n - theme/simple'; - const result2 = await cache.get(rootUri); - expect(result2).toEqual(['theme/simple']); + // Without invalidation, still returns cached result + const result2 = await loader.get(rootUri); + expect(result2).toEqual(['theme/dress']); + + // After invalidation, reads fresh + loader.invalidate(); + const result3 = await loader.get(rootUri); + expect(result3).toEqual(['theme/simple']); + expect(fs.readFile).toHaveBeenCalledTimes(2); }); it('should return null when config has no search paths', async () => { const fs = createMockFs('some_other_key: value'); - const cache = new SearchPathsLoader(fs); + const loader = new SearchPathsLoader(fs); - const result = await cache.get(rootUri); + const result = await loader.get(rootUri); expect(result).toBeNull(); }); it('should return null when config.yml does not exist', async () => { const fs = createMockFs(null); - const cache = new SearchPathsLoader(fs); + const loader = new SearchPathsLoader(fs); - const result = await cache.get(rootUri); + const result = await loader.get(rootUri); expect(result).toBeNull(); }); + it('should cache per root URI independently', async () => { + const root2 = URI.parse('file:///other-project'); + const config2Uri = 'file:///other-project/app/config.yml'; + const fs = { + readFile: vi.fn(async (uri: string) => { + if (uri === configUri) return 'theme_search_paths:\n - theme/a'; + if (uri === config2Uri) return 'theme_search_paths:\n - theme/b'; + throw new Error(`File not found: ${uri}`); + }), + readDirectory: vi.fn(async () => []), + stat: vi.fn(async () => ({ type: 1, size: 0 })), + }; + const loader = new SearchPathsLoader(fs); + + const result1 = await loader.get(rootUri); + const result2 = await loader.get(root2); + + expect(result1).toEqual(['theme/a']); + expect(result2).toEqual(['theme/b']); + expect(fs.readFile).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/platformos-language-server-common/src/utils/searchPaths.ts b/packages/platformos-language-server-common/src/utils/searchPaths.ts index 71dc00b..e7d7496 100644 --- a/packages/platformos-language-server-common/src/utils/searchPaths.ts +++ b/packages/platformos-language-server-common/src/utils/searchPaths.ts @@ -2,15 +2,31 @@ import { AbstractFileSystem, loadSearchPaths } from '@platformos/platformos-comm import { URI } from 'vscode-uri'; /** - * Loader for theme_search_paths from app/config.yml. - * Always reads fresh — config.yml bypasses the CachedFileSystem cache - * so that external edits (e.g. vim, CLI) are picked up without relying - * on file watcher notifications. + * Cached loader for theme_search_paths from app/config.yml. + * + * Caches per root URI so that repeated calls (document links, definitions, + * checks) within the same editor session don't re-read and re-parse the + * config file on every invocation. + * + * Call `invalidate()` when app/config.yml changes on disk — the file watcher + * in startServer.ts handles this. This is the only reliable invalidation + * point because config.yml changes may come from external tools (vim, CLI) + * that don't trigger editor-level change events. */ export class SearchPathsLoader { + private cache = new Map>(); + constructor(private fs: AbstractFileSystem) {} get(rootUri: URI): Promise { - return loadSearchPaths(this.fs, rootUri); + const key = rootUri.toString(); + if (!this.cache.has(key)) { + this.cache.set(key, loadSearchPaths(this.fs, rootUri)); + } + return this.cache.get(key)!; + } + + invalidate(): void { + this.cache.clear(); } }