Skip to content

Commit c6deb15

Browse files
StellaHuang95StellaHuang95
committed
address feedback
1 parent 9cccf60 commit c6deb15

2 files changed

Lines changed: 91 additions & 86 deletions

File tree

‎src/managers/builtin/pipUtils.ts‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,46 @@ import { Installable } from '../common/types';
1313
import { mergePackages } from '../common/utils';
1414
import { refreshPipPackages } from './utils';
1515

16-
export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined {
17-
// 1. Validate package name (PEP 508)
18-
if (toml.project && (toml.project as tomljs.JsonMap).name) {
19-
const name = (toml.project as tomljs.JsonMap).name as string;
20-
// PEP 508 regex: must start and end with a letter or digit, can contain -_., and alphanumeric characters. No spaces allowed.
21-
// See https://peps.python.org/pep-0508/
22-
const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/;
23-
if (!nameRegex.test(name)) {
24-
return l10n.t('Invalid package name "{0}" in pyproject.toml.', name);
25-
}
16+
export interface PyprojectToml {
17+
project?: {
18+
name?: string;
19+
version?: string;
20+
};
21+
'build-system'?: {
22+
requires?: unknown;
23+
};
24+
}
25+
export function validatePyprojectToml(toml: PyprojectToml): string | undefined {
26+
// 1. Validate required "requires" field in [build-system] section (PEP 518)
27+
const buildSystem = toml['build-system'];
28+
if (buildSystem && !buildSystem.requires) {
29+
// See PEP 518: https://peps.python.org/pep-0518/
30+
return l10n.t('Missing required field "requires" in [build-system] section of pyproject.toml.');
31+
}
32+
33+
const project = toml.project;
34+
if (!project) {
35+
return undefined;
36+
}
37+
38+
const name = project.name;
39+
// 2. Validate required "name" field in [project] section (PEP 621)
40+
// See PEP 621: https://peps.python.org/pep-0621/
41+
if (!name) {
42+
return l10n.t('Missing required field "name" in [project] section of pyproject.toml.');
2643
}
2744

28-
// 2. Validate version format (PEP 440)
29-
if (toml.project && 'version' in (toml.project as tomljs.JsonMap)) {
30-
const version = (toml.project as tomljs.JsonMap).version as string;
45+
// 3. Validate package name (PEP 508)
46+
// PEP 508 regex: must start and end with a letter or digit, can contain -_., and alphanumeric characters. No spaces allowed.
47+
// See https://peps.python.org/pep-0508/
48+
const nameRegex = /^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])$/;
49+
if (!nameRegex.test(name)) {
50+
return l10n.t('Invalid package name "{0}" in pyproject.toml.', name);
51+
}
52+
53+
// 4. Validate version format (PEP 440)
54+
const version = project.version;
55+
if (version !== undefined) {
3156
if (version.length === 0) {
3257
return l10n.t('Version cannot be empty in pyproject.toml.');
3358
}
@@ -42,24 +67,6 @@ export function validatePyprojectToml(toml: tomljs.JsonMap): string | undefined
4267
}
4368
}
4469

45-
// 3. Validate required "name" field in [project] section (PEP 621)
46-
if (toml.project) {
47-
const project = toml.project as tomljs.JsonMap;
48-
// See PEP 621: https://peps.python.org/pep-0621/
49-
if (!project.name) {
50-
return l10n.t('Missing required field "name" in [project] section of pyproject.toml.');
51-
}
52-
}
53-
54-
// 4. Validate required "requires" field in [build-system] section (PEP 518)
55-
if (toml['build-system']) {
56-
const buildSystem = toml['build-system'] as tomljs.JsonMap;
57-
// See PEP 518: https://peps.python.org/pep-0518/
58-
if (!buildSystem.requires) {
59-
return l10n.t('Missing required field "requires" in [build-system] section of pyproject.toml.');
60-
}
61-
}
62-
6370
return undefined;
6471
}
6572

‎src/test/managers/builtin/helpers.validatePyproject.unit.test.ts‎

Lines changed: 53 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import * as tomljs from '@iarna/toml';
21
import assert from 'assert';
32
import { Uri } from 'vscode';
43
import {
4+
PyprojectToml,
55
shouldProceedAfterPyprojectValidation,
66
validatePyprojectToml,
77
ValidationError,
@@ -72,168 +72,166 @@ suite('pipUtils - validatePyproject', () => {
7272
});
7373
});
7474

