diff --git a/.vscode/launch.json b/.vscode/launch.json index f1450c2..62d990c 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -12,13 +12,19 @@ "runtimeExecutable": "${execPath}", "args": [ "--extensionDevelopmentPath=${workspaceFolder}/packages/vscode", - "--disable-extensions", - // An isolated, auto-created profile, so personal user settings - // (formatters, format-on-save, keybindings) cannot leak into the - // playground. `--user-data-dir` would be the stronger isolation, but - // the extension-host debugger strips it; `--profile` is the supported - // mechanism for debug launches. + // An isolated, auto-created profile, so neither personal user settings + // (formatters, format-on-save, keybindings) nor personal extensions + // can leak into the playground — the profile starts with none + // installed, which is why `--disable-extensions` is absent: it would + // only add its "all installed extensions are temporarily disabled" + // notification to every launch. `--user-data-dir` would be the + // stronger isolation, but the extension-host debugger strips it; + // `--profile` is the supported mechanism for debug launches. "--profile=rstack-playground", + // Built-in extensions ignore the empty profile. Git is the one that + // speaks up: every fixture sits under this repo, so it offers to open + // the parent repository on each launch. + "--disable-extension=vscode.git", // A fresh profile would otherwise prompt for workspace trust and greet // with the welcome tour on every first launch. "--disable-workspace-trust", diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..2d0a372 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,19 @@ +# Ubiquitous language + +Glossary of terms used across rstack-editor. Code, docs, commit messages and reviews use these words with exactly these meanings. + +## Core + +- **Stack** — one tool integration (lint, test, fmt) hosted by the extension shell. A stack registers against the shell and reports status through it; stacks never own UI chrome. +- **Shell** — the always-activating extension core: detection, status bar, output channels, settings migration, stack lifecycle. +- **Detection** — the per-workspace-folder scan deciding which stacks a folder lights up. Detection signals are config files and installed tool binaries, never user settings. +- **Gate** — the per-stack activation condition: detected, workspace trusted, and the enable settings on. + +## fmt + +- **Cold format** — a format request served by spawning a fresh `rs fmt` process at request time; the request pays the full process start-up cost. +- **Standby** — the single pre-spawned `rs fmt` process held ready for one specific file, so the next format of that file skips the start-up cost. There is at most one standby, and it is only ever armed for the active editor's file ("the standby tracks the active editor"). An editor change that cannot be armed kills it; an editor holding nothing this stack formats leaves it to expire. +- **Arm** — create the standby for a file. Arming happens when the active editor lands on an eligible file and again right after a format consumed the previous standby. +- **Consume** — serve a format request with the armed standby. A standby serves exactly one request; a request the standby cannot serve falls back to a cold format. +- **Hot format** — a format request served by consuming the standby. +- **Expire** — kill an idle standby to reclaim its memory. An expired standby is not an error; the next eligible event simply arms a new one. diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 7cc491e..da096e1 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -31,7 +31,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - The lint × `rstack.config.*` bridge was built and deliberately removed: a partial editor-side bridge gave wrong results, and a correct one needs upstream work first. `TODO(rstack-bridge)` markers carry the plan. Do not reintroduce a partial bridge. - The test × `rstack.config.*` bridge stays thin on purpose: it points the upstream machinery at rstack's shipped shim and lets the shim interpret the config inside the worker, same as the CLI. Never re-implement rstack config semantics in the extension. -- The fmt stack is a spawn-per-request `rs fmt --stdin-filepath` MVP. Its cwd is the governing config directory because rs fmt resolves config from cwd only, and formatting errors are log-only by design. The endgame is an upstream LSP, so do not add a warm-process middle tier. +- The fmt stack is a spawn-per-request `rs fmt --stdin-filepath` MVP. Its cwd is the governing config directory because rs fmt resolves config from cwd only, and formatting errors are log-only by design. A single pre-spawned standby that tracks the active editor (see CONTEXT.md) is the accepted, bounded exception to "no warm tier". Do not grow it into a daemon: no long-lived protocol, no process pool, no cross-request state. The endgame is an upstream LSP; the standby retires with it. - `projectModules.ts` has no cache-invalidation hook and restart must not grow one. Node's ESM registry is keyed by resolved URL and process-lifetime, so clearing the local memo hands back the identical module object (verified); a `?epoch=` query does reload the entry but relative specifiers inside it do not inherit the query, yielding a fresh entry over stale dependencies. In-place reinstalls under an unchanged path need a window reload — say so, don't fake it. - The VSIX is platform-targeted for exactly one reason: the test stack's AST collection loads a native parser binding. Do not add another native dependency — it multiplies the release matrix. diff --git a/packages/vscode/src/stacks/fmt/index.ts b/packages/vscode/src/stacks/fmt/index.ts index fb2645e..ba3f0b5 100644 --- a/packages/vscode/src/stacks/fmt/index.ts +++ b/packages/vscode/src/stacks/fmt/index.ts @@ -1,11 +1,12 @@ import path from 'node:path'; import vscode from 'vscode'; +import { RSTACK_CONFIG_GLOB } from '../../detection'; import { findPackageJsonUncached, readPackageJson, } from '../../shared/packageResolve'; import { - readPackageVersion, + checkPackageVersion, reportVersionCheck, } from '../../shared/versionCheck'; import type { @@ -20,6 +21,7 @@ import { runRsFmt, stderrTail, } from './run'; +import { FmtStandby, type StandbyKey } from './standby'; // prettier 3.9.6 getSupportInfo() vscodeLanguageIds snapshot (rs fmt's pinned // prettier). Revisit when the pinned prettier changes. @@ -56,9 +58,48 @@ const SELECTOR: vscode.DocumentSelector = LANGUAGE_IDS.map((language) => ({ })); /** - * Spawn-per-request formatter backed by the project-resolved rstack CLI. The - * process cwd selects the nearest governing rstack config because `rs fmt` - * intentionally performs cwd-only config resolution. + * How long the active editor must hold still before it gets a standby. Arming + * spawns a process, so scrolling through a dozen tabs must not spawn a dozen + * children; the first arm at registration and the re-arm right after a consume + * skip the wait because both target an editor that is already settled. + */ +const ARM_DEBOUNCE_MS = 2_000; + +/** Everything a format request needs once the folder has been resolved. */ +interface FmtTarget { + readonly cwd: string; + readonly rsBinJs: string; +} + +const standbyKey = (uri: vscode.Uri, target: FmtTarget): StandbyKey => ({ + cwd: target.cwd, + filePath: uri.fsPath, + rsBinJs: target.rsBinJs, +}); + +/** + * Resolution outcome, kept free of side effects so the arming path (which must + * stay silent) and the format path (which reports and logs) can share it. + */ +type FmtResolution = + | { readonly kind: 'ok'; readonly target: FmtTarget } + | { readonly kind: 'no-folder' } + | { readonly kind: 'undetected'; readonly folder: vscode.WorkspaceFolder } + | { + readonly kind: 'missing-package'; + readonly folder: vscode.WorkspaceFolder; + readonly cwd: string; + } + | { readonly kind: 'version-mismatch'; readonly version: string | undefined }; + +/** + * Formatter backed by the project-resolved rstack CLI. The process cwd selects + * the nearest governing rstack config because `rs fmt` intentionally performs + * cwd-only config resolution. + * + * A request is served either by consuming the standby (hot) or by spawning a + * process for it (cold); the two paths differ only in where the process came + * from. */ class FmtController implements StackController { readonly id = 'fmt' as const; @@ -71,18 +112,37 @@ class FmtController implements StackController { readonly #loggedOnce = new Set(); readonly #abortController = new AbortController(); #disposed = false; + #standby: FmtStandby | undefined; + #armTimer: NodeJS.Timeout | undefined; + /** E2E-only: how the most recent format request was served. */ + #lastServe: 'hot' | 'cold' | undefined; async register(context: StackContext): Promise> { this.#context = context; this.#snapshot = context.detection; + this.#standby = new FmtStandby({ + log: (message) => context.output.debug(message), + }); const provider: vscode.DocumentFormattingEditProvider = { provideDocumentFormattingEdits: (document, _options, token) => this.provideDocumentFormattingEdits(document, token), }; + // `rs fmt` loads the project config while it drains stdin, so a parked + // process already carries the old config and every config event has to + // invalidate it. Detection cannot stand in for this watcher: its signature + // records only which config files exist, so it misses a content edit, and + // it misses the delete/create pair an atomic save produces for one + // unchanged path just the same. + const configWatcher = + vscode.workspace.createFileSystemWatcher(RSTACK_CONFIG_GLOB); + const onConfigEvent = (uri: vscode.Uri): void => + this.invalidateStandby(`${uri.fsPath} changed`); this.#subscriptions.push( context.onDidChangeDetection((snapshot) => { this.#snapshot = snapshot; this.#loggedOnce.clear(); + // The cwd, the resolved bin and the config set can all have moved. + this.invalidateStandby('detection changed'); // The reconcile leaves a still-detected controller alone, so the // running reason must follow the new snapshot here rather than wait // for the next successful format. @@ -92,9 +152,137 @@ class FmtController implements StackController { SELECTOR, provider, ), + configWatcher, + configWatcher.onDidCreate(onConfigEvent), + configWatcher.onDidChange(onConfigEvent), + configWatcher.onDidDelete(onConfigEvent), + vscode.window.onDidChangeActiveTextEditor(() => this.scheduleArm()), ); this.reportRunning(context, context.detection); - return { languages: LANGUAGE_IDS, provider }; + // The editor the user is already looking at needs no settling wait, but + // arming spawns a process and `register()` must return fast. + this.scheduleArm(0); + return { + languages: LANGUAGE_IDS, + provider, + armedFilePath: (): string | undefined => this.#standby?.armedFilePath, + lastServe: (): 'hot' | 'cold' | undefined => this.#lastServe, + }; + } + + /** Kills the standby, then re-arms the active editor through the debounce. */ + private invalidateStandby(reason: string): void { + this.#standby?.kill(reason); + this.scheduleArm(); + } + + private scheduleArm(delayMs = ARM_DEBOUNCE_MS): void { + clearTimeout(this.#armTimer); + this.#armTimer = setTimeout(() => { + this.#armTimer = undefined; + this.armActiveEditor(); + }, delayMs); + } + + /** + * Arms a standby for whatever the active editor is *now* — the invariant is + * "the standby tracks the active editor", so the editor is never captured + * when the arm was scheduled. + * + * Nothing formattable being active leaves the standby alone, whether the + * active editor holds no text document at all (a settings tab, an image + * preview) or holds one this stack does not format. Neither is worth a kill: + * an idle standby expires on its own, and both are a keystroke away + * from the file that owns it. A formattable document that cannot be armed is + * the other case — the editor really did move on, so the standby goes too. + */ + private armActiveEditor(): void { + const context = this.#context; + const standby = this.#standby; + if (!context || !standby) { + return; + } + const document = vscode.window.activeTextEditor?.document; + // The provider's own selector is the eligibility rule, so the two cannot + // drift apart. + if (!document || vscode.languages.match(SELECTOR, document) === 0) { + return; + } + // Every invalidation kills the standby before asking for a re-arm, so an + // armed standby on this file is still valid — and resolving is synchronous + // filesystem work that runs on the UI thread. + if (standby.armedFilePath === document.uri.fsPath) { + return; + } + const resolution = this.resolve(document.uri); + if (resolution.kind !== 'ok') { + // Arming is silent: an unresolvable editor only means the next format + // there is cold, which is exactly what happened before the standby. + context.output.debug( + `Standby not armed for ${document.uri.fsPath}: ${resolution.kind}`, + ); + // The editor still moved to another file, so the previous file's standby + // no longer tracks it. `arm` would have killed it; there is nothing to + // arm here, so this path has to. + standby.kill('the active editor moved to a file that cannot be armed'); + return; + } + standby.arm(standbyKey(document.uri, resolution.target)); + } + + /** + * Where a document's `rs fmt` would run. Pure: it reports no status and logs + * nothing, because the arming path must not move the status bar. + */ + private resolve(uri: vscode.Uri): FmtResolution { + const snapshot = this.#snapshot; + const folder = vscode.workspace.getWorkspaceFolder(uri); + if (!snapshot || !folder) { + return { kind: 'no-folder' }; + } + const fmtDetection = snapshot.forFolder(folder)?.stacks.fmt; + if (!fmtDetection?.detected) { + return { kind: 'undetected', folder }; + } + + const cwd = pickConfigDir( + uri.fsPath, + fmtDetection.rstackConfigFiles.map((configUri) => configUri.fsPath), + folder.uri.fsPath, + ); + const pkgJsonPath = findPackageJsonUncached('rstack', cwd); + if (!pkgJsonPath) { + return { kind: 'missing-package', folder, cwd }; + } + + // One read for both the version and the bin entry: `resolve` now runs on + // the arming path too, and `readPackageJson` re-reads from disk by design. + const pkg = readPackageJson(pkgJsonPath); + const version = typeof pkg?.version === 'string' ? pkg.version : undefined; + if (checkPackageVersion('rstack', version).kind === 'mismatch') { + // Reporting stays with the caller: the arming path must not move the + // status bar, and the format path goes through `reportVersionCheck` so + // the shared contract has one implementation. + return { kind: 'version-mismatch', version }; + } + + const bin = pkg?.bin; + let binEntry = 'bin/rs.js'; + if (typeof bin === 'string') { + binEntry = bin; + } else if (bin && typeof bin === 'object') { + const rs = (bin as Record).rs; + if (typeof rs === 'string') { + binEntry = rs; + } + } + return { + kind: 'ok', + target: { + cwd, + rsBinJs: path.resolve(path.dirname(pkgJsonPath), binEntry), + }, + }; } /** `running` always carries the reason the stack is on: where it was detected. */ @@ -130,15 +318,15 @@ class FmtController implements StackController { return []; } - const folder = vscode.workspace.getWorkspaceFolder(document.uri); - if (!folder) { + const resolution = this.resolve(document.uri); + if (resolution.kind === 'no-folder') { return []; } - const fmtDetection = snapshot.forFolder(folder)?.stacks.fmt; - if (!fmtDetection?.detected) { + if (resolution.kind === 'undetected') { // The formatter is offered per language, so a request can land in a // folder without an rstack setup. That is routine, not a fault — one // info line per folder says why nothing happened. + const folder = resolution.folder; if (!this.#loggedOnce.has(`undetected:${folder.uri.toString()}`)) { this.#loggedOnce.add(`undetected:${folder.uri.toString()}`); context.output.info( @@ -148,50 +336,27 @@ class FmtController implements StackController { return []; } - const cwd = pickConfigDir( - document.uri.fsPath, - fmtDetection.rstackConfigFiles.map((uri) => uri.fsPath), - folder.uri.fsPath, - ); // Per-request logging follows prettier-vscode's shape (same in-host, // work-per-request architecture): a fixed entry and outcome line at info, // resolution detail at debug — the channel is a LogOutputChannel, so the // user raises the level from its context menu when needed. const startedAt = Date.now(); context.output.info(`Formatting ${document.uri.fsPath}`); - const pkgJsonPath = findPackageJsonUncached('rstack', cwd); - if (!pkgJsonPath) { - const reason = `rstack is not installed in ${folder.name} (node_modules missing)`; + if (resolution.kind === 'missing-package') { + const reason = `rstack is not installed in ${resolution.folder.name} (node_modules missing)`; context.status.report({ kind: 'disabled', reason }); - if (!this.#loggedOnce.has(`missing:${cwd}`)) { - this.#loggedOnce.add(`missing:${cwd}`); - context.output.warn(`${reason}; searched from ${cwd}`); + if (!this.#loggedOnce.has(`missing:${resolution.cwd}`)) { + this.#loggedOnce.add(`missing:${resolution.cwd}`); + context.output.warn(`${reason}; searched from ${resolution.cwd}`); } return []; } - - if ( - !reportVersionCheck( - context.status, - 'rstack', - readPackageVersion(pkgJsonPath), - ) - ) { + if (resolution.kind === 'version-mismatch') { + reportVersionCheck(context.status, 'rstack', resolution.version); return []; } - const pkg = readPackageJson(pkgJsonPath); - const bin = pkg?.bin; - let binEntry = 'bin/rs.js'; - if (typeof bin === 'string') { - binEntry = bin; - } else if (bin && typeof bin === 'object') { - const rs = (bin as Record).rs; - if (typeof rs === 'string') { - binEntry = rs; - } - } - const rsBinJs = path.resolve(path.dirname(pkgJsonPath), binEntry); + const { cwd, rsBinJs } = resolution.target; context.output.debug(`cwd: ${cwd}; bin: ${rsBinJs}`); const text = document.getText(); @@ -206,18 +371,37 @@ class FmtController implements StackController { requestController.abort(); } + const key = standbyKey(document.uri, resolution.target); + // An already-cancelled request must not burn the standby. + const hot = requestController.signal.aborted + ? undefined + : this.#standby?.consume(key, { + text, + signal: requestController.signal, + }); + const serve = hot ? 'hot' : 'cold'; + this.#lastServe = serve; + let result; try { - result = await runRsFmt({ - text, - filePath: document.uri.fsPath, - cwd, - rsBinJs, - signal: requestController.signal, - }); + result = await (hot ?? + runRsFmt({ + text, + filePath: document.uri.fsPath, + cwd, + rsBinJs, + signal: requestController.signal, + })); } finally { cancellation.dispose(); this.#abortController.signal.removeEventListener('abort', abortRequest); + // A standby serves exactly one request, and a real format request is + // itself proof the active file is worth one — re-arm after cold serves + // too, so an expired or crashed standby comes back on the next use + // instead of leaving the file cold until the editor changes. Scheduled + // rather than immediate: spawning here would delay the edits the caller + // is waiting for. + this.scheduleArm(0); } if ( @@ -246,8 +430,12 @@ class FmtController implements StackController { // have changed while the format was in flight. this.reportRunning(context, this.#snapshot ?? snapshot); const edit = minimalEdit(text, result.formatted); + // The hot/cold marker is the only way to tell from a log whether the + // measured time included a process start-up. context.output.info( - `Formatting completed in ${elapsed}ms${edit ? '' : ' (already formatted)'}`, + `Formatting completed in ${elapsed}ms (${serve}${ + edit ? '' : ', already formatted' + })`, ); if (!edit) { return []; @@ -282,6 +470,12 @@ class FmtController implements StackController { dispose(): void { this.#disposed = true; this.#abortController.abort(); + // Covers `rstack.fmt.restart` (the shell rebuilds the controller) and a + // workspace losing its trust: neither may leave a process behind. + clearTimeout(this.#armTimer); + this.#armTimer = undefined; + this.#standby?.dispose(); + this.#standby = undefined; for (const subscription of this.#subscriptions.splice(0)) { subscription.dispose(); } diff --git a/packages/vscode/src/stacks/fmt/run.test.ts b/packages/vscode/src/stacks/fmt/run.test.ts index 51a1c6e..006612e 100644 --- a/packages/vscode/src/stacks/fmt/run.test.ts +++ b/packages/vscode/src/stacks/fmt/run.test.ts @@ -3,6 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from '@rstest/core'; import { minimalEdit, pickConfigDir, runRsFmt, type RsFmtRun } from './run'; +import { createStubRoot, type StubRoot } from './stubProcess'; describe('pickConfigDir', () => { let root: string; @@ -145,26 +146,17 @@ describe('minimalEdit', () => { }); describe('runRsFmt', () => { - let root: string; + let stubs: StubRoot; beforeEach(() => { - root = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), 'rstack-fmt-run-')), - ); + stubs = createStubRoot('run'); }); afterEach(() => { - fs.rmSync(root, { recursive: true, force: true }); + stubs.remove(); }); - const writeStub = (source: string): string => { - const filePath = path.join( - root, - `rs-${Math.random().toString(16).slice(2)}.js`, - ); - fs.writeFileSync(filePath, source); - return filePath; - }; + const writeStub = (source: string): string => stubs.write(source); const run = ( text: string, @@ -175,8 +167,8 @@ describe('runRsFmt', () => { > => { const options: RsFmtRun = { text, - filePath: path.join(root, 'input.ts'), - cwd: root, + filePath: path.join(stubs.path, 'input.ts'), + cwd: stubs.path, rsBinJs, signal, }; @@ -235,12 +227,16 @@ describe('runRsFmt', () => { const controller = new AbortController(); controller.abort(); await expect( - run('const value = 1;', path.join(root, 'missing.js'), controller.signal), + run( + 'const value = 1;', + path.join(stubs.path, 'missing.js'), + controller.signal, + ), ).resolves.toEqual({ kind: 'cancelled' }); }); it('names an unloadable rs entry path', async () => { - const missing = path.join(root, 'missing.js'); + const missing = path.join(stubs.path, 'missing.js'); const result = await run('const value = 1;', missing); expect(result.kind).toBe('error'); if (result.kind === 'error') { diff --git a/packages/vscode/src/stacks/fmt/run.ts b/packages/vscode/src/stacks/fmt/run.ts index 82209c4..d9effff 100644 --- a/packages/vscode/src/stacks/fmt/run.ts +++ b/packages/vscode/src/stacks/fmt/run.ts @@ -53,7 +53,7 @@ export type RsFmtResult = | { readonly kind: 'cancelled' } | { readonly kind: 'error'; readonly message: string }; -const errorMessage = (error: unknown): string => +export const errorMessage = (error: unknown): string => error instanceof Error ? error.message : String(error); /** Bounds any CLI stderr headed for a log line to its meaningful tail. */ @@ -69,18 +69,176 @@ const launchError = (rsBinJs: string, detail: string): RsFmtResult => ({ export const isRsFmtLaunchError = (result: RsFmtResult): boolean => result.kind === 'error' && result.message.startsWith(LAUNCH_ERROR_PREFIX); -export const runRsFmt = async (run: RsFmtRun): Promise => { - if (run.signal.aborted) { - return { kind: 'cancelled' }; +/** How an `rs fmt` child ended. */ +type RsFmtExit = + | { readonly kind: 'error'; readonly error: unknown } + | { readonly kind: 'close'; readonly code: number | null }; + +/** + * Only the tail of stderr is ever reported, but a parked standby can hold a + * child for minutes — a chatty one must not grow the buffer without bound. + */ +const MAX_STDERR_CHARS = 64 * 1024; + +/** Built once: `process.env` does not change over an extension host's life. */ +const CHILD_ENV = { ...process.env, ELECTRON_RUN_AS_NODE: '1' }; + +/** + * A spawned `rs fmt --stdin-filepath` child whose streams drain from the moment + * it starts. Both fmt paths share it: a cold format writes stdin immediately, + * while a standby parks the child on stdin — `rs fmt` loads the project config + * concurrently with draining stdin, so a parked child has already paid that + * cost — and writes only when a request consumes it. + */ +export interface RsFmtProcess { + readonly rsBinJs: string; + /** + * The terminal event. It is a value rather than a callback slot because the + * child has two independent observers over its life: the standby watches it + * while parked, and `serveRsFmt` watches it for the duration of a request. + */ + readonly exited: Promise; + /** Sends the document and closes stdin. */ + write(text: string): void; + kill(): void; + /** Chunks collected so far; joined once the child closes. */ + readonly stdout: readonly string[]; + readonly stderr: readonly string[]; +} + +export interface RsFmtSpawn { + readonly filePath: string; + readonly cwd: string; + readonly rsBinJs: string; +} + +export type RsFmtSpawnResult = + | { readonly kind: 'spawned'; readonly process: RsFmtProcess } + | { readonly kind: 'error'; readonly error: unknown }; + +/** Starts an `rs fmt` child and begins draining its streams. */ +export const spawnRsFmt = (options: RsFmtSpawn): RsFmtSpawnResult => { + let child: ChildProcessWithoutNullStreams; + try { + child = spawn( + process.execPath, + [ + options.rsBinJs, + 'fmt', + '--stdin-filepath', + options.filePath, + '--ignore-unknown', + ], + { + cwd: options.cwd, + env: CHILD_ENV, + stdio: 'pipe', + }, + ); + } catch (error) { + return { kind: 'error', error }; + } + + const stdout: string[] = []; + const stderr: string[] = []; + let stderrChars = 0; + child.stdout.setEncoding('utf8'); + child.stderr.setEncoding('utf8'); + child.stdout.on('data', (chunk: string) => stdout.push(chunk)); + child.stderr.on('data', (chunk: string) => { + stderr.push(chunk); + stderrChars += chunk.length; + while (stderrChars > MAX_STDERR_CHARS && stderr.length > 1) { + const dropped = stderr.shift(); + stderrChars -= dropped?.length ?? 0; + } + }); + // A child may reject the request before consuming stdin. Its exit status is + // authoritative; EPIPE and write-after-end must not become unhandled errors. + child.stdin.on('error', () => {}); + + const exited = new Promise((resolve) => { + child.on('error', (error) => resolve({ kind: 'error', error })); + child.on('close', (code) => resolve({ kind: 'close', code })); + }); + + return { + kind: 'spawned', + process: { + rsBinJs: options.rsBinJs, + stdout, + stderr, + exited, + write(text) { + try { + child.stdin.end(text); + } catch { + // Wait for the child's close/error event. + } + }, + kill() { + child.kill(); + }, + }, + }; +}; + +const interpretExit = ( + rsFmt: RsFmtProcess, + text: string, + code: number | null, +): RsFmtResult => { + const formatted = rsFmt.stdout.join(''); + const stderrText = rsFmt.stderr.join(''); + if (code === 0) { + if (formatted.length > 0 || text.trim() === '') { + return { kind: 'ok', formatted, stderr: stderrText }; + } + return { kind: 'skipped', stderr: stderrText }; + } + const tail = stderrTail(stderrText); + const missingEntry = stderrText + .split(/\r?\n/) + .some( + (line) => + line.includes('Cannot find module') && line.includes(rsFmt.rsBinJs), + ); + if (missingEntry) { + return launchError(rsFmt.rsBinJs, tail); } + return { + kind: 'error', + message: + tail || + `rs fmt exited with code ${code === null ? 'unknown' : String(code)}`, + }; +}; + +export interface RsFmtServe { + readonly process: RsFmtProcess; + readonly text: string; + readonly signal: AbortSignal; + /** + * Guard timeout. It measures the request, so for a standby it starts at + * consume time rather than at spawn time. + */ + readonly timeoutMs?: number; +} - return new Promise((resolve) => { +/** Writes the document to a child's stdin and resolves with its verdict. */ +export const serveRsFmt = (serve: RsFmtServe): Promise => + new Promise((resolve) => { + const rsFmt = serve.process; + const signal = serve.signal; + const timeoutMs = serve.timeoutMs ?? TIMEOUT_MS; let settled = false; + // Boxed rather than a plain `let`: the timeout is assigned below the + // closure that clears it, and `prefer-const` mis-reads that order as + // "never reassigned". const guard: { timeout?: NodeJS.Timeout } = {}; - let child: ChildProcessWithoutNullStreams | undefined; const onAbort = (): void => { - child?.kill(); + rsFmt.kill(); settle({ kind: 'cancelled' }); }; const settle = (result: RsFmtResult): void => { @@ -88,104 +246,59 @@ export const runRsFmt = async (run: RsFmtRun): Promise => { return; } settled = true; - if (guard.timeout) { - clearTimeout(guard.timeout); - } - run.signal.removeEventListener('abort', onAbort); + clearTimeout(guard.timeout); + signal.removeEventListener('abort', onAbort); resolve(result); }; - try { - child = spawn( - process.execPath, - [ - run.rsBinJs, - 'fmt', - '--stdin-filepath', - run.filePath, - '--ignore-unknown', - ], - { - cwd: run.cwd, - env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, - signal: run.signal, - stdio: 'pipe', - }, - ); - } catch (error) { - settle( - run.signal.aborted - ? { kind: 'cancelled' } - : launchError(run.rsBinJs, errorMessage(error)), - ); - return; - } - - const stdout: string[] = []; - const stderr: string[] = []; - child.stdout.setEncoding('utf8'); - child.stderr.setEncoding('utf8'); - child.stdout.on('data', (chunk: string) => stdout.push(chunk)); - child.stderr.on('data', (chunk: string) => stderr.push(chunk)); - child.on('error', (error) => { - settle( - run.signal.aborted - ? { kind: 'cancelled' } - : launchError(run.rsBinJs, errorMessage(error)), - ); - }); - child.on('close', (code) => { - if (run.signal.aborted) { + void rsFmt.exited.then((exit) => { + if (signal.aborted) { settle({ kind: 'cancelled' }); return; } - - const formatted = stdout.join(''); - const stderrText = stderr.join(''); - if (code === 0) { - if (formatted.length > 0 || run.text.trim() === '') { - settle({ kind: 'ok', formatted, stderr: stderrText }); - } else { - settle({ kind: 'skipped', stderr: stderrText }); - } - return; - } - const tail = stderrTail(stderrText); - const missingEntry = stderrText - .split(/\r?\n/) - .some( - (line) => - line.includes('Cannot find module') && line.includes(run.rsBinJs), - ); - if (missingEntry) { - settle(launchError(run.rsBinJs, tail)); - return; - } - settle({ - kind: 'error', - message: - tail || - `rs fmt exited with code ${code === null ? 'unknown' : String(code)}`, - }); + settle( + exit.kind === 'error' + ? launchError(rsFmt.rsBinJs, errorMessage(exit.error)) + : interpretExit(rsFmt, serve.text, exit.code), + ); }); + if (signal.aborted) { + // `addEventListener` never fires on an already-aborted signal, and a + // standby's child is not bound to the request signal at spawn time. + onAbort(); + return; + } - run.signal.addEventListener('abort', onAbort, { once: true }); + signal.addEventListener('abort', onAbort, { once: true }); guard.timeout = setTimeout(() => { - child?.kill(); + rsFmt.kill(); settle({ kind: 'error', - message: `rs fmt at ${run.rsBinJs} timed out after ${TIMEOUT_MS / 1000} seconds`, + message: `rs fmt at ${rsFmt.rsBinJs} timed out after ${timeoutMs / 1000} seconds`, }); - }, TIMEOUT_MS); - - // A child may reject the request before consuming stdin. Its exit status is - // authoritative; EPIPE and write-after-end must not become unhandled errors. - child.stdin.on('error', () => {}); - try { - child.stdin.end(run.text); - } catch { - // Wait for the child's close/error event. - } + }, timeoutMs); + + rsFmt.write(serve.text); + }); + +export const runRsFmt = async (run: RsFmtRun): Promise => { + if (run.signal.aborted) { + return { kind: 'cancelled' }; + } + + const spawned = spawnRsFmt({ + filePath: run.filePath, + cwd: run.cwd, + rsBinJs: run.rsBinJs, + }); + if (spawned.kind === 'error') { + return launchError(run.rsBinJs, errorMessage(spawned.error)); + } + + return serveRsFmt({ + process: spawned.process, + text: run.text, + signal: run.signal, }); }; diff --git a/packages/vscode/src/stacks/fmt/standby.test.ts b/packages/vscode/src/stacks/fmt/standby.test.ts new file mode 100644 index 0000000..fd2d40e --- /dev/null +++ b/packages/vscode/src/stacks/fmt/standby.test.ts @@ -0,0 +1,284 @@ +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from '@rstest/core'; +import { FmtStandby, type StandbyKey } from './standby'; +import { createStubRoot, type StubRoot } from './stubProcess'; + +/** + * The stub scripts stand in for `rs fmt`: they are spawned exactly the way the + * CLI is, so the tests exercise the real parking, stdin write and exit + * interpretation rather than a mocked child process (same technique as + * `run.test.ts`). + */ +describe('FmtStandby', () => { + let stubs: StubRoot; + let standbys: FmtStandby[]; + let logs: string[]; + + beforeEach(() => { + stubs = createStubRoot('standby'); + standbys = []; + logs = []; + }); + + afterEach(() => { + for (const standby of standbys) { + standby.dispose(); + } + stubs.remove(); + }); + + const writeStub = (source: string): string => stubs.write(source); + const echoStub = (): string => stubs.echo(); + + const key = (rsBinJs: string, file = 'input.ts'): StandbyKey => ({ + cwd: stubs.path, + filePath: path.join(stubs.path, file), + rsBinJs, + }); + + const createStandby = ( + options: { idleTimeoutMs?: number; requestTimeoutMs?: number } = {}, + ): FmtStandby => { + const standby = new FmtStandby({ + log: (message) => logs.push(message), + ...options, + }); + standbys.push(standby); + return standby; + }; + + const consume = ( + standby: FmtStandby, + standbyKey: StandbyKey, + text: string, + signal: AbortSignal = new AbortController().signal, + ) => standby.consume(standbyKey, { text, signal }); + + const eventually = async ( + probe: () => boolean, + what: string, + ): Promise => { + const deadline = Date.now() + 5_000; + while (!probe()) { + if (Date.now() >= deadline) { + throw new Error(`timed out waiting for ${what}`); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + }; + + it('arms a parked process that has not been written to', async () => { + // The stub only ever prints once it has read stdin, so seeing no output + // after arming is what "parked" means. + const stub = writeStub( + 'process.stdin.on("data", () => process.stdout.write("served"));\n', + ); + const standby = createStandby(); + const armed = key(stub); + expect(standby.arm(armed)).toBe(true); + expect(standby.armedFilePath).toBe(armed.filePath); + expect(logs).toContain(`Standby armed for ${armed.filePath}`); + + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(standby.armedFilePath).toBe(armed.filePath); + }); + + it('serves the request it was armed for from the parked process', async () => { + const standby = createStandby(); + const armed = key(echoStub()); + expect(standby.arm(armed)).toBe(true); + const text = `const value = 1;\n${'x'.repeat(1024 * 64)}\n`; + await expect(consume(standby, armed, text)).resolves.toEqual({ + kind: 'ok', + formatted: text, + stderr: '', + }); + expect(logs).toContain(`Standby consumed for ${armed.filePath}`); + }); + + it('serves exactly one request', async () => { + const standby = createStandby(); + const armed = key(echoStub()); + standby.arm(armed); + await consume(standby, armed, 'const value = 1;\n'); + expect(consume(standby, armed, 'const value = 1;\n')).toBeUndefined(); + expect(standby.armedFilePath).toBeUndefined(); + }); + + it('misses when the request key does not match', async () => { + const standby = createStandby(); + const stub = echoStub(); + const armed = key(stub); + standby.arm(armed); + + expect(consume(standby, key(stub, 'other.ts'), 'x')).toBeUndefined(); + expect( + consume(standby, { ...armed, cwd: path.join(stubs.path, 'nested') }, 'x'), + ).toBeUndefined(); + expect( + consume(standby, { ...armed, rsBinJs: echoStub() }, 'x'), + ).toBeUndefined(); + // A miss leaves the standby armed for the file it does match. + expect(standby.armedFilePath).toBe(armed.filePath); + await expect(consume(standby, armed, 'x')).resolves.toEqual({ + kind: 'ok', + formatted: 'x', + stderr: '', + }); + }); + + it('re-arms on another file by replacing the parked process', () => { + const standby = createStandby(); + const stub = echoStub(); + standby.arm(key(stub)); + const next = key(stub, 'other.ts'); + expect(standby.arm(next)).toBe(true); + expect(standby.armedFilePath).toBe(next.filePath); + expect(logs).toContain( + 'Standby killed (the active editor moved to another file)', + ); + }); + + it('keeps the same process when re-armed for the same key', () => { + const standby = createStandby(); + const armed = key(echoStub()); + standby.arm(armed); + expect(standby.arm(armed)).toBe(true); + expect( + logs.filter((line) => line.startsWith('Standby armed')), + ).toHaveLength(1); + }); + + it('kills a parked process on invalidation', () => { + const standby = createStandby(); + standby.arm(key(echoStub())); + standby.kill('the config changed'); + expect(standby.armedFilePath).toBeUndefined(); + expect(logs).toContain('Standby killed (the config changed)'); + expect(consume(standby, key(echoStub()), 'x')).toBeUndefined(); + }); + + it('discards a parked process that exits on its own', async () => { + const standby = createStandby(); + const armed = key(writeStub('process.exit(3);\n')); + expect(standby.arm(armed)).toBe(true); + await eventually( + () => standby.armedFilePath === undefined, + 'the parked process to be discarded', + ); + expect(logs).toContain('Standby killed (the parked process exited)'); + // The next request misses and the caller formats cold. + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); + + it('expires a standby nobody consumed', async () => { + const standby = createStandby({ idleTimeoutMs: 20 }); + const armed = key(echoStub()); + standby.arm(armed); + await eventually( + () => standby.armedFilePath === undefined, + 'the standby to expire', + ); + expect(logs).toContain('Standby killed (expired after idling)'); + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); + + it('does not expire a standby that is serving a request', async () => { + const standby = createStandby({ idleTimeoutMs: 30 }); + const armed = key( + writeStub( + 'process.stdin.on("data", (chunk) => setTimeout(() => { process.stdout.write(chunk); process.exit(0); }, 150));\n', + ), + ); + standby.arm(armed); + await expect(consume(standby, armed, 'late')).resolves.toEqual({ + kind: 'ok', + formatted: 'late', + stderr: '', + }); + }); + + it('starts the guard timeout at consume, not at arm', async () => { + const standby = createStandby({ requestTimeoutMs: 100 }); + const stub = writeStub('setTimeout(() => {}, 30_000);\n'); + const armed = key(stub); + standby.arm(armed); + // The parked process outlives the guard window before the request starts. + await new Promise((resolve) => setTimeout(resolve, 200)); + const result = await consume(standby, armed, 'const value = 1;\n'); + expect(result).toEqual({ + kind: 'error', + message: `rs fmt at ${stub} timed out after 0.1 seconds`, + }); + }); + + it('cancels a hot request and kills its process', async () => { + const standby = createStandby(); + const armed = key(writeStub('setTimeout(() => {}, 30_000);\n')); + standby.arm(armed); + const controller = new AbortController(); + const result = consume( + standby, + armed, + 'const value = 1;\n', + controller.signal, + ); + setTimeout(() => controller.abort(), 50); + await expect(result).resolves.toEqual({ kind: 'cancelled' }); + }); + + it('reports the CLI verdict for a failing hot request', async () => { + const standby = createStandby(); + const armed = key( + writeStub( + 'process.stdin.on("data", () => { console.error("boom"); process.exit(2); });\n', + ), + ); + standby.arm(armed); + await expect(consume(standby, armed, 'broken')).resolves.toEqual({ + kind: 'error', + message: 'boom', + }); + }); + + it('distinguishes an ignored file from a whitespace-only document', async () => { + const standby = createStandby(); + const silent = writeStub('process.stdin.resume();\n'); + const ignored = key(silent); + standby.arm(ignored); + await expect( + consume(standby, ignored, 'const value = 1;\n'), + ).resolves.toEqual({ kind: 'skipped', stderr: '' }); + + const blank = key(silent, 'blank.ts'); + standby.arm(blank); + await expect(consume(standby, blank, ' \n')).resolves.toEqual({ + kind: 'ok', + formatted: '', + stderr: '', + }); + }); + + it('does not arm after dispose', () => { + const standby = createStandby(); + standby.arm(key(echoStub())); + standby.dispose(); + expect(standby.armedFilePath).toBeUndefined(); + expect(logs).toContain('Standby killed (the fmt stack was disposed)'); + expect(standby.arm(key(echoStub()))).toBe(false); + expect(standby.armedFilePath).toBeUndefined(); + }); + + it('discards a standby whose CLI entry cannot be loaded', async () => { + const standby = createStandby(); + // The spawn succeeds — node reports an unloadable entry by exiting — so an + // arm on a broken resolution costs nothing but a debug line. + const armed = key(path.join(stubs.path, 'missing.js')); + expect(standby.arm(armed)).toBe(true); + await eventually( + () => standby.armedFilePath === undefined, + 'the unloadable standby to be discarded', + ); + expect(consume(standby, armed, 'x')).toBeUndefined(); + }); +}); diff --git a/packages/vscode/src/stacks/fmt/standby.ts b/packages/vscode/src/stacks/fmt/standby.ts new file mode 100644 index 0000000..8a56ce8 --- /dev/null +++ b/packages/vscode/src/stacks/fmt/standby.ts @@ -0,0 +1,172 @@ +import { + errorMessage, + type RsFmtProcess, + type RsFmtResult, + serveRsFmt, + spawnRsFmt, +} from './run'; + +/** + * What a standby is bound to. `rs fmt` reads `--stdin-filepath` from argv and + * resolves its config from the spawn cwd, so none of the three can change + * after the process starts: a request that does not match all three has to be + * formatted cold. + */ +export interface StandbyKey { + readonly cwd: string; + readonly filePath: string; + readonly rsBinJs: string; +} + +export const sameStandbyKey = (left: StandbyKey, right: StandbyKey): boolean => + left.cwd === right.cwd && + left.filePath === right.filePath && + left.rsBinJs === right.rsBinJs; + +/** + * How long an armed-but-unconsumed standby is kept. Expiry is not an error — + * it reclaims the memory of a process nobody is going to use, and the next + * eligible event arms a new one. + */ +const IDLE_TIMEOUT_MS = 5 * 60_000; + +export interface FmtStandbyOptions { + /** + * Debug-level sink. The standby is invisible to the user: arming never + * reports status and never logs above debug. + */ + readonly log: (message: string) => void; + /** Idle lifetime of an armed standby; injected short by the unit tests. */ + readonly idleTimeoutMs?: number; + /** Guard timeout for a hot format; starts at consume, not at spawn. */ + readonly requestTimeoutMs?: number; +} + +interface ArmedStandby { + readonly key: StandbyKey; + readonly process: RsFmtProcess; + readonly idleTimer: NodeJS.Timeout; +} + +/** + * The single pre-spawned `rs fmt` process that tracks the active editor. + * + * It is a bounded exception to the extension's "no warm tier" rule: at most one + * process, bound to one file, serving exactly one request. There is no + * protocol, no pool and no state carried between requests — everything the + * standby saves is the process start-up plus config load a cold format pays on + * the critical path. + * + * Deliberately vscode-free (like `run.ts`) so the whole lifecycle is unit + * testable; the controller owns every VS Code event that drives it. + */ +export class FmtStandby { + #armed: ArmedStandby | undefined; + #disposed = false; + + constructor(private readonly options: FmtStandbyOptions) {} + + /** The file the standby currently targets, if any. */ + get armedFilePath(): string | undefined { + return this.#armed?.key.filePath; + } + + /** + * Spawns the standby for `key`, replacing any standby on another key. + * Returns whether a standby is now armed for `key`. Failures are reported + * through `log` only: a standby that cannot be armed costs a cold format, + * never a user-visible error. + */ + arm(key: StandbyKey): boolean { + if (this.#disposed) { + return false; + } + if (this.#armed) { + if (sameStandbyKey(this.#armed.key, key)) { + return true; + } + // Replacing, not invalidating: the caller is already arming the + // replacement, so this kill must not ask for another one. + this.kill('the active editor moved to another file'); + } + + const spawned = spawnRsFmt({ + cwd: key.cwd, + filePath: key.filePath, + rsBinJs: key.rsBinJs, + }); + if (spawned.kind === 'error') { + this.options.log( + `Standby not armed for ${key.filePath}: ${errorMessage(spawned.error)}`, + ); + return false; + } + + const armed: ArmedStandby = { + key, + process: spawned.process, + idleTimer: setTimeout(() => { + this.kill('expired after idling'); + }, this.options.idleTimeoutMs ?? IDLE_TIMEOUT_MS), + }; + this.#armed = armed; + // A parked child that dies on its own (a crash, a config load throwing) is + // not an error either — the next request simply formats cold. The identity + // check is what makes this a no-op once the child has been consumed or + // replaced, so nothing has to unsubscribe. + void spawned.process.exited.then(() => { + if (this.#armed === armed) { + this.kill('the parked process exited'); + } + }); + this.options.log(`Standby armed for ${key.filePath}`); + return true; + } + + /** + * Serves one format request from the standby, or returns `undefined` when it + * cannot — nothing armed, a different key, or a standby already taken by a + * concurrent request. The caller then formats cold. + */ + consume( + key: StandbyKey, + request: { readonly text: string; readonly signal: AbortSignal }, + ): Promise | undefined { + const armed = this.#armed; + if (!armed || !sameStandbyKey(armed.key, key)) { + return undefined; + } + // A standby serves exactly one request. Taking it here is also what makes a + // second, concurrent request miss and go cold. + this.#armed = undefined; + clearTimeout(armed.idleTimer); + this.options.log(`Standby consumed for ${key.filePath}`); + return serveRsFmt({ + process: armed.process, + text: request.text, + signal: request.signal, + timeoutMs: this.options.requestTimeoutMs, + }); + } + + /** + * Invalidates the standby. The caller re-arms afterwards if the active editor + * still qualifies — a parked process has already loaded the project config, + * so a config edit (or any detection change) makes it wrong, not stale. + */ + kill(reason: string): void { + const armed = this.#armed; + if (!armed) { + return; + } + this.#armed = undefined; + clearTimeout(armed.idleTimer); + armed.process.kill(); + this.options.log(`Standby killed (${reason})`); + } + + dispose(): void { + this.#disposed = true; + this.kill('the fmt stack was disposed'); + } +} diff --git a/packages/vscode/src/stacks/fmt/stubProcess.ts b/packages/vscode/src/stacks/fmt/stubProcess.ts new file mode 100644 index 0000000..3d23c99 --- /dev/null +++ b/packages/vscode/src/stacks/fmt/stubProcess.ts @@ -0,0 +1,45 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** + * Stub-script scaffolding shared by the fmt unit suites. + * + * The suites stand in for `rs fmt` with tiny node scripts spawned exactly the + * way the CLI is, so they exercise real process behaviour — parking on stdin, + * back-pressure, EPIPE, exit codes — instead of a mocked child. That technique + * is load-bearing for both `run.test.ts` and `standby.test.ts`, so it lives + * here rather than being copied into each. + * + * Not a `*.test.ts` file (the runner would collect it as a suite) and not a + * build entry, so it never ships in the VSIX. + */ +export interface StubRoot { + /** Temp directory the stubs live in; also used as the spawn cwd. */ + readonly path: string; + /** Writes a stub script and returns its path. */ + write(source: string): string; + /** Echoes stdin back, i.e. an `rs fmt` that finds nothing to change. */ + echo(): string; + remove(): void; +} + +export const createStubRoot = (prefix: string): StubRoot => { + const root = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), `rstack-fmt-${prefix}-`)), + ); + const write = (source: string): string => { + const filePath = path.join( + root, + `rs-${Math.random().toString(16).slice(2)}.js`, + ); + fs.writeFileSync(filePath, source); + return filePath; + }; + return { + path: root, + write, + echo: () => write('process.stdin.pipe(process.stdout);\n'), + remove: () => fs.rmSync(root, { recursive: true, force: true }), + }; +}; diff --git a/packages/vscode/tests/e2e/suite/fmt.test.ts b/packages/vscode/tests/e2e/suite/fmt.test.ts index 1cc0933..c162b60 100644 --- a/packages/vscode/tests/e2e/suite/fmt.test.ts +++ b/packages/vscode/tests/e2e/suite/fmt.test.ts @@ -5,6 +5,22 @@ import { eventually } from './helpers'; const EXTENSION_ID = 'rstack.rstack'; let provider: vscode.DocumentFormattingEditProvider; +let armedFilePath: () => string | undefined; +let lastServe: () => 'hot' | 'cold' | undefined; + +/** + * Shows a document and waits for it to own the standby. Showing it is what arms + * one — the invariant is that the standby tracks the active editor — but arming + * is debounced and an earlier test may have left a standby on another file, so + * the wait polls until the armed file is this one. + */ +const armStandbyFor = async (uri: vscode.Uri): Promise => { + const editor = await vscode.window.showTextDocument(uri); + await eventually(() => { + assert.equal(armedFilePath(), uri.fsPath); + }, `the standby to be armed for ${uri.fsPath}`); + return editor; +}; const folderNamed = (name: string): vscode.WorkspaceFolder => { const folder = (vscode.workspace.workspaceFolders ?? []).find( @@ -23,6 +39,12 @@ suite('fmt', () => { const exports = await api.whenStackActive('fmt'); assert.ok(exports.provider, 'the fmt stack did not export its provider'); provider = exports.provider as vscode.DocumentFormattingEditProvider; + assert.ok( + typeof exports.armedFilePath === 'function', + 'the fmt stack did not export its standby hook', + ); + armedFilePath = exports.armedFilePath as () => string | undefined; + lastServe = exports.lastServe as () => 'hot' | 'cold' | undefined; }); test('formats through the provider without touching the workspace', async () => { @@ -60,6 +82,59 @@ suite('fmt', () => { assert.equal(applied, 'const answer = { value: "42" };\n'); }); + test('formats from the standby armed for the active editor', async () => { + const uri = vscode.Uri.joinPath( + folderNamed('rstack').uri, + 'src', + 'needs-format.ts', + ); + const editor = await armStandbyFor(uri); + + const edits = await vscode.commands.executeCommand( + 'vscode.executeFormatDocumentProvider', + uri, + { tabSize: 2, insertSpaces: true }, + ); + assert.ok(edits && edits.length > 0, 'the formatter returned no edits'); + + const text = editor.document.getText(); + let applied = text; + // The command post-processes our single minimal edit through VS Code's + // `computeMoreMinimalEdits`, so apply its result from the end backwards. + for (const edit of [...edits].sort( + (left, right) => + editor.document.offsetAt(right.range.start) - + editor.document.offsetAt(left.range.start), + )) { + const start = editor.document.offsetAt(edit.range.start); + const end = editor.document.offsetAt(edit.range.end); + applied = applied.slice(0, start) + edit.newText + applied.slice(end); + } + assert.equal(applied, 'const answer = { value: "42" };\n'); + // The cold path produces the same text, so only this proves the request + // actually consumed the standby. + assert.equal(lastServe(), 'hot'); + }); + + test('clears the standby when the active editor cannot be armed', async () => { + await armStandbyFor( + vscode.Uri.joinPath(folderNamed('rstack').uri, 'src', 'needs-format.ts'), + ); + + // TypeScript, so the provider matches it, but fmt is not detected for this + // folder — nothing can be armed, and the previous file's standby must not + // outlive the editor that owned it. + const unarmable = vscode.Uri.joinPath( + folderNamed('rslint').uri, + 'src', + 'index.ts', + ); + await vscode.window.showTextDocument(unarmable); + await eventually(() => { + assert.equal(armedFilePath(), undefined); + }, 'the standby to be cleared'); + }); + test('returns no edits for a folder where fmt is not detected', async () => { const uri = vscode.Uri.joinPath( folderNamed('rslint').uri,