From 93672666b9076b20f4a09d0fe533aa2d80032f22 Mon Sep 17 00:00:00 2001 From: cookabc Date: Tue, 27 Oct 2026 07:03:08 +0000 Subject: [PATCH] Fix medium-priority review issues in updates, versions, and caching Plan every Update All edit from the current document so a line inserted during the registry lookup cannot move a safe bump. Compare versions with PEP 440, accept extras-only requirements files, normalize cache keys, and drop closed documents from the status bar. Co-authored-by: cookabc --- CHANGELOG.md | 15 ++ src/core/batchUpdate.ts | 84 ++++++++ src/core/packageName.ts | 23 +++ src/core/pep440.ts | 278 ++++++++++++++++++++++++++ src/core/unifiedParser.ts | 27 +-- src/core/versionAnalyzer.ts | 21 +- src/core/versionResolver.ts | 183 +++++------------ src/extension.ts | 140 +++++-------- src/providers/codeLensProvider.ts | 78 ++++++-- src/providers/hoverProvider.ts | 5 +- src/providers/versionService.ts | 8 +- src/test/__mocks__/vscode.ts | 1 + src/test/unit/basic.test.ts | 21 ++ src/test/unit/batchUpdate.test.ts | 178 +++++++++++++++++ src/test/unit/mediumReview.test.ts | 262 ++++++++++++++++++++++++ src/test/unit/parser.test.ts | 7 + src/test/unit/versionAnalyzer.test.ts | 13 ++ src/test/unit/versionResolver.test.ts | 128 ++++++++++++ src/utils/dependencyUtils.ts | 15 ++ 19 files changed, 1234 insertions(+), 253 deletions(-) create mode 100644 src/core/batchUpdate.ts create mode 100644 src/core/packageName.ts create mode 100644 src/core/pep440.ts create mode 100644 src/test/unit/batchUpdate.test.ts create mode 100644 src/test/unit/mediumReview.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2016ee1..a29f854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ All notable changes to the "Python Dependencies Updater" extension will be docum ### Fixed +- "Update All" plans safe updates from the document text after the registry + requests return. Inserting or deleting lines while versions load no longer + makes a patch bump land on the wrong line. Risky updates use that same + relocation, including edits made during the confirmation dialog. +- Removed the `[UAT-PROBE]` log that `resolveCodeLens` wrote on every lens. +- A requirements file whose lines all use extras (`requests[security]>=2.0`) + is parsed again. Brackets in an extra are not treated as a TOML table. +- Version ordering follows PEP 440. `1.0.0rc1.post1` is less than `1.0.0`, + an epoch such as `1!1.0` sorts above `2.0`, and `1.0rc1+cpu` is a + pre-release. An epoch change is reported as a major update. +- Package cache keys are PEP 503-normalized, so `foo_bar` and `foo-bar` + share one entry. Closing a requirements file drops it from the dependency + cache and from the status-bar total. Hover uses the same specifier-aware + resolution as the CodeLens. + - Upgrading a multi-constraint requirement now bumps only the lower bound or pin. `requests>=2.0,<3.0` keeps its upper bound, `foo!=1.5,>=1.0` keeps the exclusion, and `django~=4.2` keeps two-component precision instead of diff --git a/src/core/batchUpdate.ts b/src/core/batchUpdate.ts new file mode 100644 index 0000000..71be7b5 --- /dev/null +++ b/src/core/batchUpdate.ts @@ -0,0 +1,84 @@ +/** + * Plan "Update All" edits against the document as it is now. + * + * Version lookups can take long enough for the user to insert or delete + * lines. Edits must be built from a fresh parse, not from the line numbers + * captured before those requests. Safe and risky updates go through the + * same relocation so a patch bump cannot land on the wrong line. + */ + +import { planLineUpdate } from "../utils/dependencyUtils"; +import type { AnyDependency } from "./unifiedParser"; +import { parseDependencies } from "./unifiedParser"; + +export interface BatchUpdate { + dep: AnyDependency; + newVersion: string; + kind: "safe" | "risky"; +} + +export interface PlannedEdit { + line: number; + startColumn: number; + endColumn: number; + replacement: string; + packageName: string; + kind: "safe" | "risky"; +} + +function dependencyKey(dep: AnyDependency): string { + const extra = "extra" in dep ? (dep.extra ?? "") : ""; + return `${dep.packageName}|${extra}`; +} + +/** + * Match each update to the current document by package name and extra, + * in document order, and plan the rewrite from the current line text. + * A dependency that disappeared while versions were loading is skipped. + */ +export function planRelocatedEdits(options: { + updates: BatchUpdate[]; + fileName: string; + currentText: string; + isTOML: boolean; +}): PlannedEdit[] { + const currentDeps = parseDependencies(options.fileName, options.currentText); + const byKey = new Map(); + for (const dep of currentDeps) { + const key = dependencyKey(dep); + const list = byKey.get(key) ?? []; + list.push(dep); + byKey.set(key, list); + } + + const lines = options.currentText.split("\n"); + const edits: PlannedEdit[] = []; + for (const update of options.updates) { + const target = byKey.get(dependencyKey(update.dep))?.shift(); + if (!target) { + continue; + } + const lineText = lines[target.line]; + if (lineText === undefined) { + continue; + } + const planned = planLineUpdate( + lineText, + update.newVersion, + options.isTOML, + target.startColumn, + ); + if (!planned) { + continue; + } + edits.push({ + line: target.line, + startColumn: planned.match.startIndex, + endColumn: planned.match.endIndex, + replacement: planned.replacement, + packageName: target.packageName, + kind: update.kind, + }); + } + return edits; +} diff --git a/src/core/packageName.ts b/src/core/packageName.ts new file mode 100644 index 0000000..caac921 --- /dev/null +++ b/src/core/packageName.ts @@ -0,0 +1,23 @@ +/** + * Package name normalization shared by the in-memory and persisted caches. + * PEP 503: lowercase, and collapse every run of `-`, `_`, or `.` to one `-`. + */ + +const DEFAULT_REGISTRY = "https://pypi.org"; + +export function normalizePackageName(name: string): string { + const base = name.split("[")[0]?.trim() ?? ""; + return base.toLowerCase().replace(/[-_.]+/g, "-"); +} + +/** + * Cache key for a package on a registry. `Foo_Bar` and `foo-bar` share one + * entry; a trailing slash on the registry does not create a second entry. + */ +export function versionCacheKey( + packageName: string, + registryUrl?: string, +): string { + const registry = (registryUrl || DEFAULT_REGISTRY).replace(/\/+$/, ""); + return `${registry}|${normalizePackageName(packageName)}`; +} diff --git a/src/core/pep440.ts b/src/core/pep440.ts new file mode 100644 index 0000000..db4f9ff --- /dev/null +++ b/src/core/pep440.ts @@ -0,0 +1,278 @@ +/** + * PEP 440 version parsing and ordering. + * + * Sort key, following the packaging reference implementation: + * (epoch, release, pre, post, dev, local) + * with trailing zeros stripped from the release, dev-only releases sorting + * before alphas, and local segments sorting after the same version without one. + */ + +const NEG = Number.NEGATIVE_INFINITY; +const POS = Number.POSITIVE_INFINITY; + +/** + * release, then optional pre, post, dev, and local. + * Separators `.`, `-`, and `_` are equivalent. `c` / `pre` / `preview` are rc. + * A bare `-N` tail (`1.0-1`) is an implicit post release. + */ +const VERSION_PATTERN = + /^v?(?:(\d+)!)?(\d+(?:\.\d+)*)(?:[-_.]?(alpha|beta|preview|pre|rc|a|b|c)[-_.]?(\d*))?(?:(?:-(\d+))|(?:[-_.]?(post|rev|r)[-_.]?(\d*)))?(?:[-_.]?dev[-_.]?(\d*))?(?:\+([0-9a-z]+(?:[-_.][0-9a-z]+)*))?$/i; + +export interface Pep440Version { + epoch: number; + /** Numeric release segments, without trailing-zero normalization. */ + release: number[]; + /** [rank, number]. Rank is 0 alpha, 1 beta, 2 rc. */ + pre: [number, number] | null; + post: number | null; + dev: number | null; + local: Array | null; +} + +function preRank(letter: string): number { + const normalized = letter.toLowerCase(); + if (normalized === "a" || normalized === "alpha") { + return 0; + } + if (normalized === "b" || normalized === "beta") { + return 1; + } + return 2; +} + +function parseLocal(local: string): Array { + return local.split(/[-_.]/).map((part) => { + if (/^\d+$/.test(part)) { + return parseInt(part, 10); + } + return part.toLowerCase(); + }); +} + +/** + * Parse a PEP 440 version. Returns null when the string is not a version + * (callers keep their previous fallback rather than throwing). + */ +export function parsePep440(version: string): Pep440Version | null { + const match = VERSION_PATTERN.exec(version.trim()); + if (!match) { + return null; + } + + const release = match[2].split(".").map((part) => parseInt(part, 10)); + const pre = + match[3] === undefined + ? null + : ([preRank(match[3]), match[4] ? parseInt(match[4], 10) : 0] as [ + number, + number, + ]); + + let post: number | null = null; + if (match[5] !== undefined) { + post = parseInt(match[5], 10); + } else if (match[6] !== undefined) { + post = match[7] ? parseInt(match[7], 10) : 0; + } + + const dev = + match[8] === undefined ? null : match[8] ? parseInt(match[8], 10) : 0; + const local = match[9] === undefined ? null : parseLocal(match[9]); + + return { + epoch: match[1] ? parseInt(match[1], 10) : 0, + release, + pre, + post, + dev, + local, + }; +} + +export function isPep440Prerelease(version: string): boolean { + const parsed = parsePep440(version); + if (!parsed) { + return false; + } + // A post-release of an rc (`1.0.0rc1.post1`) is still a pre-release. + // A developmental release is too, including `1.0.post1.dev1`. + // A local label (`1.0+cpu`) does not make a final release a pre-release. + return parsed.pre !== null || parsed.dev !== null; +} + +function stripTrailingZeros(release: number[]): number[] { + let end = release.length; + while (end > 1 && release[end - 1] === 0) { + end--; + } + return release.slice(0, end); +} + +function preKey(version: Pep440Version): [number, number] { + if (version.pre) { + return version.pre; + } + // `1.0.dev1` sorts before `1.0a1`. A post+dev release does not. + if (version.dev !== null && version.post === null) { + return [NEG, 0]; + } + return [POS, 0]; +} + +function comparePair(left: [number, number], right: [number, number]): number { + if (left[0] !== right[0]) { + return left[0] < right[0] ? -1 : 1; + } + if (left[1] !== right[1]) { + return left[1] < right[1] ? -1 : 1; + } + return 0; +} + +function compareLocalPart( + left: number | string, + right: number | string, +): number { + const leftNum = typeof left === "number"; + const rightNum = typeof right === "number"; + if (leftNum && rightNum) { + if (left !== right) { + return left < right ? -1 : 1; + } + return 0; + } + // Numeric local segments sort after alphanumeric ones. + if (leftNum) { + return 1; + } + if (rightNum) { + return -1; + } + if (left !== right) { + return left < right ? -1 : 1; + } + return 0; +} + +function compareLocal( + left: Array | null, + right: Array | null, +): number { + if (left === null && right === null) { + return 0; + } + if (left === null) { + return -1; + } + if (right === null) { + return 1; + } + const length = Math.max(left.length, right.length); + for (let i = 0; i < length; i++) { + if (i >= left.length) { + return -1; + } + if (i >= right.length) { + return 1; + } + const cmp = compareLocalPart(left[i], right[i]); + if (cmp !== 0) { + return cmp; + } + } + return 0; +} + +export function compareParsed( + left: Pep440Version, + right: Pep440Version, +): number { + if (left.epoch !== right.epoch) { + return left.epoch < right.epoch ? -1 : 1; + } + + const leftRelease = stripTrailingZeros(left.release); + const rightRelease = stripTrailingZeros(right.release); + const length = Math.max(leftRelease.length, rightRelease.length); + for (let i = 0; i < length; i++) { + const leftPart = leftRelease[i] ?? 0; + const rightPart = rightRelease[i] ?? 0; + if (leftPart !== rightPart) { + return leftPart < rightPart ? -1 : 1; + } + } + + const pre = comparePair(preKey(left), preKey(right)); + if (pre !== 0) { + return pre; + } + + const leftPost = left.post === null ? NEG : left.post; + const rightPost = right.post === null ? NEG : right.post; + if (leftPost !== rightPost) { + return leftPost < rightPost ? -1 : 1; + } + + const leftDev = left.dev === null ? POS : left.dev; + const rightDev = right.dev === null ? POS : right.dev; + if (leftDev !== rightDev) { + return leftDev < rightDev ? -1 : 1; + } + + return compareLocal(left.local, right.local); +} + +function compareFallback(left: string, right: string): number { + if (left === right) { + return 0; + } + return left < right ? -1 : 1; +} + +/** PEP 440 ordering. Unparsed strings sort before any real version. */ +export function comparePep440(left: string, right: string): number { + const parsedLeft = parsePep440(left); + const parsedRight = parsePep440(right); + if (!parsedLeft || !parsedRight) { + if (!parsedLeft && !parsedRight) { + return compareFallback(left, right); + } + return parsedLeft ? 1 : -1; + } + return compareParsed(parsedLeft, parsedRight); +} + +/** + * Equality for `==` / `!=`. A specifier without a local version ignores the + * candidate's local version (`==1.0` matches `1.0+cpu`); one that includes a + * local version compares it. + */ +export function pep440SpecifierEqual( + version: string, + specVersion: string, +): boolean { + const parsedVersion = parsePep440(version); + const parsedSpec = parsePep440(specVersion); + if (!parsedVersion || !parsedSpec) { + return version.trim() === specVersion.trim(); + } + if (parsedSpec.local === null) { + return ( + compareParsed( + { ...parsedVersion, local: null }, + { ...parsedSpec, local: null }, + ) === 0 + ); + } + return compareParsed(parsedVersion, parsedSpec) === 0; +} + +/** Release numbers padded to at least three segments, for risk analysis. */ +export function releaseNumbers(version: string): number[] { + const parsed = parsePep440(version); + const parts = parsed ? parsed.release.slice() : [0]; + while (parts.length < 3) { + parts.push(0); + } + return parts; +} diff --git a/src/core/unifiedParser.ts b/src/core/unifiedParser.ts index f260f5d..2870155 100644 --- a/src/core/unifiedParser.ts +++ b/src/core/unifiedParser.ts @@ -56,17 +56,8 @@ export function detectFileType( return { type: FileType.PyProject, isValid: false, confidence: 0.0 }; } - if ( - /requirements.*\.txt$/.test(fileName) || - fileName.endsWith(".requirements") - ) { - // For requirements.txt, we assume it's valid if it doesn't contain TOML syntax - if (!content.includes("[") || !content.includes("]")) { - return { type: FileType.Requirements, isValid: true, confidence: 0.9 }; - } - } - - // Content-based detection + // Content-based detection. Extras (`requests[security]`) also use brackets, + // so a `[` / `]` pair alone must not reject a requirements file. const pyprojectIndicators = [ "[project]", "[tool.", @@ -79,12 +70,22 @@ export function detectFileType( return content.includes(indicator); }); + if ( + /requirements.*\.txt$/.test(fileName) || + fileName.endsWith(".requirements") + ) { + if (!hasPyProjectIndicators) { + return { type: FileType.Requirements, isValid: true, confidence: 0.9 }; + } + } + if (hasPyProjectIndicators) { return { type: FileType.PyProject, isValid: true, confidence: 0.95 }; } - // Default to requirements.txt if it looks like a Python requirements file - if (content.match(/^[a-zA-Z0-9._-]+\s*[=<>!]/m)) { + // Default to requirements.txt if it looks like a Python requirements file. + // Extras may sit between the name and the operator (`pkg[extra]>=1`). + if (content.match(/^[a-zA-Z0-9._-]+(?:\[[^\]]*\])?\s*[=<>!~^]/m)) { return { type: FileType.Requirements, isValid: true, confidence: 0.8 }; } diff --git a/src/core/versionAnalyzer.ts b/src/core/versionAnalyzer.ts index 3563b37..3d276ce 100644 --- a/src/core/versionAnalyzer.ts +++ b/src/core/versionAnalyzer.ts @@ -3,15 +3,34 @@ */ import type { VersionAnalysis } from "../types"; +import { parsePep440 } from "./pep440"; import { parseVersion } from "./versionResolver"; /** - * Analyze version difference and risk level + * Analyze version difference and risk level. + * An epoch change (`1.0` → `1!1.0`) is a breaking update even when the + * release numbers themselves did not move. */ export function analyzeVersionUpdate( current: string, latest: string, ): VersionAnalysis { + const currentEpoch = parsePep440(current)?.epoch; + const latestEpoch = parsePep440(latest)?.epoch; + if ( + currentEpoch !== undefined && + latestEpoch !== undefined && + currentEpoch !== latestEpoch + ) { + return { + currentVersion: current, + latestVersion: latest, + updateType: "major", + isBreakingChange: true, + riskLevel: "high", + }; + } + const currentParts = parseVersion(current); const latestParts = parseVersion(latest); diff --git a/src/core/versionResolver.ts b/src/core/versionResolver.ts index 41561c8..8edb41c 100644 --- a/src/core/versionResolver.ts +++ b/src/core/versionResolver.ts @@ -1,131 +1,45 @@ /** * Version Resolver for Python package versions * Validates: Requirements 4.1, 4.2, 4.3, 4.4, 4.5, 4.6 + * + * Ordering and pre-release detection follow PEP 440 (epochs, pre/post/dev, + * local versions). See pep440.ts. */ import type { ResolveResult, VersionConstraint } from "../types"; - -// Pre-release identifiers -// A pre-release segment (a/alpha/b/beta/c/rc/pre/preview/dev) with its number, -// optionally followed by a .postN tail (PEP 440: X.Y.ZaN.postM is still a pre-release). -// ".post" alone is a POST-release and must NOT match, so the tag has to be at the -// end of the version (optionally closing with the post tail). -const PRERELEASE_SUFFIX_REGEX = - /(?:a|alpha|b|beta|c|rc|pre|preview|dev)\d*(?:\.post\d*)?$/i; - -// Pre-compiled regexes for performance -const VERSION_CLEAN_REGEX = /[a-zA-Z].*/g; -const POST_REGEX = /post\.?(\d*)/; -const DEV_REGEX = /dev\.?(\d*)/; -const ALPHA_REGEX = /(?:^|[.\d\-_])(a|alpha)\.?(\d*)/; -const BETA_REGEX = /(?:^|[.\d\-_])(b|beta)\.?(\d*)/; -const RC_REGEX = /(?:^|[.\d\-_])(rc|pre|c)\.?(\d*)/; +import { + comparePep440, + isPep440Prerelease, + parsePep440, + pep440SpecifierEqual, + releaseNumbers, +} from "./pep440"; /** - * Check if a version string is a pre-release version + * Check if a version string is a pre-release version. + * Includes pre-releases that also carry a post or local segment + * (`1.0.0rc1.post1`, `1.0rc1+cpu`) and developmental releases. + * Post-only (`1.0.0.post1`) and local-only (`1.0+cpu`) releases are final. */ export function isPrerelease(version: string): boolean { - return PRERELEASE_SUFFIX_REGEX.test(version.trim()); + return isPep440Prerelease(version); } /** - * Parse a version string into comparable parts - * Returns array of numbers for comparison + * Parse a version string into its release numbers. + * Epoch and pre/post/dev/local segments are not included; at least three + * components are returned so major/minor/patch indexing stays stable. */ export function parseVersion(version: string): number[] { - // Remove any pre-release suffix for numeric comparison - const cleanVersion = version.replace(VERSION_CLEAN_REGEX, ""); - const parts = cleanVersion.split(".").map((p) => { - const num = parseInt(p, 10); - return Number.isNaN(num) ? 0 : num; - }); - - // Ensure at least 3 parts for comparison - while (parts.length < 3) { - parts.push(0); - } - - return parts; + return releaseNumbers(version); } /** - * Compare two version strings - * Returns: -1 if a < b, 0 if a == b, 1 if a > b + * Compare two version strings per PEP 440. + * Returns: -1 if a < b, 0 if a == b, 1 if a > b. */ export function compareVersions(a: string, b: string): number { - const partsA = parseVersion(a); - const partsB = parseVersion(b); - - const maxLen = Math.max(partsA.length, partsB.length); - - for (let i = 0; i < maxLen; i++) { - const numA = partsA[i] || 0; - const numB = partsB[i] || 0; - - if (numA < numB) { - return -1; - } - if (numA > numB) { - return 1; - } - } - - // If numeric parts are equal, compare based on pre-release/post-release status - // Priorities: dev < alpha < beta < rc < final < post - const infoA = getPrereleaseInfo(a); - const infoB = getPrereleaseInfo(b); - - if (infoA.type !== infoB.type) { - return infoA.type < infoB.type ? -1 : 1; - } - - if (infoA.number !== infoB.number) { - return infoA.number < infoB.number ? -1 : 1; - } - - return 0; -} - -/** - * Extract pre-release information from version string - * Returns type (0-5) and number for comparison - */ -function getPrereleaseInfo(version: string): { type: number; number: number } { - const lower = version.toLowerCase(); - - // Check types in order of precedence or specific matching - // Order: dev (0) < alpha (1) < beta (2) < rc (3) < final (4) < post (5) - - if (lower.includes("post")) { - const match = lower.match(POST_REGEX); - if (match) { - return { type: 5, number: match[1] ? parseInt(match[1], 10) : 0 }; - } - } - - if (lower.includes("dev")) { - const match = lower.match(DEV_REGEX); - if (match) { - return { type: 0, number: match[1] ? parseInt(match[1], 10) : 0 }; - } - } - - let match = lower.match(ALPHA_REGEX); - if (match) { - return { type: 1, number: match[2] ? parseInt(match[2], 10) : 0 }; - } - - match = lower.match(BETA_REGEX); - if (match) { - return { type: 2, number: match[2] ? parseInt(match[2], 10) : 0 }; - } - - match = lower.match(RC_REGEX); - if (match) { - return { type: 3, number: match[2] ? parseInt(match[2], 10) : 0 }; - } - - return { type: 4, number: 0 }; + return comparePep440(a, b); } /** @@ -159,21 +73,24 @@ export function parseSpecifier(specifier: string): VersionConstraint[] { } /** - * Check if a version satisfies a wildcard constraint (==1.2.* / !=1.2.*) - * Per PEP 440, prefix matching compares the numeric segments before the ".*" + * Check if a version satisfies a wildcard constraint (==1.2.* / !=1.2.*). + * Prefix matching compares epoch and the release segments before `.*`. + * Missing segments on the candidate are treated as 0 (`1.2` matches `==1.2.0.*`). */ function satisfiesWildcard( version: string, constraint: VersionConstraint, ): boolean { - const prefix = constraint.version - .slice(0, -2) - .split(".") - .map((p) => parseInt(p, 10) || 0); - const versionParts = parseVersion(version); - - for (let i = 0; i < prefix.length; i++) { - if ((versionParts[i] || 0) !== prefix[i]) { + const prefix = parsePep440(constraint.version.slice(0, -2)); + const parsed = parsePep440(version); + if (!prefix || !parsed) { + return false; + } + if (prefix.epoch !== parsed.epoch) { + return false; + } + for (let i = 0; i < prefix.release.length; i++) { + if ((parsed.release[i] ?? 0) !== prefix.release[i]) { return false; } } @@ -196,15 +113,16 @@ function satisfiesConstraint( return constraint.operator === "==" ? matched : !matched; } + if (constraint.operator === "==" || constraint.operator === "!=") { + const equal = pep440SpecifierEqual(version, constraint.version); + return constraint.operator === "==" ? equal : !equal; + } + const cmp = compareVersions(version, constraint.version); switch (constraint.operator) { case "===": return version.trim() === constraint.version.trim(); - case "==": - return cmp === 0; - case "!=": - return cmp !== 0; case ">=": return cmp >= 0; case "<=": @@ -215,20 +133,25 @@ function satisfiesConstraint( return cmp < 0; case "~=": { // Compatible release per PEP 440: - // ~=X.Y means >=X.Y, <(X+1).0 - // ~=X.Y.Z means >=X.Y.Z, =X.Y, ==X.* + // ~=X.Y.Z means >=X.Y.Z, ==X.Y.* + // Epochs must match; a `1!` release is not compatible with epoch 0. if (cmp < 0) { return false; } - const constraintSegments = constraint.version - .split(".") - .map((p) => parseInt(p, 10) || 0); - const versionParts = parseVersion(version); + const constraintParsed = parsePep440(constraint.version); + const versionParsed = parsePep440(version); + if (!constraintParsed || !versionParsed) { + return false; + } + if (constraintParsed.epoch !== versionParsed.epoch) { + return false; + } - // Lock all segments except the last specified one + const constraintSegments = constraintParsed.release; for (let i = 0; i < constraintSegments.length - 1; i++) { - if (versionParts[i] !== constraintSegments[i]) { + if ((versionParsed.release[i] ?? 0) !== constraintSegments[i]) { return false; } } diff --git a/src/extension.ts b/src/extension.ts index dc3ae37..3c1e284 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -4,15 +4,14 @@ */ import * as vscode from "vscode"; +import { type BatchUpdate, planRelocatedEdits } from "./core/batchUpdate"; import { cacheManager } from "./core/cache"; -import type { AnyDependency } from "./core/unifiedParser"; import { isSupportedFormat, parseDependencies } from "./core/unifiedParser"; import { analyzeVersionUpdate } from "./core/versionAnalyzer"; import { isCoveredBySpecifier } from "./core/versionResolver"; import { PyDepsCodeLensProvider } from "./providers/codeLensProvider"; import { PyDepsHoverProvider } from "./providers/hoverProvider"; import { getLatestCompatible } from "./providers/versionService"; -import type { VersionAnalysis } from "./types"; import { getConfig, onConfigChange } from "./utils/configuration"; import { extractVersionNumber, @@ -114,14 +113,8 @@ export function activate(context: vscode.ExtensionContext): void { const deps = parseDependencies(document.fileName, document.getText()); const config = getConfig(); - const edit = new vscode.WorkspaceEdit(); - let safeUpdates = 0; - const riskyUpdates: Array<{ - dep: AnyDependency; - currentVersion: string; - newVersion: string; - analysis: VersionAnalysis; - }> = []; + const pendingSafe: BatchUpdate[] = []; + const riskyUpdates: Array = []; // First pass: identify safe and risky updates with progress await vscode.window.withProgress( @@ -186,65 +179,23 @@ export function activate(context: vscode.ExtensionContext): void { dep, currentVersion, newVersion, - analysis, + kind: "risky", }); } else { - const lineText = document.lineAt(dep.line).text; - const isTOML = document.languageId === "toml"; - const planned = planLineUpdate( - lineText, - newVersion, - isTOML, - dep.startColumn, - ); - - if (planned) { - const range = new vscode.Range( - dep.line, - planned.match.startIndex, - dep.line, - planned.match.endIndex, - ); - edit.replace(document.uri, range, planned.replacement); - safeUpdates++; - } + // Do not build the edit yet. `dep.line` is from the + // pre-fetch snapshot and is wrong if the user inserted + // or deleted lines while the registry was answering. + pendingSafe.push({ dep, newVersion, kind: "safe" }); } } } }, ); - // The fetches above can take a while; if the user edited the document - // meanwhile, the snapshot's line numbers no longer match the file. - // Re-parse and re-locate every dependency by name (+ extra) so the - // edits below always land on the current text. - let depsForEdits = deps; - if (document.version !== documentVersionAtStart) { - depsForEdits = parseDependencies(document.fileName, document.getText()); - Logger.log( - `Document changed during update-all; re-matching ${depsForEdits.length} dependencies`, - ); - } - const freshByName = new Map(); - if (depsForEdits !== deps) { - for (const d of depsForEdits) { - const extra = "extra" in d ? (d.extra ?? "") : ""; - const depKey = `${d.packageName}|${extra}`; - const list = freshByName.get(depKey) ?? []; - list.push(d); - freshByName.set(depKey, list); - } - } - const locate = (dep: AnyDependency): AnyDependency | undefined => { - if (depsForEdits === deps) { - return dep; - } - const extra = "extra" in dep ? (dep.extra ?? "") : ""; - return freshByName.get(`${dep.packageName}|${extra}`)?.shift(); - }; - - // Handle risky updates with confirmation - let confirmedRiskyUpdates = 0; + // Handle risky updates with confirmation. Safe edits are not + // applied until after this dialog, so a change made while it is + // open is included in the same re-parse. + let applyRisky = false; if (riskyUpdates.length > 0) { const riskyList = riskyUpdates .map( @@ -266,36 +217,42 @@ export function activate(context: vscode.ExtensionContext): void { if (choice === undefined) { // User clicked Cancel (VS Code provides this automatically) return; - } else if (choice === t("updateAllRisky")) { - const isTOML = document.languageId === "toml"; - for (const update of riskyUpdates) { - const target = locate(update.dep); - if (!target) { - continue; - } - const lineText = document.lineAt(target.line).text; - const planned = planLineUpdate( - lineText, - update.newVersion, - isTOML, - target.startColumn, - ); - - if (planned) { - const range = new vscode.Range( - target.line, - planned.match.startIndex, - target.line, - planned.match.endIndex, - ); - edit.replace(document.uri, range, planned.replacement); - confirmedRiskyUpdates++; - } - } } - // If 'Update Safe Only', we don't add risky updates to edit + applyRisky = choice === t("updateAllRisky"); + // "Update Safe Only" leaves applyRisky false. + } + + if (document.version !== documentVersionAtStart) { + Logger.log( + "Document changed during update-all; re-matching dependencies against the current text", + ); } + const currentText = document.getText(); + const plannedEdits = planRelocatedEdits({ + updates: applyRisky ? [...pendingSafe, ...riskyUpdates] : pendingSafe, + fileName: document.fileName, + currentText, + isTOML: document.languageId === "toml", + }); + + const edit = new vscode.WorkspaceEdit(); + for (const plannedEdit of plannedEdits) { + const range = new vscode.Range( + plannedEdit.line, + plannedEdit.startColumn, + plannedEdit.line, + plannedEdit.endColumn, + ); + edit.replace(document.uri, range, plannedEdit.replacement); + } + + const safeUpdates = plannedEdits.filter( + (item) => item.kind === "safe", + ).length; + const confirmedRiskyUpdates = plannedEdits.filter( + (item) => item.kind === "risky", + ).length; const totalUpdates = safeUpdates + confirmedRiskyUpdates; if (totalUpdates > 0) { @@ -361,6 +318,13 @@ export function activate(context: vscode.ExtensionContext): void { context.subscriptions.push(hoverDisposable); + // Closed files must leave the dependency cache. Otherwise the status + // bar keeps counting every requirements file the user has ever opened. + const closeDisposable = vscode.workspace.onDidCloseTextDocument((closed) => { + codeLensProvider.forgetDocument(closed.uri); + }); + context.subscriptions.push(closeDisposable); + // Handle document changes with debounce (Requirement 1.2, 1.3) const changeDisposable = vscode.workspace.onDidChangeTextDocument((event) => { if (isSupportedFormat(event.document.languageId)) { diff --git a/src/providers/codeLensProvider.ts b/src/providers/codeLensProvider.ts index edacb99..33d8a57 100644 --- a/src/providers/codeLensProvider.ts +++ b/src/providers/codeLensProvider.ts @@ -4,6 +4,7 @@ */ import * as vscode from "vscode"; +import { versionCacheKey } from "../core/packageName"; import { type AnyDependency, parseDependencies } from "../core/unifiedParser"; import { analyzeVersionUpdate } from "../core/versionAnalyzer"; import { isCoveredBySpecifier, resolveLatest } from "../core/versionResolver"; @@ -14,6 +15,7 @@ import { extractVersionNumber, hasVersionConstraint, shouldOfferVersionUpdate, + specifierForResolution, } from "../utils/dependencyUtils"; import { t } from "../utils/i18n"; import { Logger } from "../utils/logger"; @@ -76,8 +78,38 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { this.pendingFetches.clear(); } + /** + * Drop a closed document so its dependencies no longer inflate the + * status bar. The version cache is kept: another open file may use it. + */ + public forgetDocument(uri: vscode.Uri): void { + this.dependencyCache.delete(uri.toString()); + this.updateStatusBar(); + } + private cacheKey(packageName: string, registryUrl: string): string { - return `${registryUrl.replace(/\/+$/, "")}|${packageName.toLowerCase()}`; + return versionCacheKey(packageName, registryUrl); + } + + private openDocumentUris(): Set | null { + const documents = vscode.workspace.textDocuments; + if (!documents) { + return null; + } + return new Set(documents.map((document) => document.uri.toString())); + } + + /** Closed documents are not part of the status total. */ + private evictClosedDocuments(): void { + const open = this.openDocumentUris(); + if (!open) { + return; + } + for (const uri of this.dependencyCache.keys()) { + if (!open.has(uri)) { + this.dependencyCache.delete(uri); + } + } } provideCodeLenses( @@ -139,9 +171,27 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { }), ); } + + // Re-opening a file hits the version cache without a new fetch. + // Refresh the status bar then, so a document that was closed and + // reopened is counted again. Skip the first paint, before any + // lookup has succeeded, so the bar does not flash "0 up to date". + if (this.hasResolvedVersion(dependencies, config.registryUrl)) { + this.updateStatusBar(); + } return codeLenses; } + private hasResolvedVersion( + dependencies: AnyDependency[], + registryUrl: string, + ): boolean { + return dependencies.some((dep) => { + const key = this.cacheKey(dep.packageName.split("[")[0], registryUrl); + return this.versionCache.get(key)?.status === "success"; + }); + } + /** * Resolve the newest usable version for one dependency from the raw list, * honouring the dependency's own specifier (==, >=, ~=, wildcards, markers). @@ -151,11 +201,11 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { raw: PackageVersions, showPrerelease: boolean, ): string | null { - const specifier = - "section" in dep - ? dep.versionSpecifier - : dep.versionSpecifier.replace(/\\+$/, ""); - const resolved = resolveLatest(raw.versions, specifier, showPrerelease); + const resolved = resolveLatest( + raw.versions, + specifierForResolution(dep), + showPrerelease, + ); return resolved.found ? resolved.version! : null; } @@ -168,9 +218,6 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { codeLens: vscode.CodeLens, _token: vscode.CancellationToken, ): Promise { - console.error( - `[UAT-PROBE] resolveCodeLens entered; instanceof=${codeLens instanceof DependencyCodeLens}, ctor=${(codeLens as any)?.constructor?.name}`, - ); if (!(codeLens instanceof DependencyCodeLens)) { return codeLens; } @@ -341,6 +388,9 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { } private updateStatusBar(): void { + // Always drop closed files, even when no status bar is attached, + // so the dependency cache cannot grow for the life of the window. + this.evictClosedDocuments(); if (!this.statusBar) { return; } @@ -349,13 +399,8 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { let updatesAvailable = 0; let totalChecked = 0; - for (const entry of this.versionCache.values()) { - if (entry.status === "success") { - totalChecked++; - } - } - - // Count updates from the last known dependency set + // Only dependencies still in an open document. Historical files + // that were closed must not keep adding to the total. for (const cached of this.dependencyCache.values()) { for (const dep of cached.dependencies) { const key = this.cacheKey( @@ -364,6 +409,7 @@ export class PyDepsCodeLensProvider implements vscode.CodeLensProvider { ); const vEntry = this.versionCache.get(key); if (vEntry?.status === "success" && vEntry.versions) { + totalChecked++; const latest = this.resolveForDependency( dep, vEntry.versions, diff --git a/src/providers/hoverProvider.ts b/src/providers/hoverProvider.ts index debda7b..73e26ff 100644 --- a/src/providers/hoverProvider.ts +++ b/src/providers/hoverProvider.ts @@ -6,6 +6,7 @@ import * as vscode from "vscode"; import { parseDependencies } from "../core/unifiedParser"; import { getConfig } from "../utils/configuration"; +import { specifierForResolution } from "../utils/dependencyUtils"; import { t } from "../utils/i18n"; import { Logger } from "../utils/logger"; import { getLatestCompatible } from "./versionService"; @@ -46,9 +47,11 @@ export class PyDepsHoverProvider implements vscode.HoverProvider { Logger.log(`Providing hover for ${dep.packageName}`); try { const packageNameWithoutExtras = dep.packageName.split("[")[0]; + // Same resolution as the CodeLens: ranges stay inside the specifier, + // and a bare `==` pin still suggests the absolute latest. const versionInfo = await getLatestCompatible( packageNameWithoutExtras, - "", + specifierForResolution(dep), config.showPrerelease, config.cacheTTLMinutes, config.registryUrl, diff --git a/src/providers/versionService.ts b/src/providers/versionService.ts index d12da2e..d95b3bb 100644 --- a/src/providers/versionService.ts +++ b/src/providers/versionService.ts @@ -4,19 +4,19 @@ */ import { cacheManager } from "../core/cache"; +import { versionCacheKey } from "../core/packageName"; import { resolveLatest } from "../core/versionResolver"; import type { PackageVersions, VersionInfo } from "../types"; import { Logger } from "../utils/logger"; import { fetchVersions } from "./pypiClient"; -const DEFAULT_REGISTRY = "https://pypi.org"; - /** * Cache key includes the registry so that switching registries cannot serve - * data fetched from the previous one. + * data fetched from the previous one, and the package name is PEP 503 + * normalized so `foo_bar` and `foo-bar` share one entry. */ function cacheKey(packageName: string, registryUrl?: string): string { - return `${(registryUrl || DEFAULT_REGISTRY).replace(/\/+$/, "")}|${packageName}`; + return versionCacheKey(packageName, registryUrl); } /** diff --git a/src/test/__mocks__/vscode.ts b/src/test/__mocks__/vscode.ts index 8442746..43dfbae 100644 --- a/src/test/__mocks__/vscode.ts +++ b/src/test/__mocks__/vscode.ts @@ -71,6 +71,7 @@ export const window = { }; export const workspace = { + textDocuments: [] as Array<{ uri: { toString(): string } }>, getConfiguration: (_section?: string) => ({ get: (_key: string, defaultValue?: T) => defaultValue, }), diff --git a/src/test/unit/basic.test.ts b/src/test/unit/basic.test.ts index 9a94828..36b505a 100644 --- a/src/test/unit/basic.test.ts +++ b/src/test/unit/basic.test.ts @@ -23,4 +23,25 @@ describe("Unified Parser - File Variants", () => { const result = parseDependencies("requirements-dev.txt", content); assert.strictEqual(result.length, 2); }); + + it("parses a requirements file that only contains extras", () => { + const content = "requests[security]>=2.0\nuvicorn[standard]==0.20.0\n"; + const result = parseDependencies("requirements.txt", content); + assert.strictEqual(result.length, 2); + assert.strictEqual(result[0].packageName, "requests"); + assert.strictEqual(result[0].versionSpecifier, ">=2.0"); + assert.strictEqual(result[1].packageName, "uvicorn"); + assert.strictEqual(result[1].versionSpecifier, "==0.20.0"); + }); + + it("detects an extras-only dependency list without a requirements file name", () => { + const content = "requests[security,socks]>=2.0\n"; + const detection = detectFileType("deps.txt", content); + assert.strictEqual(detection.type, FileType.Requirements); + assert.strictEqual(detection.isValid, true); + const result = parseDependencies("deps.txt", content); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].packageName, "requests"); + assert.strictEqual(result[0].versionSpecifier, ">=2.0"); + }); }); diff --git a/src/test/unit/batchUpdate.test.ts b/src/test/unit/batchUpdate.test.ts new file mode 100644 index 0000000..0a43487 --- /dev/null +++ b/src/test/unit/batchUpdate.test.ts @@ -0,0 +1,178 @@ +/** + * Update All must plan every edit — including safe patch/minor bumps — + * from the document as it is after the registry requests, not from the + * line numbers captured before those requests. + */ +import * as assert from "node:assert"; +import { + type BatchUpdate, + type PlannedEdit, + planRelocatedEdits, +} from "../../core/batchUpdate"; +import { parseDependencies } from "../../core/unifiedParser"; + +function applyEdits(content: string, edits: PlannedEdit[]): string { + const lines = content.split("\n"); + const ordered = [...edits].sort((a, b) => { + if (a.line !== b.line) { + return b.line - a.line; + } + return b.startColumn - a.startColumn; + }); + for (const edit of ordered) { + const line = lines[edit.line]; + lines[edit.line] = + line.slice(0, edit.startColumn) + + edit.replacement + + line.slice(edit.endColumn); + } + return lines.join("\n"); +} + +describe("Update All relocation", () => { + const baseline = "flask==1.0.0\nrequests==2.0.0\n"; + + function safeUpdates(): BatchUpdate[] { + const deps = parseDependencies("requirements.txt", baseline); + return deps.map((dep) => ({ + dep, + // Both are non-major, so the command used to edit them from the + // snapshot before it re-located anything. + newVersion: dep.packageName === "flask" ? "1.0.1" : "2.32.3", + kind: "safe" as const, + })); + } + + it("rewrites safe updates on the lines they occupy now", () => { + const current = `# added while versions were loading + +${baseline}`; + const edits = planRelocatedEdits({ + updates: safeUpdates(), + fileName: "requirements.txt", + currentText: current, + isTOML: false, + }); + + assert.deepStrictEqual( + edits.map((edit) => edit.line), + [2, 3], + ); + assert.ok(edits.every((edit) => edit.kind === "safe")); + assert.strictEqual( + applyEdits(current, edits), + `# added while versions were loading + +flask==1.0.1 +requests==2.32.3 +`, + ); + }); + + it("does not edit a comment inserted between two safe updates", () => { + const current = "flask==1.0.0\n# inserted\nrequests==2.0.0\n"; + const edits = planRelocatedEdits({ + updates: safeUpdates(), + fileName: "requirements.txt", + currentText: current, + isTOML: false, + }); + const requests = edits.find((edit) => edit.packageName === "requests"); + assert.ok(requests); + assert.strictEqual(requests.line, 2); + assert.strictEqual( + applyEdits(current, edits), + "flask==1.0.1\n# inserted\nrequests==2.32.3\n", + ); + }); + + it("skips a safe update whose dependency was deleted", () => { + const current = "requests==2.0.0\n"; + const edits = planRelocatedEdits({ + updates: safeUpdates(), + fileName: "requirements.txt", + currentText: current, + isTOML: false, + }); + assert.strictEqual(edits.length, 1); + assert.strictEqual(edits[0].packageName, "requests"); + assert.strictEqual(edits[0].line, 0); + assert.strictEqual(applyEdits(current, edits), "requests==2.32.3\n"); + }); + + it("still rewrites an unchanged document", () => { + const edits = planRelocatedEdits({ + updates: safeUpdates(), + fileName: "requirements.txt", + currentText: baseline, + isTOML: false, + }); + assert.deepStrictEqual( + edits.map((edit) => edit.line), + [0, 1], + ); + assert.strictEqual( + applyEdits(baseline, edits), + "flask==1.0.1\nrequests==2.32.3\n", + ); + }); + + it("relocates a risky update with the same pass as the safe ones", () => { + const current = `\n${baseline}`; + const deps = parseDependencies("requirements.txt", baseline); + const updates: BatchUpdate[] = [ + { dep: deps[0], newVersion: "1.0.1", kind: "safe" }, + { dep: deps[1], newVersion: "3.0.0", kind: "risky" }, + ]; + const edits = planRelocatedEdits({ + updates, + fileName: "requirements.txt", + currentText: current, + isTOML: false, + }); + assert.deepStrictEqual( + edits.map((edit) => [edit.packageName, edit.line, edit.kind]), + [ + ["flask", 1, "safe"], + ["requests", 2, "risky"], + ], + ); + assert.strictEqual( + applyEdits(current, edits), + "\nflask==1.0.1\nrequests==3.0.0\n", + ); + }); + + it("does not confuse two pyproject entries that share a name", () => { + const baselineToml = `[project] +dependencies = ["flask>=2.0"] + +[project.optional-dependencies] +dev = ["flask>=1.0"] +`; + const current = `# shifted\n${baselineToml}`; + const deps = parseDependencies("pyproject.toml", baselineToml); + assert.strictEqual(deps.length, 2); + const updates: BatchUpdate[] = deps.map((dep) => ({ + dep, + newVersion: "extra" in dep && dep.extra === "dev" ? "1.2.0" : "2.3.0", + kind: "safe" as const, + })); + const edits = planRelocatedEdits({ + updates, + fileName: "pyproject.toml", + currentText: current, + isTOML: true, + }); + assert.strictEqual( + applyEdits(current, edits), + `# shifted +[project] +dependencies = ["flask>=2.3.0"] + +[project.optional-dependencies] +dev = ["flask>=1.2.0"] +`, + ); + }); +}); diff --git a/src/test/unit/mediumReview.test.ts b/src/test/unit/mediumReview.test.ts new file mode 100644 index 0000000..27e0e31 --- /dev/null +++ b/src/test/unit/mediumReview.test.ts @@ -0,0 +1,262 @@ +/** + * Cache identity, closed-document status totals, hover/CodeLens agreement, + * and the absence of the resolveCodeLens debug probe. + */ +import * as assert from "node:assert"; +import * as vscode from "vscode"; +import { cacheManager } from "../../core/cache"; +import { normalizePackageName, versionCacheKey } from "../../core/packageName"; +import { PyDepsCodeLensProvider } from "../../providers/codeLensProvider"; +import { PyDepsHoverProvider } from "../../providers/hoverProvider"; +import { getPackageVersions } from "../../providers/versionService"; +import type { StatusBarManager } from "../../utils/statusBar"; + +const token = { + isCancellationRequested: false, + onCancellationRequested: () => ({ dispose: () => {} }), +} as unknown as vscode.CancellationToken; + +function document( + uri: string, + content: string, + fileName = "requirements.txt", +): vscode.TextDocument { + return { + fileName, + languageId: "pip-requirements", + version: 1, + uri: { toString: () => uri }, + getText: () => content, + } as unknown as vscode.TextDocument; +} + +function setOpenDocuments(docs: vscode.TextDocument[]): void { + Object.assign(vscode.workspace, { textDocuments: docs }); +} + +function pypiResponse(versions: string[]) { + const releases: Record> = {}; + for (const version of versions) { + releases[version] = [{ yanked: false }]; + } + return { + ok: true, + status: 200, + json: async () => ({ info: { summary: "A package" }, releases }), + }; +} + +async function waitFor(predicate: () => boolean): Promise { + const start = Date.now(); + while (!predicate()) { + if (Date.now() - start > 2000) { + throw new Error("timed out"); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } +} + +describe("PEP 503 cache keys", () => { + const originalFetch = globalThis.fetch; + + beforeEach(() => { + cacheManager.clear(); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + cacheManager.clear(); + }); + + it("normalizes separators and case", () => { + assert.strictEqual(normalizePackageName("Foo_Bar"), "foo-bar"); + assert.strictEqual(normalizePackageName("foo.bar"), "foo-bar"); + assert.strictEqual(normalizePackageName("foo--bar"), "foo-bar"); + assert.strictEqual(normalizePackageName("Foo_Bar[Extra]"), "foo-bar"); + assert.strictEqual( + versionCacheKey("Foo_Bar", "https://pypi.org/"), + versionCacheKey("foo-bar", "https://pypi.org"), + ); + }); + + it("reuses one registry response for equivalent names", async () => { + let calls = 0; + globalThis.fetch = (async () => { + calls++; + return pypiResponse(["1.2.3"]); + }) as unknown as typeof fetch; + + const first = await getPackageVersions("foo-bar"); + const second = await getPackageVersions("Foo_Bar"); + const third = await getPackageVersions("FOO.BAR"); + assert.strictEqual(calls, 1); + assert.deepStrictEqual(first.versions, ["1.2.3"]); + assert.deepStrictEqual(second.versions, ["1.2.3"]); + assert.deepStrictEqual(third.versions, ["1.2.3"]); + }); +}); + +describe("CodeLens cache, status bar, and debug logging", () => { + const originalFetch = globalThis.fetch; + + afterEach(() => { + globalThis.fetch = originalFetch; + cacheManager.clear(); + setOpenDocuments([]); + }); + + it("does not log a UAT probe from resolveCodeLens", async () => { + const errors: string[] = []; + const original = console.error; + console.error = (...args: unknown[]) => { + errors.push(args.map(String).join(" ")); + }; + try { + const provider = new PyDepsCodeLensProvider(); + const lens = new vscode.CodeLens(new vscode.Range(0, 0, 0, 0)); + await provider.resolveCodeLens(lens, token); + } finally { + console.error = original; + } + assert.ok(!errors.join("\n").includes("UAT-PROBE")); + }); + + it("shares the in-memory version cache across PEP 503 names", async () => { + let calls = 0; + globalThis.fetch = (async () => { + calls++; + return pypiResponse(["1.0.0", "2.0.0"]); + }) as unknown as typeof fetch; + + const provider = new PyDepsCodeLensProvider(); + const first = document( + "file:///tmp/foo-underscore.txt", + "Foo_Bar==1.0.0\n", + ); + setOpenDocuments([first]); + const firstLens = provider + .provideCodeLenses(first, token) + .find((lens) => !lens.command); + assert.ok(firstLens); + const loaded = new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("timed out")), 2000); + provider.onDidChangeCodeLenses(() => { + clearTimeout(timer); + resolve(); + }); + }); + await provider.resolveCodeLens(firstLens, token); + await loaded; + cacheManager.clear(); + + const second = document("file:///tmp/foo-hyphen.txt", "foo-bar==1.0.0\n"); + setOpenDocuments([second]); + const secondLens = provider + .provideCodeLenses(second, token) + .find((lens) => !lens.command); + assert.ok(secondLens); + const resolved = await provider.resolveCodeLens(secondLens, token); + assert.strictEqual(calls, 1); + assert.match(resolved.command?.title ?? "", /2\.0\.0/); + }); + + it("drops closed documents from the status bar total", async () => { + globalThis.fetch = (async () => + pypiResponse(["1.0.0", "2.0.0"])) as unknown as typeof fetch; + + const status = { + updatesAvailable: -1, + totalPackages: -1, + updateStatus(updatesAvailable: number, totalPackages: number) { + this.updatesAvailable = updatesAvailable; + this.totalPackages = totalPackages; + }, + }; + const provider = new PyDepsCodeLensProvider( + status as unknown as StatusBarManager, + ); + const flask = document("file:///tmp/a/requirements.txt", "flask==1.0.0\n"); + const django = document( + "file:///tmp/b/requirements.txt", + "django==1.0.0\n", + ); + setOpenDocuments([flask, django]); + + for (const doc of [flask, django]) { + const lens = provider + .provideCodeLenses(doc, token) + .find((item) => !item.command); + assert.ok(lens); + await provider.resolveCodeLens(lens, token); + } + await waitFor(() => status.updatesAvailable === 2); + assert.strictEqual(status.totalPackages, 2); + + provider.forgetDocument(flask.uri); + assert.strictEqual(status.updatesAvailable, 1); + assert.strictEqual(status.totalPackages, 1); + + setOpenDocuments([flask, django]); + provider.provideCodeLenses(flask, token); + assert.strictEqual(status.updatesAvailable, 2); + + setOpenDocuments([django]); + provider.forgetDocument({ + toString: () => "file:///not-open", + } as vscode.Uri); + assert.strictEqual(status.updatesAvailable, 1); + assert.strictEqual(status.totalPackages, 1); + }); +}); + +describe("Hover respects the dependency specifier", () => { + beforeEach(() => { + cacheManager.clear(); + }); + + afterEach(() => { + cacheManager.clear(); + }); + + it("shows the newest version inside the range, not the absolute latest", async () => { + cacheManager.set(versionCacheKey("requests"), { + packageName: "requests", + versions: ["2.0.0", "2.5.0a1", "2.9.0", "3.1.0", "4.0.0"], + summary: "HTTP for Humans", + fetchedAt: Date.now(), + }); + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => { + throw new Error("hover should resolve from the specifier cache"); + }) as unknown as typeof fetch; + + try { + const provider = new PyDepsHoverProvider(); + const doc = document( + "file:///tmp/requirements.txt", + "requests>=2.0,<3\nrequests==2.0.0\n", + ); + const ranged = await provider.provideHover( + doc, + new vscode.Position(0, 1), + token, + ); + const pinned = await provider.provideHover( + doc, + new vscode.Position(1, 1), + token, + ); + assert.ok(ranged); + assert.ok(pinned); + const rangedText = (ranged.contents as unknown as vscode.MarkdownString) + .value; + const pinnedText = (pinned.contents as unknown as vscode.MarkdownString) + .value; + assert.match(rangedText, /2\.9\.0/); + assert.ok(!rangedText.includes("4.0.0")); + assert.match(pinnedText, /4\.0\.0/); + } finally { + globalThis.fetch = originalFetch; + } + }); +}); diff --git a/src/test/unit/parser.test.ts b/src/test/unit/parser.test.ts index b0ed4fa..49c38d3 100644 --- a/src/test/unit/parser.test.ts +++ b/src/test/unit/parser.test.ts @@ -35,5 +35,12 @@ describe("Unified Parser", () => { const result = detectFileType("random.file", content); assert.strictEqual(result.type, FileType.PyProject); }); + + it("accepts a requirements file whose only lines use extras", () => { + const content = "requests[security]>=2.0\n"; + const result = detectFileType("requirements.txt", content); + assert.strictEqual(result.type, FileType.Requirements); + assert.strictEqual(result.isValid, true); + }); }); }); diff --git a/src/test/unit/versionAnalyzer.test.ts b/src/test/unit/versionAnalyzer.test.ts index dce9358..cf4be71 100644 --- a/src/test/unit/versionAnalyzer.test.ts +++ b/src/test/unit/versionAnalyzer.test.ts @@ -70,4 +70,17 @@ describe("Version Analyzer", () => { assert.strictEqual(result.currentVersion, "1.2.3"); assert.strictEqual(result.latestVersion, "1.2.4"); }); + + it("treats an epoch change as a breaking update", () => { + const result = analyzeVersionUpdate("1.0.0", "1!1.0.0"); + assert.strictEqual(result.updateType, "major"); + assert.strictEqual(result.riskLevel, "high"); + assert.strictEqual(result.isBreakingChange, true); + }); + + it("keeps release comparison inside the same epoch", () => { + const result = analyzeVersionUpdate("1!1.0.0", "1!1.1.0"); + assert.strictEqual(result.updateType, "minor"); + assert.strictEqual(result.riskLevel, "medium"); + }); }); diff --git a/src/test/unit/versionResolver.test.ts b/src/test/unit/versionResolver.test.ts index 2d9aa50..5a317bd 100644 --- a/src/test/unit/versionResolver.test.ts +++ b/src/test/unit/versionResolver.test.ts @@ -1,6 +1,8 @@ import * as assert from "node:assert"; import { compareVersions, + isPrerelease, + parseSpecifier, parseVersion, resolve, satisfies, @@ -78,6 +80,66 @@ describe("Version Resolver", () => { it("should handle complex pre-release patterns", () => { assert.strictEqual(compareVersions("1.0.0a1", "1.0.0.a1"), 0); }); + + it("orders a pre-release post segment before the final release", () => { + // 1.0.0rc1.post1 is still an rc; the post segment only orders it + // among other rcs. It must not sort above 1.0.0. + assert.strictEqual(compareVersions("1.0.0rc1.post1", "1.0.0rc1"), 1); + assert.strictEqual(compareVersions("1.0.0rc1.post1", "1.0.0"), -1); + assert.strictEqual(compareVersions("1.0.0rc1.post1", "1.0.0rc2"), -1); + }); + + it("orders epochs ahead of the release numbers", () => { + assert.strictEqual(compareVersions("1!1.0", "2.0"), 1); + assert.strictEqual(compareVersions("1!0.1", "2020.1"), 1); + assert.strictEqual(compareVersions("1!1.0", "1!2.0"), -1); + assert.strictEqual(compareVersions("1!9.9", "2!0.1"), -1); + assert.strictEqual(compareVersions("1.0", "1!1.0"), -1); + }); + + it("orders local versions after the plain release", () => { + assert.strictEqual(compareVersions("1.0", "1.0+cpu"), -1); + assert.strictEqual(compareVersions("1.0+aaa", "1.0+abc"), -1); + assert.strictEqual(compareVersions("1.0+abc", "1.0+1"), -1); + assert.strictEqual(compareVersions("1.0+ubuntu-1", "1.0+ubuntu.1"), 0); + assert.strictEqual(compareVersions("1.0+5", "1.0+5.0"), -1); + }); + + it("treats separator and spelling aliases as the same version", () => { + assert.strictEqual(compareVersions("1.0a1", "1.0.alpha.1"), 0); + assert.strictEqual(compareVersions("1.0c1", "1.0rc1"), 0); + assert.strictEqual(compareVersions("1.0pre1", "1.0preview1"), 0); + assert.strictEqual(compareVersions("1.0-1", "1.0.post1"), 0); + assert.strictEqual(compareVersions("1.0r1", "1.0.rev1"), 0); + assert.strictEqual(compareVersions("v1.0.0", "1.0"), 0); + }); + + it("follows the PEP 440 version ordering examples", () => { + const ordered = [ + "1.0.dev456", + "1.0a1", + "1.0a2.dev456", + "1.0a12.dev456", + "1.0a12", + "1.0b1.dev456", + "1.0b2", + "1.0b2.post345.dev456", + "1.0b2.post345", + "1.0rc1.dev456", + "1.0rc1", + "1.0", + "1.0.post456.dev34", + "1.0.post456", + "1.1.dev1", + ]; + for (let i = 0; i < ordered.length - 1; i++) { + assert.strictEqual( + compareVersions(ordered[i], ordered[i + 1]), + -1, + `${ordered[i]} should be < ${ordered[i + 1]}`, + ); + } + }); }); describe("satisfies", () => { @@ -205,5 +267,71 @@ describe("Version Resolver", () => { assert.strictEqual(result.found, true); assert.strictEqual(result.version, "1.4.5"); }); + + it("requires the same epoch for a compatible release", () => { + assert.strictEqual(satisfies("1!1.5", parseSpecifier("~=1!1.0")), true); + assert.strictEqual(satisfies("1.5", parseSpecifier("~=1!1.0")), false); + assert.strictEqual(satisfies("2!1.5", parseSpecifier("~=1!1.0")), false); + }); + }); + + describe("PEP 440 pre-release and local versions", () => { + it("sees a pre-release hidden by a local label", () => { + assert.strictEqual(isPrerelease("1.0rc1+cpu"), true); + assert.strictEqual(isPrerelease("1.0.0rc1.post1"), true); + assert.strictEqual(isPrerelease("1.0.0.post1.dev1"), true); + assert.strictEqual(isPrerelease("1.0+cpu"), false); + assert.strictEqual(isPrerelease("1.0.0.post1"), false); + }); + + it("does not suggest a local pre-release over the final release", () => { + const result = resolve( + ["1.0rc1+cpu", "1.0.0rc1.post1", "1.0.0", "1.0+cpu"], + "", + ); + assert.strictEqual(result.found, true); + assert.strictEqual(result.version, "1.0+cpu"); + }); + + it("picks the highest epoch", () => { + const result = resolve(["2020.1", "2.0.0", "1!0.1"], ""); + assert.strictEqual(result.version, "1!0.1"); + }); + + it("strips the epoch before exposing release numbers", () => { + assert.deepStrictEqual(parseVersion("1!2.1.0"), [2, 1, 0]); + assert.deepStrictEqual(parseVersion("v1.2.3"), [1, 2, 3]); + }); + + it("ignores local versions on == unless the specifier has one", () => { + assert.strictEqual( + satisfies("1.2.3+cpu", parseSpecifier("==1.2.3")), + true, + ); + assert.strictEqual( + satisfies("1.2.3+cpu", parseSpecifier("==1.2.3+cpu")), + true, + ); + assert.strictEqual( + satisfies("1.2.3+gpu", parseSpecifier("==1.2.3+cpu")), + false, + ); + assert.strictEqual( + satisfies("1.2.3", parseSpecifier("==1.2.3+cpu")), + false, + ); + }); + + it("matches wildcard prefixes only inside the same epoch", () => { + assert.strictEqual( + satisfies("1!1.2.3", parseSpecifier("==1.2.*")), + false, + ); + assert.strictEqual( + satisfies("1!1.2.3", parseSpecifier("==1!1.2.*")), + true, + ); + assert.strictEqual(satisfies("1.2", parseSpecifier("==1.2.0.*")), true); + }); }); }); diff --git a/src/utils/dependencyUtils.ts b/src/utils/dependencyUtils.ts index 6719fff..b6289a6 100644 --- a/src/utils/dependencyUtils.ts +++ b/src/utils/dependencyUtils.ts @@ -325,6 +325,21 @@ export type VersionLensKind = | "up-to-date" | "update"; +/** + * Specifier passed to the version resolver for one dependency. + * Requirements continuation markers (`\`) are not part of the constraint; + * pyproject strings are already a single logical line. + */ +export function specifierForResolution(dep: { + versionSpecifier: string; + section?: string; +}): string { + if ("section" in dep && dep.section) { + return dep.versionSpecifier; + } + return dep.versionSpecifier.replace(/\\+$/, ""); +} + /** * Decide which version CodeLens a dependency should get once the registry * has answered. Unversioned and URL dependencies stay hidden so they never