75-
function verifyValidationError(toml: tomljs.JsonMap, expectedError: string | undefined) {
75+
function verifyValidationError(toml: PyprojectToml, expectedError: string | undefined) {
7676
const ActualError = validatePyprojectToml(toml);
7777
assert.strictEqual(ActualError, expectedError);
7878
}
7979

8080
suite('validatePyprojectToml - Package Name Validation (PEP 508)', () => {
8181
test('should accept valid single-character package name', () => {
82-
const toml: tomljs.JsonMap = {
83-
project: { name: 'a' } as tomljs.JsonMap,
82+
const toml: PyprojectToml = {
83+
project: { name: 'a' },
8484
};
8585
verifyValidationError(toml, undefined);
8686
});
8787

8888
test('should accept valid package name with letters and numbers', () => {
89-
const toml: tomljs.JsonMap = {
90-
project: { name: 'mypackage123' } as tomljs.JsonMap,
89+
const toml: PyprojectToml = {
90+
project: { name: 'mypackage123' },
9191
};
9292
verifyValidationError(toml, undefined);
9393
});
9494

9595
test('should accept valid package name with hyphens', () => {
96-
const toml: tomljs.JsonMap = {
97-
project: { name: 'my-package' } as tomljs.JsonMap,
96+
const toml: PyprojectToml = {
97+
project: { name: 'my-package' },
9898
};
9999
verifyValidationError(toml, undefined);
100100
});
101101

102102
test('should accept valid package name with underscores', () => {
103-
const toml: tomljs.JsonMap = {
104-
project: { name: 'my_package' } as tomljs.JsonMap,
103+
const toml: PyprojectToml = {
104+
project: { name: 'my_package' },
105105
};
106106
verifyValidationError(toml, undefined);
107107
});
108108

109109
test('should accept valid package name with dots', () => {
110-
const toml: tomljs.JsonMap = {
111-
project: { name: 'my.package' } as tomljs.JsonMap,
110+
const toml: PyprojectToml = {
111+
project: { name: 'my.package' },
112112
};
113113
verifyValidationError(toml, undefined);
114114
});
115115

116116
test('should accept valid package name with mixed separators', () => {
117-
const toml: tomljs.JsonMap = {
118-
project: { name: 'my-package_name.v2' } as tomljs.JsonMap,
117+
const toml: PyprojectToml = {
118+
project: { name: 'my-package_name.v2' },
119119
};
120120
verifyValidationError(toml, undefined);
121121
});
122122

123123
test('should accept complex valid package name', () => {
124-
const toml: tomljs.JsonMap = {
125-
project: { name: 'Django-REST-framework' } as tomljs.JsonMap,
124+
const toml: PyprojectToml = {
125+
project: { name: 'Django-REST-framework' },
126126
};
127127
verifyValidationError(toml, undefined);
128128
});
129129

130130
test('should reject package name with spaces', () => {
131-
const toml: tomljs.JsonMap = {
132-
project: { name: 'my package' } as tomljs.JsonMap,
131+
const toml: PyprojectToml = {
132+
project: { name: 'my package' },
133133
};
134134
verifyValidationError(toml, 'Invalid package name "my package" in pyproject.toml.');
135135
});
136136

137137
test('should reject package name starting with hyphen', () => {
138-
const toml: tomljs.JsonMap = {
139-
project: { name: '-mypackage' } as tomljs.JsonMap,
138+
const toml: PyprojectToml = {
139+
project: { name: '-mypackage' },
140140
};
141141
verifyValidationError(toml, 'Invalid package name "-mypackage" in pyproject.toml.');
142142
});
143143

144144
test('should reject package name ending with hyphen', () => {
145-
const toml: tomljs.JsonMap = {
146-
project: { name: 'mypackage-' } as tomljs.JsonMap,
145+
const toml: PyprojectToml = {
146+
project: { name: 'mypackage-' },
147147
};
148148
verifyValidationError(toml, 'Invalid package name "mypackage-" in pyproject.toml.');
149149
});
150150

151151
test('should reject package name starting with dot', () => {
152-
const toml: tomljs.JsonMap = {
153-
project: { name: '.mypackage' } as tomljs.JsonMap,
152+
const toml: PyprojectToml = {
153+
project: { name: '.mypackage' },
154154
};
155155
verifyValidationError(toml, 'Invalid package name ".mypackage" in pyproject.toml.');
156156
});
157157

158158
test('should reject package name ending with dot', () => {
159-
const toml: tomljs.JsonMap = {
160-
project: { name: 'mypackage.' } as tomljs.JsonMap,
159+
const toml: PyprojectToml = {
160+
project: { name: 'mypackage.' },
161161
};
162162
verifyValidationError(toml, 'Invalid package name "mypackage." in pyproject.toml.');
163163
});
164164

165165
test('should reject package name starting with underscore', () => {
166-
const toml: tomljs.JsonMap = {
167-
project: { name: '_mypackage' } as tomljs.JsonMap,
166+
const toml: PyprojectToml = {
167+
project: { name: '_mypackage' },
168168
};
169169
verifyValidationError(toml, 'Invalid package name "_mypackage" in pyproject.toml.');
170170
});
171171

172172
test('should reject package name ending with underscore', () => {
173-
const toml: tomljs.JsonMap = {
174-
project: { name: 'mypackage_' } as tomljs.JsonMap,
173+
const toml: PyprojectToml = {
174+
project: { name: 'mypackage_' },
175175
};
176176
verifyValidationError(toml, 'Invalid package name "mypackage_" in pyproject.toml.');
177177
});
178178

179179
test('should reject package name with special characters', () => {
180-
const toml: tomljs.JsonMap = {
181-
project: { name: 'my@package' } as tomljs.JsonMap,
180+
const toml: PyprojectToml = {
181+
project: { name: 'my@package' },
182182
};
183183
verifyValidationError(toml, 'Invalid package name "my@package" in pyproject.toml.');
184184
});
185185

186186
test('should reject package name with only separator', () => {
187-
const toml: tomljs.JsonMap = {
188-
project: { name: '-' } as tomljs.JsonMap,
187+
const toml: PyprojectToml = {
188+
project: { name: '-' },
189189
};
190190
verifyValidationError(toml, 'Invalid package name "-" in pyproject.toml.');
191191
});
192192

193193
test('should accept when no project section exists', () => {
194-
const toml: tomljs.JsonMap = {};
194+
const toml: PyprojectToml = {};
195195
verifyValidationError(toml, undefined);
196196
});
197197
});
198198

199199
suite('validatePyprojectToml - Required Fields (PEP 621)', () => {
200200
test('should accept valid project with name', () => {
201-
const toml: tomljs.JsonMap = {
202-
project: { name: 'test' } as tomljs.JsonMap,
201+
const toml: PyprojectToml = {
202+
project: { name: 'test' },
203203
};
204204
verifyValidationError(toml, undefined);
205205
});
206206

207207
test('should reject project without name field', () => {
208-
const toml: tomljs.JsonMap = {
209-
project: { version: '1.0.0' } as tomljs.JsonMap,
208+
const toml: PyprojectToml = {
209+
project: { version: '1.0.0' },
210210
};
211211
verifyValidationError(toml, 'Missing required field "name" in [project] section of pyproject.toml.');
212212
});
213213

214214
test('should accept when no project section exists', () => {
215-
const toml: tomljs.JsonMap = {};
215+
const toml: PyprojectToml = {};
216216
verifyValidationError(toml, undefined);
217217
});
218218
});
219219

220220
suite('validatePyprojectToml - Build System (PEP 518)', () => {
221221
test('should accept valid build-system with requires', () => {
222-
const toml: tomljs.JsonMap = {
223-
project: { name: 'test' } as tomljs.JsonMap,
222+
const toml: PyprojectToml = {
223+
project: { name: 'test' },
224224
'build-system': {
225225
requires: ['setuptools', 'wheel'],
226-
} as tomljs.JsonMap,
226+
},
227227
};
228228
verifyValidationError(toml, undefined);
229229
});
230230

231231
test('should reject build-system without requires field', () => {
232-
const toml: tomljs.JsonMap = {
233-
project: { name: 'test' } as tomljs.JsonMap,
234-
'build-system': {
235-
'build-backend': 'setuptools.build_meta',
236-
} as tomljs.JsonMap,
232+
const toml: PyprojectToml = {
233+
project: { name: 'test' },
234+
'build-system': {},
237235
};
238236
verifyValidationError(
239237
toml,
@@ -242,8 +240,8 @@ suite('pipUtils - validatePyproject', () => {
242240
});
243241

244242
test('should accept when no build-system section exists', () => {
245-
const toml: tomljs.JsonMap = {
246-
project: { name: 'test' } as tomljs.JsonMap,
243+
const toml: PyprojectToml = {
244+
project: { name: 'test' },
247245
};
248246
verifyValidationError(toml, undefined);
249247
});
@@ -256,9 +254,9 @@ suite('pipUtils - validatePyproject', () => {
256254
description: string;
257255
}
258256

259-
function createVersionToml(version: string): tomljs.JsonMap {
257+
function createVersionToml(version: string): PyprojectToml {
260258
return {
261-
project: { name: 'test', version } as tomljs.JsonMap,
259+
project: { name: 'test', version },
262260
};
263261
}
264262

@@ -459,14 +457,14 @@ suite('pipUtils - validatePyproject', () => {
459457

460458
// Edge cases
461459
test('should accept when no version field exists', () => {
462-
const toml: tomljs.JsonMap = {
463-
project: { name: 'test' } as tomljs.JsonMap,
460+
const toml: PyprojectToml = {
461+
project: { name: 'test' },
464462
};
465463
verifyValidationError(toml, undefined);
466464
});
467465

468466
test('should accept when no project section exists', () => {
469-
const toml: tomljs.JsonMap = {};
467+
const toml: PyprojectToml = {};
470468
verifyValidationError(toml, undefined);
471469
});
472470
});

0 commit comments

Comments
 (0)