From 59e3102b9d44b19bf7f2cb325951030e66be45af Mon Sep 17 00:00:00 2001 From: Chrilleweb Date: Tue, 27 Oct 2026 11:18:41 +0100 Subject: [PATCH 1/5] ref: refactor processComparison file --- packages/cli/src/core/detectUppercaseKeys.ts | 17 +++++ .../cli/src/core/scan/excludeOptionalKeys.ts | 30 ++++++++ .../cli/src/services/processComparisonFile.ts | 69 +++++-------------- .../cli/test/e2e/cli.optionalKeys.e2e.test.ts | 11 +++ .../unit/core/detectUppercaseKeys.test.ts | 19 +++++ .../core/scan/excludeOptionalKeys.test.ts | 65 +++++++++++++++++ 6 files changed, 158 insertions(+), 53 deletions(-) create mode 100644 packages/cli/src/core/detectUppercaseKeys.ts create mode 100644 packages/cli/src/core/scan/excludeOptionalKeys.ts create mode 100644 packages/cli/test/unit/core/detectUppercaseKeys.test.ts create mode 100644 packages/cli/test/unit/core/scan/excludeOptionalKeys.test.ts diff --git a/packages/cli/src/core/detectUppercaseKeys.ts b/packages/cli/src/core/detectUppercaseKeys.ts new file mode 100644 index 00000000..d75b24de --- /dev/null +++ b/packages/cli/src/core/detectUppercaseKeys.ts @@ -0,0 +1,17 @@ +import type { UppercaseWarning } from '../config/types.js'; +import { toUpperSnakeCase } from './helpers/toUpperSnakeCase.js'; + +/** A key in UPPER_SNAKE_CASE: uppercase letters, digits and underscores. */ +const UPPER_SNAKE_CASE = /^[A-Z0-9_]+$/; + +/** + * Detects environment variable keys that are not in UPPER_SNAKE_CASE. + * For example: `apiKey` → suggests `API_KEY`. + * @param keys - Array of environment variable keys to analyze + * @returns Array of uppercase warnings, each with a suggested key + */ +export function detectUppercaseKeys(keys: string[]): UppercaseWarning[] { + return keys + .filter((key) => !UPPER_SNAKE_CASE.test(key)) + .map((key) => ({ key, suggestion: toUpperSnakeCase(key) })); +} diff --git a/packages/cli/src/core/scan/excludeOptionalKeys.ts b/packages/cli/src/core/scan/excludeOptionalKeys.ts new file mode 100644 index 00000000..7f7e939e --- /dev/null +++ b/packages/cli/src/core/scan/excludeOptionalKeys.ts @@ -0,0 +1,30 @@ +import type { ScanResult } from '../../config/types.js'; + +/** + * Removes keys marked `@optional` from the missing keys of a scan result. + * + * An optional key may be left out of the env file entirely — that is what the + * annotation means — so it must not be reported as missing. The typo + * suggestions derived from those keys go too. + * @param scanResult - The result of the scan. + * @param optionalKeys - Keys marked `@optional` in the example file. + * @returns The scan result without the optional keys. + */ +export function excludeOptionalKeys( + scanResult: ScanResult, + optionalKeys: ReadonlySet, +): ScanResult { + if (optionalKeys.size === 0) return scanResult; + + const isRequired = (key: string) => !optionalKeys.has(key); + + return { + ...scanResult, + missing: scanResult.missing.filter(isRequired), + ...(scanResult.suggestions + ? { + suggestions: scanResult.suggestions.filter((s) => isRequired(s.key)), + } + : {}), + }; +} diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index b2c90439..abf8b1ee 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -3,10 +3,11 @@ import path from 'path'; import { parseEnvFile } from './parseEnvFile.js'; import { filterIgnoredKeys } from '../core/helpers/filterIgnoredKeys.js'; import { compareWithEnvFiles } from '../core/scan/compareScan.js'; +import { excludeOptionalKeys } from '../core/scan/excludeOptionalKeys.js'; import { discoverExampleScopes } from './exampleDiscovery.js'; import { findDuplicateKeys } from './duplicates.js'; import { applyFixes } from './fixEnv.js'; -import { toUpperSnakeCase } from '../core/helpers/toUpperSnakeCase.js'; +import { detectUppercaseKeys } from '../core/detectUppercaseKeys.js'; import { resolveFromCwd } from '../core/helpers/resolveFromCwd.js'; import { detectEnvExpirations } from './detectEnvExpirations.js'; import { detectInconsistentNaming } from '../core/detectInconsistentNaming.js'; @@ -14,7 +15,6 @@ import { detectMissingComments } from './detectMissingComments.js'; import { detectOptionalKeys } from './detectOptionalKeys.js'; import { detectExampleDrift } from './detectExampleDrift.js'; import { resolveExampleFile } from './resolveExampleFile.js'; -import { DEFAULT_EXAMPLE_FILE } from '../config/constants.js'; import type { ScanUsageOptions, ScanResult, @@ -86,6 +86,7 @@ export function processComparisonFile( let inconsistentNamingWarnings: InconsistentNamingWarning[] = []; let commentWarnings: CommentWarning[] = []; let driftWarnings: DriftWarning[] = []; + let error: ProcessComparisonResult['error']; const fix: FixContext = { fixApplied: false, @@ -133,32 +134,15 @@ export function processComparisonFile( ); comparedAgainst = compareFile.name; - // Keys marked `@optional` in the example file may be left out of the env - // file entirely — that is what the annotation means — so they must not be - // reported as missing. The typo suggestions derived from them go too. + // Keys marked `@optional` in the example file are not reported as missing const optionalKeys = new Set( - detectOptionalKeys( - resolveFromCwd(opts.cwd, opts.examplePath ?? DEFAULT_EXAMPLE_FILE), - ), + exampleFilePath ? detectOptionalKeys(exampleFilePath) : [], ); - if (optionalKeys.size > 0) { - scanResult.missing = scanResult.missing.filter( - (key) => !optionalKeys.has(key), - ); - if (scanResult.suggestions) { - scanResult.suggestions = scanResult.suggestions.filter( - (suggestion) => !optionalKeys.has(suggestion.key), - ); - } - } + scanResult = excludeOptionalKeys(scanResult, optionalKeys); // Detect uppercase keys if (opts.uppercaseKeys) { - for (const key of envKeys) { - if (!/^[A-Z0-9_]+$/.test(key)) { - uppercaseWarnings.push({ key, suggestion: toUpperSnakeCase(key) }); - } - } + uppercaseWarnings = detectUppercaseKeys(envKeys); } // Find duplicates @@ -188,16 +172,10 @@ export function processComparisonFile( } // Warn about .env.example keys without a documenting comment. Runs on the - // example file (documentation lives there), not the env file — using the - // explicit --example path when given, else the default `.env.example`. - if (opts.commentWarnings) { - const examplePath = resolveFromCwd( - opts.cwd, - opts.examplePath ?? DEFAULT_EXAMPLE_FILE, - ); - if (fs.existsSync(examplePath)) { - commentWarnings = detectMissingComments(examplePath); - } + // example file (documentation lives there), not the env file — the same + // example the other checks read. + if (opts.commentWarnings && exampleFilePath && exampleFull) { + commentWarnings = detectMissingComments(exampleFilePath); } // Warn when the env file this run is about has drifted from the example @@ -241,26 +219,10 @@ export function processComparisonFile( if (dupsEnv.length > 0) scanResult.duplicates.env = dupsEnv; if (dupsEx.length > 0) scanResult.duplicates.example = dupsEx; } - } catch (error) { - const errorMessage = `Could not read ${compareFile.name}: ${compareFile.path} - ${error}`; - return { - scanResult, - envVariables, - comparedAgainst, - dupsEnv, - dupsEx, - fix, - exampleFull, - exampleFile, - uppercaseWarnings, - expireWarnings, - inconsistentNamingWarnings, - commentWarnings, - driftWarnings, - error: { - message: errorMessage, - shouldExit: opts.isCiMode ?? false, - }, + } catch (err) { + error = { + message: `Could not read ${compareFile.name}: ${compareFile.path} - ${err}`, + shouldExit: opts.isCiMode ?? false, }; } @@ -278,6 +240,7 @@ export function processComparisonFile( inconsistentNamingWarnings, commentWarnings, driftWarnings, + ...(error ? { error } : {}), }; } diff --git a/packages/cli/test/e2e/cli.optionalKeys.e2e.test.ts b/packages/cli/test/e2e/cli.optionalKeys.e2e.test.ts index 542f7ab4..8618f581 100644 --- a/packages/cli/test/e2e/cli.optionalKeys.e2e.test.ts +++ b/packages/cli/test/e2e/cli.optionalKeys.e2e.test.ts @@ -65,6 +65,17 @@ describe('Optional keys (@optional)', () => { expect(res.status).toBe(0); }); + it('reads the annotation from the example file that documents .env', () => { + const cwd = setup(WITH_ANNOTATION); + fs.renameSync( + path.join(cwd, '.env.example'), + path.join(cwd, '.env.sample'), + ); + const res = runCli(cwd, []); + expect(res.stdout).not.toContain('REDIS_URL'); + expect(res.status).toBe(0); + }); + it('still reports other keys as missing', () => { const cwd = setup('# @optional\nREDIS_URL=\n\nDATABASE_URL=\n', '\n'); const res = runCli(cwd, []); diff --git a/packages/cli/test/unit/core/detectUppercaseKeys.test.ts b/packages/cli/test/unit/core/detectUppercaseKeys.test.ts new file mode 100644 index 00000000..cdd0af1f --- /dev/null +++ b/packages/cli/test/unit/core/detectUppercaseKeys.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect } from 'vitest'; +import { detectUppercaseKeys } from '../../../src/core/detectUppercaseKeys.js'; + +describe('detectUppercaseKeys', () => { + it('returns no warnings for UPPER_SNAKE_CASE keys', () => { + expect(detectUppercaseKeys(['API_KEY', 'PORT', 'S3_BUCKET_2'])).toEqual([]); + }); + + it('warns about keys that are not UPPER_SNAKE_CASE, with a suggestion', () => { + expect(detectUppercaseKeys(['apiKey', 'API_KEY', 'db-url'])).toEqual([ + { key: 'apiKey', suggestion: 'API_KEY' }, + { key: 'db-url', suggestion: 'DB_URL' }, + ]); + }); + + it('returns no warnings for an empty list', () => { + expect(detectUppercaseKeys([])).toEqual([]); + }); +}); diff --git a/packages/cli/test/unit/core/scan/excludeOptionalKeys.test.ts b/packages/cli/test/unit/core/scan/excludeOptionalKeys.test.ts new file mode 100644 index 00000000..5676bb2a --- /dev/null +++ b/packages/cli/test/unit/core/scan/excludeOptionalKeys.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect } from 'vitest'; +import { excludeOptionalKeys } from '../../../../src/core/scan/excludeOptionalKeys.js'; +import type { ScanResult } from '../../../../src/config/types.js'; + +/** + * Builds a minimal but fully-typed ScanResult, with optional overrides. + */ +function makeScanResult(overrides: Partial = {}): ScanResult { + return { + used: [], + missing: [], + unused: [], + stats: { + filesScanned: 0, + totalUsages: 0, + uniqueVariables: 0, + warningsCount: 0, + duration: 0, + }, + secrets: [], + duplicates: {}, + logged: [], + ...overrides, + }; +} + +describe('excludeOptionalKeys', () => { + it('removes optional keys from missing', () => { + const scanResult = makeScanResult({ missing: ['DB_URL', 'REDIS_URL'] }); + const result = excludeOptionalKeys(scanResult, new Set(['REDIS_URL'])); + expect(result.missing).toEqual(['DB_URL']); + }); + + it('removes typo suggestions for optional keys', () => { + const scanResult = makeScanResult({ + missing: ['DB_URL', 'REDIS_URL'], + suggestions: [ + { key: 'DB_URL', didYouMean: 'DB_URI', distance: 1 }, + { key: 'REDIS_URL', didYouMean: 'REDIS_URI', distance: 1 }, + ], + }); + const result = excludeOptionalKeys(scanResult, new Set(['REDIS_URL'])); + expect(result.suggestions).toEqual([ + { key: 'DB_URL', didYouMean: 'DB_URI', distance: 1 }, + ]); + }); + + it('leaves suggestions undefined when there were none', () => { + const scanResult = makeScanResult({ missing: ['REDIS_URL'] }); + const result = excludeOptionalKeys(scanResult, new Set(['REDIS_URL'])); + expect(result.missing).toEqual([]); + expect(result).not.toHaveProperty('suggestions'); + }); + + it('returns the scan result unchanged when no keys are optional', () => { + const scanResult = makeScanResult({ missing: ['DB_URL'] }); + expect(excludeOptionalKeys(scanResult, new Set())).toBe(scanResult); + }); + + it('does not mutate the input', () => { + const scanResult = makeScanResult({ missing: ['DB_URL', 'REDIS_URL'] }); + excludeOptionalKeys(scanResult, new Set(['REDIS_URL'])); + expect(scanResult.missing).toEqual(['DB_URL', 'REDIS_URL']); + }); +}); From a184ace7a5aa4156db76797490b4a68ff3816226 Mon Sep 17 00:00:00 2001 From: Chrilleweb Date: Tue, 27 Oct 2026 19:38:11 +0100 Subject: [PATCH 2/5] chore: scarnResult --- packages/cli/src/commands/scanUsage.ts | 16 ---- .../cli/src/services/processComparisonFile.ts | 83 ++++++------------- .../cli/test/unit/commands/scanUsage.test.ts | 27 +++--- .../services/processComparisonFile.test.ts | 22 +++-- 4 files changed, 56 insertions(+), 92 deletions(-) diff --git a/packages/cli/src/commands/scanUsage.ts b/packages/cli/src/commands/scanUsage.ts index 459c06fc..3f1d25b1 100644 --- a/packages/cli/src/commands/scanUsage.ts +++ b/packages/cli/src/commands/scanUsage.ts @@ -117,22 +117,6 @@ export async function scanUsage(opts: ScanUsageOptions): Promise { removedDuplicates = result.fix.removedDuplicates; fixedKeys = result.fix.addedEnv; gitignoreUpdated = result.fix.gitignoreUpdated; - if (result.uppercaseWarnings) { - scanResult.uppercaseWarnings = result.uppercaseWarnings; - } - if (result.expireWarnings) { - scanResult.expireWarnings = result.expireWarnings; - } - if (result.inconsistentNamingWarnings) { - scanResult.inconsistentNamingWarnings = - result.inconsistentNamingWarnings; - } - if (result.commentWarnings) { - scanResult.commentWarnings = result.commentWarnings; - } - if (result.driftWarnings) { - scanResult.driftWarnings = result.driftWarnings; - } if (result.exampleFull) { scanResult.exampleWarnings = detectSecretsInExample( result.exampleFull, diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index abf8b1ee..e0d94e99 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -19,13 +19,8 @@ import type { ScanUsageOptions, ScanResult, DuplicateResult, - UppercaseWarning, Duplicate, ComparisonFile, - ExpireWarning, - InconsistentNamingWarning, - CommentWarning, - DriftWarning, FixContext, } from '../config/types.js'; @@ -49,16 +44,6 @@ export interface ProcessComparisonResult { exampleFull?: Record | undefined; /** Basename of the example file `exampleFull` was read from. */ exampleFile?: string | undefined; - /** Uppercase keys found in the comparison file */ - uppercaseWarnings?: UppercaseWarning[]; - /** Expiration warnings found in the comparison file */ - expireWarnings?: ExpireWarning[]; - /** Inconsistent naming warnings found in the comparison file */ - inconsistentNamingWarnings?: InconsistentNamingWarning[]; - /** Comment warnings found in the comparison file */ - commentWarnings?: CommentWarning[]; - /** Drift warnings found in the comparison file */ - driftWarnings?: DriftWarning[]; /** Any error encountered while processing the comparison file */ error?: { message: string; shouldExit: boolean }; } @@ -81,14 +66,9 @@ export function processComparisonFile( let dupsEx: Duplicate[] = []; let exampleFull: Record | undefined = undefined; let exampleFile: string | undefined = undefined; - let uppercaseWarnings: UppercaseWarning[] = []; - let expireWarnings: ExpireWarning[] = []; - let inconsistentNamingWarnings: InconsistentNamingWarning[] = []; - let commentWarnings: CommentWarning[] = []; - let driftWarnings: DriftWarning[] = []; let error: ProcessComparisonResult['error']; - const fix: FixContext = { + let fix: FixContext = { fixApplied: false, removedDuplicates: [], addedEnv: [], @@ -141,9 +121,9 @@ export function processComparisonFile( scanResult = excludeOptionalKeys(scanResult, optionalKeys); // Detect uppercase keys - if (opts.uppercaseKeys) { - uppercaseWarnings = detectUppercaseKeys(envKeys); - } + scanResult.uppercaseWarnings = opts.uppercaseKeys + ? detectUppercaseKeys(envKeys) + : []; // Find duplicates if (!opts.allowDuplicates) { @@ -156,38 +136,36 @@ export function processComparisonFile( dupsEx = duplicateResults.dupsEx; } - if (opts.expireWarnings) { - expireWarnings = detectEnvExpirations(compareFile.path); - } + scanResult.expireWarnings = opts.expireWarnings + ? detectEnvExpirations(compareFile.path) + : []; // Check for inconsistent naming across env + example keys - if (opts.inconsistentNamingWarnings) { - const envKeysList = Object.keys(envFull); - const exampleKeysList = exampleFull ? Object.keys(exampleFull) : []; - - // Combine all keys for naming analysis - const allKeys = [...envKeysList, ...exampleKeysList]; - - inconsistentNamingWarnings = detectInconsistentNaming(allKeys); - } + scanResult.inconsistentNamingWarnings = opts.inconsistentNamingWarnings + ? detectInconsistentNaming([ + ...Object.keys(envFull), + ...Object.keys(exampleFull ?? {}), + ]) + : []; // Warn about .env.example keys without a documenting comment. Runs on the // example file (documentation lives there), not the env file — the same // example the other checks read. - if (opts.commentWarnings && exampleFilePath && exampleFull) { - commentWarnings = detectMissingComments(exampleFilePath); - } + scanResult.commentWarnings = + opts.commentWarnings && exampleFilePath && exampleFull + ? detectMissingComments(exampleFilePath) + : []; // Warn when the env file this run is about has drifted from the example // documenting it. The scan only ever reads one file, so without this the // example is never checked against the values actually in use. - if (opts.driftWarnings) { - driftWarnings = detectExampleDrift({ - comparisonPath: compareFile.path, - ignore: opts.ignore, - ignoreRegex: opts.ignoreRegex, - }); - } + scanResult.driftWarnings = opts.driftWarnings + ? detectExampleDrift({ + comparisonPath: compareFile.path, + ignore: opts.ignore, + ignoreRegex: opts.ignoreRegex, + }) + : []; // Apply fixes (both duplicates + missing keys + gitignore) if (opts.fix) { @@ -198,13 +176,9 @@ export function processComparisonFile( ensureGitignore: true, }); - if (changed) { - // Update state based on what was actually fixed - fix.fixApplied = true; - fix.removedDuplicates = result.removedDuplicates; - fix.addedEnv = result.addedEnv; - fix.gitignoreUpdated = result.gitignoreUpdated; + fix = { ...result, fixApplied: changed }; + if (changed) { // clear the issues that were fixed. `dupsEx` is not among them: the fix // only rewrites the env file, so duplicates in the example file survive // it and must still be reported. @@ -235,11 +209,6 @@ export function processComparisonFile( fix, exampleFull, exampleFile, - uppercaseWarnings, - expireWarnings, - inconsistentNamingWarnings, - commentWarnings, - driftWarnings, ...(error ? { error } : {}), }; } diff --git a/packages/cli/test/unit/commands/scanUsage.test.ts b/packages/cli/test/unit/commands/scanUsage.test.ts index da80afcc..08769d20 100644 --- a/packages/cli/test/unit/commands/scanUsage.test.ts +++ b/packages/cli/test/unit/commands/scanUsage.test.ts @@ -346,13 +346,16 @@ describe('scanUsage', () => { expect(processComparisonFile).toHaveBeenCalled(); }); - it('attaches commentWarnings from processComparisonFile onto the scan result', async () => { + it('passes commentWarnings from processComparisonFile through to output', async () => { vi.mocked(determineComparisonFile).mockResolvedValue({ type: 'found', file: { path: '/env/.env', name: '.env' }, }); vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, + scanResult: { + ...baseScanResult, + commentWarnings: [{ key: 'UNDOC', line: 2 }], + }, comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, @@ -364,7 +367,6 @@ describe('scanUsage', () => { addedEnv: [], gitignoreUpdated: false, }, - commentWarnings: [{ key: 'UNDOC', line: 2 }], } as ProcessComparisonResult); await scanUsage({ ...baseOpts, json: false }); @@ -377,7 +379,7 @@ describe('scanUsage', () => { ); }); - it('attaches driftWarnings from processComparisonFile onto the scan result', async () => { + it('passes driftWarnings from processComparisonFile through to output', async () => { const drift = [ { key: 'DRIFTED', envFile: '.env', exampleFile: '.env.example' }, ]; @@ -386,7 +388,7 @@ describe('scanUsage', () => { file: { path: '/env/.env', name: '.env' }, }); vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, + scanResult: { ...baseScanResult, driftWarnings: drift }, comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, @@ -398,7 +400,6 @@ describe('scanUsage', () => { addedEnv: [], gitignoreUpdated: false, }, - driftWarnings: drift, } as ProcessComparisonResult); await scanUsage({ ...baseOpts, json: false }); @@ -515,13 +516,20 @@ describe('scanUsage', () => { expect(result.exitWithError).toBe(false); }); - it('transfers uppercaseWarnings, expireWarnings and inconsistentNamingWarnings to scanResult', async () => { + it('passes uppercaseWarnings, expireWarnings and inconsistentNamingWarnings through to output', async () => { vi.mocked(determineComparisonFile).mockResolvedValue({ type: 'found', file: { path: '/env/.env', name: '.env' }, }); vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, + scanResult: { + ...baseScanResult, + uppercaseWarnings: [{ key: 'myKey', suggestion: 'MY_KEY' }], + expireWarnings: [{ key: 'OLD_KEY', date: '2024-01-01', daysLeft: -10 }], + inconsistentNamingWarnings: [ + { key1: 'A', key2: 'B', suggestion: 'A_B' }, + ], + }, comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, @@ -533,9 +541,6 @@ describe('scanUsage', () => { addedEnv: [], gitignoreUpdated: false, }, - uppercaseWarnings: [{ key: 'myKey', suggestion: 'MY_KEY' }], - expireWarnings: [{ key: 'OLD_KEY', date: '2024-01-01', daysLeft: -10 }], - inconsistentNamingWarnings: [{ key1: 'A', key2: 'B', suggestion: 'A_B' }], } as ProcessComparisonResult); await scanUsage(baseOpts); diff --git a/packages/cli/test/unit/services/processComparisonFile.test.ts b/packages/cli/test/unit/services/processComparisonFile.test.ts index 35765109..09424158 100644 --- a/packages/cli/test/unit/services/processComparisonFile.test.ts +++ b/packages/cli/test/unit/services/processComparisonFile.test.ts @@ -134,7 +134,7 @@ describe('processComparisonFile', () => { uppercaseKeys: true, }); - expect(result.uppercaseWarnings?.length).toBeGreaterThan(0); + expect(result.scanResult.uppercaseWarnings?.length).toBeGreaterThan(0); }); it('detects comment warnings on the example file when enabled', () => { @@ -143,7 +143,9 @@ describe('processComparisonFile', () => { commentWarnings: true, }); - expect(result.commentWarnings).toEqual([{ key: 'UNDOC', line: 2 }]); + expect(result.scanResult.commentWarnings).toEqual([ + { key: 'UNDOC', line: 2 }, + ]); }); it('detects drift against the comparison file when enabled', () => { @@ -155,7 +157,7 @@ describe('processComparisonFile', () => { expect(detectExampleDrift).toHaveBeenCalledWith( expect.objectContaining({ comparisonPath: compareFile.path }), ); - expect(result.driftWarnings).toEqual([ + expect(result.scanResult.driftWarnings).toEqual([ { key: 'DRIFTED', envFile: '.env', exampleFile: '.env.example' }, ]); }); @@ -167,7 +169,7 @@ describe('processComparisonFile', () => { }); expect(detectExampleDrift).not.toHaveBeenCalled(); - expect(result.driftWarnings).toEqual([]); + expect(result.scanResult.driftWarnings).toEqual([]); }); it('falls back to the default example file and skips when it does not exist', () => { @@ -179,7 +181,7 @@ describe('processComparisonFile', () => { commentWarnings: true, }); - expect(result.commentWarnings).toEqual([]); + expect(result.scanResult.commentWarnings).toEqual([]); expect(result.exampleFull).toBeUndefined(); }); @@ -231,7 +233,7 @@ describe('processComparisonFile', () => { expireWarnings: true, }); - expect(result.expireWarnings?.length).toBeGreaterThan(0); + expect(result.scanResult.expireWarnings?.length).toBeGreaterThan(0); }); it('detects inconsistent naming', () => { @@ -240,7 +242,9 @@ describe('processComparisonFile', () => { inconsistentNamingWarnings: true, }); - expect(result.inconsistentNamingWarnings?.length).toBeGreaterThan(0); + expect( + result.scanResult.inconsistentNamingWarnings?.length, + ).toBeGreaterThan(0); }); it('applies fixes when fix enabled', () => { @@ -454,7 +458,9 @@ describe('processComparisonFile', () => { }); expect(result.exampleFull).toBeUndefined(); - expect(result.inconsistentNamingWarnings?.length).toBeGreaterThan(0); + expect( + result.scanResult.inconsistentNamingWarnings?.length, + ).toBeGreaterThan(0); expect(result.error).toBeUndefined(); }); From a42bc9197e1c466696a70f9d9cb1e70049c7d8ec Mon Sep 17 00:00:00 2001 From: Chrilleweb Date: Tue, 27 Oct 2026 21:50:47 +0100 Subject: [PATCH 3/5] chore: ref --- packages/cli/src/commands/scanUsage.ts | 10 -- .../cli/src/services/processComparisonFile.ts | 27 ++-- .../cli/test/unit/commands/scanUsage.test.ts | 140 +----------------- .../services/processComparisonFile.test.ts | 49 +++++- 4 files changed, 60 insertions(+), 166 deletions(-) diff --git a/packages/cli/src/commands/scanUsage.ts b/packages/cli/src/commands/scanUsage.ts index 3f1d25b1..eeed339e 100644 --- a/packages/cli/src/commands/scanUsage.ts +++ b/packages/cli/src/commands/scanUsage.ts @@ -15,7 +15,6 @@ import { processComparisonFile } from '../services/processComparisonFile.js'; import { printComparisonError } from '../ui/scan/printComparisonError.js'; import { skipCommentedUsages } from '../core/scan/skipCommentedUsages.js'; import { frameworkValidator } from '../core/frameworks/frameworkValidator.js'; -import { detectSecretsInExample } from '../core/security/exampleSecretDetector.js'; import { checkGitignoreStatus } from '../services/git.js'; import { promptNoEnvScenario } from './prompts/promptNoEnvScenario.js'; import { @@ -110,19 +109,10 @@ export async function scanUsage(opts: ScanUsageOptions): Promise { } else { scanResult = result.scanResult; comparedAgainst = result.comparedAgainst; - if (result.exampleFile) { - scanResult.exampleFile = result.exampleFile; - } fixApplied = result.fix.fixApplied; removedDuplicates = result.fix.removedDuplicates; fixedKeys = result.fix.addedEnv; gitignoreUpdated = result.fix.gitignoreUpdated; - if (result.exampleFull) { - scanResult.exampleWarnings = detectSecretsInExample( - result.exampleFull, - result.exampleFile, - ); - } } } diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index e0d94e99..927b25da 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -15,6 +15,7 @@ import { detectMissingComments } from './detectMissingComments.js'; import { detectOptionalKeys } from './detectOptionalKeys.js'; import { detectExampleDrift } from './detectExampleDrift.js'; import { resolveExampleFile } from './resolveExampleFile.js'; +import { detectSecretsInExample } from '../core/security/exampleSecretDetector.js'; import type { ScanUsageOptions, ScanResult, @@ -40,10 +41,6 @@ export interface ProcessComparisonResult { dupsEx: Duplicate[]; /** The context of any fixes applied to the comparison file */ fix: FixContext; - /** The full contents of the example file, if it was found and read */ - exampleFull?: Record | undefined; - /** Basename of the example file `exampleFull` was read from. */ - exampleFile?: string | undefined; /** Any error encountered while processing the comparison file */ error?: { message: string; shouldExit: boolean }; } @@ -64,8 +61,6 @@ export function processComparisonFile( let comparedAgainst = ''; let dupsEnv: Duplicate[] = []; let dupsEx: Duplicate[] = []; - let exampleFull: Record | undefined = undefined; - let exampleFile: string | undefined = undefined; let error: ProcessComparisonResult['error']; let fix: FixContext = { @@ -84,10 +79,10 @@ export function processComparisonFile( ? resolveFromCwd(opts.cwd, opts.examplePath) : resolveExampleFile(compareFile.path); - if (exampleFilePath && fs.existsSync(exampleFilePath)) { - exampleFull = parseEnvFile(exampleFilePath); - exampleFile = path.basename(exampleFilePath); - } + const exampleFull = + exampleFilePath && fs.existsSync(exampleFilePath) + ? parseEnvFile(exampleFilePath) + : undefined; // Parse and filter env file const envFull = parseEnvFile(compareFile.path); @@ -120,6 +115,16 @@ export function processComparisonFile( ); scanResult = excludeOptionalKeys(scanResult, optionalKeys); + // Scan the example for secrets even when it is not the comparison file — + // an example committed with a real secret is dangerous either way. + if (exampleFilePath && exampleFull) { + scanResult.exampleFile = path.basename(exampleFilePath); + scanResult.exampleWarnings = detectSecretsInExample( + exampleFull, + scanResult.exampleFile, + ); + } + // Detect uppercase keys scanResult.uppercaseWarnings = opts.uppercaseKeys ? detectUppercaseKeys(envKeys) @@ -207,8 +212,6 @@ export function processComparisonFile( dupsEnv, dupsEx, fix, - exampleFull, - exampleFile, ...(error ? { error } : {}), }; } diff --git a/packages/cli/test/unit/commands/scanUsage.test.ts b/packages/cli/test/unit/commands/scanUsage.test.ts index 08769d20..4ac160ec 100644 --- a/packages/cli/test/unit/commands/scanUsage.test.ts +++ b/packages/cli/test/unit/commands/scanUsage.test.ts @@ -52,18 +52,6 @@ vi.mock('../../../src/core/frameworks/frameworkValidator.js', () => ({ frameworkValidator: vi.fn(() => []), })); -vi.mock('../../../src/core/security/exampleSecretDetector.js', () => ({ - detectSecretsInExample: vi.fn(() => [ - { - key: 'SECRET', - value: '123', - file: '.env.example', - message: 'hardcoded', - severity: 'high', - }, - ]), -})); - vi.mock('../../../src/commands/prompts/promptNoEnvScenario.js', () => ({ promptNoEnvScenario: vi.fn(), })); @@ -412,7 +400,7 @@ describe('scanUsage', () => { ); }); - it('attaches the resolved example file name onto the scan result', async () => { + it('passes the example file name from processComparisonFile through to output', async () => { // Carried through so findings about the example file can name it — a // project may document itself with .env.sample rather than .env.example. vi.mocked(determineComparisonFile).mockResolvedValue({ @@ -420,7 +408,7 @@ describe('scanUsage', () => { file: { path: '/env/.env', name: '.env' }, }); vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, + scanResult: { ...baseScanResult, exampleFile: '.env.sample' }, comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, @@ -432,7 +420,6 @@ describe('scanUsage', () => { addedEnv: [], gitignoreUpdated: false, }, - exampleFile: '.env.sample', } as ProcessComparisonResult); await scanUsage({ ...baseOpts, json: false }); @@ -445,34 +432,6 @@ describe('scanUsage', () => { ); }); - it('leaves the example file name unset when no example file was resolved', async () => { - vi.mocked(determineComparisonFile).mockResolvedValue({ - type: 'found', - file: { path: '/env/.env', name: '.env' }, - }); - vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], - fix: { - fixApplied: false, - removedDuplicates: [], - addedEnv: [], - gitignoreUpdated: false, - }, - exampleFile: undefined, - } as ProcessComparisonResult); - - await scanUsage({ ...baseOpts, json: false }); - - expect(vi.mocked(printScanResult).mock.calls[0]?.[0]).not.toHaveProperty( - 'exampleFile', - ); - }); - it('sets frameworkWarnings on scanResult when frameworkValidator returns results', async () => { const { frameworkValidator } = await import('../../../src/core/frameworks/frameworkValidator.js'); @@ -563,101 +522,6 @@ describe('scanUsage', () => { ); }); - it('sets exampleWarnings when comparedAgainst matches DEFAULT_EXAMPLE_FILE', async () => { - const { DEFAULT_EXAMPLE_FILE } = - await import('../../../src/config/constants.js'); - - vi.mocked(determineComparisonFile).mockResolvedValue({ - type: 'found', - file: { path: '/env/.env.example', name: DEFAULT_EXAMPLE_FILE }, - }); - vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, - comparedAgainst: DEFAULT_EXAMPLE_FILE, - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], - exampleFull: { SECRET: 'abc123' }, - fix: { - fixApplied: false, - removedDuplicates: [], - addedEnv: [], - gitignoreUpdated: false, - }, - } as ProcessComparisonResult); - - await scanUsage(baseOpts); - - const { detectSecretsInExample } = - await import('../../../src/core/security/exampleSecretDetector.js'); - expect(detectSecretsInExample).toHaveBeenCalledWith( - { SECRET: 'abc123' }, - undefined, - ); - }); - - it('scans the example for secrets even when comparing against .env', async () => { - // An example committed with a real secret is dangerous whether or not the - // scan happens to compare against it. - vi.mocked(determineComparisonFile).mockResolvedValue({ - type: 'found', - file: { path: '/env/.env', name: '.env' }, - }); - vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], - exampleFull: { SECRET: 'abc123' }, - fix: { - fixApplied: false, - removedDuplicates: [], - addedEnv: [], - gitignoreUpdated: false, - }, - } as ProcessComparisonResult); - - await scanUsage(baseOpts); - - const { detectSecretsInExample } = - await import('../../../src/core/security/exampleSecretDetector.js'); - expect(detectSecretsInExample).toHaveBeenCalledWith( - { SECRET: 'abc123' }, - undefined, - ); - }); - - it('does not set exampleWarnings when no example file was found', async () => { - vi.mocked(determineComparisonFile).mockResolvedValue({ - type: 'found', - file: { path: '/env/.env', name: '.env' }, - }); - vi.mocked(processComparisonFile).mockReturnValue({ - scanResult: { ...baseScanResult }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], - exampleFull: undefined, - fix: { - fixApplied: false, - removedDuplicates: [], - addedEnv: [], - gitignoreUpdated: false, - }, - } as ProcessComparisonResult); - - await scanUsage(baseOpts); - - const { detectSecretsInExample } = - await import('../../../src/core/security/exampleSecretDetector.js'); - expect(detectSecretsInExample).not.toHaveBeenCalled(); - }); - it('filters out commented usages before processing', async () => { vi.mocked(scanCodebase).mockResolvedValue({ ...baseScanResult, diff --git a/packages/cli/test/unit/services/processComparisonFile.test.ts b/packages/cli/test/unit/services/processComparisonFile.test.ts index 09424158..7493fac0 100644 --- a/packages/cli/test/unit/services/processComparisonFile.test.ts +++ b/packages/cli/test/unit/services/processComparisonFile.test.ts @@ -20,7 +20,8 @@ vi.mock('../../../src/core/filterIgnoredKeys.js', () => ({ })); vi.mock('../../../src/core/scan/compareScan.js', () => ({ - compareWithEnvFiles: vi.fn((scan) => scan), + // Returns a copy like the real function, so the shared base result is not mutated + compareWithEnvFiles: vi.fn((scan) => ({ ...scan })), })); vi.mock('../../../src/services/duplicates.js', () => ({ @@ -64,6 +65,18 @@ vi.mock('../../../src/services/detectOptionalKeys.js', () => ({ detectOptionalKeys: vi.fn(() => []), })); +vi.mock('../../../src/core/security/exampleSecretDetector.js', () => ({ + detectSecretsInExample: vi.fn(() => [ + { + key: 'SECRET', + value: 'abc123', + message: 'looks real', + file: '.env.example', + severity: 'high', + }, + ]), +})); + vi.mock('../../../src/services/detectExampleDrift.js', () => ({ detectExampleDrift: vi.fn(() => [ { key: 'DRIFTED', envFile: '.env', exampleFile: '.env.example' }, @@ -79,6 +92,7 @@ import { findDuplicateKeys } from '../../../src/services/duplicates.js'; import { resolveFromCwd } from '../../../src/core/helpers/resolveFromCwd.js'; import { detectOptionalKeys } from '../../../src/services/detectOptionalKeys.js'; import { detectExampleDrift } from '../../../src/services/detectExampleDrift.js'; +import { detectSecretsInExample } from '../../../src/core/security/exampleSecretDetector.js'; describe('processComparisonFile', () => { const baseScanResult: ScanResult = { @@ -182,7 +196,7 @@ describe('processComparisonFile', () => { }); expect(result.scanResult.commentWarnings).toEqual([]); - expect(result.exampleFull).toBeUndefined(); + expect(result.scanResult.exampleFile).toBeUndefined(); }); it('does not report a key marked @optional in the example file as missing', () => { @@ -315,7 +329,7 @@ describe('processComparisonFile', () => { const result = processComparisonFile(baseScanResult, compareFile, opts); expect(result.error).toBeUndefined(); - expect(result.exampleFull).toEqual({ A: '1', bKey: '2' }); + expect(result.scanResult.exampleFile).toBe('.env.example'); }); it('checks the example file for duplicates without the examplePath option', () => { @@ -397,15 +411,38 @@ describe('processComparisonFile', () => { expect(result.scanResult.duplicates?.env).toBeDefined(); }); - it('does not set exampleFull when example file does not exist on disk (line 74)', () => { + it('does not scan the example when it does not exist on disk', () => { vi.mocked(fs.existsSync).mockReturnValueOnce(false); const result = processComparisonFile(baseScanResult, compareFile, baseOpts); - expect(result.exampleFull).toBeUndefined(); + expect(result.scanResult).not.toHaveProperty('exampleFile'); + expect(result.scanResult).not.toHaveProperty('exampleWarnings'); + expect(detectSecretsInExample).not.toHaveBeenCalled(); expect(result.error).toBeUndefined(); }); + it('scans the example for secrets even when comparing against .env', () => { + // An example committed with a real secret is dangerous whether or not the + // scan happens to compare against it. + const result = processComparisonFile(baseScanResult, compareFile, baseOpts); + + expect(detectSecretsInExample).toHaveBeenCalledWith( + { A: '1', bKey: '2' }, + '.env.example', + ); + expect(result.scanResult.exampleFile).toBe('.env.example'); + expect(result.scanResult.exampleWarnings).toEqual([ + { + key: 'SECRET', + value: 'abc123', + message: 'looks real', + file: '.env.example', + severity: 'high', + }, + ]); + }); + it('tests duplicate keys against ignoreRegex patterns', () => { const result = processComparisonFile(baseScanResult, compareFile, { ...baseOpts, @@ -457,7 +494,7 @@ describe('processComparisonFile', () => { inconsistentNamingWarnings: true, }); - expect(result.exampleFull).toBeUndefined(); + expect(result.scanResult.exampleFile).toBeUndefined(); expect( result.scanResult.inconsistentNamingWarnings?.length, ).toBeGreaterThan(0); From b7cccbdda817e54be30199edfb500738a00503d3 Mon Sep 17 00:00:00 2001 From: Chrilleweb Date: Tue, 27 Oct 2026 21:57:05 +0100 Subject: [PATCH 4/5] chore: ref --- packages/cli/src/commands/scanUsage.ts | 2 +- .../cli/src/services/processComparisonFile.ts | 47 ++++-------- .../cli/test/unit/commands/scanUsage.test.ts | 32 ++------ .../services/processComparisonFile.test.ts | 74 +++++++++++-------- 4 files changed, 64 insertions(+), 91 deletions(-) diff --git a/packages/cli/src/commands/scanUsage.ts b/packages/cli/src/commands/scanUsage.ts index eeed339e..0933bde9 100644 --- a/packages/cli/src/commands/scanUsage.ts +++ b/packages/cli/src/commands/scanUsage.ts @@ -108,7 +108,7 @@ export async function scanUsage(opts: ScanUsageOptions): Promise { if (exit) return { exitWithError: true }; } else { scanResult = result.scanResult; - comparedAgainst = result.comparedAgainst; + comparedAgainst = compareFile.name; fixApplied = result.fix.fixApplied; removedDuplicates = result.fix.removedDuplicates; fixedKeys = result.fix.addedEnv; diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index 927b25da..ca8a9dd3 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -31,14 +31,6 @@ import type { export interface ProcessComparisonResult { /** The scan result after processing the comparison file */ scanResult: ScanResult; - /** The environment variables from the comparison file */ - envVariables: Record; - /** The file the comparison was made against */ - comparedAgainst: string; - /** The duplicate environment variables found in the comparison file */ - dupsEnv: Duplicate[]; - /** The duplicate example variables found in the comparison file */ - dupsEx: Duplicate[]; /** The context of any fixes applied to the comparison file */ fix: FixContext; /** Any error encountered while processing the comparison file */ @@ -57,10 +49,6 @@ export function processComparisonFile( compareFile: ComparisonFile, opts: ScanUsageOptions, ): ProcessComparisonResult { - let envVariables: Record = {}; - let comparedAgainst = ''; - let dupsEnv: Duplicate[] = []; - let dupsEx: Duplicate[] = []; let error: ProcessComparisonResult['error']; let fix: FixContext = { @@ -91,7 +79,9 @@ export function processComparisonFile( opts.ignore, opts.ignoreRegex, ); - envVariables = Object.fromEntries(envKeys.map((k) => [k, envFull[k]])); + const envVariables = Object.fromEntries( + envKeys.map((k) => [k, envFull[k]]), + ); // Monorepo support: also honor `.env.example` files in subdirectories, so a // variable documented next to where it is used is not reported as missing. @@ -107,7 +97,6 @@ export function processComparisonFile( opts.ignoreRegex, scopes, ); - comparedAgainst = compareFile.name; // Keys marked `@optional` in the example file are not reported as missing const optionalKeys = new Set( @@ -131,15 +120,9 @@ export function processComparisonFile( : []; // Find duplicates - if (!opts.allowDuplicates) { - const duplicateResults = checkDuplicates( - compareFile, - exampleFilePath, - opts, - ); - dupsEnv = duplicateResults.dupsEnv; - dupsEx = duplicateResults.dupsEx; - } + let { dupsEnv, dupsEx }: DuplicateResult = opts.allowDuplicates + ? { dupsEnv: [], dupsEx: [] } + : checkDuplicates(compareFile, exampleFilePath, opts); scanResult.expireWarnings = opts.expireWarnings ? detectEnvExpirations(compareFile.path) @@ -194,9 +177,11 @@ export function processComparisonFile( // Keep the duplicates that were not fixed for output if (dupsEnv.length > 0 || dupsEx.length > 0) { - if (!scanResult.duplicates) scanResult.duplicates = {}; - if (dupsEnv.length > 0) scanResult.duplicates.env = dupsEnv; - if (dupsEx.length > 0) scanResult.duplicates.example = dupsEx; + scanResult.duplicates = { + ...scanResult.duplicates, + ...(dupsEnv.length > 0 ? { env: dupsEnv } : {}), + ...(dupsEx.length > 0 ? { example: dupsEx } : {}), + }; } } catch (err) { error = { @@ -207,10 +192,6 @@ export function processComparisonFile( return { scanResult, - envVariables, - comparedAgainst, - dupsEnv, - dupsEx, fix, ...(error ? { error } : {}), }; @@ -235,12 +216,12 @@ function checkDuplicates( examplePath: string | null, opts: ScanUsageOptions, ): DuplicateResult { - const isIgnored = (key: string) => + const isNotIgnored = (key: string) => !opts.ignore.includes(key) && !opts.ignoreRegex.some((rx) => rx.test(key)); // Duplicates in main env file const dupsEnv = findDuplicateKeys(compareFile.path).filter(({ key }) => - isIgnored(key), + isNotIgnored(key), ); // Duplicates in example file. Skipped when it *is* the comparison file — a @@ -249,7 +230,7 @@ function checkDuplicates( // not exist, which an explicit `--example` may still point at. const dupsEx: Duplicate[] = examplePath && examplePath !== compareFile.path - ? findDuplicateKeys(examplePath).filter(({ key }) => isIgnored(key)) + ? findDuplicateKeys(examplePath).filter(({ key }) => isNotIgnored(key)) : []; return { dupsEnv, dupsEx } satisfies DuplicateResult; diff --git a/packages/cli/test/unit/commands/scanUsage.test.ts b/packages/cli/test/unit/commands/scanUsage.test.ts index 4ac160ec..80d7ba58 100644 --- a/packages/cli/test/unit/commands/scanUsage.test.ts +++ b/packages/cli/test/unit/commands/scanUsage.test.ts @@ -315,11 +315,6 @@ describe('scanUsage', () => { vi.mocked(determineComparisonFile).mockResolvedValue({ type: 'none' }); vi.mocked(processComparisonFile).mockReturnValue({ scanResult: { ...baseScanResult }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], fix: { fixApplied: false, removedDuplicates: [], @@ -332,6 +327,13 @@ describe('scanUsage', () => { expect(promptNoEnvScenario).toHaveBeenCalled(); expect(processComparisonFile).toHaveBeenCalled(); + // The name of the file picked in the prompt is what the output reports + expect(printScanResult).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + '.env', + expect.anything(), + ); }); it('passes commentWarnings from processComparisonFile through to output', async () => { @@ -344,11 +346,6 @@ describe('scanUsage', () => { ...baseScanResult, commentWarnings: [{ key: 'UNDOC', line: 2 }], }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], fix: { fixApplied: false, removedDuplicates: [], @@ -377,11 +374,6 @@ describe('scanUsage', () => { }); vi.mocked(processComparisonFile).mockReturnValue({ scanResult: { ...baseScanResult, driftWarnings: drift }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], fix: { fixApplied: false, removedDuplicates: [], @@ -409,11 +401,6 @@ describe('scanUsage', () => { }); vi.mocked(processComparisonFile).mockReturnValue({ scanResult: { ...baseScanResult, exampleFile: '.env.sample' }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], fix: { fixApplied: false, removedDuplicates: [], @@ -489,11 +476,6 @@ describe('scanUsage', () => { { key1: 'A', key2: 'B', suggestion: 'A_B' }, ], }, - comparedAgainst: '.env', - envVariables: {}, - duplicatesFound: false, - dupsEnv: [], - dupsEx: [], fix: { fixApplied: false, removedDuplicates: [], diff --git a/packages/cli/test/unit/services/processComparisonFile.test.ts b/packages/cli/test/unit/services/processComparisonFile.test.ts index 7493fac0..f5680616 100644 --- a/packages/cli/test/unit/services/processComparisonFile.test.ts +++ b/packages/cli/test/unit/services/processComparisonFile.test.ts @@ -138,7 +138,6 @@ describe('processComparisonFile', () => { it('processes normally without fix', () => { const result = processComparisonFile(baseScanResult, compareFile, baseOpts); - expect(result.comparedAgainst).toBe('.env'); expect(result.error).toBeUndefined(); }); @@ -238,7 +237,18 @@ describe('processComparisonFile', () => { allowDuplicates: false, }); - expect(result.dupsEnv.length).toBeGreaterThan(0); + expect(result.scanResult.duplicates?.env).toEqual([{ key: 'A', count: 2 }]); + }); + + it('does not mutate the duplicates object of the scan result passed in', () => { + const input: ScanResult = { ...baseScanResult, duplicates: {} }; + + processComparisonFile(input, compareFile, { + ...baseOpts, + allowDuplicates: false, + }); + + expect(input.duplicates).toEqual({}); }); it('detects expire warnings', () => { @@ -293,7 +303,8 @@ describe('processComparisonFile', () => { examplePath: '.env.example', }); - expect(result.comparedAgainst).toBe('.env.example'); + expect(result.error).toBeUndefined(); + expect(result.scanResult.exampleFile).toBe('.env.example'); }); it('returns error result when file cannot be read', () => { @@ -340,18 +351,20 @@ describe('processComparisonFile', () => { .mockReturnValueOnce([]) .mockReturnValueOnce([{ key: 'EX_KEY', count: 2 }]); - const result = processComparisonFile( - { ...baseScanResult, duplicates: {} }, - compareFile, - { ...baseOpts, examplePath: undefined, allowDuplicates: false }, - ); + const result = processComparisonFile(baseScanResult, compareFile, { + ...baseOpts, + examplePath: undefined, + allowDuplicates: false, + }); // `path.join` in resolveExampleFile, so the separator is platform-specific. expect(findDuplicateKeys).toHaveBeenNthCalledWith( 2, path.join('/env', '.env.example'), ); - expect(result.dupsEx).toEqual([{ key: 'EX_KEY', count: 2 }]); + expect(result.scanResult.duplicates?.example).toEqual([ + { key: 'EX_KEY', count: 2 }, + ]); }); it('keeps example duplicates after a fix, which only rewrites the env file', () => { @@ -359,15 +372,14 @@ describe('processComparisonFile', () => { .mockReturnValueOnce([{ key: 'A', count: 2 }]) .mockReturnValueOnce([{ key: 'EX_KEY', count: 2 }]); - const result = processComparisonFile( - { ...baseScanResult, duplicates: {} }, - compareFile, - { ...baseOpts, fix: true, allowDuplicates: false }, - ); + const result = processComparisonFile(baseScanResult, compareFile, { + ...baseOpts, + fix: true, + allowDuplicates: false, + }); expect(result.fix.fixApplied).toBe(true); // The env file was deduplicated, the example file was never touched. - expect(result.dupsEnv).toHaveLength(0); expect(result.scanResult.duplicates?.env).toBeUndefined(); expect(result.scanResult.duplicates?.example).toEqual([ { key: 'EX_KEY', count: 2 }, @@ -386,8 +398,10 @@ describe('processComparisonFile', () => { examplePath: '.env.example', }); - // dupsEnv still found, but dupsEx should be empty because same file - expect(result.dupsEx).toHaveLength(0); + // env duplicates still found, but the example is not checked a second time + expect(findDuplicateKeys).toHaveBeenCalledTimes(1); + expect(result.scanResult.duplicates?.env).toBeDefined(); + expect(result.scanResult.duplicates?.example).toBeUndefined(); }); it('does not clear state when fix returns changed=false', () => { @@ -451,7 +465,7 @@ describe('processComparisonFile', () => { }); // The duplicate key ('A') doesn't match the regex, so it's kept. - expect(result.dupsEnv.length).toBeGreaterThan(0); + expect(result.scanResult.duplicates?.env).toEqual([{ key: 'A', count: 2 }]); }); it('skips duplicate check when allowDuplicates is true (lines 105-109)', () => { @@ -460,29 +474,25 @@ describe('processComparisonFile', () => { allowDuplicates: true, }); - expect(result.dupsEnv).toHaveLength(0); - expect(result.dupsEx).toHaveLength(0); + expect(findDuplicateKeys).not.toHaveBeenCalled(); + expect(result.scanResult.duplicates).toEqual({}); }); - it('sets duplicatesFound via dupsEx when only example file has duplicates (lines 109, 154)', () => { + it('reports only example duplicates when the env file has none', () => { // First call: env file → no duplicates. Second call: example file → has duplicate. vi.mocked(findDuplicateKeys) .mockReturnValueOnce([]) .mockReturnValueOnce([{ key: 'EX_KEY', count: 2 }]); - // Use a fresh duplicates object to avoid mutation from previous tests - const result = processComparisonFile( - { ...baseScanResult, duplicates: {} }, - compareFile, - { ...baseOpts, allowDuplicates: false }, - ); + const result = processComparisonFile(baseScanResult, compareFile, { + ...baseOpts, + allowDuplicates: false, + }); - expect(result.dupsEnv).toHaveLength(0); - expect(result.dupsEx).toHaveLength(1); - // dupsEnv is empty → scanResult.duplicates.env NOT set (line 154 false branch) expect(result.scanResult.duplicates?.env).toBeUndefined(); - // dupsEx has items → scanResult.duplicates.example IS set - expect(result.scanResult.duplicates?.example).toBeDefined(); + expect(result.scanResult.duplicates?.example).toEqual([ + { key: 'EX_KEY', count: 2 }, + ]); }); it('uses empty exampleKeysList when exampleFull is undefined in inconsistent naming check (line 119)', () => { From 7bac2fb4740b02942d03e34781bc153ee9b056cf Mon Sep 17 00:00:00 2001 From: Chrilleweb Date: Wed, 28 Oct 2026 08:49:36 +0100 Subject: [PATCH 5/5] chore: smoketest --- .github/workflows/smoke-test.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml index 0dc67464..17a5ee01 100644 --- a/.github/workflows/smoke-test.yml +++ b/.github/workflows/smoke-test.yml @@ -77,12 +77,16 @@ jobs: exit 1 fi + # Plain `npm install` on purpose: this checks the tarball installs the way + # it does for a user, so it cannot be locked with `npm ci`. TypeScript is + # not installed here: the type check uses the repo's tsc, pinned by + # pnpm-lock.yaml, to keep unpinned downloads to the package's own deps. - name: Install tarball into empty project working-directory: ${{ runner.temp }}/smoke run: | npm init -y > /dev/null npm pkg set type=module - npm install --no-audit --no-fund ./dotenv-diff-*.tgz typescript + npm install --no-audit --no-fund ./dotenv-diff-*.tgz - name: Run CLI working-directory: ${{ runner.temp }}/smoke @@ -134,5 +138,5 @@ jobs: const result: DiffResult = diffEnv(parseEnvContent('A=1'), {}); export const missing: string[] = result.missing; EOF - npx --no-install tsc -p tsconfig.json + node "$GITHUB_WORKSPACE/node_modules/typescript/bin/tsc" -p tsconfig.json echo "Types OK"