From d28f34ded2590f80405a2c34f02d095f336ba9f3 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Sun, 10 Nov 2024 22:26:31 +0900 Subject: [PATCH 1/2] feat(config): Implement Zod for robust configuration validation --- package-lock.json | 13 +- package.json | 3 +- src/cli/actions/defaultAction.ts | 2 +- src/cli/actions/initAction.ts | 2 +- src/cli/cliPrint.ts | 2 +- src/cli/cliRun.ts | 2 +- src/config/configLoad.ts | 29 ++-- src/config/configSchema.ts | 81 ++++++++++ src/config/configTypes.ts | 57 ------- src/config/configValidate.ts | 72 --------- src/config/defaultConfig.ts | 2 +- src/core/file/fileProcess.ts | 2 +- src/core/file/fileSearch.ts | 2 +- src/core/output/outputGenerate.ts | 2 +- src/core/output/outputGeneratorTypes.ts | 2 +- src/core/output/outputStyleDecorate.ts | 2 +- src/core/packager.ts | 2 +- src/index.ts | 2 +- src/shared/errorHandle.ts | 17 ++ tests/config/configLoad.test.ts | 26 ++- tests/config/configSchema.test.ts | 198 +++++++++++++++++++++++ tests/config/configValidate.test.ts | 51 ------ tests/integration-tests/packager.test.ts | 2 +- tests/testing/testUtils.ts | 2 +- 24 files changed, 367 insertions(+), 208 deletions(-) create mode 100644 src/config/configSchema.ts delete mode 100644 src/config/configTypes.ts delete mode 100644 src/config/configValidate.ts create mode 100644 tests/config/configSchema.test.ts delete mode 100644 tests/config/configValidate.test.ts diff --git a/package-lock.json b/package-lock.json index 0b10e38..17d6a28 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,8 @@ "p-map": "^7.0.2", "picocolors": "^1.1.1", "strip-comments": "^2.0.1", - "tiktoken": "^1.0.17" + "tiktoken": "^1.0.17", + "zod": "^3.23.8" }, "bin": { "repomix": "bin/repomix.cjs" @@ -426,6 +427,7 @@ }, "node_modules/@clack/prompts/node_modules/is-unicode-supported": { "version": "1.3.0", + "extraneous": true, "inBundle": true, "license": "MIT", "engines": { @@ -4273,6 +4275,15 @@ "engines": { "node": ">=8" } + }, + "node_modules/zod": { + "version": "3.23.8", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.23.8.tgz", + "integrity": "sha512-XBx9AXhXktjUqnepgTiE5flcKIYWi/rme0Eaj+5Y0lftuGBq+jyRu/md4WnuxqgP1ubdpNCsYEYPxrzVHD8d6g==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } } } } diff --git a/package.json b/package.json index 2e88d14..f7fe045 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,8 @@ "p-map": "^7.0.2", "picocolors": "^1.1.1", "strip-comments": "^2.0.1", - "tiktoken": "^1.0.17" + "tiktoken": "^1.0.17", + "zod": "^3.23.8" }, "devDependencies": { "@biomejs/biome": "^1.9.4", diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index 313c46b..29f83ad 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -5,7 +5,7 @@ import type { RepomixConfigFile, RepomixConfigMerged, RepomixOutputStyle, -} from '../../config/configTypes.js'; +} from '../../config/configSchema.js'; import { type PackResult, pack } from '../../core/packager.js'; import { logger } from '../../shared/logger.js'; import { printCompletion, printSecurityCheck, printSummary, printTopFiles } from '../cliPrint.js'; diff --git a/src/cli/actions/initAction.ts b/src/cli/actions/initAction.ts index b095197..5b6b4c4 100644 --- a/src/cli/actions/initAction.ts +++ b/src/cli/actions/initAction.ts @@ -2,7 +2,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import * as prompts from '@clack/prompts'; import pc from 'picocolors'; -import type { RepomixConfigFile, RepomixOutputStyle } from '../../config/configTypes.js'; +import type { RepomixConfigFile, RepomixOutputStyle } from '../../config/configSchema.js'; import { defaultConfig, defaultFilePathMap } from '../../config/defaultConfig.js'; import { getGlobalDirectory } from '../../config/globalDirectory.js'; import { logger } from '../../shared/logger.js'; diff --git a/src/cli/cliPrint.ts b/src/cli/cliPrint.ts index bd38e2c..68f7bed 100644 --- a/src/cli/cliPrint.ts +++ b/src/cli/cliPrint.ts @@ -1,6 +1,6 @@ import path from 'node:path'; import pc from 'picocolors'; -import type { RepomixConfigMerged } from '../config/configTypes.js'; +import type { RepomixConfigMerged } from '../config/configSchema.js'; import type { SuspiciousFileResult } from '../core/security/securityCheck.js'; import { logger } from '../shared/logger.js'; diff --git a/src/cli/cliRun.ts b/src/cli/cliRun.ts index 3a363e2..c55e9e0 100644 --- a/src/cli/cliRun.ts +++ b/src/cli/cliRun.ts @@ -1,7 +1,7 @@ import process from 'node:process'; import { type OptionValues, program } from 'commander'; import pc from 'picocolors'; -import type { RepomixOutputStyle } from '../config/configTypes.js'; +import type { RepomixOutputStyle } from '../config/configSchema.js'; import { getVersion } from '../core/file/packageJsonParse.js'; import { handleError } from '../shared/errorHandle.js'; import { logger } from '../shared/logger.js'; diff --git a/src/config/configLoad.ts b/src/config/configLoad.ts index 7903e4e..00a4dba 100644 --- a/src/config/configLoad.ts +++ b/src/config/configLoad.ts @@ -1,9 +1,14 @@ import * as fs from 'node:fs/promises'; import path from 'node:path'; -import { RepomixError } from '../shared/errorHandle.js'; +import { RepomixError, rethrowValidationErrorIfZodError } from '../shared/errorHandle.js'; import { logger } from '../shared/logger.js'; -import type { RepomixConfigCli, RepomixConfigFile, RepomixConfigMerged } from './configTypes.js'; -import { RepomixConfigValidationError, validateConfig } from './configValidate.js'; +import { + type RepomixConfigCli, + type RepomixConfigFile, + type RepomixConfigMerged, + repomixConfigFileSchema, + repomixConfigMergedSchema, +} from './configSchema.js'; import { defaultConfig, defaultFilePathMap } from './defaultConfig.js'; import { getGlobalDirectory } from './globalDirectory.js'; @@ -25,7 +30,6 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu logger.trace('Loading local config from:', fullPath); - // Check local file existence const isLocalFileExists = await fs .stat(fullPath) .then((stats) => stats.isFile()) @@ -36,7 +40,6 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu } if (useDefaultConfig) { - // Try to load global config const globalConfigPath = getGlobalConfigPath(); logger.trace('Loading global config from:', globalConfigPath); @@ -61,12 +64,9 @@ const loadAndValidateConfig = async (filePath: string): Promise; + +export type RepomixConfigDefault = z.infer; + +export type RepomixConfigFile = z.infer; + +export type RepomixConfigCli = z.infer; + +export type RepomixConfigMerged = z.infer; diff --git a/src/config/configTypes.ts b/src/config/configTypes.ts deleted file mode 100644 index 99ef70e..0000000 --- a/src/config/configTypes.ts +++ /dev/null @@ -1,57 +0,0 @@ -export type RepomixOutputStyle = 'plain' | 'xml' | 'markdown'; - -interface RepomixConfigBase { - output?: { - filePath?: string; - style?: RepomixOutputStyle; - headerText?: string; - instructionFilePath?: string; - removeComments?: boolean; - removeEmptyLines?: boolean; - topFilesLength?: number; - showLineNumbers?: boolean; - copyToClipboard?: boolean; - }; - include?: string[]; - ignore?: { - useGitignore?: boolean; - useDefaultPatterns?: boolean; - customPatterns?: string[]; - }; - security?: { - enableSecurityCheck?: boolean; - }; -} - -export type RepomixConfigDefault = RepomixConfigBase & { - output: { - filePath: string; - style: RepomixOutputStyle; - headerText?: string; - instructionFilePath?: string; - removeComments: boolean; - removeEmptyLines: boolean; - topFilesLength: number; - showLineNumbers: boolean; - copyToClipboard: boolean; - }; - include: string[]; - ignore: { - useGitignore: boolean; - useDefaultPatterns: boolean; - customPatterns?: string[]; - }; - security: { - enableSecurityCheck: boolean; - }; -}; - -export type RepomixConfigFile = RepomixConfigBase; - -export type RepomixConfigCli = RepomixConfigBase; - -export type RepomixConfigMerged = RepomixConfigDefault & - RepomixConfigFile & - RepomixConfigCli & { - cwd: string; - }; diff --git a/src/config/configValidate.ts b/src/config/configValidate.ts deleted file mode 100644 index 0afc7f7..0000000 --- a/src/config/configValidate.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { RepomixError } from '../shared/errorHandle.js'; -import type { RepomixConfigFile } from './configTypes.js'; - -export class RepomixConfigValidationError extends RepomixError { - constructor(message: string) { - super(message); - this.name = 'RepomixConfigValidationError'; - } -} - -export function validateConfig(config: unknown): asserts config is RepomixConfigFile { - if (typeof config !== 'object' || config == null) { - throw new RepomixConfigValidationError('Configuration must be an object'); - } - - const { output, ignore, security } = config as Partial; - - // Validate output - if (output !== undefined) { - if (typeof output !== 'object' || output == null) { - throw new RepomixConfigValidationError('output must be an object'); - } - - const { filePath, headerText, style } = output; - if (filePath !== undefined && typeof filePath !== 'string') { - throw new RepomixConfigValidationError('output.filePath must be a string'); - } - if (headerText !== undefined && typeof headerText !== 'string') { - throw new RepomixConfigValidationError('output.headerText must be a string'); - } - if (style !== undefined) { - if (typeof style !== 'string') { - throw new RepomixConfigValidationError('output.style must be a string'); - } - if (style !== 'plain' && style !== 'xml' && style !== 'markdown') { - throw new RepomixConfigValidationError('output.style must be either "plain", "xml" or "markdown"'); - } - } - } - - // Validate ignore - if (ignore !== undefined) { - if (typeof ignore !== 'object' || ignore == null) { - throw new RepomixConfigValidationError('ignore must be an object'); - } - - const { useDefaultPatterns, customPatterns } = ignore; - if (useDefaultPatterns !== undefined && typeof useDefaultPatterns !== 'boolean') { - throw new RepomixConfigValidationError('ignore.useDefaultPatterns must be a boolean'); - } - if (customPatterns !== undefined) { - if (!Array.isArray(customPatterns)) { - throw new RepomixConfigValidationError('ignore.customPatterns must be an array'); - } - if (!customPatterns.every((pattern) => typeof pattern === 'string')) { - throw new RepomixConfigValidationError('All items in ignore.customPatterns must be strings'); - } - } - } - - // Validate security - if (security !== undefined) { - if (typeof security !== 'object' || security == null) { - throw new RepomixConfigValidationError('security must be an object'); - } - - const { enableSecurityCheck } = security; - if (enableSecurityCheck !== undefined && typeof enableSecurityCheck !== 'boolean') { - throw new RepomixConfigValidationError('security.enableSecurityCheck must be a boolean'); - } - } -} diff --git a/src/config/defaultConfig.ts b/src/config/defaultConfig.ts index 1c83999..65857a6 100644 --- a/src/config/defaultConfig.ts +++ b/src/config/defaultConfig.ts @@ -1,4 +1,4 @@ -import type { RepomixConfigDefault, RepomixOutputStyle } from './configTypes.js'; +import type { RepomixConfigDefault, RepomixOutputStyle } from './configSchema.js'; export const defaultFilePathMap: Record = { plain: 'repomix-output.txt', diff --git a/src/core/file/fileProcess.ts b/src/core/file/fileProcess.ts index fe87ee1..d6a6992 100644 --- a/src/core/file/fileProcess.ts +++ b/src/core/file/fileProcess.ts @@ -1,5 +1,5 @@ import pMap from 'p-map'; -import type { RepomixConfigMerged } from '../../config/configTypes.js'; +import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { getProcessConcurrency } from '../../shared/processConcurrency.js'; import { getFileManipulator } from './fileManipulate.js'; import type { ProcessedFile, RawFile } from './fileTypes.js'; diff --git a/src/core/file/fileSearch.ts b/src/core/file/fileSearch.ts index 60deb32..6ae8a25 100644 --- a/src/core/file/fileSearch.ts +++ b/src/core/file/fileSearch.ts @@ -1,5 +1,5 @@ import { globby } from 'globby'; -import type { RepomixConfigMerged } from '../../config/configTypes.js'; +import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { defaultIgnoreList } from '../../config/defaultIgnore.js'; import { logger } from '../../shared/logger.js'; import { sortPaths } from './filePathSort.js'; diff --git a/src/core/output/outputGenerate.ts b/src/core/output/outputGenerate.ts index c0b16e8..9064024 100644 --- a/src/core/output/outputGenerate.ts +++ b/src/core/output/outputGenerate.ts @@ -1,7 +1,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import Handlebars from 'handlebars'; -import type { RepomixConfigMerged } from '../../config/configTypes.js'; +import type { RepomixConfigMerged } from '../../config/configSchema.js'; import { RepomixError } from '../../shared/errorHandle.js'; import { generateTreeString } from '../file/fileTreeGenerate.js'; import type { ProcessedFile } from '../file/fileTypes.js'; diff --git a/src/core/output/outputGeneratorTypes.ts b/src/core/output/outputGeneratorTypes.ts index dac9f15..5ff261c 100644 --- a/src/core/output/outputGeneratorTypes.ts +++ b/src/core/output/outputGeneratorTypes.ts @@ -1,4 +1,4 @@ -import type { RepomixConfigMerged } from '../../config/configTypes.js'; +import type { RepomixConfigMerged } from '../../config/configSchema.js'; import type { ProcessedFile } from '../file/fileTypes.js'; export interface OutputGeneratorContext { diff --git a/src/core/output/outputStyleDecorate.ts b/src/core/output/outputStyleDecorate.ts index bdbbc55..5d05638 100644 --- a/src/core/output/outputStyleDecorate.ts +++ b/src/core/output/outputStyleDecorate.ts @@ -1,4 +1,4 @@ -import type { RepomixConfigMerged } from '../../config/configTypes.js'; +import type { RepomixConfigMerged } from '../../config/configSchema.js'; export const generateHeader = (generationDate: string): string => { return ` diff --git a/src/core/packager.ts b/src/core/packager.ts index eecc236..95ae2e1 100644 --- a/src/core/packager.ts +++ b/src/core/packager.ts @@ -4,7 +4,7 @@ import { setTimeout } from 'node:timers/promises'; import clipboard from 'clipboardy'; import pMap from 'p-map'; import pc from 'picocolors'; -import type { RepomixConfigMerged } from '../config/configTypes.js'; +import type { RepomixConfigMerged } from '../config/configSchema.js'; import { logger } from '../shared/logger.js'; import { getProcessConcurrency } from '../shared/processConcurrency.js'; import type { RepomixProgressCallback } from '../shared/types.js'; diff --git a/src/index.ts b/src/index.ts index 5940380..926d27e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,3 @@ export { pack } from './core/packager.js'; -export type { RepomixConfigFile as RepomixConfig } from './config/configTypes.js'; +export type { RepomixConfigFile as RepomixConfig } from './config/configSchema.js'; export { run as cli } from './cli/cliRun.js'; diff --git a/src/shared/errorHandle.ts b/src/shared/errorHandle.ts index 940a156..f4ff42e 100644 --- a/src/shared/errorHandle.ts +++ b/src/shared/errorHandle.ts @@ -1,3 +1,4 @@ +import { z } from 'zod'; import { logger } from './logger.js'; export class RepomixError extends Error { @@ -7,6 +8,13 @@ export class RepomixError extends Error { } } +export class RepomixConfigValidationError extends RepomixError { + constructor(message: string) { + super(message); + this.name = 'RepomixConfigValidationError'; + } +} + export const handleError = (error: unknown): void => { if (error instanceof RepomixError) { logger.error(`Error: ${error.message}`); @@ -19,3 +27,12 @@ export const handleError = (error: unknown): void => { logger.info('For more help, please visit: https://github.com/yamadashy/repomix/issues'); }; + +export const rethrowValidationErrorIfZodError = (error: unknown, message: string): void => { + if (error instanceof z.ZodError) { + const zodErrorText = error.errors.map((err) => `${err.path.join('.')}: ${err.message}`).join(', '); + throw new RepomixConfigValidationError( + `${message}\n ${zodErrorText}\n Please check the config file and try again.`, + ); + } +}; diff --git a/tests/config/configLoad.test.ts b/tests/config/configLoad.test.ts index ec41a84..20a028d 100644 --- a/tests/config/configLoad.test.ts +++ b/tests/config/configLoad.test.ts @@ -4,8 +4,9 @@ import path from 'node:path'; import process from 'node:process'; import { beforeEach, describe, expect, test, vi } from 'vitest'; import { loadFileConfig, mergeConfigs } from '../../src/config/configLoad.js'; -import type { RepomixConfigCli, RepomixConfigFile } from '../../src/config/configTypes.js'; +import type { RepomixConfigCli, RepomixConfigFile } from '../../src/config/configSchema.js'; import { getGlobalDirectory } from '../../src/config/globalDirectory.js'; +import { RepomixConfigValidationError } from '../../src/shared/errorHandle.js'; import { logger } from '../../src/shared/logger.js'; vi.mock('node:fs/promises'); @@ -38,6 +39,17 @@ describe('configLoad', () => { expect(result).toEqual(mockConfig); }); + test('should throw RepomixConfigValidationError for invalid config', async () => { + const invalidConfig = { + output: { filePath: 123, style: 'invalid' }, // Invalid filePath type and invalid style + ignore: { useDefaultPatterns: 'not a boolean' }, // Invalid type + }; + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(invalidConfig)); + vi.mocked(fs.stat).mockResolvedValue({ isFile: () => true } as Stats); + + await expect(loadFileConfig(process.cwd(), 'test-config.json')).rejects.toThrow(RepomixConfigValidationError); + }); + test('should load global config when local config is not found', async () => { const mockGlobalConfig = { output: { filePath: 'global-output.txt' }, @@ -91,5 +103,17 @@ describe('configLoad', () => { expect(result.ignore.customPatterns).toContain('file-ignore'); expect(result.ignore.customPatterns).toContain('cli-ignore'); }); + + test('should throw RepomixConfigValidationError for invalid merged config', () => { + const fileConfig: RepomixConfigFile = { + output: { filePath: 'file-output.txt', style: 'plain' }, + }; + const cliConfig: RepomixConfigCli = { + // @ts-ignore + output: { style: 'invalid' }, // Invalid style + }; + + expect(() => mergeConfigs(process.cwd(), fileConfig, cliConfig)).toThrow(RepomixConfigValidationError); + }); }); }); diff --git a/tests/config/configSchema.test.ts b/tests/config/configSchema.test.ts new file mode 100644 index 0000000..48acc00 --- /dev/null +++ b/tests/config/configSchema.test.ts @@ -0,0 +1,198 @@ +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { + repomixConfigBaseSchema, + repomixConfigCliSchema, + repomixConfigDefaultSchema, + repomixConfigFileSchema, + repomixConfigMergedSchema, + repomixOutputStyleSchema, +} from '../../src/config/configSchema.js'; + +describe('configSchema', () => { + describe('repomixOutputStyleSchema', () => { + it('should accept valid output styles', () => { + expect(repomixOutputStyleSchema.parse('plain')).toBe('plain'); + expect(repomixOutputStyleSchema.parse('xml')).toBe('xml'); + }); + + it('should reject invalid output styles', () => { + expect(() => repomixOutputStyleSchema.parse('invalid')).toThrow(z.ZodError); + }); + }); + + describe('repomixConfigBaseSchema', () => { + it('should accept valid base config', () => { + const validConfig = { + output: { + filePath: 'output.txt', + style: 'plain', + removeComments: true, + }, + include: ['**/*.js'], + ignore: { + useGitignore: true, + customPatterns: ['node_modules'], + }, + security: { + enableSecurityCheck: true, + }, + }; + expect(repomixConfigBaseSchema.parse(validConfig)).toEqual(validConfig); + }); + + it('should accept empty object', () => { + expect(repomixConfigBaseSchema.parse({})).toEqual({}); + }); + + it('should reject invalid types', () => { + const invalidConfig = { + output: { + filePath: 123, // Should be string + style: 'invalid', // Should be 'plain' or 'xml' + }, + include: 'not-an-array', // Should be an array + }; + expect(() => repomixConfigBaseSchema.parse(invalidConfig)).toThrow(z.ZodError); + }); + }); + + describe('repomixConfigDefaultSchema', () => { + it('should accept valid default config', () => { + const validConfig = { + output: { + filePath: 'output.txt', + style: 'plain', + removeComments: false, + removeEmptyLines: false, + topFilesLength: 5, + showLineNumbers: false, + copyToClipboard: true, + }, + include: [], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + }, + security: { + enableSecurityCheck: true, + }, + }; + expect(repomixConfigDefaultSchema.parse(validConfig)).toEqual(validConfig); + }); + + it('should reject incomplete config', () => { + const incompleteConfig = { + output: { + filePath: 'output.txt', + // Missing required fields + }, + }; + expect(() => repomixConfigDefaultSchema.parse(incompleteConfig)).toThrow(z.ZodError); + }); + }); + + describe('repomixConfigFileSchema', () => { + it('should accept valid file config', () => { + const validConfig = { + output: { + filePath: 'custom-output.txt', + style: 'xml', + }, + ignore: { + customPatterns: ['*.log'], + }, + }; + expect(repomixConfigFileSchema.parse(validConfig)).toEqual(validConfig); + }); + + it('should accept partial config', () => { + const partialConfig = { + output: { + filePath: 'partial-output.txt', + }, + }; + expect(repomixConfigFileSchema.parse(partialConfig)).toEqual(partialConfig); + }); + }); + + describe('repomixConfigCliSchema', () => { + it('should accept valid CLI config', () => { + const validConfig = { + output: { + filePath: 'cli-output.txt', + showLineNumbers: true, + }, + include: ['src/**/*.ts'], + }; + expect(repomixConfigCliSchema.parse(validConfig)).toEqual(validConfig); + }); + + it('should reject invalid CLI options', () => { + const invalidConfig = { + output: { + filePath: 123, // Should be string + }, + }; + expect(() => repomixConfigCliSchema.parse(invalidConfig)).toThrow(z.ZodError); + }); + }); + + describe('repomixConfigMergedSchema', () => { + it('should accept valid merged config', () => { + const validConfig = { + cwd: '/path/to/project', + output: { + filePath: 'merged-output.txt', + style: 'plain', + removeComments: true, + removeEmptyLines: false, + topFilesLength: 10, + showLineNumbers: true, + copyToClipboard: false, + }, + include: ['**/*.js', '**/*.ts'], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + customPatterns: ['*.log'], + }, + security: { + enableSecurityCheck: true, + }, + }; + expect(repomixConfigMergedSchema.parse(validConfig)).toEqual(validConfig); + }); + + it('should reject merged config missing required fields', () => { + const invalidConfig = { + cwd: '/path/to/project', + // Missing required output field + }; + expect(() => repomixConfigMergedSchema.parse(invalidConfig)).toThrow(z.ZodError); + }); + + it('should reject merged config with invalid types', () => { + const invalidConfig = { + cwd: '/path/to/project', + output: { + filePath: 'output.txt', + style: 'plain', + removeComments: 'not-a-boolean', // Should be boolean + removeEmptyLines: false, + topFilesLength: '5', // Should be number + showLineNumbers: false, + }, + include: ['**/*.js'], + ignore: { + useGitignore: true, + useDefaultPatterns: true, + }, + security: { + enableSecurityCheck: true, + }, + }; + expect(() => repomixConfigMergedSchema.parse(invalidConfig)).toThrow(z.ZodError); + }); + }); +}); diff --git a/tests/config/configValidate.test.ts b/tests/config/configValidate.test.ts deleted file mode 100644 index 67f7583..0000000 --- a/tests/config/configValidate.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { describe, expect, test } from 'vitest'; -import { RepomixConfigValidationError, validateConfig } from '../../src/config/configValidate.js'; - -describe('configValidate', () => { - test('should pass for a valid config', () => { - const validConfig = { - output: { filePath: 'test.txt', headerText: 'Test Header' }, - ignore: { useDefaultPatterns: true, customPatterns: ['*.log'] }, - }; - expect(() => validateConfig(validConfig)).not.toThrow(); - }); - - test('should throw for non-object config', () => { - expect(() => validateConfig('not an object')).toThrow(RepomixConfigValidationError); - }); - - test('should throw for invalid output.filePath', () => { - const invalidConfig = { output: { filePath: 123 } }; - expect(() => validateConfig(invalidConfig)).toThrow(RepomixConfigValidationError); - }); - - test('should throw for invalid ignore.useDefaultPatterns', () => { - const invalidConfig = { ignore: { useDefaultPatterns: 'true' } }; - expect(() => validateConfig(invalidConfig)).toThrow(RepomixConfigValidationError); - }); - - test('should throw for invalid ignore.customPatterns', () => { - const invalidConfig = { ignore: { customPatterns: 'not an array' } }; - expect(() => validateConfig(invalidConfig)).toThrow(RepomixConfigValidationError); - }); - - test('should pass for a valid config with output style', () => { - const validConfig = { - output: { filePath: 'test.txt', style: 'xml' }, - ignore: { useDefaultPatterns: true }, - }; - expect(() => validateConfig(validConfig)).not.toThrow(); - }); - - test('should throw for invalid output.style type', () => { - const invalidConfig = { output: { style: 123 } }; - expect(() => validateConfig(invalidConfig)).toThrow(RepomixConfigValidationError); - expect(() => validateConfig(invalidConfig)).toThrow('output.style must be a string'); - }); - - test('should throw for invalid output.style value', () => { - const invalidConfig = { output: { style: 'invalid' } }; - expect(() => validateConfig(invalidConfig)).toThrow(RepomixConfigValidationError); - expect(() => validateConfig(invalidConfig)).toThrow('output.style must be either "plain", "xml" or "markdown"'); - }); -}); diff --git a/tests/integration-tests/packager.test.ts b/tests/integration-tests/packager.test.ts index 4730ffb..e361f14 100644 --- a/tests/integration-tests/packager.test.ts +++ b/tests/integration-tests/packager.test.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import process from 'node:process'; import { afterEach, beforeEach, describe, expect, test } from 'vitest'; import { loadFileConfig, mergeConfigs } from '../../src/config/configLoad.js'; -import type { RepomixConfigFile, RepomixConfigMerged, RepomixOutputStyle } from '../../src/config/configTypes.js'; +import type { RepomixConfigFile, RepomixConfigMerged, RepomixOutputStyle } from '../../src/config/configSchema.js'; import { pack } from '../../src/core/packager.js'; import { isWindows } from '../testing/testUtils.js'; diff --git a/tests/testing/testUtils.ts b/tests/testing/testUtils.ts index ffdb62a..9652966 100644 --- a/tests/testing/testUtils.ts +++ b/tests/testing/testUtils.ts @@ -1,6 +1,6 @@ import os from 'node:os'; import process from 'node:process'; -import type { RepomixConfigMerged } from '../../src/config/configTypes.js'; +import type { RepomixConfigMerged } from '../../src/config/configSchema.js'; import { defaultConfig } from '../../src/config/defaultConfig.js'; type DeepPartial = { From 5d05730f0f9ab0e97df6259027c8a2a9fa3dd970 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Mon, 11 Nov 2024 00:06:55 +0900 Subject: [PATCH 2/2] feat(config): Migrate default config management to Zod --- src/cli/actions/defaultAction.ts | 19 ++++++--- src/cli/actions/initAction.ts | 8 +++- src/config/configLoad.ts | 26 ++++++++---- src/config/configSchema.ts | 67 ++++++++++++++++++------------- src/config/defaultConfig.ts | 28 ------------- src/shared/errorHandle.ts | 4 +- tests/config/configSchema.test.ts | 19 +++++---- tests/testing/testUtils.ts | 3 +- 8 files changed, 88 insertions(+), 86 deletions(-) delete mode 100644 src/config/defaultConfig.ts diff --git a/src/cli/actions/defaultAction.ts b/src/cli/actions/defaultAction.ts index 29f83ad..0ed8afc 100644 --- a/src/cli/actions/defaultAction.ts +++ b/src/cli/actions/defaultAction.ts @@ -1,12 +1,14 @@ import path from 'node:path'; import { loadFileConfig, mergeConfigs } from '../../config/configLoad.js'; -import type { - RepomixConfigCli, - RepomixConfigFile, - RepomixConfigMerged, - RepomixOutputStyle, +import { + type RepomixConfigCli, + type RepomixConfigFile, + type RepomixConfigMerged, + type RepomixOutputStyle, + repomixConfigCliSchema, } from '../../config/configSchema.js'; import { type PackResult, pack } from '../../core/packager.js'; +import { rethrowValidationErrorIfZodError } from '../../shared/errorHandle.js'; import { logger } from '../../shared/logger.js'; import { printCompletion, printSecurityCheck, printSummary, printTopFiles } from '../cliPrint.js'; import type { CliOptions } from '../cliRun.js'; @@ -111,5 +113,10 @@ const buildCliConfig = (options: CliOptions): RepomixConfigCli => { cliConfig.output = { ...cliConfig.output, style: options.style.toLowerCase() as RepomixOutputStyle }; } - return cliConfig; + try { + return repomixConfigCliSchema.parse(cliConfig); + } catch (error) { + rethrowValidationErrorIfZodError(error, 'Invalid cli arguments'); + throw error; + } }; diff --git a/src/cli/actions/initAction.ts b/src/cli/actions/initAction.ts index 5b6b4c4..04a181f 100644 --- a/src/cli/actions/initAction.ts +++ b/src/cli/actions/initAction.ts @@ -2,8 +2,12 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import * as prompts from '@clack/prompts'; import pc from 'picocolors'; -import type { RepomixConfigFile, RepomixOutputStyle } from '../../config/configSchema.js'; -import { defaultConfig, defaultFilePathMap } from '../../config/defaultConfig.js'; +import { + type RepomixConfigFile, + type RepomixOutputStyle, + defaultConfig, + defaultFilePathMap, +} from '../../config/configSchema.js'; import { getGlobalDirectory } from '../../config/globalDirectory.js'; import { logger } from '../../shared/logger.js'; diff --git a/src/config/configLoad.ts b/src/config/configLoad.ts index 00a4dba..b2cbd8f 100644 --- a/src/config/configLoad.ts +++ b/src/config/configLoad.ts @@ -1,15 +1,17 @@ import * as fs from 'node:fs/promises'; import path from 'node:path'; +import { z } from 'zod'; import { RepomixError, rethrowValidationErrorIfZodError } from '../shared/errorHandle.js'; import { logger } from '../shared/logger.js'; import { type RepomixConfigCli, type RepomixConfigFile, type RepomixConfigMerged, + defaultConfig, + defaultFilePathMap, repomixConfigFileSchema, repomixConfigMergedSchema, } from './configSchema.js'; -import { defaultConfig, defaultFilePathMap } from './defaultConfig.js'; import { getGlobalDirectory } from './globalDirectory.js'; const defaultConfigPath = 'repomix.config.json'; @@ -30,6 +32,7 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu logger.trace('Loading local config from:', fullPath); + // Check local file existence const isLocalFileExists = await fs .stat(fullPath) .then((stats) => stats.isFile()) @@ -40,6 +43,7 @@ export const loadFileConfig = async (rootDir: string, argConfigPath: string | nu } if (useDefaultConfig) { + // Try to load global config const globalConfigPath = getGlobalConfigPath(); logger.trace('Loading global config from:', globalConfigPath); @@ -82,32 +86,38 @@ export const mergeConfigs = ( fileConfig: RepomixConfigFile, cliConfig: RepomixConfigCli, ): RepomixConfigMerged => { + logger.trace('Default config:', defaultConfig); + + const baseConfig = defaultConfig; + // If the output file path is not provided in the config file or CLI, use the default file path for the style if (cliConfig.output?.filePath == null && fileConfig.output?.filePath == null) { - const style = cliConfig.output?.style || fileConfig.output?.style || defaultConfig.output.style; - defaultConfig.output.filePath = defaultFilePathMap[style]; + const style = cliConfig.output?.style || fileConfig.output?.style || baseConfig.output.style; + baseConfig.output.filePath = defaultFilePathMap[style]; + + logger.trace('Default output file path is set to:', baseConfig.output.filePath); } const mergedConfig = { cwd, output: { - ...defaultConfig.output, + ...baseConfig.output, ...fileConfig.output, ...cliConfig.output, }, + include: [...(baseConfig.include || []), ...(fileConfig.include || []), ...(cliConfig.include || [])], ignore: { - ...defaultConfig.ignore, + ...baseConfig.ignore, ...fileConfig.ignore, ...cliConfig.ignore, customPatterns: [ - ...(defaultConfig.ignore.customPatterns || []), + ...(baseConfig.ignore.customPatterns || []), ...(fileConfig.ignore?.customPatterns || []), ...(cliConfig.ignore?.customPatterns || []), ], }, - include: [...(defaultConfig.include || []), ...(fileConfig.include || []), ...(cliConfig.include || [])], security: { - ...defaultConfig.security, + ...baseConfig.security, ...fileConfig.security, ...cliConfig.security, }, diff --git a/src/config/configSchema.ts b/src/config/configSchema.ts index cb0d349..428b10f 100644 --- a/src/config/configSchema.ts +++ b/src/config/configSchema.ts @@ -1,8 +1,17 @@ import { z } from 'zod'; -import { RepomixError } from '../shared/errorHandle.js'; +// Output style enum export const repomixOutputStyleSchema = z.enum(['plain', 'xml', 'markdown']); +export type RepomixOutputStyle = z.infer; + +// Default values map +export const defaultFilePathMap: Record = { + plain: 'repomix-output.txt', + markdown: 'repomix-output.md', + xml: 'repomix-output.xml', +} as const; +// Base config schema export const repomixConfigBaseSchema = z.object({ output: z .object({ @@ -32,30 +41,35 @@ export const repomixConfigBaseSchema = z.object({ .optional(), }); -export const repomixConfigDefaultSchema = repomixConfigBaseSchema.and( - z.object({ - output: z.object({ - filePath: z.string(), - style: repomixOutputStyleSchema, +// Default config schema with default values +export const repomixConfigDefaultSchema = z.object({ + output: z + .object({ + filePath: z.string().default(defaultFilePathMap.plain), + style: repomixOutputStyleSchema.default('plain'), headerText: z.string().optional(), instructionFilePath: z.string().optional(), - removeComments: z.boolean(), - removeEmptyLines: z.boolean(), - topFilesLength: z.number(), - showLineNumbers: z.boolean(), - copyToClipboard: z.boolean(), - }), - include: z.array(z.string()), - ignore: z.object({ - useGitignore: z.boolean(), - useDefaultPatterns: z.boolean(), - customPatterns: z.array(z.string()).optional(), - }), - security: z.object({ - enableSecurityCheck: z.boolean(), - }), - }), -); + removeComments: z.boolean().default(false), + removeEmptyLines: z.boolean().default(false), + topFilesLength: z.number().int().min(0).default(5), + showLineNumbers: z.boolean().default(false), + copyToClipboard: z.boolean().default(false), + }) + .default({}), + include: z.array(z.string()).default([]), + ignore: z + .object({ + useGitignore: z.boolean().default(true), + useDefaultPatterns: z.boolean().default(true), + customPatterns: z.array(z.string()).default([]), + }) + .default({}), + security: z + .object({ + enableSecurityCheck: z.boolean().default(true), + }) + .default({}), +}); export const repomixConfigFileSchema = repomixConfigBaseSchema; @@ -70,12 +84,9 @@ export const repomixConfigMergedSchema = repomixConfigDefaultSchema }), ); -export type RepomixOutputStyle = z.infer; - export type RepomixConfigDefault = z.infer; - export type RepomixConfigFile = z.infer; - export type RepomixConfigCli = z.infer; - export type RepomixConfigMerged = z.infer; + +export const defaultConfig = repomixConfigDefaultSchema.parse({}); diff --git a/src/config/defaultConfig.ts b/src/config/defaultConfig.ts deleted file mode 100644 index 65857a6..0000000 --- a/src/config/defaultConfig.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { RepomixConfigDefault, RepomixOutputStyle } from './configSchema.js'; - -export const defaultFilePathMap: Record = { - plain: 'repomix-output.txt', - markdown: 'repomix-output.md', - xml: 'repomix-output.xml', -}; - -export const defaultConfig: RepomixConfigDefault = { - output: { - filePath: defaultFilePathMap.plain, - style: 'plain', - removeComments: false, - removeEmptyLines: false, - topFilesLength: 5, - showLineNumbers: false, - copyToClipboard: false, - }, - include: [], - ignore: { - useGitignore: true, - useDefaultPatterns: true, - customPatterns: [], - }, - security: { - enableSecurityCheck: true, - }, -}; diff --git a/src/shared/errorHandle.ts b/src/shared/errorHandle.ts index f4ff42e..f3a2e6f 100644 --- a/src/shared/errorHandle.ts +++ b/src/shared/errorHandle.ts @@ -30,9 +30,9 @@ export const handleError = (error: unknown): void => { export const rethrowValidationErrorIfZodError = (error: unknown, message: string): void => { if (error instanceof z.ZodError) { - const zodErrorText = error.errors.map((err) => `${err.path.join('.')}: ${err.message}`).join(', '); + const zodErrorText = error.errors.map((err) => `[${err.path.join('.')}] ${err.message}`).join('\n '); throw new RepomixConfigValidationError( - `${message}\n ${zodErrorText}\n Please check the config file and try again.`, + `${message}\n\n ${zodErrorText}\n\n Please check the config file and try again.`, ); } }; diff --git a/tests/config/configSchema.test.ts b/tests/config/configSchema.test.ts index 48acc00..94c7000 100644 --- a/tests/config/configSchema.test.ts +++ b/tests/config/configSchema.test.ts @@ -1,5 +1,6 @@ +import { outro } from '@clack/prompts'; import { describe, expect, it } from 'vitest'; -import { z } from 'zod'; +import { custom, z } from 'zod'; import { repomixConfigBaseSchema, repomixConfigCliSchema, @@ -71,6 +72,7 @@ describe('configSchema', () => { }, include: [], ignore: { + customPatterns: [], useGitignore: true, useDefaultPatterns: true, }, @@ -82,13 +84,8 @@ describe('configSchema', () => { }); it('should reject incomplete config', () => { - const incompleteConfig = { - output: { - filePath: 'output.txt', - // Missing required fields - }, - }; - expect(() => repomixConfigDefaultSchema.parse(incompleteConfig)).toThrow(z.ZodError); + const validConfig = {}; + expect(() => repomixConfigDefaultSchema.parse(validConfig)).not.toThrow(); }); }); @@ -166,8 +163,10 @@ describe('configSchema', () => { it('should reject merged config missing required fields', () => { const invalidConfig = { - cwd: '/path/to/project', - // Missing required output field + output: { + filePath: 'output.txt', + // Missing required fields + }, }; expect(() => repomixConfigMergedSchema.parse(invalidConfig)).toThrow(z.ZodError); }); diff --git a/tests/testing/testUtils.ts b/tests/testing/testUtils.ts index 9652966..5e1fef2 100644 --- a/tests/testing/testUtils.ts +++ b/tests/testing/testUtils.ts @@ -1,7 +1,6 @@ import os from 'node:os'; import process from 'node:process'; -import type { RepomixConfigMerged } from '../../src/config/configSchema.js'; -import { defaultConfig } from '../../src/config/defaultConfig.js'; +import { type RepomixConfigMerged, defaultConfig } from '../../src/config/configSchema.js'; type DeepPartial = { [P in keyof T]?: T[P] extends (infer U)[]