diff --git a/apps/vscode/build.ts b/apps/vscode/build.ts index 8595c1183..617776868 100644 --- a/apps/vscode/build.ts +++ b/apps/vscode/build.ts @@ -19,13 +19,14 @@ import * as glob from "glob"; const args = process.argv; const dev = args[2] === "dev"; const test = args[2] === "test"; -const testFiles = glob.sync("src/test/*.ts"); +const testFiles = glob.sync("src/test/{*.ts,fixtures/*.ts,utils/*.ts}"); const testBuildOptions = { entryPoints: testFiles, outdir: 'test-out', external: ['vscode', 'mocha', 'glob'], sourcemap: true, + dev, }; const defaultBuildOptions = { @@ -36,4 +37,11 @@ const defaultBuildOptions = { dev }; -runBuild(test ? testBuildOptions : defaultBuildOptions); +if (test) { + runBuild(testBuildOptions); +} else if (dev) { + runBuild(defaultBuildOptions); + runBuild(testBuildOptions); +} else { + runBuild(defaultBuildOptions); +} diff --git a/apps/vscode/package.json b/apps/vscode/package.json index 596dbe845..74a4d332d 100644 --- a/apps/vscode/package.json +++ b/apps/vscode/package.json @@ -1003,6 +1003,20 @@ "default": true, "markdownDescription": "Show parameter help when editing function calls." }, + "quarto.cells.diagnostics.enabled": { + "order": 25, + "scope": "window", + "type": "boolean", + "default": true, + "markdownDescription": "Enable diagnostics (linting) for code blocks from language servers." + }, + "quarto.cells.diagnostics.debounceDelay": { + "order": 26, + "scope": "window", + "type": "number", + "default": 500, + "markdownDescription": "Delay in milliseconds before updating diagnostics after document changes." + }, "quarto.cells.background.enabled": { "type": "boolean", "description": "Enable coloring the background of executable code cells.", @@ -1049,7 +1063,7 @@ "markdownDescription": "Millisecond delay between background color updates." }, "quarto.cells.useReticulate": { - "order": 25, + "order": 27, "scope": "window", "type": "boolean", "default": true, diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index 21e41cee3..c5840ae16 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -24,7 +24,6 @@ import { Definition, LogOutputChannel, Uri, - Diagnostic, window, ColorThemeKind } from "vscode"; @@ -49,7 +48,6 @@ import { ProvideHoverSignature, ProvideSignatureHelpSignature, State, - HandleDiagnosticsSignature } from "vscode-languageclient"; import { MarkdownEngine } from "../markdown/engine"; import { @@ -58,7 +56,6 @@ import { virtualDoc, withVirtualDocUri, } from "../vdoc/vdoc"; -import { isVirtualDoc } from "../vdoc/vdoc-tempfile"; import { activateVirtualDocEmbeddedContent } from "../vdoc/vdoc-content"; import { vdocCompletions } from "../vdoc/vdoc-completion"; @@ -79,7 +76,7 @@ export async function activateLsp( context: ExtensionContext, quartoContext: QuartoContext, engine: MarkdownEngine, - outputChannel: LogOutputChannel + outputChannel: LogOutputChannel, ) { // The server is implemented in node @@ -105,7 +102,6 @@ export async function activateLsp( const config = workspace.getConfiguration("quarto"); activateVirtualDocEmbeddedContent(); const middleware: Middleware = { - handleDiagnostics: createDiagnosticFilter(), provideCompletionItem: embeddedCodeCompletionProvider(engine), provideDefinition: embeddedGoToDefinitionProvider(engine), provideDocumentFormattingEdits: embeddedDocumentFormattingProvider(engine), @@ -363,21 +359,3 @@ function isWithinYamlComment(doc: TextDocument, pos: Position) { const line = doc.lineAt(pos.line).text; return !!line.match(/^\s*#\s*\| /); } - -/** - * Creates a diagnostic handler middleware that filters out diagnostics from virtual documents - * - * @returns A handler function for the middleware - */ -export function createDiagnosticFilter() { - return (uri: Uri, diagnostics: Diagnostic[], next: HandleDiagnosticsSignature) => { - // If this is not a virtual document, pass through all diagnostics - if (!isVirtualDoc(uri)) { - next(uri, diagnostics); - return; - } - - // For virtual documents, filter out all diagnostics - next(uri, []); - }; -} diff --git a/apps/vscode/src/main.ts b/apps/vscode/src/main.ts index e4eca1d70..254645a7a 100644 --- a/apps/vscode/src/main.ts +++ b/apps/vscode/src/main.ts @@ -19,6 +19,7 @@ import { tryAcquirePositronApi } from "@posit-dev/positron"; import { MarkdownEngine } from "./markdown/engine"; import { kQuartoDocSelector } from "./core/doc"; import { activateLsp, deactivate as deactivateLsp } from "./lsp/client"; +import { activateEmbeddedDiagnostics, type EmbeddedDiagnosticsService } from "./providers/diagnostics"; import { cellCommands } from "./providers/cell/commands"; import { quartoCellExecuteCodeLensProvider } from "./providers/cell/codelens"; import { activateQuartoAssistPanel } from "./providers/assist/panel"; @@ -50,6 +51,8 @@ import { activateContextKeySetter } from "./providers/context-keys"; import { CommandManager } from "./core/command"; import { createQuartoExtensionApi, QuartoExtensionApi } from "./api"; +let embeddedDiagnostics: EmbeddedDiagnosticsService | undefined; + /** * Entry point for the entire extension! This initializes the LSP, quartoContext, extension host, and more... */ @@ -117,6 +120,10 @@ export async function activate(context: vscode.ExtensionContext): Promise Promise; +} + +/** + * Tracks the diagnostic state for an embedded language in a document. + */ +interface DiagnosticSession { + /** The document this session belongs to. */ + readonly documentUri: Uri; + + /** The embedded language (Python, R, etc.). */ + readonly language: EmbeddedLanguage; + + /** Code blocks for this language, used to filter diagnostics by position. */ + readonly languageBlocks: (TokenMath | TokenCodeBlock)[]; + + /** The active virtual document awaiting diagnostics, if any. */ + activeVdoc?: ActiveVdoc; + + /** Last received diagnostics for this language (stale-until-replaced on edits). */ + diagnostics: Diagnostic[]; +} + +/** + * Surfaces language-server diagnostics from embedded code cells in Quarto documents. + * + * Creates a temporary virtual document per language, waits for the language server + * to publish diagnostics on it, then maps the diagnostics back onto the original + * `.qmd` file. Each language's lifecycle is independent - one slow or non-responsive + * language server won't block diagnostics from other languages. + */ +export class EmbeddedDiagnosticsManager extends Disposable { + private readonly _onDidUpdateDiagnostics = this._register( + new EventEmitter() + ); + + /** Event fired when embedded diagnostics are updated for a document. */ + public readonly onDidUpdateDiagnostics = this._onDidUpdateDiagnostics.event; + + private readonly _onDidActivateVdoc = this._register( + new EventEmitter() + ); + + /** Event fired when a virtual document is activated (created and opened). */ + public readonly onDidActivateVdoc = this._onDidActivateVdoc.event; + + private readonly _onDidDisposeVdoc = this._register( + new EventEmitter() + ); + + /** Event fired when a virtual document is disposed. */ + public readonly onDidDisposeVdoc = this._onDidDisposeVdoc.event; + + /** Diagnostic collection for Quarto documents. */ + private readonly diagnosticCollection = this._register( + languages.createDiagnosticCollection("quarto") + ); + + /** Sessions keyed by document URI string, then language ID. */ + private readonly sessionsByDocument = new Map>(); + + /** Reverse index: vdoc URI string → session. */ + private readonly sessionByVdocUri = new Map(); + + /** Debounce timers for document changes, keyed by URI string. */ + private readonly debounceTimers = new Map(); + + constructor( + private readonly engine: MarkdownEngine, + private readonly outputChannel: LogOutputChannel, + /** Timeout for waiting for the language server to publish diagnostics. */ + private readonly timeoutMs = DEFAULT_TIMEOUT_MS, + ) { + super(); + + // Listen for diagnostics arriving on virtual documents. + this._register(languages.onDidChangeDiagnostics(async (event) => { + for (const uri of event.uris) { + const session = this.sessionByVdocUri.get(uri.toString()); + if (session) { + await this.handleDiagnosticsReceived(session, uri); + } + } + })); + + // Document lifecycle. + this._register(workspace.onDidOpenTextDocument(async (doc) => { + if (isQuartoDoc(doc)) { + await this.handleDocumentOpen(doc); + } + })); + + this._register(workspace.onDidChangeTextDocument(async (e) => { + if (isQuartoDoc(e.document)) { + await this.handleDocumentChange(e.document); + } + })); + + this._register(workspace.onDidCloseTextDocument(async (doc) => { + if (isQuartoDoc(doc)) { + await this.handleDocumentClose(doc); + } + })); + + // Process already-open documents. + for (const doc of workspace.textDocuments) { + if (isQuartoDoc(doc)) { + this.handleDocumentOpen(doc).catch((error) => { + this.outputChannel.error( + `[EmbeddedDiagnostics] Failed to initialize ${workspace.asRelativePath(doc.uri)}: ` + + JSON.stringify(error) + ); + }); + } + } + } + + // --- Document lifecycle --- + + private async handleDocumentOpen(document: TextDocument): Promise { + await this.createSessionsForDocument(document); + } + + private async handleDocumentChange(document: TextDocument): Promise { + const key = document.uri.toString(); + const existingTimer = this.debounceTimers.get(key); + if (existingTimer) { + clearTimeout(existingTimer); + } + + const debounceDelay = workspace + .getConfiguration("quarto.cells.diagnostics") + .get("debounceDelay", 500); + + const timer = setTimeout(() => { + this.debounceTimers.delete(key); + this.recreateSessionsForDocument(document).catch((error) => { + this.outputChannel.error( + `[EmbeddedDiagnostics] Failed to recreate sessions for ` + + `${workspace.asRelativePath(document.uri)}: ` + + JSON.stringify(error) + ); + }); + }, debounceDelay); + + this.debounceTimers.set(key, timer); + } + + private async handleDocumentClose(document: TextDocument): Promise { + const key = document.uri.toString(); + + // Cancel pending debounce. + const timer = this.debounceTimers.get(key); + if (timer) { + clearTimeout(timer); + this.debounceTimers.delete(key); + } + + // Dispose all sessions for this document. + await this.removeSessionsForDocument(document.uri, "session-removed"); + + // Clear published diagnostics. + this.diagnosticCollection.delete(document.uri); + this._onDidUpdateDiagnostics.fire({ documentUri: document.uri, diagnostics: [] }); + } + + // --- Session management --- + + private async createSessionsForDocument(document: TextDocument): Promise { + const tokens = this.engine.parse(document); + + // Create or append blocks to each language's session for the document. + for (const block of tokens.filter(isExecutableLanguageBlock)) { + const languageName = languageNameFromBlock(block); + if (!languageName) { continue; } + const language = embeddedLanguage(languageName); + if (!language) { continue; } + const session = this.getOrCreateSession(document.uri, language); + session.languageBlocks.push(block); + } + + const docSessions = this.getSessionsForDocument(document.uri); + if (docSessions.length === 0) { + // No executable cells - clear stale diagnostics that + // won't be superseded by a new vdoc round-trip. + this.diagnosticCollection.delete(document.uri); + this._onDidUpdateDiagnostics.fire({ documentUri: document.uri, diagnostics: [] }); + return; + } else { + // Concurrently activate sessions for this document. + await Promise.all(docSessions.map(s => this.activateSession(s, document, tokens))); + } + } + + private async recreateSessionsForDocument(document: TextDocument): Promise { + await this.removeSessionsForDocument(document.uri, "document-changed"); + await this.createSessionsForDocument(document); + } + + private async activateSession( + session: DiagnosticSession, + document: TextDocument, + tokens: Token[], + ): Promise { + try { + const vdocContent = virtualDocForLanguage( + document, tokens, session.language, "diagnostics" + ); + + const dir = this.shouldUseLocalTempFile(session.language) + ? quartoVdocDir(document.uri.fsPath) + : VIRTUAL_DOC_TEMP_DIRECTORY; + const vdoc = await virtualDocUriFromTempFile( + vdocContent, dir, { warmup: false } + ); + + const timeout = setTimeout(async () => { + this.outputChannel.warn( + `[EmbeddedDiagnostics] Language server for ${session.language.ids[0]} ` + + `did not respond within ${this.timeoutMs}ms ` + + `for ${workspace.asRelativePath(session.documentUri)}` + ); + await this.disposeActiveVdoc(session, 'timeout'); + // The happy path (handleDiagnosticsReceived) replaces old diagnostics + // with fresh ones. On timeout, no replacement is coming - clear explicitly. + session.diagnostics = []; + this.publishDiagnostics(session.documentUri); + }, this.timeoutMs); + + session.activeVdoc = { + uri: vdoc.uri, + cleanup: async () => { + clearTimeout(timeout); + if (vdoc.cleanup) { + await vdoc.cleanup(); + } + }, + }; + this.sessionByVdocUri.set(vdoc.uri.toString(), session); + this._onDidActivateVdoc.fire({ + uri: vdoc.uri, + documentUri: session.documentUri, + language: session.language.ids[0], + }); + + this.outputChannel.debug( + `[EmbeddedDiagnostics] Activated vdoc for ` + + `${session.language.ids[0]} in ${workspace.asRelativePath(session.documentUri)}: ` + + vdoc.uri.toString() + ); + } catch (error) { + this.outputChannel.error( + `[EmbeddedDiagnostics] Failed to create vdoc for ` + + `${session.language.ids[0]} in ${workspace.asRelativePath(session.documentUri)}: ` + + JSON.stringify(error) + ); + // Same as timeout - no replacement diagnostics are coming. + session.diagnostics = []; + this.publishDiagnostics(session.documentUri); + } + } + + // --- Diagnostics handling --- + + private async handleDiagnosticsReceived(session: DiagnosticSession, vdocUri: Uri): Promise { + const rawDiagnostics = languages.getDiagnostics(vdocUri); + + this.outputChannel.debug( + `[EmbeddedDiagnostics] Received ${rawDiagnostics.length} diagnostics for ` + + `${session.language.ids[0]} in ${workspace.asRelativePath(session.documentUri)}` + ); + + // Filter: only keep diagnostics that map to a real language block. + const mapped: Diagnostic[] = []; + for (const diagnostic of rawDiagnostics) { + const block = languageBlockAtPosition(session.languageBlocks, diagnostic.range.start); + if (block !== undefined) { + mapped.push(new Diagnostic(diagnostic.range, diagnostic.message, diagnostic.severity)); + } else { + this.outputChannel.error( + `[EmbeddedDiagnostics] Could not find language block for diagnostic at ` + + `[${diagnostic.range.start.line}, ${diagnostic.range.start.character}] ` + + `for ${session.language.ids[0]} in ${workspace.asRelativePath(session.documentUri)}` + ); + } + } + + session.diagnostics = mapped; + await this.disposeActiveVdoc(session, 'diagnostics-received'); + if (this.isDisposed) { + // Manager got disposed while disposing the vdoc. + return; + } + this.publishDiagnostics(session.documentUri); + } + + private publishDiagnostics(documentUri: Uri): void { + const allDiagnostics = this.getSessionsForDocument(documentUri) + .flatMap(s => s.diagnostics); + + this.diagnosticCollection.set(documentUri, allDiagnostics); + this._onDidUpdateDiagnostics.fire({ documentUri: documentUri, diagnostics: allDiagnostics }); + } + + // --- Helpers --- + + private getOrCreateSession(documentUri: Uri, language: EmbeddedLanguage): DiagnosticSession { + const docKey = documentUri.toString(); + const langKey = language.ids[0]; + + let langMap = this.sessionsByDocument.get(docKey); + if (!langMap) { + langMap = new Map(); + this.sessionsByDocument.set(docKey, langMap); + } + + let session = langMap.get(langKey); + if (!session) { + session = { + documentUri, + language, + languageBlocks: [], + diagnostics: [] + }; + langMap.set(langKey, session); + } + return session; + } + + private getSessionsForDocument(documentUri: Uri): DiagnosticSession[] { + const langMap = this.sessionsByDocument.get(documentUri.toString()); + return langMap ? [...langMap.values()] : []; + } + + private async removeSessionsForDocument(documentUri: Uri, reason: VdocDisposeReason): Promise { + const key = documentUri.toString(); + const langMap = this.sessionsByDocument.get(key); + if (!langMap) { return; } + + await Promise.allSettled( + [...langMap.values()].map(session => this.disposeActiveVdoc(session, reason)) + ); + this.sessionsByDocument.delete(key); + } + + private async disposeActiveVdoc(session: DiagnosticSession, reason: VdocDisposeReason): Promise { + const { activeVdoc } = session; + if (activeVdoc) { + // First unset the session's active vdoc so that we don't accidentally + // process diagnostics that arrive while we're cleaning up the old vdoc. + session.activeVdoc = undefined; + this.sessionByVdocUri.delete(activeVdoc.uri.toString()); + + await activeVdoc.cleanup(); + + this.outputChannel.debug( + `[EmbeddedDiagnostics] Disposed vdoc for ` + + `${session.language.ids[0]} in ${workspace.asRelativePath(session.documentUri)} ` + + `(reason: ${reason})` + ); + this._onDidDisposeVdoc.fire({ + documentUri: session.documentUri, + language: session.language.ids[0], + uri: activeVdoc.uri, + reason, + }); + } + } + + private shouldUseLocalTempFile(language: EmbeddedLanguage): boolean { + if (language.localTempFile) { + return true; + } + + // The vscode-R extension uses the languageserver R package + // which does not provide diagnostics for files in the system + // temp directory. Use a local temp file in that case. + if (language.ids.includes("r")) { + const rExt = extensions.getExtension("REditorSupport.r"); + if (rExt?.isActive) { + const rLspConfig = workspace.getConfiguration("r.lsp"); + if ( + rLspConfig.get("enabled", false) && + rLspConfig.get("diagnostics", false) + ) { + return true; + } + } + } + + // Default to a non-local temp file - it's less invasive. + return false; + } + + /** Settled when all active vdocs from dispose() have been cleaned up. */ + private _disposePromise?: Promise[]>; + + public override dispose(): void { + super.dispose(); + + for (const timer of this.debounceTimers.values()) { + clearTimeout(timer); + } + this.debounceTimers.clear(); + + const allSessions = [...this.sessionsByDocument.values()] + .flatMap(m => [...m.values()]); + + // Best-effort async cleanup — awaited via deactivate() during extension deactivation. + this._disposePromise = Promise.allSettled( + allSessions + .filter(s => s.activeVdoc) + .map(s => this.disposeActiveVdoc(s, 'session-removed')) + ); + + this.sessionsByDocument.clear(); + this.sessionByVdocUri.clear(); + } + + /** + * Awaitable cleanup for use during extension deactivation. + * Resolves when all active vdocs have been disposed (or failed). + */ + async deactivate(): Promise { + this.dispose(); + await this._disposePromise; + } +} + +export interface EmbeddedDiagnosticsService extends VscodeDisposable { + /** Awaitable cleanup for use during extension deactivation. */ + deactivate(): Promise; +} + +/** + * Activates cell diagnostics if enabled, and watches for setting changes + * to create/dispose the manager dynamically. + */ +export function activateEmbeddedDiagnostics( + engine: MarkdownEngine, + outputChannel: LogOutputChannel, +): EmbeddedDiagnosticsService { + let manager: EmbeddedDiagnosticsManager | undefined; + + function isEnabled(): boolean { + return workspace + .getConfiguration("quarto.cells.diagnostics") + .get("enabled", true); + } + + function createManager(): void { + if (!manager) { + manager = new EmbeddedDiagnosticsManager(engine, outputChannel); + } + } + + function disposeManager(): void { + if (manager) { + manager.dispose(); + manager = undefined; + } + } + + if (isEnabled()) { + createManager(); + } + + const configListener = workspace.onDidChangeConfiguration((e) => { + if (e.affectsConfiguration("quarto.cells.diagnostics.enabled")) { + if (isEnabled()) { + createManager(); + } else { + disposeManager(); + } + } + }); + + return { + dispose() { + configListener.dispose(); + disposeManager(); + }, + async deactivate() { + if (manager) { + await manager.deactivate(); + } + }, + }; +} diff --git a/apps/vscode/src/test/diagnosticFiltering.test.ts b/apps/vscode/src/test/diagnosticFiltering.test.ts deleted file mode 100644 index 673bcf7b7..000000000 --- a/apps/vscode/src/test/diagnosticFiltering.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import * as vscode from "vscode"; -import * as assert from "assert"; -import { createDiagnosticFilter } from "../lsp/client"; - -suite("Diagnostic Filtering", function () { - - test("Diagnostic filter removes diagnostics for virtual documents", async function () { - // Create mocks - const virtualDocUri = vscode.Uri.file("/tmp/.vdoc.12345678-1234-1234-1234-123456789abc.py"); - const regularDocUri = vscode.Uri.file("/tmp/regular-file.py"); - - // Create some test diagnostics - const testDiagnostics = [ - new vscode.Diagnostic( - new vscode.Range(0, 0, 0, 10), - "Test diagnostic message", - vscode.DiagnosticSeverity.Error - ) - ]; - - // Create a mock diagnostics handler function to verify behavior - let capturedUri: vscode.Uri | undefined; - let capturedDiagnostics: vscode.Diagnostic[] | undefined; - - const mockHandler = (uri: vscode.Uri, diagnostics: vscode.Diagnostic[]) => { - capturedUri = uri; - capturedDiagnostics = diagnostics; - }; - - // Create the filter function - const diagnosticFilter = createDiagnosticFilter(); - - // Test with a virtual document - diagnosticFilter(virtualDocUri, testDiagnostics, mockHandler); - - // Verify diagnostics were filtered (empty array) - assert.strictEqual(capturedUri, virtualDocUri, "URI should be passed through"); - assert.strictEqual(capturedDiagnostics!.length, 0, "Diagnostics should be empty for virtual documents"); - - // Reset captured values - capturedUri = undefined; - capturedDiagnostics = undefined; - - // Test with a regular document - diagnosticFilter(regularDocUri, testDiagnostics, mockHandler); - - // Verify diagnostics were not filtered - assert.strictEqual(capturedUri, regularDocUri, "URI should be passed through"); - assert.strictEqual(capturedDiagnostics!.length, testDiagnostics.length, "Diagnostics should not be filtered for regular documents"); - assert.deepStrictEqual(capturedDiagnostics!, testDiagnostics, "Original diagnostics should be passed through unchanged"); - }); - -}); diff --git a/apps/vscode/src/test/diagnostics.test.ts b/apps/vscode/src/test/diagnostics.test.ts new file mode 100644 index 000000000..844860b57 --- /dev/null +++ b/apps/vscode/src/test/diagnostics.test.ts @@ -0,0 +1,425 @@ +import * as assert from "assert"; +import * as vscode from "vscode"; +import { randomUUID } from "crypto"; +import { LanguageClient } from "vscode-languageclient/node"; +import { examplesUri, raceTimeout } from "./test-utils"; +import { testLanguageClient } from "./fixtures/test-language-client"; +import { DidUpdateDiagnosticsEvent, EmbeddedDiagnosticsManager, VdocDisposeReason } from "../providers/diagnostics"; +import { VIRTUAL_DOC_TEMP_DIRECTORY, deleteDocument, quartoVdocDir } from "../vdoc/vdoc-tempfile"; +import { MarkdownEngine } from "../markdown/engine"; +import { TestLogOutputChannel } from "./fixtures/test-log-output-channel"; +import { DisposableStore } from "core"; +import { eventToPromise, filterEvent } from "./utils/event"; +import { Uri } from "vscode"; + +/** Create a diagnostics manager for tests, registered with the given disposable store. */ +function createTestManager(disposables: DisposableStore, timeoutMs?: number) { + return disposables.add( + new EmbeddedDiagnosticsManager(new MarkdownEngine(), new TestLogOutputChannel(), timeoutMs) + ); +} + +suite("Diagnostics", function () { + const disposables = new DisposableStore(); + + /** Test docs to be deleted during teardown. See the note on {@link openExampleTextDocument} */ + const toDelete: vscode.TextDocument[] = []; + /** All vdoc URIs created during tests, to check for leaks during teardown. */ + const vdocUris: vscode.Uri[] = []; + let client: LanguageClient; + let manager: EmbeddedDiagnosticsManager; + + setup(async function () { + manager = createTestManager(disposables); + + // Track vdoc URIs for the leak check during teardown. + disposables.add(manager.onDidActivateVdoc((e) => { + vdocUris.push(e.uri); + })); + + // Start a test language server. + client = testLanguageClient(); + await client.start(); + }); + + teardown(async function () { + disposables.clear(); + await Promise.all(toDelete.map(doc => deleteDocument(doc))); + await client.stop(); + await vscode.commands.executeCommand("workbench.action.closeAllEditors"); + + // Check for leaked vdocs. + const leaked = (await Promise.all( + vdocUris.map(async (uri) => { + const exists = await vscode.workspace.fs.stat(uri).then(() => true, () => false); + return exists ? uri : null; + }) + )).filter((uri): uri is vscode.Uri => uri !== null); + assert.strictEqual( + leaked.length, + 0, + `Leaked vdocs:\n${leaked.map(u => u.fsPath).join("\n")}` + ); + vdocUris.length = 0; + }); + + test("receives diagnostics in the .qmd for embedded languages", async function () { + const { event } = await openAndAwaitDiagnostics(manager, "diagnostics-python-undefined.qmd", toDelete); + + assert.strictEqual(event.diagnostics.length, 1, "Expected one diagnostic"); + assert.strictEqual( + event.diagnostics[0].message, + "test-diagnostic: undefined_var is not defined", + "Expected diagnostic message to match" + ); + assert.strictEqual( + event.diagnostics[0].range.start.line, + 8, + `Diagnostic should be on line 8, got line ${event.diagnostics[0].range.start.line}` + ); + }); + + test("updates diagnostics when .qmd edited", async function () { + const { uri, event, doc } = await openAndAwaitDiagnostics(manager, "diagnostics-python-none.qmd", toDelete); + + assert.strictEqual( + event.diagnostics.length, + 0, + `Expected no initial diagnostics, got ${JSON.stringify(event.diagnostics)}` + ); + + const updated = nextDiagnostics(manager, uri); + const editor = await vscode.window.showTextDocument(doc); + await editor.edit((editBuilder) => { + editBuilder.insert(new vscode.Position(0, 0), "```{python}\nundefined_var\n```\n"); + }); + const updatedEvent = await raceTimeout(updated, 3000); + assert.ok(updatedEvent, "Timed out waiting for updated diagnostics"); + + assert.strictEqual( + updatedEvent.diagnostics.length, + 1, + `Expected one diagnostic after adding a cell, got ${updatedEvent.diagnostics.length}` + ); + }); + + test("receives diagnostics for multiple languages independently", async function () { + this.timeout(15000); + + const doc = await openExampleTextDocument("diagnostics-multilang.qmd", toDelete); + const uri = doc.uri; + + // Subscribe before showing so we don't miss events fired during document open. + const events: DidUpdateDiagnosticsEvent[] = []; + const gotBoth = new Promise((resolve) => { + const listener = manager.onDidUpdateDiagnostics((e) => { + if (isUriEqual(e.documentUri, uri)) { + events.push(e); + if (events.length >= 2) { + listener.dispose(); + resolve(true); + } + } + }); + }); + + await vscode.window.showTextDocument(doc); + + const result = await raceTimeout(gotBoth, 12000); + assert.strictEqual(result, true, "Timed out waiting for multi-language diagnostics"); + + // The final published diagnostics should contain entries from both languages. + const finalDiagnostics = vscode.languages.getDiagnostics(uri); + assert.ok( + finalDiagnostics.length >= 2, + `Expected at least 2 diagnostics (one per language), got ${finalDiagnostics.length}` + ); + }); + + test("times out for unresponsive language servers without blocking others", async function () { + // Julia has no language server registered in tests, so it will time out. + // Python should still get its diagnostics independently. + const { event } = await openAndAwaitDiagnostics(manager, "diagnostics-timeout.qmd", toDelete); + + // Python diagnostics should be present despite Julia timing out. + assert.ok( + event.diagnostics.length >= 1, + `Expected at least 1 diagnostic from Python, got ${event.diagnostics.length}` + ); + assert.ok( + event.diagnostics.some(d => d.message.includes("undefined_var")), + "Expected Python diagnostic about undefined_var" + ); + }); + + test("cleans up vdoc after timeout when language server does not respond", async function () { + const shortTimeoutManager = createTestManager(disposables, 200); + const doc = await openExampleTextDocument("diagnostics-julia-only.qmd", toDelete); + + const disposal = nextVdocDisposal(shortTimeoutManager, "timeout", "julia"); + await vscode.window.showTextDocument(doc); + + const result = await raceTimeout(disposal, 2000); + assert.ok(result, "Expected Julia vdoc to be disposed via timeout"); + + const exists = await vscode.workspace.fs.stat(result.uri).then(() => true, () => false); + assert.strictEqual(exists, false, "Expected vdoc file to be deleted after timeout"); + }); + + test("clears stale diagnostics after timeout", async function () { + const shortTimeoutManager = createTestManager(disposables, 200); + + const { uri, doc } = await openAndAwaitDiagnostics( + shortTimeoutManager, "diagnostics-timeout.qmd", toDelete + ); + assert.ok(vscode.languages.getDiagnostics(uri).length >= 1, "Should have Python diagnostics initially"); + + // Wait for the initial Julia timeout before editing, + // otherwise nextDiagnostics catches that event instead. + await raceTimeout(nextVdocDisposal(shortTimeoutManager, "timeout", "julia"), 2000); + + // Delete the Python cell, keeping only Julia (which will timeout). + const cleared = nextDiagnostics(shortTimeoutManager, uri); + const editor = await vscode.window.showTextDocument(doc); + await editor.edit((editBuilder) => { + editBuilder.delete( + new vscode.Range( + new vscode.Position(11, 0), + new vscode.Position(doc.lineCount, 0) + ) + ); + }); + + const event = await raceTimeout(cleared, 3000); + assert.ok(event, "Expected diagnostics update after timeout"); + assert.strictEqual(event.diagnostics.length, 0, + "Stale Python diagnostics should be cleared after Julia-only timeout"); + }); + + test("clears diagnostics when error is fixed", async function () { + const { uri, doc } = await openAndAwaitDiagnostics(manager, "diagnostics-python-undefined.qmd", toDelete); + + const cleared = nextDiagnostics(manager, uri); + const editor = await vscode.window.showTextDocument(doc); + const line = doc.lineAt(8); + await editor.edit((editBuilder) => { + editBuilder.replace(line.range, "x = 0"); + }); + const event = await raceTimeout(cleared, 4000); + assert.ok(event, "Timed out waiting for diagnostics to clear"); + + assert.strictEqual( + event.diagnostics.length, + 0, + "Diagnostics should be cleared after fixing the error" + ); + }); + + test("cleans up vdoc after diagnostics are received", async function () { + const doc = await openExampleTextDocument("diagnostics-python-undefined.qmd", toDelete); + const disposal = nextVdocDisposal(manager, "diagnostics-received", "python"); + await vscode.window.showTextDocument(doc); + + const result = await raceTimeout(disposal, 4000); + assert.ok(result, "Expected Python vdoc to be disposed after diagnostics received"); + + const exists = await vscode.workspace.fs.stat(result.uri).then(() => true, () => false); + assert.strictEqual(exists, false, "Expected vdoc file to be deleted after diagnostics received"); + }); + + test("cleans up vdoc when document is closed", async function () { + // Julia (no LS in tests) so the vdoc stays alive long enough to be + // disposed by closing the document rather than by receiving diagnostics. + const doc = await openExampleTextDocument("diagnostics-julia-only.qmd", toDelete); + const disposal = nextVdocDisposal(manager, "session-removed", "julia"); + + await vscode.window.showTextDocument(doc); + await vscode.languages.setTextDocumentLanguage(doc, "plaintext"); + await vscode.commands.executeCommand("workbench.action.closeAllEditors"); + + const result = await raceTimeout(disposal, 4000); + assert.ok(result, "Expected vdoc to be disposed when document is closed"); + + const exists = await vscode.workspace.fs.stat(result.uri).then(() => true, () => false); + assert.strictEqual(exists, false, "Expected vdoc file to be deleted after document close"); + }); + + test("reports diagnostics from multiple cells of the same language", async function () { + const { event } = await openAndAwaitDiagnostics(manager, "diagnostics-python-multicell.qmd", toDelete); + + const diagnostics = event.diagnostics; + assert.strictEqual(diagnostics.length, 2, "Expected one diagnostic per cell"); + + const lines = diagnostics.map(d => d.range.start.line).sort((a, b) => a - b); + assert.deepStrictEqual( + lines, + [8, 14], + `Expected diagnostics on lines 8 and 14, got ${JSON.stringify(lines)}` + ); + }); + + test("maps diagnostic line numbers correctly with content above cell", async function () { + const { event } = await openAndAwaitDiagnostics(manager, "diagnostics-python-offset.qmd", toDelete); + + const diagnostics = event.diagnostics; + assert.strictEqual(diagnostics.length, 1, "Expected one diagnostic"); + assert.strictEqual( + diagnostics[0].range.start.line, + 13, + `Diagnostic should be on line 13 (after extra content), got line ${diagnostics[0].range.start.line}` + ); + }); + + test("clears diagnostics when all executable cells are removed", async function () { + const { uri, doc } = await openAndAwaitDiagnostics(manager, "diagnostics-python-undefined.qmd", toDelete); + + // Remove the entire code cell, leaving only markdown. + const cleared = nextDiagnostics(manager, uri); + const editor = await vscode.window.showTextDocument(doc); + const fullRange = new vscode.Range( + new vscode.Position(7, 0), + new vscode.Position(doc.lineCount, 0) + ); + await editor.edit((editBuilder) => { + editBuilder.replace(fullRange, "No code here.\n"); + }); + const event = await raceTimeout(cleared, 4000); + assert.ok(event, "Timed out waiting for diagnostics to clear after removing all cells"); + + assert.strictEqual( + event.diagnostics.length, + 0, + "Diagnostics should be cleared when no executable cells remain" + ); + }); + + test("clears diagnostics when document is closed", async function () { + const { uri, doc } = await openAndAwaitDiagnostics(manager, "diagnostics-python-offset.qmd", toDelete); + + const cleared = nextDiagnostics(manager, uri); + await deleteDocument(doc); + const event = await raceTimeout(cleared, 4000); + assert.ok(event, "Timed out waiting for diagnostics to clear on close"); + + assert.strictEqual( + event.diagnostics.length, + 0, + "Diagnostics should be cleared after closing the document" + ); + }); + + suite("vdoc location", () => { + test("places typescript vdoc in local directory", async function () { + const { uri, event } = await openAndAwaitVdocActivation(manager, "diagnostics-typescript.qmd", "ts", toDelete); + const expectedDir = quartoVdocDir(uri.fsPath); + assert.ok( + event.uri.fsPath.startsWith(expectedDir), + `Expected TypeScript vdoc in local dir (${expectedDir}), got ${event.uri.fsPath}` + ); + }); + + test("places python vdoc in global temp directory", async function () { + const { event } = await openAndAwaitVdocActivation(manager, "diagnostics-python-undefined.qmd", "python", toDelete); + assert.ok( + event.uri.fsPath.startsWith(VIRTUAL_DOC_TEMP_DIRECTORY), + `Expected Python vdoc in global temp dir (${VIRTUAL_DOC_TEMP_DIRECTORY}), got ${event.uri.fsPath}` + ); + }); + }); +}); + +/** + * Copy a fixture file to a unique URI and open it. + * + * VS Code keeps text documents in memory even after their editors are closed, + * so a fixture opened by one test remains in `workspace.textDocuments` for + * subsequent tests. When `EmbeddedDiagnosticsManager` is constructed it + * notifies the language server about all already-open documents. + * + * Copying to a fresh URI guarantees the document has never been seen before, + * and lets us delete it to fire onDidCloseTextDocument events. + */ +async function openExampleTextDocument(fixture: string, toDelete: vscode.TextDocument[]): Promise { + const source = examplesUri(fixture); + const dest = Uri.joinPath(source, "..", `tmp-${randomUUID()}-${fixture}`); + await vscode.workspace.fs.copy(source, dest); + const doc = await vscode.workspace.openTextDocument(dest); + toDelete.push(doc); + return doc; +} + +function isUriEqual(a: vscode.Uri, b: vscode.Uri) { + return a.toString() === b.toString(); +} + +/** + * Subscribe to the next diagnostics event for a URI. + * Call before the triggering action. + */ +function nextDiagnostics(manager: EmbeddedDiagnosticsManager, uri: vscode.Uri) { + return eventToPromise( + filterEvent( + manager.onDidUpdateDiagnostics, + (e) => isUriEqual(e.documentUri, uri) + ) + ); +} + +/** + * Subscribe to the next vdoc disposal event matching reason and language. + * Call before the triggering action. + */ +function nextVdocDisposal( + manager: EmbeddedDiagnosticsManager, + reason: VdocDisposeReason, + language: string +) { + return eventToPromise( + filterEvent( + manager.onDidDisposeVdoc, + (e) => e.reason === reason && e.language === language + ) + ); +} + +/** + * Subscribe to the next vdoc activation event matching a language. + * Call before the triggering action. + */ +function nextVdocActivation( + manager: EmbeddedDiagnosticsManager, + documentUri: Uri, + language: string +) { + return eventToPromise( + filterEvent( + manager.onDidActivateVdoc, + (e) => isUriEqual(e.documentUri, documentUri) && + e.language === language + ) + ); +} + +/** Open a .qmd fixture and wait for its first diagnostics event. */ +async function openAndAwaitDiagnostics(manager: EmbeddedDiagnosticsManager, fixture: string, toDelete: vscode.TextDocument[]) { + const doc = await openExampleTextDocument(fixture, toDelete); + const diagnostics = nextDiagnostics(manager, doc.uri); + await vscode.window.showTextDocument(doc); + const event = await raceTimeout(diagnostics, 4000); + if (!event) { + throw new Error(`Timed out waiting for diagnostics on ${fixture}`); + } + return { uri: doc.uri, event, doc }; +} + +/** Open a .qmd fixture and wait for its virtual document to activate for a given language. */ +async function openAndAwaitVdocActivation(manager: EmbeddedDiagnosticsManager, fixture: string, language: string, toDelete: vscode.TextDocument[]) { + const doc = await openExampleTextDocument(fixture, toDelete); + const activation = nextVdocActivation(manager, doc.uri, language); + await vscode.window.showTextDocument(doc); + const event = await raceTimeout(activation, 4000); + if (!event) { + throw new Error(`Timed out waiting for vdoc activation for ${language} in ${fixture}`); + } + return { uri: doc.uri, event, doc }; +} diff --git a/apps/vscode/src/test/examples/.vscode/settings.json b/apps/vscode/src/test/examples/.vscode/settings.json index 02a3041cd..daf7962c2 100644 --- a/apps/vscode/src/test/examples/.vscode/settings.json +++ b/apps/vscode/src/test/examples/.vscode/settings.json @@ -1,3 +1,6 @@ { - "quarto.symbols.exportToWorkspace": "default" + "quarto.symbols.exportToWorkspace": "default", + // Disable the main diagnostics manager during tests, + // we test against a test-specific instance instead. + "quarto.cells.diagnostics.enabled": false } diff --git a/apps/vscode/src/test/examples/diagnostics-julia-only.qmd b/apps/vscode/src/test/examples/diagnostics-julia-only.qmd new file mode 100644 index 000000000..70fc2ba02 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-julia-only.qmd @@ -0,0 +1,8 @@ +--- +title: "Julia only (no language server)" +format: html +--- + +```{julia} +undefined_var = 1 +``` diff --git a/apps/vscode/src/test/examples/diagnostics-multilang.qmd b/apps/vscode/src/test/examples/diagnostics-multilang.qmd new file mode 100644 index 000000000..5166db55f --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-multilang.qmd @@ -0,0 +1,16 @@ +--- +title: "Multi-language diagnostics" +format: html +--- + +## Python + +```{python} +x = undefined_var +``` + +## R + +```{r} +y <- undefined_var +``` diff --git a/apps/vscode/src/test/examples/diagnostics-python-multicell.qmd b/apps/vscode/src/test/examples/diagnostics-python-multicell.qmd new file mode 100644 index 000000000..608b01ba3 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-python-multicell.qmd @@ -0,0 +1,16 @@ +--- +title: "Multi-cell test" +format: html +--- + +## First cell + +```{python} +x = undefined_var +``` + +## Second cell + +```{python} +y = undefined_var +``` diff --git a/apps/vscode/src/test/examples/diagnostics-python-none.qmd b/apps/vscode/src/test/examples/diagnostics-python-none.qmd new file mode 100644 index 000000000..4f361a167 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-python-none.qmd @@ -0,0 +1,10 @@ +--- +title: "Diagnostics test" +format: html +--- + +## Code + +```{python} +x = 0 +``` diff --git a/apps/vscode/src/test/examples/diagnostics-python-offset.qmd b/apps/vscode/src/test/examples/diagnostics-python-offset.qmd new file mode 100644 index 000000000..925cc2b73 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-python-offset.qmd @@ -0,0 +1,15 @@ +--- +title: "Line offset test" +format: html +--- + +## Extra content above the code cell + +This paragraph adds lines above the cell so that `undefined_var` +appears at a different line offset than in simpler fixtures. + +More lines to push the cell down. + +```{python} +x = undefined_var +``` diff --git a/apps/vscode/src/test/examples/diagnostics-python-undefined.qmd b/apps/vscode/src/test/examples/diagnostics-python-undefined.qmd new file mode 100644 index 000000000..063ccc9d1 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-python-undefined.qmd @@ -0,0 +1,10 @@ +--- +title: "Diagnostics test" +format: html +--- + +## Code + +```{python} +x = undefined_var +``` diff --git a/apps/vscode/src/test/examples/diagnostics-timeout.qmd b/apps/vscode/src/test/examples/diagnostics-timeout.qmd new file mode 100644 index 000000000..9762876f7 --- /dev/null +++ b/apps/vscode/src/test/examples/diagnostics-timeout.qmd @@ -0,0 +1,16 @@ +--- +title: "Timeout test" +format: html +--- + +## Julia (no language server registered) + +```{julia} +undefined_var = 1 +``` + +## Python (has language server) + +```{python} +x = undefined_var +``` diff --git a/apps/vscode/src/test/fixtures/test-language-client.ts b/apps/vscode/src/test/fixtures/test-language-client.ts new file mode 100644 index 000000000..86cd736bc --- /dev/null +++ b/apps/vscode/src/test/fixtures/test-language-client.ts @@ -0,0 +1,30 @@ +import path from "node:path"; +import { LanguageClient, LanguageClientOptions, ServerOptions, TransportKind } from "vscode-languageclient/node"; +import { TestLogOutputChannel } from "./test-log-output-channel"; + +/** + * A {@link LanguageClient} for testing, which connects to `test-language-server.js`. + */ +export function testLanguageClient(): LanguageClient { + const serverModule = path.join(__dirname, "test-language-server.js"); + + const serverOptions: ServerOptions = { + run: { module: serverModule, transport: TransportKind.ipc }, + debug: { module: serverModule, transport: TransportKind.ipc }, + }; + + const clientOptions: LanguageClientOptions = { + documentSelector: [ + { language: "python" }, + { language: "r" }, + ], + outputChannel: new TestLogOutputChannel("Test Language Client"), + }; + + return new LanguageClient( + "test-language-server", + "Test Language Server", + serverOptions, + clientOptions + ); +} diff --git a/apps/vscode/src/test/fixtures/test-language-server.ts b/apps/vscode/src/test/fixtures/test-language-server.ts new file mode 100644 index 000000000..4b4f91dde --- /dev/null +++ b/apps/vscode/src/test/fixtures/test-language-server.ts @@ -0,0 +1,80 @@ +import { + createConnection, + DiagnosticSeverity, + TextDocuments, +} from "vscode-languageserver/node"; +import { TextDocument } from "vscode-languageserver-textdocument"; + +/** + * This module defines a language server for testing. + */ + +const undefinedVarRegExp = /undefined_var/; + +const connection = createConnection(); +const documents = new TextDocuments(TextDocument); +const { console } = connection; + +/** + * Publish diagnostics for a text document. + */ +function publishDiagnostics(document: TextDocument) { + // Get the document's lines. + const allText = document.getText(); + const lines = allText.split("\n"); + + // Find instances of "undefined_var" and create diagnostics for them. + const diagnostics = []; + for (const [line, text] of lines.entries()) { + const match = text.match(undefinedVarRegExp); + if (match && match.index !== undefined) { + diagnostics.push({ + range: { + start: { line, character: match.index }, + end: { line, character: match.index + match[0].length }, + }, + message: "test-diagnostic: undefined_var is not defined", + severity: DiagnosticSeverity.Warning, + }); + } + } + + // Publish the diagnostics to the client. + console.log(`Publishing ${diagnostics.length} diagnostics for ${document.uri}\n` + + diagnostics.map(d => `- ${d.message} at [${d.range.start.line}, ${d.range.start.character}]`).join("\n") + ); + connection.sendDiagnostics({ uri: document.uri, diagnostics }); +} + +// Initialize the server. +connection.onInitialize(() => { + console.log(`Initialized!`); + return { + capabilities: {}, + }; +}); + +// Publish diagnostics on document open. +documents.onDidOpen(({ document }) => { + console.log(`Document opened: ${document.uri}`); + publishDiagnostics(document); +}); + +// Publish diagnostics on document change. +documents.onDidChangeContent(({ document }) => { + console.log(`Document changed: ${document.uri}`); + publishDiagnostics(document); +}); + +// Clear diagnostics on document close. +documents.onDidClose(({ document }) => { + console.log(`Document closed: ${document.uri}`); + console.log(`Publishing 0 diagnostics for ${document.uri}`); + connection.sendDiagnostics({ uri: document.uri, diagnostics: [] }); +}); + +// Connect the text document manager. +documents.listen(connection); + +// Listen on the connection. +connection.listen(); diff --git a/apps/vscode/src/test/fixtures/test-log-output-channel.ts b/apps/vscode/src/test/fixtures/test-log-output-channel.ts new file mode 100644 index 000000000..cecae9403 --- /dev/null +++ b/apps/vscode/src/test/fixtures/test-log-output-channel.ts @@ -0,0 +1,31 @@ +import { EventEmitter, LogLevel, LogOutputChannel } from "vscode"; + +/** + * A {@link LogOutputChannel} that logs to the console. + * Quiet by default when run by Claude Code; set `VERBOSE=1` to override. + */ +export class TestLogOutputChannel implements LogOutputChannel { + logLevel: LogLevel = (process.env.CLAUDE_CODE && !process.env.VERBOSE) + ? LogLevel.Off + : LogLevel.Trace; + onDidChangeLogLevel = new EventEmitter().event; + + constructor(public readonly name = "") { } + + append(value: string) { + if (this.logLevel !== LogLevel.Off) { + console.log(this.name ? `[${this.name}] ${value}` : value); + } + } + appendLine(value: string) { this.append(value); } + clear() { } + show() { } + hide() { } + dispose() { } + replace(_value: any) { } + trace(value: string) { if (this.logLevel <= LogLevel.Trace) { this.append(value); } } + debug(value: string) { if (this.logLevel <= LogLevel.Debug) { this.append(value); } } + info(value: string) { if (this.logLevel <= LogLevel.Info) { this.append(value); } } + warn(value: string) { if (this.logLevel <= LogLevel.Warning) { this.append(value); } } + error(value: string) { if (this.logLevel <= LogLevel.Error) { this.append(value); } } +} diff --git a/apps/vscode/src/test/test-utils.ts b/apps/vscode/src/test/test-utils.ts index 62ef1b450..152a18c67 100644 --- a/apps/vscode/src/test/test-utils.ts +++ b/apps/vscode/src/test/test-utils.ts @@ -15,7 +15,7 @@ export const TEST_PATH = path.join(EXTENSION_ROOT_DIR, "src", "test"); export const WORKSPACE_PATH = path.join(TEST_PATH, "examples"); export const WORKSPACE_OUT_PATH = path.join(TEST_PATH, "examples-out"); -function examplesUri(fileName: string = ''): vscode.Uri { +export function examplesUri(fileName: string = ''): vscode.Uri { return vscode.Uri.file(path.join(WORKSPACE_PATH, fileName)); } export function examplesOutUri(fileName: string = ''): vscode.Uri { @@ -34,7 +34,7 @@ export async function openAndShowExamplesOutTextDocument(fileName: string) { return openAndShowUri(examplesOutUri(fileName)); } -async function openAndShowUri(uri: vscode.Uri) { +export async function openAndShowUri(uri: vscode.Uri) { const doc = await vscode.workspace.openTextDocument(uri); const editor = await vscode.window.showTextDocument(doc); return { doc, editor }; @@ -83,3 +83,18 @@ ${RESET_COLOR_ESCAPE_CODE}`); return content; } } + +/** + * Races a promise against a timeout, returning `undefined` if + * the timeout is reached before the promise resolves. + */ +export async function raceTimeout(promise: Promise, ms: number): Promise { + let timeout: NodeJS.Timeout; + const timeoutPromise = new Promise((resolve) => { + timeout = setTimeout(() => resolve(undefined), ms); + }); + return Promise.race([ + promise.finally(() => clearTimeout(timeout)), + timeoutPromise + ]); +} diff --git a/apps/vscode/src/test/utils/event.ts b/apps/vscode/src/test/utils/event.ts new file mode 100644 index 000000000..e7273b548 --- /dev/null +++ b/apps/vscode/src/test/utils/event.ts @@ -0,0 +1,40 @@ +import { Event } from "vscode"; + +export function filterEvent( + event: Event, + filter: (e: T) => boolean, +): Event { + return (listener, thisArgs?, disposables?) => { + return event((e) => { + if (filter(e)) { + listener.call(thisArgs, e); + } + }, null, disposables); + }; +} + +export function onceEvent(event: Event): Event { + return (listener, thisArgs?, disposables?) => { + const result = event(e => { + result.dispose(); + return listener.call(thisArgs, e); + }, null, disposables); + + return result; + }; +} + +export function debounceEvent(event: Event, delay: number): Event { + return (listener, thisArgs?, disposables?) => { + let timer: NodeJS.Timeout; + return event(e => { + clearTimeout(timer); + timer = setTimeout(() => listener.call(thisArgs, e), delay); + }, null, disposables); + }; +} + +export function eventToPromise(event: Event): Promise { + const once = onceEvent(event); + return new Promise(resolve => once(e => resolve(e))); +} diff --git a/apps/vscode/src/vdoc/languages.ts b/apps/vscode/src/vdoc/languages.ts index dbce368b6..8c999895a 100644 --- a/apps/vscode/src/vdoc/languages.ts +++ b/apps/vscode/src/vdoc/languages.ts @@ -23,6 +23,11 @@ export interface EmbeddedLanguage { emptyLine?: string; comment?: string; trigger?: string[]; + /** + * Lines of code to inject at the top of the virtual document. + * Used to disable diagnostics for virtual documents that were + * created for non-diagnostic actions. + */ inject?: string[]; canFormat?: boolean; canFormatDocument?: boolean; @@ -88,6 +93,11 @@ interface LanguageOptions { type?: "content" | "tempfile"; localTempFile?: boolean; emptyLine?: string; + /** + * Lines of code to inject at the top of the virtual document. + * Used to disable diagnostics for virtual documents that were + * created for non-diagnostic actions. + */ inject?: string[]; canFormat?: boolean; canFormatDocument?: boolean; diff --git a/apps/vscode/src/vdoc/vdoc-tempfile.ts b/apps/vscode/src/vdoc/vdoc-tempfile.ts index 0a0bb3374..8742917c0 100644 --- a/apps/vscode/src/vdoc/vdoc-tempfile.ts +++ b/apps/vscode/src/vdoc/vdoc-tempfile.ts @@ -21,44 +21,37 @@ import * as uuid from "uuid"; import { commands, Hover, + languages, Position, TextDocument, Uri, workspace, - WorkspaceEdit, } from "vscode"; import { VirtualDoc, VirtualDocUri } from "./vdoc"; +interface VirtualDocTempFileOptions { + /** Fire a hover request to prime the language server before returning. */ + warmup: boolean; +} + /** - * Create an on disk temporary file containing the contents of the virtual document + * Create an on-disk temporary file for a virtual document and open it. * - * @param virtualDoc The document to use when populating the temporary file - * @param docPath The path to the original document the virtual document is - * based on. When `local` is `true`, this is used to determine the directory - * to create the temporary file in. - * @param local Whether or not the temporary file should be created "locally" in - * the workspace next to `docPath` or in a temporary directory outside the - * workspace. - * @returns A `VirtualDocUri` + * @param virtualDoc The virtual document content to write. + * @param directory The directory to create the file in. */ export async function virtualDocUriFromTempFile( virtualDoc: VirtualDoc, - docPath: string, - local: boolean + directory: string, + options: VirtualDocTempFileOptions, ): Promise { - const useLocal = local || virtualDoc.language.localTempFile; - - // If `useLocal`, then create the temporary document alongside the `docPath` - // so tools like formatters have access to workspace configuration. Otherwise, - // create it in a temp directory. - const virtualDocFilepath = useLocal - ? createVirtualDocLocalFile(virtualDoc, path.dirname(docPath)) - : createVirtualDocTempfile(virtualDoc); + const filepath = generateVirtualDocFilepath(directory, virtualDoc.language.extension); + createVirtualDoc(filepath, virtualDoc.content); - const virtualDocUri = Uri.file(virtualDocFilepath); + const virtualDocUri = Uri.file(filepath); const virtualDocTextDocument = await workspace.openTextDocument(virtualDocUri); - if (!useLocal) { + if (options.warmup) { // TODO: Reevaluate whether this is necessary. Old comment: // > if this is the first time getting a virtual doc for this // > language then execute a dummy request to cause it to load @@ -69,7 +62,7 @@ export async function virtualDocUriFromTempFile( ); } - return { + return { uri: virtualDocTextDocument.uri, cleanup: async () => await deleteDocument(virtualDocTextDocument), }; @@ -85,45 +78,53 @@ export async function virtualDocUriFromTempFile( * * @param doc The `TextDocument` to delete */ -async function deleteDocument(doc: TextDocument) { +export async function deleteDocument(doc: TextDocument) { try { + // First set the language to 'plaintext' so that the language client + // closes the text document in the language server, which clears + // diagnostics for the file. This stops diagnostics from building + // up even after virtual docs are cleaned up. + // + // Unfortunately, workspace.fs.delete does not trigger the + // vscode.window.onDidCloseTextDocument event, which the language + // client relies on to send the textDocument/didClose notification + // to the language server. + // + // vscode.WorkspaceEdit *does* trigger onDidCloseTextDocument, + // but doesn't support skipping the trash - see the note above + // re #708. + await languages.setTextDocumentLanguage(doc, "plaintext"); + await workspace.fs.delete(doc.uri, { useTrash: false }); } catch (error) { + // It's okay if the file is already deleted. + if (error instanceof Error && error.message.includes("ENOENT")) { + return; + } const msg = error instanceof Error ? error.message : JSON.stringify(error); - console.log(`Error removing vdoc at ${doc.fileName}: ${msg}`); + console.error(`Error removing vdoc at ${doc.fileName}: ${msg}`); } } tmp.setGracefulCleanup(); -const VIRTUAL_DOC_TEMP_DIRECTORY = tmp.dirSync().name; +export const VIRTUAL_DOC_TEMP_DIRECTORY = tmp.dirSync().name; /** - * Creates a virtual document in a temporary directory - * - * The temporary directory is automatically cleaned up on process exit. + * Resolves the `.quarto/vdoc/` directory for the workspace containing `docPath`. * - * @param virtualDoc The document to use when populating the temporary file - * @returns The path to the temporary file + * Falls back to the source file's directory if no workspace folder is found + * (e.g., when working with a single file outside a workspace). */ -function createVirtualDocTempfile(virtualDoc: VirtualDoc): string { - const filepath = generateVirtualDocFilepath(VIRTUAL_DOC_TEMP_DIRECTORY, virtualDoc.language.extension); - createVirtualDoc(filepath, virtualDoc.content); - return filepath; -} - -/** - * Creates a virtual document in the provided directory - * - * @param virtualDoc The document to use when populating the temporary file - * @param directory The directory to create the temporary file in - * @returns The path to the temporary file - */ -function createVirtualDocLocalFile(virtualDoc: VirtualDoc, directory: string): string { - const filepath = generateVirtualDocFilepath(directory, virtualDoc.language.extension); - createVirtualDoc(filepath, virtualDoc.content); - return filepath; +export function quartoVdocDir(docPath: string): string { + const sourceDirectory = path.dirname(docPath); + const workspaceFolder = workspace.workspaceFolders?.find( + (folder) => sourceDirectory.startsWith(folder.uri.fsPath) + ); + return workspaceFolder + ? Uri.joinPath(workspaceFolder.uri, ".quarto", "vdoc").fsPath + : sourceDirectory; } /** @@ -133,7 +134,7 @@ function createVirtualDoc(filepath: string, content: string): void { const directory = path.dirname(filepath); if (!fs.existsSync(directory)) { - fs.mkdirSync(directory); + fs.mkdirSync(directory, { recursive: true }); } fs.writeFileSync(filepath, content); @@ -150,14 +151,3 @@ function createVirtualDoc(filepath: string, content: string): void { function generateVirtualDocFilepath(directory: string, extension: string): string { return path.join(directory, ".vdoc." + uuid.v4() + "." + extension); } - -export function isVirtualDoc(uri: Uri): boolean { - // Check for tempfile virtual docs - if (uri.scheme === "file") { - const filename = path.basename(uri.fsPath); - // Virtual docs have a specific filename pattern .vdoc.[uuid].[extension] - return filename.startsWith(".vdoc.") && filename.split(".").length > 3; - } - - return false; -} diff --git a/apps/vscode/src/vdoc/vdoc.ts b/apps/vscode/src/vdoc/vdoc.ts index c17720e45..3cd500b86 100644 --- a/apps/vscode/src/vdoc/vdoc.ts +++ b/apps/vscode/src/vdoc/vdoc.ts @@ -13,6 +13,7 @@ * */ +import * as path from "path"; import { Position, TextDocument, Uri, Range, SemanticTokens } from "vscode"; import { Token, isExecutableLanguageBlock, languageBlockAtPosition, languageNameFromBlock } from "quarto-core"; @@ -20,7 +21,7 @@ import { isQuartoDoc } from "../core/doc"; import { MarkdownEngine } from "../markdown/engine"; import { embeddedLanguage, EmbeddedLanguage } from "./languages"; import { virtualDocUriFromEmbeddedContent } from "./vdoc-content"; -import { virtualDocUriFromTempFile } from "./vdoc-tempfile"; +import { virtualDocUriFromTempFile, VIRTUAL_DOC_TEMP_DIRECTORY } from "./vdoc-tempfile"; import { decodeSemanticTokens, encodeSemanticTokens } from "../providers/semantic-tokens"; export interface VirtualDoc { @@ -29,10 +30,12 @@ export interface VirtualDoc { } export enum VirtualDocStyle { - /// Every block corresponding to the current position's language + /** Every block corresponding to the current position's language */ + // eslint-disable-next-line @typescript-eslint/naming-convention Language, - /// Only the block corresponding to the current position + /** Only the block corresponding to the current position */ + // eslint-disable-next-line @typescript-eslint/naming-convention Block } @@ -84,14 +87,15 @@ function virtualDocForBlock(document: TextDocument, block: Token, language: Embe export function virtualDocForLanguage( document: TextDocument, tokens: Token[], - language: EmbeddedLanguage + language: EmbeddedLanguage, + action?: VirtualDocAction, ): VirtualDoc { const lines = linesForLanguage(document, language); for (const languageBlock of tokens.filter(isBlockOfLanguage(language))) { fillLinesFromBlock(lines, document, languageBlock); } padLinesForLanguage(lines, language); - return virtualDocForCode(lines, language); + return virtualDocForCode(lines, language, action); } function linesForLanguage(document: TextDocument, language: EmbeddedLanguage) { @@ -118,11 +122,16 @@ function padLinesForLanguage(lines: string[], language: EmbeddedLanguage) { } } -export function virtualDocForCode(code: string[], language: EmbeddedLanguage) { +export function virtualDocForCode( + code: string[], + language: EmbeddedLanguage, + action?: VirtualDocAction, +) { const lines = [...code]; - if (language.inject) { + // For non-diagnostic actions, inject lines of code to disable diagnostics. + if (language.inject && action !== "diagnostics") { lines.unshift(...language.inject); } @@ -141,7 +150,8 @@ export type VirtualDocAction = "statementRange" | "helpTopic" | "executeSelectionAtPositionInteractive" | - "semanticTokens"; + "semanticTokens" | + "diagnostics"; export type VirtualDocUri = { uri: Uri, cleanup?: () => Promise; }; @@ -183,13 +193,16 @@ async function virtualDocUri( action: VirtualDocAction ): Promise { - // format and definition actions use a transient local vdoc - // (so they can get project-specific paths and formatting config) - const local = ["format", "definition"].includes(action); + if (virtualDoc.language.type === "content") { + return { uri: virtualDocUriFromEmbeddedContent(virtualDoc, parentUri) }; + } + + // format and definition actions use a local vdoc alongside the source + // so tools like formatters have access to workspace configuration + const local = ["format", "definition"].includes(action) || virtualDoc.language.localTempFile; + const dir = local ? path.dirname(parentUri.fsPath) : VIRTUAL_DOC_TEMP_DIRECTORY; - return virtualDoc.language.type === "content" - ? { uri: virtualDocUriFromEmbeddedContent(virtualDoc, parentUri) } - : await virtualDocUriFromTempFile(virtualDoc, parentUri.fsPath, local); + return await virtualDocUriFromTempFile(virtualDoc, dir, { warmup: !local }); } export function languageAtPosition(tokens: Token[], position: Position) { diff --git a/claude.md b/claude.md index b96a6f5cb..26b56e319 100644 --- a/claude.md +++ b/claude.md @@ -51,6 +51,8 @@ The turborepo pipeline helps optimize build times by caching build artifacts and Testing procedures vary by component: - VS Code extension: Run `yarn test-vscode [--label 'label' --grep 'pattern']` to compile test files and run them with the vscode-test CLI + - Test log output is suppressed automatically when run by Claude Code; set `VERBOSE=1` when debugging failures + - The output is small enough to read directly — don't pipe through `tail` or `grep` - Read the [test configuration file](./apps/vscode/.vscode-test.mjs) for valid labels - Other components have specific test commands defined in their respective package.json files diff --git a/packages/core/src/dispose.ts b/packages/core/src/dispose.ts index 2cef98eab..a31463d6b 100644 --- a/packages/core/src/dispose.ts +++ b/packages/core/src/dispose.ts @@ -15,81 +15,95 @@ */ export interface IDisposable { - dispose(): void; + dispose(): void; } export class MultiDisposeError extends Error { - constructor( - public readonly errors: unknown[] - ) { - super(`Encountered errors while disposing of store. Errors: [${errors.join(', ')}]`); - } + constructor( + public readonly errors: unknown[] + ) { + super(`Encountered errors while disposing of store. Errors: [${errors.join(', ')}]`); + } } export function disposeAll(disposables: Iterable) { - const errors: unknown[] = []; - - for (const disposable of disposables) { - try { - disposable.dispose(); - } catch (e) { - errors.push(e); - } - } - - if (errors.length === 1) { - throw errors[0]; - } else if (errors.length > 1) { - throw new MultiDisposeError(errors); - } + const errors: unknown[] = []; + + for (const disposable of disposables) { + try { + disposable.dispose(); + } catch (e) { + errors.push(e); + } + } + + if (errors.length === 1) { + throw errors[0]; + } else if (errors.length > 1) { + throw new MultiDisposeError(errors); + } } export interface IDisposable { - dispose(): void; + dispose(): void; } export abstract class Disposable { - #isDisposed = false; - - protected _disposables: IDisposable[] = []; - - public dispose() { - if (this.#isDisposed) { - return; - } - this.#isDisposed = true; - disposeAll(this._disposables); - } - - protected _register(value: T): T { - if (this.#isDisposed) { - value.dispose(); - } else { - this._disposables.push(value); - } - return value; - } - - protected get isDisposed() { - return this.#isDisposed; - } + #isDisposed = false; + + protected _disposables: IDisposable[] = []; + + public dispose() { + if (this.#isDisposed) { + return; + } + this.#isDisposed = true; + disposeAll(this._disposables); + } + + protected _register(value: T): T { + if (this.#isDisposed) { + value.dispose(); + } else { + this._disposables.push(value); + } + return value; + } + + protected get isDisposed() { + return this.#isDisposed; + } } export class DisposableStore extends Disposable { - readonly #items = new Set(); - - public override dispose() { - super.dispose(); - disposeAll(this.#items); - this.#items.clear(); - } - - public add(item: T): T { - if (this.isDisposed) { - console.warn('Adding to disposed store. Item will be leaked'); - } - - this.#items.add(item); - return item; - } + readonly #items = new Set(); + + public override dispose() { + super.dispose(); + this.clear(); + } + + public add(item: T): T { + if (this.isDisposed) { + console.warn('Adding to disposed store. Item will be leaked'); + } + + this.#items.add(item); + return item; + } + + /** + * Dispose of all registered disposables but do not mark this object as disposed. + */ + public clear(): void { + if (this.#items.size === 0) { + return; + } + + try { + disposeAll(this.#items); + } finally { + this.#items.clear(); + } + } }