From 94fdaaf8d86fd0e60887c31d07c8fa8a27c504de Mon Sep 17 00:00:00 2001 From: Maciej Krajowski-Kukiel Date: Wed, 25 Mar 2026 08:50:27 +0100 Subject: [PATCH] fix ctrl + click --- .../src/checks/translation-utils.ts | 15 +- .../src/documents-locator/DocumentsLocator.ts | 22 ++- .../RenderPartialDefinitionProvider.ts | 13 +- .../documentLinks/DocumentLinksProvider.ts | 13 +- .../src/test/test-setup.js | 6 + .../prettier-plugin-liquid/vitest.config.mjs | 1 - .../vscode-extension/src/browser/extension.ts | 5 +- .../src/common/middleware.spec.ts | 186 +++++------------- .../vscode-extension/src/common/middleware.ts | 65 ++---- .../vscode-extension/src/node/extension.ts | 5 +- 10 files changed, 120 insertions(+), 211 deletions(-) diff --git a/packages/platformos-check-common/src/checks/translation-utils.ts b/packages/platformos-check-common/src/checks/translation-utils.ts index 2b22dac..e9688c3 100644 --- a/packages/platformos-check-common/src/checks/translation-utils.ts +++ b/packages/platformos-check-common/src/checks/translation-utils.ts @@ -11,14 +11,15 @@ export async function discoverModules( ): Promise> { const modules = new Set(); for (const dirUri of moduleDirUris) { - const stat = await fs.stat(dirUri).catch(() => undefined); - if (!stat || stat.type !== FileType.Directory) continue; - - const entries = await fs.readDirectory(dirUri); - for (const [entryUri, entryType] of entries) { - if (entryType === FileType.Directory) { - modules.add(entryUri.split('/').pop()!); + try { + const entries = await fs.readDirectory(dirUri); + for (const [entryUri, entryType] of entries) { + if (entryType === FileType.Directory) { + modules.add(entryUri.split('/').pop()!); + } } + } catch { + // Directory doesn't exist or isn't accessible — skip } } return modules; diff --git a/packages/platformos-common/src/documents-locator/DocumentsLocator.ts b/packages/platformos-common/src/documents-locator/DocumentsLocator.ts index b3f78e0..331165e 100644 --- a/packages/platformos-common/src/documents-locator/DocumentsLocator.ts +++ b/packages/platformos-common/src/documents-locator/DocumentsLocator.ts @@ -264,7 +264,8 @@ export class DocumentsLocator { } /** - * Returns the canonical default URI for a missing file so it can be created. + * Returns the canonical URI where `fileName` would live — used as a + * go-to-definition fallback when the file doesn't exist yet. * Returns undefined for theme_render_rc (ambiguous search path) and asset. */ locateDefault(rootUri: URI, nodeName: DocumentType, fileName: string): string | undefined { @@ -289,6 +290,9 @@ export class DocumentsLocator { basePath = parsed.isModule ? `modules/${parsed.moduleName}/public/graphql` : 'app/graphql'; ext = '.graphql'; break; + case 'theme_render_rc': // ambiguous — multiple search paths, no single canonical location + case 'asset': // no canonical creation path + return undefined; default: return undefined; } @@ -296,6 +300,22 @@ export class DocumentsLocator { return Utils.joinPath(rootUri, basePath, parsed.key + ext).toString(); } + /** + * Resolves `fileName` to a filesystem URI (if the file exists), or falls + * back to the canonical default URI from `locateDefault`. + */ + async locateOrDefault( + rootUri: URI, + nodeName: DocumentType, + fileName: string, + themeSearchPaths?: string[] | null, + ): Promise { + return ( + (await this.locate(rootUri, nodeName, fileName, themeSearchPaths)) ?? + this.locateDefault(rootUri, nodeName, fileName) + ); + } + async locate( rootUri: URI, nodeName: DocumentType, diff --git a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts index 87195ef..6dc6861 100644 --- a/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts +++ b/packages/platformos-language-server-common/src/definitions/providers/RenderPartialDefinitionProvider.ts @@ -47,13 +47,12 @@ export class RenderPartialDefinitionProvider implements BaseDefinitionProvider { 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, - )) ?? this.documentsLocator.locateDefault(root, docType, (node as LiquidString).value); + const fileUri = await this.documentsLocator.locateOrDefault( + root, + docType, + (node as LiquidString).value, + searchPaths, + ); if (!fileUri) return []; const sourceCode = this.documentManager.get(params.textDocument.uri); diff --git a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts index e9f61b8..4a670cf 100644 --- a/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts +++ b/packages/platformos-language-server-common/src/documentLinks/DocumentLinksProvider.ts @@ -64,17 +64,18 @@ function documentLinksVisitor( // render, include, function, theme_render_rc all have a .partial field if ('partial' in markup && isLiquidString(markup.partial)) { - const uri = - (await documentsLocator.locate(root, name, markup.partial.value, searchPaths)) ?? - documentsLocator.locateDefault(root, name, markup.partial.value); + const uri = await documentsLocator.locateOrDefault( + root, + name, + markup.partial.value, + searchPaths, + ); return DocumentLink.create(range(textDocument, markup.partial), uri); } // graphql has a .graphql field if ('graphql' in markup && isLiquidString(markup.graphql)) { - const uri = - (await documentsLocator.locate(root, name, markup.graphql.value)) ?? - documentsLocator.locateDefault(root, name, markup.graphql.value); + const uri = await documentsLocator.locateOrDefault(root, name, markup.graphql.value); return DocumentLink.create(range(textDocument, markup.graphql), uri); } }, diff --git a/packages/prettier-plugin-liquid/src/test/test-setup.js b/packages/prettier-plugin-liquid/src/test/test-setup.js index a842717..94e1bc1 100644 --- a/packages/prettier-plugin-liquid/src/test/test-setup.js +++ b/packages/prettier-plugin-liquid/src/test/test-setup.js @@ -1,5 +1,6 @@ const moduleAlias = require('module-alias'); const path = require('path'); +const fs = require('fs'); const prettierMajor = process.env.PRETTIER_MAJOR; const prettierPath = @@ -8,6 +9,11 @@ const prettierPath = : path.join(__dirname, '..', '..', '..', '..', 'node_modules', 'prettier2'); export function setup() { + // Generate the Ohm grammar shim once, before any test workers start. + // This avoids a race condition when parallel workers all try to write the + // same file simultaneously (causes SyntaxError: Unexpected end of input on Windows). + require(path.join(__dirname, '..', '..', '..', 'liquid-html-parser', 'build', 'shims.js')); + moduleAlias.addAlias('prettier', prettierPath); console.error('===================================='); console.error(`Prettier version: ${require('prettier').version}`); diff --git a/packages/prettier-plugin-liquid/vitest.config.mjs b/packages/prettier-plugin-liquid/vitest.config.mjs index 2dab585..511e15d 100644 --- a/packages/prettier-plugin-liquid/vitest.config.mjs +++ b/packages/prettier-plugin-liquid/vitest.config.mjs @@ -13,6 +13,5 @@ export default defineConfig({ }, }, globalSetup: ['./src/test/test-setup.js'], - setupFiles: ['../liquid-html-parser/build/shims.js'], }, }); diff --git a/packages/vscode-extension/src/browser/extension.ts b/packages/vscode-extension/src/browser/extension.ts index 8991926..89d0346 100644 --- a/packages/vscode-extension/src/browser/extension.ts +++ b/packages/vscode-extension/src/browser/extension.ts @@ -11,7 +11,7 @@ import LiquidFormatter from '../common/formatter'; import { vscodePrettierFormat } from './formatter'; import { documentSelectors } from '../common/constants'; import { openLocation } from '../common/commands'; -import { buildMiddleware, openFileMissingCommand } from '../common/middleware'; +import { middleware } from '../common/middleware'; import { createReferencesTreeView, setupContext, @@ -31,7 +31,6 @@ export async function activate(context: ExtensionContext) { client!.sendRequest('workspace/executeCommand', { command: runChecksCommand }); }), commands.registerCommand('platformosLiquid.openLocation', openLocation), - commands.registerCommand('platformosLiquid.openFile', openFileMissingCommand), languages.registerDocumentFormattingEditProvider( [{ language: 'liquid' }], new LiquidFormatter(vscodePrettierFormat), @@ -58,7 +57,7 @@ async function startServer(context: ExtensionContext) { console.log('Starting App Check Language Server'); const clientOptions: LanguageClientOptions = { documentSelector: documentSelectors as DocumentSelector, - middleware: buildMiddleware(), + middleware, }; client = createWorkerLanguageClient(context, clientOptions); diff --git a/packages/vscode-extension/src/common/middleware.spec.ts b/packages/vscode-extension/src/common/middleware.spec.ts index c21df88..f03786d 100644 --- a/packages/vscode-extension/src/common/middleware.spec.ts +++ b/packages/vscode-extension/src/common/middleware.spec.ts @@ -1,174 +1,82 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { mockShowTextDocument, mockStat, MockDocumentLink } = vi.hoisted(() => { +const { mockStat, MockDocumentLink } = vi.hoisted(() => { class MockDocumentLink { constructor( public range: any, public target?: any, ) {} } - return { - mockShowTextDocument: vi.fn(), - mockStat: vi.fn(), - MockDocumentLink, - }; + return { mockStat: vi.fn(), MockDocumentLink }; }); vi.mock('vscode', () => ({ - DocumentLink: MockDocumentLink, - Uri: { - parse: (str: string) => ({ toString: () => str, _str: str }), - }, - window: { showTextDocument: mockShowTextDocument }, workspace: { fs: { stat: mockStat } }, })); -import { buildMiddleware, openFileMissingCommand } from './middleware'; +import { middleware } from './middleware'; const EXISTING_URI = { toString: () => 'file:///project/app/views/partials/exists.liquid' }; const MISSING_URI = { toString: () => 'file:///project/app/views/partials/missing.liquid' }; const fakeRange = { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }; const fakeDocument = {} as any; -const fakePosition = {} as any; const fakeToken = {} as any; function makeLink(target: any) { return new MockDocumentLink(fakeRange, target) as any; } -describe('middleware', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe('openFileMissingCommand', () => { - it('calls window.showTextDocument with a parsed URI', () => { - openFileMissingCommand('file:///project/app/views/partials/new.liquid'); - expect(mockShowTextDocument).toHaveBeenCalledOnce(); - expect(mockShowTextDocument.mock.calls[0][0].toString()).to.equal( - 'file:///project/app/views/partials/new.liquid', - ); - }); - }); - - describe('provideDocumentLinks', () => { - const { provideDocumentLinks } = buildMiddleware(); - - it('returns null when next returns null', async () => { - const next = vi.fn().mockResolvedValue(null); - const result = await provideDocumentLinks!(fakeDocument, fakeToken, next); - expect(result).to.equal(null); - }); - - it('returns links unchanged when all targets exist', async () => { - mockStat.mockResolvedValue({}); - const link = makeLink(EXISTING_URI); - const next = vi.fn().mockResolvedValue([link]); - - const result = await provideDocumentLinks!(fakeDocument, fakeToken, next); - - expect(result).to.deep.equal([link]); - expect(mockStat).toHaveBeenCalledWith(EXISTING_URI); - }); - - it('replaces target with command URI when file is missing', async () => { - mockStat.mockRejectedValue(new Error('file not found')); - const link = makeLink(MISSING_URI); - const next = vi.fn().mockResolvedValue([link]); - - const result = (await provideDocumentLinks!(fakeDocument, fakeToken, next)) as any[]; - - expect(result).toHaveLength(1); - const commandArg = encodeURIComponent(JSON.stringify([MISSING_URI.toString()])); - expect(result[0].target.toString()).to.equal( - `command:platformosLiquid.openFile?${commandArg}`, - ); - expect(result[0].range).to.equal(fakeRange); - }); - - it('handles mixed links: existing unchanged, missing gets command URI', async () => { - mockStat - .mockResolvedValueOnce({}) // first link exists - .mockRejectedValueOnce(new Error('not found')); // second link missing - - const existingLink = makeLink(EXISTING_URI); - const missingLink = makeLink(MISSING_URI); - const next = vi.fn().mockResolvedValue([existingLink, missingLink]); - - const result = (await provideDocumentLinks!(fakeDocument, fakeToken, next)) as any[]; - - expect(result).toHaveLength(2); - expect(result[0]).to.equal(existingLink); - expect(result[1].target.toString()).to.include('command:platformosLiquid.openFile'); - }); +describe('provideDocumentLinks middleware', () => { + const { provideDocumentLinks } = middleware; - it('passes through links with no target unchanged', async () => { - const link = makeLink(undefined); - const next = vi.fn().mockResolvedValue([link]); + beforeEach(() => vi.clearAllMocks()); - const result = await provideDocumentLinks!(fakeDocument, fakeToken, next); - - expect(result).to.deep.equal([link]); - expect(mockStat).not.toHaveBeenCalled(); - }); + it('returns null when next returns null', async () => { + const next = vi.fn().mockResolvedValue(null); + expect(await provideDocumentLinks!(fakeDocument, fakeToken, next)).to.equal(null); }); - describe('provideDefinition', () => { - const { provideDefinition } = buildMiddleware(); - - it('returns null when next returns null', async () => { - const next = vi.fn().mockResolvedValue(null); - const result = await provideDefinition!(fakeDocument, fakePosition, fakeToken, next); - expect(result).to.equal(null); - }); - - it('returns result unchanged when target file exists (Location shape)', async () => { - mockStat.mockResolvedValue({}); - const location = { uri: EXISTING_URI }; - const next = vi.fn().mockResolvedValue(location); - - const result = await provideDefinition!(fakeDocument, fakePosition, fakeToken, next); - - expect(result).to.equal(location); - expect(mockShowTextDocument).not.toHaveBeenCalled(); - }); - - it('returns result unchanged when target file exists (LocationLink shape)', async () => { - mockStat.mockResolvedValue({}); - const locationLink = { - targetUri: EXISTING_URI, - targetRange: fakeRange, - originSelectionRange: fakeRange, - }; - const next = vi.fn().mockResolvedValue([locationLink]); - - const result = await provideDefinition!(fakeDocument, fakePosition, fakeToken, next); - - expect(result).to.deep.equal([locationLink]); - expect(mockShowTextDocument).not.toHaveBeenCalled(); - }); - - it('opens missing file and returns null (Location shape)', async () => { - mockStat.mockRejectedValue(new Error('not found')); - const location = { uri: MISSING_URI }; - const next = vi.fn().mockResolvedValue(location); - - const result = await provideDefinition!(fakeDocument, fakePosition, fakeToken, next); - - expect(result).to.equal(null); - expect(mockShowTextDocument).toHaveBeenCalledWith(MISSING_URI); - }); + it('returns links unchanged when all targets exist', async () => { + mockStat.mockResolvedValue({}); + const link = makeLink(EXISTING_URI); + const result = await provideDocumentLinks!( + fakeDocument, + fakeToken, + vi.fn().mockResolvedValue([link]), + ); + expect(result).to.deep.equal([link]); + }); - it('opens missing file and returns null (LocationLink shape)', async () => { - mockStat.mockRejectedValue(new Error('not found')); - const locationLink = { targetUri: MISSING_URI, targetRange: fakeRange }; - const next = vi.fn().mockResolvedValue([locationLink]); + it('removes link when target file is missing — ctrl+click falls through to go-to-definition', async () => { + mockStat.mockRejectedValue(new Error('file not found')); + const result = await provideDocumentLinks!( + fakeDocument, + fakeToken, + vi.fn().mockResolvedValue([makeLink(MISSING_URI)]), + ); + expect(result).to.deep.equal([]); + }); - const result = await provideDefinition!(fakeDocument, fakePosition, fakeToken, next); + it('keeps existing links, removes missing ones', async () => { + mockStat.mockResolvedValueOnce({}).mockRejectedValueOnce(new Error('not found')); + const existingLink = makeLink(EXISTING_URI); + const result = (await provideDocumentLinks!( + fakeDocument, + fakeToken, + vi.fn().mockResolvedValue([existingLink, makeLink(MISSING_URI)]), + )) as any[]; + expect(result).to.deep.equal([existingLink]); + }); - expect(result).to.equal(null); - expect(mockShowTextDocument).toHaveBeenCalledWith(MISSING_URI); - }); + it('passes through links with no target unchanged', async () => { + const link = makeLink(undefined); + const result = await provideDocumentLinks!( + fakeDocument, + fakeToken, + vi.fn().mockResolvedValue([link]), + ); + expect(result).to.deep.equal([link]); + expect(mockStat).not.toHaveBeenCalled(); }); }); diff --git a/packages/vscode-extension/src/common/middleware.ts b/packages/vscode-extension/src/common/middleware.ts index bf9a95a..751b683 100644 --- a/packages/vscode-extension/src/common/middleware.ts +++ b/packages/vscode-extension/src/common/middleware.ts @@ -1,46 +1,23 @@ -import { DocumentLink, Location, LocationLink, Uri, window, workspace } from 'vscode'; +import { workspace } from 'vscode'; import type { Middleware } from 'vscode-languageclient'; -export function openFileMissingCommand(uriString: string) { - window.showTextDocument(Uri.parse(uriString)); -} - -export function buildMiddleware(): Middleware { - return { - provideDocumentLinks: async (document, token, next) => { - const links = await next(document, token); - if (!links) return links; - return Promise.all( - links.map(async (link) => { - if (!link.target) return link; - try { - await workspace.fs.stat(link.target); - return link; - } catch { - const commandArg = encodeURIComponent(JSON.stringify([link.target.toString()])); - return new DocumentLink( - link.range, - Uri.parse(`command:platformosLiquid.openFile?${commandArg}`), - ); - } - }), - ); - }, - provideDefinition: async (document, position, token, next) => { - const result = await next(document, position, token); - if (!result) return result; - const defs = Array.isArray(result) ? result : [result]; - const first = defs[0]; - if (!first) return result; - const targetUri = - 'targetUri' in first ? (first as LocationLink).targetUri : (first as Location).uri; - try { - await workspace.fs.stat(targetUri); - return result; - } catch { - await window.showTextDocument(targetUri); - return null; - } - }, - }; -} +export const middleware: Middleware = { + provideDocumentLinks: async (document, token, next) => { + const links = await next(document, token); + if (!links) return links; + const results = await Promise.all( + links.map(async (link) => { + if (!link.target) return link; + try { + await workspace.fs.stat(link.target); + return link; + } catch { + // File doesn't exist — remove so ctrl+click falls through to + // go-to-definition, which VS Code handles natively (opens empty editor). + return null; + } + }), + ); + return results.filter((l) => l !== null); + }, +}; diff --git a/packages/vscode-extension/src/node/extension.ts b/packages/vscode-extension/src/node/extension.ts index 76632fa..222897b 100644 --- a/packages/vscode-extension/src/node/extension.ts +++ b/packages/vscode-extension/src/node/extension.ts @@ -18,7 +18,7 @@ import { watchReferencesTreeViewConfig, } from '../common/ReferencesProvider'; import { openLocation } from '../common/commands'; -import { buildMiddleware, openFileMissingCommand } from '../common/middleware'; +import { middleware } from '../common/middleware'; const sleep = (ms: number) => new Promise((res) => setTimeout(res, ms)); @@ -33,7 +33,6 @@ export async function activate(context: ExtensionContext) { client!.sendRequest('workspace/executeCommand', { command: runChecksCommand }); }), commands.registerCommand('platformosLiquid.openLocation', openLocation), - commands.registerCommand('platformosLiquid.openFile', openFileMissingCommand), languages.registerDocumentFormattingEditProvider( [{ language: 'liquid' }], new LiquidFormatter(vscodePrettierFormat), @@ -68,7 +67,7 @@ async function startServer(context: ExtensionContext) { const clientOptions: LanguageClientOptions = { documentSelector: documentSelectors as DocumentSelector, - middleware: buildMiddleware(), + middleware, }; client = new LanguageClient(