diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index 37ad2faf7..c6fac6247 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -21,6 +21,7 @@ This guide covers the full testing lifecycle: - Pip commands that return JSON must pass `--disable-pip-version-check`; the process helper combines stderr with stdout, so update notices can otherwise make valid JSON unparseable (1). - When a view subscribes to a newly added provider event, TypeMoq-based view tests must return a real `EventEmitter.event`; an unstubbed event yields an undefined disposable and fails during teardown (1). - Test agent selections across a full reload with `python.defaultInterpreterPath` set. An effective manager value equal to the extension default does not prove a workspace value was saved; tool-owned persistence must inspect `workspaceValue` or startup can restore the global interpreter (1). +- `vscode.executeCodeLensProvider` verifies provider output, not visible editor refresh. VS Code cancels its debounced CodeLens refresh when the editor loses focus; account for background UI automation restoring focus to another app before diagnosing a stale rendered lens (1). ### When to Use This Guide diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index f5e4cecb1..feed6ab6b 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -101,7 +101,9 @@ When you create a script, the extension generates a single `.py` file with PEP 7 An inline-script environment is built from the script's `# /// script` block and stored in the extension's cache, where it is shared by every script with the same dependencies and base interpreter. Because editing one would silently change the others, these environments are not user-managed: the Python Environments views do not offer install, uninstall, or version-change actions for them. Their package list remains visible. -A CodeLens above the `# /// script` block offers **Set up environment for this script**, and the same action is available as a quick fix on an unresolved import. For a few seconds after setup succeeds it is replaced by a **Script environment ready (Python X.Y.Z)** confirmation naming the Python that was selected — useful when `requires-python` matches several installed versions, or when one was installed on demand. The confirmation is plain text rather than a clickable action, and it expires on its own; at every other time the setup CodeLens behaves exactly as before. +A CodeLens stays above the `# /// script` block, including while its metadata is incomplete, malformed, or unsaved. It offers **Set up environment for this script** until the block matches a validated environment, then displays **Script environment ready (Python X.Y.Z)** as persistent, non-clickable text. The ready label also appears for environments restored after reopening VS Code. + +The CodeLens follows the live block text: editing the block offers setup immediately without requiring a save, while editing Python code outside it does not change its fingerprint. Diagnostics continue to explain malformed metadata as you type; a warning popup appears only if you try to set up an invalid block. Setup saves valid unsaved changes before creating the environment. Execution and interpreter routing still use validated, saved metadata, and saving can restore a matching existing association without rebuilding it. Setup records which distributions it installed. If that record and the environment's contents later disagree — for example after installing a package into it from a terminal — every script sharing the environment needs setup again. Saving or reopening a script does not repair it; use the script's setup action to rebuild from its declared dependencies. diff --git a/src/common/inlineScript/block.ts b/src/common/inlineScript/block.ts new file mode 100644 index 000000000..bf4892b10 --- /dev/null +++ b/src/common/inlineScript/block.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export interface InlineScriptBlock { + readonly start: number; + readonly end: number; +} + +/** + * Locate the first recognizable script opener and its next closing marker, without validation. + * Incomplete blocks are included so editor actions remain available while typing. + * This approximate presentation range must not be used for metadata validation or fingerprints. + */ +export function findInlineScriptBlock(text: string): InlineScriptBlock | undefined { + const opener = /^[\t \uFEFF]*#[\t ]*\/\/\/[\t ]+script\b[^\r\n]*/gm.exec(text); + if (!opener) { + return undefined; + } + const closerPattern = /^[\t ]*#[\t ]*\/\/\/[\t ]*$/gm; + closerPattern.lastIndex = opener.index + opener[0].length; + const closer = closerPattern.exec(text); + return { + start: opener.index + (opener.index === 0 && text.startsWith('\uFEFF') ? 1 : 0), + end: closer ? closer.index + closer[0].length : text.length, + }; +} diff --git a/src/common/inlineScript/metadata.ts b/src/common/inlineScript/metadata.ts index cb61a9655..b0c22393f 100644 --- a/src/common/inlineScript/metadata.ts +++ b/src/common/inlineScript/metadata.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as tomljs from '@iarna/toml'; +import { createHash } from 'crypto'; import * as fs from 'fs/promises'; import { l10n, Uri } from 'vscode'; import { traceVerbose, traceWarn } from '../logging'; @@ -29,6 +30,8 @@ export interface InlineScriptMetadata { */ readonly range: { readonly start: number; readonly end: number }; readonly sourceRange?: { readonly start: number; readonly end: number }; + /** Saved block fingerprint for edit detection and presentation, separate from semantic/cache identity. */ + readonly sourceHash?: string; } /** @@ -70,6 +73,24 @@ export type InlineScriptMetadataParseResult = | { readonly kind: 'none' } | { readonly kind: 'invalid'; readonly problems: readonly InlineScriptMetadataProblem[] }; +/** + * Explain why parsed metadata cannot be used by the interactive setup action. + * This presentation check does not change the parser's tolerant metadata or routing contract. + */ +export function getInlineScriptSetupProblem( + result: InlineScriptMetadataParseResult, +): 'metadata' | 'requires-python' | undefined { + if ( + result.kind !== 'parsed' || + result.problems.length > 0 || + result.metadata.dependencies?.some((dependency) => dependency.trim().length === 0) + ) { + return 'metadata'; + } + const requirement = result.metadata.requiresPython?.trim(); + return requirement && !PythonVersionSpecifier.tryParse(requirement) ? 'requires-python' : undefined; +} + /** * Canonical block regex from the PEP 723 spec, translated to JavaScript * (Python's `(?P...)` becomes `(?...)` in JS). The flag @@ -122,6 +143,57 @@ const NO_METADATA: InlineScriptMetadataParseResult = { kind: 'none' }; const OPENER_PREFIX = '# /// '; +/** + * Fingerprint the same structural script block selected by the metadata parser, without parsing TOML, + * logging, or I/O. Malformed markers return undefined; ignored body examples do not affect the hash. + * Normalize BOM/line endings as the parser does so disk and editor representations compare equally. + */ +export function getInlineScriptSourceHash(scriptText: string): string | undefined { + const text = scriptText.replace(/^\uFEFF/, '').replace(/\r\n?/g, '\n'); + const { scriptMatches, problems } = scanInlineScriptBlocks( + text, + (start, end) => ({ start, end }), + '', + false, + ); + return scriptMatches.length === 1 && problems.length === 0 + ? hashScriptBlock(scriptMatches[0][0]) + : undefined; +} + +function hashScriptBlock(block: string): string { + return createHash('sha256').update(block, 'utf8').digest('hex'); +} + +function scanInlineScriptBlocks( + text: string, + toSourceRange: (start: number, end: number) => { start: number; end: number }, + where: string, + logProblems = true, +): { scriptMatches: RegExpMatchArray[]; problems: InlineScriptMetadataProblem[] } { + const scriptMatches: RegExpMatchArray[] = []; + // matchAll leaves the shared regex's lastIndex untouched. + for (const match of text.matchAll(BLOCK_RE)) { + if (match.groups?.type === 'script') { + scriptMatches.push(match); + } + } + const matchedRanges = scriptMatches.map((match) => ({ start: match.index!, end: match.index! + match[0].length })); + const problems: InlineScriptMetadataProblem[] = []; + const headerEnd = headerRegionEnd(text); + for (const opener of findScriptOpeners(text)) { + if (matchedRanges.some((range) => opener.offset >= range.start && opener.offset < range.end)) { + continue; + } + const { problem, ignorableBelowHeader } = diagnoseMalformedBlock(text, opener, toSourceRange, where, logProblems); + if (ignorableBelowHeader && opener.offset >= headerEnd) { + continue; + } + problems.push(problem); + } + return { scriptMatches, problems }; +} + /** 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}` : ''; @@ -147,37 +219,7 @@ export function parseInlineScriptMetadata(scriptText: string, source?: string): 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. - // - // `matchAll` constructs a fresh iterator and does not mutate the - // shared `BLOCK_RE.lastIndex`, so this loop is re-entrant and safe - // even if a caller (or an exception) ever interrupts a previous - // pass. - const scriptMatches: RegExpMatchArray[] = []; - for (const m of text.matchAll(BLOCK_RE)) { - // Per spec, tools MUST NOT read non-standardized block types. - // The only standardized type today is `script`. - if (m.groups?.type === 'script') { - scriptMatches.push(m); - } - } - - 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); - } + const { scriptMatches, problems } = scanInlineScriptBlocks(text, toSourceRange, where); if (scriptMatches.length === 0) { if (problems.length === 0) { @@ -368,7 +410,7 @@ export function parseInlineScriptMetadata(scriptText: string, source?: string): end += 1; } - return { + const result: InlineScriptMetadataParseResult = { kind: 'parsed', problems, metadata: { @@ -379,6 +421,13 @@ export function parseInlineScriptMetadata(scriptText: string, source?: string): sourceRange: toSourceRange(matchStart, end), }, }; + return { + ...result, + metadata: { + ...result.metadata, + sourceHash: getInlineScriptSetupProblem(result) === undefined ? hashScriptBlock(match[0]) : undefined, + }, + }; } /** 1-based line number of `offset` within LF-normalized `text`. */ @@ -447,13 +496,16 @@ function diagnoseMalformedBlock( opener: ScriptOpener, toSourceRange: (start: number, end: number) => { start: number; end: number }, where: string, + logProblems: boolean, ): 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`, - ); + if (logProblems) { + traceWarn( + `inline script metadata${where}: the \`# /// script\` marker on line ${countLines(text, opener.offset)} has trailing whitespace`, + ); + } return { ignorableBelowHeader: false, problem: { @@ -475,9 +527,11 @@ function diagnoseMalformedBlock( } if (line !== line.trimEnd() && line.trimEnd() === CLOSER_LINE) { - traceWarn( - `inline script metadata${where}: the closing \`# ///\` marker on line ${countLines(text, offset)} has trailing whitespace`, - ); + if (logProblems) { + traceWarn( + `inline script metadata${where}: the closing \`# ///\` marker on line ${countLines(text, offset)} has trailing whitespace`, + ); + } return { ignorableBelowHeader: false, problem: { @@ -494,10 +548,12 @@ function diagnoseMalformedBlock( // 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)}`, - ); + if (logProblems) { + traceWarn( + `inline script metadata${where}: invalid content line ${countLines(text, offset)} ` + + `(expected '#' or '# '): ${JSON.stringify(line)}`, + ); + } return { ignorableBelowHeader: !isComment, problem: { @@ -520,9 +576,11 @@ function diagnoseMalformedBlock( offset = lineEnd + 1; } - traceWarn( - `inline script metadata${where}: the \`# /// script\` block on line ${countLines(text, opener.offset)} is missing its closing \`# ///\` marker`, - ); + if (logProblems) { + 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 }, @@ -661,6 +719,28 @@ export async function readInlineScriptMetadataFromFile( uri: Uri, strict = false, ): Promise { + const text = await readInlineScriptHeaderFromFile(uri, strict); + if (text === undefined) { + return undefined; + } + const result = parseInlineScriptMetadata(text, uri.fsPath); + if ( + strict && + (result.kind === 'invalid' || + (result.kind === 'parsed' && result.problems.some((problem) => problem.severity === 'error'))) + ) { + throw new Error(l10n.t('Fix the PEP 723 metadata in {0} before configuring its environment.', uri.fsPath)); + } + return result.kind === 'parsed' ? result.metadata : undefined; +} + +/** + * Read at most MAX_HEADER_BYTES of a local script as UTF-8, without parsing its metadata. + * Unsupported URI schemes and I/O failures return undefined and are logged. + * @param uri The local script to read. + * @param strict Throw on I/O errors instead of treating them as absent. + */ +export async function readInlineScriptHeaderFromFile(uri: Uri, strict = false): Promise { if (uri.scheme !== 'file') { traceVerbose(`inline script metadata: skipping non-file URI scheme '${uri.scheme}'`); return undefined; @@ -683,15 +763,7 @@ export async function readInlineScriptMetadataFromFile( return undefined; } - const result = parseInlineScriptMetadata(text, uri.fsPath); - if ( - strict && - (result.kind === 'invalid' || - (result.kind === 'parsed' && result.problems.some((problem) => problem.severity === 'error'))) - ) { - throw new Error(l10n.t('Fix the PEP 723 metadata in {0} before configuring its environment.', uri.fsPath)); - } - return result.kind === 'parsed' ? result.metadata : undefined; + return text; } /** diff --git a/src/common/inlineScript/routingRegistry.ts b/src/common/inlineScript/routingRegistry.ts index 0f778e2dc..c1a6e7c19 100644 --- a/src/common/inlineScript/routingRegistry.ts +++ b/src/common/inlineScript/routingRegistry.ts @@ -41,6 +41,8 @@ interface ScriptRoutingState { readonly metadataIdentity?: string; readonly metadataRevision: number; readonly validatedAssociation: boolean; + readonly liveMetadataMatchesSaved?: boolean; + readonly environmentVersion?: string; readonly environmentUnavailable?: boolean; } @@ -51,8 +53,10 @@ export class InlineScriptRoutingRegistry implements Disposable { private readonly _onDidChangeRouteability = new EventEmitter(); private readonly _onDidChangeMetadata = new EventEmitter(); private readonly _onDidChangeAvailability = new EventEmitter(); + private readonly _onDidChangeEnvironmentVersion = new EventEmitter(); public readonly onDidChangeAvailability: Event = this._onDidChangeAvailability.event; + public readonly onDidChangeEnvironmentVersion: Event = this._onDidChangeEnvironmentVersion.event; public readonly onDidChangeRouteability: Event = this._onDidChangeRouteability.event; @@ -75,6 +79,7 @@ export class InlineScriptRoutingRegistry implements Disposable { metadata, metadataIdentity, metadataRevision, + liveMetadataMatchesSaved: metadata ? true : undefined, validatedAssociation: state.metadataIdentity === metadataIdentity ? state.validatedAssociation : false, environmentUnavailable: @@ -100,6 +105,7 @@ export class InlineScriptRoutingRegistry implements Disposable { metadata: undefined, metadataIdentity: undefined, metadataRevision, + liveMetadataMatchesSaved: undefined, }; }, true, @@ -126,7 +132,18 @@ export class InlineScriptRoutingRegistry implements Disposable { return scriptPath ? this.states.get(scriptPath)?.uri : undefined; } - public setValidatedAssociation(script: Uri | string, validatedAssociation: boolean): void { + /** The Python version of the validated script environment, when one is known. */ + public getEnvironmentVersion(script: Uri | string): string | undefined { + const scriptPath = getInlineScriptRoutingKey(script); + const state = scriptPath ? this.states.get(scriptPath) : undefined; + return this.isRouteable(state) ? state?.environmentVersion : undefined; + } + + public setValidatedAssociation( + script: Uri | string, + validatedAssociation: boolean, + environmentVersion?: string, + ): void { const scriptPath = getInlineScriptRoutingKey(script); if (!scriptPath) { return; @@ -135,6 +152,7 @@ export class InlineScriptRoutingRegistry implements Disposable { ...state, uri: script instanceof Uri ? script : state.uri, validatedAssociation, + environmentVersion: validatedAssociation ? environmentVersion ?? state.environmentVersion : undefined, environmentUnavailable: validatedAssociation ? state.environmentUnavailable : false, })); } @@ -159,6 +177,18 @@ export class InlineScriptRoutingRegistry implements Disposable { return scriptPath ? this.states.get(scriptPath)?.validatedAssociation === true : false; } + /** + * Temporarily suppress routing while an open document's live metadata block differs from the + * saved block. The validated association is retained so Undo can restore routing immediately. + */ + public setLiveMetadataMatchesSaved(uri: Uri, matches: boolean): void { + const scriptPath = getInlineScriptRoutingKey(uri); + if (!scriptPath || !this.states.get(scriptPath)?.metadata) { + return; + } + this.update(scriptPath, (state) => ({ ...state, uri, liveMetadataMatchesSaved: matches })); + } + public noteSetupOutcome(script: Uri | string, outcome: InlineScriptSetupOutcome): void { const scriptPath = getInlineScriptRoutingKey(script); if (scriptPath) { @@ -200,6 +230,7 @@ export class InlineScriptRoutingRegistry implements Disposable { this._onDidChangeMetadata.dispose(); this._onDidChangeRouteability.dispose(); this._onDidChangeAvailability.dispose(); + this._onDidChangeEnvironmentVersion.dispose(); } private update( @@ -241,10 +272,17 @@ export class InlineScriptRoutingRegistry implements Disposable { if (previouslyUnavailable !== (routeable && next.environmentUnavailable === true) && next.uri) { this._onDidChangeAvailability.fire(next.uri); } + if ( + (previousRouteable ? previous.environmentVersion : undefined) !== + (routeable ? next.environmentVersion : undefined) && + next.uri + ) { + this._onDidChangeEnvironmentVersion.fire(next.uri); + } } private isRouteable(state: ScriptRoutingState | undefined): boolean { - return !!state?.metadata && state.validatedAssociation; + return !!state?.metadata && state.validatedAssociation && state.liveMetadataMatchesSaved !== false; } private nextMetadataRevision(scriptPath: string): number { diff --git a/src/common/localize.ts b/src/common/localize.ts index 3982a86fb..617130f9a 100644 --- a/src/common/localize.ts +++ b/src/common/localize.ts @@ -31,6 +31,15 @@ export namespace InlineScriptStrings { export const saveFailedBeforeSetup = l10n.t( 'Could not save this script, so its environment was not set up. Save the file and try again.', ); + export const invalidMetadataBeforeSetup = l10n.t( + "Fix the '# /// script' metadata before setting up this script's environment. Check the block markers, TOML, and declared dependencies.", + ); + export const invalidPythonRequirement = l10n.t( + "The script's 'requires-python' must be a valid Python version specifier, for example '>=3.11'.", + ); + export const scriptReadFailedBeforeSetup = l10n.t( + 'Could not read this script, so its environment was not set up. Make sure the file is available and try again.', + ); export const updatePythonExtension = l10n.t( 'The environment for this script was created. Update the Python extension for the full inline script experience.', diff --git a/src/features/inlineScript/codeLens.ts b/src/features/inlineScript/codeLens.ts index 460c4b8dc..5ba3bce79 100644 --- a/src/features/inlineScript/codeLens.ts +++ b/src/features/inlineScript/codeLens.ts @@ -11,45 +11,32 @@ import { languages, Range, TextDocument, - Uri, } from 'vscode'; +import { findInlineScriptBlock } from '../../common/inlineScript/block'; +import { getInlineScriptSourceHash, sliceHeaderBytes } from '../../common/inlineScript/metadata'; import { getInlineScriptRoutingKey, InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; import { InlineScriptStrings } from '../../common/localize'; - -export const READY_CONFIRMATION_TIMEOUT_MS = 5000; +import { shortenVersionString } from '../../managers/common/utils'; /** - * Shows a single "Set up environment for this script" CodeLens above a `.py` file's PEP 723 - * `# /// script` block, but only when the file has saved inline metadata that is not currently - * backed by a validated inline-script environment. - * - * The provider is a pure observer of {@link InlineScriptRoutingRegistry}: - * - `getMetadata` returns the last saved metadata (the detector clears it while the metadata - * region is dirty), so the lens tracks *saved* metadata and disappears while it is being edited. - * - `shouldRoute` is true once a validated association matching the current metadata exists, so the - * lens hides after setup and reappears if a metadata change later invalidates that association. + * Keep a CodeLens above the live inline block, including malformed or unsaved metadata. + * Only an unchanged block backed by a validated environment displays the persistent ready label. + * Text detection and hashing do not parse TOML or change saved-metadata routing. */ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposable { private readonly _onDidChangeCodeLenses = new EventEmitter(); public readonly onDidChangeCodeLenses = this._onDidChangeCodeLenses.event; private readonly subscriptions: Disposable[] = []; - private readonly readyConfirmations = new Map< - string, - { readonly version: string | undefined; readonly timer: ReturnType } - >(); private disposed = false; constructor( private readonly routing: InlineScriptRoutingRegistry, private readonly setupCommand: string, - private readonly confirmationTimeoutMs: number = READY_CONFIRMATION_TIMEOUT_MS, ) { this.subscriptions.push( this.routing.onDidChangeRouteability(() => this._onDidChangeCodeLenses.fire()), this.routing.onDidChangeAvailability(() => this._onDidChangeCodeLenses.fire()), - // Only metadata arriving or changing can add/replace a lens; a scan that finds no metadata - // (the common case for ordinary .py files) needs no refresh. Hiding a lens for an - // edited/removed block is handled by VS Code re-querying on the document change itself. + this.routing.onDidChangeEnvironmentVersion(() => this._onDidChangeCodeLenses.fire()), this.routing.onDidChangeMetadata((e) => { if (e.metadata !== undefined) { this._onDidChangeCodeLenses.fire(); @@ -58,49 +45,30 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl ); } - public noteEnvironmentReady(uri: Uri, version: string | undefined): void { - const key = getInlineScriptRoutingKey(uri); - if (this.disposed || !key) { - return; - } - this.clearConfirmation(key); - this.readyConfirmations.set(key, { - version, - timer: setTimeout(() => { - this.readyConfirmations.delete(key); - this._onDidChangeCodeLenses.fire(); - }, this.confirmationTimeoutMs), - }); - this._onDidChangeCodeLenses.fire(); - } - public provideCodeLenses(document: TextDocument, _token: CancellationToken): CodeLens[] { - if (document.isDirty) { - // The association is validated against the saved file (the manager refuses to validate a - // dirty document), so only offer setup for a clean document. This also avoids anchoring the - // lens at a stale offset if the block moved on an unsaved edit. + const uri = document.uri; + if (this.disposed || !getInlineScriptRoutingKey(uri)) { return []; } - const uri = document.uri; - const metadata = this.routing.getMetadata(uri); - if (!metadata) { - // No saved PEP 723 metadata (or it is currently being edited). + const header = sliceHeaderBytes(document.getText()); + const block = findInlineScriptBlock(header); + if (!block) { return []; } - const offset = metadata.sourceRange?.start ?? metadata.range.start; - const position = document.positionAt(offset); + const position = document.positionAt(block.start); const range = new Range(position, position); - if (this.routing.shouldRoute(uri) && !this.routing.isEnvironmentUnavailable(uri)) { - // A validated inline-script environment matching the current metadata already exists. - const key = getInlineScriptRoutingKey(uri); - const confirmation = key ? this.readyConfirmations.get(key) : undefined; - if (!confirmation) { - return []; - } + const savedHash = this.routing.getMetadata(uri)?.sourceHash; + if ( + this.routing.shouldRoute(uri) && + !this.routing.isEnvironmentUnavailable(uri) && + savedHash !== undefined && + savedHash === getInlineScriptSourceHash(header) + ) { + const version = this.routing.getEnvironmentVersion(uri); // An empty command id renders the title as plain, non-clickable text. return [ new CodeLens(range, { - title: InlineScriptStrings.environmentReady(confirmation.version), + title: InlineScriptStrings.environmentReady(version ? shortenVersionString(version) : undefined), command: '', }), ]; @@ -118,18 +86,8 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl this.disposed = true; this.subscriptions.forEach((s) => s.dispose()); this.subscriptions.length = 0; - this.readyConfirmations.forEach((confirmation) => clearTimeout(confirmation.timer)); - this.readyConfirmations.clear(); this._onDidChangeCodeLenses.dispose(); } - - private clearConfirmation(key: string): void { - const existing = this.readyConfirmations.get(key); - if (existing) { - clearTimeout(existing.timer); - this.readyConfirmations.delete(key); - } - } } /** diff --git a/src/features/inlineScript/lazyDetector.ts b/src/features/inlineScript/lazyDetector.ts index 5af895089..ed4bc0c40 100644 --- a/src/features/inlineScript/lazyDetector.ts +++ b/src/features/inlineScript/lazyDetector.ts @@ -3,7 +3,11 @@ import * as path from 'path'; import { Disposable, TextDocument, TextDocumentChangeEvent, TextDocumentContentChangeEvent, Uri } from 'vscode'; -import { readInlineScriptMetadataFromFile } from '../../common/inlineScript/metadata'; +import { + getInlineScriptSourceHash, + readInlineScriptMetadataFromFile, + sliceHeaderBytes, +} from '../../common/inlineScript/metadata'; import { getInlineScriptRoutingKey, InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; import { traceVerbose, traceWarn } from '../../common/logging'; import { EventNames } from '../../common/telemetry/constants'; @@ -278,7 +282,18 @@ export class InlineScriptLazyDetector implements Disposable { if (this.routingRegistry) { const key = this.getReadKey(e.document.uri); const metadata = this.routingRegistry.getMetadata(e.document.uri); - if ( + // Saved offsets can become stale when an unchanged block moves, so fingerprinted headers are compared directly. + if (metadata?.sourceHash) { + const liveMetadataMatchesSaved = + getInlineScriptSourceHash(sliceHeaderBytes(e.document.getText())) === metadata.sourceHash; + if (!liveMetadataMatchesSaved && this.inFlight.has(key)) { + this.advanceRoutingReadGeneration(key); + } + this.routingRegistry.setLiveMetadataMatchesSaved( + e.document.uri, + liveMetadataMatchesSaved, + ); + } else if ( (metadata && this.contentChangesMayAffectMetadata( e.contentChanges, diff --git a/src/features/inlineScript/setupCodeAction.ts b/src/features/inlineScript/setupCodeAction.ts index 769d7b0fd..695dd5f62 100644 --- a/src/features/inlineScript/setupCodeAction.ts +++ b/src/features/inlineScript/setupCodeAction.ts @@ -62,8 +62,8 @@ export function isUnresolvedImportDiagnostic(diagnostic: Diagnostic): boolean { * Offers "Set up this script's Python environment" as a quick fix on an unresolved import in a `.py` * file that declares a PEP 723 `# /// script` block and has no inline-script environment yet. * - * Complements the CodeLens, which is hidden while the document is dirty — the moment a user has just - * typed the import that does not resolve. This provider parses the in-memory buffer instead. + * Complements the inline-block CodeLens with an action at the unresolved import. Both use the live + * document, so the action remains available while the user is editing valid metadata. * * `diagnostics` and `isPreferred` are both left unset: setup installs the block's declared * dependencies verbatim and may not resolve the import at all, so the action must not claim to fix diff --git a/src/features/inlineScript/setupEnvironment.ts b/src/features/inlineScript/setupEnvironment.ts index 81ac711d8..620b628b6 100644 --- a/src/features/inlineScript/setupEnvironment.ts +++ b/src/features/inlineScript/setupEnvironment.ts @@ -4,7 +4,13 @@ import { commands, Disposable, l10n, QuickPickItem, TextDocument, Uri, window } from 'vscode'; import { PythonEnvironment } from '../../api'; import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; -import { readInlineScriptMetadataFromFile } from '../../common/inlineScript/metadata'; +import { + getInlineScriptSetupProblem, + parseInlineScriptMetadata, + readInlineScriptHeaderFromFile, + readInlineScriptMetadataFromFile, + sliceHeaderBytes, +} from '../../common/inlineScript/metadata'; import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; import { InlineScriptStrings } from '../../common/localize'; import { traceError, traceInfo, traceVerbose } from '../../common/logging'; @@ -17,7 +23,6 @@ import { } from '../../common/window.apis'; import { asRelativePath, findFiles, getOpenTextDocuments } from '../../common/workspace.apis'; import type { EnvironmentManagers } from '../envManagers'; -import { shortenVersionString } from '../../managers/common/utils'; import { registerInlineScriptCodeLens } from './codeLens'; import { promptUpdateExtensionsForInlineScripts } from './extensionVersionCheck'; import { registerInlineScriptSetupCodeAction } from './setupCodeAction'; @@ -107,35 +112,63 @@ function findOpenDocument(scriptUri: Uri): TextDocument | undefined { } /** - * Save `scriptUri` if it is open with unsaved changes, so setup reads what the user actually sees. + * Save dirty scripts and seed saved metadata for open scripts, including clean Undo/Revert results. * * Returns `false` when the document could not be saved; setup must not run in that case. */ async function saveScriptBeforeSetup(scriptUri: Uri, routing: InlineScriptRoutingRegistry): Promise { const document = findOpenDocument(scriptUri); - if (!document?.isDirty) { + if (!document) { return true; } - if (!(await document.save())) { - traceError(`Could not save ${scriptUri.fsPath} before setting up its inline-script environment.`); - return false; + if (document.isDirty) { + if (!(await document.save())) { + traceError(`Could not save ${scriptUri.fsPath} before setting up its inline-script environment.`); + return false; + } + traceVerbose(`Saved ${scriptUri.fsPath} before setting up its inline-script environment.`); } - traceVerbose(`Saved ${scriptUri.fsPath} before setting up its inline-script environment.`); // Seeding here is load-bearing: without it a just-typed block goes from no metadata to an // identity while `create` runs, which `setUpInlineScriptEnvironment` reads as a concurrent edit // and silently skips the association. + const savedVersion = document.version; + const metadataRevision = routing.getMetadataRevision(scriptUri); const metadata = await readInlineScriptMetadataFromFile(scriptUri); - if (metadata) { + if ( + metadata && + !document.isDirty && + document.version === savedVersion && + routing.getMetadataRevision(scriptUri) === metadataRevision + ) { routing.setMetadata(scriptUri, metadata); } return true; } +async function validateScriptBeforeSetup(uri: Uri): Promise { + const document = findOpenDocument(uri); + const text = document ? sliceHeaderBytes(document.getText()) : await readInlineScriptHeaderFromFile(uri); + if (text === undefined) { + showErrorMessage(InlineScriptStrings.scriptReadFailedBeforeSetup); + return false; + } + const result = parseInlineScriptMetadata(text, uri.fsPath); + const problem = getInlineScriptSetupProblem(result); + if (problem === 'metadata') { + showWarningMessage(InlineScriptStrings.invalidMetadataBeforeSetup); + return false; + } + if (problem === 'requires-python') { + showWarningMessage(InlineScriptStrings.invalidPythonRequirement); + return false; + } + return true; +} + /** Handler for the single-file setup command, shared by the CodeLens and the quick fix. */ export function setupInlineScriptEnvironmentHandler( em: EnvironmentManagers, routing: InlineScriptRoutingRegistry, - onEnvironmentReady?: (scriptUri: Uri, version: string | undefined) => void, ): (scriptUri?: Uri) => Promise { return async (scriptUri?: Uri): Promise => { const uri = scriptUri ?? window.activeTextEditor?.document.uri; @@ -146,12 +179,18 @@ export function setupInlineScriptEnvironmentHandler( showErrorMessage(l10n.t('The inline script environment manager is not available yet. Try again shortly.')); return; } - if (!(await saveScriptBeforeSetup(uri, routing))) { - showErrorMessage(InlineScriptStrings.saveFailedBeforeSetup); - return; - } let environment: PythonEnvironment | undefined; try { + if (!(await validateScriptBeforeSetup(uri))) { + return; + } + if (!(await saveScriptBeforeSetup(uri, routing))) { + showErrorMessage(InlineScriptStrings.saveFailedBeforeSetup); + return; + } + if (!(await validateScriptBeforeSetup(uri))) { + return; + } environment = await setUpInlineScriptEnvironment(uri, em, routing); } catch (error) { traceError(`Failed to set up the inline-script environment for ${uri.fsPath}:`, error); @@ -166,7 +205,6 @@ export function setupInlineScriptEnvironmentHandler( notifyInlineScriptSetupOutcome(uri, routing); return; } - onEnvironmentReady?.(uri, shortenVersionString(environment.version)); // Kept out of the try: the environment is already set up, so a failure in this follow-up // must not be reported to the user as a setup failure. await promptUpdateExtensionsForInlineScripts().catch((error) => @@ -371,9 +409,7 @@ export function registerInlineScriptUx(em: EnvironmentManagers, routing: InlineS registerInlineScriptSetupCodeAction(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND), commands.registerCommand( SETUP_INLINE_SCRIPT_ENV_COMMAND, - setupInlineScriptEnvironmentHandler(em, routing, (uri, version) => - codeLens.provider.noteEnvironmentReady(uri, version), - ), + setupInlineScriptEnvironmentHandler(em, routing), ), commands.registerCommand(SETUP_INLINE_SCRIPT_ENVS_COMMAND, () => setUpInlineScriptEnvironmentsInWorkspace(em, routing), diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 39dfcded4..3067a06a6 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -2370,7 +2370,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.clearValidatedRouteableState(uri); return; } - this.routingRegistry.setValidatedAssociation(uri, true); + this.routingRegistry.setValidatedAssociation(uri, true, environment.version); } private async updateValidatedStateForSelection(script: ScriptReference): Promise { @@ -2405,6 +2405,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.routingRegistry.setValidatedAssociation( script.uri, this.routingRegistry.getMetadataIdentity(script.uri) === savedMetadata.identity, + environment.version, ); } diff --git a/src/test/common/inlineScript/block.unit.test.ts b/src/test/common/inlineScript/block.unit.test.ts new file mode 100644 index 000000000..84dab308d --- /dev/null +++ b/src/test/common/inlineScript/block.unit.test.ts @@ -0,0 +1,130 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import { findInlineScriptBlock } from '../../../common/inlineScript/block'; +import { getInlineScriptSourceHash } from '../../../common/inlineScript/metadata'; + +suite('Inline script live block', () => { + const block = '# /// script\n# dependencies = ["requests"]\n# ///'; + + test('recognizes valid, empty, incomplete, and malformed blocks without parsing', () => { + for (const text of [block, '# /// script\n# ///', '# /// script', `${block}\n# /// script`, '# /// script \n#bad']) { + const found = findInlineScriptBlock(text); + assert.ok(found, text); + assert.strictEqual(found.start, 0); + assert.ok(found.end >= found.start); + } + }); + + test('does not recognize ordinary files, mentions, or other block types', () => { + for (const text of ['', 'print("hello")', '# mentions # /// script', '# /// pyproject\n# ///', '# /// scripting']) { + assert.strictEqual(findInlineScriptBlock(text), undefined, text); + } + }); + + test('excludes Python code and the closing marker newline from the hash', () => { + const expected = getInlineScriptSourceHash(block); + assert.match(expected!, /^[a-f0-9]{64}$/); + assert.strictEqual(getInlineScriptSourceHash(`${block}\nprint("first")`), expected); + assert.strictEqual(getInlineScriptSourceHash(`${block}\nprint("second")\n`), expected); + assert.strictEqual(getInlineScriptSourceHash(`#!/usr/bin/env python\n\n${block}`), expected); + assert.strictEqual(getInlineScriptSourceHash(`${block}\n# ordinary comment`), expected); + }); + + test('fingerprints raw metadata changes rather than semantic dependency identity', () => { + const expected = getInlineScriptSourceHash(block); + for (const edited of [ + block.replace('requests', 'Requests'), + block.replace(' = ', '='), + block.replace('# dependencies', '# additional comment\n# dependencies'), + block.replace('"requests"', '"requests'), + block.slice(0, block.lastIndexOf('# ///')), + ]) { + assert.notStrictEqual(getInlineScriptSourceHash(edited), expected); + } + }); + + test('keeps the first anchor but withholds the fingerprint for multiple blocks', () => { + const original = findInlineScriptBlock(block)!; + const multiple = findInlineScriptBlock(`${block}\nprint("body")\n${block}`)!; + assert.strictEqual(multiple.start, original.start); + assert.strictEqual(multiple.end, original.end); + assert.strictEqual(getInlineScriptSourceHash(`${block}\nprint("body")\n${block}`), undefined); + }); + + test('preserves the source span across BOM, CRLF, and lone CR', () => { + const expectedHash = getInlineScriptSourceHash(block); + for (const newline of ['\n', '\r\n', '\r']) { + const sourceBlock = block.replace(/\n/g, newline); + const source = `\uFEFF${sourceBlock}${newline}print("body")`; + const found = findInlineScriptBlock(source)!; + assert.strictEqual(found.start, 1); + assert.strictEqual(source.slice(found.start, found.end), sourceBlock); + assert.strictEqual(getInlineScriptSourceHash(source), expectedHash); + } + }); + + test('normalizes mixed line endings for editor and disk fingerprint comparison', () => { + const mixed = '# /// script\r\n# dependencies = ["requests"]\n# ///'; + assert.strictEqual(getInlineScriptSourceHash(mixed), getInlineScriptSourceHash(block)); + }); + + test('recognizes malformed content before a closing marker', () => { + const source = '# /// script\nnot_a_comment\n# ///\nprint("body")'; + const found = findInlineScriptBlock(source)!; + assert.strictEqual(source.slice(found.start, found.end), '# /// script\nnot_a_comment\n# ///'); + }); + + test('includes embedded closing markers followed by further comment content', () => { + const source = '# /// script\n# [tool.example]\n# value = """\n# ///\n# text\n# """\n# ///\nprint("body")'; + const hash = getInlineScriptSourceHash(source); + assert.ok(hash); + assert.notStrictEqual( + getInlineScriptSourceHash(source.replace('# text', '# edited text')), + hash, + ); + }); + + test('does not omit dependencies between embedded closing and opening markers', () => { + const source = [ + '# /// script', + '# note = """', + '# ///', + '# """', + '# dependencies = ["requests"]', + '# other = """', + '# /// script', + '# """', + '# ///', + ].join('\n'); + assert.notStrictEqual( + getInlineScriptSourceHash(source), + getInlineScriptSourceHash(source.replace('requests', 'httpx')), + ); + }); + + test('does not ignore a BOM inserted before a non-leading marker', () => { + const source = `# prefix\n${block}`; + assert.notStrictEqual( + getInlineScriptSourceHash(source), + getInlineScriptSourceHash(source.replace('# /// script', '\uFEFF# /// script')), + ); + }); + + test('withholds a fingerprint when a preceding non-script block consumes the script marker', () => { + assert.ok(getInlineScriptSourceHash(`# note\n${block}`)); + assert.ok(findInlineScriptBlock(`# /// other\n${block}`), 'the setup action still needs an anchor'); + assert.strictEqual(getInlineScriptSourceHash(`# /// other\n${block}`), undefined); + }); + + test('does not fingerprint ignored unfinished metadata examples in the Python body', () => { + const source = `${block}\n\n"""\n# /// script\n# dependencies = ["example"]\n"""\nprint("body")`; + assert.strictEqual(getInlineScriptSourceHash(source), getInlineScriptSourceHash(block)); + assert.strictEqual(getInlineScriptSourceHash(source.replace('"body"', '"changed"')), getInlineScriptSourceHash(block)); + }); + + test('fingerprinting does not parse TOML', () => { + assert.ok(getInlineScriptSourceHash('# /// script\n# dependencies = [\n# ///')); + }); +}); diff --git a/src/test/common/inlineScript/metadata.unit.test.ts b/src/test/common/inlineScript/metadata.unit.test.ts index 6c0f40532..713e59233 100644 --- a/src/test/common/inlineScript/metadata.unit.test.ts +++ b/src/test/common/inlineScript/metadata.unit.test.ts @@ -9,6 +9,7 @@ import * as sinon from 'sinon'; import { Uri } from 'vscode'; import { InlineScriptMetadata, + getInlineScriptSourceHash, MAX_HEADER_BYTES, matchesPythonVersion, readInlineScriptMetadata, @@ -93,6 +94,62 @@ suite('inlineScriptMetadata', () => { assert.deepStrictEqual(md.tool, { mybuild: { extra: 'thing' } }); }); + test('fingerprints dependencies surrounded by marker text inside valid TOML strings', () => { + const text = script([ + '# /// script', + '# note = """', + '# ///', + '# """', + '# dependencies = ["requests"]', + '# other = """', + '# /// script', + '# """', + '# ///', + ]); + const original = readInlineScriptMetadata(text); + const changed = readInlineScriptMetadata(text.replace('requests', 'httpx')); + assert.ok(original?.sourceHash); + assert.ok(changed?.sourceHash); + assert.deepStrictEqual(original.dependencies, ['requests']); + assert.deepStrictEqual(changed.dependencies, ['httpx']); + assert.notStrictEqual(original.sourceHash, changed.sourceHash); + }); + + test('fingerprints every non-newline character of valid metadata with embedded markers', () => { + const fixtures = [ + script(['# /// script', '# requires-python = ">=3.11"', '# dependencies = ["requests"]', '# ///']), + script([ + '# /// script', + '# note = """', + '# ///', + '# """', + '# dependencies = ["requests"]', + '# other = """', + '# /// script', + '# /// script', + '# content', + '# """', + '# ///', + ]), + ]; + for (const source of fixtures) { + const metadata = readInlineScriptMetadata(source); + assert.ok(metadata?.sourceHash); + for (let offset = metadata.sourceRange!.start; offset < metadata.sourceRange!.end; offset += 1) { + if (source[offset] === '\n' || source[offset] === '\r') { + continue; + } + const replacement = source[offset] === 'x' ? 'y' : 'x'; + const changed = source.slice(0, offset) + replacement + source.slice(offset + 1); + assert.notStrictEqual( + getInlineScriptSourceHash(changed), + metadata.sourceHash, + `Fingerprint omitted metadata character at ${offset}`, + ); + } + } + }); + test('multiple `script` blocks returns undefined and logs a warning', () => { const text = script([ '# /// script', diff --git a/src/test/common/inlineScript/routingRegistry.unit.test.ts b/src/test/common/inlineScript/routingRegistry.unit.test.ts index c05f907bb..0e353afd1 100644 --- a/src/test/common/inlineScript/routingRegistry.unit.test.ts +++ b/src/test/common/inlineScript/routingRegistry.unit.test.ts @@ -15,6 +15,21 @@ const METADATA = { }; suite('InlineScriptRoutingRegistry', () => { + test('publishes interpreter versions only for validated associations', () => { + const registry = new InlineScriptRoutingRegistry(); + const uri = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + const versions: (string | undefined)[] = []; + registry.onDidChangeEnvironmentVersion(() => versions.push(registry.getEnvironmentVersion(uri))); + registry.setMetadata(uri, METADATA); + registry.setValidatedAssociation(uri, true, '3.12.4'); + registry.setValidatedAssociation(uri, true, '3.12.4'); + registry.setValidatedAssociation(uri, true, '3.13.1'); + registry.setValidatedAssociation(uri, false); + assert.deepStrictEqual(versions, ['3.12.4', '3.13.1', undefined]); + assert.strictEqual(registry.getEnvironmentVersion(uri), undefined); + registry.dispose(); + }); + test('temporary unavailability preserves routing and notifies only on availability changes', () => { const registry = new InlineScriptRoutingRegistry(); const uri = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); @@ -84,6 +99,29 @@ suite('InlineScriptRoutingRegistry', () => { registry.dispose(); }); + test('temporarily suppresses routing while live metadata differs and restores it on Undo', () => { + const registry = new InlineScriptRoutingRegistry(); + const uri = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + const routeabilityEvents: boolean[] = []; + registry.onDidChangeRouteability((event) => routeabilityEvents.push(event.routeable)); + registry.setMetadata(uri, { ...METADATA, sourceHash: 'saved-block' }); + registry.setValidatedAssociation(uri, true, '3.12.4'); + + registry.setLiveMetadataMatchesSaved(uri, false); + + assert.strictEqual(registry.shouldRoute(uri), false); + assert.strictEqual(registry.hasValidatedAssociation(uri), true); + assert.strictEqual(registry.getMetadata(uri)?.sourceHash, 'saved-block'); + assert.strictEqual(registry.getEnvironmentVersion(uri), undefined); + + registry.setLiveMetadataMatchesSaved(uri, true); + + assert.strictEqual(registry.shouldRoute(uri), true); + assert.strictEqual(registry.getEnvironmentVersion(uri), '3.12.4'); + assert.deepStrictEqual(routeabilityEvents, [true, false, true]); + registry.dispose(); + }); + test('keeps metadata revisions monotonic after an empty state is removed', () => { const registry = new InlineScriptRoutingRegistry(); const uri = Uri.file('/workspace/script.py'); diff --git a/src/test/features/inlineScript/codeLens.unit.test.ts b/src/test/features/inlineScript/codeLens.unit.test.ts index acae445f0..f40cc0ec2 100644 --- a/src/test/features/inlineScript/codeLens.unit.test.ts +++ b/src/test/features/inlineScript/codeLens.unit.test.ts @@ -2,32 +2,19 @@ // Licensed under the MIT License. import assert from 'assert'; +import * as path from 'path'; import * as sinon from 'sinon'; -import { Position, TextDocument, Uri } from 'vscode'; -import { InlineScriptMetadata } from '../../../common/inlineScript/metadata'; +import { Uri } from 'vscode'; +import { MAX_HEADER_BYTES, readInlineScriptMetadata } from '../../../common/inlineScript/metadata'; import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routingRegistry'; -import { InlineScriptCodeLensProvider, READY_CONFIRMATION_TIMEOUT_MS } from '../../../features/inlineScript/codeLens'; +import { InlineScriptCodeLensProvider } from '../../../features/inlineScript/codeLens'; +import { MockDocument } from '../../mocks/mockDocument'; const SETUP_COMMAND = 'python-envs.setupInlineScriptEnv'; - -function makeMetadata(): InlineScriptMetadata { - return { - dependencies: ['requests'], - range: { start: 0, end: 24 }, - sourceRange: { start: 0, end: 24 }, - }; -} - -function makeDocument(uri: Uri, isDirty = false): TextDocument { - return { - uri, - isDirty, - positionAt: (offset: number) => new Position(0, offset), - } as unknown as TextDocument; -} +const SCRIPT = '# /// script\n# dependencies = ["requests"]\n# ///\n\nprint("hello")\n'; suite('Inline script CodeLens provider', () => { - const scriptUri = Uri.file('/workspace/app.py'); + const scriptUri = Uri.file(path.join(process.cwd(), 'lens-tests', 'app.py')); let routing: InlineScriptRoutingRegistry; let provider: InlineScriptCodeLensProvider; @@ -39,130 +26,210 @@ suite('Inline script CodeLens provider', () => { teardown(() => { provider.dispose(); routing.dispose(); + sinon.restore(); }); - test('shows no CodeLens when the file has no saved inline metadata', () => { - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); - assert.strictEqual(lenses.length, 0); - }); - - test('shows a setup CodeLens when metadata exists but no environment is associated', () => { - routing.setMetadata(scriptUri, makeMetadata()); - - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); - + function document(text = SCRIPT, dirty = false, uri = scriptUri): MockDocument { + const doc = new MockDocument(text, uri.fsPath, async () => true); + sinon.stub(doc, 'uri').get(() => uri); + sinon.stub(doc, 'isDirty').get(() => dirty); + return doc; + } + + function ready(text = SCRIPT, version: string | undefined = '3.12.4', uri = scriptUri): void { + const metadata = readInlineScriptMetadata(text); + assert.ok(metadata); + routing.setMetadata(uri, metadata); + routing.setValidatedAssociation(uri, true, version); + } + + function lens(text = SCRIPT, dirty = false, uri = scriptUri) { + const lenses = provider.provideCodeLenses(document(text, dirty, uri), {} as never); assert.strictEqual(lenses.length, 1); - assert.strictEqual(lenses[0].command?.command, SETUP_COMMAND); - assert.deepStrictEqual(lenses[0].command?.arguments, [scriptUri]); - }); + return lenses[0]; + } - test('shows no CodeLens while the document has unsaved changes', () => { - routing.setMetadata(scriptUri, makeMetadata()); - - const lenses = provider.provideCodeLenses(makeDocument(scriptUri, true), {} as never); - - assert.strictEqual(lenses.length, 0); + test('does not decorate ordinary Python files', () => { + assert.deepStrictEqual(provider.provideCodeLenses(document('print("hello")'), {} as never), []); }); - test('hides the CodeLens once a validated association makes the script routeable', () => { - routing.setMetadata(scriptUri, makeMetadata()); - routing.setValidatedAssociation(scriptUri, true); - assert.strictEqual(routing.shouldRoute(scriptUri), true); - - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); - - assert.strictEqual(lenses.length, 0); + test('offers setup for a newly typed block before detection or saving', () => { + const result = lens(SCRIPT, true); + assert.strictEqual(result.command?.command, SETUP_COMMAND); + assert.deepStrictEqual(result.command?.arguments, [scriptUri]); + assert.strictEqual(routing.getMetadata(scriptUri), undefined, 'presentation must not seed saved routing'); }); - test('offers setup while a selected environment is temporarily unavailable and hides it on recovery', () => { - routing.setMetadata(scriptUri, makeMetadata()); - routing.setValidatedAssociation(scriptUri, true); - let refreshes = 0; - const subscription = provider.onDidChangeCodeLenses(() => { - refreshes += 1; + for (const text of [ + '# /// script', + '# /// script\n# dependencies = [', + '# /// script\n# dependencies = ["requests",]\nnot_a_comment\n# ///', + '# /// script\n# dependencies = 1\n# ///', + '# /// script \n# dependencies = []\n# /// ', + ' # /// script\n# broken TOML\n# ///', + ]) { + test(`keeps setup available for malformed metadata: ${JSON.stringify(text)}`, () => { + assert.strictEqual(lens(text, true).command?.command, SETUP_COMMAND); + assert.strictEqual(lens(text).command?.command, SETUP_COMMAND); }); + } - routing.setEnvironmentUnavailable(scriptUri, true); - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); - assert.strictEqual(lenses.length, 1); - assert.strictEqual(lenses[0].command?.command, SETUP_COMMAND); - assert.strictEqual(routing.shouldRoute(scriptUri), true); - routing.setEnvironmentUnavailable(scriptUri, false); - - assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0); - assert.strictEqual(refreshes, 2); - subscription.dispose(); + test('always shows the ready interpreter for a validated unchanged block', () => { + ready(); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.12.4)'); + assert.strictEqual(lens().command?.command, ''); }); - test('refreshes CodeLenses when routing state changes', () => { - let fireCount = 0; - const sub = provider.onDidChangeCodeLenses(() => (fireCount += 1)); + test('does not expire the ready label', () => { + const clock = sinon.useFakeTimers(); + ready(); + clock.tick(60_000); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.12.4)'); + assert.strictEqual(clock.countTimers(), 0); + }); - routing.setMetadata(scriptUri, makeMetadata()); - routing.setValidatedAssociation(scriptUri, true); + test('shows the ready label when the association was restored before the provider was created', () => { + provider.dispose(); + ready(SCRIPT, '3.13.2.final.0'); + provider = new InlineScriptCodeLensProvider(routing, SETUP_COMMAND); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.13.2)'); + }); - sub.dispose(); - assert.ok(fireCount >= 1, 'onDidChangeCodeLenses should fire when routing state changes'); + test('omits an unknown interpreter version without hiding the ready label', () => { + ready(SCRIPT, ''); + assert.strictEqual(lens().command?.title, 'Script environment ready'); }); - suite('post-setup confirmation', () => { - let clock: sinon.SinonFakeTimers; + test('keeps ready visible during unsaved body-only edits', () => { + ready(); + assert.strictEqual(lens(SCRIPT.replace('hello', 'changed body'), true).command?.command, ''); + }); - setup(() => { - clock = sinon.useFakeTimers(); - routing.setMetadata(scriptUri, makeMetadata()); - routing.setValidatedAssociation(scriptUri, true); - }); + test('offers setup immediately for raw block edits even before routing catches up', () => { + ready(); + assert.strictEqual(lens(SCRIPT.replace('requests', 'Requests'), true).command?.command, SETUP_COMMAND); + assert.strictEqual(routing.shouldRoute(scriptUri), true, 'the lens must not change interpreter routing'); + }); - teardown(() => clock.restore()); + test('keeps setup visible after the detector clears dirty metadata', () => { + ready(); + routing.clearMetadata(scriptUri); + routing.setValidatedAssociation(scriptUri, false); + assert.strictEqual(lens(SCRIPT.replace('"requests"', '"requests'), true).command?.command, SETUP_COMMAND); + }); - test('replaces the hidden setup lens with a non-clickable confirmation naming the version', () => { - provider.noteEnvironmentReady(scriptUri, '3.12.4'); + test('returns to ready after unchanged requirements are saved and revalidated', () => { + ready(); + const edited = SCRIPT.replace('requests', 'Requests'); + assert.strictEqual(lens(edited, true).command?.command, SETUP_COMMAND); + ready(edited); + assert.strictEqual(lens(edited).command?.command, ''); + }); - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); + test('does not show ready for an additional unsaved script block', () => { + ready(); + assert.strictEqual(lens(`${SCRIPT}\n# /// script\n# ///`, true).command?.command, SETUP_COMMAND); + }); - assert.strictEqual(lenses.length, 1); - assert.strictEqual(lenses[0].command?.title, 'Script environment ready (Python 3.12.4)'); - assert.strictEqual(lenses[0].command?.command, '', 'the confirmation must not be clickable'); + for (const invalid of [ + `${SCRIPT}\n# /// script\n#bad`, + SCRIPT.replace('["requests"]', '["requests", ""]'), + SCRIPT.replace('# dependencies', '# requires-python = "invalid"\n# dependencies'), + ]) { + test(`keeps setup after malformed metadata is saved over a ready association: ${JSON.stringify(invalid)}`, () => { + ready(); + const parsed = readInlineScriptMetadata(invalid); + assert.ok(parsed, 'the tolerant parser intentionally retains usable metadata'); + routing.setMetadata(scriptUri, parsed); + routing.setValidatedAssociation(scriptUri, true, '3.12.4'); + assert.strictEqual(lens(invalid).command?.command, SETUP_COMMAND); }); + } - test('omits the version when none was resolved', () => { - provider.noteEnvironmentReady(scriptUri, undefined); + test('shows ready when the editor normalizes saved mixed line endings', () => { + const disk = '# /// script\r\n# dependencies = ["requests"]\n# ///\rprint("hello")'; + ready(disk); + assert.strictEqual(lens(disk.replace(/\r\n?/g, '\n')).command?.command, ''); + }); - const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); + test('offers setup if a preceding non-script opener hides the previously validated block', () => { + const original = `# note\n${SCRIPT}`; + ready(original); + assert.strictEqual(lens(original.replace('# note', '# /// other'), true).command?.command, SETUP_COMMAND); + }); - assert.strictEqual(lenses.length, 1); - assert.strictEqual(lenses[0].command?.title, 'Script environment ready'); - }); + test('keeps ready for body edits following an ignored metadata example', () => { + const original = `${SCRIPT}\n"""\n# /// script\n# dependencies = ["example"]\n"""\nprint("body")`; + ready(original); + assert.strictEqual(lens(original.replace('"body"', '"changed"'), true).command?.command, ''); + }); - test('expires on its own and refreshes so the lens disappears', () => { - provider.noteEnvironmentReady(scriptUri, '3.12.4'); - let fireCount = 0; - const sub = provider.onDidChangeCodeLenses(() => (fireCount += 1)); + test('keeps a lens when the closing marker is removed', () => { + ready(); + assert.strictEqual(lens(SCRIPT.replace('# ///\n', ''), true).command?.command, SETUP_COMMAND); + }); - clock.tick(READY_CONFIRMATION_TIMEOUT_MS + 1); - sub.dispose(); + test('removes the lens when the script block is removed', () => { + ready(); + assert.deepStrictEqual(provider.provideCodeLenses(document('print("hello")', true), {} as never), []); + }); - assert.strictEqual(fireCount, 1, 'expiry must refresh the lenses'); - assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0); - }); + test('anchors the lens to the live marker with BOM and CRLF', () => { + const text = `\uFEFF#!/usr/bin/env python\r\n${SCRIPT.replace(/\n/g, '\r\n')}`; + ready(text); + const result = lens(text); + assert.strictEqual(result.range.start.line, 1); + assert.strictEqual(result.range.start.character, 0); + assert.strictEqual(result.command?.command, ''); + }); - test('shows nothing for a routed script that was not just set up', () => { - assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0); - }); + test('offers retry during temporary unavailability and restores ready on recovery', () => { + ready(); + routing.setEnvironmentUnavailable(scriptUri, true); + assert.strictEqual(lens().command?.command, SETUP_COMMAND); + routing.setEnvironmentUnavailable(scriptUri, false); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.12.4)'); + }); - test('stays hidden while the document is dirty', () => { - provider.noteEnvironmentReady(scriptUri, '3.12.4'); + test('offers setup after an environment is invalidated', () => { + ready(); + routing.setValidatedAssociation(scriptUri, false); + assert.strictEqual(lens().command?.command, SETUP_COMMAND); + }); - assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri, true), {} as never).length, 0); - }); + test('keeps two scripts and their interpreter versions independent', () => { + const other = Uri.file(path.join(process.cwd(), 'lens-tests', 'other.py')); + ready(); + ready(SCRIPT, '3.11.9', other); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.12.4)'); + assert.strictEqual(lens(SCRIPT, true, other).command?.title, 'Script environment ready (Python 3.11.9)'); + }); - test('does not leak timers past disposal', () => { - provider.noteEnvironmentReady(scriptUri, '3.12.4'); + test('refreshes when a validated interpreter version changes without a routeability change', () => { + ready(); + const changed = sinon.spy(); + provider.onDidChangeCodeLenses(changed); + routing.setValidatedAssociation(scriptUri, true, '3.13.1'); + sinon.assert.calledOnce(changed); + assert.strictEqual(lens().command?.title, 'Script environment ready (Python 3.13.1)'); + }); - provider.dispose(); + test('does not decorate resources unsupported by inline setup', () => { + for (const uri of [Uri.parse('untitled:app.py'), scriptUri.with({ path: `${scriptUri.path}i` })]) { + assert.deepStrictEqual(provider.provideCodeLenses(document(SCRIPT, false, uri), {} as never), []); + } + assert.deepStrictEqual( + provider.provideCodeLenses(document(`${'# padding\n'.repeat(MAX_HEADER_BYTES)}${SCRIPT}`), {} as never), + [], + ); + }); - assert.doesNotThrow(() => clock.tick(READY_CONFIRMATION_TIMEOUT_MS + 1)); - }); + test('stops publishing after disposal', () => { + const changed = sinon.spy(); + provider.onDidChangeCodeLenses(changed); + provider.dispose(); + ready(); + sinon.assert.notCalled(changed); + assert.deepStrictEqual(provider.provideCodeLenses(document(), {} as never), []); }); }); diff --git a/src/test/features/inlineScript/lazyDetector.unit.test.ts b/src/test/features/inlineScript/lazyDetector.unit.test.ts index 37ae03447..fa524776e 100644 --- a/src/test/features/inlineScript/lazyDetector.unit.test.ts +++ b/src/test/features/inlineScript/lazyDetector.unit.test.ts @@ -470,7 +470,202 @@ suite('InlineScriptLazyDetector', () => { detector.dispose(); }); - test('invalidates routing for a CRLF dependency edit using source offsets', async () => { + test('preserves routing when a whole-document body edit leaves the block unchanged', async () => { + const uri = Uri.file(path.join(process.cwd(), 'body-edit.py')); + const original = '# /// script\n# dependencies = []\n# ///\n\nprint("old")'; + const changed = original.replace('"old"', '"new"'); + const metadata = ism.readInlineScriptMetadata(original)!; + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [{ range: undefined as never, rangeOffset: 0, rangeLength: original.length, text: changed }], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + assert.strictEqual(routingRegistry.getEnvironmentVersion(uri), '3.12.4'); + assert.deepStrictEqual(routingRegistry.getMetadata(uri), metadata); + detector.dispose(); + }); + + test('still invalidates a whole-document edit when the raw metadata changes', async () => { + const uri = Uri.file(path.join(process.cwd(), 'header-edit.py')); + const original = '# /// script\n# dependencies = []\n# ///\n'; + const changed = original.replace('dependencies = []', 'dependencies=[]'); + readMetadataStub.resolves(ism.readInlineScriptMetadata(original)); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [{ range: undefined as never, rangeOffset: 0, rangeLength: original.length, text: changed }], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + detector.dispose(); + }); + + test('restores routeability when an unsaved metadata edit is undone to the saved fingerprint', async () => { + const uri = Uri.file(path.join(process.cwd(), 'header-undo.py')); + const original = '# /// script\n# dependencies = []\n# ///\n'; + const changed = original.replace('dependencies = []', 'dependencies = ["requests"]'); + const metadata = ism.readInlineScriptMetadata(original); + assert.ok(metadata?.sourceHash); + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + const changeDocument = (text: string) => { + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => text }, + contentChanges: [{ range: undefined as never, rangeOffset: 0, rangeLength: original.length, text }], + reason: undefined, + }); + }; + + changeDocument(changed); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + assert.strictEqual(routingRegistry.hasValidatedAssociation(uri), true); + assert.deepStrictEqual(routingRegistry.getMetadata(uri), metadata); + + changeDocument(original); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + assert.strictEqual(routingRegistry.getEnvironmentVersion(uri), '3.12.4'); + detector.dispose(); + }); + + test('does not restore stale routing when a pending read completes after a live metadata edit', async () => { + const uri = Uri.file(path.join(process.cwd(), 'pending-header-read.py')); + const original = '# /// script\n# dependencies = ["requests"]\n# ///\n'; + const changed = original.replace('requests', 'httpx'); + const metadata = ism.readInlineScriptMetadata(original); + assert.ok(metadata?.sourceHash); + const staleRead = createDeferred(); + readMetadataStub.returns(staleRead.promise); + routingRegistry.setMetadata(uri, metadata); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + const detector = createDetector(); + + const opening = fireOpen(uri); + assert.strictEqual(readMetadataStub.callCount, 1); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [ + { range: undefined as never, rangeOffset: original.indexOf('requests'), rangeLength: 8, text: 'httpx' }, + ], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + + staleRead.resolve(metadata); + await opening; + + assert.deepStrictEqual(routingRegistry.getMetadata(uri), metadata); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + detector.dispose(); + }); + + test('invalidates dependency edits after an unchanged block moves beyond its old offsets', async () => { + const uri = Uri.file(path.join(process.cwd(), 'moved-header.py')); + const original = '# /// script\n# dependencies = ["requests"]\n# ///\n'; + const metadata = ism.readInlineScriptMetadata(original)!; + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + const prefix = '# leading comment\n'.repeat(20); + const moved = prefix + original; + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => moved }, + contentChanges: [{ range: undefined as never, rangeOffset: 0, rangeLength: 0, text: prefix }], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + const dependencyOffset = moved.indexOf('requests'); + assert.ok(dependencyOffset > metadata.sourceRange!.end); + const changed = moved.replace('requests', 'httpx'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [{ range: undefined as never, rangeOffset: dependencyOffset, rangeLength: 8, text: 'httpx' }], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + detector.dispose(); + }); + + test('invalidates changed dependencies between embedded TOML closing and opening markers', async () => { + const uri = Uri.file(path.join(process.cwd(), 'embedded-markers.py')); + const original = [ + '# /// script', + '# note = """', + '# ///', + '# """', + '# dependencies = ["requests"]', + '# other = """', + '# /// script', + '# """', + '# ///', + ].join('\n'); + const metadata = ism.readInlineScriptMetadata(original); + assert.ok(metadata?.sourceHash); + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => original.replace('requests', 'httpx') }, + contentChanges: [ + { range: undefined as never, rangeOffset: original.indexOf('requests'), rangeLength: 8, text: 'httpx' }, + ], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + detector.dispose(); + }); + + test('invalidates when a preceding non-script opener changes structural block selection', async () => { + const uri = Uri.file(path.join(process.cwd(), 'preceding-marker.py')); + const original = '# note\n# /// script\n# dependencies = ["requests"]\n# ///'; + const changed = original.replace('# note', '# /// other'); + const metadata = ism.readInlineScriptMetadata(original); + assert.ok(metadata?.sourceHash); + assert.strictEqual(ism.readInlineScriptMetadata(changed), undefined); + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [{ range: undefined as never, rangeOffset: 0, rangeLength: 6, text: '# /// other' }], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + detector.dispose(); + }); + + test('keeps routing for body edits after an ignored unfinished metadata example', async () => { + const uri = Uri.file(path.join(process.cwd(), 'body-example.py')); + const original = '# /// script\n# dependencies = []\n# ///\n\n"""\n# /// script\n# dependencies = ["example"]\n"""\nprint("body")'; + const metadata = ism.readInlineScriptMetadata(original); + assert.ok(metadata?.sourceHash); + readMetadataStub.resolves(metadata); + const detector = createDetector(); + await fireOpen(uri); + routingRegistry.setValidatedAssociation(uri, true, '3.12.4'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => original.replace('"body"', '"changed"') }, + contentChanges: [ + { range: undefined as never, rangeOffset: original.indexOf('"body"'), rangeLength: 6, text: '"changed"' }, + ], + reason: undefined, + }); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + detector.dispose(); + }); + + test('invalidates routing for a CRLF dependency edit beyond normalized offsets', async () => { const uri = Uri.file(path.resolve('/elsewhere/crlf.py')); const source = [ '# /// script', @@ -493,9 +688,21 @@ suite('InlineScriptLazyDetector', () => { await fireOpen(uri); routingRegistry.setValidatedAssociation(uri, true); - fireChange(uri, makeContentChanges(dependencyOffset)); + const changed = source.replace('requests', 'httpx'); + changeListener!({ + document: { ...makeDoc(uri), isDirty: true, getText: () => changed }, + contentChanges: [ + { + range: undefined as never, + rangeOffset: source.indexOf('requests'), + rangeLength: 'requests'.length, + text: 'httpx', + }, + ], + reason: undefined, + }); - assert.strictEqual(routingRegistry.getMetadata(uri), undefined); + assert.deepStrictEqual(routingRegistry.getMetadata(uri), metadata); assert.strictEqual(routingRegistry.shouldRoute(uri), false); detector.dispose(); }); diff --git a/src/test/features/inlineScript/setupEnvironment.unit.test.ts b/src/test/features/inlineScript/setupEnvironment.unit.test.ts index fd4908645..dee2d08be 100644 --- a/src/test/features/inlineScript/setupEnvironment.unit.test.ts +++ b/src/test/features/inlineScript/setupEnvironment.unit.test.ts @@ -9,8 +9,12 @@ import { PythonEnvironment } from '../../../api'; import { INLINE_SCRIPT_MANAGER_ID } from '../../../common/constants'; import { InlineScriptMetadata } from '../../../common/inlineScript/metadata'; import * as metadataApi from '../../../common/inlineScript/metadata'; -import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routingRegistry'; +import { + getInlineScriptMetadataRoutingIdentity, + InlineScriptRoutingRegistry, +} from '../../../common/inlineScript/routingRegistry'; import { InlineScriptStrings } from '../../../common/localize'; +import { createDeferred } from '../../../common/utils/deferred'; import * as winapi from '../../../common/window.apis'; import * as wapi from '../../../common/workspace.apis'; import { @@ -20,8 +24,10 @@ import { setupInlineScriptEnvironmentHandler, } from '../../../features/inlineScript/setupEnvironment'; import * as extensionVersionCheck from '../../../features/inlineScript/extensionVersionCheck'; +import { InlineScriptCodeLensProvider } from '../../../features/inlineScript/codeLens'; import type { EnvironmentManagers } from '../../../features/envManagers'; import { InternalEnvironmentManager } from '../../../managers/common/registeredManagers'; +import { MockDocument } from '../../mocks/mockDocument'; function makeEnv(): PythonEnvironment { return { @@ -328,7 +334,9 @@ suite('setupInlineScriptEnvironmentHandler', () => { let errorStub: sinon.SinonStub; let saveStub: sinon.SinonStub; let promptStub: sinon.SinonStub; - let readySpy: sinon.SinonStub; + let warningStub: sinon.SinonStub; + let readHeaderStub: sinon.SinonStub; + let currentText: string; setup(() => { em = typemoq.Mock.ofType(); @@ -339,8 +347,9 @@ suite('setupInlineScriptEnvironmentHandler', () => { openDocumentsStub = sinon.stub(wapi, 'getOpenTextDocuments').returns([]); errorStub = sinon.stub(winapi, 'showErrorMessage').resolves(undefined); sinon.stub(winapi, 'showInformationMessage').resolves(undefined); - sinon.stub(winapi, 'showWarningMessage').resolves(undefined); - readySpy = sinon.stub(); + warningStub = sinon.stub(winapi, 'showWarningMessage').resolves(undefined); + currentText = '# /// script\n# dependencies = ["requests"]\n# ///\nprint("hello")'; + readHeaderStub = sinon.stub(metadataApi, 'readInlineScriptHeaderFromFile').callsFake(async () => currentText); promptStub = sinon.stub(extensionVersionCheck, 'promptUpdateExtensionsForInlineScripts').resolves(); saveStub = sinon.stub().resolves(true); }); @@ -350,8 +359,22 @@ suite('setupInlineScriptEnvironmentHandler', () => { sinon.restore(); }); - function openDirtyDocument(isDirty = true): void { - openDocumentsStub.returns([{ uri: scriptUri, isDirty, save: saveStub }]); + function openDirtyDocument(isDirty = true) { + const document = { + uri: scriptUri, + isDirty, + version: 1, + getText: () => currentText, + save: async (): Promise => { + const saved = await saveStub(); + if (saved) { + document.isDirty = false; + } + return saved; + }, + }; + openDocumentsStub.returns([document]); + return document; } function expectEnvironmentCreated(): PythonEnvironment { @@ -427,27 +450,132 @@ suite('setupInlineScriptEnvironmentHandler', () => { sinon.assert.notCalled(errorStub); }); - test('reports the ready environment and its Python version once setup succeeds', async () => { - expectEnvironmentCreated(); + for (const malformed of [ + '# /// script', + '# /// script\n# dependencies = [\n# ///', + '# /// script\n# dependencies = "requests"\n# ///', + '# /// script\n# dependencies = [""]\n# ///', + '# /// script\n# ///\n# /// script\n# ///', + '# /// script\n# ///\n\n# /// script\n#bad', + ]) { + test(`warns on explicit setup without saving or creating malformed metadata: ${JSON.stringify(malformed)}`, async () => { + currentText = malformed; + openDirtyDocument(); + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + sinon.assert.calledOnce(warningStub); + sinon.assert.notCalled(errorStub); + sinon.assert.notCalled(saveStub); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + }); + } + + test('warns about an invalid Python requirement before installing anything', async () => { + currentText = '# /// script\n# requires-python = "not a version specifier"\n# ///'; + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + sinon.assert.calledOnceWithExactly(warningStub, InlineScriptStrings.invalidPythonRequirement); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + }); - await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri); + test('rechecks metadata after saving instead of creating from a changed invalid block', async () => { + openDirtyDocument(); + saveStub.callsFake(async () => { + currentText = '# /// script\n# dependencies = [\n# ///'; + return true; + }); + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + sinon.assert.calledOnce(saveStub); + sinon.assert.calledOnce(warningStub); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + }); - sinon.assert.calledOnceWithExactly(readySpy, scriptUri, '3.12.0'); + test('does not restore routing when an older post-save read finishes after a newer header edit', async () => { + const metadata = metadataApi.readInlineScriptMetadata(currentText)!; + routing.setMetadata(scriptUri, metadata); + routing.setValidatedAssociation(scriptUri, true, '3.12.0'); + const subscription = routing.onDidChangeMetadata((event) => { + if (event.metadata) { + routing.setValidatedAssociation(scriptUri, true, '3.12.0'); + } + }); + const document = openDirtyDocument(); + const readStarted = createDeferred(); + const read = createDeferred(); + readMetadataStub.callsFake(() => { + readStarted.resolve(); + return read.promise; + }); + const pending = setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + await readStarted.promise; + currentText = '# /// script\n# dependencies = [\n# ///'; + document.version += 1; + document.isDirty = true; + routing.clearMetadata(scriptUri); + routing.setValidatedAssociation(scriptUri, false); + read.resolve(metadata); + await pending; + assert.strictEqual(routing.shouldRoute(scriptUri), false); + assert.strictEqual(routing.getMetadata(scriptUri), undefined); + sinon.assert.calledOnce(warningStub); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + subscription.dispose(); }); - test('reports no ready environment when setup produced none', async () => { - manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.resolve(undefined)); + test('explicit setup restores metadata, routing, and ready after a clean Undo or Revert', async () => { + const metadata = metadataApi.readInlineScriptMetadata(currentText)!; + routing.setMetadata(scriptUri, metadata); + routing.setValidatedAssociation(scriptUri, true, '3.12.0'); + routing.clearMetadata(scriptUri); + routing.setValidatedAssociation(scriptUri, false); + openDirtyDocument(false); + readMetadataStub.resolves(metadata); + const environment = makeEnv(); + manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.resolve(environment)); + em.setup((m) => m.setEnvironment(scriptUri, environment)).returns(async () => { + routing.setValidatedAssociation( + scriptUri, + routing.getMetadataIdentity(scriptUri) === getInlineScriptMetadataRoutingIdentity(metadata), + environment.version, + ); + }); - await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri); + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); - sinon.assert.notCalled(readySpy); + sinon.assert.notCalled(saveStub); + assert.deepStrictEqual(routing.getMetadata(scriptUri), metadata); + assert.strictEqual(routing.shouldRoute(scriptUri), true); + const provider = new InlineScriptCodeLensProvider(routing, 'setup'); + try { + const document = new MockDocument(currentText, scriptUri.fsPath, async () => true); + const lenses = provider.provideCodeLenses(document, {} as never); + assert.strictEqual(lenses.length, 1); + assert.strictEqual(lenses[0].command?.command, ''); + assert.strictEqual(lenses[0].command?.title, 'Script environment ready (Python 3.12.0)'); + } finally { + provider.dispose(); + } }); - test('reports no ready environment when setup throws', async () => { - manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.reject(new Error('boom'))); + test('reports save exceptions instead of letting the command reject', async () => { + openDirtyDocument(); + saveStub.rejects(new Error('Cannot save')); + await assert.doesNotReject(setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri)); + sinon.assert.calledOnce(errorStub); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + }); - await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri); + test('reports an unreadable closed script without trying setup', async () => { + readHeaderStub.resolves(undefined); + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + sinon.assert.calledOnceWithExactly(errorStub, InlineScriptStrings.scriptReadFailedBeforeSetup); + sinon.assert.notCalled(warningStub); + manager.verify((m) => m.create(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.never()); + }); - sinon.assert.notCalled(readySpy); + test('accepts an empty block and creates its environment', async () => { + currentText = '# /// script\n# ///'; + expectEnvironmentCreated(); + await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri); + manager.verify((m) => m.create(scriptUri, undefined), typemoq.Times.once()); + sinon.assert.notCalled(warningStub); }); }); diff --git a/src/test/integration/inlineScriptCodeLens.integration.test.ts b/src/test/integration/inlineScriptCodeLens.integration.test.ts new file mode 100644 index 000000000..a4ef78a29 --- /dev/null +++ b/src/test/integration/inlineScriptCodeLens.integration.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as fs from 'fs/promises'; +import * as path from 'path'; +import * as vscode from 'vscode'; +import { readInlineScriptMetadata } from '../../common/inlineScript/metadata'; +import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; +import { registerInlineScriptCodeLens } from '../../features/inlineScript/codeLens'; +import { registerInlineScriptDiagnostics } from '../../features/inlineScript/diagnostics'; +import { InlineScriptLazyDetector } from '../../features/inlineScript/lazyDetector'; +import { sleep, waitForCondition } from '../testUtils'; + +const SETUP_COMMAND = 'python-envs.test.inlineScriptLensSetup'; +const SCRIPT = '# /// script\n# dependencies = []\n# ///\n\nprint("hello")\n'; + +suite('Integration: Live inline script CodeLens', function () { + this.timeout(30_000); + + let root: string; + let document: vscode.TextDocument; + let routing: InlineScriptRoutingRegistry; + let registration: vscode.Disposable; + let diagnostics: vscode.Disposable; + let detector: InlineScriptLazyDetector; + let nextFile = 0; + + suiteSetup(async () => { + const workspace = vscode.workspace.workspaceFolders?.[0]; + assert.ok(workspace, 'Inline CodeLens integration tests require an open workspace'); + root = await fs.mkdtemp(path.join(workspace.uri.fsPath, 'pep723-lens-integration-')); + }); + + setup(async () => { + routing = new InlineScriptRoutingRegistry(); + registration = registerInlineScriptCodeLens(routing, SETUP_COMMAND).disposable; + diagnostics = registerInlineScriptDiagnostics(); + detector = new InlineScriptLazyDetector(routing); + detector.activate(); + const uri = vscode.Uri.file(path.join(root, `script-${nextFile++}.py`)); + await fs.writeFile(uri.fsPath, SCRIPT); + document = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(document, { preview: false }); + await waitForCondition( + () => routing.getMetadata(document.uri) !== undefined, + 5_000, + 'The detector should read the saved script before editing', + ); + }); + + teardown(async () => { + registration.dispose(); + diagnostics.dispose(); + detector.dispose(); + routing.dispose(); + await vscode.window.showTextDocument(document); + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + }); + + suiteTeardown(async () => { + await fs.rm(root, { recursive: true, force: true }); + }); + + async function lenses(): Promise { + const all = await vscode.commands.executeCommand( + 'vscode.executeCodeLensProvider', + document.uri, + ); + return (all ?? []).filter( + (lens) => + lens.command?.command === SETUP_COMMAND || lens.command?.title.startsWith('Script environment ready'), + ); + } + + async function replace(text: string): Promise { + const edit = new vscode.WorkspaceEdit(); + edit.replace( + document.uri, + new vscode.Range(document.positionAt(0), document.positionAt(document.getText().length)), + text, + ); + assert.strictEqual(await vscode.workspace.applyEdit(edit), true); + } + + function validateSavedEnvironment(version = '3.12.4'): void { + const metadata = readInlineScriptMetadata(document.getText()); + assert.ok(metadata); + routing.setMetadata(document.uri, metadata); + routing.setValidatedAssociation(document.uri, true, version); + } + + test('offers setup for unsaved malformed metadata while diagnostics remain visible', async () => { + await replace('# /// script\n# dependencies = [\n# ///\n'); + assert.strictEqual(document.isDirty, true); + const found = await lenses(); + assert.strictEqual(found.length, 1); + assert.strictEqual(found[0].command?.command, SETUP_COMMAND); + await waitForCondition( + () => + vscode.languages.getDiagnostics(document.uri).some((diagnostic) => diagnostic.code === 'invalid-toml'), + 5_000, + 'Malformed inline metadata should retain its diagnostic', + ); + }); + + test('retains setup for an unfinished marker block before any save', async () => { + await replace('# /// script'); + assert.strictEqual((await lenses())[0]?.command?.command, SETUP_COMMAND); + await waitForCondition( + () => + vscode.languages.getDiagnostics(document.uri).some((diagnostic) => diagnostic.code === 'unterminated-block'), + 5_000, + 'Unfinished inline metadata should retain its diagnostic', + ); + }); + + test('keeps a validated ready label beyond the former five-second expiry', async () => { + validateSavedEnvironment(); + assert.strictEqual((await lenses())[0]?.command?.title, 'Script environment ready (Python 3.12.4)'); + await sleep(5_200); + const found = await lenses(); + assert.strictEqual(found.length, 1); + assert.strictEqual(found[0].command?.command, ''); + assert.strictEqual(found[0].command?.title, 'Script environment ready (Python 3.12.4)'); + }); + + test('body edits keep ready while block edits offer setup and Undo restores ready without saving', async () => { + validateSavedEnvironment(); + await replace(SCRIPT.replace('hello', 'dirty body')); + assert.strictEqual(document.isDirty, true); + assert.strictEqual((await lenses())[0]?.command?.command, ''); + await replace(SCRIPT.replace('[]', '["requests"]')); + assert.strictEqual((await lenses())[0]?.command?.command, SETUP_COMMAND); + assert.strictEqual(routing.shouldRoute(document.uri), false, 'the detector must still invalidate changed headers'); + await replace(SCRIPT); + assert.strictEqual((await lenses())[0]?.command?.command, ''); + assert.strictEqual(routing.shouldRoute(document.uri), true, 'Undo must restore the validated route'); + }); + + test('restored associations show the version and availability changes switch the label', async () => { + validateSavedEnvironment('3.13.2'); + registration.dispose(); + registration = registerInlineScriptCodeLens(routing, SETUP_COMMAND).disposable; + assert.strictEqual((await lenses())[0]?.command?.title, 'Script environment ready (Python 3.13.2)'); + routing.setEnvironmentUnavailable(document.uri, true); + assert.strictEqual((await lenses())[0]?.command?.command, SETUP_COMMAND); + routing.setEnvironmentUnavailable(document.uri, false); + assert.strictEqual((await lenses())[0]?.command?.command, ''); + }); + + test('ordinary files and removed blocks have no inline CodeLens', async () => { + validateSavedEnvironment(); + await replace('print("ordinary Python")\n'); + assert.deepStrictEqual(await lenses(), []); + }); + + test('a saved and revalidated block restores ready without an expiry timer', async () => { + validateSavedEnvironment(); + await replace(SCRIPT.replace('dependencies = []', 'dependencies=[]')); + assert.strictEqual((await lenses())[0]?.command?.command, SETUP_COMMAND); + assert.strictEqual(await document.save(), true); + validateSavedEnvironment(); + assert.strictEqual((await lenses())[0]?.command?.command, ''); + }); + + test('compares a real normalized text document against its mixed-ending disk metadata', async () => { + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + const uri = vscode.Uri.file(path.join(root, 'mixed-endings.py')); + const disk = '# /// script\r\n# dependencies = []\n# ///\rprint("hello")'; + await fs.writeFile(uri.fsPath, disk); + document = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(document); + assert.notStrictEqual(document.getText(), disk, 'VS Code should normalize the mixed line endings'); + const metadata = readInlineScriptMetadata(await fs.readFile(uri.fsPath, 'utf8')); + assert.ok(metadata); + routing.setMetadata(uri, metadata); + routing.setValidatedAssociation(uri, true, '3.12.4'); + assert.strictEqual((await lenses())[0]?.command?.title, 'Script environment ready (Python 3.12.4)'); + }); +}); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 1ded8ad5d..c73fb7a3a 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -7840,9 +7840,12 @@ suite('InlineScriptEnvManager', () => { test('recovers an old usable environment after a single stamp failure without a save', async () => { const uri = scriptUri('recover.py'); + const scriptText = '# /// script\n# requires-python = ">=3.11"\n# dependencies = ["requests"]\n# ///\n'; + const metadata = metadataReader.readInlineScriptMetadata(scriptText)!; + readMetadataStub.resolves(metadata); const environment = await createOwnedEnvironment(); await manager.set(uri, environment); - await triggerSavedMetadataChange(routingRegistry, manager, uri); + await triggerSavedMetadataChange(routingRegistry, manager, uri, metadata); setSidecar( await makeSidecar({ lastUsedAt: new Date(NOW.getTime() - 20 * 24 * 60 * 60 * 1000).toISOString(), @@ -7858,7 +7861,7 @@ suite('InlineScriptEnvManager', () => { const provider = new InlineScriptCodeLensProvider(routingRegistry, 'setup'); try { const document = new MockDocument( - '# /// script\n# dependencies = ["requests"]\n# ///\n', + scriptText, uri.fsPath, async () => true, ); @@ -7874,7 +7877,10 @@ suite('InlineScriptEnvManager', () => { await ready.promise; assert.strictEqual(await manager.get(uri), environment); - assert.strictEqual(provider.provideCodeLenses(document, {} as never).length, 0); + const lenses = provider.provideCodeLenses(document, {} as never); + assert.strictEqual(lenses.length, 1); + assert.strictEqual(lenses[0].command?.command, ''); + assert.strictEqual(lenses[0].command?.title, 'Script environment ready (Python 3.12.4)'); assert.strictEqual(writeMetaStub.callCount, 2); sinon.assert.notCalled(createWithProgressStub); } finally {