From 89fd1e6c5dedc43f1d0eb8d59b24d1446bf278c0 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 14 Sep 2026 15:50:07 -0700 Subject: [PATCH 1/2] feat: report diagnostics for malformed PEP 723 inline script metadata Malformed inline script metadata was previously invisible. A block with a typo -- a missing closing `# ///`, a content line without the required space after `#`, a TOML syntax error -- collapsed to `undefined` in the parser, so no CodeLens appeared and nothing explained why. The file simply looked like ordinary Python. The parser now returns a discriminated result (`valid` / `invalid` / `none`) carrying structured problems with source ranges, and a new `InlineScriptDiagnosticsPublisher` surfaces them as squiggles on open, save, debounced change (300ms) and clears on close, delete, rename and dispose. Diagnostics are validated against the live editor buffer so squiggles match what is on screen, while provisioning continues to read the saved file. Both share one parser, so the two views cannot disagree about what "valid" means. Gated behind the existing `python-envs.inlineScripts.enabled` setting. No Pylance or Python extension changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/common/inlineScript/metadata.ts | 368 ++++++++++++++++-- src/common/localize.ts | 38 ++ src/common/workspace.apis.ts | 8 + src/extension.ts | 5 + src/features/inlineScript/diagnostics.ts | 223 +++++++++++ .../metadataDiagnostics.unit.test.ts | 349 +++++++++++++++++ .../inlineScript/diagnostics.unit.test.ts | 318 +++++++++++++++ 7 files changed, 1267 insertions(+), 42 deletions(-) create mode 100644 src/features/inlineScript/diagnostics.ts create mode 100644 src/test/common/inlineScript/metadataDiagnostics.unit.test.ts create mode 100644 src/test/features/inlineScript/diagnostics.unit.test.ts diff --git a/src/common/inlineScript/metadata.ts b/src/common/inlineScript/metadata.ts index 0ef424fe..01fac5a6 100644 --- a/src/common/inlineScript/metadata.ts +++ b/src/common/inlineScript/metadata.ts @@ -28,14 +28,6 @@ export interface InlineScriptMetadata { * newline (or end of string if there is no trailing newline). */ readonly range: { readonly start: number; readonly end: number }; - /** - * Character offsets of the same metadata block in the original source - * text. Unlike {@link range}, these include a leading BOM and preserve - * CRLF, so they can be compared with TextDocument change offsets. - * - * Optional to keep manually constructed metadata compatible; parser - * results always supply it. - */ readonly sourceRange?: { readonly start: number; readonly end: number }; } @@ -48,6 +40,30 @@ export interface InlineScriptMetadata { */ export const MAX_HEADER_BYTES = 8 * 1024; +export type InlineScriptMetadataProblemCode = + | 'unterminated-block' + | 'multiple-blocks' + | 'invalid-content-line' + | 'invalid-block-marker' + | 'invalid-toml' + | 'invalid-field-type'; + +export interface InlineScriptMetadataProblem { + readonly code: InlineScriptMetadataProblemCode; + readonly severity: 'error' | 'warning'; + readonly sourceRange: { readonly start: number; readonly end: number }; + readonly detail?: string; +} + +export type InlineScriptMetadataParseResult = + | { + readonly kind: 'valid'; + readonly metadata: InlineScriptMetadata; + readonly problems: readonly InlineScriptMetadataProblem[]; + } + | { readonly kind: 'none' } + | { readonly kind: 'invalid'; readonly problems: readonly InlineScriptMetadataProblem[] }; + /** * Canonical block regex from the PEP 723 spec, translated to JavaScript * (Python's `(?P...)` becomes `(?...)` in JS). The flag @@ -64,8 +80,15 @@ export const MAX_HEADER_BYTES = 8 * 1024; * which constructs a fresh iterator each call and does NOT mutate the * regex's `lastIndex`. Do not call `BLOCK_RE.exec` directly — that * would reintroduce the stateful-lastIndex footgun. + * + * Deviation from the spec regex: `*` not `+`, because the prose spec + * permits an empty block and takes precedence over the regex. */ -const BLOCK_RE = /^# \/\/\/ (?[a-zA-Z0-9-]+)$\s(?(^#(| .*)$\s)+)^# \/\/\/$/gm; +const BLOCK_RE = /^# \/\/\/ (?[a-zA-Z0-9-]+)$\s(?(^#(| .*)$\s)*)^# \/\/\/$/gm; + +const OPENER_SCAN_RE = /^# \/\/\/ (?[a-zA-Z0-9-]+)(?[ \t]*)$/gm; + +const CLOSER_LINE = '# ///'; /** * Parse PEP 723 `script` metadata from script source text. @@ -85,9 +108,19 @@ const BLOCK_RE = /^# \/\/\/ (?[a-zA-Z0-9-]+)$\s(?(^#(| .*)$\s)+)^ * identical whether or not it is supplied. */ export function readInlineScriptMetadata(scriptText: string, source?: string): InlineScriptMetadata | undefined { + const result = parseInlineScriptMetadata(scriptText, source); + return result.kind === 'valid' ? result.metadata : undefined; +} + +const NO_METADATA: InlineScriptMetadataParseResult = { kind: 'none' }; + +const OPENER_PREFIX = '# /// '; + +/** As `readInlineScriptMetadata`, but reports why and where parsing failed. Offsets index the original `scriptText`. */ +export function parseInlineScriptMetadata(scriptText: string, source?: string): InlineScriptMetadataParseResult { const where = source ? ` in ${source}` : ''; if (!scriptText) { - return undefined; + return NO_METADATA; } // Strip a single leading UTF-8 BOM (\uFEFF). Files saved as @@ -96,13 +129,17 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I // match. const bomOffset = scriptText.charCodeAt(0) === 0xfeff ? 1 : 0; const sourceText = scriptText.slice(bomOffset); - let text = sourceText; // Normalize CRLF and lone CR to LF so the canonical regex (which // was authored assuming `.` matches `\r`, true in Python's re but // not in JavaScript) behaves consistently. The offsets in `range` // refer to this normalized text. - text = text.replace(/\r\n?/g, '\n'); + const text = sourceText.replace(/\r\n?/g, '\n'); + + const toSourceRange = (start: number, end: number): { start: number; end: number } => ({ + start: bomOffset + sourceOffsetForNormalizedOffset(sourceText, start), + end: bomOffset + sourceOffsetForNormalizedOffset(sourceText, end), + }); // Collect ALL matches first so we can detect the "multiple script // blocks" error case the spec requires us to surface. @@ -120,15 +157,43 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I } } + const matchedRanges = scriptMatches.map((m) => ({ start: m.index!, end: m.index! + m[0].length })); + const problems: InlineScriptMetadataProblem[] = []; + const headerEnd = headerRegionEnd(text); + for (const opener of findScriptOpeners(text)) { + if (matchedRanges.some((r) => opener.offset >= r.start && opener.offset < r.end)) { + continue; + } + const problem = diagnoseMalformedBlock(text, opener, toSourceRange, where); + // Unclosed blocks are ignored per spec; only flag one in the leading + // comment region, where it is a header being typed rather than an example. + if (problem.code === 'unterminated-block' && opener.offset >= headerEnd) { + continue; + } + problems.push(problem); + } + if (scriptMatches.length === 0) { - traceVerbose(`inline script metadata${where}: no \`# /// script\` block found`); - return undefined; + if (problems.length === 0) { + traceVerbose(`inline script metadata${where}: no \`# /// script\` block found`); + return NO_METADATA; + } + return { kind: 'invalid', problems }; } if (scriptMatches.length > 1) { traceWarn( `inline script metadata${where}: ${scriptMatches.length} \`# /// script\` blocks found; per PEP 723 multiple blocks of the same type MUST be an error.`, ); - return undefined; + for (const extra of scriptMatches.slice(1)) { + const start = extra.index!; + problems.push({ + code: 'multiple-blocks', + severity: 'error', + sourceRange: toSourceRange(start, lineEndOffset(text, start)), + detail: String(scriptMatches.length), + }); + } + return { kind: 'invalid', problems }; } const match = scriptMatches[0]; @@ -146,11 +211,15 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I // safety against regex-engine quirks and to keep the // reconstruction logic obvious. const reconstructed: string[] = []; + const reconstructedOrigins: number[] = []; const contentLines = rawContent.split('\n'); // 1-based file line of the `# /// script` marker. Content lines start on // the next line, so content index `i` sits on `blockStartLine + 1 + i`. const blockStartLine = countLines(text, matchStart); + let lineOffset = matchStart + OPENER_PREFIX.length + match.groups!.type.length + 1; for (const [index, line] of contentLines.entries()) { + const lineStart = lineOffset; + lineOffset += line.length + 1; // step over the '\n' that terminated this line if (line.length === 0) { // Final element after splitting on the trailing '\n' that // belongs to the last content line. Not a real line. @@ -161,11 +230,18 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I `inline script metadata${where}: invalid content line ${blockStartLine + 1 + index} ` + `(must start with '#'): ${JSON.stringify(line)}`, ); - return undefined; + problems.push({ + code: 'invalid-content-line', + severity: 'error', + sourceRange: toSourceRange(lineStart, lineStart + line.length), + detail: line, + }); + return { kind: 'invalid', problems }; } if (line.length === 1) { // Bare '#': a blank content line within the block. reconstructed.push(''); + reconstructedOrigins.push(lineStart + 1); continue; } if (line[1] !== ' ') { @@ -175,9 +251,16 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I `inline script metadata${where}: invalid content line ${blockStartLine + 1 + index} ` + `(expected '#' or '# '): ${JSON.stringify(line)}`, ); - return undefined; + problems.push({ + code: 'invalid-content-line', + severity: 'error', + sourceRange: toSourceRange(lineStart, lineStart + line.length), + detail: line, + }); + return { kind: 'invalid', problems }; } reconstructed.push(line.slice(2)); + reconstructedOrigins.push(lineStart + 2); } let parsed: tomljs.JsonMap; @@ -189,25 +272,40 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I // TOML, not the script, so it is translated here rather than shown. The // full error (with stack and excerpt) goes to the debug level for anyone // diagnosing the parser itself. - const tomlRow = getTomlErrorRow(err); - const at = tomlRow === undefined ? '' : ` (line ${blockStartLine + 1 + tomlRow})`; - traceWarn( - `inline script metadata${where}: invalid TOML in the \`# /// script\` block${at}: ${describeTomlError(err)}`, - ); + const position = getTomlErrorPosition(err); + const detail = describeTomlError(err); + const at = position === undefined ? '' : ` (line ${blockStartLine + 1 + position.row})`; + traceWarn(`inline script metadata${where}: invalid TOML in the \`# /// script\` block${at}: ${detail}`); traceVerbose(`inline script metadata${where}: TOML parse error detail:`, err); - return undefined; + problems.push({ + code: 'invalid-toml', + severity: 'error', + sourceRange: tomlErrorSourceRange(text, reconstructedOrigins, position, matchStart, toSourceRange), + detail, + }); + return { kind: 'invalid', problems }; } // Validate the small set of known fields. Unknown top-level keys // are tolerated — the spec reserves room for future tool tables // and we don't want to be brittle. + const fieldRange = (key: string) => + findKeyRange(reconstructed, reconstructedOrigins, key, toSourceRange) ?? + toSourceRange(matchStart, lineEndOffset(text, matchStart)); + let requiresPython: string | undefined; if (parsed['requires-python'] !== undefined) { if (typeof parsed['requires-python'] !== 'string') { traceWarn( `inline script metadata${where}: 'requires-python' must be a string, got ${typeof parsed['requires-python']}`, ); - return undefined; + problems.push({ + code: 'invalid-field-type', + severity: 'error', + sourceRange: fieldRange('requires-python'), + detail: 'requires-python', + }); + return { kind: 'invalid', problems }; } requiresPython = parsed['requires-python']; } @@ -216,12 +314,24 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I if (parsed.dependencies !== undefined) { if (!Array.isArray(parsed.dependencies)) { traceWarn(`inline script metadata${where}: \`dependencies\` must be an array of strings`); - return undefined; + problems.push({ + code: 'invalid-field-type', + severity: 'error', + sourceRange: fieldRange('dependencies'), + detail: 'dependencies', + }); + return { kind: 'invalid', problems }; } for (const dep of parsed.dependencies) { if (typeof dep !== 'string') { traceWarn(`inline script metadata${where}: each entry in \`dependencies\` must be a string`); - return undefined; + problems.push({ + code: 'invalid-field-type', + severity: 'error', + sourceRange: fieldRange('dependencies'), + detail: 'dependencies', + }); + return { kind: 'invalid', problems }; } } // Defensive copy + freeze so consumers can't mutate the cached @@ -233,7 +343,13 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I if (parsed.tool !== undefined) { if (typeof parsed.tool !== 'object' || Array.isArray(parsed.tool) || parsed.tool === null) { traceWarn(`inline script metadata${where}: \`tool\` must be a table`); - return undefined; + problems.push({ + code: 'invalid-field-type', + severity: 'error', + sourceRange: fieldRange('tool'), + detail: 'tool', + }); + return { kind: 'invalid', problems }; } tool = parsed.tool as tomljs.JsonMap; } @@ -247,13 +363,14 @@ export function readInlineScriptMetadata(scriptText: string, source?: string): I } return { - requiresPython, - dependencies, - tool, - range: { start: matchStart, end }, - sourceRange: { - start: bomOffset + sourceOffsetForNormalizedOffset(sourceText, matchStart), - end: bomOffset + sourceOffsetForNormalizedOffset(sourceText, end), + kind: 'valid', + problems, + metadata: { + requiresPython, + dependencies, + tool, + range: { start: matchStart, end }, + sourceRange: toSourceRange(matchStart, end), }, }; } @@ -269,16 +386,176 @@ function countLines(text: string, offset: number): number { return line; } -/** - * Zero-based row reported by `@iarna/toml`, relative to the reconstructed TOML - * payload. Returns `undefined` when the thrown value does not carry one. - */ -function getTomlErrorRow(err: unknown): number | undefined { +function lineEndOffset(text: string, offset: number): number { + const eol = text.indexOf('\n', offset); + return eol === -1 ? text.length : eol; +} + +/** Offset at which the file's leading blank/comment region ends, i.e. where real code starts. */ +function headerRegionEnd(text: string): number { + let offset = 0; + while (offset < text.length) { + const lineEnd = lineEndOffset(text, offset); + const trimmed = text.slice(offset, lineEnd).trim(); + if (trimmed.length > 0 && !trimmed.startsWith('#')) { + return offset; + } + if (lineEnd >= text.length) { + break; + } + offset = lineEnd + 1; + } + return text.length; +} + +interface ScriptOpener { + readonly offset: number; + readonly lineEnd: number; + readonly trailing: string; +} + +function findScriptOpeners(text: string): ScriptOpener[] { + const openers: ScriptOpener[] = []; + for (const m of text.matchAll(OPENER_SCAN_RE)) { + if (m.groups?.type !== 'script') { + continue; + } + const offset = m.index!; + openers.push({ + offset, + lineEnd: offset + m[0].length, + trailing: m.groups.trailing ?? '', + }); + } + return openers; +} + +function diagnoseMalformedBlock( + text: string, + opener: ScriptOpener, + toSourceRange: (start: number, end: number) => { start: number; end: number }, + where: string, +): InlineScriptMetadataProblem { + const openerRange = toSourceRange(opener.offset, opener.lineEnd); + + if (opener.trailing.length > 0) { + traceWarn( + `inline script metadata${where}: the \`# /// script\` marker on line ${countLines(text, opener.offset)} has trailing whitespace`, + ); + return { + code: 'invalid-block-marker', + severity: 'error', + sourceRange: openerRange, + detail: `${OPENER_PREFIX}script${opener.trailing}`, + }; + } + + let offset = opener.lineEnd + 1; // first character of the line after the opener + while (offset <= text.length) { + const lineEnd = lineEndOffset(text, offset); + const line = text.slice(offset, lineEnd); + + if (line === CLOSER_LINE) { + break; + } + + if (line !== line.trimEnd() && line.trimEnd() === CLOSER_LINE) { + traceWarn( + `inline script metadata${where}: the closing \`# ///\` marker on line ${countLines(text, offset)} has trailing whitespace`, + ); + return { + code: 'invalid-block-marker', + severity: 'error', + sourceRange: toSourceRange(offset, lineEnd), + detail: line, + }; + } + + if (!isValidContentLine(line)) { + if (line.startsWith('#')) { + traceWarn( + `inline script metadata${where}: invalid content line ${countLines(text, offset)} ` + + `(expected '#' or '# '): ${JSON.stringify(line)}`, + ); + return { + code: 'invalid-content-line', + severity: 'error', + sourceRange: toSourceRange(offset, lineEnd), + detail: line, + }; + } + break; + } + + if (lineEnd >= text.length) { + break; + } + offset = lineEnd + 1; + } + + traceWarn( + `inline script metadata${where}: the \`# /// script\` block on line ${countLines(text, opener.offset)} is missing its closing \`# ///\` marker`, + ); + return { code: 'unterminated-block', severity: 'warning', sourceRange: openerRange }; +} + +function isValidContentLine(line: string): boolean { + if (line.length === 0 || line[0] !== '#') { + return false; + } + return line.length === 1 || line[1] === ' '; +} + +function getTomlErrorPosition(err: unknown): { row: number; column: number } | undefined { if (typeof err !== 'object' || err === null) { return undefined; } - const row = (err as { line?: unknown }).line; - return typeof row === 'number' && Number.isInteger(row) && row >= 0 ? row : undefined; + const { line, col } = err as { line?: unknown; col?: unknown }; + if (typeof line !== 'number' || !Number.isInteger(line) || line < 0) { + return undefined; + } + const column = typeof col === 'number' && Number.isInteger(col) && col >= 0 ? col : 0; + return { row: line, column }; +} + +/** `@iarna/toml` reports `col` one past the offending character, and overshoots the line end entirely on end-of-line failures. */ +function tomlErrorSourceRange( + text: string, + reconstructedOrigins: readonly number[], + position: { row: number; column: number } | undefined, + matchStart: number, + toSourceRange: (start: number, end: number) => { start: number; end: number }, +): { start: number; end: number } { + if (position === undefined || position.row >= reconstructedOrigins.length) { + return toSourceRange(matchStart, lineEndOffset(text, matchStart)); + } + const origin = reconstructedOrigins[position.row]; + const lineEnd = lineEndOffset(text, origin); + const offending = origin + Math.max(0, position.column - 1); + const start = offending < lineEnd ? offending : origin; + return toSourceRange(Math.min(start, lineEnd), lineEnd); +} + +function findKeyRange( + reconstructed: readonly string[], + reconstructedOrigins: readonly number[], + key: string, + toSourceRange: (start: number, end: number) => { start: number; end: number }, +): { start: number; end: number } | undefined { + for (const [index, line] of reconstructed.entries()) { + const leadingWhitespace = line.length - line.trimStart().length; + const trimmed = line.slice(leadingWhitespace); + if (!trimmed.startsWith(key)) { + continue; + } + const after = trimmed.slice(key.length).trimStart(); + if (!after.startsWith('=')) { + continue; + } + const origin = reconstructedOrigins[index]; + return toSourceRange(origin + leadingWhitespace, origin + line.length); + } + return undefined; } /** @@ -310,6 +587,14 @@ function sourceOffsetForNormalizedOffset(sourceText: string, normalizedOffset: n return sourceOffset; } +export function sliceHeaderBytes(text: string): string { + const buffer = Buffer.from(text, 'utf-8'); + if (buffer.byteLength <= MAX_HEADER_BYTES) { + return text; + } + return buffer.subarray(0, MAX_HEADER_BYTES).toString('utf-8'); +} + /** * Read PEP 723 metadata from a file. Reads only the first * `MAX_HEADER_BYTES` bytes of the file — PEP 723 blocks live at the @@ -328,7 +613,6 @@ export async function readInlineScriptMetadataFromFile(uri: Uri): Promise=3.11'."); + case 'dependencies': + return l10n.t( + "Inline script metadata: 'dependencies' must be an array of strings, for example ['requests'].", + ); + case 'tool': + return l10n.t("Inline script metadata: 'tool' must be a table."); + default: + return l10n.t("Inline script metadata: '{0}' has the wrong type.", field); + } + } } export namespace Interpreter { diff --git a/src/common/workspace.apis.ts b/src/common/workspace.apis.ts index d8f48d6c..ed7276b9 100644 --- a/src/common/workspace.apis.ts +++ b/src/common/workspace.apis.ts @@ -95,6 +95,14 @@ export function onDidSaveTextDocument( return workspace.onDidSaveTextDocument(listener, thisArgs, disposables); } +export function onDidCloseTextDocument( + listener: (e: TextDocument) => any, + thisArgs?: any, + disposables?: Disposable[], +): Disposable { + return workspace.onDidCloseTextDocument(listener, thisArgs, disposables); +} + export function onDidChangeTextDocument( listener: (e: TextDocumentChangeEvent) => any, thisArgs?: any, diff --git a/src/extension.ts b/src/extension.ts index 5222a005..6b2d5355 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -69,6 +69,7 @@ import { import { PythonEnvironmentManagers } from './features/envManagers'; import { EnvVarManager, PythonEnvVariableManager } from './features/execution/envVariableManager'; import { latchInlineScriptFeatureActivation } from './features/inlineScript/activation'; +import { registerInlineScriptDiagnostics } from './features/inlineScript/diagnostics'; import { InlineScriptLazyDetector } from './features/inlineScript/lazyDetector'; import { registerInlineScriptUx } from './features/inlineScript/setupEnvironment'; import { @@ -229,6 +230,10 @@ export async function activate(context: ExtensionContext): Promise(); diff --git a/src/features/inlineScript/diagnostics.ts b/src/features/inlineScript/diagnostics.ts new file mode 100644 index 00000000..4f22a61f --- /dev/null +++ b/src/features/inlineScript/diagnostics.ts @@ -0,0 +1,223 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { + Diagnostic, + DiagnosticCollection, + DiagnosticSeverity, + Disposable, + languages, + Range, + TextDocument, + Uri, +} from 'vscode'; +import { + InlineScriptMetadataProblem, + parseInlineScriptMetadata, + sliceHeaderBytes, +} from '../../common/inlineScript/metadata'; +import { getInlineScriptRoutingKey } from '../../common/inlineScript/routingRegistry'; +import { InlineScriptStrings } from '../../common/localize'; +import { traceVerbose } from '../../common/logging'; +import { createSimpleDebounce, SimpleDebounce } from '../../common/utils/debounce'; +import { isSameOrParentPath } from '../../common/utils/pathUtils'; +import { + getOpenTextDocuments, + onDidChangeTextDocument, + onDidCloseTextDocument, + onDidDeleteFiles, + onDidOpenTextDocument, + onDidRenameFiles, + onDidSaveTextDocument, +} from '../../common/workspace.apis'; + +const VALIDATION_DEBOUNCE_MS = 300; + +const DIAGNOSTIC_COLLECTION_NAME = 'python-envs-inline-script'; + +/** Publishes diagnostics for malformed PEP 723 inline script metadata, validating the live buffer. */ +export class InlineScriptDiagnosticsPublisher implements Disposable { + private readonly subscriptions: Disposable[] = []; + // `createSimpleDebounce` owns one timer, so a shared instance would let + // edits in one file cancel another's pending validation. + private readonly pending = new Map(); + private readonly published = new Map(); + private disposed = false; + + constructor(private readonly collection: DiagnosticCollection) {} + + /** Documents open at activation are replayed via `setImmediate`, because `onLanguage:python` fires after editors are restored. */ + public activate(): void { + this.subscriptions.push( + onDidOpenTextDocument((doc) => this.validate(doc)), + onDidSaveTextDocument((doc) => this.validate(doc)), + onDidChangeTextDocument((e) => this.scheduleValidation(e.document)), + onDidCloseTextDocument((doc) => this.clear(doc.uri)), + onDidDeleteFiles((e) => e.files.forEach((uri) => this.clearTree(uri))), + onDidRenameFiles((e) => + e.files.forEach((file) => { + this.clearTree(file.oldUri); + this.revalidateIfOpen(file.newUri); + }), + ), + ); + const handle = setImmediate(() => this.replayOpenDocuments()); + this.subscriptions.push(new Disposable(() => clearImmediate(handle))); + } + + public dispose(): void { + this.disposed = true; + this.subscriptions.forEach((s) => s.dispose()); + this.subscriptions.length = 0; + this.pending.forEach((entry) => entry.debounce.dispose()); + this.pending.clear(); + this.published.clear(); + this.collection.dispose(); + } + + private replayOpenDocuments(): void { + if (this.disposed) { + return; + } + const docs = getOpenTextDocuments().filter((doc) => shouldValidateUri(doc.uri)); + traceVerbose(`inlineScriptDiagnostics: activation replay over ${docs.length} candidate .py document(s)`); + for (const doc of docs) { + this.validate(doc); + } + } + + private scheduleValidation(document: TextDocument): void { + if (this.disposed || !shouldValidateUri(document.uri)) { + return; + } + const key = document.uri.toString(); + const existing = this.pending.get(key); + if (existing) { + existing.document = document; + existing.debounce.trigger(); + return; + } + const entry = { + document, + debounce: createSimpleDebounce(VALIDATION_DEBOUNCE_MS, () => { + const queued = this.pending.get(key); + this.pending.delete(key); + if (queued) { + this.validate(queued.document); + } + }), + }; + this.pending.set(key, entry); + entry.debounce.trigger(); + } + + private validate(document: TextDocument): void { + if (this.disposed || !shouldValidateUri(document.uri)) { + return; + } + this.cancelPending(document.uri); + + const uri = document.uri; + const result = parseInlineScriptMetadata(sliceHeaderBytes(document.getText()), uri.fsPath); + const problems = result.kind === 'none' ? [] : result.problems; + if (problems.length === 0) { + this.clear(uri); + return; + } + + const diagnostics = problems.map((problem) => toDiagnostic(document, problem)); + this.collection.set(uri, diagnostics); + this.published.set(uri.toString(), uri); + traceVerbose( + `inlineScriptDiagnostics: published ${diagnostics.length} problem(s) for ${uri.fsPath}: ` + + problems.map((p) => p.code).join(', '), + ); + } + + private revalidateIfOpen(uri: Uri): void { + if (this.disposed || !shouldValidateUri(uri)) { + return; + } + const key = uri.toString(); + const doc = getOpenTextDocuments().find((d) => d.uri.toString() === key); + if (doc) { + this.validate(doc); + } + } + + private clear(uri: Uri): void { + this.cancelPending(uri); + const key = uri.toString(); + if (this.published.delete(key)) { + this.collection.delete(uri); + } + } + + private clearTree(uri: Uri): void { + this.clear(uri); + if (uri.scheme !== 'file') { + return; + } + for (const published of Array.from(this.published.values())) { + if (published.scheme === 'file' && isSameOrParentPath(uri.fsPath, published.fsPath)) { + this.clear(published); + } + } + } + + private cancelPending(uri: Uri): void { + const key = uri.toString(); + const entry = this.pending.get(key); + if (entry) { + entry.debounce.dispose(); + this.pending.delete(key); + } + } +} + +/** Whether inline script metadata is meaningful for `uri` — a local `.py` file. */ +export function shouldValidateUri(uri: Uri): boolean { + return getInlineScriptRoutingKey(uri) !== undefined; +} + +function toDiagnostic(document: TextDocument, problem: InlineScriptMetadataProblem): Diagnostic { + const range = new Range( + document.positionAt(problem.sourceRange.start), + document.positionAt(problem.sourceRange.end), + ); + const diagnostic = new Diagnostic(range, messageFor(problem), severityFor(problem)); + diagnostic.source = InlineScriptStrings.diagnosticSource; + diagnostic.code = problem.code; + return diagnostic; +} + +function messageFor(problem: InlineScriptMetadataProblem): string { + switch (problem.code) { + case 'unterminated-block': + return InlineScriptStrings.unterminatedBlock; + case 'multiple-blocks': + return InlineScriptStrings.multipleBlocks; + case 'invalid-content-line': + return InlineScriptStrings.invalidContentLine(problem.detail ?? ''); + case 'invalid-block-marker': + return InlineScriptStrings.invalidBlockMarker(problem.detail ?? ''); + case 'invalid-toml': + return InlineScriptStrings.invalidToml(problem.detail ?? ''); + case 'invalid-field-type': + return InlineScriptStrings.invalidFieldType(problem.detail ?? ''); + default: + return InlineScriptStrings.unterminatedBlock; + } +} + +function severityFor(problem: InlineScriptMetadataProblem): DiagnosticSeverity { + return problem.severity === 'warning' ? DiagnosticSeverity.Warning : DiagnosticSeverity.Error; +} + +export function registerInlineScriptDiagnostics(): Disposable { + const publisher = new InlineScriptDiagnosticsPublisher( + languages.createDiagnosticCollection(DIAGNOSTIC_COLLECTION_NAME), + ); + publisher.activate(); + return publisher; +} diff --git a/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts b/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts new file mode 100644 index 00000000..762b7387 --- /dev/null +++ b/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts @@ -0,0 +1,349 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { + InlineScriptMetadataParseResult, + InlineScriptMetadataProblem, + InlineScriptMetadataProblemCode, + MAX_HEADER_BYTES, + parseInlineScriptMetadata, + readInlineScriptMetadata, + sliceHeaderBytes, +} from '../../../common/inlineScript/metadata'; +import * as logging from '../../../common/logging'; + +const VARIANTS: ReadonlyArray<{ name: string; build: (lines: string[]) => string }> = [ + { name: 'LF', build: (lines) => lines.join('\n') }, + { name: 'CRLF', build: (lines) => lines.join('\r\n') }, + { name: 'BOM + LF', build: (lines) => `\uFEFF${lines.join('\n')}` }, + { name: 'BOM + CRLF', build: (lines) => `\uFEFF${lines.join('\r\n')}` }, +]; + +function underlined(source: string, problem: InlineScriptMetadataProblem): string { + return source.slice(problem.sourceRange.start, problem.sourceRange.end); +} + +function problems(result: InlineScriptMetadataParseResult): readonly InlineScriptMetadataProblem[] { + return result.kind === 'none' ? [] : result.problems; +} + +function onlyProblem(result: InlineScriptMetadataParseResult): InlineScriptMetadataProblem { + const found = problems(result); + assert.strictEqual(found.length, 1, `expected exactly one problem, got: ${found.map((p) => p.code).join(', ')}`); + return found[0]; +} + +function assertProblemAcrossVariants( + lines: string[], + code: InlineScriptMetadataProblemCode, + expectedText: string, +): void { + for (const variant of VARIANTS) { + const source = variant.build(lines); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'invalid', `[${variant.name}] expected an invalid result`); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, code, `[${variant.name}] wrong problem code`); + assert.strictEqual(underlined(source, problem), expectedText, `[${variant.name}] wrong underlined text`); + assert.strictEqual( + readInlineScriptMetadata(source), + undefined, + `[${variant.name}] wrapper should be undefined`, + ); + } +} + +suite('inlineScriptMetadata diagnostics', () => { + setup(() => { + sinon.stub(logging, 'traceWarn'); + sinon.stub(logging, 'traceVerbose'); + }); + + teardown(() => { + sinon.restore(); + }); + + suite('malformed vs. absent', () => { + test('empty input reports no block', () => { + assert.strictEqual(parseInlineScriptMetadata('').kind, 'none'); + }); + + test('ordinary Python reports no block', () => { + const text = ['#!/usr/bin/env python3', 'import sys', 'print("hello")'].join('\n'); + assert.strictEqual(parseInlineScriptMetadata(text).kind, 'none'); + }); + + test('a comment that merely mentions the marker is not a block', () => { + const text = ['# see the `# /// script` docs', 'print("hi")'].join('\n'); + assert.strictEqual(parseInlineScriptMetadata(text).kind, 'none'); + }); + + test('a non-script block type is left alone', () => { + const text = ['# /// pyproject', '# x = 1'].join('\n'); + assert.strictEqual(parseInlineScriptMetadata(text).kind, 'none'); + }); + + test('a well-formed block is valid with no problems', () => { + const text = ['# /// script', '# dependencies = ["requests"]', '# ///', 'print("hi")'].join('\n'); + const result = parseInlineScriptMetadata(text); + assert.strictEqual(result.kind, 'valid'); + assert.deepStrictEqual(problems(result), []); + }); + + test('a missing closing marker is reported rather than silently ignored', () => { + const text = ['# /// script', '# dependencies = ["requests"]', 'print("hi")'].join('\n'); + const result = parseInlineScriptMetadata(text); + assert.strictEqual(result.kind, 'invalid'); + assert.strictEqual(onlyProblem(result).code, 'unterminated-block'); + }); + }); + + suite('problem ranges across BOM and CRLF', () => { + test('unterminated block underlines the opening marker', () => { + assertProblemAcrossVariants( + ['# /// script', '# dependencies = ["requests"]', 'print("hi")'], + 'unterminated-block', + '# /// script', + ); + }); + + test('unterminated block after leading content still underlines its own marker', () => { + assertProblemAcrossVariants( + ['#!/usr/bin/env python3', '# a comment', '', '# /// script', '# dependencies = []', 'print("hi")'], + 'unterminated-block', + '# /// script', + ); + }); + + test('unterminated block at end of file with no trailing newline', () => { + assertProblemAcrossVariants(['# /// script', '# x = 1'], 'unterminated-block', '# /// script'); + }); + + test('an empty block is valid per the prose spec, not a problem', () => { + for (const variant of VARIANTS) { + const source = variant.build(['# /// script', '# ///', 'print("hi")']); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'valid', `[${variant.name}] expected a valid result`); + assert.deepStrictEqual(problems(result), [], `[${variant.name}] expected no problems`); + } + }); + + test('opening marker with trailing whitespace underlines the whole marker', () => { + assertProblemAcrossVariants(['# /// script ', '# x = 1', '# ///'], 'invalid-block-marker', '# /// script '); + }); + + test('closing marker with trailing whitespace underlines the closer', () => { + assertProblemAcrossVariants( + ['# /// script', '# x = 1', '# /// ', 'print("hi")'], + 'invalid-block-marker', + '# /// ', + ); + }); + + test('invalid content line underlines that line', () => { + assertProblemAcrossVariants( + ['# /// script', '# x = 1', '#no-space-after-hash', '# ///'], + 'invalid-content-line', + '#no-space-after-hash', + ); + }); + + test('multiple blocks underline the redundant marker, not the first', () => { + for (const variant of VARIANTS) { + const source = variant.build([ + '# /// script', + '# dependencies = ["a"]', + '# ///', + 'print("hi")', + '# /// script', + '# dependencies = ["b"]', + '# ///', + ]); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'invalid', `[${variant.name}] expected invalid`); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'multiple-blocks'); + assert.strictEqual(underlined(source, problem), '# /// script'); + assert.ok( + problem.sourceRange.start > source.indexOf('print'), + `[${variant.name}] should anchor on the SECOND block`, + ); + } + }); + }); + + suite('TOML errors', () => { + test('column information underlines the offending text, not the whole block', () => { + for (const variant of VARIANTS) { + const source = variant.build(['# /// script', '# requires-python = >=3.11', '# ///']); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'invalid', `[${variant.name}] expected invalid`); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'invalid-toml'); + assert.strictEqual(underlined(source, problem), '>=3.11', `[${variant.name}] wrong underlined text`); + } + }); + + test('an end-of-line failure underlines the whole line rather than collapsing to nothing', () => { + for (const variant of VARIANTS) { + const source = variant.build(['# /// script', '# dependencies = ["requests', '# ///']); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'invalid', `[${variant.name}] expected invalid`); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'invalid-toml'); + assert.strictEqual( + underlined(source, problem), + 'dependencies = ["requests', + `[${variant.name}] wrong underlined text`, + ); + } + }); + + test('error on a later line maps to that line', () => { + for (const variant of VARIANTS) { + const source = variant.build([ + '# /// script', + '# requires-python = ">=3.11"', + '# dependencies = ["ok"]', + '# broken = ', + '# ///', + ]); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'invalid', `[${variant.name}] expected invalid`); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'invalid-toml'); + const lineStart = source.indexOf('# broken = '); + assert.ok( + problem.sourceRange.start >= lineStart && + problem.sourceRange.start <= lineStart + '# broken = '.length, + `[${variant.name}] expected range on the broken line, got ${problem.sourceRange.start} vs ${lineStart}`, + ); + } + }); + + test('detail carries the raw parser message without coordinates', () => { + const source = ['# /// script', '# dependencies = ["requests', '# ///'].join('\n'); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.ok(problem.detail && problem.detail.length > 0, 'expected a detail message'); + assert.ok( + !/row \d+, col \d+/.test(problem.detail!), + `detail should not leak payload coordinates: ${problem.detail}`, + ); + }); + }); + + suite('field types', () => { + test('requires-python of the wrong type underlines its key line', () => { + assertProblemAcrossVariants( + ['# /// script', '# requires-python = 3.11', '# ///'], + 'invalid-field-type', + '# requires-python = 3.11'.slice(2), + ); + }); + + test('dependencies of the wrong type underlines its key line', () => { + assertProblemAcrossVariants( + ['# /// script', '# dependencies = "requests"', '# ///'], + 'invalid-field-type', + 'dependencies = "requests"', + ); + }); + + test('a mixed-type dependencies array is rejected by the TOML parser itself', () => { + const source = ['# /// script', '# dependencies = ["ok", 3]', '# ///'].join('\n'); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.strictEqual(problem.code, 'invalid-toml'); + }); + + test('tool of the wrong type is reported', () => { + const source = ['# /// script', '# tool = "uv"', '# ///'].join('\n'); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.strictEqual(problem.code, 'invalid-field-type'); + assert.strictEqual(problem.detail, 'tool'); + }); + + test('a similarly-named key does not steal the range', () => { + const source = ['# /// script', '# dependencies-extra = "x"', '# dependencies = "requests"', '# ///'].join( + '\n', + ); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.strictEqual(underlined(source, problem), 'dependencies = "requests"'); + }); + }); + + suite('valid metadata alongside a broken block', () => { + test('a stray opener after a valid block is reported without withholding metadata', () => { + const source = [ + '# /// script', + '# dependencies = ["requests"]', + '# ///', + '', + '# /// script', + '# dependencies = ["oops"]', + ].join('\n'); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'valid'); + assert.deepStrictEqual(result.kind === 'valid' ? result.metadata.dependencies : undefined, ['requests']); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'unterminated-block'); + assert.strictEqual(problem.severity, 'warning'); + assert.ok(readInlineScriptMetadata(source), 'wrapper should still return the valid metadata'); + }); + + test('an unclosed block below real code is ignored, as the spec requires', () => { + const source = ['"""', '# /// script', '# dependencies = ["docs"]', '"""', 'print("hi")'].join('\n'); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'none'); + assert.deepStrictEqual(problems(result), []); + }); + + test('a marker quoted inside a valid block is not a second block', () => { + const source = ['# /// script', '# # /// script', '# dependencies = []', '# ///'].join('\n'); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'valid'); + assert.deepStrictEqual(problems(result), []); + }); + }); + + suite('severity', () => { + test('spec violations are errors and mid-edit states are warnings', () => { + const cases: ReadonlyArray<[string[], InlineScriptMetadataProblemCode, 'error' | 'warning']> = [ + [['# /// script', '# x = 1'], 'unterminated-block', 'warning'], + [['# /// script', '#bad', '# ///'], 'invalid-content-line', 'error'], + [['# /// script ', '# x = 1', '# ///'], 'invalid-block-marker', 'error'], + [['# /// script', '# x = ', '# ///'], 'invalid-toml', 'error'], + [['# /// script', '# tool = "uv"', '# ///'], 'invalid-field-type', 'error'], + ]; + for (const [lines, code, severity] of cases) { + const problem = onlyProblem(parseInlineScriptMetadata(lines.join('\n'))); + assert.strictEqual(problem.code, code, `wrong code for ${JSON.stringify(lines)}`); + assert.strictEqual(problem.severity, severity, `wrong severity for ${code}`); + } + }); + }); + + suite('sliceHeaderBytes', () => { + test('short text is returned unchanged', () => { + const text = '# /// script\n# ///\n'; + assert.strictEqual(sliceHeaderBytes(text), text); + }); + + test('text is clipped to the same byte budget the file reader uses', () => { + const text = 'x'.repeat(MAX_HEADER_BYTES + 100); + assert.strictEqual(Buffer.byteLength(sliceHeaderBytes(text), 'utf-8'), MAX_HEADER_BYTES); + }); + + test('clipping is by bytes, so multi-byte characters count for more than one', () => { + const text = 'é'.repeat(MAX_HEADER_BYTES); + assert.strictEqual(sliceHeaderBytes(text).length, MAX_HEADER_BYTES / 2); + }); + + test('a block beyond the budget is invisible, matching the on-disk reader', () => { + const padding = `${'# padding\n'.repeat(Math.ceil(MAX_HEADER_BYTES / 10))}`; + const text = `${padding}# /// script\n# dependencies = []\n# ///\n`; + assert.strictEqual(parseInlineScriptMetadata(sliceHeaderBytes(text)).kind, 'none'); + }); + }); +}); diff --git a/src/test/features/inlineScript/diagnostics.unit.test.ts b/src/test/features/inlineScript/diagnostics.unit.test.ts new file mode 100644 index 00000000..e657ab63 --- /dev/null +++ b/src/test/features/inlineScript/diagnostics.unit.test.ts @@ -0,0 +1,318 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { + Diagnostic, + DiagnosticCollection, + DiagnosticSeverity, + Disposable, + Position, + TextDocument, + TextDocumentChangeEvent, + Uri, +} from 'vscode'; +import * as logging from '../../../common/logging'; +import * as wapi from '../../../common/workspace.apis'; +import { InlineScriptDiagnosticsPublisher, shouldValidateUri } from '../../../features/inlineScript/diagnostics'; + +const DEBOUNCE_MS = 300; + +const BROKEN_SCRIPT = ['# /// script', '# dependencies = ["requests"]', 'print("hi")'].join('\n'); +const VALID_SCRIPT = ['# /// script', '# dependencies = ["requests"]', '# ///', 'print("hi")'].join('\n'); +const PLAIN_SCRIPT = 'print("hi")\n'; + +function makeDoc(uri: Uri, text: string): TextDocument { + return { + uri, + getText: () => text, + positionAt: (offset: number) => new Position(0, offset), + } as unknown as TextDocument; +} + +function makeCollection() { + const entries = new Map(); + let disposed = false; + const collection = { + set: (uri: Uri, diagnostics: readonly Diagnostic[]) => entries.set(uri.toString(), diagnostics), + delete: (uri: Uri) => entries.delete(uri.toString()), + clear: () => entries.clear(), + dispose: () => { + disposed = true; + entries.clear(); + }, + } as unknown as DiagnosticCollection; + return { + collection, + entries, + get disposed() { + return disposed; + }, + for(uri: Uri): readonly Diagnostic[] | undefined { + return entries.get(uri.toString()); + }, + }; +} + +suite('InlineScriptDiagnosticsPublisher', () => { + const scriptUri = Uri.file('/workspace/app.py'); + const otherUri = Uri.file('/workspace/other.py'); + + let clock: sinon.SinonFakeTimers; + let sink: ReturnType; + let publisher: InlineScriptDiagnosticsPublisher; + let openDocs: TextDocument[]; + + let openListener: ((doc: TextDocument) => unknown) | undefined; + let saveListener: ((doc: TextDocument) => unknown) | undefined; + let changeListener: ((e: TextDocumentChangeEvent) => unknown) | undefined; + let closeListener: ((doc: TextDocument) => unknown) | undefined; + let deleteListener: ((e: { files: readonly Uri[] }) => unknown) | undefined; + let renameListener: ((e: { files: readonly { oldUri: Uri; newUri: Uri }[] }) => unknown) | undefined; + + function captureListener(assign: (listener: T) => void) { + return (listener: T) => { + assign(listener); + return new Disposable(() => undefined); + }; + } + + setup(() => { + clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + sinon.stub(logging, 'traceWarn'); + sinon.stub(logging, 'traceVerbose'); + + openListener = undefined; + saveListener = undefined; + changeListener = undefined; + closeListener = undefined; + deleteListener = undefined; + renameListener = undefined; + openDocs = []; + + sinon.stub(wapi, 'onDidOpenTextDocument').callsFake(captureListener((l) => (openListener = l))); + sinon.stub(wapi, 'onDidSaveTextDocument').callsFake(captureListener((l) => (saveListener = l))); + sinon.stub(wapi, 'onDidChangeTextDocument').callsFake(captureListener((l) => (changeListener = l))); + sinon.stub(wapi, 'onDidCloseTextDocument').callsFake(captureListener((l) => (closeListener = l))); + sinon.stub(wapi, 'onDidDeleteFiles').callsFake(captureListener((l) => (deleteListener = l))); + sinon.stub(wapi, 'onDidRenameFiles').callsFake(captureListener((l) => (renameListener = l))); + sinon.stub(wapi, 'getOpenTextDocuments').callsFake(() => openDocs); + + sink = makeCollection(); + publisher = new InlineScriptDiagnosticsPublisher(sink.collection); + publisher.activate(); + }); + + teardown(() => { + publisher.dispose(); + clock.restore(); + sinon.restore(); + }); + + function change(doc: TextDocument): void { + changeListener?.({ document: doc, contentChanges: [], reason: undefined } as TextDocumentChangeEvent); + } + + suite('publishing', () => { + test('a malformed block produces a diagnostic on open', () => { + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + + const published = sink.for(scriptUri); + assert.ok(published, 'expected diagnostics to be published'); + assert.strictEqual(published!.length, 1); + assert.strictEqual(published![0].code, 'unterminated-block'); + assert.strictEqual(published![0].severity, DiagnosticSeverity.Warning); + assert.ok(published![0].message.length > 0, 'expected a localized message'); + }); + + test('spec violations are published as errors', () => { + openListener?.(makeDoc(scriptUri, ['# /// script', '#bad', '# ///'].join('\n'))); + + const published = sink.for(scriptUri); + assert.strictEqual(published![0].severity, DiagnosticSeverity.Error); + assert.strictEqual(published![0].code, 'invalid-content-line'); + }); + + test('a well-formed block publishes nothing', () => { + openListener?.(makeDoc(scriptUri, VALID_SCRIPT)); + assert.strictEqual(sink.for(scriptUri), undefined); + }); + + test('a file with no block publishes nothing', () => { + openListener?.(makeDoc(scriptUri, PLAIN_SCRIPT)); + assert.strictEqual(sink.for(scriptUri), undefined); + }); + + test('fixing the block removes the diagnostic', () => { + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + assert.ok(sink.for(scriptUri), 'expected an initial diagnostic'); + + saveListener?.(makeDoc(scriptUri, VALID_SCRIPT)); + assert.strictEqual(sink.for(scriptUri), undefined, 'diagnostic should be cleared once valid'); + }); + + test('non-Python and non-file documents are ignored', () => { + const notPython = Uri.file('/workspace/notes.txt'); + const untitled = Uri.parse('untitled:Untitled-1'); + + openListener?.(makeDoc(notPython, BROKEN_SCRIPT)); + openListener?.(makeDoc(untitled, BROKEN_SCRIPT)); + + assert.strictEqual(sink.entries.size, 0); + assert.strictEqual(shouldValidateUri(notPython), false); + assert.strictEqual(shouldValidateUri(untitled), false); + }); + + test('validation reads the live buffer, not the file on disk', () => { + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + assert.ok(sink.for(scriptUri)); + }); + }); + + suite('debouncing changes', () => { + test('nothing is published until the debounce elapses', () => { + change(makeDoc(scriptUri, BROKEN_SCRIPT)); + clock.tick(DEBOUNCE_MS - 1); + assert.strictEqual(sink.for(scriptUri), undefined, 'should not publish mid-edit'); + + clock.tick(1); + assert.ok(sink.for(scriptUri), 'should publish once typing settles'); + }); + + test('rapid edits only validate once, against the final text', () => { + change(makeDoc(scriptUri, '# /// script')); + clock.tick(100); + change(makeDoc(scriptUri, '# /// script\n# dependencies = []')); + clock.tick(100); + change(makeDoc(scriptUri, VALID_SCRIPT)); + clock.tick(100); + assert.strictEqual(sink.for(scriptUri), undefined, 'no intermediate squiggle should appear'); + + clock.tick(DEBOUNCE_MS); + assert.strictEqual(sink.for(scriptUri), undefined, 'final text is valid, so nothing is published'); + }); + + test('each document debounces independently', () => { + change(makeDoc(scriptUri, BROKEN_SCRIPT)); + clock.tick(DEBOUNCE_MS - 50); + change(makeDoc(otherUri, BROKEN_SCRIPT)); + clock.tick(50); + + assert.ok(sink.for(scriptUri), 'first document should have been validated on schedule'); + assert.strictEqual(sink.for(otherUri), undefined, 'second document is still within its own debounce'); + + clock.tick(DEBOUNCE_MS); + assert.ok(sink.for(otherUri)); + }); + + test('a save supersedes a queued change rather than being overwritten by it', () => { + change(makeDoc(scriptUri, BROKEN_SCRIPT)); + saveListener?.(makeDoc(scriptUri, VALID_SCRIPT)); + assert.strictEqual(sink.for(scriptUri), undefined); + + clock.tick(DEBOUNCE_MS * 2); + assert.strictEqual(sink.for(scriptUri), undefined, 'stale queued validation must not resurrect a squiggle'); + }); + }); + + suite('clearing', () => { + test('closing a document clears its diagnostics', () => { + const doc = makeDoc(scriptUri, BROKEN_SCRIPT); + openListener?.(doc); + assert.ok(sink.for(scriptUri)); + + closeListener?.(doc); + assert.strictEqual(sink.for(scriptUri), undefined); + }); + + test('closing cancels any queued validation', () => { + const doc = makeDoc(scriptUri, BROKEN_SCRIPT); + change(doc); + closeListener?.(doc); + + clock.tick(DEBOUNCE_MS * 2); + assert.strictEqual(sink.for(scriptUri), undefined, 'a closed document must not gain a squiggle'); + }); + + test('deleting a file clears its diagnostics', () => { + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + deleteListener?.({ files: [scriptUri] }); + assert.strictEqual(sink.for(scriptUri), undefined); + }); + + test('deleting a folder clears diagnostics for files inside it', () => { + const nested = Uri.file('/workspace/pkg/app.py'); + openListener?.(makeDoc(nested, BROKEN_SCRIPT)); + assert.ok(sink.for(nested)); + + deleteListener?.({ files: [Uri.file('/workspace/pkg')] }); + assert.strictEqual(sink.for(nested), undefined); + }); + + test('renaming clears the old path and re-validates the new one', () => { + const renamed = Uri.file('/workspace/renamed.py'); + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + assert.ok(sink.for(scriptUri)); + + openDocs = [makeDoc(renamed, BROKEN_SCRIPT)]; + renameListener?.({ files: [{ oldUri: scriptUri, newUri: renamed }] }); + + assert.strictEqual(sink.for(scriptUri), undefined, 'old path must not keep a squiggle'); + assert.ok(sink.for(renamed), 'new path should be validated'); + }); + + test('renaming a file that is not open leaves nothing behind', () => { + const renamed = Uri.file('/workspace/renamed.py'); + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + + openDocs = []; + renameListener?.({ files: [{ oldUri: scriptUri, newUri: renamed }] }); + + assert.strictEqual(sink.entries.size, 0); + }); + + test('disposing tears down the collection and stops queued work', () => { + change(makeDoc(scriptUri, BROKEN_SCRIPT)); + publisher.dispose(); + + clock.tick(DEBOUNCE_MS * 2); + assert.strictEqual(sink.disposed, true, 'collection should be disposed'); + assert.strictEqual(sink.entries.size, 0, 'no diagnostics should survive disposal'); + }); + + test('events arriving after disposal are ignored', () => { + publisher.dispose(); + openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT)); + assert.strictEqual(sink.entries.size, 0); + }); + }); + + suite('activation replay', () => { + test('documents already open at activation are validated', async () => { + publisher.dispose(); + sink = makeCollection(); + openDocs = [makeDoc(scriptUri, BROKEN_SCRIPT), makeDoc(otherUri, VALID_SCRIPT)]; + + publisher = new InlineScriptDiagnosticsPublisher(sink.collection); + publisher.activate(); + await new Promise((resolve) => setImmediate(resolve)); + + assert.ok(sink.for(scriptUri), 'malformed open document should be validated'); + assert.strictEqual(sink.for(otherUri), undefined, 'valid open document should stay clean'); + }); + + test('the replay is cancelled if the publisher is disposed first', async () => { + publisher.dispose(); + sink = makeCollection(); + openDocs = [makeDoc(scriptUri, BROKEN_SCRIPT)]; + + publisher = new InlineScriptDiagnosticsPublisher(sink.collection); + publisher.activate(); + publisher.dispose(); + await new Promise((resolve) => setImmediate(resolve)); + + assert.strictEqual(sink.entries.size, 0); + }); + }); +}); From f5027d866f78eb9812f39b0d0266cbc5b64546e2 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 14 Sep 2026 16:54:11 -0700 Subject: [PATCH 2/2] fix: address review feedback on inline script metadata diagnostics - Report a bad content line instead of a bogus "missing closing marker" when the closing `# ///` is present. A blank line between fields is the common trigger and now gets a message that names the real problem. Blocks below the leading comment region stay silent as before, so documentation examples are unaffected. - Cancel debounced validations for descendants of a deleted or renamed folder. Previously only published entries were swept, so a file edited within the debounce window could gain a squiggle after it was gone. - Rename the parse discriminant `valid` to `parsed`: it reports that metadata is usable, not that the input was problem-free, and it can carry error-severity problems from a second malformed block. - Assert the exact TOML detail and the exact localized diagnostic messages so a code-to-message mis-mapping cannot pass unnoticed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/common/inlineScript/metadata.ts | 91 ++++++++++++++----- src/common/localize.ts | 8 +- src/features/inlineScript/diagnostics.ts | 12 ++- .../metadataDiagnostics.unit.test.ts | 79 ++++++++++++++-- .../inlineScript/diagnostics.unit.test.ts | 43 ++++++++- 5 files changed, 200 insertions(+), 33 deletions(-) diff --git a/src/common/inlineScript/metadata.ts b/src/common/inlineScript/metadata.ts index 01fac5a6..7dad32fa 100644 --- a/src/common/inlineScript/metadata.ts +++ b/src/common/inlineScript/metadata.ts @@ -55,9 +55,15 @@ export interface InlineScriptMetadataProblem { readonly detail?: string; } +/** + * The discriminant reports whether usable metadata was produced, NOT whether the + * input was problem-free: `parsed` still carries `problems`, and those may include + * `error` severities raised by a second, malformed block elsewhere in the file. + * Consumers that care about correctness must inspect `problems` in every case. + */ export type InlineScriptMetadataParseResult = | { - readonly kind: 'valid'; + readonly kind: 'parsed'; readonly metadata: InlineScriptMetadata; readonly problems: readonly InlineScriptMetadataProblem[]; } @@ -109,7 +115,7 @@ const CLOSER_LINE = '# ///'; */ export function readInlineScriptMetadata(scriptText: string, source?: string): InlineScriptMetadata | undefined { const result = parseInlineScriptMetadata(scriptText, source); - return result.kind === 'valid' ? result.metadata : undefined; + return result.kind === 'parsed' ? result.metadata : undefined; } const NO_METADATA: InlineScriptMetadataParseResult = { kind: 'none' }; @@ -164,10 +170,10 @@ export function parseInlineScriptMetadata(scriptText: string, source?: string): if (matchedRanges.some((r) => opener.offset >= r.start && opener.offset < r.end)) { continue; } - const problem = diagnoseMalformedBlock(text, opener, toSourceRange, where); - // Unclosed blocks are ignored per spec; only flag one in the leading - // comment region, where it is a header being typed rather than an example. - if (problem.code === 'unterminated-block' && opener.offset >= headerEnd) { + const { problem, ignorableBelowHeader } = diagnoseMalformedBlock(text, opener, toSourceRange, where); + // Blocks the spec tells us to ignore are only worth flagging in the leading comment + // region, where they are a header being typed rather than a documentation example. + if (ignorableBelowHeader && opener.offset >= headerEnd) { continue; } problems.push(problem); @@ -363,7 +369,7 @@ export function parseInlineScriptMetadata(scriptText: string, source?: string): } return { - kind: 'valid', + kind: 'parsed', problems, metadata: { requiresPython, @@ -430,12 +436,18 @@ function findScriptOpeners(text: string): ScriptOpener[] { return openers; } +interface MalformedBlockDiagnosis { + readonly problem: InlineScriptMetadataProblem; + /** True when the spec lets us ignore the block entirely, so it need not be flagged outside the header region. */ + readonly ignorableBelowHeader: boolean; +} + function diagnoseMalformedBlock( text: string, opener: ScriptOpener, toSourceRange: (start: number, end: number) => { start: number; end: number }, where: string, -): InlineScriptMetadataProblem { +): MalformedBlockDiagnosis { const openerRange = toSourceRange(opener.offset, opener.lineEnd); if (opener.trailing.length > 0) { @@ -443,10 +455,13 @@ function diagnoseMalformedBlock( `inline script metadata${where}: the \`# /// script\` marker on line ${countLines(text, opener.offset)} has trailing whitespace`, ); return { - code: 'invalid-block-marker', - severity: 'error', - sourceRange: openerRange, - detail: `${OPENER_PREFIX}script${opener.trailing}`, + ignorableBelowHeader: false, + problem: { + code: 'invalid-block-marker', + severity: 'error', + sourceRange: openerRange, + detail: `${OPENER_PREFIX}script${opener.trailing}`, + }, }; } @@ -464,24 +479,36 @@ function diagnoseMalformedBlock( `inline script metadata${where}: the closing \`# ///\` marker on line ${countLines(text, offset)} has trailing whitespace`, ); return { - code: 'invalid-block-marker', - severity: 'error', - sourceRange: toSourceRange(offset, lineEnd), - detail: line, + ignorableBelowHeader: false, + problem: { + code: 'invalid-block-marker', + severity: 'error', + sourceRange: toSourceRange(offset, lineEnd), + detail: line, + }, }; } if (!isValidContentLine(line)) { - if (line.startsWith('#')) { + // A non-comment line usually just means the block ended unclosed, but when the + // closing marker is still ahead the author wrote a real block around a bad line. + const isComment = line.startsWith('#'); + if (isComment || hasCloserAhead(text, lineEnd)) { traceWarn( `inline script metadata${where}: invalid content line ${countLines(text, offset)} ` + `(expected '#' or '# '): ${JSON.stringify(line)}`, ); return { - code: 'invalid-content-line', - severity: 'error', - sourceRange: toSourceRange(offset, lineEnd), - detail: line, + ignorableBelowHeader: !isComment, + problem: { + code: 'invalid-content-line', + severity: 'error', + sourceRange: toSourceRange( + offset, + line.length > 0 ? lineEnd : Math.min(lineEnd + 1, text.length), + ), + detail: line, + }, }; } break; @@ -496,7 +523,10 @@ function diagnoseMalformedBlock( traceWarn( `inline script metadata${where}: the \`# /// script\` block on line ${countLines(text, opener.offset)} is missing its closing \`# ///\` marker`, ); - return { code: 'unterminated-block', severity: 'warning', sourceRange: openerRange }; + return { + ignorableBelowHeader: true, + problem: { code: 'unterminated-block', severity: 'warning', sourceRange: openerRange }, + }; } function isValidContentLine(line: string): boolean { @@ -506,6 +536,23 @@ function isValidContentLine(line: string): boolean { return line.length === 1 || line[1] === ' '; } +/** Whether a closing `# ///` marker follows `from`, stopping at the next block opener. */ +function hasCloserAhead(text: string, from: number): boolean { + let offset = from + 1; + while (offset <= text.length) { + const lineEnd = lineEndOffset(text, offset); + const line = text.slice(offset, lineEnd); + if (line === CLOSER_LINE) { + return true; + } + if (line.startsWith(OPENER_PREFIX) || lineEnd >= text.length) { + return false; + } + offset = lineEnd + 1; + } + return false; +} + function getTomlErrorPosition(err: unknown): { row: number; column: number } | undefined { if (typeof err !== 'object' || err === null) { return undefined; diff --git a/src/common/localize.ts b/src/common/localize.ts index d133cb44..e6f8db55 100644 --- a/src/common/localize.ts +++ b/src/common/localize.ts @@ -45,9 +45,15 @@ export namespace InlineScriptStrings { "A script may contain only one '# /// script' block. Remove this block or merge it into the first one.", ); export function invalidContentLine(line: string): string { + const found = line.trim(); + if (found.length === 0) { + return l10n.t( + "Lines inside a '# /// script' block must be exactly '#' or start with '# '. This line is blank; use '#' for a blank metadata line.", + ); + } return l10n.t( "Lines inside a '# /// script' block must be exactly '#' or start with '# '. Found: {0}", - line.trim(), + found, ); } export function invalidBlockMarker(marker: string): string { diff --git a/src/features/inlineScript/diagnostics.ts b/src/features/inlineScript/diagnostics.ts index 4f22a61f..43bf9127 100644 --- a/src/features/inlineScript/diagnostics.ts +++ b/src/features/inlineScript/diagnostics.ts @@ -158,9 +158,15 @@ export class InlineScriptDiagnosticsPublisher implements Disposable { if (uri.scheme !== 'file') { return; } - for (const published of Array.from(this.published.values())) { - if (published.scheme === 'file' && isSameOrParentPath(uri.fsPath, published.fsPath)) { - this.clear(published); + // Pending entries have never published, so `published` alone would leave a + // debounced validation to fire for a file that no longer exists. + const tracked = [ + ...this.published.values(), + ...Array.from(this.pending.values(), (entry) => entry.document.uri), + ]; + for (const candidate of tracked) { + if (candidate.scheme === 'file' && isSameOrParentPath(uri.fsPath, candidate.fsPath)) { + this.clear(candidate); } } } diff --git a/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts b/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts index 762b7387..1fc687a1 100644 --- a/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts +++ b/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts @@ -88,7 +88,7 @@ suite('inlineScriptMetadata diagnostics', () => { test('a well-formed block is valid with no problems', () => { const text = ['# /// script', '# dependencies = ["requests"]', '# ///', 'print("hi")'].join('\n'); const result = parseInlineScriptMetadata(text); - assert.strictEqual(result.kind, 'valid'); + assert.strictEqual(result.kind, 'parsed'); assert.deepStrictEqual(problems(result), []); }); @@ -125,7 +125,7 @@ suite('inlineScriptMetadata diagnostics', () => { for (const variant of VARIANTS) { const source = variant.build(['# /// script', '# ///', 'print("hi")']); const result = parseInlineScriptMetadata(source); - assert.strictEqual(result.kind, 'valid', `[${variant.name}] expected a valid result`); + assert.strictEqual(result.kind, 'parsed', `[${variant.name}] expected a valid result`); assert.deepStrictEqual(problems(result), [], `[${variant.name}] expected no problems`); } }); @@ -226,12 +226,60 @@ suite('inlineScriptMetadata diagnostics', () => { test('detail carries the raw parser message without coordinates', () => { const source = ['# /// script', '# dependencies = ["requests', '# ///'].join('\n'); const problem = onlyProblem(parseInlineScriptMetadata(source)); - assert.ok(problem.detail && problem.detail.length > 0, 'expected a detail message'); + assert.strictEqual(problem.detail, 'Unterminated string'); assert.ok( !/row \d+, col \d+/.test(problem.detail!), `detail should not leak payload coordinates: ${problem.detail}`, ); }); + + test('a non-comment line before an existing closer is bad content, not a missing marker', () => { + assertProblemAcrossVariants( + ['# /// script', '# requires-python = ">=3.11"', 'not_a_comment = 1', '# ///'], + 'invalid-content-line', + 'not_a_comment = 1', + ); + }); + + test('a blank line before an existing closer is bad content, not a missing marker', () => { + for (const variant of VARIANTS) { + const source = variant.build([ + '# /// script', + '# requires-python = ">=3.11"', + '', + '# dependencies = []', + '# ///', + ]); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.strictEqual(problem.code, 'invalid-content-line', `[${variant.name}] wrong problem code`); + assert.strictEqual(problem.severity, 'error', `[${variant.name}] wrong severity`); + assert.strictEqual(problem.detail, '', `[${variant.name}] expected the blank line as detail`); + } + }); + + test('a non-comment line with no closer ahead stays a missing-marker warning', () => { + assertProblemAcrossVariants( + ['# /// script', '# requires-python = ">=3.11"', 'not_a_comment = 1'], + 'unterminated-block', + '# /// script', + ); + }); + + test('a closer belonging to a later block does not absolve an unclosed one', () => { + for (const variant of VARIANTS) { + const source = variant.build([ + '# /// script', + '# x = 1', + 'code = 1', + '# /// script', + '# y = 2', + '# ///', + ]); + const problem = onlyProblem(parseInlineScriptMetadata(source)); + assert.strictEqual(problem.code, 'unterminated-block', `[${variant.name}] wrong problem code`); + assert.strictEqual(underlined(source, problem), '# /// script', `[${variant.name}] wrong range`); + } + }); }); suite('field types', () => { @@ -284,14 +332,33 @@ suite('inlineScriptMetadata diagnostics', () => { '# dependencies = ["oops"]', ].join('\n'); const result = parseInlineScriptMetadata(source); - assert.strictEqual(result.kind, 'valid'); - assert.deepStrictEqual(result.kind === 'valid' ? result.metadata.dependencies : undefined, ['requests']); + assert.strictEqual(result.kind, 'parsed'); + assert.deepStrictEqual(result.kind === 'parsed' ? result.metadata.dependencies : undefined, ['requests']); const problem = onlyProblem(result); assert.strictEqual(problem.code, 'unterminated-block'); assert.strictEqual(problem.severity, 'warning'); assert.ok(readInlineScriptMetadata(source), 'wrapper should still return the valid metadata'); }); + test('parsed metadata can still carry error-severity problems', () => { + const source = ['# /// script', '# x = 1', '# ///', '', '# /// script', '#bad'].join('\n'); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'parsed', 'metadata is usable even though the file has an error'); + const problem = onlyProblem(result); + assert.strictEqual(problem.code, 'invalid-content-line'); + assert.strictEqual(problem.severity, 'error'); + assert.ok(readInlineScriptMetadata(source), 'the wrapper still yields metadata'); + }); + + test('a malformed example below real code stays silent, closer or not', () => { + const source = ['print("x")', '"""', '# /// script', '# deps = []', 'prose line', '# ///', '"""'].join( + '\n', + ); + const result = parseInlineScriptMetadata(source); + assert.strictEqual(result.kind, 'none'); + assert.deepStrictEqual(problems(result), []); + }); + test('an unclosed block below real code is ignored, as the spec requires', () => { const source = ['"""', '# /// script', '# dependencies = ["docs"]', '"""', 'print("hi")'].join('\n'); const result = parseInlineScriptMetadata(source); @@ -302,7 +369,7 @@ suite('inlineScriptMetadata diagnostics', () => { test('a marker quoted inside a valid block is not a second block', () => { const source = ['# /// script', '# # /// script', '# dependencies = []', '# ///'].join('\n'); const result = parseInlineScriptMetadata(source); - assert.strictEqual(result.kind, 'valid'); + assert.strictEqual(result.kind, 'parsed'); assert.deepStrictEqual(problems(result), []); }); }); diff --git a/src/test/features/inlineScript/diagnostics.unit.test.ts b/src/test/features/inlineScript/diagnostics.unit.test.ts index e657ab63..492ce849 100644 --- a/src/test/features/inlineScript/diagnostics.unit.test.ts +++ b/src/test/features/inlineScript/diagnostics.unit.test.ts @@ -14,6 +14,7 @@ import { Uri, } from 'vscode'; import * as logging from '../../../common/logging'; +import { InlineScriptStrings } from '../../../common/localize'; import * as wapi from '../../../common/workspace.apis'; import { InlineScriptDiagnosticsPublisher, shouldValidateUri } from '../../../features/inlineScript/diagnostics'; @@ -123,7 +124,27 @@ suite('InlineScriptDiagnosticsPublisher', () => { assert.strictEqual(published!.length, 1); assert.strictEqual(published![0].code, 'unterminated-block'); assert.strictEqual(published![0].severity, DiagnosticSeverity.Warning); - assert.ok(published![0].message.length > 0, 'expected a localized message'); + assert.strictEqual(published![0].message, InlineScriptStrings.unterminatedBlock); + assert.strictEqual(published![0].source, InlineScriptStrings.diagnosticSource); + }); + + test('each problem code maps to its own message', () => { + const cases: [string, string, string][] = [ + [['# /// script', '#bad', '# ///'].join('\n'), 'invalid-content-line', '#bad'], + [['# /// script ', '# x = 1', '# ///'].join('\n'), 'invalid-block-marker', '# /// script '], + ]; + for (const [text, code, detail] of cases) { + const uri = Uri.file(`/workspace/${code}.py`); + openListener?.(makeDoc(uri, text)); + const published = sink.for(uri); + assert.strictEqual(published![0].code, code); + assert.strictEqual( + published![0].message, + code === 'invalid-content-line' + ? InlineScriptStrings.invalidContentLine(detail) + : InlineScriptStrings.invalidBlockMarker(detail), + ); + } }); test('spec violations are published as errors', () => { @@ -250,6 +271,26 @@ suite('InlineScriptDiagnosticsPublisher', () => { assert.strictEqual(sink.for(nested), undefined); }); + test('deleting a folder cancels a queued validation for a file inside it', () => { + const nested = Uri.file('/workspace/pkg/app.py'); + change(makeDoc(nested, BROKEN_SCRIPT)); + + deleteListener?.({ files: [Uri.file('/workspace/pkg')] }); + clock.tick(DEBOUNCE_MS * 2); + + assert.strictEqual(sink.for(nested), undefined, 'a deleted file must not gain a squiggle'); + }); + + test('renaming a folder cancels a queued validation for a file inside it', () => { + const nested = Uri.file('/workspace/pkg/app.py'); + change(makeDoc(nested, BROKEN_SCRIPT)); + + renameListener?.({ files: [{ oldUri: Uri.file('/workspace/pkg'), newUri: Uri.file('/workspace/pkg2') }] }); + clock.tick(DEBOUNCE_MS * 2); + + assert.strictEqual(sink.for(nested), undefined, 'a renamed-away file must not gain a squiggle'); + }); + test('renaming clears the old path and re-validates the new one', () => { const renamed = Uri.file('/workspace/renamed.py'); openListener?.(makeDoc(scriptUri, BROKEN_SCRIPT));