diff --git a/src/common/inlineScript/metadata.ts b/src/common/inlineScript/metadata.ts index 0ef424fe..7dad32fa 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,36 @@ 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; +} + +/** + * 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: 'parsed'; + 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 +86,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 +114,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 === 'parsed' ? 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 +135,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 +163,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, 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); + } + 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 +217,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 +236,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 +257,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 +278,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 +320,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 +349,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 +369,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: 'parsed', + problems, + metadata: { + requiresPython, + dependencies, + tool, + range: { start: matchStart, end }, + sourceRange: toSourceRange(matchStart, end), }, }; } @@ -269,16 +392,217 @@ 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; +} + +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, +): MalformedBlockDiagnosis { + 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 { + ignorableBelowHeader: false, + problem: { + 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 { + ignorableBelowHeader: false, + problem: { + code: 'invalid-block-marker', + severity: 'error', + sourceRange: toSourceRange(offset, lineEnd), + detail: line, + }, + }; + } + + if (!isValidContentLine(line)) { + // 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 { + 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; + } + + 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 { + ignorableBelowHeader: true, + problem: { 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] === ' '; +} + +/** 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; } - 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 +634,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 +660,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..43bf9127 --- /dev/null +++ b/src/features/inlineScript/diagnostics.ts @@ -0,0 +1,229 @@ +// 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; + } + // 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); + } + } + } + + 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..1fc687a1 --- /dev/null +++ b/src/test/common/inlineScript/metadataDiagnostics.unit.test.ts @@ -0,0 +1,416 @@ +// 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, 'parsed'); + 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, 'parsed', `[${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.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', () => { + 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, '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); + 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, 'parsed'); + 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..492ce849 --- /dev/null +++ b/src/test/features/inlineScript/diagnostics.unit.test.ts @@ -0,0 +1,359 @@ +// 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 { InlineScriptStrings } from '../../../common/localize'; +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.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', () => { + 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('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)); + 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); + }); + }); +});