From 836b242d76435d91179672a896904965ced53490 Mon Sep 17 00:00:00 2001 From: Hanna Skryl <80118140+hanna-skryl@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:57:59 -0500 Subject: [PATCH] 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 | 32 ++++++++++++++++--- .../src/lib/category-config.unit.test.ts | 6 ++-- packages/models/src/lib/group.ts | 1 + packages/models/src/lib/group.unit.test.ts | 2 +- .../models/src/lib/implementation/schemas.ts | 20 ++++++------ .../src/lib/lighthouse-plugin.unit.test.ts | 10 ++++++ 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 ++++++++ 14 files changed, 108 insertions(+), 27 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/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..bb310f5db 100644 --- a/packages/core/src/lib/implementation/read-rc-file.ts +++ b/packages/core/src/lib/implementation/read-rc-file.ts @@ -1,11 +1,17 @@ -import { join } from 'node:path'; +import { bold } from 'ansis'; +import path, { join } from 'node:path'; +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'; +import { + fileExists, + importModule, + zodErrorMessageBuilder, +} from '@code-pushup/utils'; export class ConfigPathError extends Error { constructor(configPath: string) { @@ -13,6 +19,13 @@ export class ConfigPathError extends Error { } } +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 +40,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: zodErrorMessageBuilder, + }); + throw isZodErrorLike(error) + ? new ConfigValidationError(filepath, validationError.message) + : error; + } } export async function autoloadRc(tsconfig?: string): Promise { @@ -35,8 +57,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; diff --git a/packages/models/src/lib/category-config.unit.test.ts b/packages/models/src/lib/category-config.unit.test.ts index dede5596a..d234c8292 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 a category, 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 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 aad2d63c2..31e795cd9 100644 --- a/packages/models/src/lib/group.ts +++ b/packages/models/src/lib/group.ts @@ -32,6 +32,7 @@ export const groupSchema = scorableSchema( 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 d45b68ab6..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 a category 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 6f785b111..f68436687 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(); @@ -172,7 +172,7 @@ export function scorableSchema>( slug: slugSchema.describe('Human-readable unique ID, e.g. "performance"'), refs: z .array(refSchema) - .min(1) + .min(1, { message: 'In a category, there has to be at least one ref' }) // refs are unique .refine( refs => !duplicateCheckFn(refs), @@ -180,11 +180,13 @@ export function scorableSchema>( message: duplicateMessageFn(refs), }), ) - // categories weights are correct - .refine(hasNonZeroWeightedRef, () => ({ - message: - 'In a category there has to be at least one ref with weight > 0', - })), + // 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 9bcb20015..18034a36d 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 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..28e638072 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 { zodErrorMessageBuilder } 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..ae7813634 --- /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 zodErrorMessageBuilder: 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..2ce8be6b5 --- /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]'], + ])('should format error path %j as %j', (input, expected) => { + expect(formatErrorPath(input)).toBe(expected); + }); +});