From 676cbb7b11a332f394c5ee9fa0086a3bf7d18538 Mon Sep 17 00:00:00 2001 From: hanna-skryl Date: Wed, 11 Dec 2024 20:00:33 -0500 Subject: [PATCH 1/4] feat(core): enhance config validation --- package-lock.json | 11 ++-- package.json | 3 +- packages/core/package.json | 3 +- .../read-rc-file.integration.test.ts | 4 +- .../src/lib/implementation/read-rc-file.ts | 49 ++++++++++++++- packages/models/src/lib/category-config.ts | 17 ++--- .../src/lib/category-config.unit.test.ts | 6 +- packages/models/src/lib/group.ts | 17 ++--- packages/models/src/lib/group.unit.test.ts | 2 +- .../models/src/lib/implementation/schemas.ts | 63 ++++++++++++------- .../src/lib/lighthouse-plugin.unit.test.ts | 10 +++ 11 files changed, 133 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index 25e7d2d2d..32e4ba6af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,7 +31,8 @@ "vscode-material-icons": "^0.1.1", "yaml": "^2.5.1", "yargs": "^17.7.2", - "zod": "^3.23.8" + "zod": "^3.23.8", + "zod-validation-error": "^3.4.0" }, "devDependencies": { "@beaussan/nx-knip": "^0.0.5-15", @@ -27746,10 +27747,10 @@ } }, "node_modules/zod-validation-error": { - "version": "3.3.1", - "resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-3.3.1.tgz", - "integrity": "sha512-uFzCZz7FQis256dqw4AhPQgD6f3pzNca/Zh62RNELavlumQB3nDIUFbF5JQfFLcMbO1s02Q7Xg/gpcOBlEnYZA==", - "dev": true, + "version": "3.4.0", + "resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-3.4.0.tgz", + "integrity": "sha512-ZOPR9SVY6Pb2qqO5XHt+MkkTRxGXb4EVtnjc9JpXUOtUB1T9Ru7mZOT361AN3MsetVe7R0a1KZshJDZdgp9miQ==", + "license": "MIT", "engines": { "node": ">=18.0.0" }, diff --git a/package.json b/package.json index fbe017407..3b89d4117 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,8 @@ "vscode-material-icons": "^0.1.1", "yaml": "^2.5.1", "yargs": "^17.7.2", - "zod": "^3.23.8" + "zod": "^3.23.8", + "zod-validation-error": "^3.4.0" }, "devDependencies": { "@beaussan/nx-knip": "^0.0.5-15", diff --git a/packages/core/package.json b/packages/core/package.json index 4d319224c..60d2c7481 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -41,7 +41,8 @@ "dependencies": { "@code-pushup/models": "0.56.0", "@code-pushup/utils": "0.56.0", - "ansis": "^3.3.0" + "ansis": "^3.3.0", + "zod-validation-error": "^3.4.0" }, "peerDependencies": { "@code-pushup/portal-client": "^0.9.0" diff --git a/packages/core/src/lib/implementation/read-rc-file.integration.test.ts b/packages/core/src/lib/implementation/read-rc-file.integration.test.ts index 9a780f58e..b6a86bd1a 100644 --- a/packages/core/src/lib/implementation/read-rc-file.integration.test.ts +++ b/packages/core/src/lib/implementation/read-rc-file.integration.test.ts @@ -1,7 +1,7 @@ import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect } from 'vitest'; -import { readRcByPath } from './read-rc-file.js'; +import { ConfigValidationError, readRcByPath } from './read-rc-file.js'; describe('readRcByPath', () => { const configDirPath = join( @@ -69,7 +69,7 @@ describe('readRcByPath', () => { it('should throw if the configuration is empty', async () => { await expect( readRcByPath(join(configDirPath, 'code-pushup.empty.config.js')), - ).rejects.toThrow(`"code": "invalid_type",`); + ).rejects.toThrow(expect.any(ConfigValidationError)); }); it('should throw if the configuration is invalid', async () => { diff --git a/packages/core/src/lib/implementation/read-rc-file.ts b/packages/core/src/lib/implementation/read-rc-file.ts index 093e44460..679f24a3e 100644 --- a/packages/core/src/lib/implementation/read-rc-file.ts +++ b/packages/core/src/lib/implementation/read-rc-file.ts @@ -1,4 +1,10 @@ -import { join } from 'node:path'; +import { bold, red } from 'ansis'; +import path, { join } from 'node:path'; +import { + type MessageBuilder, + fromError, + isZodErrorLike, +} from 'zod-validation-error'; import { CONFIG_FILE_NAME, type CoreConfig, @@ -7,12 +13,42 @@ import { } from '@code-pushup/models'; import { fileExists, importModule } from '@code-pushup/utils'; +function formatErrorPath(errorPath: (string | number)[]): string { + return errorPath + .map((key, index) => { + if (typeof key === 'number') { + return `[${key}]`; + } + return index > 0 ? `.${key}` : key; + }) + .join(''); +} + +const coreConfigMessageBuilder: MessageBuilder = issues => + issues + .map(issue => { + const formattedMessage = red(`${bold(issue.code)}: ${issue.message}`); + const formattedPath = formatErrorPath(issue.path); + if (formattedPath) { + return `Validation error at ${bold(formattedPath)}\n${formattedMessage}\n`; + } + return `${formattedMessage}\n`; + }) + .join('\n'); + export class ConfigPathError extends Error { constructor(configPath: string) { super(`Provided path '${configPath}' is not valid.`); } } +export class ConfigValidationError extends Error { + constructor(configPath: string, message: string) { + const relativePath = path.relative(process.cwd(), configPath); + super(`Failed parsing core config in ${bold(relativePath)}.\n\n${message}`); + } +} + export async function readRcByPath( filepath: string, tsconfig?: string, @@ -27,7 +63,16 @@ export async function readRcByPath( const cfg = await importModule({ filepath, tsconfig, format: 'esm' }); - return coreConfigSchema.parse(cfg); + try { + return coreConfigSchema.parse(cfg); + } catch (error) { + const validationError = fromError(error, { + messageBuilder: coreConfigMessageBuilder, + }); + throw isZodErrorLike(error) + ? new ConfigValidationError(filepath, validationError.message) + : error; + } } export async function autoloadRc(tsconfig?: string): Promise { diff --git a/packages/models/src/lib/category-config.ts b/packages/models/src/lib/category-config.ts index 16dd04aa9..4f5e4ce2d 100644 --- a/packages/models/src/lib/category-config.ts +++ b/packages/models/src/lib/category-config.ts @@ -23,13 +23,14 @@ export const categoryRefSchema = weightedRefSchema( ); export type CategoryRef = z.infer; -export const categoryConfigSchema = scorableSchema( - 'Category with a score calculated from audits and groups from various plugins', - categoryRefSchema, - getDuplicateRefsInCategoryMetrics, - duplicateRefsInCategoryMetricsErrorMsg, -) - .merge( +export const categoryConfigSchema = z + .intersection( + scorableSchema( + 'Category with a score calculated from audits and groups from various plugins', + categoryRefSchema, + getDuplicateRefsInCategoryMetrics, + duplicateRefsInCategoryMetricsErrorMsg, + ), metaSchema({ titleDescription: 'Category Title', docsUrlDescription: 'Category docs URL', @@ -37,7 +38,7 @@ export const categoryConfigSchema = scorableSchema( description: 'Meta info for category', }), ) - .merge( + .and( z.object({ isBinary: z .boolean({ diff --git a/packages/models/src/lib/category-config.unit.test.ts b/packages/models/src/lib/category-config.unit.test.ts index dede5596a..c1e52d704 100644 --- a/packages/models/src/lib/category-config.unit.test.ts +++ b/packages/models/src/lib/category-config.unit.test.ts @@ -129,7 +129,7 @@ describe('categoryConfigSchema', () => { title: 'This category is empty for now', refs: [], } satisfies CategoryConfig), - ).toThrow('In a category there has to be at least one ref'); + ).toThrow('In category in-progress, there has to be at least one ref'); }); it('should throw for duplicate category references', () => { @@ -175,7 +175,9 @@ describe('categoryConfigSchema', () => { }, ], } satisfies CategoryConfig), - ).toThrow('In a category there has to be at least one ref with weight > 0'); + ).toThrow( + 'In category informational, there has to be at least one ref with weight > 0. Affected refs: functional/immutable-data, lighthouse-experimental', + ); }); }); diff --git a/packages/models/src/lib/group.ts b/packages/models/src/lib/group.ts index aad2d63c2..686067452 100644 --- a/packages/models/src/lib/group.ts +++ b/packages/models/src/lib/group.ts @@ -25,13 +25,16 @@ export const groupMetaSchema = metaSchema({ }); export type GroupMeta = z.infer; -export const groupSchema = scorableSchema( - 'A group aggregates a set of audits into a single score which can be referenced from a category. ' + - 'E.g. the group slug "performance" groups audits and can be referenced in a category', - groupRefSchema, - getDuplicateRefsInGroups, - duplicateRefsInGroupsErrorMsg, -).merge(groupMetaSchema); +export const groupSchema = z.intersection( + scorableSchema( + 'A group aggregates a set of audits into a single score which can be referenced from a category. ' + + 'E.g. the group slug "performance" groups audits and can be referenced in a category', + groupRefSchema, + getDuplicateRefsInGroups, + duplicateRefsInGroupsErrorMsg, + ), + groupMetaSchema, +); export type Group = z.infer; export const groupsSchema = z diff --git a/packages/models/src/lib/group.unit.test.ts b/packages/models/src/lib/group.unit.test.ts index d45b68ab6..465150ca2 100644 --- a/packages/models/src/lib/group.unit.test.ts +++ b/packages/models/src/lib/group.unit.test.ts @@ -69,7 +69,7 @@ describe('groupSchema', () => { title: 'Empty group', refs: [], } satisfies Group), - ).toThrow('In a category there has to be at least one ref'); + ).toThrow('In category empty-group, there has to be at least one ref'); }); it('should throw for duplicate group references', () => { diff --git a/packages/models/src/lib/implementation/schemas.ts b/packages/models/src/lib/implementation/schemas.ts index 6f785b111..8daa1ef4e 100644 --- a/packages/models/src/lib/implementation/schemas.ts +++ b/packages/models/src/lib/implementation/schemas.ts @@ -38,7 +38,7 @@ export const slugSchema = z 'The slug has to follow the pattern [0-9a-z] followed by multiple optional groups of -[0-9a-z]. e.g. my-slug', }) .max(MAX_SLUG_LENGTH, { - message: `slug can be max ${MAX_SLUG_LENGTH} characters long`, + message: `The slug can be max ${MAX_SLUG_LENGTH} characters long`, }); /** Schema for a general description property */ @@ -105,7 +105,7 @@ export function metaSchema(options?: { export const filePathSchema = z .string() .trim() - .min(1, { message: 'path is invalid' }); + .min(1, { message: 'The path is invalid' }); /** Schema for a fileNameSchema */ export const fileNameSchema = z @@ -114,7 +114,7 @@ export const fileNameSchema = z .regex(filenameRegex, { message: `The filename has to be valid`, }) - .min(1, { message: 'file name is invalid' }); + .min(1, { message: 'The file name is invalid' }); /** Schema for a positiveInt */ export const positiveIntSchema = z.number().int().positive(); @@ -167,26 +167,43 @@ export function scorableSchema>( duplicateCheckFn: (metrics: z.infer[]) => false | string[], duplicateMessageFn: (metrics: z.infer[]) => string, ) { - return z.object( - { - slug: slugSchema.describe('Human-readable unique ID, e.g. "performance"'), - refs: z - .array(refSchema) - .min(1) - // refs are unique - .refine( - refs => !duplicateCheckFn(refs), - refs => ({ - message: duplicateMessageFn(refs), - }), - ) - // categories weights are correct - .refine(hasNonZeroWeightedRef, () => ({ - message: - 'In a category there has to be at least one ref with weight > 0', - })), - }, - { description }, + return ( + z + .object( + { + slug: slugSchema.describe( + 'Human-readable unique ID, e.g. "performance"', + ), + refs: z + .array(refSchema) + .min(1) + // refs are unique + .refine( + refs => !duplicateCheckFn(refs), + refs => ({ + message: duplicateMessageFn(refs), + }), + ), + }, + { description }, + ) + // category weights are correct + .superRefine(({ slug, refs }, ctx) => { + if (refs.length === 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `In category ${slug}, there has to be at least one ref`, + path: ['refs'], + }); + } else if (!hasNonZeroWeightedRef(refs)) { + const affectedRefs = refs.map(ref => ref.slug).join(', '); + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `In category ${slug}, there has to be at least one ref with weight > 0. Affected refs: ${affectedRefs}`, + path: ['refs'], + }); + } + }) ); } diff --git a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts index 9bcb20015..8ed57423f 100644 --- a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts +++ b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts @@ -50,4 +50,14 @@ describe('lighthousePlugin-config-object', () => { ]), ); }); + + it('should throw when filtering groups by zero-weight onlyAudits', () => { + const pluginConfig = lighthousePlugin('https://code-pushup-portal.com', { + onlyAudits: ['csp-xss'], + }); + + expect(() => pluginConfigSchema.parse(pluginConfig)).toThrow( + 'In category best-practices, there has to be at least one ref with weight > 0. Affected refs: csp-xss', + ); + }); }); From 98ad90ebbb7f5a620ec78fb59d3080b0cb895d95 Mon Sep 17 00:00:00 2001 From: hanna-skryl Date: Wed, 11 Dec 2024 20:26:24 -0500 Subject: [PATCH 2/4] feat(core): enhance config validation --- packages/core/src/lib/implementation/read-rc-file.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/lib/implementation/read-rc-file.ts b/packages/core/src/lib/implementation/read-rc-file.ts index 679f24a3e..9df0a8586 100644 --- a/packages/core/src/lib/implementation/read-rc-file.ts +++ b/packages/core/src/lib/implementation/read-rc-file.ts @@ -80,8 +80,8 @@ export async function autoloadRc(tsconfig?: string): Promise { let ext = ''; // eslint-disable-next-line functional/no-loop-statements for (const extension of SUPPORTED_CONFIG_FILE_FORMATS) { - const path = `${CONFIG_FILE_NAME}.${extension}`; - const exists = await fileExists(path); + const filePath = `${CONFIG_FILE_NAME}.${extension}`; + const exists = await fileExists(filePath); if (exists) { ext = extension; From 5efe94a4691b3c0d36e0f5a2610dad0dacffdaae Mon Sep 17 00:00:00 2001 From: hanna-skryl Date: Thu, 12 Dec 2024 12:53:26 -0500 Subject: [PATCH 3/4] feat(core): enhance config validation --- .../src/lib/implementation/read-rc-file.ts | 37 +++--------- packages/models/src/lib/category-config.ts | 17 +++--- .../src/lib/category-config.unit.test.ts | 4 +- packages/models/src/lib/group.ts | 18 +++--- packages/models/src/lib/group.unit.test.ts | 2 +- .../models/src/lib/implementation/schemas.ts | 59 +++++++------------ .../src/lib/lighthouse-plugin.unit.test.ts | 2 +- packages/utils/package.json | 3 +- packages/utils/src/index.ts | 1 + packages/utils/src/lib/zod-validation.ts | 25 ++++++++ .../utils/src/lib/zod-validation.unit.test.ts | 14 +++++ 11 files changed, 91 insertions(+), 91 deletions(-) create mode 100644 packages/utils/src/lib/zod-validation.ts create mode 100644 packages/utils/src/lib/zod-validation.unit.test.ts diff --git a/packages/core/src/lib/implementation/read-rc-file.ts b/packages/core/src/lib/implementation/read-rc-file.ts index 9df0a8586..32b577929 100644 --- a/packages/core/src/lib/implementation/read-rc-file.ts +++ b/packages/core/src/lib/implementation/read-rc-file.ts @@ -1,40 +1,17 @@ -import { bold, red } from 'ansis'; +import { bold } from 'ansis'; import path, { join } from 'node:path'; -import { - type MessageBuilder, - fromError, - isZodErrorLike, -} from 'zod-validation-error'; +import { fromError, isZodErrorLike } from 'zod-validation-error'; import { CONFIG_FILE_NAME, type CoreConfig, SUPPORTED_CONFIG_FILE_FORMATS, coreConfigSchema, } from '@code-pushup/models'; -import { fileExists, importModule } from '@code-pushup/utils'; - -function formatErrorPath(errorPath: (string | number)[]): string { - return errorPath - .map((key, index) => { - if (typeof key === 'number') { - return `[${key}]`; - } - return index > 0 ? `.${key}` : key; - }) - .join(''); -} - -const coreConfigMessageBuilder: MessageBuilder = issues => - issues - .map(issue => { - const formattedMessage = red(`${bold(issue.code)}: ${issue.message}`); - const formattedPath = formatErrorPath(issue.path); - if (formattedPath) { - return `Validation error at ${bold(formattedPath)}\n${formattedMessage}\n`; - } - return `${formattedMessage}\n`; - }) - .join('\n'); +import { + coreConfigMessageBuilder, + fileExists, + importModule, +} from '@code-pushup/utils'; export class ConfigPathError extends Error { constructor(configPath: string) { diff --git a/packages/models/src/lib/category-config.ts b/packages/models/src/lib/category-config.ts index 4f5e4ce2d..16dd04aa9 100644 --- a/packages/models/src/lib/category-config.ts +++ b/packages/models/src/lib/category-config.ts @@ -23,14 +23,13 @@ export const categoryRefSchema = weightedRefSchema( ); export type CategoryRef = z.infer; -export const categoryConfigSchema = z - .intersection( - scorableSchema( - 'Category with a score calculated from audits and groups from various plugins', - categoryRefSchema, - getDuplicateRefsInCategoryMetrics, - duplicateRefsInCategoryMetricsErrorMsg, - ), +export const categoryConfigSchema = scorableSchema( + 'Category with a score calculated from audits and groups from various plugins', + categoryRefSchema, + getDuplicateRefsInCategoryMetrics, + duplicateRefsInCategoryMetricsErrorMsg, +) + .merge( metaSchema({ titleDescription: 'Category Title', docsUrlDescription: 'Category docs URL', @@ -38,7 +37,7 @@ export const categoryConfigSchema = z description: 'Meta info for category', }), ) - .and( + .merge( z.object({ isBinary: z .boolean({ diff --git a/packages/models/src/lib/category-config.unit.test.ts b/packages/models/src/lib/category-config.unit.test.ts index c1e52d704..f82df8a48 100644 --- a/packages/models/src/lib/category-config.unit.test.ts +++ b/packages/models/src/lib/category-config.unit.test.ts @@ -129,7 +129,7 @@ describe('categoryConfigSchema', () => { title: 'This category is empty for now', refs: [], } satisfies CategoryConfig), - ).toThrow('In category in-progress, there has to be at least one ref'); + ).toThrow('In a category, there has to be at least one ref'); }); it('should throw for duplicate category references', () => { @@ -176,7 +176,7 @@ describe('categoryConfigSchema', () => { ], } satisfies CategoryConfig), ).toThrow( - 'In category informational, there has to be at least one ref with weight > 0. Affected refs: functional/immutable-data, lighthouse-experimental', + /In a category, there has to be at least one ref with weight > 0. Affected refs: \\"functional\/immutable-data\\", \\"lighthouse-experimental\\"/, ); }); }); diff --git a/packages/models/src/lib/group.ts b/packages/models/src/lib/group.ts index 686067452..31e795cd9 100644 --- a/packages/models/src/lib/group.ts +++ b/packages/models/src/lib/group.ts @@ -25,16 +25,14 @@ export const groupMetaSchema = metaSchema({ }); export type GroupMeta = z.infer; -export const groupSchema = z.intersection( - scorableSchema( - 'A group aggregates a set of audits into a single score which can be referenced from a category. ' + - 'E.g. the group slug "performance" groups audits and can be referenced in a category', - groupRefSchema, - getDuplicateRefsInGroups, - duplicateRefsInGroupsErrorMsg, - ), - groupMetaSchema, -); +export const groupSchema = scorableSchema( + 'A group aggregates a set of audits into a single score which can be referenced from a category. ' + + 'E.g. the group slug "performance" groups audits and can be referenced in a category', + groupRefSchema, + getDuplicateRefsInGroups, + duplicateRefsInGroupsErrorMsg, +).merge(groupMetaSchema); + export type Group = z.infer; export const groupsSchema = z diff --git a/packages/models/src/lib/group.unit.test.ts b/packages/models/src/lib/group.unit.test.ts index 465150ca2..e0f9b5149 100644 --- a/packages/models/src/lib/group.unit.test.ts +++ b/packages/models/src/lib/group.unit.test.ts @@ -69,7 +69,7 @@ describe('groupSchema', () => { title: 'Empty group', refs: [], } satisfies Group), - ).toThrow('In category empty-group, there has to be at least one ref'); + ).toThrow('In a category, there has to be at least one ref'); }); it('should throw for duplicate group references', () => { diff --git a/packages/models/src/lib/implementation/schemas.ts b/packages/models/src/lib/implementation/schemas.ts index 8daa1ef4e..01c3a5206 100644 --- a/packages/models/src/lib/implementation/schemas.ts +++ b/packages/models/src/lib/implementation/schemas.ts @@ -167,43 +167,28 @@ export function scorableSchema>( duplicateCheckFn: (metrics: z.infer[]) => false | string[], duplicateMessageFn: (metrics: z.infer[]) => string, ) { - return ( - z - .object( - { - slug: slugSchema.describe( - 'Human-readable unique ID, e.g. "performance"', - ), - refs: z - .array(refSchema) - .min(1) - // refs are unique - .refine( - refs => !duplicateCheckFn(refs), - refs => ({ - message: duplicateMessageFn(refs), - }), - ), - }, - { description }, - ) - // category weights are correct - .superRefine(({ slug, refs }, ctx) => { - if (refs.length === 0) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `In category ${slug}, there has to be at least one ref`, - path: ['refs'], - }); - } else if (!hasNonZeroWeightedRef(refs)) { - const affectedRefs = refs.map(ref => ref.slug).join(', '); - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `In category ${slug}, there has to be at least one ref with weight > 0. Affected refs: ${affectedRefs}`, - path: ['refs'], - }); - } - }) + return z.object( + { + slug: slugSchema.describe('Human-readable unique ID, e.g. "performance"'), + refs: z + .array(refSchema) + .min(1, { message: 'In a category, there has to be at least one ref' }) + // refs are unique + .refine( + refs => !duplicateCheckFn(refs), + refs => ({ + message: duplicateMessageFn(refs), + }), + ) + // category weights are correct + .refine(hasNonZeroWeightedRef, refs => { + const affectedRefs = refs.map(ref => `"${ref.slug}"`).join(', '); + return { + message: `In a category, there has to be at least one ref with weight > 0. Affected refs: ${affectedRefs}`, + }; + }), + }, + { description }, ); } diff --git a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts index 8ed57423f..2f810e170 100644 --- a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts +++ b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts @@ -57,7 +57,7 @@ describe('lighthousePlugin-config-object', () => { }); expect(() => pluginConfigSchema.parse(pluginConfig)).toThrow( - 'In category best-practices, there has to be at least one ref with weight > 0. Affected refs: csp-xss', + /In a category, there has to be at least one ref with weight > 0. Affected refs: \\"csp-xss\\"/, ); }); }); diff --git a/packages/utils/package.json b/packages/utils/package.json index 68da5b75b..af18362a9 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -33,6 +33,7 @@ "esbuild": "^0.19.2", "multi-progress-bars": "^5.0.3", "semver": "^7.6.0", - "simple-git": "^3.20.0" + "simple-git": "^3.20.0", + "zod-validation-error": "^3.4.0" } } diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index e53715e75..c22523878 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -122,3 +122,4 @@ export type { WithRequired, } from './lib/types.js'; export { verboseUtils } from './lib/verbose-utils.js'; +export { coreConfigMessageBuilder } from './lib/zod-validation.js'; diff --git a/packages/utils/src/lib/zod-validation.ts b/packages/utils/src/lib/zod-validation.ts new file mode 100644 index 000000000..04104193f --- /dev/null +++ b/packages/utils/src/lib/zod-validation.ts @@ -0,0 +1,25 @@ +import { bold, red } from 'ansis'; +import type { MessageBuilder } from 'zod-validation-error'; + +export function formatErrorPath(errorPath: (string | number)[]): string { + return errorPath + .map((key, index) => { + if (typeof key === 'number') { + return `[${key}]`; + } + return index > 0 ? `.${key}` : key; + }) + .join(''); +} + +export const coreConfigMessageBuilder: MessageBuilder = issues => + issues + .map(issue => { + const formattedMessage = red(`${bold(issue.code)}: ${issue.message}`); + const formattedPath = formatErrorPath(issue.path); + if (formattedPath) { + return `Validation error at ${bold(formattedPath)}\n${formattedMessage}\n`; + } + return `${formattedMessage}\n`; + }) + .join('\n'); diff --git a/packages/utils/src/lib/zod-validation.unit.test.ts b/packages/utils/src/lib/zod-validation.unit.test.ts new file mode 100644 index 000000000..11fe3bce4 --- /dev/null +++ b/packages/utils/src/lib/zod-validation.unit.test.ts @@ -0,0 +1,14 @@ +import { formatErrorPath } from './zod-validation'; + +describe('formatErrorPath', () => { + it.each([ + [['categories', 1, 'slug'], 'categories[1].slug'], + [['plugins', 2, 'groups', 0, 'refs'], 'plugins[2].groups[0].refs'], + [['refs', 0, 'slug'], 'refs[0].slug'], + [['categories'], 'categories'], + [[], ''], + [['path', 5], 'path[5]'], + ])('formats error path correctly for $input', (input, expected) => { + expect(formatErrorPath(input)).toBe(expected); + }); +}); From 725afc41163c75cbe3c75814aefad898a87c9feb Mon Sep 17 00:00:00 2001 From: hanna-skryl Date: Fri, 13 Dec 2024 07:49:56 -0500 Subject: [PATCH 4/4] feat(core): enhance config validation --- packages/core/src/lib/implementation/read-rc-file.ts | 4 ++-- packages/models/src/lib/category-config.unit.test.ts | 2 +- packages/models/src/lib/implementation/schemas.ts | 2 +- .../plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts | 2 +- packages/utils/src/index.ts | 2 +- packages/utils/src/lib/zod-validation.ts | 2 +- packages/utils/src/lib/zod-validation.unit.test.ts | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/lib/implementation/read-rc-file.ts b/packages/core/src/lib/implementation/read-rc-file.ts index 32b577929..bb310f5db 100644 --- a/packages/core/src/lib/implementation/read-rc-file.ts +++ b/packages/core/src/lib/implementation/read-rc-file.ts @@ -8,9 +8,9 @@ import { coreConfigSchema, } from '@code-pushup/models'; import { - coreConfigMessageBuilder, fileExists, importModule, + zodErrorMessageBuilder, } from '@code-pushup/utils'; export class ConfigPathError extends Error { @@ -44,7 +44,7 @@ export async function readRcByPath( return coreConfigSchema.parse(cfg); } catch (error) { const validationError = fromError(error, { - messageBuilder: coreConfigMessageBuilder, + messageBuilder: zodErrorMessageBuilder, }); throw isZodErrorLike(error) ? new ConfigValidationError(filepath, validationError.message) diff --git a/packages/models/src/lib/category-config.unit.test.ts b/packages/models/src/lib/category-config.unit.test.ts index f82df8a48..d234c8292 100644 --- a/packages/models/src/lib/category-config.unit.test.ts +++ b/packages/models/src/lib/category-config.unit.test.ts @@ -176,7 +176,7 @@ describe('categoryConfigSchema', () => { ], } satisfies CategoryConfig), ).toThrow( - /In a category, there has to be at least one ref with weight > 0. Affected refs: \\"functional\/immutable-data\\", \\"lighthouse-experimental\\"/, + 'In a category, there has to be at least one ref with weight > 0. Affected refs: functional/immutable-data, lighthouse-experimental', ); }); }); diff --git a/packages/models/src/lib/implementation/schemas.ts b/packages/models/src/lib/implementation/schemas.ts index 01c3a5206..f68436687 100644 --- a/packages/models/src/lib/implementation/schemas.ts +++ b/packages/models/src/lib/implementation/schemas.ts @@ -182,7 +182,7 @@ export function scorableSchema>( ) // category weights are correct .refine(hasNonZeroWeightedRef, refs => { - const affectedRefs = refs.map(ref => `"${ref.slug}"`).join(', '); + const affectedRefs = refs.map(ref => ref.slug).join(', '); return { message: `In a category, there has to be at least one ref with weight > 0. Affected refs: ${affectedRefs}`, }; diff --git a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts index 2f810e170..18034a36d 100644 --- a/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts +++ b/packages/plugin-lighthouse/src/lib/lighthouse-plugin.unit.test.ts @@ -57,7 +57,7 @@ describe('lighthousePlugin-config-object', () => { }); expect(() => pluginConfigSchema.parse(pluginConfig)).toThrow( - /In a category, there has to be at least one ref with weight > 0. Affected refs: \\"csp-xss\\"/, + 'In a category, there has to be at least one ref with weight > 0. Affected refs: csp-xss', ); }); }); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index c22523878..28e638072 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -122,4 +122,4 @@ export type { WithRequired, } from './lib/types.js'; export { verboseUtils } from './lib/verbose-utils.js'; -export { coreConfigMessageBuilder } from './lib/zod-validation.js'; +export { zodErrorMessageBuilder } from './lib/zod-validation.js'; diff --git a/packages/utils/src/lib/zod-validation.ts b/packages/utils/src/lib/zod-validation.ts index 04104193f..ae7813634 100644 --- a/packages/utils/src/lib/zod-validation.ts +++ b/packages/utils/src/lib/zod-validation.ts @@ -12,7 +12,7 @@ export function formatErrorPath(errorPath: (string | number)[]): string { .join(''); } -export const coreConfigMessageBuilder: MessageBuilder = issues => +export const zodErrorMessageBuilder: MessageBuilder = issues => issues .map(issue => { const formattedMessage = red(`${bold(issue.code)}: ${issue.message}`); diff --git a/packages/utils/src/lib/zod-validation.unit.test.ts b/packages/utils/src/lib/zod-validation.unit.test.ts index 11fe3bce4..2ce8be6b5 100644 --- a/packages/utils/src/lib/zod-validation.unit.test.ts +++ b/packages/utils/src/lib/zod-validation.unit.test.ts @@ -8,7 +8,7 @@ describe('formatErrorPath', () => { [['categories'], 'categories'], [[], ''], [['path', 5], 'path[5]'], - ])('formats error path correctly for $input', (input, expected) => { + ])('should format error path %j as %j', (input, expected) => { expect(formatErrorPath(input)).toBe(expected); }); });