feat(core): enhance config validation

hanna-skryl · web-flow · commit 836b242d7643 · 2024-12-13T09:57:59.000-05:00
diff --git a/package-lock.json b/package-lock.json
diff --git 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
@@ -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
@@ -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
@@ -1,18 +1,31 @@
-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) {
     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,16 +40,25 @@ 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<CoreConfig> {
   // eslint-disable-next-line functional/no-let
   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
@@ -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
@@ -32,6 +32,7 @@ export const groupSchema = scorableSchema(
   getDuplicateRefsInGroups,
   duplicateRefsInGroupsErrorMsg,
 ).merge(groupMetaSchema);
+
 export type Group = z.infer<typeof groupSchema>;
 
 export const groupsSchema = z
diff --git 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
@@ -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,19 +172,21 @@ export function scorableSchema<T extends ReturnType<typeof weightedRefSchema>>(
       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),
           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',
-        })),
+        // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+  });
+});
