From 62bfe6366533fc5030546d5e0aec982a7bbfcc88 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 11 Dec 2025 13:16:48 -0800 Subject: [PATCH 01/10] add validatePyprojectToml --- src/managers/builtin/pipUtils.ts | 133 +++++++++++++++++- src/managers/builtin/venvManager.ts | 4 +- src/managers/builtin/venvStepBasedFlow.ts | 3 +- src/managers/builtin/venvUtils.ts | 3 +- .../managers/builtin/pipUtils.unit.test.ts | 10 +- 5 files changed, 138 insertions(+), 15 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index d221f5a8..6a959001 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -1,7 +1,7 @@ import * as tomljs from '@iarna/toml'; import * as fse from 'fs-extra'; import * as path from 'path'; -import { LogOutputChannel, ProgressLocation, QuickInputButtons, QuickPickItem, Uri } from 'vscode'; +import { l10n, LogOutputChannel, ProgressLocation, QuickInputButtons, QuickPickItem, Uri } from 'vscode'; import { PackageManagementOptions, PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { PackageManagement, Pickers, VenvManagerStrings } from '../../common/localize'; @@ -13,6 +13,88 @@ import { Installable } from '../common/types'; import { mergePackages } from '../common/utils'; import { refreshPipPackages } from './utils'; +/** + * Validates pyproject.toml fields according to PEP 508, PEP 440, PEP 621, PEP 517/518 + * Returns error message if invalid, undefined if valid + */ +function validatePyprojectToml(toml: tomljs.JsonMap, filePath: string): string | undefined { + // 1. Validate package name (PEP 508) + if (toml.project && (toml.project as tomljs.JsonMap).name) { + const name = (toml.project as tomljs.JsonMap).name as string; + // PEP 508 regex: must start and end with a letter or digit, can contain -_. + const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/; + if (!nameRegex.test(name)) { + return l10n.t( + 'Invalid package name "{0}" in {1}. Package names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters. No spaces allowed. See PEP 508: https://peps.python.org/pep-0508/', + name, + path.basename(filePath), + ); + } + } + + // 2. Validate version format (PEP 440) + if (toml.project && (toml.project as tomljs.JsonMap).version) { + const version = (toml.project as tomljs.JsonMap).version as string; + // PEP 440 simplified regex + const versionRegex = + /^([1-9][0-9]*!)?(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*((a|b|rc)(0|[1-9][0-9]*))?(\.post(0|[1-9][0-9]*))?(\.dev(0|[1-9][0-9]*))?$/; + if (!versionRegex.test(version)) { + return l10n.t( + 'Invalid version "{0}" in {1}. Versions must follow PEP 440 format (e.g., "1.0.0", "2.1a3"). See https://peps.python.org/pep-0440/', + version, + path.basename(filePath), + ); + } + } + + // 3. Validate required fields (PEP 621) + if (toml.project) { + const project = toml.project as tomljs.JsonMap; + if (!project.name) { + return l10n.t( + 'Missing required field "name" in [project] section of {0}. See PEP 621: https://peps.python.org/pep-0621/', + path.basename(filePath), + ); + } + } + + // 4. Validate build system (PEP 517/518) + if (toml['build-system']) { + const buildSystem = toml['build-system'] as tomljs.JsonMap; + if (!buildSystem.requires) { + return l10n.t( + 'Missing required field "requires" in [build-system] section of {0}. See PEP 517: https://peps.python.org/pep-0517/', + path.basename(filePath), + ); + } + if (!buildSystem['build-backend']) { + return l10n.t( + 'Missing required field "build-backend" in [build-system] section of {0}. See PEP 518: https://peps.python.org/pep-0518/', + path.basename(filePath), + ); + } + } + + // 5. Validate dependencies format (PEP 508) + if (toml.project && (toml.project as tomljs.JsonMap).dependencies) { + const deps = (toml.project as tomljs.JsonMap).dependencies as string[]; + if (Array.isArray(deps)) { + for (const dep of deps) { + // Basic check for common mistakes + if (dep.includes(' ') || /\s{2,}/.test(dep)) { + return l10n.t( + 'Invalid dependency "{0}" in {1}. Contains extra whitespace. See PEP 508: https://peps.python.org/pep-0508/', + dep, + path.basename(filePath), + ); + } + } + } + } + + return undefined; // No errors +} + async function tomlParse(fsPath: string, log?: LogOutputChannel): Promise { try { const content = await fse.readFile(fsPath, 'utf-8'); @@ -148,6 +230,28 @@ export interface PipPackages { uninstall: string[]; } +export interface ProjectInstallableResult { + /** + * List of installable packages from requirements.txt and pyproject.toml files + */ + installables: Installable[]; + + /** + * Validation error information if pyproject.toml validation failed + */ + validationError?: { + /** + * Human-readable error message describing the validation issue + */ + message: string; + + /** + * URI to the pyproject.toml file that has the validation error + */ + fileUri: Uri; + }; +} + export async function getWorkspacePackagesToInstall( api: PythonEnvironmentApi, options: PackageManagementOptions, @@ -155,7 +259,8 @@ export async function getWorkspacePackagesToInstall( environment?: PythonEnvironment, log?: LogOutputChannel, ): Promise { - const installable = (await getProjectInstallable(api, project)) ?? []; + const result = await getProjectInstallable(api, project); + const installable = result.installables; let common = await getCommonPackages(); let installed: string[] | undefined; if (environment) { @@ -168,12 +273,14 @@ export async function getWorkspacePackagesToInstall( export async function getProjectInstallable( api: PythonEnvironmentApi, projects?: PythonProject[], -): Promise { +): Promise { if (!projects) { - return []; + return { installables: [] }; } const exclude = '**/{.venv*,.git,.nox,.tox,.conda,site-packages,__pypackages__}/**'; const installable: Installable[] = []; + let validationError: { message: string; fileUri: Uri } | undefined; + await withProgress( { location: ProgressLocation.Notification, @@ -204,6 +311,18 @@ export async function getProjectInstallable( filtered.map(async (uri) => { if (uri.fsPath.endsWith('.toml')) { const toml = await tomlParse(uri.fsPath); + + // Validate pyproject.toml and capture first error only + if (!validationError) { + const error = validatePyprojectToml(toml, uri.fsPath); + if (error) { + validationError = { + message: error, + fileUri: uri, + }; + } + } + installable.push(...getTomlInstallable(toml, uri)); } else { const name = path.basename(uri.fsPath); @@ -219,7 +338,11 @@ export async function getProjectInstallable( ); }, ); - return installable; + + return { + installables: installable, + validationError, + }; } export function isPipInstallCommand(command: string): boolean { diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index f0a2af14..e042cc11 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -232,9 +232,7 @@ export class VenvManager implements EnvironmentManager { } } else if (result?.envCreationErr) { // Show error message to user when environment creation failed - showErrorMessage( - l10n.t('Failed to create virtual environment: {0}', result.envCreationErr), - ); + showErrorMessage(l10n.t('Failed to create virtual environment: {0}', result.envCreationErr)); } return result?.environment ?? undefined; } finally { diff --git a/src/managers/builtin/venvStepBasedFlow.ts b/src/managers/builtin/venvStepBasedFlow.ts index beb7c1d2..87693c83 100644 --- a/src/managers/builtin/venvStepBasedFlow.ts +++ b/src/managers/builtin/venvStepBasedFlow.ts @@ -335,7 +335,8 @@ export async function createStepBasedVenvFlow( // Get workspace dependencies to install const project = api.getPythonProject(venvRoot); - const installables = await getProjectInstallable(api, project ? [project] : undefined); + const result = await getProjectInstallable(api, project ? [project] : undefined); + const installables = result.installables; const allPackages = []; allPackages.push(...(installables?.flatMap((i) => i.args ?? []) ?? [])); if (options.additionalPackages) { diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index 6ee09d21..bf7e7f58 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -396,7 +396,8 @@ export async function quickCreateVenv( const project = api.getPythonProject(venvRoot); sendTelemetryEvent(EventNames.VENV_CREATION, undefined, { creationType: 'quick' }); - const installables = await getProjectInstallable(api, project ? [project] : undefined); + const result = await getProjectInstallable(api, project ? [project] : undefined); + const installables = result.installables; const allPackages = []; allPackages.push(...(installables?.flatMap((i) => i.args ?? []) ?? [])); if (additionalPackages) { diff --git a/src/test/managers/builtin/pipUtils.unit.test.ts b/src/test/managers/builtin/pipUtils.unit.test.ts index 076596e8..5ae58126 100644 --- a/src/test/managers/builtin/pipUtils.unit.test.ts +++ b/src/test/managers/builtin/pipUtils.unit.test.ts @@ -74,7 +74,7 @@ suite('Pip Utils - getProjectInstallable', () => { // Act: Call getProjectInstallable const workspacePath = Uri.file('/test/path/root').fsPath; const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; - const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + const result = (await getProjectInstallable(mockApi as PythonEnvironmentApi, projects)).installables; // Assert: Should find all three requirements files assert.strictEqual(result.length, 3, 'Should find three requirements files'); @@ -119,7 +119,7 @@ suite('Pip Utils - getProjectInstallable', () => { // Act: Call getProjectInstallable const workspacePath = Uri.file('/test/path/root').fsPath; const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; - const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + const result = (await getProjectInstallable(mockApi as PythonEnvironmentApi, projects)).installables; // Assert: Should deduplicate and only have 2 unique files assert.strictEqual(result.length, 2, 'Should deduplicate and have 2 unique files'); @@ -149,7 +149,7 @@ suite('Pip Utils - getProjectInstallable', () => { // Act: Call getProjectInstallable const workspacePath = Uri.file('/test/path/root').fsPath; const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; - const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + const result = (await getProjectInstallable(mockApi as PythonEnvironmentApi, projects)).installables; // Assert: Should find all files assert.strictEqual(result.length, 3, 'Should find three files'); @@ -164,7 +164,7 @@ suite('Pip Utils - getProjectInstallable', () => { test('should return empty array when no projects provided', async () => { // Act: Call with no projects - const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, undefined); + const result = (await getProjectInstallable(mockApi as PythonEnvironmentApi, undefined)).installables; // Assert: Should return empty array assert.strictEqual(result.length, 0, 'Should return empty array'); @@ -189,7 +189,7 @@ suite('Pip Utils - getProjectInstallable', () => { // Act: Call with only workspace project const workspacePath = Uri.file('/test/path/root').fsPath; const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }]; - const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects); + const result = (await getProjectInstallable(mockApi as PythonEnvironmentApi, projects)).installables; // Assert: Should only include files from workspace assert.strictEqual(result.length, 1, 'Should only include files from project directory'); From f1e448643d39d852f769659db72507547e89b434 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 11 Dec 2025 15:58:56 -0800 Subject: [PATCH 02/10] add shouldProceedAfterPyprojectValidation --- src/common/localize.ts | 10 +++ src/managers/builtin/pipUtils.ts | 90 ++++++++++++++++++----- src/managers/builtin/venvStepBasedFlow.ts | 14 +++- src/managers/builtin/venvUtils.ts | 8 +- 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/common/localize.ts b/src/common/localize.ts index 2077430b..0e2acf5d 100644 --- a/src/common/localize.ts +++ b/src/common/localize.ts @@ -64,6 +64,16 @@ export namespace Pickers { export const selectProject = l10n.t('Select a project, folder or script'); export const selectProjects = l10n.t('Select one or more projects, folders or scripts'); } + + export namespace pyProject { + export const validationError = l10n.t('Invalid pyproject.toml'); + export const validationErrorAction = l10n.t( + 'The pyproject.toml file has formatting errors. What would you like to do?', + ); + export const openFile = l10n.t('Open pyproject.toml'); + export const continueAnyway = l10n.t('Continue Anyway'); + export const cancel = l10n.t('Cancel'); + } } export namespace ProjectViews { diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index 6a959001..db2112ed 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -1,7 +1,7 @@ import * as tomljs from '@iarna/toml'; import * as fse from 'fs-extra'; import * as path from 'path'; -import { l10n, LogOutputChannel, ProgressLocation, QuickInputButtons, QuickPickItem, Uri } from 'vscode'; +import { l10n, LogOutputChannel, ProgressLocation, QuickInputButtons, QuickPickItem, Uri, window } from 'vscode'; import { PackageManagementOptions, PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import { PackageManagement, Pickers, VenvManagerStrings } from '../../common/localize'; @@ -160,11 +160,12 @@ async function getCommonPackages(): Promise { } async function selectWorkspaceOrCommon( - installable: Installable[], + installableResult: ProjectInstallableResult, common: Installable[], showSkipOption: boolean, installed: string[], ): Promise { + const installable = installableResult.installables; if (installable.length === 0 && common.length === 0) { return undefined; } @@ -206,7 +207,20 @@ async function selectWorkspaceOrCommon( if (selected && !Array.isArray(selected)) { try { if (selected.label === PackageManagement.workspaceDependencies) { - return await selectFromInstallableToInstall(installable, undefined, { showBackButton }); + const selectedInstallables = await selectFromInstallableToInstall(installable, undefined, { + showBackButton, + }); + + const validationError = installableResult.validationError; + const shouldProceed = await shouldProceedAfterPyprojectValidation( + validationError, + selectedInstallables?.install ?? [], + ); + if (!shouldProceed) { + return undefined; + } + + return selectedInstallables; } else if (selected.label === PackageManagement.searchCommonPackages) { return await selectFromCommonPackagesToInstall(common, installed, undefined, { showBackButton }); } else if (selected.label === PackageManagement.skipPackageInstallation) { @@ -218,7 +232,7 @@ async function selectWorkspaceOrCommon( // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (ex: any) { if (ex === QuickInputButtons.Back) { - return selectWorkspaceOrCommon(installable, common, showSkipOption, installed); + return selectWorkspaceOrCommon(installableResult, common, showSkipOption, installed); } } } @@ -239,17 +253,19 @@ export interface ProjectInstallableResult { /** * Validation error information if pyproject.toml validation failed */ - validationError?: { - /** - * Human-readable error message describing the validation issue - */ - message: string; - - /** - * URI to the pyproject.toml file that has the validation error - */ - fileUri: Uri; - }; + validationError?: ValidationError; +} + +export interface ValidationError { + /** + * Human-readable error message describing the validation issue + */ + message: string; + + /** + * URI to the pyproject.toml file that has the validation error + */ + fileUri: Uri; } export async function getWorkspacePackagesToInstall( @@ -259,15 +275,14 @@ export async function getWorkspacePackagesToInstall( environment?: PythonEnvironment, log?: LogOutputChannel, ): Promise { - const result = await getProjectInstallable(api, project); - const installable = result.installables; + const installableResult = await getProjectInstallable(api, project); let common = await getCommonPackages(); let installed: string[] | undefined; if (environment) { installed = (await refreshPipPackages(environment, log, { showProgress: true }))?.map((pkg) => pkg.name); common = mergePackages(common, installed ?? []); } - return selectWorkspaceOrCommon(installable, common, !!options.showSkipOption, installed ?? []); + return selectWorkspaceOrCommon(installableResult, common, !!options.showSkipOption, installed ?? []); } export async function getProjectInstallable( @@ -345,6 +360,45 @@ export async function getProjectInstallable( }; } +export async function shouldProceedAfterPyprojectValidation( + validationError: ValidationError | undefined, + install: string[], +): Promise { + // 1. If no validation error or no installables selected, proceed + if (!validationError || install.length === 0) { + return true; + } + + const selectedTomlInstallables = install.some((arg, index, arr) => arg === '-e' && index + 1 < arr.length); + if (!selectedTomlInstallables) { + // 2. If no toml installables selected, proceed + return true; + } + + // 3. Otherwise, show error message and ask user what to do + const openButton = { title: Pickers.pyProject.openFile }; + const continueButton = { title: Pickers.pyProject.continueAnyway }; + const cancelButton = { title: Pickers.pyProject.cancel, isCloseAffordance: true }; + + const selection = await window.showErrorMessage( + validationError.message, + { modal: true }, + openButton, + continueButton, + cancelButton, + ); + + if (selection === continueButton) { + return true; + } + + if (selection === openButton) { + await window.showTextDocument(validationError.fileUri); + } + + return false; +} + export function isPipInstallCommand(command: string): boolean { // Regex to match pip install commands, capturing variations like: // pip install package diff --git a/src/managers/builtin/venvStepBasedFlow.ts b/src/managers/builtin/venvStepBasedFlow.ts index 87693c83..2e8e7ded 100644 --- a/src/managers/builtin/venvStepBasedFlow.ts +++ b/src/managers/builtin/venvStepBasedFlow.ts @@ -7,7 +7,12 @@ import { EventNames } from '../../common/telemetry/constants'; import { sendTelemetryEvent } from '../../common/telemetry/sender'; import { showInputBoxWithButtons, showQuickPickWithButtons } from '../../common/window.apis'; import { NativePythonFinder } from '../common/nativePythonFinder'; -import { getProjectInstallable, getWorkspacePackagesToInstall, PipPackages } from './pipUtils'; +import { + getProjectInstallable, + getWorkspacePackagesToInstall, + PipPackages, + shouldProceedAfterPyprojectValidation, +} from './pipUtils'; import { CreateEnvironmentResult, createWithProgress, ensureGlobalEnv } from './venvUtils'; /** @@ -342,6 +347,13 @@ export async function createStepBasedVenvFlow( if (options.additionalPackages) { allPackages.push(...options.additionalPackages); } + + const validationError = result.validationError; + const shouldProceed = await shouldProceedAfterPyprojectValidation(validationError, allPackages); + if (!shouldProceed) { + return undefined; + } + return await createWithProgress(nativeFinder, api, log, manager, state.basePython, venvRoot, quickEnvPath, { install: allPackages, uninstall: [], diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index bf7e7f58..9c778317 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -26,7 +26,7 @@ import { } from '../common/nativePythonFinder'; import { getShellActivationCommands, shortVersion, sortEnvironments } from '../common/utils'; import { runPython, runUV, shouldUseUv } from './helpers'; -import { getProjectInstallable, PipPackages } from './pipUtils'; +import { getProjectInstallable, PipPackages, shouldProceedAfterPyprojectValidation } from './pipUtils'; import { resolveSystemPythonEnvironmentPath } from './utils'; import { addUvEnvironment, removeUvEnvironment, UV_ENVS_KEY } from './uvEnvironments'; import { createStepBasedVenvFlow } from './venvStepBasedFlow'; @@ -404,6 +404,12 @@ export async function quickCreateVenv( allPackages.push(...additionalPackages); } + const validationError = result.validationError; + const shouldProceed = await shouldProceedAfterPyprojectValidation(validationError, allPackages); + if (!shouldProceed) { + return undefined; + } + // Check if .venv already exists let venvPath = path.join(venvRoot.fsPath, '.venv'); if (await fsapi.pathExists(venvPath)) { From c63db5ecd1d3f8fed0e2c48235511ae2c5f4934b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 11 Dec 2025 16:35:54 -0800 Subject: [PATCH 03/10] add tests --- .../helpers.validatePyproject.unit.test.ts | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/test/managers/builtin/helpers.validatePyproject.unit.test.ts diff --git a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts new file mode 100644 index 00000000..01f31e6b --- /dev/null +++ b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts @@ -0,0 +1,67 @@ +import assert from 'assert'; +import { Uri } from 'vscode'; +import { shouldProceedAfterPyprojectValidation, ValidationError } from '../../../managers/builtin/pipUtils'; + +suite('pipUtils - validatePyproject', () => { + const mockValidationError: ValidationError = { + message: 'Invalid package name "my package" in pyproject.toml.', + fileUri: Uri.file('/test/path/pyproject.toml'), + }; + + test('should return true when no validation error exists', async () => { + // Arrange: no validation error + const validationError = undefined; + const install = ['-e', '/test/path']; + + // Act + const result = await shouldProceedAfterPyprojectValidation(validationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when no validation error'); + }); + + test('should return true when install array is empty', async () => { + // Arrange: validation error exists but no packages selected + const install: string[] = []; + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when no packages selected'); + }); + + test('should return true when only requirements.txt packages selected (no -e flag)', async () => { + // Arrange: validation error exists but only requirements.txt packages selected + const install = ['-r', '/test/requirements.txt']; + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when no TOML packages selected'); + }); + + test('should return true when only PyPI packages selected (no flags at all)', async () => { + // Arrange: only PyPI package names, no flags + const install = ['numpy', 'pandas', 'requests']; + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when only PyPI packages selected'); + }); + + test('should not trigger on -e flag at end of array without following argument', async () => { + // Arrange: -e flag is last item (malformed, but should not crash) + const install = ['numpy', '-e']; + // This is edge case - -e at end means no path follows, so index + 1 < arr.length is false + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should not crash on malformed -e flag at end'); + }); +}); From d348717150d23b4b839846a1ddd4197d8184e7f1 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 11 Dec 2025 17:14:20 -0800 Subject: [PATCH 04/10] update dialog --- src/managers/builtin/pipUtils.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index db2112ed..dfb473d7 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -380,13 +380,7 @@ export async function shouldProceedAfterPyprojectValidation( const continueButton = { title: Pickers.pyProject.continueAnyway }; const cancelButton = { title: Pickers.pyProject.cancel, isCloseAffordance: true }; - const selection = await window.showErrorMessage( - validationError.message, - { modal: true }, - openButton, - continueButton, - cancelButton, - ); + const selection = await window.showErrorMessage(validationError.message, openButton, continueButton, cancelButton); if (selection === continueButton) { return true; From d277639a9cf686e7d10368abd85f6fb64d42503b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 12 Dec 2025 10:41:20 -0800 Subject: [PATCH 05/10] improve error messages --- src/common/localize.ts | 5 +-- src/managers/builtin/pipUtils.ts | 74 +++++++++----------------------- 2 files changed, 22 insertions(+), 57 deletions(-) diff --git a/src/common/localize.ts b/src/common/localize.ts index 0e2acf5d..fd9a8843 100644 --- a/src/common/localize.ts +++ b/src/common/localize.ts @@ -66,10 +66,7 @@ export namespace Pickers { } export namespace pyProject { - export const validationError = l10n.t('Invalid pyproject.toml'); - export const validationErrorAction = l10n.t( - 'The pyproject.toml file has formatting errors. What would you like to do?', - ); + export const validationErrorAction = l10n.t(' What would you like to do?'); export const openFile = l10n.t('Open pyproject.toml'); export const continueAnyway = l10n.t('Continue Anyway'); export const cancel = l10n.t('Cancel'); diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index dfb473d7..d081480c 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -13,86 +13,49 @@ import { Installable } from '../common/types'; import { mergePackages } from '../common/utils'; import { refreshPipPackages } from './utils'; -/** - * Validates pyproject.toml fields according to PEP 508, PEP 440, PEP 621, PEP 517/518 - * Returns error message if invalid, undefined if valid - */ -function validatePyprojectToml(toml: tomljs.JsonMap, filePath: string): string | undefined { +function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined { // 1. Validate package name (PEP 508) if (toml.project && (toml.project as tomljs.JsonMap).name) { const name = (toml.project as tomljs.JsonMap).name as string; - // PEP 508 regex: must start and end with a letter or digit, can contain -_. + // PEP 508 regex: must start and end with a letter or digit, can contain -_., and alphanumeric characters. No spaces allowed. + // See https://peps.python.org/pep-0508/ const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/; if (!nameRegex.test(name)) { - return l10n.t( - 'Invalid package name "{0}" in {1}. Package names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters. No spaces allowed. See PEP 508: https://peps.python.org/pep-0508/', - name, - path.basename(filePath), - ); + return l10n.t('Invalid package name "{0}" in pyproject.toml.', name); } } // 2. Validate version format (PEP 440) if (toml.project && (toml.project as tomljs.JsonMap).version) { const version = (toml.project as tomljs.JsonMap).version as string; - // PEP 440 simplified regex + // PEP 440 version regex. Versions must follow PEP 440 format (e.g., "1.0.0", "2.1a3"). + // See https://peps.python.org/pep-0440/ const versionRegex = - /^([1-9][0-9]*!)?(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*((a|b|rc)(0|[1-9][0-9]*))?(\.post(0|[1-9][0-9]*))?(\.dev(0|[1-9][0-9]*))?$/; + /^([0-9]+!)?(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*((a|b|c|rc)([0-9]+)?)?(\.post([0-9]+)?)?(\.dev([0-9]+)?)?(\+[a-zA-Z0-9._-]+)?$/; if (!versionRegex.test(version)) { - return l10n.t( - 'Invalid version "{0}" in {1}. Versions must follow PEP 440 format (e.g., "1.0.0", "2.1a3"). See https://peps.python.org/pep-0440/', - version, - path.basename(filePath), - ); + return l10n.t('Invalid version "{0}" in pyproject.toml.', version); } } // 3. Validate required fields (PEP 621) if (toml.project) { const project = toml.project as tomljs.JsonMap; + // See PEP 621: https://peps.python.org/pep-0621/ if (!project.name) { - return l10n.t( - 'Missing required field "name" in [project] section of {0}. See PEP 621: https://peps.python.org/pep-0621/', - path.basename(filePath), - ); + return l10n.t('Missing required field "name" in [project] section of pyproject.toml.'); } } - // 4. Validate build system (PEP 517/518) + // 4. Validate build system (PEP 518) if (toml['build-system']) { const buildSystem = toml['build-system'] as tomljs.JsonMap; + // See PEP 518: https://peps.python.org/pep-0518/ if (!buildSystem.requires) { - return l10n.t( - 'Missing required field "requires" in [build-system] section of {0}. See PEP 517: https://peps.python.org/pep-0517/', - path.basename(filePath), - ); - } - if (!buildSystem['build-backend']) { - return l10n.t( - 'Missing required field "build-backend" in [build-system] section of {0}. See PEP 518: https://peps.python.org/pep-0518/', - path.basename(filePath), - ); + return l10n.t('Missing required field "requires" in [build-system] section of pyproject.toml.'); } } - // 5. Validate dependencies format (PEP 508) - if (toml.project && (toml.project as tomljs.JsonMap).dependencies) { - const deps = (toml.project as tomljs.JsonMap).dependencies as string[]; - if (Array.isArray(deps)) { - for (const dep of deps) { - // Basic check for common mistakes - if (dep.includes(' ') || /\s{2,}/.test(dep)) { - return l10n.t( - 'Invalid dependency "{0}" in {1}. Contains extra whitespace. See PEP 508: https://peps.python.org/pep-0508/', - dep, - path.basename(filePath), - ); - } - } - } - } - - return undefined; // No errors + return undefined; } async function tomlParse(fsPath: string, log?: LogOutputChannel): Promise { @@ -329,7 +292,7 @@ export async function getProjectInstallable( // Validate pyproject.toml and capture first error only if (!validationError) { - const error = validatePyprojectToml(toml, uri.fsPath); + const error = validatePyprojectToml(toml); if (error) { validationError = { message: error, @@ -380,7 +343,12 @@ export async function shouldProceedAfterPyprojectValidation( const continueButton = { title: Pickers.pyProject.continueAnyway }; const cancelButton = { title: Pickers.pyProject.cancel, isCloseAffordance: true }; - const selection = await window.showErrorMessage(validationError.message, openButton, continueButton, cancelButton); + const selection = await window.showErrorMessage( + validationError.message + Pickers.pyProject.validationErrorAction, + openButton, + continueButton, + cancelButton, + ); if (selection === continueButton) { return true; From b18e19e43432e9bb3e541d26c3e81652f631966e Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 12 Dec 2025 10:51:59 -0800 Subject: [PATCH 06/10] improve version regex --- src/managers/builtin/pipUtils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index d081480c..9310db9b 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -30,8 +30,10 @@ function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined { const version = (toml.project as tomljs.JsonMap).version as string; // PEP 440 version regex. Versions must follow PEP 440 format (e.g., "1.0.0", "2.1a3"). // See https://peps.python.org/pep-0440/ + // This regex is adapted from the official python 'packaging' library: + // https://github.com/pypa/packaging/blob/main/src/packaging/version.py const versionRegex = - /^([0-9]+!)?(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*((a|b|c|rc)([0-9]+)?)?(\.post([0-9]+)?)?(\.dev([0-9]+)?)?(\+[a-zA-Z0-9._-]+)?$/; + /^v?([0-9]+!)?([0-9]+(?:\.[0-9]+)*)(?:[-_.]?(a|b|c|rc|alpha|beta|pre|preview)[-_.]?([0-9]+)?)?(?:(?:-([0-9]+))|(?:[-_.]?(post|rev|r)[-_.]?([0-9]+)?))?(?:[-_.]?(dev)[-_.]?([0-9]+)?)?(?:\+([a-z0-9]+(?:[-_.][a-z0-9]+)*))?$/i; if (!versionRegex.test(version)) { return l10n.t('Invalid version "{0}" in pyproject.toml.', version); } From f56998b2ec3d27ee21b02846482c34f3cf1aa96d Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 12 Dec 2025 11:23:26 -0800 Subject: [PATCH 07/10] add tests --- src/managers/builtin/pipUtils.ts | 2 +- .../helpers.validatePyproject.unit.test.ts | 277 +++++++++++++++--- 2 files changed, 235 insertions(+), 44 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index 9310db9b..8fa240c1 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -13,7 +13,7 @@ import { Installable } from '../common/types'; import { mergePackages } from '../common/utils'; import { refreshPipPackages } from './utils'; -function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined { +export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined { // 1. Validate package name (PEP 508) if (toml.project && (toml.project as tomljs.JsonMap).name) { const name = (toml.project as tomljs.JsonMap).name as string; diff --git a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts index 01f31e6b..4b3446a9 100644 --- a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts +++ b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts @@ -1,67 +1,258 @@ +import * as tomljs from '@iarna/toml'; import assert from 'assert'; import { Uri } from 'vscode'; -import { shouldProceedAfterPyprojectValidation, ValidationError } from '../../../managers/builtin/pipUtils'; +import { + shouldProceedAfterPyprojectValidation, + validatePyprojectToml, + ValidationError, +} from '../../../managers/builtin/pipUtils'; suite('pipUtils - validatePyproject', () => { - const mockValidationError: ValidationError = { - message: 'Invalid package name "my package" in pyproject.toml.', - fileUri: Uri.file('/test/path/pyproject.toml'), - }; + suite('shouldProceedAfterPyprojectValidation', () => { + const mockValidationError: ValidationError = { + message: 'Invalid package name "my package" in pyproject.toml.', + fileUri: Uri.file('/test/path/pyproject.toml'), + }; - test('should return true when no validation error exists', async () => { - // Arrange: no validation error - const validationError = undefined; - const install = ['-e', '/test/path']; + test('should return true when no validation error exists', async () => { + // Arrange: no validation error + const validationError = undefined; + const install = ['-e', '/test/path']; - // Act - const result = await shouldProceedAfterPyprojectValidation(validationError, install); + // Act + const result = await shouldProceedAfterPyprojectValidation(validationError, install); - // Assert - assert.strictEqual(result, true, 'Should proceed when no validation error'); - }); + // Assert + assert.strictEqual(result, true, 'Should proceed when no validation error'); + }); + + test('should return true when install array is empty', async () => { + // Arrange: validation error exists but no packages selected + const install: string[] = []; + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when no packages selected'); + }); + + test('should return true when only requirements.txt packages selected (no -e flag)', async () => { + // Arrange: validation error exists but only requirements.txt packages selected + const install = ['-r', '/test/requirements.txt']; + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should proceed when no TOML packages selected'); + }); - test('should return true when install array is empty', async () => { - // Arrange: validation error exists but no packages selected - const install: string[] = []; + test('should return true when only PyPI packages selected (no flags at all)', async () => { + // Arrange: only PyPI package names, no flags + const install = ['numpy', 'pandas', 'requests']; - // Act - const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); - // Assert - assert.strictEqual(result, true, 'Should proceed when no packages selected'); + // Assert + assert.strictEqual(result, true, 'Should proceed when only PyPI packages selected'); + }); + + test('should not trigger on -e flag at end of array without following argument', async () => { + // Arrange: -e flag is last item (malformed, but should not crash) + const install = ['numpy', '-e']; + // This is edge case - -e at end means no path follows, so index + 1 < arr.length is false + + // Act + const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + + // Assert + assert.strictEqual(result, true, 'Should not crash on malformed -e flag at end'); + }); }); - test('should return true when only requirements.txt packages selected (no -e flag)', async () => { - // Arrange: validation error exists but only requirements.txt packages selected - const install = ['-r', '/test/requirements.txt']; + function verifyValidationError(toml: tomljs.JsonMap, expectedError: string | undefined) { + const ActualError = validatePyprojectToml(toml); + assert.strictEqual(ActualError, expectedError); + } + + suite('validatePyprojectToml - Package Name Validation (PEP 508)', () => { + test('should accept valid single-character package name', () => { + const toml: tomljs.JsonMap = { + project: { name: 'a' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept valid package name with letters and numbers', () => { + const toml: tomljs.JsonMap = { + project: { name: 'mypackage123' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept valid package name with hyphens', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my-package' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept valid package name with underscores', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my_package' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept valid package name with dots', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my.package' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept valid package name with mixed separators', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my-package_name.v2' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept complex valid package name', () => { + const toml: tomljs.JsonMap = { + project: { name: 'Django-REST-framework' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should reject package name with spaces', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my package' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "my package" in pyproject.toml.'); + }); + + test('should reject package name starting with hyphen', () => { + const toml: tomljs.JsonMap = { + project: { name: '-mypackage' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "-mypackage" in pyproject.toml.'); + }); + + test('should reject package name ending with hyphen', () => { + const toml: tomljs.JsonMap = { + project: { name: 'mypackage-' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "mypackage-" in pyproject.toml.'); + }); + + test('should reject package name starting with dot', () => { + const toml: tomljs.JsonMap = { + project: { name: '.mypackage' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name ".mypackage" in pyproject.toml.'); + }); + + test('should reject package name ending with dot', () => { + const toml: tomljs.JsonMap = { + project: { name: 'mypackage.' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "mypackage." in pyproject.toml.'); + }); + + test('should reject package name starting with underscore', () => { + const toml: tomljs.JsonMap = { + project: { name: '_mypackage' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "_mypackage" in pyproject.toml.'); + }); + + test('should reject package name ending with underscore', () => { + const toml: tomljs.JsonMap = { + project: { name: 'mypackage_' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "mypackage_" in pyproject.toml.'); + }); + + test('should reject empty package name', () => { + const toml: tomljs.JsonMap = { + project: { name: '' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "" in pyproject.toml.'); + }); + + test('should reject package name with special characters', () => { + const toml: tomljs.JsonMap = { + project: { name: 'my@package' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "my@package" in pyproject.toml.'); + }); - // Act - const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + test('should reject package name with only separator', () => { + const toml: tomljs.JsonMap = { + project: { name: '-' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Invalid package name "-" in pyproject.toml.'); + }); - // Assert - assert.strictEqual(result, true, 'Should proceed when no TOML packages selected'); + test('should accept when no project section exists', () => { + const toml: tomljs.JsonMap = {}; + verifyValidationError(toml, undefined); + }); }); - test('should return true when only PyPI packages selected (no flags at all)', async () => { - // Arrange: only PyPI package names, no flags - const install = ['numpy', 'pandas', 'requests']; + suite('validatePyprojectToml - Required Fields (PEP 621)', () => { + test('should accept valid project with name', () => { + const toml: tomljs.JsonMap = { + project: { name: 'test' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); - // Act - const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + test('should reject project without name field', () => { + const toml: tomljs.JsonMap = { + project: { version: '1.0.0' } as tomljs.JsonMap, + }; + verifyValidationError(toml, 'Missing required field "name" in [project] section of pyproject.toml.'); + }); - // Assert - assert.strictEqual(result, true, 'Should proceed when only PyPI packages selected'); + test('should accept when no project section exists', () => { + const toml: tomljs.JsonMap = {}; + verifyValidationError(toml, undefined); + }); }); - test('should not trigger on -e flag at end of array without following argument', async () => { - // Arrange: -e flag is last item (malformed, but should not crash) - const install = ['numpy', '-e']; - // This is edge case - -e at end means no path follows, so index + 1 < arr.length is false + suite('validatePyprojectToml - Build System (PEP 518)', () => { + test('should accept valid build-system with requires', () => { + const toml: tomljs.JsonMap = { + project: { name: 'test' } as tomljs.JsonMap, + 'build-system': { + requires: ['setuptools', 'wheel'], + } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); - // Act - const result = await shouldProceedAfterPyprojectValidation(mockValidationError, install); + test('should reject build-system without requires field', () => { + const toml: tomljs.JsonMap = { + project: { name: 'test' } as tomljs.JsonMap, + 'build-system': { + 'build-backend': 'setuptools.build_meta', + } as tomljs.JsonMap, + }; + verifyValidationError( + toml, + 'Missing required field "requires" in [build-system] section of pyproject.toml.', + ); + }); - // Assert - assert.strictEqual(result, true, 'Should not crash on malformed -e flag at end'); + test('should accept when no build-system section exists', () => { + const toml: tomljs.JsonMap = { + project: { name: 'test' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); }); }); From 32f9f3a2e3a98fc25b2ae8d21c48ffe7c3b8ef32 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 12 Dec 2025 14:19:01 -0800 Subject: [PATCH 08/10] add more tests --- src/managers/builtin/pipUtils.ts | 5 +- .../helpers.validatePyproject.unit.test.ts | 229 +++++++++++++++++- 2 files changed, 226 insertions(+), 8 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index 8fa240c1..9e37bcfd 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -26,8 +26,11 @@ export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined } // 2. Validate version format (PEP 440) - if (toml.project && (toml.project as tomljs.JsonMap).version) { + if (toml.project && 'version' in (toml.project as tomljs.JsonMap)) { const version = (toml.project as tomljs.JsonMap).version as string; + if (version.length === 0) { + return l10n.t('Version cannot be empty in pyproject.toml.'); + } // PEP 440 version regex. Versions must follow PEP 440 format (e.g., "1.0.0", "2.1a3"). // See https://peps.python.org/pep-0440/ // This regex is adapted from the official python 'packaging' library: diff --git a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts index 4b3446a9..7bb27542 100644 --- a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts +++ b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts @@ -176,13 +176,6 @@ suite('pipUtils - validatePyproject', () => { verifyValidationError(toml, 'Invalid package name "mypackage_" in pyproject.toml.'); }); - test('should reject empty package name', () => { - const toml: tomljs.JsonMap = { - project: { name: '' } as tomljs.JsonMap, - }; - verifyValidationError(toml, 'Invalid package name "" in pyproject.toml.'); - }); - test('should reject package name with special characters', () => { const toml: tomljs.JsonMap = { project: { name: 'my@package' } as tomljs.JsonMap, @@ -255,4 +248,226 @@ suite('pipUtils - validatePyproject', () => { verifyValidationError(toml, undefined); }); }); + + suite('validatePyprojectToml - Version Validation (PEP 440)', () => { + interface VersionTestCase { + version: string; + expectedError: string | undefined; + description: string; + } + + function createVersionToml(version: string): tomljs.JsonMap { + return { + project: { name: 'test', version } as tomljs.JsonMap, + }; + } + + const versionTestCases: VersionTestCase[] = [ + // Basic release versions + { version: '1.0', expectedError: undefined, description: 'simple version 1.0' }, + { version: '1.0.0', expectedError: undefined, description: 'version with three parts 1.0.0' }, + { version: '1.2.3.4.5', expectedError: undefined, description: 'version with many parts 1.2.3.4.5' }, + { version: '1.0.01', expectedError: undefined, description: 'version with leading zeros 1.0.01' }, + { version: '0', expectedError: undefined, description: 'single digit version 0' }, + { version: '2024.1.15', expectedError: undefined, description: 'large version numbers 2024.1.15' }, + + // Epoch versions + { version: '1!1.0', expectedError: undefined, description: 'epoch version 1!1.0' }, + { version: '2!0.0.1', expectedError: undefined, description: 'epoch version 2!0.0.1' }, + { version: '100!1.0.0', expectedError: undefined, description: 'large epoch 100!1.0.0' }, + + // Pre-release versions - Alpha + { version: '1.0a1', expectedError: undefined, description: 'alpha version 1.0a1' }, + { version: '1.0.a1', expectedError: undefined, description: 'alpha with dot separator 1.0.a1' }, + { version: '1.0-a1', expectedError: undefined, description: 'alpha with hyphen separator 1.0-a1' }, + { version: '1.0_a1', expectedError: undefined, description: 'alpha with underscore 1.0_a1' }, + { version: '1.0a', expectedError: undefined, description: 'alpha without number 1.0a' }, + { version: '1.0alpha1', expectedError: undefined, description: 'long form alpha 1.0alpha1' }, + { version: '1.0.alpha.1', expectedError: undefined, description: 'alpha with separators 1.0.alpha.1' }, + { version: '1.0a999', expectedError: undefined, description: 'alpha with large number 1.0a999' }, + + // Pre-release versions - Beta + { version: '1.0b1', expectedError: undefined, description: 'beta version 1.0b1' }, + { version: '1.0beta1', expectedError: undefined, description: 'long form beta 1.0beta1' }, + { version: '1.0.beta.2', expectedError: undefined, description: 'beta with separators 1.0.beta.2' }, + { version: '1.0b', expectedError: undefined, description: 'beta without number 1.0b' }, + + // Pre-release versions - RC + { version: '1.0rc1', expectedError: undefined, description: 'rc version 1.0rc1' }, + { version: '1.0c1', expectedError: undefined, description: 'c version 1.0c1' }, + { version: '1.0.rc.3', expectedError: undefined, description: 'rc with separators 1.0.rc.3' }, + { version: '1.0rc', expectedError: undefined, description: 'rc without number 1.0rc' }, + + // Pre-release versions - Other + { version: '1.0preview1', expectedError: undefined, description: 'preview version 1.0preview1' }, + { version: '1.0pre1', expectedError: undefined, description: 'pre version 1.0pre1' }, + { + version: '1.0-preview-2', + expectedError: undefined, + description: 'preview with separators 1.0-preview-2', + }, + + // Post-release versions + { version: '1.0.post1', expectedError: undefined, description: 'post version 1.0.post1' }, + { version: '1.0post1', expectedError: undefined, description: 'post without dot 1.0post1' }, + { version: '1.0-post1', expectedError: undefined, description: 'post with hyphen 1.0-post1' }, + { version: '1.0_post1', expectedError: undefined, description: 'post with underscore 1.0_post1' }, + { version: '1.0.post', expectedError: undefined, description: 'post without number 1.0.post' }, + { version: '1.0-1', expectedError: undefined, description: 'implicit post version 1.0-1' }, + { version: '1.0-5', expectedError: undefined, description: 'implicit post version 1.0-5' }, + { version: '1.0rev1', expectedError: undefined, description: 'rev version 1.0rev1' }, + { version: '1.0r1', expectedError: undefined, description: 'r version 1.0r1' }, + { version: '1.0.rev.2', expectedError: undefined, description: 'rev with separators 1.0.rev.2' }, + { version: '1.0.post999', expectedError: undefined, description: 'post with large number 1.0.post999' }, + + // Dev versions + { version: '1.0.dev1', expectedError: undefined, description: 'dev version 1.0.dev1' }, + { version: '1.0dev1', expectedError: undefined, description: 'dev without dot 1.0dev1' }, + { version: '1.0-dev1', expectedError: undefined, description: 'dev with hyphen 1.0-dev1' }, + { version: '1.0_dev1', expectedError: undefined, description: 'dev with underscore 1.0_dev1' }, + { version: '1.0.dev', expectedError: undefined, description: 'dev without number 1.0.dev' }, + { version: '1.0.dev999', expectedError: undefined, description: 'dev with large number 1.0.dev999' }, + + // Local versions + { version: '1.0+abc', expectedError: undefined, description: 'local version 1.0+abc' }, + { version: '1.0+abc.def', expectedError: undefined, description: 'local with dots 1.0+abc.def' }, + { version: '1.0+abc-def', expectedError: undefined, description: 'local with hyphens 1.0+abc-def' }, + { version: '1.0+abc_def', expectedError: undefined, description: 'local with underscores 1.0+abc_def' }, + { version: '1.0+abc.5', expectedError: undefined, description: 'local with numbers 1.0+abc.5' }, + { + version: '1.0+abc.def-ghi_jkl', + expectedError: undefined, + description: 'local with mixed separators 1.0+abc.def-ghi_jkl', + }, + { version: '1.0+001', expectedError: undefined, description: 'numeric local version 1.0+001' }, + { version: '1.0+g1234567', expectedError: undefined, description: 'git hash-like local 1.0+g1234567' }, + + // Combined versions + { version: '1.0a1.post1', expectedError: undefined, description: 'pre + post 1.0a1.post1' }, + { + version: '1.0a1.post1.dev2', + expectedError: undefined, + description: 'pre + post + dev 1.0a1.post1.dev2', + }, + { version: '1.0.post1.dev2', expectedError: undefined, description: 'post + dev 1.0.post1.dev2' }, + { version: '1.0a1.dev1', expectedError: undefined, description: 'pre + dev 1.0a1.dev1' }, + { version: '1.0a1+local', expectedError: undefined, description: 'pre + local 1.0a1+local' }, + { version: '1.0.post1+local', expectedError: undefined, description: 'post + local 1.0.post1+local' }, + { version: '1.0.dev1+local', expectedError: undefined, description: 'dev + local 1.0.dev1+local' }, + { + version: '1!1.0a1.post1.dev2+abc', + expectedError: undefined, + description: 'epoch + all components 1!1.0a1.post1.dev2+abc', + }, + { + version: '2!1.2.3rc4.post5.dev6+local.version', + expectedError: undefined, + description: 'full complex version 2!1.2.3rc4.post5.dev6+local.version', + }, + { version: '1.0rc1-1', expectedError: undefined, description: 'rc + implicit post 1.0rc1-1' }, + + // Version with v prefix + { version: 'v1.0', expectedError: undefined, description: 'version with v prefix v1.0' }, + { version: 'v1.0.0', expectedError: undefined, description: 'version with v prefix v1.0.0' }, + { version: 'v1.0a1', expectedError: undefined, description: 'v prefix with pre-release v1.0a1' }, + { version: 'v1.0-1', expectedError: undefined, description: 'v prefix with implicit post v1.0-1' }, + { + version: 'v1!2.0rc1.post2.dev3+local', + expectedError: undefined, + description: 'v prefix with all components v1!2.0rc1.post2.dev3+local', + }, + + // Case insensitivity + { version: '1.0A1', expectedError: undefined, description: 'uppercase alpha 1.0A1' }, + { version: '1.0ALPHA1', expectedError: undefined, description: 'uppercase ALPHA 1.0ALPHA1' }, + { version: '1.0Alpha1', expectedError: undefined, description: 'mixed case Alpha 1.0Alpha1' }, + { version: '1.0POST1', expectedError: undefined, description: 'uppercase POST 1.0POST1' }, + { version: '1.0DEV1', expectedError: undefined, description: 'uppercase DEV 1.0DEV1' }, + { version: '1.0RC1', expectedError: undefined, description: 'uppercase RC 1.0RC1' }, + { version: 'V1.0', expectedError: undefined, description: 'uppercase V prefix V1.0' }, + { + version: '1.0Alpha1.POST2.Dev3', + expectedError: undefined, + description: 'mixed case components 1.0Alpha1.POST2.Dev3', + }, + + // Invalid versions + { + version: '.1.0', + expectedError: 'Invalid version ".1.0" in pyproject.toml.', + description: 'starting with dot .1.0', + }, + { + version: '1.0.', + expectedError: 'Invalid version "1.0." in pyproject.toml.', + description: 'ending with dot 1.0.', + }, + { + version: 'abc', + expectedError: 'Invalid version "abc" in pyproject.toml.', + description: 'completely invalid abc', + }, + { version: '', expectedError: 'Version cannot be empty in pyproject.toml.', description: 'empty version' }, + { + version: '1..0', + expectedError: 'Invalid version "1..0" in pyproject.toml.', + description: 'double dots 1..0', + }, + { + version: '1.0 rc1', + expectedError: 'Invalid version "1.0 rc1" in pyproject.toml.', + description: 'spaces 1.0 rc1', + }, + { + version: '1.0gamma1', + expectedError: 'Invalid version "1.0gamma1" in pyproject.toml.', + description: 'invalid pre-release keyword 1.0gamma1', + }, + { + version: '1 1.0', + expectedError: 'Invalid version "1 1.0" in pyproject.toml.', + description: 'epoch without exclamation 1 1.0', + }, + { + version: '1.0local', + expectedError: 'Invalid version "1.0local" in pyproject.toml.', + description: 'local without plus 1.0local', + }, + { + version: '1.0+abc+def', + expectedError: 'Invalid version "1.0+abc+def" in pyproject.toml.', + description: 'multiple local markers 1.0+abc+def', + }, + { + version: '1!', + expectedError: 'Invalid version "1!" in pyproject.toml.', + description: 'only epoch 1!', + }, + { + version: '1.0--a1', + expectedError: 'Invalid version "1.0--a1" in pyproject.toml.', + description: 'invalid separator combinations 1.0--a1', + }, + ]; + + versionTestCases.forEach(({ version, expectedError, description }) => { + test(`should ${expectedError ? 'reject' : 'accept'} ${description}`, () => { + const toml = createVersionToml(version); + verifyValidationError(toml, expectedError); + }); + }); + + // Edge cases + test('should accept when no version field exists', () => { + const toml: tomljs.JsonMap = { + project: { name: 'test' } as tomljs.JsonMap, + }; + verifyValidationError(toml, undefined); + }); + + test('should accept when no project section exists', () => { + const toml: tomljs.JsonMap = {}; + verifyValidationError(toml, undefined); + }); + }); }); From 9cccf603b7e812976088d7995d344b290c34ce76 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 12 Dec 2025 15:25:23 -0800 Subject: [PATCH 09/10] clean up --- src/managers/builtin/pipUtils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index 9e37bcfd..e0f40957 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -42,7 +42,7 @@ export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined } } - // 3. Validate required fields (PEP 621) + // 3. Validate required "name" field in [project] section (PEP 621) if (toml.project) { const project = toml.project as tomljs.JsonMap; // See PEP 621: https://peps.python.org/pep-0621/ @@ -51,7 +51,7 @@ export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined } } - // 4. Validate build system (PEP 518) + // 4. Validate required "requires" field in [build-system] section (PEP 518) if (toml['build-system']) { const buildSystem = toml['build-system'] as tomljs.JsonMap; // See PEP 518: https://peps.python.org/pep-0518/ @@ -214,7 +214,7 @@ export interface PipPackages { export interface ProjectInstallableResult { /** - * List of installable packages from requirements.txt and pyproject.toml files + * List of installable packages from pyproject.toml file */ installables: Installable[]; @@ -295,7 +295,7 @@ export async function getProjectInstallable( if (uri.fsPath.endsWith('.toml')) { const toml = await tomlParse(uri.fsPath); - // Validate pyproject.toml and capture first error only + // Validate pyproject.toml if (!validationError) { const error = validatePyprojectToml(toml); if (error) { From c6deb15b9315be487038f11ba9dedbcf35b52176 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 15 Dec 2025 14:05:36 -0800 Subject: [PATCH 10/10] address feedback --- src/managers/builtin/pipUtils.ts | 69 ++++++----- .../helpers.validatePyproject.unit.test.ts | 108 +++++++++--------- 2 files changed, 91 insertions(+), 86 deletions(-) diff --git a/src/managers/builtin/pipUtils.ts b/src/managers/builtin/pipUtils.ts index e0f40957..9fd540ab 100644 --- a/src/managers/builtin/pipUtils.ts +++ b/src/managers/builtin/pipUtils.ts @@ -13,21 +13,46 @@ import { Installable } from '../common/types'; import { mergePackages } from '../common/utils'; import { refreshPipPackages } from './utils'; -export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined { - // 1. Validate package name (PEP 508) - if (toml.project && (toml.project as tomljs.JsonMap).name) { - const name = (toml.project as tomljs.JsonMap).name as string; - // PEP 508 regex: must start and end with a letter or digit, can contain -_., and alphanumeric characters. No spaces allowed. - // See https://peps.python.org/pep-0508/ - const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/; - if (!nameRegex.test(name)) { - return l10n.t('Invalid package name "{0}" in pyproject.toml.', name); - } +export interface PyprojectToml { + project?: { + name?: string; + version?: string; + }; + 'build-system'?: { + requires?: unknown; + }; +} +export function validatePyprojectToml(toml: PyprojectToml): string | undefined { + // 1. Validate required "requires" field in [build-system] section (PEP 518) + const buildSystem = toml['build-system']; + if (buildSystem && !buildSystem.requires) { + // See PEP 518: https://peps.python.org/pep-0518/ + return l10n.t('Missing required field "requires" in [build-system] section of pyproject.toml.'); + } + + const project = toml.project; + if (!project) { + return undefined; + } + + const name = project.name; + // 2. Validate required "name" field in [project] section (PEP 621) + // See PEP 621: https://peps.python.org/pep-0621/ + if (!name) { + return l10n.t('Missing required field "name" in [project] section of pyproject.toml.'); } - // 2. Validate version format (PEP 440) - if (toml.project && 'version' in (toml.project as tomljs.JsonMap)) { - const version = (toml.project as tomljs.JsonMap).version as string; + // 3. Validate package name (PEP 508) + // PEP 508 regex: must start and end with a letter or digit, can contain -_., and alphanumeric characters. No spaces allowed. + // See https://peps.python.org/pep-0508/ + const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/; + if (!nameRegex.test(name)) { + return l10n.t('Invalid package name "{0}" in pyproject.toml.', name); + } + + // 4. Validate version format (PEP 440) + const version = project.version; + if (version !== undefined) { if (version.length === 0) { return l10n.t('Version cannot be empty in pyproject.toml.'); } @@ -42,24 +67,6 @@ export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined } } - // 3. Validate required "name" field in [project] section (PEP 621) - if (toml.project) { - const project = toml.project as tomljs.JsonMap; - // See PEP 621: https://peps.python.org/pep-0621/ - if (!project.name) { - return l10n.t('Missing required field "name" in [project] section of pyproject.toml.'); - } - } - - // 4. Validate required "requires" field in [build-system] section (PEP 518) - if (toml['build-system']) { - const buildSystem = toml['build-system'] as tomljs.JsonMap; - // See PEP 518: https://peps.python.org/pep-0518/ - if (!buildSystem.requires) { - return l10n.t('Missing required field "requires" in [build-system] section of pyproject.toml.'); - } - } - return undefined; } diff --git a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts index 7bb27542..579c3c74 100644 --- a/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts +++ b/src/test/managers/builtin/helpers.validatePyproject.unit.test.ts @@ -1,7 +1,7 @@ -import * as tomljs from '@iarna/toml'; import assert from 'assert'; import { Uri } from 'vscode'; import { + PyprojectToml, shouldProceedAfterPyprojectValidation, validatePyprojectToml, ValidationError, @@ -72,168 +72,166 @@ suite('pipUtils - validatePyproject', () => { }); }); - function verifyValidationError(toml: tomljs.JsonMap, expectedError: string | undefined) { + function verifyValidationError(toml: PyprojectToml, expectedError: string | undefined) { const ActualError = validatePyprojectToml(toml); assert.strictEqual(ActualError, expectedError); } suite('validatePyprojectToml - Package Name Validation (PEP 508)', () => { test('should accept valid single-character package name', () => { - const toml: tomljs.JsonMap = { - project: { name: 'a' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'a' }, }; verifyValidationError(toml, undefined); }); test('should accept valid package name with letters and numbers', () => { - const toml: tomljs.JsonMap = { - project: { name: 'mypackage123' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'mypackage123' }, }; verifyValidationError(toml, undefined); }); test('should accept valid package name with hyphens', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my-package' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my-package' }, }; verifyValidationError(toml, undefined); }); test('should accept valid package name with underscores', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my_package' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my_package' }, }; verifyValidationError(toml, undefined); }); test('should accept valid package name with dots', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my.package' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my.package' }, }; verifyValidationError(toml, undefined); }); test('should accept valid package name with mixed separators', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my-package_name.v2' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my-package_name.v2' }, }; verifyValidationError(toml, undefined); }); test('should accept complex valid package name', () => { - const toml: tomljs.JsonMap = { - project: { name: 'Django-REST-framework' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'Django-REST-framework' }, }; verifyValidationError(toml, undefined); }); test('should reject package name with spaces', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my package' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my package' }, }; verifyValidationError(toml, 'Invalid package name "my package" in pyproject.toml.'); }); test('should reject package name starting with hyphen', () => { - const toml: tomljs.JsonMap = { - project: { name: '-mypackage' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: '-mypackage' }, }; verifyValidationError(toml, 'Invalid package name "-mypackage" in pyproject.toml.'); }); test('should reject package name ending with hyphen', () => { - const toml: tomljs.JsonMap = { - project: { name: 'mypackage-' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'mypackage-' }, }; verifyValidationError(toml, 'Invalid package name "mypackage-" in pyproject.toml.'); }); test('should reject package name starting with dot', () => { - const toml: tomljs.JsonMap = { - project: { name: '.mypackage' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: '.mypackage' }, }; verifyValidationError(toml, 'Invalid package name ".mypackage" in pyproject.toml.'); }); test('should reject package name ending with dot', () => { - const toml: tomljs.JsonMap = { - project: { name: 'mypackage.' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'mypackage.' }, }; verifyValidationError(toml, 'Invalid package name "mypackage." in pyproject.toml.'); }); test('should reject package name starting with underscore', () => { - const toml: tomljs.JsonMap = { - project: { name: '_mypackage' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: '_mypackage' }, }; verifyValidationError(toml, 'Invalid package name "_mypackage" in pyproject.toml.'); }); test('should reject package name ending with underscore', () => { - const toml: tomljs.JsonMap = { - project: { name: 'mypackage_' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'mypackage_' }, }; verifyValidationError(toml, 'Invalid package name "mypackage_" in pyproject.toml.'); }); test('should reject package name with special characters', () => { - const toml: tomljs.JsonMap = { - project: { name: 'my@package' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'my@package' }, }; verifyValidationError(toml, 'Invalid package name "my@package" in pyproject.toml.'); }); test('should reject package name with only separator', () => { - const toml: tomljs.JsonMap = { - project: { name: '-' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: '-' }, }; verifyValidationError(toml, 'Invalid package name "-" in pyproject.toml.'); }); test('should accept when no project section exists', () => { - const toml: tomljs.JsonMap = {}; + const toml: PyprojectToml = {}; verifyValidationError(toml, undefined); }); }); suite('validatePyprojectToml - Required Fields (PEP 621)', () => { test('should accept valid project with name', () => { - const toml: tomljs.JsonMap = { - project: { name: 'test' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'test' }, }; verifyValidationError(toml, undefined); }); test('should reject project without name field', () => { - const toml: tomljs.JsonMap = { - project: { version: '1.0.0' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { version: '1.0.0' }, }; verifyValidationError(toml, 'Missing required field "name" in [project] section of pyproject.toml.'); }); test('should accept when no project section exists', () => { - const toml: tomljs.JsonMap = {}; + const toml: PyprojectToml = {}; verifyValidationError(toml, undefined); }); }); suite('validatePyprojectToml - Build System (PEP 518)', () => { test('should accept valid build-system with requires', () => { - const toml: tomljs.JsonMap = { - project: { name: 'test' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'test' }, 'build-system': { requires: ['setuptools', 'wheel'], - } as tomljs.JsonMap, + }, }; verifyValidationError(toml, undefined); }); test('should reject build-system without requires field', () => { - const toml: tomljs.JsonMap = { - project: { name: 'test' } as tomljs.JsonMap, - 'build-system': { - 'build-backend': 'setuptools.build_meta', - } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'test' }, + 'build-system': {}, }; verifyValidationError( toml, @@ -242,8 +240,8 @@ suite('pipUtils - validatePyproject', () => { }); test('should accept when no build-system section exists', () => { - const toml: tomljs.JsonMap = { - project: { name: 'test' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'test' }, }; verifyValidationError(toml, undefined); }); @@ -256,9 +254,9 @@ suite('pipUtils - validatePyproject', () => { description: string; } - function createVersionToml(version: string): tomljs.JsonMap { + function createVersionToml(version: string): PyprojectToml { return { - project: { name: 'test', version } as tomljs.JsonMap, + project: { name: 'test', version }, }; } @@ -459,14 +457,14 @@ suite('pipUtils - validatePyproject', () => { // Edge cases test('should accept when no version field exists', () => { - const toml: tomljs.JsonMap = { - project: { name: 'test' } as tomljs.JsonMap, + const toml: PyprojectToml = { + project: { name: 'test' }, }; verifyValidationError(toml, undefined); }); test('should accept when no project section exists', () => { - const toml: tomljs.JsonMap = {}; + const toml: PyprojectToml = {}; verifyValidationError(toml, undefined); }); });