From 89c2b81efb05ee57bba2e734471c840e1c309a40 Mon Sep 17 00:00:00 2001 From: elliot Date: Wed, 29 Apr 2026 13:51:48 -0400 Subject: [PATCH] Add diagnostics for cells --- apps/vscode/package.json | 16 +- apps/vscode/src/lsp/client.ts | 16 +- apps/vscode/src/main.ts | 35 ++- .../src/providers/embedded-diagnostics.ts | 280 ++++++++++++++++++ 4 files changed, 339 insertions(+), 8 deletions(-) create mode 100644 apps/vscode/src/providers/embedded-diagnostics.ts diff --git a/apps/vscode/package.json b/apps/vscode/package.json index f778218f..c01dd997 100644 --- a/apps/vscode/package.json +++ b/apps/vscode/package.json @@ -970,6 +970,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.", @@ -1016,7 +1030,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 21e41cee..1fb2f741 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -72,6 +72,7 @@ import { imageHover } from "../providers/hover-image"; import { LspInitializationOptions, QuartoContext } from "quarto-core"; import { extensionHost } from "../host"; import semver from "semver"; +import { EmbeddedDiagnosticsManager } from "../providers/embedded-diagnostics"; let client: LanguageClient; @@ -79,7 +80,8 @@ export async function activateLsp( context: ExtensionContext, quartoContext: QuartoContext, engine: MarkdownEngine, - outputChannel: LogOutputChannel + outputChannel: LogOutputChannel, + diagnosticsManager?: EmbeddedDiagnosticsManager ) { // The server is implemented in node @@ -105,7 +107,7 @@ export async function activateLsp( const config = workspace.getConfiguration("quarto"); activateVirtualDocEmbeddedContent(); const middleware: Middleware = { - handleDiagnostics: createDiagnosticFilter(), + handleDiagnostics: createDiagnosticFilter(diagnosticsManager), provideCompletionItem: embeddedCodeCompletionProvider(engine), provideDefinition: embeddedGoToDefinitionProvider(engine), provideDocumentFormattingEdits: embeddedDocumentFormattingProvider(engine), @@ -369,7 +371,7 @@ function isWithinYamlComment(doc: TextDocument, pos: Position) { * * @returns A handler function for the middleware */ -export function createDiagnosticFilter() { +export function createDiagnosticFilter(diagnosticsManager?: EmbeddedDiagnosticsManager) { return (uri: Uri, diagnostics: Diagnostic[], next: HandleDiagnosticsSignature) => { // If this is not a virtual document, pass through all diagnostics if (!isVirtualDoc(uri)) { @@ -377,7 +379,11 @@ export function createDiagnosticFilter() { return; } - // For virtual documents, filter out all diagnostics - next(uri, []); + // For virtual docs from Quarto LSP, let diagnostics manager handle them + // (but most diagnostics come from other language servers via onDidChangeDiagnostics) + const remapped = diagnosticsManager?.handleDiagnostics(uri, diagnostics); + + // Suppress vdoc diagnostics from being published by the LSP + next(uri, remapped ?? []); }; } diff --git a/apps/vscode/src/main.ts b/apps/vscode/src/main.ts index eb3287ab..ad2bd1fb 100644 --- a/apps/vscode/src/main.ts +++ b/apps/vscode/src/main.ts @@ -17,8 +17,9 @@ import * as vscode from "vscode"; import * as path from "path"; import { tryAcquirePositronApi } from "@posit-dev/positron"; import { MarkdownEngine } from "./markdown/engine"; -import { kQuartoDocSelector } from "./core/doc"; +import { kQuartoDocSelector, isQuartoDoc } from "./core/doc"; import { activateLsp, deactivate as deactivateLsp } from "./lsp/client"; +import { EmbeddedDiagnosticsManager } from "./providers/embedded-diagnostics"; import { cellCommands } from "./providers/cell/commands"; import { quartoCellExecuteCodeLensProvider } from "./providers/cell/codelens"; import { activateQuartoAssistPanel } from "./providers/assist/panel"; @@ -116,8 +117,38 @@ export async function activate(context: vscode.ExtensionContext): Promise { + if (isQuartoDoc(doc)) { + diagnosticsManager.handleDocumentOpen(doc); + } + }), + vscode.workspace.onDidChangeTextDocument((e) => { + if (isQuartoDoc(e.document)) { + diagnosticsManager.handleDocumentChange(e.document); + } + }), + vscode.workspace.onDidCloseTextDocument((doc) => { + if (isQuartoDoc(doc)) { + diagnosticsManager.handleDocumentClose(doc); + } + }) + ); + + // Process already-open documents + vscode.workspace.textDocuments.forEach((doc) => { + if (isQuartoDoc(doc)) { + diagnosticsManager.handleDocumentOpen(doc); + } + }); + // lsp - const lspClient = await activateLsp(context, quartoContext, engine, outputChannel); + const lspClient = await activateLsp(context, quartoContext, engine, outputChannel, diagnosticsManager); // provide visual editor const editorCommands = activateEditor(context, host, quartoContext, lspClient, engine); diff --git a/apps/vscode/src/providers/embedded-diagnostics.ts b/apps/vscode/src/providers/embedded-diagnostics.ts new file mode 100644 index 00000000..0a689a4f --- /dev/null +++ b/apps/vscode/src/providers/embedded-diagnostics.ts @@ -0,0 +1,280 @@ +/* + * embedded-diagnostics.ts + * + * Copyright (C) 2022-2026 by Posit Software, PBC + * + * Unless you have received this program directly from Posit Software pursuant + * to the terms of a commercial license agreement with Posit Software, then + * this program is licensed to you under the terms of version 3 of the + * GNU Affero General Public License. This program is distributed WITHOUT + * ANY EXPRESS OR IMPLIED WARRANTY, INCLUDING THOSE OF NON-INFRINGEMENT, + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. Please refer to the + * AGPL (http://www.gnu.org/licenses/agpl-3.0.txt) for more details. + * + */ + +import { + Diagnostic, + DiagnosticCollection, + Disposable, + TextDocument, + Uri, + languages, + workspace, +} from "vscode"; +import { + Token, + isExecutableLanguageBlock, + languageBlockAtPosition, + languageNameFromBlock, +} from "quarto-core"; + +import { MarkdownEngine } from "../markdown/engine"; +import { embeddedLanguage, EmbeddedLanguage } from "../vdoc/languages"; +import { VirtualDoc } from "../vdoc/vdoc"; +import * as fs from "fs"; +import * as path from "path"; +import * as uuid from "uuid"; + +interface VirtualDocInfo { + realDocUri: Uri; + tokens: Token[]; + cleanup: () => void; +} + +export class EmbeddedDiagnosticsManager implements Disposable { + private diagnosticCollection: DiagnosticCollection; + private vdocToReal = new Map(); + private disposables: Disposable[] = []; + private debounceTimers = new Map(); + + constructor(private engine: MarkdownEngine) { + this.diagnosticCollection = languages.createDiagnosticCollection("quarto-embedded"); + this.disposables.push(this.diagnosticCollection); + + // Clean up any leftover virtual docs from previous session + this.cleanupAllVirtualDocs(); + + // TODO: can we listen more specifically to particular vdocs? + // Listen to diagnostic changes from all language servers + this.disposables.push( + languages.onDidChangeDiagnostics((event) => { + for (const uri of event.uris) { + const vdocInfo = this.vdocToReal.get(uri.toString()); + if (vdocInfo) { + this.handleDiagnosticsForVirtualDoc(uri, vdocInfo); + } + } + }) + ); + } + + private async cleanupAllVirtualDocs(): Promise { + const workspaceFolders = workspace.workspaceFolders; + if (!workspaceFolders) return; + + for (const folder of workspaceFolders) { + try { + const quartoDir = Uri.joinPath(folder.uri, ".quarto"); + const files = await workspace.fs.readDirectory(quartoDir); + + for (const [filename, fileType] of files) { + if (fileType === 1 && filename.startsWith(".vdoc.")) { + await workspace.fs.delete(Uri.joinPath(quartoDir, filename), { useTrash: false }); + } + } + } catch { + // Directory doesn't exist, that's fine + } + } + } + + async handleDocumentOpen(document: TextDocument): Promise { + if (!workspace.getConfiguration("quarto.cells.diagnostics").get("enabled", true)) { + return; + } + this.createVirtualDocs(document); + } + + handleDocumentChange(document: TextDocument): void { + if (!workspace.getConfiguration("quarto.cells.diagnostics").get("enabled", true)) { + return; + } + + const docKey = document.uri.toString(); + const existingTimer = this.debounceTimers.get(docKey); + if (existingTimer) clearTimeout(existingTimer); + + const debounceDelay = workspace.getConfiguration("quarto.cells.diagnostics").get("debounceDelay", 500); + const timer = setTimeout(async () => { + this.debounceTimers.delete(docKey); + await this.recreateVirtualDocs(document); + }, debounceDelay); + + this.debounceTimers.set(docKey, timer); + } + + handleDocumentClose(document: TextDocument): void { + const docKey = document.uri.toString(); + + const timer = this.debounceTimers.get(docKey); + if (timer) { + clearTimeout(timer); + this.debounceTimers.delete(docKey); + } + + this.cleanupVirtualDocsForDocument(docKey); + this.diagnosticCollection.delete(document.uri); + } + + private cleanupVirtualDocsForDocument(docKey: string): void { + for (const [vdocKey, vdocInfo] of this.vdocToReal.entries()) { + if (vdocInfo.realDocUri.toString() === docKey) { + vdocInfo.cleanup(); + this.vdocToReal.delete(vdocKey); + } + } + } + + private async recreateVirtualDocs(document: TextDocument): Promise { + this.cleanupVirtualDocsForDocument(document.uri.toString()); + this.diagnosticCollection.delete(document.uri); + this.createVirtualDocs(document); + } + + private async createVirtualDocs(document: TextDocument): Promise { + const tokens = this.engine.parse(document); + + // Group code blocks by language + const languageMap = new Map(); + for (const token of tokens) { + if (isExecutableLanguageBlock(token)) { + const lang = languageNameFromBlock(token); + if (lang) { + const blocks = languageMap.get(lang) ?? []; + blocks.push(token); + languageMap.set(lang, blocks); + } + } + } + + // Create one virtual doc per language + for (const [langName] of languageMap) { + const language = embeddedLanguage(langName); + if (!language) continue; + + try { + const vdocContent = this.createVirtualDocContent(document, tokens, language); + const { uri, cleanup } = await this.writeVirtualDocFile(vdocContent, document.uri, language); + + this.vdocToReal.set(uri.toString(), { + realDocUri: document.uri, + tokens, + cleanup, + }); + } catch (error) { + console.debug(`Failed to create virtual doc for ${langName}:`, error); + } + } + } + + // TODO: this maybe shouldn't be implemented here, + // this creates a virtual doc without the inject + // lines that i.e. in python disable linting like + // `# type: ignore`. We should co-locate this with + // where vdoc content is usually created in `virtualDocForCode` + // in vdoc.ts + + private createVirtualDocContent( + document: TextDocument, + tokens: Token[], + language: EmbeddedLanguage + ): VirtualDoc { + const lines: string[] = []; + for (let i = 0; i < document.lineCount; i++) { + lines.push(language.emptyLine || ""); + } + + for (const block of tokens.filter( + (token) => isExecutableLanguageBlock(token) && languageNameFromBlock(token) === language.ids[0] + )) { + for (let line = block.range.start.line + 1; line < block.range.end.line && line < document.lineCount; line++) { + lines[line] = document.lineAt(line).text; + } + } + + return { + language, + content: lines.join("\n") + "\n", + }; + } + + // creates a virtual doc in the workspace under a `.quarto` folder. + // This probably isn't a good user experience, + // but its how I got it to work for now (LSPs don't seem to + // want to give diagnostics for files that aren't in the workspace). + private async writeVirtualDocFile( + vdocContent: VirtualDoc, + documentUri: Uri, + language: EmbeddedLanguage + ): Promise<{ uri: Uri; cleanup: () => void; }> { + const docDir = path.dirname(documentUri.fsPath); + const quartoDir = path.join(docDir, ".quarto"); + + if (!fs.existsSync(quartoDir)) { + fs.mkdirSync(quartoDir, { recursive: true }); + } + + const filename = `.vdoc.${uuid.v4()}.${language.extension}`; + const filepath = path.join(quartoDir, filename); + + fs.writeFileSync(filepath, vdocContent.content); + + const uri = Uri.file(filepath); + await workspace.openTextDocument(uri); + + return { + uri, + cleanup: async () => { + try { + await workspace.fs.delete(uri, { useTrash: false }); + } catch (error) { + console.debug(`Failed to delete virtual doc: ${filepath}`, error); + } + } + }; + } + + private handleDiagnosticsForVirtualDoc(uri: Uri, vdocInfo: VirtualDocInfo): void { + const diagnostics = languages.getDiagnostics(uri); + const mappedDiagnostics: Diagnostic[] = []; + + for (const diagnostic of diagnostics) { + const block = languageBlockAtPosition(vdocInfo.tokens, diagnostic.range.start); + if (block) { + mappedDiagnostics.push(new Diagnostic(diagnostic.range, diagnostic.message, diagnostic.severity)); + } + } + + this.diagnosticCollection.set(vdocInfo.realDocUri, mappedDiagnostics); + } + + dispose(): void { + for (const timer of this.debounceTimers.values()) { + clearTimeout(timer); + } + this.debounceTimers.clear(); + + for (const vdocInfo of this.vdocToReal.values()) { + vdocInfo.cleanup(); + } + this.vdocToReal.clear(); + + this.cleanupAllVirtualDocs(); + + for (const disposable of this.disposables) { + disposable.dispose(); + } + this.disposables = []; + } +}