diff --git a/.vscode-test.mjs b/.vscode-test.mjs index d022829a0..07905dd0e 100644 --- a/.vscode-test.mjs +++ b/.vscode-test.mjs @@ -29,7 +29,14 @@ function testProfile(name, lspEnabled) { const directory = path.resolve('.vscode-test/user-data', name); mkdirSync(path.join(directory, 'User'), { recursive: true }); writeFileSync(path.join(directory, 'User/settings.json'), JSON.stringify({ 'r.lsp.enabled': lspEnabled })); - return ['--user-data-dir', directory]; + // Unattended runs can be occluded by another app. Keep browser paint and + // resize callbacks active so webview assertions observe actual loading. + return [ + '--user-data-dir', directory, + '--disable-renderer-backgrounding', + '--disable-background-timer-throttling', + '--disable-backgrounding-occluded-windows' + ]; } export default defineConfig([ diff --git a/src/completions.ts b/src/completions.ts index 789acdbe3..d093a57cb 100644 --- a/src/completions.ts +++ b/src/completions.ts @@ -222,8 +222,12 @@ export class HelpLinkHoverProvider implements vscode.HoverProvider { async provideHover( document: vscode.TextDocument, position: vscode.Position, + cancellation?: vscode.CancellationToken, ): Promise { - if (!config().get('helpPanel.enableHoverLinks')) { + if ( + cancellation?.isCancellationRequested || + !config().get('helpPanel.enableHoverLinks') + ) { return null; } @@ -242,8 +246,20 @@ export class HelpLinkHoverProvider implements vscode.HoverProvider { const re = /([a-zA-Z0-9._:])+/; const wordRange = document.getWordRangeAtPosition(position, re); + if (!wordRange) { + return null; + } + const version = document.version; const token = document.getText(wordRange); const aliases = (await globalRHelp?.getMatchingAliases(token)) || []; + if ( + !aliases.length || + cancellation?.isCancellationRequested || + document.isClosed || + document.version !== version + ) { + return null; + } const mds = aliases.map((a) => { const cmdText = `${a.package}::${a.alias}`; const args = [`/library/${a.package}/html/${a.name}.html`]; diff --git a/src/cppProperties.ts b/src/cppProperties.ts index bdbaf3de9..3ad4f9ff7 100644 --- a/src/cppProperties.ts +++ b/src/cppProperties.ts @@ -3,8 +3,14 @@ import * as fs from 'fs'; import * as path from 'path'; import { Uri, window } from 'vscode'; -import { getRpath, getCurrentWorkspaceFolder, executeRCommand, createTempDir } from './util'; -import { execSync } from 'child_process'; +import { + getRpath, + getCurrentWorkspaceFolder, + executeRCommand, + createTempDir, + spawnAsync, + ToRStringLiteral, +} from './util'; import { extensionContext } from './extension'; export async function generateCppProperties(): Promise { @@ -52,14 +58,18 @@ async function generateCppPropertiesProc(workspaceFolder: string) { }); } - const compileOutputCpp = collectCompilerOutput(rPath, workspaceFolder, 'cpp'); - const compileOutputC = collectCompilerOutput(rPath, workspaceFolder, 'c'); - - if (fs.existsSync(path.join(workspaceFolder, cleanupFile))) { - await executeRCommand(`system("sh ./${cleanupFile}")`, workspaceFolder, (e: Error) => { - void window.showErrorMessage(e.message); - return ''; - }); + let compileOutputCpp: string; + let compileOutputC: string; + try { + compileOutputCpp = await collectCompilerOutput(rPath, workspaceFolder, 'cpp'); + compileOutputC = await collectCompilerOutput(rPath, workspaceFolder, 'c'); + } finally { + if (fs.existsSync(path.join(workspaceFolder, cleanupFile))) { + await executeRCommand(`system("sh ./${cleanupFile}")`, workspaceFolder, (e: Error) => { + void window.showErrorMessage(e.message); + return ''; + }); + } } const compileInfo = extractCompilerInfo(compileOutputCpp); @@ -67,10 +77,14 @@ async function generateCppPropertiesProc(workspaceFolder: string) { const compileStdC = extractCompilerStd(compileOutputC); const compileCall = extractCompilerCall(compileOutputCpp); const compilerPath = compileCall - ? await executeRCommand(`cat(Sys.which("${compileCall}"))`, workspaceFolder, (e: Error) => { - void window.showErrorMessage(e.message); - return ''; - }) + ? await executeRCommand( + `cat(Sys.which(${ToRStringLiteral(compileCall, '"')}))`, + workspaceFolder, + (e: Error) => { + void window.showErrorMessage(e.message); + return ''; + }, + ) : ''; const intelliSensePlatform = platformChoose('windows', 'macos', 'linux'); @@ -134,10 +148,14 @@ async function collectRLinkingTo(workspaceFolder: string): Promise { .asAbsolutePath('R/cppProperties/extractLinkingTo.R') .replace(/\\/g, '/'); const linkingToIncludesStr = ( - await executeRCommand(`source('${rScript}')`, workspaceFolder, (e: Error) => { - void window.showErrorMessage(e.message); - return ''; - }) + await executeRCommand( + `source(${ToRStringLiteral(rScript, '"')})`, + workspaceFolder, + (e: Error) => { + void window.showErrorMessage(e.message); + return ''; + }, + ) )?.trim(); if (!linkingToIncludesStr || linkingToIncludesStr === '') { return []; @@ -190,37 +208,44 @@ function extractCompilerCall(compileOutput: string): string | undefined { } const m = ccalls[1].match(rxComp); - return m?.[1]; + return m ? ensureUnquoted(m[1]) : undefined; } -function collectCompilerOutput(rPath: string, workspaceFolder: string, testExtension: 'cpp' | 'c') { +async function collectCompilerOutput( + rPath: string, + workspaceFolder: string, + testExtension: 'cpp' | 'c', +): Promise { const makevarsFiles = ['Makevars', 'Makevars.win', 'Makevars.ucrt']; const srcFolder = path.join(workspaceFolder, 'src'); const tempFolder = createTempDir(workspaceFolder); - // Copy makevars - if (fs.existsSync(srcFolder)) { - const projectMakevarsFiles = fs - .readdirSync(srcFolder) - .filter((fn) => makevarsFiles.includes(fn)); - for (const f of projectMakevarsFiles) { - fs.copyFileSync(path.join(srcFolder, f), path.join(tempFolder, f)); + try { + // Copy makevars + if (fs.existsSync(srcFolder)) { + const projectMakevarsFiles = fs + .readdirSync(srcFolder) + .filter((fn) => makevarsFiles.includes(fn)); + for (const f of projectMakevarsFiles) { + fs.copyFileSync(path.join(srcFolder, f), path.join(tempFolder, f)); + } } - } - - // Create dummy source file - const testFile = `comp_test.${testExtension}`; - fs.writeFileSync(path.join(tempFolder, testFile), ''); - - // Compile dummy - const command = `"${rPath}" CMD SHLIB ${testFile}`; - const compileOutput = execSync(command, { - cwd: tempFolder, - }).toString(); - // Cleanup - fs.rmSync(tempFolder, { recursive: true, force: true }); - - return compileOutput; + // Create dummy source file + const testFile = `comp_test.${testExtension}`; + fs.writeFileSync(path.join(tempFolder, testFile), ''); + + // Compile dummy + const result = await spawnAsync(rPath, ['CMD', 'SHLIB', testFile], { cwd: tempFolder }); + if (result.status !== 0) { + throw ( + result.error ?? + new Error(result.stderr || 'Could not determine R compiler settings.') + ); + } + return result.stdout; + } finally { + fs.rmSync(tempFolder, { recursive: true, force: true }); + } } diff --git a/src/extension.ts b/src/extension.ts index 7dc94f7fe..d584bd5a2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -243,22 +243,24 @@ export async function activate( // register on-enter rule for roxygen comments const wordPattern = /(-?\d*\.\d\w*)|([^`~!@$^&*()=+[{\]}\\|;:'",<>/\s]+)/g; - vscode.languages.setLanguageConfiguration('r', { - onEnterRules: [ - { - // Automatically continue roxygen comments: #' - action: { indentAction: vscode.IndentAction.None, appendText: "#' " }, - beforeText: /^\s*#'\s*[^\s]/, // matches a non-empty roxygen line - }, - { - // Automatically continue roxygen comments: #' - action: { indentAction: vscode.IndentAction.None, appendText: "#' " }, - beforeText: /^\s*#'/, // matches any roxygen comment line, even an empty one - previousLineText: /^\s*([^#\s].*|#[^'\s].*|#'\s*[^\s].*|)$/, // matches everything but an empty roxygen line - }, - ], - wordPattern, - }); + context.subscriptions.push( + vscode.languages.setLanguageConfiguration('r', { + onEnterRules: [ + { + // Automatically continue roxygen comments: #' + action: { indentAction: vscode.IndentAction.None, appendText: "#' " }, + beforeText: /^\s*#'\s*[^\s]/, // matches a non-empty roxygen line + }, + { + // Automatically continue roxygen comments: #' + action: { indentAction: vscode.IndentAction.None, appendText: "#' " }, + beforeText: /^\s*#'/, // matches any roxygen comment line, even an empty one + previousLineText: /^\s*([^#\s].*|#[^'\s].*|#'\s*[^\s].*|)$/, // matches everything but an empty roxygen line + }, + ], + wordPattern, + }), + ); // register terminal-provider context.subscriptions.push( @@ -279,30 +281,44 @@ export async function activate( context.subscriptions.push(new InteractiveManager(context)); // register codelens and completion providers for r markdown and r files - vscode.languages.registerCodeLensProvider( - ['r', 'rmd'], - new rmarkdown.RMarkdownCodeLensProvider(), + const codeLensProvider = new rmarkdown.RMarkdownCodeLensProvider(); + context.subscriptions.push( + codeLensProvider, + vscode.languages.registerCodeLensProvider(['r', 'rmd'], codeLensProvider), + ); + context.subscriptions.push( + vscode.languages.registerCompletionItemProvider( + 'rmd', + new rmarkdown.RMarkdownCompletionItemProvider(), + ' ', + ',', + ), ); - vscode.languages.registerCompletionItemProvider( - 'rmd', - new rmarkdown.RMarkdownCompletionItemProvider(), - ' ', - ',', + context.subscriptions.push( + vscode.languages.registerFoldingRangeProvider('r', new rmarkdown.RChunkFoldingProvider()), ); - vscode.languages.registerFoldingRangeProvider('r', new rmarkdown.RChunkFoldingProvider()); // register (session) hover and completion providers - vscode.languages.registerHoverProvider(['r', 'rmd'], new completions.HoverProvider()); - vscode.languages.registerHoverProvider(['r', 'rmd'], new completions.HelpLinkHoverProvider()); - vscode.languages.registerCompletionItemProvider( - ['r', 'rmd'], - new completions.StaticCompletionItemProvider(), - '@', + context.subscriptions.push( + vscode.languages.registerHoverProvider(['r', 'rmd'], new completions.HoverProvider()), + ); + context.subscriptions.push( + vscode.languages.registerHoverProvider( + ['r', 'rmd'], + new completions.HelpLinkHoverProvider(), + ), + ); + context.subscriptions.push( + vscode.languages.registerCompletionItemProvider( + ['r', 'rmd'], + new completions.StaticCompletionItemProvider(), + '@', + ), ); // register task provider const taskProvider = new RTaskProvider(); - vscode.tasks.registerTaskProvider(taskProvider.type, taskProvider); + context.subscriptions.push(vscode.tasks.registerTaskProvider(taskProvider.type, taskProvider)); // deploy session watcher (if configured by user) if (enableSessionWatcher) { @@ -321,7 +337,9 @@ export async function activate( // track active text editor rstudioapi.trackLastActiveTextEditor(vscode.window.activeTextEditor); - vscode.window.onDidChangeActiveTextEditor(rstudioapi.trackLastActiveTextEditor); + context.subscriptions.push( + vscode.window.onDidChangeActiveTextEditor(rstudioapi.trackLastActiveTextEditor), + ); // register the R Workspace tree view // creates a custom context value for the workspace view diff --git a/src/helpViewer/packages.ts b/src/helpViewer/packages.ts index 77610865c..58255c42e 100644 --- a/src/helpViewer/packages.ts +++ b/src/helpViewer/packages.ts @@ -81,6 +81,8 @@ export class PackageManager { // public favoriteNames: string[] = []; public favoriteNames: Set = new Set(); + private readonly indexLoads = new Map>(); + private cacheGeneration = 0; constructor(args: PackageManagerOptions) { this.rHelp = args.rHelp; @@ -98,6 +100,8 @@ export class PackageManager { // Funciton to clear only the cached files regarding an individual package etc. public async clearCachedFiles(re?: string | RegExp): Promise { + this.cacheGeneration++; + this.indexLoads.clear(); let cache: CachedIndexFiles | undefined; if (re) { const oldCache = this.state.get('r.helpPanel.cachedIndexFiles', []); @@ -161,13 +165,21 @@ export class PackageManager { // Is used frequently when list of favorites is shared globally to sync between sessions private pullFavoriteNames() { if (this.state) { - this.favoriteNames = - this.state.get('r.helpPanel.favoriteNamesSet') || this.favoriteNames; + const saved = this.state.get('r.helpPanel.favoriteNamesSet'); + if (saved !== undefined) { + // Memento persists JSON. Older versions saved a Set, which becomes + // an empty object after reloading the extension host. + this.favoriteNames = new Set( + (Array.isArray(saved) ? saved : saved instanceof Set ? [...saved] : []).filter( + (name): name is string => typeof name === 'string', + ), + ); + } } } private pushFavoriteNames() { if (this.state) { - void this.state.update('r.helpPanel.favoriteNamesSet', this.favoriteNames); + void this.state.update('r.helpPanel.favoriteNamesSet', [...this.favoriteNames]); } } @@ -375,15 +387,26 @@ export class PackageManager { // only read and parse file if not cached yet if (!indexItems) { - const helpFile = await this.rHelp.getHelpFileForPath(path, false); - if (!helpFile?.html) { - // set missing files to null - indexItems = undefined; - } else { - // parse and cache file - indexItems = parseIndexFile(helpFile.html); + let load = this.indexLoads.get(path); + if (!load) { + const generation = this.cacheGeneration; + load = (async () => { + const helpFile = await this.rHelp.getHelpFileForPath(path, false); + const items = helpFile?.html ? parseIndexFile(helpFile.html) : undefined; + if (generation === this.cacheGeneration) { + await this.updateCachedIndexFile(path, items); + } + return items; + })(); + this.indexLoads.set(path, load); + } + try { + indexItems = await load; + } finally { + if (this.indexLoads.get(path) === load) { + this.indexLoads.delete(path); + } } - void this.updateCachedIndexFile(path, indexItems); } // return cache entry. make new array to avoid messing with the cache diff --git a/src/helpViewer/panel.ts b/src/helpViewer/panel.ts index b6e40ad11..8a9c8901b 100644 --- a/src/helpViewer/panel.ts +++ b/src/helpViewer/panel.ts @@ -132,6 +132,7 @@ export class HelpPanel { // called e.g. when the webview panel is closed by the user this.panel.onDidDispose(() => { + this.scrollYCallback?.(0); this.panel = undefined; this.history = []; this.forwardHistory = []; @@ -269,16 +270,37 @@ export class HelpPanel { // Get current scrollY from webview private async getScrollY(): Promise { + if (!this.panel) { + return 0; + } this.scrollYCallback?.(0); - const scrollYPromise = new Promise((resolve, reject) => { - const timeout = setTimeout(() => reject('GetScrollY message timed out after 1s'), 1000); - this.scrollYCallback = (y: number) => { + let finish!: (y: number) => void; + const scrollYPromise = new Promise((resolve) => { + let settled = false; + // Hidden or replaced webview documents may not acknowledge messages. + // Losing a scroll position must not disable Back, Forward or Refresh. + const timeout = setTimeout(() => finish(0), 1000); + finish = (y: number) => { + if (settled) { + return; + } + settled = true; clearTimeout(timeout); - this.scrollYCallback = undefined; + if (this.scrollYCallback === finish) { + this.scrollYCallback = undefined; + } resolve(y); }; + this.scrollYCallback = finish; }); - void this.panel?.webview.postMessage({ command: 'getScrollY' }); + void this.panel.webview.postMessage({ command: 'getScrollY' }).then( + (delivered) => { + if (!delivered) { + finish(0); + } + }, + () => finish(0), + ); return scrollYPromise; } diff --git a/src/interactive/client.ts b/src/interactive/client.ts index ee5c05930..833767ad5 100644 --- a/src/interactive/client.ts +++ b/src/interactive/client.ts @@ -117,20 +117,50 @@ export class AgentClient extends EventEmitter { this.emit('disconnect'); } }); - await new Promise((resolve, reject) => { - const timer = setTimeout(() => { - socket.destroy(); - reject(new Error('Agent connection timed out')); - }, options.timeout ?? 5000); - socket.once('connect', () => { - clearTimeout(timer); - resolve(); - }); - socket.once('error', (error) => { - clearTimeout(timer); - reject(error); + try { + await new Promise((resolve, reject) => { + const finish = (error?: Error): void => { + clearTimeout(timer); + socket.off('connect', connected); + socket.off('error', finish); + socket.off('close', disconnected); + if (error) { + reject(error); + } else { + resolve(); + } + }; + const connected = (): void => finish(); + const disconnected = (): void => finish(new Error('Session agent disconnected')); + const timer = setTimeout(() => { + finish(new Error('Agent connection timed out')); + socket.destroy(); + }, options.timeout ?? 5000); + socket.once('connect', connected); + socket.once('error', finish); + socket.once('close', disconnected); }); - }); + await this.initialize(socket, options); + } catch (error) { + // An obsolete attempt must never close a replacement connection. + if (this.socket === socket) { + this.close(); + } + throw error; + } + } + + private assertConnection(socket: net.Socket): void { + if (this.socket !== socket || socket.destroyed) { + throw new Error('Session agent disconnected during connection'); + } + } + + private async initialize( + socket: net.Socket, + options: { claim?: boolean; timeout?: number }, + ): Promise { + this.assertConnection(socket); const manifest = await this.request( 'hello', { @@ -141,21 +171,26 @@ export class AgentClient extends EventEmitter { }, options.timeout, ); + this.assertConnection(socket); if (manifest.id !== this.manifest.id || manifest.generation !== this.manifest.generation) { throw new Error( 'R session identity changed; refresh the session list before connecting', ); } this.manifest = manifest; - this.control = + const control = options.claim === false ? false : await this.request('claim', {}, options.timeout); + this.assertConnection(socket); + this.control = control; this.ready = true; - this.emit('activity'); this.heartbeat = setInterval(() => { void this.request<{ control: boolean; status: SessionManifest['status'] }>('heartbeat') .then((value) => { + if (this.socket !== socket || socket.destroyed) { + return; + } this.control = value.control; this.manifest.status = value.status; this.emit('activity'); @@ -163,6 +198,7 @@ export class AgentClient extends EventEmitter { .catch(() => socket.destroy()); }, 20000); this.heartbeat.unref(); + this.emit('activity'); } private emitEvent(event: SessionEvent): void { diff --git a/src/interactive/framing.ts b/src/interactive/framing.ts index 1c6fc4f6d..c50ffebe5 100644 --- a/src/interactive/framing.ts +++ b/src/interactive/framing.ts @@ -4,7 +4,8 @@ import { MAX_MESSAGE_BYTES, object } from './protocol'; /** Preserve split UTF-8 characters and reject oversized unterminated frames. */ export class JsonLines { private decoder = new StringDecoder('utf8'); - private buffer = ''; + private parts: string[] = []; + private bytes = 0; constructor( private readonly receive: (message: Record) => void, @@ -12,20 +13,37 @@ export class JsonLines { ) {} push(chunk: Buffer): void { - this.buffer += this.decoder.write(chunk); + const text = this.decoder.write(chunk); + let start = 0; let end: number; - while ((end = this.buffer.indexOf('\n')) >= 0) { - const line = this.buffer.slice(0, end); - this.buffer = this.buffer.slice(end + 1); - if (Buffer.byteLength(line) > this.limit) { - throw new Error('IPC frame exceeds size limit'); - } + // Search and count only newly decoded text. Rescanning an incomplete + // workspace/table frame on each socket read makes fragmented input quadratic. + while ((end = text.indexOf('\n', start)) >= 0) { + this.append(text.slice(start, end)); + const line = this.parts.join(''); + this.parts = []; + this.bytes = 0; if (line.trim()) { this.receive(object(JSON.parse(line))); } + start = end + 1; } - if (Buffer.byteLength(this.buffer) > this.limit) { + this.append(text.slice(start)); + } + + private append(text: string): void { + this.bytes += Buffer.byteLength(text); + if (this.bytes > this.limit) { throw new Error('IPC frame exceeds size limit'); } + if (text) { + // Bound fragment bookkeeping for peers which split every character. + const last = this.parts.length - 1; + if (last >= 0 && this.parts[last].length + text.length <= 32768) { + this.parts[last] += text; + } else { + this.parts.push(text); + } + } } } diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 310cd5de2..cc09502be 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -765,9 +765,7 @@ export class InteractiveManager const edits: vscode.NotebookEdit[] = []; for (const cell of view.notebook.getCells().slice().reverse()) { const id = cell.metadata.rExecutionId as string | undefined; - const record = - id && - this.transcriptCells(view).find((cell) => cell.record.id === id)?.record; + const record = id && this.transcriptCell(view, id)?.record; if ( record && !view.executions.has(record.id) && @@ -1953,10 +1951,7 @@ export class InteractiveManager } // Re-running creates a new execution identity, even if the same cell is selected. const previousId = cell.metadata.rExecutionId as string | undefined; - if ( - previousId && - this.transcriptCells(view).some((cell) => cell.record.id === previousId) - ) { + if (previousId && this.transcriptCell(view, previousId)) { await this.submit(view, code, cell.metadata.rSource as SourceLocation | undefined); continue; } diff --git a/src/languageServerTransport.ts b/src/languageServerTransport.ts new file mode 100644 index 000000000..8161f485b --- /dev/null +++ b/src/languageServerTransport.ts @@ -0,0 +1,78 @@ +import * as net from 'node:net'; +import type { DisposableProcess } from './util'; + +/** Settle TCP startup even when R fails before it can connect to the listener. */ +export function connectTcpLanguageServer( + spawn: (port: number) => DisposableProcess, + signal?: AbortSignal, +): Promise<{ reader: net.Socket; writer: net.Socket }> { + return new Promise((resolve, reject) => { + let child: DisposableProcess | undefined; + let settled = false; + const cleanup = () => { + server.removeListener('error', fail); + child?.removeListener('error', fail); + child?.removeListener('exit', exited); + signal?.removeEventListener('abort', aborted); + }; + const fail = (error: Error) => { + if (settled) { + return; + } + settled = true; + cleanup(); + try { + child?.dispose(); + } catch (cleanupError) { + error.cause ??= cleanupError; + } finally { + // Wait for close before rejecting so a failed start cannot leave + // a listening socket behind, including a listen/spawn error. + server.close(() => reject(error)); + } + }; + const exited = (code: number | null, signal: NodeJS.Signals | null) => { + fail( + new Error( + `R language server exited before connecting ${ + signal ? `from signal ${signal}` : `with exit code ${code ?? 'null'}` + }.`, + ), + ); + }; + const aborted = () => { + const error = new Error('R language server startup was cancelled.'); + error.name = 'AbortError'; + fail(error); + }; + const server = net.createServer((socket) => { + if (settled) { + socket.destroy(); + return; + } + settled = true; + cleanup(); + server.close(); + resolve({ reader: socket, writer: socket }); + }); + server.once('error', fail); + signal?.addEventListener('abort', aborted, { once: true }); + if (signal?.aborted) { + aborted(); + return; + } + server.listen(0, '127.0.0.1', () => { + try { + child = spawn((server.address() as net.AddressInfo).port); + if (settled) { + child.dispose(); + return; + } + child.once('error', fail); + child.once('exit', exited); + } catch (error) { + fail(error instanceof Error ? error : new Error(String(error))); + } + }); + }); +} diff --git a/src/languageService.ts b/src/languageService.ts index 5b047841a..a6073d016 100644 --- a/src/languageService.ts +++ b/src/languageService.ts @@ -2,7 +2,6 @@ import * as os from 'os'; import { basename, dirname } from 'path'; -import * as net from 'net'; import { URL } from 'url'; import * as fs from 'fs'; import { @@ -38,6 +37,7 @@ import { CommonOptions } from 'child_process'; import { boundSessionForDocument, onDidBindSessionDocument, Session } from './session'; import { SessionSignatureHelpProvider } from './signatureHelp'; import { completionMiddleware } from './completionMiddleware'; +import { connectTcpLanguageServer } from './languageServerTransport'; export class LanguageService implements Disposable { private readonly clients: Map = new Map(); @@ -47,16 +47,21 @@ export class LanguageService implements Disposable { private readonly sessionSignatures = new SessionSignatureHelpProvider(); private readonly clientUpdates = new Map>(); private readonly listeners: Disposable[] = []; + private readonly startup: Promise; + private readonly startupCancellation = new AbortController(); private disposed = false; constructor() { this.outputChannel = window.createOutputChannel('R Language Server', { log: true }); this.config = workspace.getConfiguration('r'); - void this.startLanguageService(); + this.startup = this.startLanguageService().catch((error: unknown) => { + this.outputChannel.appendLine(`Failed to start language server: ${String(error)}`); + }); } dispose(): Thenable { this.disposed = true; + this.startupCancellation.abort(); this.listeners.forEach((listener) => { listener.dispose(); }); @@ -94,8 +99,12 @@ export class LanguageService implements Disposable { client.outputChannel.show(); } } - if (client.needsStop()) { - void client.stop(); + if (client.isRunning()) { + void client.stop().catch((error: unknown) => { + client.outputChannel.appendLine( + `Failed to stop language server: ${String(error)}`, + ); + }); } }); return childProcess; @@ -174,29 +183,11 @@ export class LanguageService implements Disposable { rScriptPath, ); - const tcpServerOptions = () => - new Promise((resolve, reject) => { - // Use a TCP socket because of problems with blocking STDIO - const server = net.createServer((socket) => { - // 'connection' listener - console.log('R process connected'); - socket.on('end', () => { - console.log('R process disconnected'); - }); - socket.on('error', (e: Error) => { - console.log(`R process error: ${e.message}`); - reject(e); - }); - server.close(); - resolve({ reader: socket, writer: socket }); - }); - // Listen on random port - server.listen(0, '127.0.0.1', () => { - const port = (server.address() as net.AddressInfo).port; - env.VSCR_LSP_PORT = String(port); - return this.spawnServer(client, rPath, args, options); - }); - }); + const tcpServerOptions = (): Promise => + connectTcpLanguageServer((port) => { + env.VSCR_LSP_PORT = String(port); + return this.spawnServer(client, rPath, args, options); + }, this.startupCancellation.signal); // Options to control the language client const clientOptions: LanguageClientOptions = { @@ -608,12 +599,17 @@ export class LanguageService implements Disposable { this.outputChannel, workspaceFolder?.uri, ); + if (this.disposed) { + await client.stop(); + return; + } this.clients.set('global', client); this.startMultiLanguageService(true); } } private async stopLanguageService(): Promise { + await this.startup; await Promise.all(this.clientUpdates.values()); const promises: Thenable[] = []; for (const client of this.clients.values()) { diff --git a/src/plotViewer/httpgdViewer.ts b/src/plotViewer/httpgdViewer.ts index 257ac279d..332696246 100644 --- a/src/plotViewer/httpgdViewer.ts +++ b/src/plotViewer/httpgdViewer.ts @@ -38,28 +38,36 @@ export class HttpgdManager { public async showViewer(urlString: string): Promise { await Promise.resolve(); const url = new URL(urlString); - const host = url.host; const token = url.searchParams.get('token') || undefined; - const ind = this.viewers.findIndex((viewer) => viewer.host === host); + const ind = this.viewers.findIndex((viewer) => viewer.origin === url.origin); if (ind >= 0) { const viewer = this.viewers.splice(ind, 1)[0]; - this.viewers.unshift(viewer); - viewer.show(); - } else { - const conf = config(); - const colorTheme = conf.get('plot.defaults.colorTheme', 'vscode'); - this.viewerOptions.stripStyles = colorTheme === 'vscode'; - this.viewerOptions.previewPlotLayout = conf.get( - 'plot.defaults.plotPreviewLayout', - 'multirow', + if (viewer.token === token) { + this.viewers.unshift(viewer); + viewer.show(); + return; + } + // A restarted device can reuse its port with a new authentication + // token. Retire the connection rather than reusing stale credentials. + viewer.dispose(); + viewer.webviewPanel?.dispose(); + this.recentlyActiveViewers = this.recentlyActiveViewers.filter( + (active) => active !== viewer, ); - this.viewerOptions.refreshTimeoutLength = conf.get('plot.timing.refreshInterval', 10); - this.viewerOptions.resizeTimeoutLength = conf.get('plot.timing.resizeInterval', 100); - this.viewerOptions.fullWindow = conf.get('plot.defaults.fullWindowMode', false); - this.viewerOptions.token = token; - const viewer = new HttpgdViewer(host, this.viewerOptions); - this.viewers.unshift(viewer); } + const conf = config(); + const colorTheme = conf.get('plot.defaults.colorTheme', 'vscode'); + this.viewerOptions.stripStyles = colorTheme === 'vscode'; + this.viewerOptions.previewPlotLayout = conf.get( + 'plot.defaults.plotPreviewLayout', + 'multirow', + ); + this.viewerOptions.refreshTimeoutLength = conf.get('plot.timing.refreshInterval', 10); + this.viewerOptions.resizeTimeoutLength = conf.get('plot.timing.resizeInterval', 100); + this.viewerOptions.fullWindow = conf.get('plot.defaults.fullWindowMode', false); + this.viewerOptions.token = token; + const viewer = new HttpgdViewer(url.origin, this.viewerOptions); + this.viewers.unshift(viewer); } public registerActiveViewer(viewer: HttpgdViewer): void { @@ -115,6 +123,7 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { readonly id: string; readonly parent: HttpgdManager; readonly host: string; + readonly origin: string; readonly token?: string; webviewPanel?: vscode.WebviewPanel; readonly api: HttpgdClient; @@ -163,12 +172,14 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { } constructor(host: string, options: HttpgdViewerOptions) { - this.host = host; - this.id = host; + const url = new URL(host.includes('://') ? host : `http://${host}`); + this.host = url.host; + this.origin = url.origin; + this.id = this.host; this.token = options.token; this.parent = options.parent; - this.api = new HttpgdClient(this.host, this.token); + this.api = new HttpgdClient(this.origin, this.token); this.api.onPlotsChanged((newState) => { void this.refreshPlotsDelayed(newState.plots).catch((err) => { this.reportRefreshError(err); @@ -206,7 +217,7 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { this.stripStyles = options.stripStyles ?? this.defaultStripStyles; this.previewPlotLayout = options.previewPlotLayout ?? this.defaultPreviewPlotLayout; this.fullWindow = options.fullWindow ?? this.defaultFullWindow; - this.resizeTimeoutLength = options.refreshTimeoutLength ?? this.resizeTimeoutLength; + this.resizeTimeoutLength = options.resizeTimeoutLength ?? this.resizeTimeoutLength; this.refreshTimeoutLength = options.refreshTimeoutLength ?? this.refreshTimeoutLength; void this.api.connect().catch((err) => { if (!this.disposed) { @@ -269,11 +280,18 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { } public openExternal(): void { - let urlString = `http://${this.host}/live`; + const url = new URL('/live', this.origin); if (this.token) { - urlString += `?token=${this.token}`; - } - const uri = vscode.Uri.parse(urlString); + url.searchParams.set('token', this.token); + } + // Uri.parse decodes query escapes, including delimiters within a token. + // Preserve URLSearchParams' encoding when handing the URL to VS Code. + const uri = vscode.Uri.from({ + scheme: url.protocol.slice(0, -1), + authority: url.host, + path: url.pathname, + query: url.search.slice(1), + }); void vscode.env.openExternal(uri); } @@ -283,7 +301,7 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { if ( plt && (plt.height !== this.viewHeight || - plt.width !== this.viewHeight || + plt.width !== this.viewWidth || plt.zoom !== this.zoom) ) { await this.refreshPlots(this.api.getPlots()); @@ -489,10 +507,11 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { return; } const nPlots = this.plots.length; - let plotIds = plotsIdResponse.map((x) => x.id); - plotIds = plotIds.filter((id) => !this.hiddenPlots.includes(id)); + const hidden = new Set(this.hiddenPlots); + const existing = new Map(this.plots.map((plot) => [plot.id, plot])); + const plotIds = plotsIdResponse.map((x) => x.id).filter((id) => !hidden.has(id)); const newPlotPromises = plotIds.map(async (id) => { - const plot = this.plots.find((plt) => plt.id === id); + const plot = existing.get(id); if (force || !plot || id === this.activePlot) { return await this.getPlotContent(id, this.viewWidth, this.viewHeight, this.zoom); } else { @@ -503,7 +522,7 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { if (this.disposed) { return; } - const oldPlotIds = this.plots.map((plt) => plt.id); + const oldPlotIds = new Set(this.plots.map((plt) => plt.id)); this.plots = newPlots; if (this.plots.length !== nPlots) { this.activePlot = this.plots[this.plots.length - 1]?.id; @@ -512,7 +531,7 @@ export class HttpgdViewer implements IHttpgdViewer, PlotViewer { this.refreshHtml(); } else { for (const plt of this.plots) { - if (oldPlotIds.includes(plt.id)) { + if (oldPlotIds.has(plt.id)) { this.updatePlot(plt); } else { this.addPlot(plt); diff --git a/src/plotViewer/standardViewer.ts b/src/plotViewer/standardViewer.ts index 16633c16e..7b4efa239 100644 --- a/src/plotViewer/standardViewer.ts +++ b/src/plotViewer/standardViewer.ts @@ -76,19 +76,22 @@ export class StandardPlotViewer implements PlotViewer { ); this.panel = panel; const generation = ++this.panelGeneration; + let initialRequest = false; panel.iconPath = new UriIcon('graph'); - panel.webview.html = this.getHtml(); panel.webview.onDidReceiveMessage( async (msg: { type: string; width?: number; height?: number }) => { if ( this.panel === panel && this.panelGeneration === generation && - msg.type === 'resize' + (msg.type === 'resize' || (msg.type === 'ready' && !initialRequest)) ) { - this.viewWidth = msg.width || this.viewWidth; - this.viewHeight = msg.height || this.viewHeight; + initialRequest = true; + if (msg.type === 'resize') { + this.viewWidth = msg.width || this.viewWidth; + this.viewHeight = msg.height || this.viewHeight; + } await this.requestPlot(); } }, @@ -100,6 +103,9 @@ export class StandardPlotViewer implements PlotViewer { this.panelGeneration++; } }); + // Install the receiver before starting the document so its first ready + // message cannot be missed, even without a visible layout/resize event. + panel.webview.html = this.getHtml(); } private requestPlot(): Promise { @@ -293,6 +299,9 @@ export class StandardPlotViewer implements PlotViewer { } } }); + // Hidden webviews can suspend ResizeObserver callbacks. Request the + // initial plot once scripts are listening, then refine its size on resize. + vscode.postMessage({ type: 'ready' }); diff --git a/src/preview.ts b/src/preview.ts index 259432764..c5f7d3690 100644 --- a/src/preview.ts +++ b/src/preview.ts @@ -1,6 +1,7 @@ 'use strict'; -import { rmSync, statSync } from 'node:fs'; +import { rm, stat, access } from 'node:fs/promises'; +import { join } from 'node:path'; import { commands, extensions, window, workspace } from 'vscode'; import { runTextInTerm } from './rTerminal'; @@ -8,17 +9,17 @@ import { getWordOrSelection } from './selection'; import { config, checkForSpecialCharacters, - checkIfFileExists, delay, createTempDir, getCurrentWorkspaceFolder, + ToRStringLiteral, } from './util'; export async function previewEnvironment(): Promise { if (config().get('sessionWatcher')) { await runTextInTerm('View(globalenv())'); } else { - if (!checkcsv()) { + if (!(await checkcsv())) { return; } const currentWorkspaceFolder = getCurrentWorkspaceFolder()?.uri.fsPath; @@ -26,7 +27,7 @@ export async function previewEnvironment(): Promise { return; } const tmpDir = createTempDir(currentWorkspaceFolder, true); - const pathToTmpCsv = `${tmpDir}/environment.csv`; + const pathToTmpCsv = join(tmpDir, 'environment.csv'); const envName = 'name=ls()'; const envClass = 'class=sapply(ls(), function(x) {class(get(x, envir = parent.env(environment())))[1]})'; @@ -36,10 +37,9 @@ export async function previewEnvironment(): Promise { 'write.csv(data.frame(' + `${envName},` + `${envClass},` + - `${envOut}), '` + - `${pathToTmpCsv}', row.names=FALSE, quote = TRUE)`; - await runTextInTerm(rWriteCsvCommand); - await openTmpCSV(pathToTmpCsv, tmpDir); + `${envOut}), ` + + `${ToRStringLiteral(pathToTmpCsv, '"')}, row.names=FALSE, quote = TRUE)`; + await writeAndOpenTmpCSV(rWriteCsvCommand, pathToTmpCsv, tmpDir); } } @@ -48,7 +48,7 @@ export async function previewDataframe(): Promise { const symbol = getWordOrSelection(); await runTextInTerm(`View(${symbol || 'undefined'})`); } else { - if (!checkcsv()) { + if (!(await checkcsv())) { return; } const currentWorkspaceFolder = getCurrentWorkspaceFolder()?.uri.fsPath; @@ -70,70 +70,68 @@ export async function previewDataframe(): Promise { const tmpDir = createTempDir(currentWorkspaceFolder, true); // Create R write CSV command. Turn off row names and quotes, they mess with Excel Viewer. - const pathToTmpCsv = `${tmpDir}/${dataframeName}.csv`; - const rWriteCsvCommand = - `write.csv(${dataframeName}, ` + `'${pathToTmpCsv}', row.names = FALSE, quote = FALSE)`; - await runTextInTerm(rWriteCsvCommand); - await openTmpCSV(pathToTmpCsv, tmpDir); + const pathToTmpCsv = join(tmpDir, `${dataframeName}.csv`); + const rWriteCsvCommand = `write.csv(${dataframeName}, ${ToRStringLiteral(pathToTmpCsv, '"')}, row.names = FALSE, quote = FALSE)`; + await writeAndOpenTmpCSV(rWriteCsvCommand, pathToTmpCsv, tmpDir); } } -async function openTmpCSV(pathToTmpCsv: string, tmpDir: string): Promise { - await delay(350); // Needed since file size has not yet changed - - if (!checkIfFileExists(pathToTmpCsv)) { - void window.showErrorMessage('Dataframe failed to display.'); - rmSync(tmpDir, { recursive: true, force: true }); - - return false; - } - - // Async poll for R to complete writing CSV. - const success = await waitForFileToFinish(pathToTmpCsv); - if (!success) { - void window.showWarningMessage( - 'Visual Studio Code currently limits opening files to 20 MB.', +async function writeAndOpenTmpCSV( + command: string, + pathToTmpCsv: string, + tmpDir: string, +): Promise { + try { + const submitted = await runTextInTerm( + `base::invisible({ ${command}; base::file.create(${ToRStringLiteral(`${pathToTmpCsv}.ready`, '"')}) })`, ); - rmSync(tmpDir, { recursive: true, force: true }); - - return false; - } - - // Open CSV in Excel Viewer and clean up. - void workspace.openTextDocument(pathToTmpCsv).then(async (file) => { - await commands.executeCommand('csv.preview', file.uri); - rmSync(tmpDir, { recursive: true, force: true }); - }); -} - -async function waitForFileToFinish(filePath: string): Promise { - const fileBusy = true; - let currentSize = 0; - let previousSize = 1; - - while (fileBusy) { - const stats = statSync(filePath); - currentSize = stats.size; - - // UPDATE: We are now limited to 20 mb by MODEL_TOKENIZATION_LIMIT - // Https://github.com/Microsoft/vscode/blob/master/src/vs/editor/common/model/textModel.ts#L34 - if (currentSize > 2 * 10000000) { - // 20 MB + if (!submitted) { return false; } - - if (currentSize === previousSize) { - return true; + // R may be busy before it starts writing. A completion marker avoids both + // a fixed startup delay and mistaking a temporarily stable file for a full CSV. + const deadline = Date.now() + 30000; + let ready = false; + while (Date.now() < deadline) { + ready = await access(`${pathToTmpCsv}.ready`).then( + () => true, + () => false, + ); + if (ready) { + break; + } + await delay(50); } - previousSize = currentSize; - await delay(50); + if (!ready) { + void window.showErrorMessage( + 'Dataframe failed to display: R did not finish writing the CSV within 30 seconds.', + ); + return false; + } + if ((await stat(pathToTmpCsv)).size > 20 * 1000000) { + void window.showWarningMessage('The CSV preview supports files up to 20 MB.'); + return false; + } + const file = await workspace.openTextDocument(pathToTmpCsv); + await commands.executeCommand('csv.preview', file.uri); + } catch (error) { + void window.showErrorMessage(`Dataframe failed to display: ${String(error)}`); + return false; + } finally { + await rm(tmpDir, { recursive: true, force: true }); } } -function checkcsv(): boolean { +async function checkcsv(): Promise { const iscsv = extensions.getExtension('GrapeCity.gc-excelviewer'); - if (iscsv !== undefined && iscsv.isActive) { - return true; + if (iscsv !== undefined) { + try { + await iscsv.activate(); + return true; + } catch (error) { + void window.showErrorMessage(`Could not activate the CSV viewer: ${String(error)}`); + return false; + } } void window .showInformationMessage( diff --git a/src/rmarkdown/chunks.ts b/src/rmarkdown/chunks.ts index a70583298..2493ad99b 100644 --- a/src/rmarkdown/chunks.ts +++ b/src/rmarkdown/chunks.ts @@ -80,8 +80,17 @@ export interface RMarkdownChunk { codeRange: vscode.Range; } +const chunkCache = new WeakMap< + vscode.TextDocument, + { version: number; languageId: string; chunks: RMarkdownChunk[] } +>(); + // Scan document and return chunk info (e.g. ID, chunk range) from all chunks export function getChunks(document: vscode.TextDocument): RMarkdownChunk[] { + const cached = chunkCache.get(document); + if (cached?.version === document.version && cached.languageId === document.languageId) { + return cached.chunks; + } const lines = document.getText().split(/\r?\n/); const chunks: RMarkdownChunk[] = []; @@ -140,6 +149,11 @@ export function getChunks(document: vscode.TextDocument): RMarkdownChunk[] { if (isRDoc && chunkStartLine !== undefined) { finishChunk(lines.length - 1, lines.length - 1); } + chunkCache.set(document, { + version: document.version, + languageId: document.languageId, + chunks, + }); return chunks; } @@ -147,11 +161,6 @@ export function getCurrentChunk( chunks: RMarkdownChunk[], line: number, ): RMarkdownChunk | undefined { - const textEditor = vscode.window.activeTextEditor; - if (!textEditor) { - return; - } - // Case: If `chunks` is empty, return undefined if (chunks.length === 0) { return undefined; @@ -427,11 +436,7 @@ export function getCodeLenses( // Iterate through all code chunks for getting chunk information for both CodeLens and chunk background color (set by `editor.setDecorations`) let codeLenses: vscode.CodeLens[] = []; - for (let i = 1; i <= chunks.length; i++) { - const chunk = chunks.find((e) => e.id === i); - if (!chunk) { - continue; - } + for (const chunk of chunks) { const chunkRange = chunk.chunkRange; const line = chunk.startLine; diff --git a/src/rmarkdown/index.ts b/src/rmarkdown/index.ts index 061226696..e3b183d55 100644 --- a/src/rmarkdown/index.ts +++ b/src/rmarkdown/index.ts @@ -35,6 +35,10 @@ export class RMarkdownCodeLensProvider implements vscode.CodeLensProvider { private readonly currentCellTop: vscode.TextEditorDecorationType; private readonly currentCellBottom: vscode.TextEditorDecorationType; private onDidChangeTextEditorSelectionHandler: vscode.Disposable | undefined; + private readonly highlightedCells = new WeakMap< + vscode.TextEditor, + { version: number; chunk: RMarkdownChunk | undefined } + >(); constructor() { this.decoration = vscode.window.createTextEditorDecorationType({ @@ -57,54 +61,30 @@ export class RMarkdownCodeLensProvider implements vscode.CodeLensProvider { // Register the event listener and store the disposable this.onDidChangeTextEditorSelectionHandler = vscode.window.onDidChangeTextEditorSelection( - () => this.onDidChangeTextEditorSelection(), + ({ textEditor }) => { + if (isRDocument(textEditor.document)) { + this.highlightCurrentChunk(getChunks(textEditor.document), textEditor); + } + }, ); } - // Event handler for text editor selection change - private onDidChangeTextEditorSelection() { - // Get the active editor - const editor = vscode.window.activeTextEditor; - - if (editor) { - const document = editor.document; - const chunks = getChunks(document); - - // Call highlightCurrentChunk with the updated chunks and document - this.highlight(chunks, document); - } - } - - private highlightCurrentChunk(chunks: RMarkdownChunk[], document: vscode.TextDocument) { - for (const editor of vscode.window.visibleTextEditors) { - if (editor.document.uri.toString() === document.uri.toString()) { - const lines = document.getText().split(/\r?\n/); - const currentLine = editor.selection.active.line; - const currentChunk = getCurrentChunk(chunks, currentLine); - - if (currentChunk) { - // set top border - const currentChunkStart = new vscode.Range( - new vscode.Position(currentChunk.startLine, 0), - new vscode.Position( - currentChunk.startLine, - lines[currentChunk.startLine].length, - ), - ); - editor.setDecorations(this.currentCellTop, [currentChunkStart]); - - // set bottom border - const currentChunkEnd = new vscode.Range( - new vscode.Position(currentChunk.endLine, 0), - new vscode.Position( - currentChunk.endLine, - lines[currentChunk.endLine].length, - ), - ); - editor.setDecorations(this.currentCellBottom, [currentChunkEnd]); - } - } + private highlightCurrentChunk(chunks: RMarkdownChunk[], editor: vscode.TextEditor) { + const document = editor.document; + const currentChunk = getCurrentChunk(chunks, editor.selection.active.line); + const previous = this.highlightedCells.get(editor); + if (previous?.version === document.version && previous.chunk === currentChunk) { + return; } + this.highlightedCells.set(editor, { version: document.version, chunk: currentChunk }); + editor.setDecorations( + this.currentCellTop, + currentChunk ? [document.lineAt(currentChunk.startLine).range] : [], + ); + editor.setDecorations( + this.currentCellBottom, + currentChunk ? [document.lineAt(currentChunk.endLine).range] : [], + ); } private highlightChunks(chunks: RMarkdownChunk[], document: vscode.TextDocument) { @@ -124,7 +104,11 @@ export class RMarkdownCodeLensProvider implements vscode.CodeLensProvider { // Highlight differently for `.R` and `.Rmd` files const isRDoc = isRDocument(document); if (isRDoc) { - this.highlightCurrentChunk(chunks, document); + for (const editor of vscode.window.visibleTextEditors) { + if (editor.document === document) { + this.highlightCurrentChunk(chunks, editor); + } + } } else { this.highlightChunks(chunks, document); } @@ -154,6 +138,10 @@ export class RMarkdownCodeLensProvider implements vscode.CodeLensProvider { if (this.onDidChangeTextEditorSelectionHandler) { this.onDidChangeTextEditorSelectionHandler.dispose(); } + this.decoration.dispose(); + this.currentCellTop.dispose(); + this.currentCellBottom.dispose(); + this._onDidChangeCodeLenses.dispose(); } } diff --git a/src/rstudioapi.ts b/src/rstudioapi.ts index 22850ebf0..58608d37c 100644 --- a/src/rstudioapi.ts +++ b/src/rstudioapi.ts @@ -20,7 +20,7 @@ import { sessionDir, sessionDirectoryExists } from './session'; import { runTextInTerm, chooseTerminal } from './rTerminal'; import { config } from './util'; -let lastActiveTextEditor: TextEditor; +let lastActiveTextEditor: TextEditor | undefined; // Types for rstudioapi export type RSCoord = number | 'Inf' | '-Inf'; @@ -63,7 +63,7 @@ export function activeEditorContext(): RSDocumentContext { // path // contents // selection - a list of selections - const currentEditor = getLastActiveTextEditor(); + const currentEditor = requireActiveTextEditor(); const currentDocument = currentEditor.document; return { id: { external: currentDocument.uri.toString() }, @@ -153,7 +153,9 @@ export async function replaceTextInCurrentSelection( const target = findTargetUri(id); console.info(`[replaceTextInCurrentSelection] inserting: ${text} into ${target.path}`); const edit = new WorkspaceEdit(); - edit.replace(target, getLastActiveTextEditor().selection, text); + const targetDocument = await workspace.openTextDocument(target); + const editor = await reuseOrCreateEditor(targetDocument); + edit.replace(target, editor.selection, text); await applyEditorEdit(edit); } @@ -228,18 +230,24 @@ export async function setSelections(ranges: RSRange[], id: string | null): Promi export async function documentSave(id: string | null): Promise { const target = findTargetUri(id); const targetDocument = await workspace.openTextDocument(target); - await targetDocument.save(); + if (!(await targetDocument.save())) { + throw new Error('Could not save document.'); + } } export async function documentSaveAll(): Promise { - await workspace.saveAll(); + if (!(await workspace.saveAll())) { + throw new Error('Could not save all documents.'); + } } export async function documentClose(id: string | null, save: boolean): Promise { const target = findTargetUri(id); if (save) { const targetDocument = await workspace.openTextDocument(target); - await targetDocument.save(); + if (!(await targetDocument.save())) { + throw new Error('Could not save document; it remains open.'); + } } const tabs = window.tabGroups.all.flatMap((g) => g.tabs); const targetTabs = tabs.filter( @@ -256,18 +264,19 @@ export function projectPath(): { path: string | undefined } { if (workspace.workspaceFolders.length === 1) { // In single root common case, this will always work. return { - path: workspace.workspaceFolders[0].uri.path, + path: workspace.workspaceFolders[0].uri.fsPath, }; } else if (workspace.workspaceFolders.length > 1) { // In less common multi-root folder case is a bit tricky. If the active // text editor has scheme 'untitled:' (is unsaved), then // workspace.getWorkspaceFolder() won't be able to find its Uri in any // folder and will return undefined. - const currentDocument = getLastActiveTextEditor().document; - const currentDocFolder = workspace.getWorkspaceFolder(currentDocument.uri); + const currentDocument = getLastActiveTextEditor()?.document; + const currentDocFolder = + currentDocument && workspace.getWorkspaceFolder(currentDocument.uri); if (typeof currentDocFolder !== 'undefined') { return { - path: currentDocFolder.uri.path, + path: currentDocFolder.uri.fsPath, }; } } @@ -493,8 +502,16 @@ function getLastActiveTextEditor() { : window.activeTextEditor; } +function requireActiveTextEditor(): TextEditor { + const editor = getLastActiveTextEditor(); + if (!editor) { + throw new Error('No active text editor is available.'); + } + return editor; +} + function findTargetUri(id: string | null) { - return id === null ? getLastActiveTextEditor().document.uri : Uri.parse(id); + return id === null ? requireActiveTextEditor().document.uri : Uri.parse(id); } async function reuseOrCreateEditor(targetDocument: TextDocument) { @@ -503,19 +520,15 @@ async function reuseOrCreateEditor(targetDocument: TextDocument) { // Sometimes the last active is not visible in the case it was overtaken by a // WebViewPanel. - const KnownEditors: TextEditor[] = []; - - KnownEditors.push(lastActiveTextEditor); - KnownEditors.push(...window.visibleTextEditors); - - const matchingTextEditors = KnownEditors.filter( - (editor) => editor.document.uri.toString() === targetDocument.uri.toString(), + const knownEditors = [getLastActiveTextEditor(), ...window.visibleTextEditors]; + const matchingEditor = knownEditors.find( + (editor) => editor?.document.uri.toString() === targetDocument.uri.toString(), ); - if (matchingTextEditors.length === 0) { + if (!matchingEditor) { const newEditor = await window.showTextDocument(targetDocument, ViewColumn.Beside); return newEditor; } else { - return matchingTextEditors[0]; + return matchingEditor; } } diff --git a/src/selection.ts b/src/selection.ts index a068f2444..d171aec6c 100644 --- a/src/selection.ts +++ b/src/selection.ts @@ -231,7 +231,6 @@ export function extendSelection( const flagsFinish = { 0: false, 1: false }; // 1 represents looking forward, 0 represents looking back. let flagAbort = false; const unmatched = { 0: [] as string[], 1: [] as string[] }; - let curChar = ''; let quoteChar = ''; while (!flagAbort && !(flagsFinish[0] && flagsFinish[1])) { const { nextChar, nextPos, isEndOfCodeLine, isEndOfFile } = getNextChar( @@ -262,21 +261,15 @@ export function extendSelection( } } else { if (nextChar === quoteChar) { - if (lookingForward) { - if (curChar !== '\\') { - quoteChar = ''; - } - } else { - const next = getNextChar( - poss[lookingForward ? 1 : 0], - lookingForward, - getLineFromCache, - getEndsInOperatorFromCache, - lineCount, - ); - if (next.nextChar !== '\\') { - quoteChar = ''; - } + const text = getLineFromCache(nextPos.line); + let backslashes = 0; + for (let i = nextPos.character - 1; i >= 0 && text[i] === '\\'; i--) { + backslashes++; + } + // Paired backslashes escape each other; only an odd run escapes + // the quote, regardless of the direction used to scan the call. + if (backslashes % 2 === 0) { + quoteChar = ''; } } } @@ -291,8 +284,6 @@ export function extendSelection( flagAbort = true; } } - - curChar = nextChar; } if (flagAbort) { return { startLine: line, endLine: line }; diff --git a/src/session.ts b/src/session.ts index e2c6668bc..ecc11cb73 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1245,44 +1245,43 @@ export async function refreshTerminalDiscoveryFiles( if (attachmentLifecycleStopping) { return; } - for (const term of terminals) { - if (generation !== attachmentLifecycleGeneration || attachmentLifecycleStopping) { - return; - } - if (!isNativeTerminalEligible(term)) { - continue; - } - const envPath = terminalDiscoveryPath(term); - const pidWait = await nativeTerminalPidWaits.waitFor(term, () => term.processId, { - isEligible: () => isNativeTerminalEligible(term), - }); - if (pidWait.kind === 'invalidated') { - if (pidWait.reason === 'lifecycle') { + await Promise.all( + terminals.map(async (term) => { + if (generation !== attachmentLifecycleGeneration || attachmentLifecycleStopping) { return; } - continue; - } - const terminalPid = pidWait.value; - const isCurrent = () => - generation === attachmentLifecycleGeneration && - !attachmentLifecycleStopping && - pidWait.isCurrent() && - isNativeTerminalEligible(term); - if (!terminalPid || !isCurrent()) { - continue; - } - let discoveryPath = envPath; - discoveryPath ??= await findDiscoveryFileForTerminal(terminalPid); - if (discoveryPath && isCurrent()) { - await updateTerminalSessionDiscoveryFile( - term, - discoveryPath, - pipePath, - terminalPid, - isCurrent, - ); - } - } + if (!isNativeTerminalEligible(term)) { + return; + } + const envPath = terminalDiscoveryPath(term); + const pidWait = await nativeTerminalPidWaits.waitFor(term, () => term.processId, { + isEligible: () => isNativeTerminalEligible(term), + }); + if (pidWait.kind === 'invalidated') { + return; + } + const terminalPid = pidWait.value; + const isCurrent = () => + generation === attachmentLifecycleGeneration && + !attachmentLifecycleStopping && + pidWait.isCurrent() && + isNativeTerminalEligible(term); + if (!terminalPid || !isCurrent()) { + return; + } + let discoveryPath = envPath; + discoveryPath ??= await findDiscoveryFileForTerminal(terminalPid); + if (discoveryPath && isCurrent()) { + await updateTerminalSessionDiscoveryFile( + term, + discoveryPath, + pipePath, + terminalPid, + isCurrent, + ); + } + }), + ); } export async function updateTerminalDiscovery(terminal: vscode.Terminal): Promise { @@ -3602,38 +3601,87 @@ async function findLocalTerminal( signal?: AbortSignal, ): Promise { const candidates = new Map(); - for (const terminal of window.terminals) { + const pending = new Map(); + const cancelPending = (): void => { + for (const controller of pending.values()) { + controller.abort(); + } + }; + let invalidated = false; + let exactMatch!: (candidate: NativeTerminalCandidate) => void; + const exact = new Promise((resolve) => { + exactMatch = resolve; + }); + // A terminal's processId can remain pending indefinitely. Read candidates + // concurrently, return exact matches immediately, and bound other PID waits. + const timer = setTimeout(cancelPending, 1000); + signal?.addEventListener('abort', cancelPending, { once: true }); + try { if (signal?.aborted) { return undefined; } - if (!isNativeTerminalEligible(terminal)) { - continue; - } - const isCurrent = terminalRegistry.snapshot(terminal); - const pidWait = await nativeTerminalPidWaits.waitFor(terminal, () => terminal.processId, { - signal, - isEligible: () => isNativeTerminalEligible(terminal), - }); - if (pidWait.kind === 'invalidated') { - if (pidWait.reason !== 'terminal') { - return undefined; + const reads = window.terminals.filter(isNativeTerminalEligible).map(async (terminal) => { + if (signal?.aborted || invalidated) { + return; } - continue; - } - const terminalPid = pidWait.value; - if (terminalPid === undefined || !pidWait.isCurrent() || !isCurrent()) { - continue; + const isCurrent = terminalRegistry.snapshot(terminal); + const controller = new AbortController(); + pending.set(terminal, controller); + try { + const pidWait = await nativeTerminalPidWaits.waitFor( + terminal, + () => terminal.processId, + { + signal: controller.signal, + isEligible: () => isNativeTerminalEligible(terminal), + }, + ); + if (pidWait.kind === 'invalidated') { + if (pidWait.reason === 'lifecycle') { + invalidated = true; + cancelPending(); + } + return; + } + const terminalPid = pidWait.value; + const candidate = { + terminal, + pid: String(terminalPid), + isCurrent: () => + !signal?.aborted && + pidWait.isCurrent() && + isCurrent() && + isNativeTerminalEligible(terminal), + }; + if (terminalPid === undefined || !candidate.isCurrent()) { + return; + } + candidates.set(terminalPid, candidate); + if (candidate.pid === rPid) { + exactMatch(candidate); + } + } finally { + // Completed candidates keep their validity checks after we + // cancel unfinished reads on early return or at the deadline. + pending.delete(terminal); + } + }); + const complete = Promise.all(reads).then(() => undefined); + const direct = await Promise.race([exact, complete]); + if (signal?.aborted || invalidated) { + return undefined; } - const candidate = { - terminal, - pid: String(terminalPid), - isCurrent: () => - pidWait.isCurrent() && isCurrent() && isNativeTerminalEligible(terminal), - }; - if (candidate.pid === rPid) { - return candidate; + if (direct?.isCurrent()) { + return direct; } - candidates.set(terminalPid, candidate); + await complete; + } finally { + clearTimeout(timer); + signal?.removeEventListener('abort', cancelPending); + cancelPending(); + } + if (signal?.aborted || invalidated) { + return undefined; } if (!candidates.size) { return undefined; diff --git a/src/test/common/editorState.ts b/src/test/common/editorState.ts index 207d74fd5..565681d63 100644 --- a/src/test/common/editorState.ts +++ b/src/test/common/editorState.ts @@ -40,7 +40,7 @@ export async function executeEditorCommand( await vscode.commands.executeCommand(command, { text }); await waitForEditorState( () => editor.document.version > version, - `The ${command} command did not change the intended document`, + `The ${command} command for ${JSON.stringify(text)} did not change the intended document`, ); await vscode.commands.executeCommand('revealLine', { lineNumber: editor.selection.active.line, diff --git a/src/test/node/interactiveCore.test.ts b/src/test/node/interactiveCore.test.ts index 8ecd928c6..b886146fc 100644 --- a/src/test/node/interactiveCore.test.ts +++ b/src/test/node/interactiveCore.test.ts @@ -342,6 +342,27 @@ suite('Interactive protocol and persistence', () => { assert.throws(() => parser.push(Buffer.alloc(101, 97)), /size limit/); }); + test('bounds each fragmented frame by UTF-8 bytes and resets at blank and CRLF lines', () => { + const received: Record[] = []; + const first = JSON.stringify({ text: 'λ🙂'.repeat(10000) }); + const limit = Buffer.byteLength(first); + const parser = new JsonLines((message) => received.push(message), limit); + const bytes = Buffer.from(first + '\n\n {"text":"next"}\r\n'); + for (let start = 0; start < bytes.length; start += 7) { + parser.push(bytes.subarray(start, start + 7)); + } + assert.deepStrictEqual(received, [{ text: 'λ🙂'.repeat(10000) }, { text: 'next' }]); + const oversized = new JsonLines( + () => assert.fail('Oversized frames must not be delivered'), + limit, + ); + const incomplete = Buffer.from(first); + for (let start = 0; start < incomplete.length; start += 13) { + oversized.push(incomplete.subarray(start, start + 13)); + } + assert.throws(() => oversized.push(Buffer.from(' \n')), /size limit/); + }); + test('session probes verify identity and live status without claiming control and bound unresponsive endpoints', async function () { if (process.platform === 'win32') { this.skip(); @@ -411,6 +432,52 @@ suite('Interactive protocol and persistence', () => { assert.strictEqual(await probeSession(manifest), undefined); }); + test('a failed handshake closes its socket and a canceled attempt leaves its replacement connected', async function () { + if (process.platform === 'win32') { + this.skip(); + } + const manifest = { + id: 'connection', + generation: 'first', + protocol: 1, + endpoint: path.join(directory, 'connection.sock'), + token: 'test-token', + } as SessionManifest; + let identity = { ...manifest, generation: 'other' }; + let disconnected!: () => void; + const peerClosed = new Promise((resolve) => { + disconnected = resolve; + }); + const server = net.createServer((socket) => { + socket.on('error', () => undefined); + socket.on('close', disconnected); + const parser = new JsonLines((message) => { + socket.write(JSON.stringify({ id: message.id, result: identity }) + '\n'); + }); + socket.on('data', (chunk: Buffer) => parser.push(chunk)); + }); + const client = new AgentClient(manifest); + try { + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(manifest.endpoint, resolve); + }); + await assert.rejects(client.connect({ claim: false }), /identity changed/); + await peerClosed; + assert.strictEqual(client.connected, false); + + identity = manifest; + const canceled = client.connect({ claim: false, timeout: 1000 }); + const replacement = client.connect({ claim: false, timeout: 1000 }); + await Promise.all([assert.rejects(canceled, /disconnected/), replacement]); + assert.strictEqual(client.connected, true); + assert.deepStrictEqual(await client.request('heartbeat'), manifest); + } finally { + client.close(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + test('admission survives reopening and never accepts changed code with a reused ID', () => { let journal = new SessionJournal(directory, 'generation'); const first = journal.accept({ id: 'one', code: 'counter <- counter + 1' }); diff --git a/src/test/node/languageServerTransport.test.ts b/src/test/node/languageServerTransport.test.ts new file mode 100644 index 000000000..b56c64fa1 --- /dev/null +++ b/src/test/node/languageServerTransport.test.ts @@ -0,0 +1,141 @@ +import * as assert from 'node:assert'; +import { EventEmitter } from 'node:events'; +import * as net from 'node:net'; +import { connectTcpLanguageServer } from '../../languageServerTransport'; +import type { DisposableProcess } from '../../util'; + +suite('Language server TCP startup', function () { + this.timeout(2000); + + function mockChild() { + let disposals = 0; + const child = Object.assign(new EventEmitter(), { + dispose: () => { + disposals++; + }, + }) as unknown as DisposableProcess; + return { + child, + get disposals() { + return disposals; + }, + }; + } + + async function assertPortClosed(port: number) { + const socket = net.createConnection({ host: '127.0.0.1', port }); + try { + const result = await new Promise((resolve) => { + socket.once('error', resolve); + socket.once('connect', () => resolve(undefined)); + }); + assert.ok(result, 'a failed startup must close its listening port'); + assert.strictEqual((result as NodeJS.ErrnoException).code, 'ECONNREFUSED'); + } finally { + socket.destroy(); + } + } + + test('early R exit rejects startup and closes the listening socket', async () => { + const child = mockChild(); + let port = 0; + const startup = connectTcpLanguageServer((assignedPort) => { + port = assignedPort; + setImmediate(() => child.child.emit('exit', 10, null)); + return child.child; + }); + await assert.rejects(startup, /exited before connecting with exit code 10/); + assert.strictEqual(child.disposals, 1); + assert.strictEqual(child.child.listenerCount('error'), 0); + assert.strictEqual(child.child.listenerCount('exit'), 0); + await assertPortClosed(port); + }); + + test('asynchronous spawn errors reject startup and clean up the child', async () => { + const child = mockChild(); + let port = 0; + const error = new Error('ENOENT'); + const startup = connectTcpLanguageServer((assignedPort) => { + port = assignedPort; + setImmediate(() => child.child.emit('error', error)); + return child.child; + }); + await assert.rejects(startup, (candidate) => candidate === error); + assert.strictEqual(child.disposals, 1); + await assertPortClosed(port); + }); + + test('synchronous spawn failures release their listener', async () => { + let port = 0; + const error = new Error('spawn failed'); + const startup = connectTcpLanguageServer((assignedPort) => { + port = assignedPort; + throw error; + }); + await assert.rejects(startup, (candidate) => candidate === error); + await assertPortClosed(port); + }); + + test('cancellation terminates R and closes a listener while startup is still pending', async () => { + const child = mockChild(); + const cancellation = new AbortController(); + let started!: (port: number) => void; + const listening = new Promise((resolve) => { + started = resolve; + }); + const startup = connectTcpLanguageServer((port) => { + started(port); + return child.child; + }, cancellation.signal); + const port = await listening; + const rejected = assert.rejects( + startup, + (error: unknown) => error instanceof Error && error.name === 'AbortError', + ); + cancellation.abort(); + await rejected; + assert.strictEqual(child.disposals, 1); + assert.strictEqual(child.child.listenerCount('error'), 0); + assert.strictEqual(child.child.listenerCount('exit'), 0); + await assertPortClosed(port); + }); + + test('an already cancelled start never launches R', async () => { + const cancellation = new AbortController(); + cancellation.abort(); + let spawns = 0; + await assert.rejects( + connectTcpLanguageServer(() => { + spawns++; + return mockChild().child; + }, cancellation.signal), + (error: unknown) => error instanceof Error && error.name === 'AbortError', + ); + assert.strictEqual(spawns, 0); + }); + + test('a connection succeeds after a failed attempt without disposing the running child', async () => { + await assert.rejects( + connectTcpLanguageServer(() => { + throw new Error('first attempt'); + }), + ); + const child = mockChild(); + const cancellation = new AbortController(); + let socket: net.Socket | undefined; + const transport = await connectTcpLanguageServer((port) => { + socket = net.createConnection({ host: '127.0.0.1', port }); + return child.child; + }, cancellation.signal); + try { + cancellation.abort(); + assert.strictEqual(transport.reader, transport.writer); + assert.strictEqual(child.disposals, 0); + assert.strictEqual(child.child.listenerCount('error'), 0); + assert.strictEqual(child.child.listenerCount('exit'), 0); + } finally { + socket?.destroy(); + transport.reader.destroy(); + } + }); +}); diff --git a/src/test/suite/chunkDecorations.test.ts b/src/test/suite/chunkDecorations.test.ts new file mode 100644 index 000000000..96aaba02f --- /dev/null +++ b/src/test/suite/chunkDecorations.test.ts @@ -0,0 +1,84 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { RMarkdownCodeLensProvider } from '../../rmarkdown'; + +suite('R chunk decorations', () => { + let sandbox: sinon.SinonSandbox; + let provider: RMarkdownCodeLensProvider; + let selectionChanged: (event: vscode.TextEditorSelectionChangeEvent) => unknown; + let decorationDisposals: sinon.SinonStub[]; + let listenerDisposal: sinon.SinonStub; + + setup(() => { + sandbox = sinon.createSandbox(); + decorationDisposals = []; + listenerDisposal = sandbox.stub(); + sandbox.stub(vscode.window, 'createTextEditorDecorationType').callsFake(() => { + const dispose = sandbox.stub(); + decorationDisposals.push(dispose); + return { key: String(decorationDisposals.length), dispose }; + }); + sandbox.stub(vscode.window, 'onDidChangeTextEditorSelection').callsFake((listener) => { + selectionChanged = listener; + return new vscode.Disposable(listenerDisposal); + }); + provider = new RMarkdownCodeLensProvider(); + }); + teardown(() => { + provider.dispose(); + sandbox.restore(); + }); + + test('cursor movement inside a chunk reuses its scan and borders', () => { + let text = '# %%\nx <- 1\ny <- 2'; + const getText = sandbox.stub().callsFake(() => text); + const document = { + version: 1, + languageId: 'r', + getText, + lineAt: (line: number) => ({ + range: new vscode.Range(line, 0, line, text.split('\n')[line].length), + }), + } as unknown as vscode.TextDocument; + const decorations = sandbox.stub(); + const editor = { + document, + selection: new vscode.Selection(1, 0, 1, 0), + setDecorations: decorations, + } as unknown as vscode.TextEditor; + const event = { textEditor: editor } as vscode.TextEditorSelectionChangeEvent; + selectionChanged(event); + assert.strictEqual(getText.callCount, 1); + assert.strictEqual(decorations.callCount, 2); + + Object.assign(editor, { selection: new vscode.Selection(2, 0, 2, 0) }); + selectionChanged(event); + assert.strictEqual(getText.callCount, 1); + assert.strictEqual(decorations.callCount, 2); + + text = 'x <- 1\ny <- 2'; + Object.assign(document, { version: 2 }); + selectionChanged(event); + assert.strictEqual(getText.callCount, 2); + assert.deepStrictEqual(decorations.getCall(2).args[1], []); + assert.deepStrictEqual(decorations.getCall(3).args[1], []); + }); + + test('selection changes in other languages do not scan the document', () => { + const getText = sandbox.stub().returns('large unrelated document'); + selectionChanged({ + textEditor: { + document: { languageId: 'python', getText }, + }, + } as unknown as vscode.TextEditorSelectionChangeEvent); + sinon.assert.notCalled(getText); + }); + + test('disposal releases every decoration type and the selection listener', () => { + provider.dispose(); + assert.strictEqual(decorationDisposals.length, 3); + decorationDisposals.forEach((dispose) => sinon.assert.calledOnce(dispose)); + sinon.assert.calledOnce(listenerDisposal); + }); +}); diff --git a/src/test/suite/chunks.test.ts b/src/test/suite/chunks.test.ts index fdea14c42..3e5cfa4ae 100644 --- a/src/test/suite/chunks.test.ts +++ b/src/test/suite/chunks.test.ts @@ -1,6 +1,6 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; -import { getChunks } from '../../rmarkdown/chunks'; +import { getChunks, getCurrentChunk } from '../../rmarkdown/chunks'; suite('R code chunk boundaries', () => { async function chunkCode(text: string, language = 'r') { @@ -27,4 +27,37 @@ suite('R code chunk boundaries', () => { test('section headers end an R chunk without dropping its preceding line', async () => { assert.deepStrictEqual(await chunkCode('# %%\nx <- 1\n# section ----'), ['x <- 1']); }); + + test('reuses chunk scans until the document version or language changes', () => { + let scans = 0; + const document = { + version: 1, + languageId: 'r', + getText: () => { + scans++; + return '# %%\nx <- 1'; + }, + } as vscode.TextDocument; + const first = getChunks(document); + assert.strictEqual(getChunks(document), first); + assert.strictEqual(scans, 1); + + Object.assign(document, { version: 2 }); + assert.notStrictEqual(getChunks(document), first); + assert.strictEqual(scans, 2); + + Object.assign(document, { languageId: 'rmd' }); + assert.deepStrictEqual(getChunks(document), []); + assert.strictEqual(scans, 3); + }); + + test('current chunk lookup does not depend on an active editor', async () => { + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: '# %%\nx <- 1\n# %%\ny <- 2', + }); + const chunks = getChunks(document); + assert.strictEqual(getCurrentChunk(chunks, 0), chunks[0]); + assert.strictEqual(getCurrentChunk(chunks, 3), chunks[1]); + }); }); diff --git a/src/test/suite/completions.test.ts b/src/test/suite/completions.test.ts index 79b72eaff..7017334f6 100644 --- a/src/test/suite/completions.test.ts +++ b/src/test/suite/completions.test.ts @@ -3,7 +3,78 @@ import * as sinon from 'sinon'; import * as assert from 'assert'; import * as session from '../../session'; -import { HoverProvider, LiveCompletionItemProvider } from '../../completions'; +import * as extension from '../../extension'; +import * as util from '../../util'; +import { + HelpLinkHoverProvider, + HoverProvider, + LiveCompletionItemProvider, +} from '../../completions'; + +suite('Help link hover', () => { + let sandbox: sinon.SinonSandbox; + let aliases: sinon.SinonStub; + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(util, 'config').returns({ + get: () => true, + } as unknown as vscode.WorkspaceConfiguration); + aliases = sandbox.stub().resolves([]); + sandbox.stub(extension, 'globalRHelp').value({ getMatchingAliases: aliases }); + }); + teardown(() => sandbox.restore()); + + test('punctuation does not look up the entire document as a help alias', async () => { + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: '# comment\nx <- 1', + }); + const hover = await new HelpLinkHoverProvider().provideHover( + document, + new vscode.Position(0, 0), + ); + assert.strictEqual(hover, null); + sinon.assert.notCalled(aliases); + }); + + test('an unmatched alias contributes no empty hover', async () => { + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: 'unknown_function()', + }); + assert.strictEqual( + await new HelpLinkHoverProvider().provideHover(document, new vscode.Position(0, 5)), + null, + ); + sinon.assert.calledOnceWithExactly(aliases, 'unknown_function'); + }); + + test('cancellation discards a pending help alias lookup', async () => { + let finish!: (value: unknown[]) => void; + aliases.returns( + new Promise((resolve) => { + finish = resolve; + }), + ); + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: 'sum()', + }); + const cancellation = new vscode.CancellationTokenSource(); + try { + const hover = new HelpLinkHoverProvider().provideHover( + document, + new vscode.Position(0, 1), + cancellation.token, + ); + cancellation.cancel(); + finish([{ package: 'base', alias: 'sum', name: 'sum' }]); + assert.strictEqual(await hover, null); + } finally { + cancellation.dispose(); + } + }); +}); suite('Session hover and completion', () => { let sandbox: sinon.SinonSandbox; diff --git a/src/test/suite/extendSelection.test.ts b/src/test/suite/extendSelection.test.ts index 96e6d3c7f..9a0efbde2 100644 --- a/src/test/suite/extendSelection.test.ts +++ b/src/test/suite/extendSelection.test.ts @@ -12,6 +12,41 @@ import { extendSelection } from '../../selection'; // Defines a Mocha test suite to group tests of similar kind together suite('extendSelection Tests', () => { + test('selects complete calls containing strings ending in escaped backslashes', () => { + for (const text of [ + String.raw`print( + "a\\", 1 +) +next_call()`, + String.raw`print("a\\", + 1 +) +next_call()`, + String.raw`print( + 'a\\\\', 1 +) +next_call()`, + ]) { + const lines = text.split('\n'); + for (let line = 0; line <= 2; line++) { + assert.deepStrictEqual( + extendSelection(line, (i) => lines[i], lines.length), + { + startLine: 0, + endLine: 2, + }, + ); + } + assert.deepStrictEqual( + extendSelection(3, (i) => lines[i], lines.length), + { + startLine: 3, + endLine: 3, + }, + ); + } + }); + test('Selecting multi-line {} bracketed expression', () => { const doc = ` function (x) { diff --git a/src/test/suite/fileGeneration.test.ts b/src/test/suite/fileGeneration.test.ts index 015c1192a..4a7f81a38 100644 --- a/src/test/suite/fileGeneration.test.ts +++ b/src/test/suite/fileGeneration.test.ts @@ -61,6 +61,85 @@ suite('Generated file overwrite prompts', () => { assert.strictEqual(fs.readFileSync(cppPath, 'utf8'), 'user compiler settings'); }); + test('compiler probes run asynchronously and clean up after failure', async () => { + sandbox.stub(vscode.window, 'showWarningMessage').resolves('Yes' as never); + (util.getRpath as sinon.SinonStub).resolves('/R path/R'); + const cppPath = path.join(directory, '.vscode', 'c_cpp_properties.json'); + fs.mkdirSync(path.dirname(cppPath)); + fs.writeFileSync(cppPath, 'user compiler settings'); + const cleanupFile = process.platform === 'win32' ? 'cleanup.win' : 'cleanup'; + fs.writeFileSync(path.join(directory, cleanupFile), ''); + const cleanup = sandbox.stub(util, 'executeRCommand').resolves(''); + let finish!: (result: Awaited>) => void; + const probe = sandbox.stub(util, 'spawnAsync').returns( + new Promise((resolve) => { + finish = resolve; + }), + ); + let completed = false; + const result = generateCppProperties(); + void result.then( + () => { + completed = true; + }, + () => { + completed = true; + }, + ); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(completed, false); + sinon.assert.calledOnce(probe); + assert.deepStrictEqual(probe.firstCall.args.slice(0, 2), [ + '/R path/R', + ['CMD', 'SHLIB', 'comp_test.cpp'], + ]); + const temporary = String(probe.firstCall.args[2]?.cwd); + assert.strictEqual(fs.existsSync(path.join(temporary, 'comp_test.cpp')), true); + const rejected = assert.rejects(result, /compiler unavailable/); + finish({ + pid: 1, + output: [], + stdout: '', + stderr: 'compiler unavailable', + status: 1, + signal: null, + }); + await rejected; + assert.strictEqual(fs.existsSync(temporary), false); + assert.strictEqual(fs.readFileSync(cppPath, 'utf8'), 'user compiler settings'); + sinon.assert.calledOnce(cleanup); + }); + + test('compiler configuration handles executable and include paths containing spaces', async () => { + (util.getRpath as sinon.SinonStub).resolves('/R path/R'); + sandbox.stub(util, 'spawnAsync').resolves({ + pid: 1, + output: [], + stderr: '', + status: 0, + signal: null, + stdout: 'R CMD SHLIB\n"/compiler path/clang++" -I"include path" -DTEST -std=c++17\n', + }); + const command = sandbox.stub(util, 'executeRCommand').resolves('/compiler path/clang++'); + await generateCppProperties(); + assert.strictEqual(command.firstCall.args[0], 'cat(Sys.which("/compiler path/clang++"))'); + const settings = JSON.parse( + fs.readFileSync(path.join(directory, '.vscode', 'c_cpp_properties.json'), 'utf8'), + ) as { + configurations: Array<{ + compilerPath: string; + includePath: string[]; + defines: string[]; + }>; + }; + assert.strictEqual(settings.configurations[0].compilerPath, '/compiler path/clang++'); + assert.ok( + settings.configurations[0].includePath.some((value) => value.endsWith('include path')), + ); + assert.deepStrictEqual(settings.configurations[0].defines, ['TEST']); + assert.deepStrictEqual(fs.readdirSync(directory), ['.vscode']); + }); + test('unavailable R preserves the existing lint configuration after confirming overwrite', async () => { sandbox .stub(vscode.window, 'showWarningMessage') diff --git a/src/test/suite/helpPackages.test.ts b/src/test/suite/helpPackages.test.ts new file mode 100644 index 000000000..742c3eeda --- /dev/null +++ b/src/test/suite/helpPackages.test.ts @@ -0,0 +1,100 @@ +import * as assert from 'assert'; +import type { Memento } from 'vscode'; +import '../../extension'; +import { PackageManager } from '../../helpViewer/packages'; +import type { HelpFile, RHelp } from '../../helpViewer'; + +function jsonState(initial: Record = {}): Memento { + const values = new Map(Object.entries(initial)); + return { + keys: () => [...values.keys()], + get: (key: string, fallback?: T) => (values.get(key) as T | undefined) ?? fallback, + update: (key: string, value: unknown) => { + if (value === undefined) { + values.delete(key); + } else { + values.set(key, JSON.parse(JSON.stringify(value)) as unknown); + } + return Promise.resolve(); + }, + } as Memento; +} + +const index = (name: string): HelpFile => ({ + requestPath: '/library/example/html/00Index.html', + html: `
${name}Description
`, +}); + +function manager(state: Memento, getHelpFileForPath: () => Promise) { + return new PackageManager({ + rPath: 'R', + persistentState: state, + rHelp: { getHelpFileForPath } as unknown as RHelp, + }); +} + +suite('Help package state and index requests', () => { + test('favorites survive JSON persistence and can be removed after reloading', () => { + const state = jsonState(); + const first = manager(state, () => Promise.resolve(undefined)); + first.addFavorite('base'); + first.addFavorite('stats'); + const reloaded = manager(state, () => Promise.resolve(undefined)); + assert.deepStrictEqual([...reloaded.favoriteNames], ['base', 'stats']); + assert.deepStrictEqual(reloaded.removeFavorite('base'), ['stats']); + assert.deepStrictEqual( + [...manager(state, () => Promise.resolve(undefined)).favoriteNames], + ['stats'], + ); + }); + + test('recovers legacy serialized Sets and ignores invalid favorite values', () => { + for (const saved of [{}, ['base', 1, null], new Set(['base'])]) { + const packages = manager(jsonState({ 'r.helpPanel.favoriteNamesSet': saved }), () => + Promise.resolve(undefined), + ); + assert.doesNotThrow(() => packages.addFavorite('stats')); + assert.ok(packages.favoriteNames.has('stats')); + assert.ok([...packages.favoriteNames].every((name) => typeof name === 'string')); + } + }); + + test('concurrent topic requests share one pending index fetch', async () => { + let requests = 0; + let finish!: (file: HelpFile) => void; + const file = new Promise((resolve) => { + finish = resolve; + }); + const packages = manager(jsonState(), () => { + requests++; + return file; + }); + const first = packages.getTopics('example'); + const second = packages.getTopics('example'); + assert.strictEqual(requests, 1); + finish(index('first')); + const [a, b] = await Promise.all([first, second]); + assert.deepStrictEqual(a, b); + await packages.getTopics('example'); + assert.strictEqual(requests, 1); + }); + + test('clearing the cache prevents an older pending fetch from restoring stale topics', async () => { + let finish!: (file: HelpFile) => void; + const oldFile = new Promise((resolve) => { + finish = resolve; + }); + let requests = 0; + const packages = manager(jsonState(), () => + ++requests === 1 ? oldFile : Promise.resolve(index('latest')), + ); + const pending = packages.getTopics('example'); + await packages.clearCachedFiles(); + finish(index('stale')); + await pending; + const latest = await packages.getTopics('example'); + assert.strictEqual(requests, 2); + assert.ok(latest?.some((topic) => topic.name === 'latest')); + assert.ok(!latest?.some((topic) => topic.name === 'stale')); + }); +}); diff --git a/src/test/suite/helpPanel.test.ts b/src/test/suite/helpPanel.test.ts new file mode 100644 index 000000000..72edf50de --- /dev/null +++ b/src/test/suite/helpPanel.test.ts @@ -0,0 +1,81 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as path from 'path'; +import { HelpPanel } from '../../helpViewer/panel'; +import type { RHelp } from '../../helpViewer'; +import * as util from '../../util'; +import { mockExtensionContext } from '../common/mockvscode'; + +suite('Help panel navigation', () => { + let sandbox: sinon.SinonSandbox; + let panel: vscode.WebviewPanel; + let help: HelpPanel; + let closed: vscode.EventEmitter; + + setup(async () => { + sandbox = sinon.createSandbox(); + mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); + sandbox.stub(vscode.commands, 'executeCommand').resolves(); + sandbox + .stub(util, 'config') + .returns({ get: () => 'Two' } as unknown as vscode.WorkspaceConfiguration); + closed = new vscode.EventEmitter(); + panel = { + active: true, + reveal: sandbox.stub(), + dispose: () => closed.fire(), + onDidDispose: closed.event, + onDidChangeViewState: sandbox.stub(), + webview: { + html: '', + asWebviewUri: (uri: vscode.Uri) => uri, + onDidReceiveMessage: sandbox.stub(), + postMessage: sandbox.stub().resolves(true), + }, + } as unknown as vscode.WebviewPanel; + help = new HelpPanel( + { webviewScriptPath: '/help.js', webviewStylePath: '/help.css' }, + {} as RHelp, + panel, + ); + await help.showHelpFile({ requestPath: '/first', html: 'First' }); + await help.showHelpFile({ requestPath: '/second', html: 'Second' }); + }); + + teardown(() => { + try { + help?.dispose(); + closed?.dispose(); + } finally { + sandbox.restore(); + } + }); + + test('Back still works when a webview cannot acknowledge its scroll position', async () => { + const clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const pending = help.goBack(); + await clock.tickAsync(1000); + await pending; + assert.match(panel.webview.html, /First/); + assert.strictEqual(clock.countTimers(), 0); + }); + + test('closing the panel settles pending navigation without waiting for a timeout', async () => { + const clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const pending = help.goBack(); + panel.dispose(); + await pending; + assert.strictEqual(help.panel, undefined); + assert.strictEqual(clock.countTimers(), 0); + }); + + test('failed delivery falls back immediately instead of waiting for the scroll timeout', async () => { + const clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + (panel.webview.postMessage as sinon.SinonStub).resolves(false); + await help.goBack(); + await clock.tickAsync(0); + assert.match(panel.webview.html, /First/); + assert.strictEqual(clock.countTimers(), 0); + }); +}); diff --git a/src/test/suite/htmlWidgetBrowser.test.ts b/src/test/suite/htmlWidgetBrowser.test.ts index b155f0bab..6e23f4128 100644 --- a/src/test/suite/htmlWidgetBrowser.test.ts +++ b/src/test/suite/htmlWidgetBrowser.test.ts @@ -12,12 +12,41 @@ import { restoreHtmlViewer, runHtmlViewerCommand, showWebView, - shutdownHtmlWidgetViewers, } from '../../webViewer'; import { extensionContext } from '../../extension'; import { mockExtensionContext } from '../common/mockvscode'; import { waitForValue } from '../common/sessionConnections'; +const fixtureSandboxes = new Set(); +const fixtureViewers = new Map< + sinon.SinonSandbox, + ReturnType +>(); + +function createFixtureSandbox(): sinon.SinonSandbox { + const sandbox = sinon.createSandbox(); + fixtureSandboxes.add(sandbox); + return sandbox; +} + +function initializeFixtureViewers( + sandbox: sinon.SinonSandbox, + sessions: Parameters[1], +): ReturnType { + const manager = initializeHtmlWidgetViewers(extensionContext, sessions); + fixtureViewers.set(sandbox, manager); + return manager; +} + +async function shutdownFixtureViewers(sandbox: sinon.SinonSandbox): Promise { + const manager = fixtureViewers.get(sandbox); + fixtureViewers.delete(sandbox); + if (manager) { + manager.dispose(); + await manager.flush(); + } +} + async function focusHtmlViewer(panel: vscode.WebviewPanel): Promise { panel.reveal(panel.viewColumn, false); // reveal() queues a workbench request. Await editor focus before running @@ -31,9 +60,19 @@ async function focusHtmlViewer(panel: vscode.WebviewPanel): Promise { } suite('HTML widget browser rendering', () => { + teardown(async () => { + // Mocha timeouts do not cancel an async test body. Release its own + // manager and mocks here, even if the body's finally block runs later. + try { + await Promise.all([...fixtureViewers.keys()].map(shutdownFixtureViewers)); + } finally { + fixtureSandboxes.forEach((sandbox) => sandbox.restore()); + fixtureSandboxes.clear(); + } + }); test('CSP-blocked acknowledgements unlock navigation and stop the spinner after ten seconds in a real webview', async function () { this.timeout(30000); - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); const statusItems = sandbox.spy(vscode.window, 'createStatusBarItem'); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') @@ -51,7 +90,7 @@ suite('HTML widget browser rendering', () => { source.rVer = '4.6.1'; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - const manager = initializeHtmlWidgetViewers(extensionContext, { + const manager = initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -83,6 +122,7 @@ suite('HTML widget browser rendering', () => { const context = session.getViewerSessionContext(source.sessionId); await showWebView(first, 'First', 'Two', context); const entry = [...manager.viewers.values()][0]; + await focusHtmlViewer(entry.panel); await waitForValue(() => acknowledgements.includes(entry.revision) ? true : undefined, ); @@ -133,7 +173,7 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); @@ -141,7 +181,7 @@ suite('HTML widget browser rendering', () => { }); test('document edits preserve authored titles, SVG accessibility, and tag-shaped script text', async () => { - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -150,7 +190,7 @@ suite('HTML widget browser rendering', () => { let result: vscode.Uri | undefined; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - initializeHtmlWidgetViewers(extensionContext, { + initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -216,14 +256,14 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); } }); test('authored local bases load relative resources from their own directories', async () => { - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -240,7 +280,7 @@ suite('HTML widget browser rendering', () => { let result: vscode.Uri | undefined; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - initializeHtmlWidgetViewers(extensionContext, { + initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -330,14 +370,14 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); } }); test('authored bases cannot read files outside the output and trusted workspace', async () => { - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -376,7 +416,7 @@ suite('HTML widget browser rendering', () => { }, ]); mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - initializeHtmlWidgetViewers(extensionContext, { + initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -444,7 +484,7 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); @@ -452,7 +492,7 @@ suite('HTML widget browser rendering', () => { }); test('Cmd/Ctrl+F inside the widget opens native Find while other keys stay with the widget', async () => { - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -470,7 +510,7 @@ suite('HTML widget browser rendering', () => { source.rVer = '4.6.1'; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - initializeHtmlWidgetViewers(extensionContext, { + initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -548,7 +588,7 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); @@ -556,7 +596,7 @@ suite('HTML widget browser rendering', () => { }); test('fragment links scroll to IDs and named anchors inside the widget without leaving its document', async () => { - const sandbox = sinon.createSandbox(); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -573,7 +613,7 @@ suite('HTML widget browser rendering', () => { source.rVer = '4.6.1'; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - initializeHtmlWidgetViewers(extensionContext, { + initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -664,15 +704,16 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); } }); - test('history navigation keeps the previous HTML visible without rerunning it', async () => { - const sandbox = sinon.createSandbox(); + test('history navigation keeps the previous HTML visible without rerunning it', async function () { + this.timeout(45000); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -692,7 +733,7 @@ suite('HTML widget browser rendering', () => { source.rVer = '4.6.1'; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - const viewerManager = initializeHtmlWidgetViewers(extensionContext, { + const viewerManager = initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -713,6 +754,10 @@ suite('HTML widget browser rendering', () => { // Delay replacement delivery to exercise the interval between the // options and HTML workbench messages, without delaying a clear. const webview = panel.webview; + let disposed = false; + panel.onDidDispose(() => { + disposed = true; + }); const descriptor = Object.getOwnPropertyDescriptor( Object.getPrototypeOf(webview), 'html', @@ -729,7 +774,9 @@ suite('HTML widget browser rendering', () => { writes.push( new Promise((resolve) => setTimeout(() => { - descriptor.set!.call(webview, html); + if (!disposed) { + descriptor.set!.call(webview, html); + } resolve(); }, 500), ), @@ -767,6 +814,7 @@ suite('HTML widget browser rendering', () => { const html = fixture('html'); const loaded = async (count: number) => { await Promise.all(writes); + await focusHtmlViewer(panels[0]); await waitForValue(() => reports.length >= count && [...viewerManager.viewers.values()].some( @@ -851,15 +899,16 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); } }); - test('native actions navigate, remove, and restore full documents with relative resources', async () => { - const sandbox = sinon.createSandbox(); + test('native actions navigate, remove, and restore full documents with relative resources', async function () { + this.timeout(45000); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -876,7 +925,7 @@ suite('HTML widget browser rendering', () => { source.rVer = '4.6.1'; try { mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); - const viewerManager = initializeHtmlWidgetViewers(extensionContext, { + const viewerManager = initializeFixtureViewers(sandbox, { resolveSession: (source) => session.getViewerSessionContext(source.sessionId, source)!, getActiveSessionId: () => session.activeSession?.sessionId, @@ -933,6 +982,7 @@ suite('HTML widget browser rendering', () => { const second = fixture('second'); const count = (name: string) => urls.filter((uri) => uri.path === `/${name}`).length; const loaded = async (name: string, previous: number) => { + await focusHtmlViewer(panels.at(-1)!); const uri = await waitForValue(() => { const reports = urls.filter((uri) => uri.path === `/${name}`); const latest = reports.at(-1); @@ -1012,14 +1062,15 @@ suite('HTML widget browser rendering', () => { panels.forEach((panel) => { panel.dispose(); }); - await shutdownHtmlWidgetViewers(); + await shutdownFixtureViewers(sandbox); session.unregisterSessionTransport(source); sandbox.restore(); fs.rmSync(directory, { recursive: true, force: true }); } }); - test('registered native commands operate on HTML outputs received by the activated extension', async () => { - const sandbox = sinon.createSandbox(); + test('registered native commands operate on HTML outputs received by the activated extension', async function () { + this.timeout(45000); + const sandbox = createFixtureSandbox(); sandbox .stub(vscode.window, 'registerWebviewPanelSerializer') .returns({ dispose: sandbox.stub() }); @@ -1133,8 +1184,8 @@ suite('HTML widget browser rendering', () => { const generation = () => Number(/data-generation="(\d+)"/.exec(viewer.webview.html)?.[1]); const ready = () => loaded.get(viewer) === generation(); - await waitForValue(() => (ready() ? true : undefined)); await focusHtmlViewer(viewer); + await waitForValue(() => (ready() ? true : undefined)); const key = { back: 'canGoBack', forward: 'canGoForward', diff --git a/src/test/suite/httpgdViewer.test.ts b/src/test/suite/httpgdViewer.test.ts new file mode 100644 index 000000000..c522d4eaf --- /dev/null +++ b/src/test/suite/httpgdViewer.test.ts @@ -0,0 +1,97 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { HttpgdClient } from '../../plotViewer/httpgdClient'; +import { HttpgdManager, HttpgdViewer } from '../../plotViewer/httpgdViewer'; +import { mockExtensionContext } from '../common/mockvscode'; +import * as util from '../../util'; + +class TestViewer extends HttpgdViewer { + renderCount = 0; + resize(height: number, width: number): void { + this.handleResize(height, width); + } + protected override resizePlot(): Promise { + this.renderCount++; + return Promise.resolve(); + } + protected override refreshPlots(): Promise { + this.renderCount++; + return Promise.resolve(); + } +} + +suite('httpgd viewer requests', () => { + let sandbox: sinon.SinonSandbox; + let manager: HttpgdManager; + const extensionRoot = path.resolve(__dirname, '../../..'); + setup(() => { + sandbox = sinon.createSandbox(); + mockExtensionContext(extensionRoot, sandbox); + sandbox.stub(HttpgdClient.prototype, 'connect').resolves(); + sandbox.stub(vscode.commands, 'executeCommand').resolves(); + sandbox.stub(util, 'config').returns({ + get: (_key: string, fallback: unknown) => fallback, + } as vscode.WorkspaceConfiguration); + manager = new HttpgdManager(); + }); + teardown(() => { + try { + manager?.viewers.forEach((viewer) => viewer.dispose()); + } finally { + sandbox.restore(); + } + }); + + test('focusing a correctly sized non-square plot does not request another render', async () => { + const viewer = new TestViewer('localhost:1234', manager.viewerOptions); + manager.viewers.push(viewer); + viewer.plots = [{ id: '1', data: '', width: 800, height: 600, zoom: 1 }]; + await viewer.focusPlot('1'); + assert.strictEqual(viewer.renderCount, 0); + viewer.viewWidth = 900; + await viewer.focusPlot('1'); + assert.strictEqual(viewer.renderCount, 1); + }); + + test('resizing uses its configured interval independently of plot refresh', async () => { + const clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const viewer = new TestViewer('localhost:1234', { + ...manager.viewerOptions, + refreshTimeoutLength: 10, + resizeTimeoutLength: 100, + }); + manager.viewers.push(viewer); + viewer.resize(600, 900); + await clock.tickAsync(10); + assert.strictEqual(viewer.renderCount, 0); + await clock.tickAsync(90); + assert.strictEqual(viewer.renderCount, 1); + }); + + test('HTTPS URLs stay HTTPS when opening the live viewer', async () => { + await manager.showViewer('https://plots.example:8443/live?token=secret%26value'); + const open = sandbox.stub(vscode.env, 'openExternal').resolves(true); + manager.viewers[0].openExternal(); + assert.strictEqual( + open.firstCall.args[0].toString(true), + 'https://plots.example:8443/live?token=secret%26value', + ); + assert.strictEqual(manager.viewers[0].origin, 'https://plots.example:8443'); + }); + + test('a changed device token replaces the connection at the same origin', async () => { + await manager.showViewer('http://localhost:1234/live?token=first'); + const old = manager.viewers[0]; + const show = sandbox.stub(old, 'show'); + await manager.showViewer('http://localhost:1234/live?token=first'); + sinon.assert.calledOnce(show); + const disconnect = sandbox.spy(old.api, 'disconnect'); + await manager.showViewer('http://localhost:1234/live?token=second'); + sinon.assert.calledOnce(disconnect); + assert.strictEqual(manager.viewers.length, 1); + assert.notStrictEqual(manager.viewers[0], old); + assert.strictEqual(manager.viewers[0].token, 'second'); + }); +}); diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index 89ff5d79e..f2c83fe05 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -28,6 +28,7 @@ import { StreamMessageReader, StreamMessageWriter, createMessageConnection, + DocumentOnTypeFormattingRequest, } from 'vscode-languageclient/node'; function treeTooltip(item: vscode.TreeItem): string { @@ -1331,10 +1332,15 @@ function treeTooltip(item: vscode.TreeItem): string { () => languageService().clients.get(opened.inputUri.toString())?.isRunning() === true, ); - assert.ok( - languageService().clients.get(opened.inputUri.toString())?.initializeResult - ?.capabilities.documentOnTypeFormattingProvider, - 'languageserver must advertise on-type formatting', + // Current servers register formatting dynamically after initialize; + // the active provider also covers servers with static capabilities. + await until( + () => + languageService() + .clients.get(opened.inputUri.toString()) + ?.getFeature(DocumentOnTypeFormattingRequest.method) + .getProvider(document) !== undefined, + 'languageserver must register on-type formatting for the Interactive input', ); try { for (const [text, ch, expected] of [ diff --git a/src/test/suite/interactiveInputFeatures.test.ts b/src/test/suite/interactiveInputFeatures.test.ts index 56ac77154..b25892826 100644 --- a/src/test/suite/interactiveInputFeatures.test.ts +++ b/src/test/suite/interactiveInputFeatures.test.ts @@ -14,6 +14,7 @@ suite('Interactive input formatting with independent providers', () => { let providers: vscode.Disposable[]; let previousOnType: boolean | undefined; let previousOnPaste: boolean | undefined; + let previousAutoClosingBrackets: string | undefined; const selector = { scheme: 'vscode-interactive-input', language: 'r' }; const settings = (): vscode.WorkspaceConfiguration => vscode.workspace.getConfiguration('editor', { languageId: 'r' }); @@ -106,6 +107,23 @@ suite('Interactive input formatting with independent providers', () => { editor = vscode.window.activeTextEditor!; previousOnType = settings().inspect('formatOnType')?.globalLanguageValue; previousOnPaste = settings().inspect('formatOnPaste')?.globalLanguageValue; + previousAutoClosingBrackets = + settings().inspect('autoClosingBrackets')?.globalLanguageValue; + // This fixture verifies provider triggers. Auto-closing can consume a + // typed ')' by moving the cursor, which intentionally makes no edit. + // The language-server integration suite separately covers that behavior. + await settings().update( + 'autoClosingBrackets', + 'never', + vscode.ConfigurationTarget.Global, + true, + ); + await until( + () => + vscode.workspace + .getConfiguration('editor', editor.document) + .get('autoClosingBrackets') === 'never', + ); }); setup(async () => { providers = []; @@ -125,6 +143,12 @@ suite('Interactive input formatting with independent providers', () => { suiteTeardown(async () => { await setFormatSetting('formatOnType', previousOnType); await setFormatSetting('formatOnPaste', previousOnPaste); + await settings().update( + 'autoClosingBrackets', + previousAutoClosingBrackets, + vscode.ConfigurationTarget.Global, + true, + ); controller.dispose(); await vscode.window.tabGroups.close( vscode.window.tabGroups.all diff --git a/src/test/suite/languageService.test.ts b/src/test/suite/languageService.test.ts new file mode 100644 index 000000000..abefd3583 --- /dev/null +++ b/src/test/suite/languageService.test.ts @@ -0,0 +1,128 @@ +import * as assert from 'node:assert'; +import { EventEmitter } from 'node:events'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import type { LanguageClient } from 'vscode-languageclient/node'; +import { LanguageService } from '../../languageService'; +import { connectTcpLanguageServer } from '../../languageServerTransport'; +import * as util from '../../util'; + +suite('Language service startup lifecycle', () => { + let sandbox: sinon.SinonSandbox; + let appendLine: sinon.SinonStub; + const prototype = LanguageService.prototype as unknown as { + createClient: () => Promise; + startMultiLanguageService: () => void; + spawnServer: ( + client: LanguageClient, + executable: string, + args: string[], + options: { cwd: string }, + ) => util.DisposableProcess; + }; + + setup(() => { + sandbox = sinon.createSandbox(); + appendLine = sandbox.stub(); + sandbox.stub(vscode.window, 'createOutputChannel').returns({ + appendLine, + } as unknown as vscode.LogOutputChannel); + sandbox.stub(vscode.workspace, 'getConfiguration').returns({ + get: () => false, + } as unknown as vscode.WorkspaceConfiguration); + }); + teardown(() => sandbox.restore()); + + test('an initial startup rejection is logged and does not block disposal', async () => { + sandbox.stub(prototype, 'createClient').rejects(new Error('R exited before connecting')); + const register = sandbox.stub(prototype, 'startMultiLanguageService'); + const service = new LanguageService(); + await service.dispose(); + sinon.assert.calledWithMatch(appendLine, 'Failed to start language server:'); + sinon.assert.notCalled(register); + }); + + test('disposal during startup stops the eventual client without registering listeners', async () => { + let finish!: (client: LanguageClient) => void; + sandbox.stub(prototype, 'createClient').returns( + new Promise((resolve) => { + finish = resolve; + }), + ); + const register = sandbox.stub(prototype, 'startMultiLanguageService'); + const stop = sandbox.stub().resolves(); + const service = new LanguageService(); + const disposal = service.dispose(); + finish({ stop } as unknown as LanguageClient); + await disposal; + sinon.assert.calledOnce(stop); + sinon.assert.notCalled(register); + }); + + test('disposal cancels TCP startup while R remains alive without connecting', async () => { + const disposeChild = sandbox.stub(); + const child = Object.assign(new EventEmitter(), { + dispose: disposeChild, + }) as unknown as util.DisposableProcess; + let started!: () => void; + const listening = new Promise((resolve) => { + started = resolve; + }); + sandbox.stub(prototype, 'createClient').callsFake(function (this: LanguageService) { + const cancellation = (this as unknown as { startupCancellation: AbortController }) + .startupCancellation; + return connectTcpLanguageServer(() => { + started(); + return child; + }, cancellation.signal).then(() => { + throw new Error('R never connected'); + }); + }); + const register = sandbox.stub(prototype, 'startMultiLanguageService'); + const service = new LanguageService(); + await listening; + await service.dispose(); + sinon.assert.calledOnce(disposeChild); + sinon.assert.notCalled(register); + }); + + test('a missing package still prompts for installation without stopping a starting client', () => { + const child = Object.assign(new EventEmitter(), { + stdout: new EventEmitter(), + stderr: new EventEmitter(), + }) as unknown as util.DisposableProcess; + sandbox.stub(util, 'spawn').returns(child); + const install = sandbox.stub(util, 'promptToInstallRPackage').resolves(); + const stop = sandbox.stub().resolves(); + const client = { + outputChannel: { appendLine, show: sandbox.stub() }, + isRunning: () => false, + stop, + } as unknown as LanguageClient; + prototype.spawnServer(client, 'R', [], { cwd: '/project' }); + child.emit('exit', 10, null); + sinon.assert.calledOnce(install); + assert.strictEqual(install.firstCall.args[0], 'languageserver'); + sinon.assert.notCalled(stop); + }); + + test('a rejected stop after R exits is logged', async () => { + const child = Object.assign(new EventEmitter(), { + stdout: new EventEmitter(), + stderr: new EventEmitter(), + }) as unknown as util.DisposableProcess; + sandbox.stub(util, 'spawn').returns(child); + const client = { + outputChannel: { appendLine }, + isRunning: () => true, + stop: sandbox.stub().rejects(new Error('shutdown failed')), + } as unknown as LanguageClient; + prototype.spawnServer(client, 'R', [], { cwd: '/project' }); + child.emit('exit', 0, null); + await Promise.resolve(); + sinon.assert.calledWithMatch( + appendLine, + 'Failed to stop language server: Error: shutdown failed', + ); + }); +}); diff --git a/src/test/suite/preview.test.ts b/src/test/suite/preview.test.ts new file mode 100644 index 000000000..8053f3df8 --- /dev/null +++ b/src/test/suite/preview.test.ts @@ -0,0 +1,138 @@ +import * as assert from 'assert'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as extension from '../../extension'; +import * as rTerminal from '../../rTerminal'; +import * as selection from '../../selection'; +import * as util from '../../util'; +import { previewDataframe, previewEnvironment } from '../../preview'; + +suite('CSV previews', () => { + let sandbox: sinon.SinonSandbox; + let directory: string; + let temporary: string; + let activate: sinon.SinonStub; + let execute: sinon.SinonStub; + let run: sinon.SinonStub; + let error: sinon.SinonStub; + setup(() => { + sandbox = sinon.createSandbox(); + directory = fs.mkdtempSync(path.join(os.tmpdir(), "vscode-r-preview-'")); + temporary = path.join(directory, 'preview'); + fs.mkdirSync(temporary); + sandbox.stub(extension, 'enableSessionWatcher').value(false); + sandbox + .stub(util, 'config') + .returns({ get: () => false } as unknown as vscode.WorkspaceConfiguration); + sandbox + .stub(util, 'getCurrentWorkspaceFolder') + .returns({ uri: vscode.Uri.file(directory) } as vscode.WorkspaceFolder); + sandbox.stub(util, 'createTempDir').returns(temporary); + sandbox.stub(selection, 'getWordOrSelection').returns('iris'); + activate = sandbox.stub().resolves(); + sandbox + .stub(vscode.extensions, 'getExtension') + .returns({ isActive: false, activate } as unknown as vscode.Extension); + execute = sandbox.stub(vscode.commands, 'executeCommand').resolves(); + run = sandbox.stub(rTerminal, 'runTextInTerm').resolves(true); + error = sandbox.stub(vscode.window, 'showErrorMessage').resolves(undefined); + }); + teardown(() => { + sandbox.restore(); + fs.rmSync(directory, { recursive: true, force: true }); + }); + + for (const [name, preview] of [ + ['iris', previewDataframe], + ['environment', previewEnvironment], + ] as const) { + test(`${name} preview waits for completed output and activates an installed viewer`, async () => { + const csv = path.join(temporary, `${name}.csv`); + const open = sandbox + .stub(vscode.workspace, 'openTextDocument') + .resolves({ uri: vscode.Uri.file(csv) } as vscode.TextDocument); + let polls = 0; + sandbox.stub(util, 'delay').callsFake(() => { + polls++; + sinon.assert.notCalled(open); + fs.writeFileSync(csv, polls === 1 ? 'partial,' : 'complete,output'); + if (polls === 2) { + fs.writeFileSync(`${csv}.ready`, ''); + } + return Promise.resolve(); + }); + await preview(); + assert.strictEqual(polls, 2); + sinon.assert.calledOnce(activate); + sinon.assert.calledOnce(open); + sinon.assert.calledWith(execute, 'csv.preview', vscode.Uri.file(csv)); + sinon.assert.notCalled(error); + const code = String(run.firstCall.args[0]); + assert.ok(code.includes(util.ToRStringLiteral(csv, '"'))); + assert.ok(code.includes('base::file.create(')); + assert.strictEqual(fs.existsSync(temporary), false); + }); + } + + test('a stalled R writer times out and removes its temporary files', async () => { + const now = sandbox.stub(Date, 'now').returns(0); + sandbox.stub(util, 'delay').callsFake(() => { + now.returns(30000); + return Promise.resolve(); + }); + const open = sandbox.stub(vscode.workspace, 'openTextDocument'); + await previewDataframe(); + sinon.assert.calledOnce(error); + assert.match(String(error.firstCall.args[0]), /within 30 seconds/); + sinon.assert.notCalled(open); + assert.strictEqual(fs.existsSync(temporary), false); + }); + + test('a cancelled terminal submission does not wait for CSV output', async () => { + run.resolves(false); + const delay = sandbox.stub(util, 'delay'); + const open = sandbox.stub(vscode.workspace, 'openTextDocument'); + await previewDataframe(); + sinon.assert.notCalled(delay); + sinon.assert.notCalled(open); + sinon.assert.notCalled(error); + assert.strictEqual(fs.existsSync(temporary), false); + }); + + test('preview failure is awaited and still cleans up output', async () => { + fs.writeFileSync(path.join(temporary, 'iris.csv'), 'a,b'); + fs.writeFileSync(path.join(temporary, 'iris.csv.ready'), ''); + sandbox.stub(vscode.workspace, 'openTextDocument').rejects(new Error('cannot open CSV')); + await previewDataframe(); + sinon.assert.calledOnce(error); + assert.match(String(error.firstCall.args[0]), /cannot open CSV/); + assert.strictEqual(fs.existsSync(temporary), false); + }); + + test('the R environment export includes global objects in a path with quotes', async () => { + const csv = path.join(temporary, 'environment.csv'); + run.callsFake(async (code: string) => { + await promisify(execFile)('Rscript', [ + '--vanilla', + '-e', + `csv_preview_object <- 42; ${code}`, + ]); + return true; + }); + let contents = ''; + sandbox.stub(vscode.workspace, 'openTextDocument').callsFake(() => { + contents = fs.readFileSync(csv, 'utf8'); + return Promise.resolve({ uri: vscode.Uri.file(csv) } as vscode.TextDocument); + }); + await previewEnvironment(); + assert.match(contents, /csv_preview_object/); + assert.match(contents, /numeric/); + sinon.assert.notCalled(error); + assert.strictEqual(fs.existsSync(temporary), false); + }); +}); diff --git a/src/test/suite/rstudioapi.test.ts b/src/test/suite/rstudioapi.test.ts index 734c5f2fe..edb7fb0e8 100644 --- a/src/test/suite/rstudioapi.test.ts +++ b/src/test/suite/rstudioapi.test.ts @@ -1,7 +1,16 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; -import { documentNew, insertOrModifyText, replaceTextInCurrentSelection } from '../../rstudioapi'; +import { + documentClose, + documentSave, + documentSaveAll, + documentNew, + insertOrModifyText, + replaceTextInCurrentSelection, + setSelections, + projectPath, +} from '../../rstudioapi'; import { handleEditorRequest } from '../../session'; suite('RStudio editor request completion', () => { @@ -134,6 +143,7 @@ suite('RStudio editor request completion', () => { test('selection replacement rejects an edit that VS Code cannot apply', async () => { const doc = document(); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); sandbox.stub(vscode.window, 'activeTextEditor').value({ document: doc, selection: new vscode.Selection(0, 0, 0, 1), @@ -144,4 +154,59 @@ suite('RStudio editor request completion', () => { /Could not apply editor changes/, ); }); + + test('selection replacement uses the requested document editor', async () => { + const active = document(); + const target = { + ...document(), + uri: vscode.Uri.file('/project/other.R'), + } as vscode.TextDocument; + const selection = new vscode.Selection(0, 2, 0, 4); + sandbox.stub(vscode.window, 'activeTextEditor').value({ + document: active, + selection: new vscode.Selection(0, 0, 0, 1), + }); + sandbox.stub(vscode.window, 'visibleTextEditors').value([{ document: target, selection }]); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(target); + const apply = sandbox.stub(vscode.workspace, 'applyEdit').resolves(true); + await replaceTextInCurrentSelection('x', target.uri.toString()); + const edit = apply.firstCall.args[0] as vscode.WorkspaceEdit; + assert.ok(edit.get(target.uri)[0].range.isEqual(selection)); + assert.strictEqual(edit.has(active.uri), false); + }); + + test('explicit selection requests can show a document without an active editor', async () => { + const doc = document(); + sandbox.stub(vscode.window, 'activeTextEditor').value(undefined); + sandbox.stub(vscode.window, 'visibleTextEditors').value([]); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + const editor = { document: doc, selections: [] } as unknown as vscode.TextEditor; + const show = sandbox.stub(vscode.window, 'showTextDocument').resolves(editor); + await setSelections([{ start: [1, 1], end: [1, 2] }], doc.uri.toString()); + sinon.assert.calledOnce(show); + assert.deepStrictEqual(editor.selections, [new vscode.Selection(0, 0, 0, 1)]); + }); + + test('document close keeps tabs open when saving fails', async () => { + const doc = { ...document(), save: () => Promise.resolve(false) } as vscode.TextDocument; + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + const close = sandbox.stub().resolves(true); + sandbox.stub(vscode.window, 'tabGroups').value({ ...vscode.window.tabGroups, close }); + await assert.rejects(documentClose(doc.uri.toString(), true), /Could not save document/); + sinon.assert.notCalled(close); + }); + + test('save requests reject when VS Code cannot save documents', async () => { + const doc = { ...document(), save: () => Promise.resolve(false) } as vscode.TextDocument; + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + sandbox.stub(vscode.workspace, 'saveAll').resolves(false); + await assert.rejects(documentSave(doc.uri.toString()), /Could not save document/); + await assert.rejects(documentSaveAll(), /Could not save all documents/); + }); + + test('project paths are native filesystem paths', () => { + const uri = vscode.Uri.file(process.platform === 'win32' ? 'C:\\project' : '/project'); + sandbox.stub(vscode.workspace, 'workspaceFolders').value([{ uri }]); + assert.strictEqual(projectPath().path, uri.fsPath); + }); }); diff --git a/src/test/suite/sessionTerminalIntegration.test.ts b/src/test/suite/sessionTerminalIntegration.test.ts index 38e2b5f51..be387891e 100644 --- a/src/test/suite/sessionTerminalIntegration.test.ts +++ b/src/test/suite/sessionTerminalIntegration.test.ts @@ -221,6 +221,33 @@ suite('External Terminal Integration', () => { } }); + test('discovery refresh updates later native terminals while an unrelated PID remains pending', async () => { + const pendingPid = deferred(); + const external = makeTerminal(pendingPid.promise); + const discoveryFile = await session.createSessionDiscoveryFile('old-sess'); + const native = { + ...makeTerminal(Promise.resolve(55114)), + creationOptions: { env: { SESS_DISCOVERY_FILE: discoveryFile } }, + } as vscode.Terminal; + setTerminals([external, native], native); + const refresh = session.refreshTerminalDiscoveryFiles('new-sess', [external, native]); + try { + let endpoint: string | undefined; + for (let attempt = 0; attempt < 100; attempt++) { + endpoint = ((await readJson(discoveryFile)) as { endpoint?: string }).endpoint; + if (endpoint === 'new-sess') { + break; + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + assert.strictEqual(endpoint, 'new-sess'); + } finally { + pendingPid.resolve(undefined); + await refresh; + await fsp.rm(discoveryFile, { force: true }); + } + }); + test('discovery refresh skips a newly registered terminal and continues with live native terminals', async () => { let pidRead!: () => void; const readStarted = new Promise((resolve) => { diff --git a/src/test/suite/sessionTerminalLifecycle.test.ts b/src/test/suite/sessionTerminalLifecycle.test.ts index 014c7e218..18a642db0 100644 --- a/src/test/suite/sessionTerminalLifecycle.test.ts +++ b/src/test/suite/sessionTerminalLifecycle.test.ts @@ -175,6 +175,88 @@ suite('Session Terminal Lifecycle', () => { sandbox.restore(); }); + test('an unresolved unrelated PID does not delay an exact native terminal match', async () => { + const pendingPid = deferred(); + const unrelatedSend = sandbox.stub(); + const unrelated = { + processId: pendingPid.promise, + sendText: unrelatedSend, + } as unknown as vscode.Terminal; + const sendText = sandbox.stub(); + const terminal = { + processId: Promise.resolve(46290), + sendText, + show: sandbox.stub(), + } as unknown as vscode.Terminal; + sandbox.stub(vscode.window, 'terminals').value([unrelated, terminal]); + sandbox.stub(vscode.window, 'activeTerminal').value(terminal); + const socket = await connections.attach('exact-beyond-pending', 46290); + const owner = session.activeSession; + assert.ok(owner); + assert.strictEqual(socket._terminalPid, 46290); + sinon.assert.notCalled(ancestors); + await session.executeSessionCode(owner, 'View(iris)'); + sinon.assert.calledOnceWithExactly(sendText, 'View(iris)'); + + pendingPid.resolve(46290); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(await session.waitForTerminalReady(unrelated, 1), false); + await session.executeSessionCode(owner, 'rm(iris)'); + sinon.assert.calledWithExactly(sendText, 'rm(iris)'); + sinon.assert.notCalled(unrelatedSend); + }); + + for (const hasAncestor of [false, true]) { + test(`native discovery bounds unresolved PID reads and preserves ancestor ownership (${String(hasAncestor)})`, async () => { + const entered = deferred(); + const pendingPid = deferred(); + const unrelated = { + get processId() { + entered.resolve(); + return pendingPid.promise; + }, + } as unknown as vscode.Terminal; + const sendText = sandbox.stub(); + const terminal = { + processId: Promise.resolve(46291), + sendText, + show: sandbox.stub(), + } as unknown as vscode.Terminal; + sandbox.stub(vscode.window, 'terminals').value([unrelated, terminal]); + sandbox.stub(vscode.window, 'activeTerminal').value(terminal); + ancestors.withArgs(46292).resolves(hasAncestor ? [46293, 46291] : []); + const socket = await connections.startAttach('bounded-native-pids', 46292); + const clock = sandbox.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + await entered.promise; + assert.strictEqual(socket._sessionId, undefined); + await clock.tickAsync(1000); + await settleWithFakeTime( + waitFor(() => (socket._sessionId ? true : undefined)), + clock, + ); + assert.strictEqual(socket._terminalPid, hasAncestor ? 46291 : undefined); + } finally { + clock.restore(); + } + const owner = session.activeSession; + assert.ok(owner); + pendingPid.resolve(46292); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(await session.waitForTerminalReady(unrelated, 1), false); + if (hasAncestor) { + await session.executeSessionCode(owner, 'View(iris)'); + sinon.assert.calledOnceWithExactly(sendText, 'View(iris)'); + } else { + await assert.rejects( + session.executeSessionCode(owner, 'View(iris)'), + /no attached terminal/, + ); + sinon.assert.notCalled(sendText); + } + }); + } + for (const { delayedPid, parents } of [ { delayedPid: false, parents: [] }, { delayedPid: true, parents: [] }, diff --git a/src/test/suite/standardPlotViewer.test.ts b/src/test/suite/standardPlotViewer.test.ts index 06ad2b150..33324831d 100644 --- a/src/test/suite/standardPlotViewer.test.ts +++ b/src/test/suite/standardPlotViewer.test.ts @@ -34,6 +34,7 @@ suite('Standard plot viewer request coordination', () => { let panels: Array<{ panel: vscode.WebviewPanel; resize: (width: number, height: number) => Promise; + ready: () => Promise; postMessage: sinon.SinonStub; }>; @@ -76,6 +77,7 @@ suite('Standard plot viewer request coordination', () => { postMessage, resize: (width, height) => receiveMessage?.({ type: 'resize', width, height }) ?? Promise.resolve(), + ready: () => receiveMessage?.({ type: 'ready' }) ?? Promise.resolve(), }); return panel; }); @@ -360,7 +362,7 @@ suite('Standard plot viewer request coordination', () => { sinon.assert.notCalled(newPanel.postMessage); }); - test('waits for the initial webview resize before requesting the first plot', async () => { + test('waits for a webview startup message before requesting the first plot', async () => { const request = sandbox .stub(session, 'sessionRequest') .resolves({ data: 'plot', format: 'svg' }); @@ -370,6 +372,49 @@ suite('Standard plot viewer request coordination', () => { sinon.assert.calledOnce(request); }); + test('requests the first plot when the webview is ready without a layout resize', async () => { + const owner = { sessionId: 'ready-owner' } as session.Session; + activeSession.value(owner); + const request = sandbox + .stub(session, 'sessionRequest') + .resolves({ data: 'plot', format: 'svg' }); + await viewer.update(); + sinon.assert.notCalled(request); + await panels[0].ready(); + sinon.assert.calledOnceWithExactly( + request, + { + method: 'plot_latest', + params: { width: 800, height: 600, format: 'svglite', devArgs: undefined }, + }, + owner, + ); + sinon.assert.calledOnceWithExactly(panels[0].postMessage, { + type: 'update', + data: 'plot', + format: 'svg', + }); + await panels[0].ready(); + sinon.assert.calledOnce(request); + await panels[0].resize(640, 480); + sinon.assert.calledTwice(request); + }); + + test('ignores readiness from a disposed panel and does not rerender after an initial resize', async () => { + const request = sandbox + .stub(session, 'sessionRequest') + .resolves({ data: 'plot', format: 'svg' }); + await viewer.update(); + const old = panels[0]; + viewer.dispose(); + await viewer.update(); + await old.ready(); + sinon.assert.notCalled(request); + await panels[1].resize(640, 480); + await panels[1].ready(); + sinon.assert.calledOnce(request); + }); + test('handles a request that resolves without plot data without becoming stuck', async () => { const request = sandbox.stub(session, 'sessionRequest'); request.onFirstCall().resolves(undefined); diff --git a/src/test/suite/util.test.ts b/src/test/suite/util.test.ts index c9c51f735..52c19865f 100644 --- a/src/test/suite/util.test.ts +++ b/src/test/suite/util.test.ts @@ -16,6 +16,18 @@ suite('R command utilities', () => { }); teardown(() => sandbox.restore()); + test('in-memory help state preserves falsy values and supports key deletion', async () => { + const state = new util.DummyMemento(); + for (const value of [false, 0, '', null]) { + await state.update('cached', value); + assert.strictEqual(state.get('cached', 'fallback'), value); + assert.deepStrictEqual(state.keys(), ['cached']); + } + await state.update('cached', undefined); + assert.strictEqual(state.get('cached', 'fallback'), 'fallback'); + assert.deepStrictEqual(state.keys(), []); + }); + test('R literals escape quotes, backslashes and control characters', () => { assert.strictEqual( util.ToRStringLiteral('a"b\'c\\d\n\r\t\b\f\v', '"'), diff --git a/src/util.ts b/src/util.ts index e67a25a63..4bbc932f1 100644 --- a/src/util.ts +++ b/src/util.ts @@ -464,18 +464,22 @@ export class DummyMemento implements vscode.Memento { items = new Map(); public get(key: string, defaultValue?: T): T | undefined { if (this.items.has(key)) { - return this.items.get(key) || defaultValue; + return this.items.get(key); } else { return defaultValue; } } // oxlint-disable-next-line typescript/require-await public async update(key: string, value: any): Promise { - this.items.set(key, value); + if (value === undefined) { + this.items.delete(key); + } else { + this.items.set(key, value); + } } public keys(): readonly string[] { - return Object.keys(this.items); + return [...this.items.keys()]; } }