From 016af2068120dc9ca9f0f9dc6eb87e05073d1a0f Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 12:59:21 -0400 Subject: [PATCH 1/9] Remove unused imports --- apps/vscode/src/host/hooks.ts | 2 +- apps/vscode/src/providers/assist/render-assist.ts | 2 +- apps/vscode/src/providers/format.ts | 1 - apps/vscode/src/vdoc/vdoc-completion.ts | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/vscode/src/host/hooks.ts b/apps/vscode/src/host/hooks.ts index 50502c30..13bd2708 100644 --- a/apps/vscode/src/host/hooks.ts +++ b/apps/vscode/src/host/hooks.ts @@ -20,7 +20,7 @@ import { ExtensionHost, HostWebviewPanel, HostStatementRangeProvider, HostHelpTo import { CellExecutor, cellExecutorForLanguage, executableLanguages, isKnitrDocument, pythonWithReticulate } from './executors'; import { ExecuteQueue } from './execute-queue'; import { MarkdownEngine } from '../markdown/engine'; -import { virtualDoc, virtualDocUri, adjustedPosition, unadjustedRange, withVirtualDocUri } from "../vdoc/vdoc"; +import { virtualDoc, adjustedPosition, unadjustedRange, withVirtualDocUri } from "../vdoc/vdoc"; import { EmbeddedLanguage } from '../vdoc/languages'; declare global { diff --git a/apps/vscode/src/providers/assist/render-assist.ts b/apps/vscode/src/providers/assist/render-assist.ts index 46e0159d..30bc9c5b 100644 --- a/apps/vscode/src/providers/assist/render-assist.ts +++ b/apps/vscode/src/providers/assist/render-assist.ts @@ -35,7 +35,7 @@ import { import { JsonRpcRequestTransport, escapeRegExpCharacters } from "core"; import { CodeViewCellContext, kCodeViewAssist } from "editor-types"; import { embeddedLanguage } from "../../vdoc/languages"; -import { virtualDocForCode, virtualDocUri, withVirtualDocUri } from "../../vdoc/vdoc"; +import { virtualDocForCode, withVirtualDocUri } from "../../vdoc/vdoc"; import { getHover, getSignatureHelpHover } from "../../core/hover"; import { Hover as LspHover, MarkupKind } from "vscode-languageserver-types"; import { MarkupContent } from "vscode-languageclient"; diff --git a/apps/vscode/src/providers/format.ts b/apps/vscode/src/providers/format.ts index acb0c691..92c078e5 100644 --- a/apps/vscode/src/providers/format.ts +++ b/apps/vscode/src/providers/format.ts @@ -44,7 +44,6 @@ import { VirtualDoc, virtualDocForCode, virtualDocForLanguage, - virtualDocUri, withVirtualDocUri, } from "../vdoc/vdoc"; diff --git a/apps/vscode/src/vdoc/vdoc-completion.ts b/apps/vscode/src/vdoc/vdoc-completion.ts index a8f2b0fd..96590dd5 100644 --- a/apps/vscode/src/vdoc/vdoc-completion.ts +++ b/apps/vscode/src/vdoc/vdoc-completion.ts @@ -15,7 +15,7 @@ import { commands, Position, Uri, CompletionList, CompletionItem, Range } from "vscode"; import { EmbeddedLanguage } from "./languages"; -import { adjustedPosition, unadjustedRange, VirtualDoc, virtualDocUri, withVirtualDocUri } from "./vdoc"; +import { adjustedPosition, unadjustedRange, VirtualDoc, withVirtualDocUri } from "./vdoc"; export async function vdocCompletions( vdoc: VirtualDoc, From 0da0c68a640e9b83411a268437f3dfe6c0952e79 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:13:57 -0400 Subject: [PATCH 2/9] Add `withVirtualDocUri()` return type --- apps/vscode/src/vdoc/vdoc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/vscode/src/vdoc/vdoc.ts b/apps/vscode/src/vdoc/vdoc.ts index bfe942a9..91ebdbf3 100644 --- a/apps/vscode/src/vdoc/vdoc.ts +++ b/apps/vscode/src/vdoc/vdoc.ts @@ -127,7 +127,7 @@ export async function withVirtualDocUri( parentUri: Uri, action: VirtualDocAction, f: (uri: Uri) => Promise -) { +): Promise { const vdocUri = await virtualDocUri(vdoc, parentUri, action); try { From 1742215b18e921b9b72eccd6753ba447b221cad1 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:16:15 -0400 Subject: [PATCH 3/9] Document try-finally without catch rationale --- apps/vscode/src/vdoc/vdoc.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/vscode/src/vdoc/vdoc.ts b/apps/vscode/src/vdoc/vdoc.ts index 91ebdbf3..06a9bed9 100644 --- a/apps/vscode/src/vdoc/vdoc.ts +++ b/apps/vscode/src/vdoc/vdoc.ts @@ -130,6 +130,8 @@ export async function withVirtualDocUri( ): Promise { const vdocUri = await virtualDocUri(vdoc, parentUri, action); + // try-finally without a catch allows `f()` to propagate an exception up to the caller + // while still allowing us to clean up the vdoc tempfile. try { return await f(vdocUri.uri); } finally { From 8ce0179e3a81d6d6b5105a39ed5bf27506ae25a3 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:18:56 -0400 Subject: [PATCH 4/9] Use `withVirtualDocUri()` in `embeddedHoverProvider()` --- apps/vscode/src/lsp/client.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index 8d0897f5..9e0ab1ea 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -23,6 +23,7 @@ import { LocationLink, Definition, LogOutputChannel, + Uri } from "vscode"; import { LanguageClient, @@ -52,6 +53,7 @@ import { unadjustedRange, virtualDoc, virtualDocUri, + withVirtualDocUri, } from "../vdoc/vdoc"; import { activateVirtualDocEmbeddedContent } from "../vdoc/vdoc-content"; import { vdocCompletions } from "../vdoc/vdoc-completion"; @@ -225,19 +227,13 @@ function embeddedHoverProvider(engine: MarkdownEngine) { const vdoc = await virtualDoc(document, position, engine); if (vdoc) { - // get uri for hover - const vdocUri = await virtualDocUri(vdoc, document.uri, "hover"); - - // execute hover - try { - return getHover(vdocUri.uri, vdoc.language, position); - } catch (error) { - console.log(error); - } finally { - if (vdocUri.cleanup) { - await vdocUri.cleanup(); + return await withVirtualDocUri(vdoc, document.uri, "hover", async (uri: Uri) => { + try { + return await getHover(uri, vdoc.language, position); + } catch (error) { + console.log(error); } - } + }); } // default to server delegation From bc936d97d7687e302014eb60888e94d707c5f790 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:21:35 -0400 Subject: [PATCH 5/9] Use `withVirtualDocUri()` in `embeddedSignatureHelpProvider()` --- apps/vscode/src/lsp/client.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index 9e0ab1ea..b13eac29 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -251,16 +251,13 @@ function embeddedSignatureHelpProvider(engine: MarkdownEngine) { ) => { const vdoc = await virtualDoc(document, position, engine); if (vdoc) { - const vdocUri = await virtualDocUri(vdoc, document.uri, "signature"); - try { - return getSignatureHelpHover(vdocUri.uri, vdoc.language, position, context.triggerCharacter); - } catch (error) { - return undefined; - } finally { - if (vdocUri.cleanup) { - await vdocUri.cleanup(); + return await withVirtualDocUri(vdoc, document.uri, "signature", async (uri: Uri) => { + try { + return await getSignatureHelpHover(uri, vdoc.language, position, context.triggerCharacter); + } catch (error) { + return undefined; } - } + }); } else { return await next(document, position, context, token); } From 9c3050b014799128ead6420d858e7c843851c778 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:24:32 -0400 Subject: [PATCH 6/9] Use `withVirtualDocUri()` in `embeddedGoToDefinitionProvider()` --- apps/vscode/src/lsp/client.ts | 107 +++++++++++++++++----------------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index b13eac29..c2c975f6 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -273,64 +273,61 @@ function embeddedGoToDefinitionProvider(engine: MarkdownEngine) { ): Promise => { const vdoc = await virtualDoc(document, position, engine); if (vdoc) { - const vdocUri = await virtualDocUri(vdoc, document.uri, "definition"); - try { - const definitions = await commands.executeCommand< - ProviderResult - >( - "vscode.executeDefinitionProvider", - vdocUri.uri, - adjustedPosition(vdoc.language, position) - ); - const resolveLocation = (location: Location) => { - if (location.uri.toString() === vdocUri.uri.toString()) { - return new Location( - document.uri, - unadjustedRange(vdoc.language, location.range) - ); - } else { - return location; - } - }; - const resolveLocationLink = (location: LocationLink) => { - if (location.targetUri.toString() === vdocUri.uri.toString()) { - const locationLink: LocationLink = { - targetRange: unadjustedRange(vdoc.language, location.targetRange), - originSelectionRange: location.originSelectionRange - ? unadjustedRange(vdoc.language, location.originSelectionRange) - : undefined, - targetSelectionRange: location.targetSelectionRange - ? unadjustedRange(vdoc.language, location.targetSelectionRange) - : undefined, - targetUri: document.uri, - }; - return locationLink; - } else { - return location; - } - }; - if (definitions instanceof Location) { - return resolveLocation(definitions); - } else if (Array.isArray(definitions) && definitions.length > 0) { - if (definitions[0] instanceof Location) { - return definitions.map((definition) => - resolveLocation(definition as Location) - ); + return await withVirtualDocUri(vdoc, document.uri, "definition", async (uri: Uri) => { + try { + const definitions = await commands.executeCommand< + ProviderResult + >( + "vscode.executeDefinitionProvider", + uri, + adjustedPosition(vdoc.language, position) + ); + const resolveLocation = (location: Location) => { + if (location.uri.toString() === uri.toString()) { + return new Location( + document.uri, + unadjustedRange(vdoc.language, location.range) + ); + } else { + return location; + } + }; + const resolveLocationLink = (location: LocationLink) => { + if (location.targetUri.toString() === uri.toString()) { + const locationLink: LocationLink = { + targetRange: unadjustedRange(vdoc.language, location.targetRange), + originSelectionRange: location.originSelectionRange + ? unadjustedRange(vdoc.language, location.originSelectionRange) + : undefined, + targetSelectionRange: location.targetSelectionRange + ? unadjustedRange(vdoc.language, location.targetSelectionRange) + : undefined, + targetUri: document.uri, + }; + return locationLink; + } else { + return location; + } + }; + if (definitions instanceof Location) { + return resolveLocation(definitions); + } else if (Array.isArray(definitions) && definitions.length > 0) { + if (definitions[0] instanceof Location) { + return definitions.map((definition) => + resolveLocation(definition as Location) + ); + } else { + return definitions.map((definition) => + resolveLocationLink(definition as LocationLink) + ); + } } else { - return definitions.map((definition) => - resolveLocationLink(definition as LocationLink) - ); + return definitions; } - } else { - return definitions; - } - } catch (error) { - return undefined; - } finally { - if (vdocUri.cleanup) { - await vdocUri.cleanup(); + } catch (error) { + return undefined; } - } + }); } else { return await next(document, position, token); } From 14ae96fb092eb04d0770b8dfa8474c4a91a06319 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:30:03 -0400 Subject: [PATCH 7/9] Remove unused import --- apps/vscode/src/lsp/client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index c2c975f6..0799d3d8 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -52,7 +52,6 @@ import { adjustedPosition, unadjustedRange, virtualDoc, - virtualDocUri, withVirtualDocUri, } from "../vdoc/vdoc"; import { activateVirtualDocEmbeddedContent } from "../vdoc/vdoc-content"; From a3236fffdd67dac2567216dc980829b2f5766fc2 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:30:52 -0400 Subject: [PATCH 8/9] Un-`export` the footgun that is `virtualDocUri()` --- apps/vscode/src/vdoc/vdoc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/vscode/src/vdoc/vdoc.ts b/apps/vscode/src/vdoc/vdoc.ts index 06a9bed9..37829d7e 100644 --- a/apps/vscode/src/vdoc/vdoc.ts +++ b/apps/vscode/src/vdoc/vdoc.ts @@ -141,7 +141,7 @@ export async function withVirtualDocUri( } } -export async function virtualDocUri( +async function virtualDocUri( virtualDoc: VirtualDoc, parentUri: Uri, action: VirtualDocAction From 3aad1bb48e0fc53d4fab838b0c875660b51ebf21 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Mon, 19 May 2025 13:31:44 -0400 Subject: [PATCH 9/9] Document `withVirtualDocUri()` and `virtualDocUri()` --- apps/vscode/src/vdoc/vdoc.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apps/vscode/src/vdoc/vdoc.ts b/apps/vscode/src/vdoc/vdoc.ts index 37829d7e..f1441fe8 100644 --- a/apps/vscode/src/vdoc/vdoc.ts +++ b/apps/vscode/src/vdoc/vdoc.ts @@ -122,6 +122,16 @@ export type VirtualDocAction = export type VirtualDocUri = { uri: Uri, cleanup?: () => Promise }; +/** + * Execute a callback on a virtual document's temporary URI + * + * This method automatically cleans up the temporary URI after executing `f`. + * + * @param vdoc The virtual document to create a temporary URI for + * @param parentUri The virtual document's original URI it was virtualized from + * @param f The callback to execute + * @returns A Promise evaluating to an object of type `T` returned by `f` + */ export async function withVirtualDocUri( vdoc: VirtualDoc, parentUri: Uri, @@ -141,6 +151,9 @@ export async function withVirtualDocUri( } } +// To be used through `withVirtualDocUri()`. Not safe to export on its own! The +// cleanup hook must be called, and relying on the caller to do this is a huge +// footgun. async function virtualDocUri( virtualDoc: VirtualDoc, parentUri: Uri,