diff --git a/.changeset/tall-eels-smile.md b/.changeset/tall-eels-smile.md new file mode 100644 index 00000000..3675990e --- /dev/null +++ b/.changeset/tall-eels-smile.md @@ -0,0 +1,5 @@ +--- +'dotenv-diff': major +--- + +fix: duplicates --json output in scan diff --git a/docs/baseline.md b/docs/baseline.md index c475b8d3..310073b7 100644 --- a/docs/baseline.md +++ b/docs/baseline.md @@ -37,7 +37,7 @@ Baseline suppression supports the same categories produced by scan usage checks, - missing variables - unused variables -- duplicate keys (`.env` / `.env.example`) +- duplicate keys (in the file the scan compared against) - framework warnings - uppercase key warnings - inconsistent naming warnings diff --git a/packages/cli/src/baseline/scanBaseline.ts b/packages/cli/src/baseline/scanBaseline.ts index 5eed1b5f..1eb2755e 100644 --- a/packages/cli/src/baseline/scanBaseline.ts +++ b/packages/cli/src/baseline/scanBaseline.ts @@ -3,6 +3,7 @@ import fs from 'fs'; import type { BaselineEntry, BaselineFile, + BaselineRule, ScanResult, } from '../config/types.js'; import { resolveFromCwd } from '../core/helpers/resolveFromCwd.js'; @@ -28,7 +29,8 @@ export function loadBaselineFile(cwd: string): BaselineFile | null { 'version' in parsed && Array.isArray((parsed as { entries?: unknown }).entries) ) { - return parsed as BaselineFile; + const file = parsed as BaselineFile; + return { ...file, entries: file.entries.map(migrateEntry) }; } return null; } catch { @@ -36,6 +38,29 @@ export function loadBaselineFile(cwd: string): BaselineFile | null { } } +/** + * Rule names that older versions wrote for warnings that have since been + * renamed. Migrating them here — at the single point where a baseline file + * enters the program — keeps the rest of the codebase on one name per rule, + * and means an upgrade never silently un-suppresses a warning. + */ +const RENAMED_RULES: Record = { + // <=3.4 attributed scan duplicates to an env/example pair that does not + // exist: a scan only ever reads one file. + 'duplicate-env': 'duplicate', + 'duplicate-example': 'duplicate', +}; + +/** + * Rewrites a baseline entry written by an older version to its current rule name. + * @param entry - The entry as read from disk + * @returns The entry with an up-to-date rule name + */ +function migrateEntry(entry: BaselineEntry): BaselineEntry { + const renamed = RENAMED_RULES[entry.rule]; + return renamed ? { ...entry, rule: renamed } : entry; +} + /** * Writes a baseline file to disk and returns the absolute path it was written to. * @param cwd - Current working directory to resolve the baseline file from @@ -104,12 +129,8 @@ export function collectBaselineEntries( entries.push({ rule: 'example-secret', key: warning.key }); } - for (const dup of scanResult.duplicates.env ?? []) { - entries.push({ rule: 'duplicate-env', key: dup.key }); - } - - for (const dup of scanResult.duplicates.example ?? []) { - entries.push({ rule: 'duplicate-example', key: dup.key }); + for (const dup of scanResult.duplicates.keys ?? []) { + entries.push({ rule: 'duplicate', key: dup.key }); } // variable + file uniquely identifies a framework warning without line numbers @@ -175,14 +196,10 @@ export function applyBaselineEntries( (s) => !has('secret', fingerprint(`${s.file}:${s.snippet}`)), ), duplicates: { - ...(scanResult.duplicates.env != null && { - env: scanResult.duplicates.env.filter( - (d) => !has('duplicate-env', d.key), - ), - }), - ...(scanResult.duplicates.example != null && { - example: scanResult.duplicates.example.filter( - (d) => !has('duplicate-example', d.key), + ...scanResult.duplicates, + ...(scanResult.duplicates.keys != null && { + keys: scanResult.duplicates.keys.filter( + (d) => !has('duplicate', d.key), ), }), }, diff --git a/packages/cli/src/commands/scanUsage.ts b/packages/cli/src/commands/scanUsage.ts index a8c9920b..622b1b0d 100644 --- a/packages/cli/src/commands/scanUsage.ts +++ b/packages/cli/src/commands/scanUsage.ts @@ -214,8 +214,7 @@ function calculateStats(scanResult: ScanResult): void { (scanResult.secrets?.length ?? 0) + scanResult.missing.length + scanResult.unused.length + - (scanResult.duplicates?.env?.length ?? 0) + - (scanResult.duplicates?.example?.length ?? 0); + (scanResult.duplicates?.keys?.length ?? 0); scanResult.stats = { filesScanned: scanResult.stats.filesScanned, diff --git a/packages/cli/src/config/types.ts b/packages/cli/src/config/types.ts index c6897ac9..9a10abd5 100644 --- a/packages/cli/src/config/types.ts +++ b/packages/cli/src/config/types.ts @@ -48,6 +48,21 @@ export interface DuplicateResult { dupsEx: Duplicate[]; } +/** + * Duplicate keys found by a scan. + * + * A scan only ever reads a single file, so — unlike `--compare`, which works on + * a real env/example pair — there is no second file to attribute duplicates to. + * `file` names the file the keys were actually read from, so the console header + * and the JSON output can never disagree about which file is meant. + */ +export interface ScanDuplicates { + /** Basename of the file the duplicates were found in. */ + file?: string; + /** The duplicated keys in that file, with their occurrence counts. */ + keys?: Duplicate[]; +} + /** * Type representing a single category for comparison */ @@ -283,10 +298,7 @@ export interface ScanResult { declaredKeys?: string[]; stats: ScanStats; secrets: SecretFinding[]; - duplicates: { - env?: Duplicate[]; - example?: Duplicate[]; - }; + duplicates: ScanDuplicates; frameworkWarnings?: FrameworkWarning[]; exampleWarnings?: ExampleSecretWarning[]; logged: EnvUsage[]; @@ -526,8 +538,7 @@ export type BaselineRule = | 'logged' | 'secret' | 'example-secret' - | 'duplicate-env' - | 'duplicate-example' + | 'duplicate' | 'framework' | 'uppercase' | 'expire' diff --git a/packages/cli/src/core/scan/computeExitDecision.ts b/packages/cli/src/core/scan/computeExitDecision.ts index 31341ad7..47b17b77 100644 --- a/packages/cli/src/core/scan/computeExitDecision.ts +++ b/packages/cli/src/core/scan/computeExitDecision.ts @@ -70,8 +70,7 @@ function hasStrictViolation( ): boolean { return ( scan.unused.length > 0 || - (scan.duplicates?.env?.length ?? 0) > 0 || - (scan.duplicates?.example?.length ?? 0) > 0 || + (scan.duplicates?.keys?.length ?? 0) > 0 || (scan.secrets?.length ?? 0) > 0 || (scan.exampleWarnings?.length ?? 0) > 0 || (scan.frameworkWarnings?.length ?? 0) > 0 || diff --git a/packages/cli/src/core/scan/computeHealthScore.ts b/packages/cli/src/core/scan/computeHealthScore.ts index 9a14f532..4ec36875 100644 --- a/packages/cli/src/core/scan/computeHealthScore.ts +++ b/packages/cli/src/core/scan/computeHealthScore.ts @@ -46,8 +46,7 @@ export function computeHealthScore(scan: ScanResult): number { score -= (scan.driftWarnings?.length ?? 0) * 2; // === 10. Duplicate definitions === - score -= (scan.duplicates?.env?.length ?? 0) * 10; - score -= (scan.duplicates?.example?.length ?? 0) * 10; + score -= (scan.duplicates?.keys?.length ?? 0) * 10; // Never go below 0 or above 100 return Math.max(0, Math.min(100, score)); diff --git a/packages/cli/src/services/printScanResult.ts b/packages/cli/src/services/printScanResult.ts index 5603a265..2b6cf153 100644 --- a/packages/cli/src/services/printScanResult.ts +++ b/packages/cli/src/services/printScanResult.ts @@ -11,7 +11,7 @@ import { printHeader } from '../ui/scan/printHeader.js'; import { printStats } from '../ui/scan/printStats.js'; import { printMissing } from '../ui/scan/printMissing.js'; import { printUnused } from '../ui/scan/printUnused.js'; -import { printDuplicates } from '../ui/shared/printDuplicates.js'; +import { printScanDuplicates } from '../ui/scan/printScanDuplicates.js'; import { printSecrets } from '../ui/scan/printSecrets.js'; import { printFixTips } from '../ui/shared/printFixTips.js'; import { printAutoFix } from '../ui/shared/printAutoFix.js'; @@ -87,12 +87,10 @@ export function printScanResult( printUnused(scanResult.unused, comparedAgainst, opts.strict); } - // Duplicates - printDuplicates( - comparedAgainst || DEFAULT_ENV_FILE, - 'example file', - scanResult.duplicates?.env ?? [], - scanResult.duplicates?.example ?? [], + // Duplicates — always attributed to the one file the scan read + printScanDuplicates( + scanResult.duplicates?.file || comparedAgainst || DEFAULT_ENV_FILE, + scanResult.duplicates?.keys ?? [], isJson, opts.fix ?? false, opts.strict, @@ -156,8 +154,8 @@ export function printScanResult( printFixTips( { missing: scanResult.missing, - duplicatesEnv: scanResult.duplicates?.env ?? [], - duplicatesEx: scanResult.duplicates?.example ?? [], + duplicatesEnv: scanResult.duplicates?.keys ?? [], + duplicatesEx: [], gitignoreIssue: hasGitignoreIssue ? { reason: 'not-ignored' } : null, }, hasGitignoreIssue, diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index 048bebf4..e2fe6f06 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -18,7 +18,6 @@ import { DEFAULT_EXAMPLE_FILE } from '../config/constants.js'; import type { ScanUsageOptions, ScanResult, - DuplicateResult, UppercaseWarning, Duplicate, ComparisonFile, @@ -39,10 +38,8 @@ export interface ProcessComparisonResult { 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 duplicate keys found in the comparison file */ + duplicates: 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 */ @@ -77,8 +74,7 @@ export function processComparisonFile( ): ProcessComparisonResult { let envVariables: Record = {}; let comparedAgainst = ''; - let dupsEnv: Duplicate[] = []; - let dupsEx: Duplicate[] = []; + let duplicates: Duplicate[] = []; let exampleFull: Record | undefined = undefined; let exampleFile: string | undefined = undefined; let uppercaseWarnings: UppercaseWarning[] = []; @@ -163,9 +159,7 @@ export function processComparisonFile( // Find duplicates if (!opts.allowDuplicates) { - const duplicateResults = checkDuplicates(compareFile, opts); - dupsEnv = duplicateResults.dupsEnv; - dupsEx = duplicateResults.dupsEx; + duplicates = checkDuplicates(compareFile, opts); } if (opts.expireWarnings) { @@ -212,7 +206,7 @@ export function processComparisonFile( const { changed, result } = applyFixes({ envPath: compareFile.path, missingKeys: scanResult.missing, - duplicateKeys: dupsEnv.map((d) => d.key), + duplicateKeys: duplicates.map((d) => d.key), ensureGitignore: true, }); @@ -225,19 +219,15 @@ export function processComparisonFile( // clear the issues that were fixed scanResult.missing = []; - dupsEnv = []; - dupsEx = []; + duplicates = []; } } - // Keep duplicates for output if not fixed - if ( - (dupsEnv.length > 0 || dupsEx.length > 0) && - (!opts.fix || !fix.fixApplied) - ) { - if (!scanResult.duplicates) scanResult.duplicates = {}; - if (dupsEnv.length > 0) scanResult.duplicates.env = dupsEnv; - if (dupsEx.length > 0) scanResult.duplicates.example = dupsEx; + // Keep duplicates for output if not fixed. They are always reported against + // the file the scan actually read, which is not necessarily `.env` — with + // `--example` it is the example file itself. + if (duplicates.length > 0 && (!opts.fix || !fix.fixApplied)) { + scanResult.duplicates = { file: compareFile.name, keys: duplicates }; } } catch (error) { const errorMessage = `Could not read ${compareFile.name}: ${compareFile.path} - ${error}`; @@ -245,8 +235,7 @@ export function processComparisonFile( scanResult, envVariables, comparedAgainst, - dupsEnv, - dupsEx, + duplicates, fix, exampleFull, exampleFile, @@ -266,8 +255,7 @@ export function processComparisonFile( scanResult, envVariables, comparedAgainst, - dupsEnv, - dupsEx, + duplicates, fix, exampleFull, exampleFile, @@ -280,38 +268,23 @@ export function processComparisonFile( } /** - * Check for duplicate keys in env and example files + * Check for duplicate keys in the file the scan is comparing against. + * + * A scan reads exactly one file, so there is only one place duplicates can come + * from. Attributing them to an env/example pair — the way `--compare` does — is + * what made the same finding show up under two different names. * @param compareFile - The file to compare against * @param opts - Scan options - * @returns Object containing duplicate keys in env and example files + * @returns Duplicate keys found in the comparison file */ function checkDuplicates( compareFile: ComparisonFile, opts: ScanUsageOptions, -): DuplicateResult { +): Duplicate[] { const isIgnored = (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 }) => + return findDuplicateKeys(compareFile.path).filter(({ key }) => isIgnored(key), ); - - // Duplicates in example file - let dupsEx: Duplicate[] = []; - - if (opts.examplePath) { - const examplePath = resolveFromCwd(opts.cwd, opts.examplePath); - - const exampleIsDifferentFile = - fs.existsSync(examplePath) && examplePath !== compareFile.path; - - if (exampleIsDifferentFile) { - dupsEx = findDuplicateKeys(examplePath).filter(({ key }) => - isIgnored(key), - ); - } - } - - return { dupsEnv, dupsEx } satisfies DuplicateResult; } diff --git a/packages/cli/src/services/scanCodebase.ts b/packages/cli/src/services/scanCodebase.ts index ed72b85a..4ce3d9a9 100644 --- a/packages/cli/src/services/scanCodebase.ts +++ b/packages/cli/src/services/scanCodebase.ts @@ -90,10 +90,7 @@ export async function scanCodebase(opts: ScanOptions): Promise { warningsCount: 0, duration: 0, }, - duplicates: { - env: [], - example: [], - }, + duplicates: { keys: [] }, logged: loggedVariables, fileContentMap, }; diff --git a/packages/cli/src/ui/scan/printScanDuplicates.ts b/packages/cli/src/ui/scan/printScanDuplicates.ts new file mode 100644 index 00000000..5966eb7a --- /dev/null +++ b/packages/cli/src/ui/scan/printScanDuplicates.ts @@ -0,0 +1,45 @@ +import { + label, + value, + warning, + error, + divider, + header, + padLabel, +} from '../theme.js'; +import type { Duplicate } from '../../config/types.js'; + +/** + * Prints duplicate keys found by a scan. + * + * Unlike the compare-mode printer this takes a single file, because a scan only + * ever reads one — the header therefore always names the file the keys came + * from, matching the `duplicates.file` field in `--json` output. + * @param file The name of the file the duplicates were found in. + * @param duplicates The duplicate keys with their occurrence counts. + * @param json Whether output is in JSON format (nothing is printed then). + * @param fix Whether fix mode is enabled (skips printing, as they will be fixed). + * @param strict Whether strict mode is enabled. + * @returns void + */ +export function printScanDuplicates( + file: string, + duplicates: Duplicate[], + json: boolean, + fix: boolean = false, + strict: boolean = false, +): void { + if (json || fix || duplicates.length === 0) return; + + const indicator = strict ? error('▸') : warning('▸'); + + console.log(); + console.log(`${indicator} ${header(`Duplicate keys in ${file}`)}`); + console.log(`${divider}`); + + for (const { key, count } of duplicates) { + console.log(`${label(padLabel(key))}${value(`${count} occurrences`)}`); + } + + console.log(`${divider}`); +} diff --git a/packages/cli/src/ui/scan/scanJsonOutput.ts b/packages/cli/src/ui/scan/scanJsonOutput.ts index 4c4dd8c6..efea48bd 100644 --- a/packages/cli/src/ui/scan/scanJsonOutput.ts +++ b/packages/cli/src/ui/scan/scanJsonOutput.ts @@ -49,8 +49,10 @@ interface ScanJsonOutput { snippet: string; }>; duplicates?: { - env?: Duplicate[]; - example?: Duplicate[]; + /** The file the duplicate keys were found in. */ + file: string; + /** The duplicated keys with their occurrence counts. */ + keys: Duplicate[]; }; gitignoreIssue?: { reason: GitignoreIssue }; logged?: EnvUsage[]; @@ -169,12 +171,13 @@ export function scanJsonOutput( output.unused = scanResult.unused; } - const hasDuplicates = - (scanResult.duplicates.env?.length ?? 0) > 0 || - (scanResult.duplicates.example?.length ?? 0) > 0; + const duplicateKeys = scanResult.duplicates.keys ?? []; - if (hasDuplicates) { - output.duplicates = scanResult.duplicates; + if (duplicateKeys.length > 0) { + output.duplicates = { + file: scanResult.duplicates.file ?? comparedAgainst, + keys: duplicateKeys, + }; } if (gitignoreIssue) { diff --git a/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts b/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts index b9500013..6d2664e2 100644 --- a/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts +++ b/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts @@ -27,8 +27,7 @@ interface Counts { example: number; expire: number; inconsistent: number; - dupEnv: number; - dupExample: number; + dup: number; } const fill = (n: number) => Array.from({ length: n }, () => ({})); @@ -49,7 +48,7 @@ function buildScan(c: Counts): ScanResult { ...Array.from({ length: c.medSecrets }, () => ({ severity: 'medium' })), ...Array.from({ length: c.lowSecrets }, () => ({ severity: 'low' })), ], - duplicates: { env: fill(c.dupEnv), example: fill(c.dupExample) }, + duplicates: { file: '.env', keys: fill(c.dup) }, } as unknown as ScanResult; } @@ -67,8 +66,7 @@ function expectedScore(c: Counts): number { c.example * 10 - c.expire * 5 - c.inconsistent * 3 - - c.dupEnv * 10 - - c.dupExample * 10; + c.dup * 10; return Math.max(0, Math.min(100, raw)); } @@ -84,8 +82,7 @@ const countsArb: fc.Arbitrary = fc.record({ example: fc.nat({ max: 8 }), expire: fc.nat({ max: 8 }), inconsistent: fc.nat({ max: 8 }), - dupEnv: fc.nat({ max: 8 }), - dupExample: fc.nat({ max: 8 }), + dup: fc.nat({ max: 8 }), }); describe('computeHealthScore (property-based)', () => { @@ -123,8 +120,7 @@ describe('computeHealthScore (property-based)', () => { example: 0, expire: 0, inconsistent: 0, - dupEnv: 0, - dupExample: 0, + dup: 0, }; expect(computeHealthScore(buildScan(clean))).toBe(100); }); @@ -142,8 +138,7 @@ describe('computeHealthScore (property-based)', () => { example: fc.nat({ max: 5 }), expire: fc.nat({ max: 5 }), inconsistent: fc.nat({ max: 5 }), - dupEnv: fc.nat({ max: 5 }), - dupExample: fc.nat({ max: 5 }), + dup: fc.nat({ max: 5 }), }); fc.assert( fc.property(countsArb, nonNeg, (base, extra) => { diff --git a/packages/cli/test/unit/baseline/scanBaseline.test.ts b/packages/cli/test/unit/baseline/scanBaseline.test.ts index 8a4f3fab..4f489ae4 100644 --- a/packages/cli/test/unit/baseline/scanBaseline.test.ts +++ b/packages/cli/test/unit/baseline/scanBaseline.test.ts @@ -96,6 +96,48 @@ describe('loadBaselineFile', () => { fs.writeFileSync(path.join(tmpDir, BASELINE_FILE), '"just a string"'); expect(loadBaselineFile(tmpDir)).toBeNull(); }); + + it('migrates duplicate rules written by older versions', () => { + fs.writeFileSync( + path.join(tmpDir, BASELINE_FILE), + JSON.stringify({ + version: 1, + createdAt: '2024-01-01', + entries: [ + { rule: 'duplicate-env', key: 'A' }, + { rule: 'duplicate-example', key: 'B' }, + { rule: 'missing', key: 'C' }, + ], + }), + ); + + expect(loadBaselineFile(tmpDir)!.entries).toEqual([ + { rule: 'duplicate', key: 'A' }, + { rule: 'duplicate', key: 'B' }, + { rule: 'missing', key: 'C' }, + ]); + }); + + it('keeps suppressing a duplicate recorded under the old rule name', () => { + fs.writeFileSync( + path.join(tmpDir, BASELINE_FILE), + JSON.stringify({ + version: 1, + createdAt: '2024-01-01', + entries: [{ rule: 'duplicate-env', key: 'DUP' }], + }), + ); + + const after = applyBaselineEntries( + { + ...emptyScanResult, + duplicates: { file: '.env.example', keys: [{ key: 'DUP', count: 2 }] }, + }, + loadBaselineFile(tmpDir)!.entries, + ); + + expect(after.duplicates.keys).toHaveLength(0); + }); }); // --------------------------------------------------------------------------- @@ -218,20 +260,12 @@ describe('collectBaselineEntries', () => { expect(result).toContainEqual({ rule: 'example-secret', key: 'DB_PASS' }); }); - it('collects env duplicate keys', () => { - const result = collectBaselineEntries({ - ...emptyScanResult, - duplicates: { env: [{ key: 'DUP', count: 2 }] }, - }); - expect(result).toContainEqual({ rule: 'duplicate-env', key: 'DUP' }); - }); - - it('collects example duplicate keys', () => { + it('collects duplicate keys', () => { const result = collectBaselineEntries({ ...emptyScanResult, - duplicates: { example: [{ key: 'EX_DUP', count: 2 }] }, + duplicates: { file: '.env', keys: [{ key: 'DUP', count: 2 }] }, }); - expect(result).toContainEqual({ rule: 'duplicate-example', key: 'EX_DUP' }); + expect(result).toContainEqual({ rule: 'duplicate', key: 'DUP' }); }); it('collects framework warnings with file', () => { @@ -468,31 +502,21 @@ describe('applyBaselineEntries', () => { expect(after.secrets).toHaveLength(1); }); - it('suppresses duplicate-env key', () => { - const result: ScanResult = { - ...emptyScanResult, - duplicates: { env: [{ key: 'DUP', count: 2 }] }, - }; - const entries: BaselineEntry[] = [{ rule: 'duplicate-env', key: 'DUP' }]; - const after = applyBaselineEntries(result, entries); - expect(after.duplicates.env).toHaveLength(0); - }); - - it('suppresses duplicate-example key', () => { + it('suppresses duplicate key', () => { const result: ScanResult = { ...emptyScanResult, - duplicates: { example: [{ key: 'EX', count: 2 }] }, + duplicates: { file: '.env', keys: [{ key: 'DUP', count: 2 }] }, }; - const entries: BaselineEntry[] = [{ rule: 'duplicate-example', key: 'EX' }]; + const entries: BaselineEntry[] = [{ rule: 'duplicate', key: 'DUP' }]; const after = applyBaselineEntries(result, entries); - expect(after.duplicates.example).toHaveLength(0); + expect(after.duplicates.keys).toHaveLength(0); }); it('leaves duplicates undefined when scanResult has no duplicates', () => { const result: ScanResult = { ...emptyScanResult, duplicates: {} }; const after = applyBaselineEntries(result, []); - expect(after.duplicates.env).toBeUndefined(); - expect(after.duplicates.example).toBeUndefined(); + expect(after.duplicates.keys).toBeUndefined(); + expect(after.duplicates.file).toBeUndefined(); }); it('suppresses example-secret warning', () => { @@ -714,8 +738,8 @@ describe('applyBaselineEntries', () => { }, ], duplicates: { - env: [{ key: 'D', count: 2 }], - example: [{ key: 'E', count: 2 }], + file: '.env', + keys: [{ key: 'D', count: 2 }], }, frameworkWarnings: [ { @@ -741,8 +765,7 @@ describe('applyBaselineEntries', () => { expect(after.logged).toHaveLength(0); expect(after.secrets).toHaveLength(0); expect(after.exampleWarnings).toHaveLength(0); - expect(after.duplicates.env).toHaveLength(0); - expect(after.duplicates.example).toHaveLength(0); + expect(after.duplicates.keys).toHaveLength(0); expect(after.frameworkWarnings).toHaveLength(0); expect(after.uppercaseWarnings).toHaveLength(0); expect(after.expireWarnings).toHaveLength(0); diff --git a/packages/cli/test/unit/commands/scanUsage.test.ts b/packages/cli/test/unit/commands/scanUsage.test.ts index 5a8484b5..3e9d5ed5 100644 --- a/packages/cli/test/unit/commands/scanUsage.test.ts +++ b/packages/cli/test/unit/commands/scanUsage.test.ts @@ -330,8 +330,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], fix: { fixApplied: false, removedDuplicates: [], @@ -356,8 +355,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], fix: { fixApplied: false, removedDuplicates: [], @@ -390,8 +388,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], fix: { fixApplied: false, removedDuplicates: [], @@ -464,8 +461,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], fix: { fixApplied: false, removedDuplicates: [], @@ -510,8 +506,7 @@ describe('scanUsage', () => { comparedAgainst: DEFAULT_EXAMPLE_FILE, envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], exampleFull: { SECRET: 'abc123' }, fix: { fixApplied: false, @@ -543,8 +538,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], exampleFull: { SECRET: 'abc123' }, fix: { fixApplied: false, @@ -574,8 +568,7 @@ describe('scanUsage', () => { comparedAgainst: '.env', envVariables: {}, duplicatesFound: false, - dupsEnv: [], - dupsEx: [], + duplicates: [], exampleFull: undefined, fix: { fixApplied: false, @@ -683,8 +676,7 @@ describe('scanUsage', () => { it('returns exitWithError true in JSON strict mode for each warning type', async () => { const cases: Partial[] = [ - { duplicates: { env: [{ key: 'A', count: 2 }] } }, - { duplicates: { example: [{ key: 'B', count: 2 }] } }, + { duplicates: { file: '.env', keys: [{ key: 'A', count: 2 }] } }, { secrets: [ { diff --git a/packages/cli/test/unit/core/scan/computeExitDecision.test.ts b/packages/cli/test/unit/core/scan/computeExitDecision.test.ts index 80c6e84b..07850592 100644 --- a/packages/cli/test/unit/core/scan/computeExitDecision.test.ts +++ b/packages/cli/test/unit/core/scan/computeExitDecision.test.ts @@ -95,10 +95,9 @@ describe('computeExitDecision', () => { describe('strict violations', () => { const cases: Array<[string, Partial]> = [ ['unused keys', { unused: ['OLD_KEY'] }], - ['duplicate env keys', { duplicates: { env: [{ key: 'K', count: 2 }] } }], [ - 'duplicate example keys', - { duplicates: { example: [{ key: 'K', count: 2 }] } }, + 'duplicate keys', + { duplicates: { file: '.env', keys: [{ key: 'K', count: 2 }] } }, ], [ 'medium severity secrets', diff --git a/packages/cli/test/unit/services/printScanResult.test.ts b/packages/cli/test/unit/services/printScanResult.test.ts index 0c8b5a53..372df8ad 100644 --- a/packages/cli/test/unit/services/printScanResult.test.ts +++ b/packages/cli/test/unit/services/printScanResult.test.ts @@ -20,8 +20,8 @@ vi.mock('../../../src/ui/scan/printUnused.js', () => ({ printUnused: vi.fn(), })); -vi.mock('../../../src/ui/shared/printDuplicates.js', () => ({ - printDuplicates: vi.fn(), +vi.mock('../../../src/ui/scan/printScanDuplicates.js', () => ({ + printScanDuplicates: vi.fn(), })); vi.mock('../../../src/ui/scan/printSecrets.js', () => ({ @@ -102,7 +102,7 @@ import { printAutoFix } from '../../../src/ui/shared/printAutoFix.js'; import { checkGitignoreStatus } from '../../../src/services/git.js'; import { printGitignoreWarning } from '../../../src/ui/shared/printGitignore.js'; import { printStats } from '../../../src/ui/scan/printStats.js'; -import { printDuplicates } from '../../../src/ui/shared/printDuplicates.js'; +import { printScanDuplicates } from '../../../src/ui/scan/printScanDuplicates.js'; import { printUnused } from '../../../src/ui/scan/printUnused.js'; import { printFrameworkWarnings } from '../../../src/ui/scan/printFrameworkWarnings.js'; import { printUppercaseWarning } from '../../../src/ui/scan/printUppercaseWarning.js'; @@ -447,10 +447,8 @@ describe('printScanResult', () => { '', ); - expect(printDuplicates).toHaveBeenCalledWith( + expect(printScanDuplicates).toHaveBeenCalledWith( '.env', - 'example file', - [], [], false, false, @@ -458,6 +456,28 @@ describe('printScanResult', () => { ); }); + it('names the file the duplicates were actually found in', () => { + printScanResult( + { + ...baseScanResult, + duplicates: { + file: '.env.example', + keys: [{ key: 'FEFOEOF', count: 2 }], + }, + }, + baseOpts, + '.env.example', + ); + + expect(printScanDuplicates).toHaveBeenCalledWith( + '.env.example', + [{ key: 'FEFOEOF', count: 2 }], + false, + false, + undefined, + ); + }); + it('does not print console log warning when logged is undefined', () => { printScanResult( { diff --git a/packages/cli/test/unit/services/processComparisonFile.test.ts b/packages/cli/test/unit/services/processComparisonFile.test.ts index 85be982a..0816c2c8 100644 --- a/packages/cli/test/unit/services/processComparisonFile.test.ts +++ b/packages/cli/test/unit/services/processComparisonFile.test.ts @@ -221,7 +221,7 @@ describe('processComparisonFile', () => { allowDuplicates: false, }); - expect(result.dupsEnv.length).toBeGreaterThan(0); + expect(result.duplicates.length).toBeGreaterThan(0); }); it('detects expire warnings', () => { @@ -268,7 +268,7 @@ describe('processComparisonFile', () => { allowDuplicates: false, }); - expect(result.scanResult.duplicates?.env).toBeDefined(); + expect(result.scanResult.duplicates?.keys).toBeDefined(); }); it('Will Load .env.example trough examplePath', () => { @@ -320,20 +320,23 @@ describe('processComparisonFile', () => { expect(result.exampleFull).toEqual({ A: '1', bKey: '2' }); }); - it('skips example duplicate check when examplePath equals compareFile path', async () => { - // resolveFromCwd returns the compareFile path → same file → skip - const { resolveFromCwd } = - await import('../../../src/core/helpers/resolveFromCwd.js'); - vi.mocked(resolveFromCwd).mockReturnValue(compareFile.path); + it('reports duplicates once, against the file the scan actually read', () => { + const exampleFile: ComparisonFile = { + path: '/env/.env.example', + name: '.env.example', + }; - const result = processComparisonFile(baseScanResult, compareFile, { - ...baseOpts, - allowDuplicates: false, - examplePath: '.env.example', - }); + const result = processComparisonFile( + { ...baseScanResult, duplicates: {} }, + exampleFile, + { ...baseOpts, allowDuplicates: false, examplePath: '.env.example' }, + ); - // dupsEnv still found, but dupsEx should be empty because same file - expect(result.dupsEx).toHaveLength(0); + // The example file IS the comparison file here, so its duplicates must be + // reported under that name — not counted a second time as "env" duplicates. + expect(findDuplicateKeys).toHaveBeenCalledTimes(1); + expect(result.scanResult.duplicates.file).toBe('.env.example'); + expect(result.scanResult.duplicates.keys).toHaveLength(1); }); it('does not clear state when fix returns changed=false', () => { @@ -354,7 +357,7 @@ describe('processComparisonFile', () => { expect(result.fix.fixApplied).toBe(false); // duplicates should still be present on scanResult - expect(result.scanResult.duplicates?.env).toBeDefined(); + expect(result.scanResult.duplicates?.keys).toBeDefined(); }); it('does not set exampleFull when example file does not exist on disk (line 74)', () => { @@ -374,7 +377,7 @@ describe('processComparisonFile', () => { }); // The duplicate key ('A') doesn't match the regex, so it's kept. - expect(result.dupsEnv.length).toBeGreaterThan(0); + expect(result.duplicates.length).toBeGreaterThan(0); }); it('skips duplicate check when allowDuplicates is true (lines 105-109)', () => { @@ -383,15 +386,11 @@ describe('processComparisonFile', () => { allowDuplicates: true, }); - expect(result.dupsEnv).toHaveLength(0); - expect(result.dupsEx).toHaveLength(0); + expect(result.duplicates).toHaveLength(0); }); - it('sets duplicatesFound via dupsEx when only example file has duplicates (lines 109, 154)', () => { - // First call: env file → no duplicates. Second call: example file → has duplicate. - vi.mocked(findDuplicateKeys) - .mockReturnValueOnce([]) - .mockReturnValueOnce([{ key: 'EX_KEY', count: 2 }]); + it('leaves duplicates unset when the comparison file has none', () => { + vi.mocked(findDuplicateKeys).mockReturnValueOnce([]); // Use a fresh duplicates object to avoid mutation from previous tests const result = processComparisonFile( @@ -400,12 +399,9 @@ describe('processComparisonFile', () => { { ...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.duplicates).toHaveLength(0); + expect(result.scanResult.duplicates?.keys).toBeUndefined(); + expect(result.scanResult.duplicates?.file).toBeUndefined(); }); it('uses empty exampleKeysList when exampleFull is undefined in inconsistent naming check (line 119)', () => { @@ -435,6 +431,6 @@ describe('processComparisonFile', () => { }); expect(result.scanResult.duplicates).toBeDefined(); - expect(result.scanResult.duplicates?.env).toBeDefined(); + expect(result.scanResult.duplicates?.keys).toBeDefined(); }); }); diff --git a/packages/cli/test/unit/ui/scan/printScanDuplicates.test.ts b/packages/cli/test/unit/ui/scan/printScanDuplicates.test.ts new file mode 100644 index 00000000..17fc9137 --- /dev/null +++ b/packages/cli/test/unit/ui/scan/printScanDuplicates.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { printScanDuplicates } from '../../../../src/ui/scan/printScanDuplicates.js'; + +vi.mock('../../../../src/ui/theme.js', () => ({ + UI_LABEL_WIDTH: 28, + padLabel: (text: string) => text, + label: (text: string) => `L(${text})`, + value: (text: string) => `V(${text})`, + warning: (text: string) => `W(${text})`, + error: (text: string) => `E(${text})`, + divider: '---', + header: (text: string) => `H(${text})`, +})); + +describe('printScanDuplicates', () => { + let logSpy: ReturnType; + + beforeEach(() => { + logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('does nothing when json is true', () => { + printScanDuplicates('.env', [{ key: 'A', count: 2 }], true); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('does nothing when fix is true', () => { + printScanDuplicates('.env', [{ key: 'A', count: 2 }], false, true); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('does nothing when there are no duplicates', () => { + printScanDuplicates('.env', [], false); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('names the file the keys were found in', () => { + printScanDuplicates('.env.example', [{ key: 'FEFOEOF', count: 2 }], false); + + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('W(▸) H(Duplicate keys in .env.example)'), + ); + expect(logSpy).toHaveBeenCalledWith('L(FEFOEOF)V(2 occurrences)'); + }); + + it('prints every duplicate key', () => { + printScanDuplicates( + '.env', + [ + { key: 'A', count: 2 }, + { key: 'B', count: 3 }, + ], + false, + ); + + expect(logSpy).toHaveBeenCalledWith('L(A)V(2 occurrences)'); + expect(logSpy).toHaveBeenCalledWith('L(B)V(3 occurrences)'); + }); + + it('uses strict formatting when strict mode is enabled', () => { + printScanDuplicates('.env', [{ key: 'A', count: 2 }], false, false, true); + + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('E(▸) H(Duplicate keys in .env)'), + ); + }); +}); diff --git a/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts b/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts index d28d83b9..a232f9d2 100644 --- a/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts +++ b/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts @@ -238,24 +238,37 @@ describe('scanJsonOutput', () => { expect(result.frameworkWarnings?.[0]?.framework).toBe('sveltekit'); }); - it('includes duplicates when present', () => { + it('includes duplicates when present, named after the file they came from', () => { const scanResult = makeScanResult({ duplicates: { - env: [{ key: 'API_KEY', count: 2 }], - example: [{ key: 'DB_URL', count: 3 }], + file: '.env.example', + keys: [ + { key: 'API_KEY', count: 2 }, + { key: 'DB_URL', count: 3 }, + ], }, }); - const result = scanJsonOutput(scanResult, ''); + const result = scanJsonOutput(scanResult, '.env.example'); expect(result.duplicates).toBeDefined(); - expect(result.duplicates?.env).toHaveLength(1); - expect(result.duplicates?.example).toHaveLength(1); + expect(result.duplicates?.file).toBe('.env.example'); + expect(result.duplicates?.keys).toHaveLength(2); + }); + + it('falls back to comparedAgainst when the duplicates file is unset', () => { + const scanResult = makeScanResult({ + duplicates: { keys: [{ key: 'API_KEY', count: 2 }] }, + }); + + const result = scanJsonOutput(scanResult, '.env.local'); + + expect(result.duplicates?.file).toBe('.env.local'); }); it('omits duplicates when none exist', () => { const scanResult = makeScanResult({ - duplicates: { env: [], example: [] }, + duplicates: { keys: [] }, }); const result = scanJsonOutput(scanResult, '');