diff --git a/api/src/main.ts b/api/src/main.ts index aa381414..d66f6bdc 100644 --- a/api/src/main.ts +++ b/api/src/main.ts @@ -578,6 +578,11 @@ export interface PackageInfo { * The URIs associated with the package. */ readonly uris?: readonly Uri[]; + + /** + * Whether the package is a transitive dependency. + */ + readonly isTransitive?: boolean; } /** @@ -670,9 +675,9 @@ export interface PackageManager { /** * Refreshes the package list for the specified Python environment. * @param environment - The Python environment for which to refresh the package list. - * @returns A promise that resolves when the refresh is complete. + * @returns A promise that resolves with the refreshed list of packages, or undefined. */ - refresh(environment: PythonEnvironment): Promise; + refresh(environment: PythonEnvironment): Promise; /** * Retrieves the list of packages for the specified Python environment. @@ -687,6 +692,20 @@ export interface PackageManager { */ onDidChangePackages?: Event; + /** + * Fetches the names of direct (non-transitive) packages for the specified Python environment. + * + * **Caveat:** Most package managers cannot track user install intent. For pip, this uses + * `pip list --not-required` which returns packages with no installed dependents (leaf packages), + * not necessarily packages the user explicitly installed. For example, if a user runs + * `pip install flask werkzeug`, werkzeug will still be reported as transitive because flask + * depends on it. This is a best-effort approximation. + * + * @param environment - The Python environment for which to fetch direct package names. + * @returns A promise that resolves to a set of package name strings, or undefined if not supported. + */ + getDirectPackageNames?(environment: PythonEnvironment): Promise | undefined>; + /** * Clears the package manager's cache. * @returns A promise that resolves when the cache is cleared. @@ -1029,9 +1048,9 @@ export interface PythonPackageGetterApi { * Refresh the list of packages in a Python Environment. * * @param environment The Python Environment for which the list of packages is to be refreshed. - * @returns A promise that resolves when the list of packages has been refreshed. + * @returns A promise that resolves with the refreshed list of packages, or undefined. */ - refreshPackages(environment: PythonEnvironment): Promise; + refreshPackages(environment: PythonEnvironment): Promise; /** * Get the list of packages in a Python Environment. diff --git a/src/api.ts b/src/api.ts index b3ab24cb..6e07a505 100644 --- a/src/api.ts +++ b/src/api.ts @@ -572,6 +572,11 @@ export interface PackageInfo { * The URIs associated with the package. */ readonly uris?: readonly Uri[]; + + /** + * Whether the package is a transitive dependency. + */ + readonly isTransitive?: boolean; } /** @@ -664,9 +669,9 @@ export interface PackageManager { /** * Refreshes the package list for the specified Python environment. * @param environment - The Python environment for which to refresh the package list. - * @returns A promise that resolves when the refresh is complete. + * @returns A promise that resolves with the refreshed list of packages, or undefined. */ - refresh(environment: PythonEnvironment): Promise; + refresh(environment: PythonEnvironment): Promise; /** * Retrieves the list of packages for the specified Python environment. @@ -681,6 +686,20 @@ export interface PackageManager { */ onDidChangePackages?: Event; + /** + * Fetches the names of direct (non-transitive) packages for the specified Python environment. + * + * **Caveat:** Most package managers cannot track user install intent. For pip, this uses + * `pip list --not-required` which returns packages with no installed dependents (leaf packages), + * not necessarily packages the user explicitly installed. For example, if a user runs + * `pip install flask werkzeug`, werkzeug will still be reported as transitive because flask + * depends on it. This is a best-effort approximation. + * + * @param environment - The Python environment for which to fetch direct package names. + * @returns A promise that resolves to a set of package name strings, or undefined if not supported. + */ + getDirectPackageNames?(environment: PythonEnvironment): Promise | undefined>; + /** * Clears the package manager's cache. * @returns A promise that resolves when the cache is cleared. @@ -1023,9 +1042,9 @@ export interface PythonPackageGetterApi { * Refresh the list of packages in a Python Environment. * * @param environment The Python Environment for which the list of packages is to be refreshed. - * @returns A promise that resolves when the list of packages has been refreshed. + * @returns A promise that resolves with the refreshed list of packages, or undefined. */ - refreshPackages(environment: PythonEnvironment): Promise; + refreshPackages(environment: PythonEnvironment): Promise; /** * Get the list of packages in a Python Environment. diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index a10f8885..d6a7f816 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -305,6 +305,20 @@ export async function removeEnvironmentCommand(context: unknown, managers: Envir export async function handlePackageUninstall(context: unknown, em: EnvironmentManagers) { if (context instanceof PackageTreeItem || context instanceof ProjectPackage) { + if (context.pkg.isTransitive) { + const confirm = await showInformationMessage( + l10n.t( + 'The package "{0}" is a transitive dependency. Uninstalling it may break other packages that depend on it.', + context.pkg.name, + ), + { modal: true }, + l10n.t('Uninstall Anyway'), + l10n.t('Cancel'), + ); + if (confirm !== l10n.t('Uninstall Anyway')) { + return; + } + } const moduleName = context.pkg.name; const environment = context instanceof ProjectPackage ? context.parent.environment : context.parent.environment; const packageManager = em.getPackageManager(environment); diff --git a/src/features/pythonApi.ts b/src/features/pythonApi.ts index b20de34f..1cd8217a 100644 --- a/src/features/pythonApi.ts +++ b/src/features/pythonApi.ts @@ -250,7 +250,7 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi { } return manager.manage(context, options); } - async refreshPackages(context: PythonEnvironment): Promise { + async refreshPackages(context: PythonEnvironment): Promise { await waitForEnvManagerId([context.envId.managerId]); const manager = this.envManagers.getPackageManager(context); if (!manager) { diff --git a/src/features/views/envManagersView.ts b/src/features/views/envManagersView.ts index 131484e6..044815aa 100644 --- a/src/features/views/envManagersView.ts +++ b/src/features/views/envManagersView.ts @@ -252,9 +252,13 @@ export class EnvManagerView implements TreeDataProvider, Disposable const views: EnvTreeItem[] = []; if (pkgManager) { - const packages = await pkgManager.getPackages(environment); + let packages = await pkgManager.refresh(environment); if (packages && packages.length > 0) { - views.push(...packages.map((p) => new PackageTreeItem(p, parent, pkgManager))); + views.push( + ...packages + .sort((a, b) => (a.isTransitive === b.isTransitive ? 0 : a.isTransitive ? 1 : -1)) + .map((p) => new PackageTreeItem(p, parent, pkgManager)), + ); } else { views.push(new EnvInfoTreeItem(parent, ProjectViews.noPackages)); } diff --git a/src/features/views/projectView.ts b/src/features/views/projectView.ts index ba689ad9..e6fd7f3b 100644 --- a/src/features/views/projectView.ts +++ b/src/features/views/projectView.ts @@ -244,7 +244,7 @@ export class ProjectView implements TreeDataProvider { return [new ProjectEnvironmentInfo(environmentItem, ProjectViews.noPackageManager)]; } - let packages = await pkgManager.getPackages(environment); + let packages = await pkgManager.refresh(environment); if (!packages) { return [new ProjectEnvironmentInfo(environmentItem, ProjectViews.noPackages)]; } diff --git a/src/features/views/treeViewItems.ts b/src/features/views/treeViewItems.ts index f79cb948..0293afb7 100644 --- a/src/features/views/treeViewItems.ts +++ b/src/features/views/treeViewItems.ts @@ -1,4 +1,4 @@ -import { Command, MarkdownString, ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode'; +import { Command, MarkdownString, ThemeIcon, TreeItem, TreeItemCollapsibleState, l10n } from 'vscode'; import { EnvironmentGroupInfo, IconPath, Package, PythonEnvironment, PythonProject } from '../../api'; import { EnvViewStrings, UvInstallStrings, VenvManagerStrings } from '../../common/localize'; import { InternalEnvironmentManager, InternalPackageManager } from '../../internal.api'; @@ -210,10 +210,13 @@ export class PackageTreeItem implements EnvTreeItem { public readonly manager: InternalPackageManager, ) { const item = new TreeItem(pkg.displayName); - item.iconPath = pkg.iconPath; - item.contextValue = 'python-package'; - item.description = pkg.description ?? pkg.version; - item.tooltip = pkg.tooltip; + const defaultIcon = pkg.isTransitive ? new ThemeIcon('list-tree') : new ThemeIcon('package'); + item.iconPath = pkg.iconPath ?? defaultIcon; + item.contextValue = pkg.isTransitive ? 'python-package-transitive' : 'python-package'; + item.description = (pkg.isTransitive ? l10n.t('(transitive) ') : '') + (pkg.description ?? pkg.version); + item.tooltip = pkg.isTransitive + ? l10n.t('This package is a dependency of another installed package. It may also have been explicitly installed.') + : pkg.tooltip; this.treeItem = item; } } @@ -431,7 +434,7 @@ export class ProjectPackage implements ProjectTreeItem { this.id = ProjectPackage.getId(parent, pkg); const item = new TreeItem(this.pkg.displayName, TreeItemCollapsibleState.None); item.iconPath = this.pkg.iconPath; - item.contextValue = 'python-package'; + item.contextValue = this.pkg.isTransitive ? 'python-package-transitive' : 'python-package'; item.description = this.pkg.description ?? this.pkg.version; item.tooltip = this.pkg.tooltip; this.treeItem = item; diff --git a/src/internal.api.ts b/src/internal.api.ts index 4c80b527..801d4f91 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -364,7 +364,7 @@ export class InternalPackageManager implements PackageManager { } } - refresh(environment: PythonEnvironment): Promise { + refresh(environment: PythonEnvironment): Promise { return this.manager.refresh(environment); } @@ -446,6 +446,8 @@ export class PythonPackageImpl implements Package { public readonly iconPath?: IconPath; public readonly uris?: readonly Uri[]; + public readonly isTransitive?: boolean; + constructor( public readonly pkgId: PackageId, info: PackageInfo, @@ -457,6 +459,7 @@ export class PythonPackageImpl implements Package { this.tooltip = info.tooltip; this.iconPath = info.iconPath; this.uris = info.uris; + this.isTransitive = info.isTransitive; } } diff --git a/src/managers/builtin/pipListUtils.ts b/src/managers/builtin/pipListUtils.ts index e0ca55ca..80519d89 100644 --- a/src/managers/builtin/pipListUtils.ts +++ b/src/managers/builtin/pipListUtils.ts @@ -1,11 +1,20 @@ +import { LogOutputChannel } from 'vscode'; + export interface PipPackage { name: string; version: string; displayName: string; description: string; } +export function parseUvTree(data: string): string[] { + return data + .split('\n') + .map((line) => line.trim()) + .map((line) => line.split(/\s+/, 1)[0]) + .filter((name) => !!name); +} -export function parsePipListJson(data: string): PipPackage[] { +export function parsePipListJson(data: string, log?: LogOutputChannel): PipPackage[] { try { const json = JSON.parse(data); if (Array.isArray(json)) { @@ -18,8 +27,8 @@ export function parsePipListJson(data: string): PipPackage[] { description: version, })); } - } catch (_) { - // If JSON parsing fails, return an empty array. The caller can decide how to handle this case. + } catch (ex) { + log?.error('Failed to parse pip list JSON output', ex); } return []; } diff --git a/src/managers/builtin/pipPackageManager.ts b/src/managers/builtin/pipPackageManager.ts index 1a517adc..2a85f5ac 100644 --- a/src/managers/builtin/pipPackageManager.ts +++ b/src/managers/builtin/pipPackageManager.ts @@ -21,7 +21,7 @@ import { } from '../../api'; import { updatePackagesAndNotify } from '../common/packageChanges'; import { getWorkspacePackagesToInstall } from './pipUtils'; -import { managePackages, refreshPipPackages } from './utils'; +import { managePackages, normalizePackageName, refreshPipDirectPackageNames, refreshPipPackages } from './utils'; import { VenvManager } from './venvManager'; export class PipPackageManager implements PackageManager, Disposable { @@ -101,16 +101,21 @@ export class PipPackageManager implements PackageManager, Disposable { ); } - async refresh(environment: PythonEnvironment): Promise { - await window.withProgress( + async refresh(environment: PythonEnvironment): Promise { + return window.withProgress( { location: ProgressLocation.Window, title: 'Refreshing packages', }, async () => { - await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => { - this._onDidChangePackages.fire({ environment, manager: this, changes }); - }); + return updatePackagesAndNotify( + this, + environment, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, + ); }, ); } @@ -129,4 +134,15 @@ export class PipPackageManager implements PackageManager, Disposable { this._onDidChangePackages.dispose(); this.packages.clear(); } + + /** + * Returns direct (non-transitive) package names using `pip list --not-required` or `uv pip tree --depth=0`. + * + * Note: These commands return packages with no installed dependents (leaf packages), not packages + * the user explicitly installed. pip/uv do not track install intent. + */ + async getDirectPackageNames(environment: PythonEnvironment): Promise | undefined> { + const data = await refreshPipDirectPackageNames(environment, this.log); + return data ? new Set(data.map(normalizePackageName)) : undefined; + } } diff --git a/src/managers/builtin/utils.ts b/src/managers/builtin/utils.ts index 0e6f7c62..dc44fe75 100644 --- a/src/managers/builtin/utils.ts +++ b/src/managers/builtin/utils.ts @@ -23,7 +23,7 @@ import { } from '../common/nativePythonFinder'; import { shortenVersionString, sortEnvironments } from '../common/utils'; import { runPython, runUV, shouldUseUv } from './helpers'; -import { parsePipListJson, PipPackage } from './pipListUtils'; +import { parsePipListJson, parseUvTree, PipPackage } from './pipListUtils'; const PIXI_EXTENSION_ID = 'renan-r-santos.pixi-code'; const PIXI_RECOMMEND_DONT_ASK_KEY = 'pixi-extension-recommend-dont-ask'; @@ -200,7 +200,7 @@ async function execPipList(environment: PythonEnvironment, log?: LogOutputChanne try { return await runPython( environment.execInfo.run.executable, - ['-m', 'pip', 'list', '--format=json'], + ['-m', 'pip', 'list', '--format=json', ...(args ?? [])], undefined, log, undefined, @@ -235,7 +235,7 @@ export async function refreshPipPackages( data = await execPipList(environment, log); } - return parsePipListJson(data); + return parsePipListJson(data, log); } catch (e) { log?.error('Error refreshing packages', e); showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log); @@ -243,6 +243,34 @@ export async function refreshPipPackages( } } +/** + * Returns names of packages with no installed dependents (leaf packages). + * + * Uses `pip list --not-required` (pip) or `uv pip tree --depth=0` (uv). These report + * packages that nothing else depends on, which is a proxy for "directly installed" but + * not equivalent — e.g., `pip install flask werkzeug` will report werkzeug as having + * dependents (flask) even though the user installed it explicitly. + */ +export async function refreshPipDirectPackageNames( + environment: PythonEnvironment, + log?: LogOutputChannel, +): Promise { + const useUv = await shouldUseUv(log, environment.environmentPath.fsPath); + if (useUv) { + const treeOutput = await runUV( + ['pip', 'tree', '--python', environment.execInfo.run.executable, '--depth=0'], + undefined, + log, + undefined, + PIP_LIST_TIMEOUT_MS, + ); + return parseUvTree(treeOutput); + } + const data = await execPipList(environment, log, ['--not-required']); + const packages = parsePipListJson(data); + return packages.map((pkg) => pkg.name); +} + export async function managePackages( environment: PythonEnvironment, options: PackageManagementOptions, @@ -360,3 +388,7 @@ export async function resolveSystemPythonEnvironmentPath( } return undefined; } + +export function normalizePackageName(name: string): string { + return name.replace(/[-_.]+/g, '-').toLowerCase(); +} diff --git a/src/managers/common/packageChanges.ts b/src/managers/common/packageChanges.ts index c2afa122..3e16ae36 100644 --- a/src/managers/common/packageChanges.ts +++ b/src/managers/common/packageChanges.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { Package, PackageChangeKind, PackageManager, PythonEnvironment } from '../../api'; +import { normalizePackageName } from '../builtin/utils'; /** * Callback invoked with the computed changes when at least one change is detected. @@ -15,17 +16,17 @@ export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: P * @returns An array of changes indicating which packages were added or removed. */ export function getPackageChanges(before: Package[], after: Package[]): { kind: PackageChangeKind; pkg: Package }[] { - const beforeSet = new Set(before.map(({ name, version }) => `${name}==${version}`)); - const afterSet = new Set(after.map(({ name, version }) => `${name}==${version}`)); + const beforeSet = new Set(before.map(({ name, version }) => `${normalizePackageName(name)}==${version}`)); + const afterSet = new Set(after.map(({ name, version }) => `${normalizePackageName(name)}==${version}`)); const changes: { kind: PackageChangeKind; pkg: Package }[] = []; for (const pkg of after) { - if (!beforeSet.has(`${pkg.name}==${pkg.version}`)) { + if (!beforeSet.has(`${normalizePackageName(pkg.name)}==${pkg.version}`)) { changes.push({ kind: PackageChangeKind.add, pkg }); } } for (const pkg of before) { - if (!afterSet.has(`${pkg.name}==${pkg.version}`)) { + if (!afterSet.has(`${normalizePackageName(pkg.name)}==${pkg.version}`)) { changes.push({ kind: PackageChangeKind.remove, pkg }); } } @@ -46,10 +47,26 @@ export async function updatePackagesAndNotify( environment: PythonEnvironment, before: Package[] | undefined, onChanges: PackageChangesCallback, -): Promise { - const after = (await packageManager.getPackages(environment, { skipCache: true })) ?? []; - const changes = getPackageChanges(before ?? [], after); +): Promise { + const [after, afterDirectDependenciesNames] = await Promise.all([ + packageManager.getPackages(environment, { skipCache: true }).then((pkgs) => pkgs ?? []), + // Handle transitive dependencies (best-effort, don't break package refresh on failure) + packageManager.getDirectPackageNames?.(environment).catch(() => undefined), + ]); + + // Enrich packages with transitive dependency info (best-effort, creates new objects to respect readonly) + const enriched = afterDirectDependenciesNames && afterDirectDependenciesNames.size > 0 + ? after.map((pkg) => ({ + ...pkg, + isTransitive: !afterDirectDependenciesNames.has(normalizePackageName(pkg.name)), + })) + : after; + + // Fire change event + const changes = getPackageChanges(before ?? [], enriched); if (changes.length > 0) { onChanges(changes); } + + return enriched; } diff --git a/src/managers/conda/condaPackageManager.ts b/src/managers/conda/condaPackageManager.ts index 6a492ffa..27c26a4f 100644 --- a/src/managers/conda/condaPackageManager.ts +++ b/src/managers/conda/condaPackageManager.ts @@ -95,16 +95,21 @@ export class CondaPackageManager implements PackageManager, Disposable { ); } - async refresh(environment: PythonEnvironment): Promise { - await withProgress( + async refresh(environment: PythonEnvironment): Promise { + return withProgress( { location: ProgressLocation.Window, title: CondaStrings.condaRefreshingPackages, }, async () => { - await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => { - this._onDidChangePackages.fire({ environment, manager: this, changes }); - }); + return updatePackagesAndNotify( + this, + environment, + this.packages.get(environment.envId.id), + (changes) => { + this._onDidChangePackages.fire({ environment, manager: this, changes }); + }, + ); }, ); } diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 0498e225..4e72cd23 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -24,6 +24,7 @@ import { } from '../../api'; import { spawnProcess } from '../../common/childProcess.apis'; import { showErrorMessage, showInputBox, withProgress } from '../../common/window.apis'; +import { normalizePackageName } from '../builtin/utils'; import { updatePackagesAndNotify } from '../common/packageChanges'; import { PoetryManager } from './poetryManager'; import { getPoetry } from './poetryUtils'; @@ -108,15 +109,15 @@ export class PoetryPackageManager implements PackageManager, Disposable { ); } - async refresh(environment: PythonEnvironment): Promise { - await withProgress( + async refresh(environment: PythonEnvironment): Promise { + return withProgress( { location: ProgressLocation.Window, title: 'Refreshing Poetry packages', }, async () => { try { - await updatePackagesAndNotify( + return await updatePackagesAndNotify( this, environment, this.packages.get(environment.envId.id), @@ -133,6 +134,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { this.log.show(); } }); + return undefined; } }, ); @@ -263,6 +265,22 @@ export class PoetryPackageManager implements PackageManager, Disposable { // Convert to Package objects using the API return poetryPackages.map((pkg) => this.api.createPackageItem(pkg, environment, this)); } + + async getDirectPackageNames(_environment: PythonEnvironment): Promise | undefined> { + try { + const topLevelResult = await runPoetry(['show', '--no-ansi', '--top-level'], undefined, this.log); + const names = topLevelResult + .split('\n') + .map((line) => line.trim()) + .map((line) => line.match(/^([a-zA-Z0-9._-]+)/)?.[1] ?? '') + .filter((name) => !!name) + .map(normalizePackageName); + return new Set(names); + } catch (err) { + this.log.error(`Error fetching direct package names with Poetry: ${err}`); + return undefined; + } + } } export async function runPoetry( diff --git a/src/test/managers/builtin/normalizePackageName.unit.test.ts b/src/test/managers/builtin/normalizePackageName.unit.test.ts new file mode 100644 index 00000000..cd59e39b --- /dev/null +++ b/src/test/managers/builtin/normalizePackageName.unit.test.ts @@ -0,0 +1,46 @@ +import assert from 'assert'; +import { normalizePackageName } from '../../../managers/builtin/utils'; + +suite('normalizePackageName', () => { + test('should lowercase names', () => { + assert.strictEqual(normalizePackageName('Requests'), 'requests'); + assert.strictEqual(normalizePackageName('NUMPY'), 'numpy'); + }); + + test('should replace underscores with hyphens', () => { + assert.strictEqual(normalizePackageName('my_package'), 'my-package'); + }); + + test('should replace dots with hyphens', () => { + assert.strictEqual(normalizePackageName('zope.interface'), 'zope-interface'); + }); + + test('should collapse consecutive separators into a single hyphen', () => { + assert.strictEqual(normalizePackageName('my__package'), 'my-package'); + assert.strictEqual(normalizePackageName('my_-package'), 'my-package'); + assert.strictEqual(normalizePackageName('my_.package'), 'my-package'); + }); + + test('should handle mixed separators and casing', () => { + assert.strictEqual(normalizePackageName('My_Package.Name'), 'my-package-name'); + assert.strictEqual(normalizePackageName('Foo-Bar_Baz'), 'foo-bar-baz'); + }); + + test('should return already-normalized names unchanged', () => { + assert.strictEqual(normalizePackageName('requests'), 'requests'); + assert.strictEqual(normalizePackageName('my-package'), 'my-package'); + }); + + test('should handle single-word names', () => { + assert.strictEqual(normalizePackageName('pip'), 'pip'); + }); + + test('should produce equal results for equivalent package names', () => { + const variants = ['My_Package', 'my-package', 'my.package', 'My.Package', 'MY_PACKAGE', 'my_package']; + const normalized = variants.map(normalizePackageName); + assert.ok( + normalized.every((n) => n === normalized[0]), + `All variants should normalize to the same value, got: ${JSON.stringify(normalized)}`, + ); + }); +}); diff --git a/src/test/managers/builtin/pipListUtils.unit.test.ts b/src/test/managers/builtin/pipListUtils.unit.test.ts index 6ba342de..0bc978e9 100644 --- a/src/test/managers/builtin/pipListUtils.unit.test.ts +++ b/src/test/managers/builtin/pipListUtils.unit.test.ts @@ -1,12 +1,28 @@ import assert from 'assert'; import * as fs from 'fs-extra'; import * as path from 'path'; -import { parsePipListJson } from '../../../managers/builtin/pipListUtils'; +import * as sinon from 'sinon'; +import { LogOutputChannel } from 'vscode'; +import { parsePipListJson, parseUvTree } from '../../../managers/builtin/pipListUtils'; import { EXTENSION_TEST_ROOT } from '../../constants'; const TEST_DATA_ROOT = path.join(EXTENSION_TEST_ROOT, 'managers', 'builtin'); suite('Pip List JSON Parser tests', () => { + let log: LogOutputChannel; + + setup(() => { + log = { + error: sinon.stub(), + warn: sinon.stub(), + info: sinon.stub(), + } as unknown as LogOutputChannel; + }); + + teardown(() => { + sinon.restore(); + }); + const testNames = ['piplist1', 'piplist2', 'piplist3']; testNames.forEach((testName) => { @@ -16,7 +32,7 @@ suite('Pip List JSON Parser tests', () => { ); const pipListOutput = JSON.stringify(expected.packages); - const actualPackages = parsePipListJson(pipListOutput); + const actualPackages = parsePipListJson(pipListOutput, log); assert.equal(actualPackages.length, expected.packages.length, 'Unexpected number of packages'); actualPackages.forEach((actualPackage) => { @@ -36,12 +52,23 @@ suite('Pip List JSON Parser tests', () => { }); test('Returns an empty array for invalid JSON input', () => { - assert.deepStrictEqual(parsePipListJson('not json'), []); + assert.deepStrictEqual(parsePipListJson('not json', log), []); + }); + + test('Logs error when JSON parsing fails', () => { + parsePipListJson('not valid json', log); + assert.ok((log.error as sinon.SinonStub).calledOnce, 'Expected error to be logged'); + }); + + test('Returns empty array without logging when no log is provided', () => { + const result = parsePipListJson('not valid json'); + assert.deepStrictEqual(result, []); }); test('Skips items without a name or version', () => { const actualPackages = parsePipListJson( JSON.stringify([{ name: 'pip', version: '24.0' }, { name: 'setuptools' }, { version: '1.0.0' }]), + log, ); assert.deepStrictEqual(actualPackages, [ @@ -53,4 +80,44 @@ suite('Pip List JSON Parser tests', () => { }, ]); }); + + test('Returns empty array for non-array JSON', () => { + const result = parsePipListJson('{"name": "pip"}', log); + assert.deepStrictEqual(result, []); + }); + + test('Returns empty array for empty array JSON', () => { + const result = parsePipListJson('[]', log); + assert.deepStrictEqual(result, []); + }); +}); + +suite('parseUvTree tests', () => { + test('Parses uv pip tree output with depth 0', () => { + const input = 'requests v2.31.0\nflask v3.0.0\n'; + const result = parseUvTree(input); + assert.deepStrictEqual(result, ['requests', 'flask']); + }); + + test('Handles empty output', () => { + assert.deepStrictEqual(parseUvTree(''), []); + }); + + test('Filters blank lines', () => { + const input = 'requests v2.31.0\n\n\nflask v3.0.0\n'; + const result = parseUvTree(input); + assert.deepStrictEqual(result, ['requests', 'flask']); + }); + + test('Handles single package', () => { + const input = 'pip v24.0\n'; + const result = parseUvTree(input); + assert.deepStrictEqual(result, ['pip']); + }); + + test('Trims leading whitespace from indented lines', () => { + const input = ' requests v2.31.0\n flask v3.0.0\n'; + const result = parseUvTree(input); + assert.deepStrictEqual(result, ['requests', 'flask']); + }); }); diff --git a/src/test/managers/common/packageChanges.unit.test.ts b/src/test/managers/common/packageChanges.unit.test.ts index 81e8235f..1f65b3c7 100644 --- a/src/test/managers/common/packageChanges.unit.test.ts +++ b/src/test/managers/common/packageChanges.unit.test.ts @@ -169,5 +169,127 @@ suite('packageChanges', () => { assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.add)); assert.ok(changes.some((c: { kind: PackageChangeKind }) => c.kind === PackageChangeKind.remove)); }); + + test('marks transitive packages when getDirectPackageNames is provided', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'urllib3', version: '2.0.0' } as Package, + { name: 'charset-normalizer', version: '3.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().resolves(new Set(['requests'])); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, false, 'requests should be direct'); + assert.strictEqual(result![1].isTransitive, true, 'urllib3 should be transitive'); + assert.strictEqual(result![2].isTransitive, true, 'charset-normalizer should be transitive'); + // Original objects should not be mutated + assert.strictEqual(after[0].isTransitive, undefined, 'original should not be mutated'); + }); + + test('does not mark packages transitive when getDirectPackageNames is not implemented', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'urllib3', version: '2.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(result![1].isTransitive, undefined, 'should not be set'); + }); + + test('does not mark packages transitive when getDirectPackageNames returns undefined', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'urllib3', version: '2.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().resolves(undefined); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(result![1].isTransitive, undefined, 'should not be set'); + }); + + test('does not mark packages transitive when getDirectPackageNames returns empty set', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'urllib3', version: '2.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().resolves(new Set()); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(result![1].isTransitive, undefined, 'should not be set'); + }); + + test('all packages marked direct when all are in direct set', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'flask', version: '3.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().resolves(new Set(['requests', 'flask'])); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, false, 'requests should be direct'); + assert.strictEqual(result![1].isTransitive, false, 'flask should be direct'); + }); + + test('all packages marked transitive when none are in direct set', async () => { + const after = [ + { name: 'urllib3', version: '2.0.0' } as Package, + { name: 'charset-normalizer', version: '3.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().resolves(new Set(['requests'])); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, true, 'urllib3 should be transitive'); + assert.strictEqual(result![1].isTransitive, true, 'charset-normalizer should be transitive'); + }); + + test('leaves isTransitive undefined when getDirectPackageNames throws', async () => { + const after = [ + { name: 'requests', version: '2.31.0' } as Package, + { name: 'urllib3', version: '2.0.0' } as Package, + ]; + getPackagesStub.resolves(after); + const getDirectPackageNamesStub = sinon.stub().rejects(new Error('command failed')); + (packageManager as unknown as Record).getDirectPackageNames = getDirectPackageNamesStub; + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.ok(result); + assert.strictEqual(result![0].isTransitive, undefined, 'should not be set on error'); + assert.strictEqual(result![1].isTransitive, undefined, 'should not be set on error'); + assert.ok(onChanges.calledOnce, 'should still fire change event'); + }); }); });