diff --git a/packages/platformos-language-server-common/src/diagnostics/runChecks.spec.ts b/packages/platformos-language-server-common/src/diagnostics/runChecks.spec.ts index 5a74b177..3dc61b34 100644 --- a/packages/platformos-language-server-common/src/diagnostics/runChecks.spec.ts +++ b/packages/platformos-language-server-common/src/diagnostics/runChecks.spec.ts @@ -9,6 +9,7 @@ import { MockFileSystem } from '@platformos/platformos-check-common/src/test'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { Connection } from 'vscode-languageserver'; import { DocumentManager } from '../documents'; +import { DocumentBackedFileSystem } from '../server/DocumentBackedFileSystem'; import { DiagnosticsManager } from './DiagnosticsManager'; import { makeRunChecks } from './runChecks'; @@ -274,4 +275,89 @@ describe('Module: runChecks', () => { diagnostics: [], }); }); + + describe('when a referenced partial is edited in the editor', () => { + const callerURI = path.join(rootUri, 'app', 'views', 'pages', 'index.liquid'); + const partialURI = path.join(rootUri, 'app', 'lib', 'my_partial.liquid'); + // The caller passes `arg`, which is only a known parameter once the partial + // references it. + const caller = `{% function res = 'my_partial', arg: 'value' %}`; + // On disk the partial only knows `followships`, so `arg` looks unknown. + const partialOnDisk = `{% liquid\n assign followships = followships | default: null\n%}`; + // In the editor the partial now uses `arg`, making it a known parameter. + const partialInBuffer = `{% liquid\n log arg, type: 'arg'\n assign followships = followships | default: null\n%}`; + + const partialCallArguments = allChecks.filter((c) => c.meta.code === 'PartialCallArguments'); + + const unknownArgDiagnostic = { + source: 'platformos-check', + code: 'PartialCallArguments', + codeDescription: { href: expect.any(String) }, + message: 'Unknown parameter arg passed to function call', + severity: 1, + range: { + start: { line: 0, character: 32 }, + end: { line: 0, character: 44 }, + }, + }; + + function makeRunChecksWithFs(fs: MockFileSystem | DocumentBackedFileSystem) { + return makeRunChecks(documentManager, diagnosticsManager, { + fs: fs as MockFileSystem, + loadConfig: async () => ({ + settings: {}, + checks: partialCallArguments, + rootUri, + }), + platformosDocset: { + graphQL: async () => null, + filters: async () => [], + objects: async () => [], + liquidDrops: async () => [], + tags: async () => [], + }, + jsonValidationSet: { + schemas: async () => [], + }, + }); + } + + beforeEach(() => { + expect(partialCallArguments).to.have.lengthOf(1); + fs = new MockFileSystem( + { + '.pos': '', + 'app/views/pages/index.liquid': caller, + 'app/lib/my_partial.liquid': partialOnDisk, + }, + rootUri, + ); + documentManager.open(callerURI, caller, 0); + documentManager.open(partialURI, partialInBuffer, 0); + }); + + it('reports a stale offense when the check reads the partial straight from disk', async () => { + runChecks = makeRunChecksWithFs(fs); + + await runChecks([callerURI]); + + expect(connection.sendDiagnostics).toHaveBeenCalledWith({ + uri: callerURI, + version: 0, + diagnostics: [unknownArgDiagnostic], + }); + }); + + it('reads the in-editor buffer of the partial through DocumentBackedFileSystem', async () => { + runChecks = makeRunChecksWithFs(new DocumentBackedFileSystem(fs, documentManager)); + + await runChecks([callerURI]); + + expect(connection.sendDiagnostics).toHaveBeenCalledWith({ + uri: callerURI, + version: 0, + diagnostics: [], + }); + }); + }); }); diff --git a/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.spec.ts b/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.spec.ts new file mode 100644 index 00000000..3fe364e3 --- /dev/null +++ b/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.spec.ts @@ -0,0 +1,54 @@ +import { AbstractFileSystem, FileType } from '@platformos/platformos-common'; +import { MockFileSystem } from '@platformos/platformos-check-common/src/test'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { DocumentManager } from '../documents'; +import { DocumentBackedFileSystem } from './DocumentBackedFileSystem'; + +describe('Module: DocumentBackedFileSystem', () => { + const rootUri = 'mock-fs:'; + const fooUri = 'mock-fs:/app/views/partials/foo.liquid'; + const barUri = 'mock-fs:/app/views/partials/bar.liquid'; + let diskFs: AbstractFileSystem; + let documentManager: DocumentManager; + let fs: DocumentBackedFileSystem; + + beforeEach(() => { + diskFs = new MockFileSystem( + { + 'app/views/partials/foo.liquid': 'saved foo', + 'app/views/partials/bar.liquid': 'saved bar', + }, + rootUri, + ); + documentManager = new DocumentManager(diskFs); + fs = new DocumentBackedFileSystem(diskFs, documentManager); + }); + + it('serves the in-editor buffer for a tracked document instead of the disk content', async () => { + documentManager.open(fooUri, 'edited foo', 1); + + expect(await fs.readFile(fooUri)).to.equal('edited foo'); + }); + + it('reflects later edits to a tracked document without any cache invalidation', async () => { + documentManager.open(fooUri, 'edited foo', 1); + documentManager.change(fooUri, 'edited foo again', 2); + + expect(await fs.readFile(fooUri)).to.equal('edited foo again'); + }); + + it('falls back to the underlying filesystem for untracked documents', async () => { + expect(await fs.readFile(barUri)).to.equal('saved bar'); + }); + + it('delegates readDirectory to the underlying filesystem', async () => { + expect(await fs.readDirectory('mock-fs:/app/views/partials')).to.deep.equal([ + ['mock-fs:/app/views/partials/foo.liquid', FileType.File], + ['mock-fs:/app/views/partials/bar.liquid', FileType.File], + ]); + }); + + it('delegates stat to the underlying filesystem', async () => { + expect(await fs.stat(fooUri)).to.deep.equal({ type: FileType.File, size: 'saved foo'.length }); + }); +}); diff --git a/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.ts b/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.ts new file mode 100644 index 00000000..9ddb659a --- /dev/null +++ b/packages/platformos-language-server-common/src/server/DocumentBackedFileSystem.ts @@ -0,0 +1,40 @@ +import { AbstractFileSystem, FileStat, FileTuple } from '@platformos/platformos-common'; +import { DocumentManager } from '../documents'; + +/** + * A filesystem layer that serves the content of tracked documents from the + * DocumentManager and delegates everything else to the underlying filesystem. + * + * Checks read the files they reference (e.g. the partial targeted by a + * `render`/`function` call) through `context.fs.readFile`. Without this layer + * those reads hit the disk-backed CachedFileSystem, which returns the last saved + * — and cached — version of the file. As a result, an edit to a partial did not + * update cross-file diagnostics on the *calling* file until the partial was saved + * and its cache entry invalidated. + * + * The DocumentManager always holds the latest in-editor buffer for open documents + * (and the latest disk content for preloaded ones), so preferring it keeps + * cross-file checks in sync with what the user is actually editing. + */ +export class DocumentBackedFileSystem implements AbstractFileSystem { + constructor( + private readonly fs: AbstractFileSystem, + private readonly documentManager: DocumentManager, + ) {} + + async readFile(uri: string): Promise { + const document = this.documentManager.get(uri); + if (document) { + return document.source; + } + return this.fs.readFile(uri); + } + + readDirectory(uri: string): Promise { + return this.fs.readDirectory(uri); + } + + stat(uri: string): Promise { + return this.fs.stat(uri); + } +} diff --git a/packages/platformos-language-server-common/src/server/startServer.ts b/packages/platformos-language-server-common/src/server/startServer.ts index a99280ce..58f78225 100644 --- a/packages/platformos-language-server-common/src/server/startServer.ts +++ b/packages/platformos-language-server-common/src/server/startServer.ts @@ -47,6 +47,7 @@ import { debounce } from '../utils'; import { SearchPathsLoader } from '../utils/searchPaths'; import { VERSION } from '../version'; import { CachedFileSystem } from './CachedFileSystem'; +import { DocumentBackedFileSystem } from './DocumentBackedFileSystem'; import { Configuration, INCLUDE_FILES_FROM_DISK } from './Configuration'; import { safe } from './safe'; import { AppGraphManager } from './AppGraphManager'; @@ -163,9 +164,14 @@ export function startServer( // These are augmented here so that the caching is maintained over different runs. const platformosDocset = new AugmentedPlatformOSDocset(remotePlatformOSDocset); + // Checks read the files they reference (e.g. the partial targeted by a + // render/function call) through `context.fs`. Serve those reads from the + // DocumentManager so cross-file diagnostics reflect the latest in-editor buffer + // rather than a stale, disk-cached copy. + const checksFs = new DocumentBackedFileSystem(fs, documentManager); const runChecks = debounce( makeRunChecks(documentManager, diagnosticsManager, { - fs, + fs: checksFs, loadConfig, platformosDocset, jsonValidationSet,