diff --git a/api/src/main.ts b/api/src/main.ts index aa381414..e7ac1ec0 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; } /** @@ -687,6 +692,13 @@ export interface PackageManager { */ onDidChangePackages?: Event; + /** + * Fetches the names of direct (non-transitive) packages for the specified Python environment. + * @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. diff --git a/src/api.ts b/src/api.ts index b3ab24cb..d57d915b 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; } /** @@ -681,6 +686,13 @@ export interface PackageManager { */ onDidChangePackages?: Event; + /** + * Fetches the names of direct (non-transitive) packages for the specified Python environment. + * @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. diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index a10f8885..407e09ec 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -305,6 +305,9 @@ 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) { + 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/views/envManagersView.ts b/src/features/views/envManagersView.ts index 131484e6..de5ec305 100644 --- a/src/features/views/envManagersView.ts +++ b/src/features/views/envManagersView.ts @@ -252,9 +252,17 @@ export class EnvManagerView implements TreeDataProvider, Disposable const views: EnvTreeItem[] = []; if (pkgManager) { - const packages = await pkgManager.getPackages(environment); + let packages = await pkgManager.getPackages(environment); + if (!packages || packages.length === 0) { + await pkgManager.refresh(environment); + packages = await pkgManager.getPackages(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..38ccc469 100644 --- a/src/features/views/projectView.ts +++ b/src/features/views/projectView.ts @@ -245,6 +245,10 @@ export class ProjectView implements TreeDataProvider { } let packages = await pkgManager.getPackages(environment); + if (!packages || packages.length === 0) { + await pkgManager.refresh(environment); + packages = await pkgManager.getPackages(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..577ddf8c 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,9 +210,10 @@ 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; + 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.tooltip; this.treeItem = item; } @@ -431,7 +432,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..8d2f547a 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -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..8e39e8d7 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, refreshPipDirectPackageNames, refreshPipPackages } from './utils'; import { VenvManager } from './venvManager'; export class PipPackageManager implements PackageManager, Disposable { @@ -129,4 +129,9 @@ export class PipPackageManager implements PackageManager, Disposable { this._onDidChangePackages.dispose(); this.packages.clear(); } + + async getDirectPackageNames(environment: PythonEnvironment): Promise | undefined> { + const data = await refreshPipDirectPackageNames(environment, this.log); + return data ? new Set(data) : undefined; + } } diff --git a/src/managers/builtin/utils.ts b/src/managers/builtin/utils.ts index 0e6f7c62..a57e7a3b 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,26 @@ export async function refreshPipPackages( } } +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, diff --git a/src/managers/common/packageChanges.ts b/src/managers/common/packageChanges.ts index c2afa122..cbcfba22 100644 --- a/src/managers/common/packageChanges.ts +++ b/src/managers/common/packageChanges.ts @@ -48,6 +48,21 @@ export async function updatePackagesAndNotify( onChanges: PackageChangesCallback, ): Promise { const after = (await packageManager.getPackages(environment, { skipCache: true })) ?? []; + + // Handle transitive dependencies (best-effort, don't break package refresh on failure) + let afterDirectDependenciesNames: Set | undefined; + try { + afterDirectDependenciesNames = await packageManager.getDirectPackageNames?.(environment); + } catch { + // If direct package detection fails, leave isTransitive undefined rather than breaking refresh + } + if (afterDirectDependenciesNames && afterDirectDependenciesNames.size > 0) { + for (const pkg of after) { + (pkg as { isTransitive?: boolean }).isTransitive = !afterDirectDependenciesNames.has(pkg.name); + } + } + + // Fire change event const changes = getPackageChanges(before ?? [], after); if (changes.length > 0) { onChanges(changes); diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 0498e225..b4d646f4 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -263,6 +263,21 @@ 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); + 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/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..ca1b064b 100644 --- a/src/test/managers/common/packageChanges.unit.test.ts +++ b/src/test/managers/common/packageChanges.unit.test.ts @@ -169,5 +169,118 @@ 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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, false, 'requests should be direct'); + assert.strictEqual(after[1].isTransitive, true, 'urllib3 should be transitive'); + assert.strictEqual(after[2].isTransitive, true, 'charset-normalizer should be transitive'); + }); + + 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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(after[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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(after[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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, undefined, 'should not be set'); + assert.strictEqual(after[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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, false, 'requests should be direct'); + assert.strictEqual(after[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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, true, 'urllib3 should be transitive'); + assert.strictEqual(after[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(); + + await updatePackagesAndNotify(packageManager, environment, undefined, onChanges); + + assert.strictEqual(after[0].isTransitive, undefined, 'should not be set on error'); + assert.strictEqual(after[1].isTransitive, undefined, 'should not be set on error'); + assert.ok(onChanges.calledOnce, 'should still fire change event'); + }); }); });