Skip to content

Commit 2dc057b

Browse files
committed
refactor(errors): tighten weak error messages to state the rule
Audit pass against the ERROR MESSAGES doctrine (what/where/saw/fix). Four messages said "invalid" or "illegal" without naming the rule; rewrite each to state the actual contract. Terse, library-API tier — the four ingredients collapse into a single line per message. - npm.ts: "Invalid scoped package specifier" → "npm scoped specifier must contain '/' after scope (e.g. '@scope/name')" - vers.ts: "invalid semver version 'X'" → "semver version 'X' must match MAJOR.MINOR.PATCH (e.g. '1.2.3')" - validate.ts qualifier: "qualifier 'X' contains an illegal character" → "qualifier key 'X' must match [a-z0-9.\-_]" - validate.ts type: "type 'X' contains an illegal character" → "type 'X' must match [A-Za-z0-9.\-]" - vers.ts: "empty constraint" → "VERS constraint must not be empty (use '*' for the wildcard)" Test assertions updated to match the new messages. All 1775 tests green. Other throws audited and left alone — they already state the rule (e.g. "bazel requires a 'version' component", "cpan 'namespace' component must be UPPERCASE").
1 parent b875767 commit 2dc057b

6 files changed

Lines changed: 20 additions & 14 deletions

File tree

src/purl-types/npm.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,9 @@ export function parseNpmSpecifier(specifier: unknown): NpmPackageComponents {
390390
// Find the second slash (after @scope/)
391391
const slashIndex = StringPrototypeIndexOf(specifier, '/')
392392
if (slashIndex === -1) {
393-
throw new Error('Invalid scoped package specifier.')
393+
throw new Error(
394+
'npm scoped specifier must contain "/" after scope (e.g. "@scope/name").',
395+
)
394396
}
395397

396398
// Find the @ after the scope

src/validate.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ function validateQualifierKey(
195195
)
196196
) {
197197
if (throws) {
198-
throw new PurlError(`qualifier "${key}" contains an illegal character`)
198+
throw new PurlError(`qualifier key "${key}" must match [a-z0-9.\\-_]`)
199199
}
200200
return false
201201
}
@@ -455,7 +455,7 @@ function validateType(
455455
)
456456
) {
457457
if (throws) {
458-
throw new PurlError(`type "${type}" contains an illegal character`)
458+
throw new PurlError(`type "${type}" must match [A-Za-z0-9.\\-]`)
459459
/* v8 ignore next -- Unreachable code after throw. */
460460
}
461461
return false

src/vers.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ const regexSemverNumberedGroups = ObjectFreeze(
9494
function parseSemver(version: string): SemverParts {
9595
const match = RegExpPrototypeExec(regexSemverNumberedGroups, version)
9696
if (!match) {
97-
throw new PurlError(`invalid semver version "${version}"`)
97+
throw new PurlError(
98+
`semver version "${version}" must match MAJOR.MINOR.PATCH (e.g. "1.2.3")`,
99+
)
98100
}
99101
const major = Number(match[1])
100102
const minor = Number(match[2])
@@ -225,7 +227,9 @@ function parseConstraint(raw: string): VersConstraint {
225227
}
226228
// Bare version implies equality
227229
if (trimmed.length === 0) {
228-
throw new PurlError('empty constraint')
230+
throw new PurlError(
231+
'VERS constraint must not be empty (use "*" for the wildcard)',
232+
)
229233
}
230234
return ObjectFreeze({
231235
__proto__: null,

test/package-url.test.mts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ describe('PackageURL', () => {
310310
undefined,
311311
undefined,
312312
),
313-
).toThrow(/contains an illegal character|cannot start with a number/)
313+
).toThrow(/must match \[A-Za-z0-9\.\\-\]|cannot start with a number/)
314314
},
315315
)
316316

@@ -730,7 +730,7 @@ describe('PackageURL', () => {
730730

731731
it('should reject invalid scoped package format', () => {
732732
expect(() => PackageURL.fromNpm('@babel')).toThrow(
733-
'Invalid scoped package specifier',
733+
'npm scoped specifier must contain "/" after scope',
734734
)
735735
})
736736
})
@@ -803,7 +803,7 @@ describe('PackageURL', () => {
803803
)
804804

805805
expect(() => PackageURL.fromSpec('npm', '@babel')).toThrow(
806-
'Invalid scoped package specifier',
806+
'npm scoped specifier must contain "/" after scope',
807807
)
808808
})
809809

test/parse-npm-specifier.test.mts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,13 +216,13 @@ describe('parseNpmSpecifier', () => {
216216

217217
it('should throw on invalid scoped package (missing slash)', () => {
218218
expect(() => parseNpmSpecifier('@babel')).toThrow(
219-
'Invalid scoped package specifier.',
219+
'npm scoped specifier must contain "/" after scope',
220220
)
221221
})
222222

223223
it('should throw on invalid scoped package (only @ symbol)', () => {
224224
expect(() => parseNpmSpecifier('@')).toThrow(
225-
'Invalid scoped package specifier.',
225+
'npm scoped specifier must contain "/" after scope',
226226
)
227227
})
228228
})

test/purl-edge-cases.test.mts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ describe('Edge cases and additional coverage', () => {
909909
validateQualifierKey(value as string, opts),
910910
'key',
911911
'key!invalid' as unknown,
912-
/qualifier "key!invalid" contains an illegal character/,
912+
/qualifier key "key!invalid" must match \[a-z0-9\.\\-_\]/,
913913
'validkey' as unknown,
914914
],
915915
[
@@ -927,7 +927,7 @@ describe('Edge cases and additional coverage', () => {
927927
validateType(value, opts),
928928
'type',
929929
'type$illegal' as unknown,
930-
/type "type\$illegal" contains an illegal character/,
930+
/type "type\$illegal" must match \[A-Za-z0-9\.\\-\]/,
931931
'validtype' as unknown,
932932
],
933933
[
@@ -1209,7 +1209,7 @@ describe('Edge cases and additional coverage', () => {
12091209
// Test lines 73-76 - illegal character in key
12101210
expect(() =>
12111211
validateQualifierKey('key!invalid', { throws: true }),
1212-
).toThrow(/qualifier "key!invalid" contains an illegal character/)
1212+
).toThrow(/qualifier key "key!invalid" must match \[a-z0-9\.\\-_\]/)
12131213

12141214
expect(validateQualifierKey('key!invalid', { throws: false })).toBe(false)
12151215
})
@@ -1842,7 +1842,7 @@ describe('Edge cases and additional coverage', () => {
18421842

18431843
// Test URL parsing failure (line 145 branch) - malformed URL
18441844
expect(() => PackageURL.fromString('pkg::')).toThrow(
1845-
'type ":" contains an illegal character',
1845+
'type ":" must match [A-Za-z0-9.\\-]',
18461846
)
18471847

18481848
// Test the maybeUrlWithAuth branch where afterColon.length !== trimmedAfterColon.length

0 commit comments

Comments
 (0)