From 7e9a29874918ecb3e16b62b227ab22ad13496471 Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Wed, 13 May 2026 22:16:54 -0600 Subject: [PATCH] fix(dead-export-finder): resolve false negatives hiding all dead exports Three bugs caused the CLI to report zero dead exports: 1. `Options.repeated` + `Options.optional` returns `Some([])` not `None` when no `--packages` flag is given. The empty Set filter rejected all packages, so `graph.analyze` received an empty array and found nothing. 2. The file scanner only loaded `.gitignore` from the package root, not the workspace root. Since most monorepos have `dist/` in the root `.gitignore`, per-package scans included build artifacts. The compiled `.d.ts` files duplicated source exports, polluting results with false positives once the primary bug was fixed. 3. Config files (`vite.config.ts`, `eslint.config.mjs`, etc.) export `default` for tooling consumption, not for code. These were flagged as dead exports. Fixes: - Check `packagesOpt.value.length > 0` before creating the filter set - Walk from package root up to workspace root collecting `.gitignore` files - Add `DEFAULT_IGNORE` patterns for `*.config.{ts,mjs,cjs,js}` - Pass `workspace.root` through to `scanner.scan()` calls Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/fix-dead-export-false-negatives.md | 9 ++ packages/dead-export-finder/src/cli.ts | 7 +- .../src/lib/export-graph.test.ts | 107 +++++++++++++++++ .../src/lib/file-scanner.test.ts | 58 ++++++++++ .../src/lib/file-scanner.ts | 52 +++++++-- .../src/lib/integration.test.ts | 109 ++++++++++++++++++ 6 files changed, 328 insertions(+), 14 deletions(-) create mode 100644 .changeset/fix-dead-export-false-negatives.md diff --git a/.changeset/fix-dead-export-false-negatives.md b/.changeset/fix-dead-export-false-negatives.md new file mode 100644 index 0000000..8c6aa50 --- /dev/null +++ b/.changeset/fix-dead-export-false-negatives.md @@ -0,0 +1,9 @@ +--- +'@wolfcola/dead-export-finder': patch +--- + +fix(dead-export-finder): resolve false negatives hiding all dead exports + +- Fix empty package filter caused by `Options.repeated` + `Options.optional` returning `Some([])` instead of `None`, which silently filtered out all packages and reported zero dead exports +- Inherit `.gitignore` patterns from workspace root so `dist/` build artifacts are excluded from per-package scans +- Add default ignore patterns for config files (`*.config.{ts,mjs,cjs,js}`) that export for tooling, not for code diff --git a/packages/dead-export-finder/src/cli.ts b/packages/dead-export-finder/src/cli.ts index f401661..97dd38a 100644 --- a/packages/dead-export-finder/src/cli.ts +++ b/packages/dead-export-finder/src/cli.ts @@ -72,7 +72,7 @@ const scanWorkspace = ( workspace.packages, Arr.map((pkg) => pipe( - scanner.scan(pkg.root, ignoreGlobs), + scanner.scan(pkg.root, ignoreGlobs, workspace.root), Effect.catchTag('GlobError', (e) => Effect.gen(function* () { const msg = `failed to scan files in ${pkg.root}: ${String(e.cause)}`; @@ -279,7 +279,10 @@ const command = Command.make( yield* Console.log(`Found ${workspace.packages.length} packages`); } - const packageFilter = packagesOpt._tag === 'Some' ? new Set(packagesOpt.value) : null; + const packageFilter = + packagesOpt._tag === 'Some' && packagesOpt.value.length > 0 + ? new Set(packagesOpt.value) + : null; const targetPackages = packageFilter !== null diff --git a/packages/dead-export-finder/src/lib/export-graph.test.ts b/packages/dead-export-finder/src/lib/export-graph.test.ts index 5df79bd..c655f0f 100644 --- a/packages/dead-export-finder/src/lib/export-graph.test.ts +++ b/packages/dead-export-finder/src/lib/export-graph.test.ts @@ -222,6 +222,113 @@ it.effect('package specifier re-export does not crash or create incorrect edges' }), ); +it.effect('empty packages array produces zero dead exports', () => + Effect.gen(function* () { + // When no packages are provided (the bug that occurred when Options.repeated + // returned Some([]) instead of None), every file is unmappable and silently + // skipped, producing zero dead exports even though dead code exists. + const allExports = new Map([ + ['/test/utils/src/index.ts', [makeExport('publicFn', '/test/utils/src/index.ts')]], + ['/test/utils/src/internal.ts', [makeExport('helperFn', '/test/utils/src/internal.ts')]], + ]); + + const allImports = new Map(); + + const result = yield* analyze([], allExports, allImports); + + // With no packages, nothing can be attributed → nothing flagged + expect(result.deadExports).toHaveLength(0); + // But exports are still counted + expect(result.totalExports).toBe(2); + }), +); + +it.effect('file not covered by star re-export is flagged as dead', () => + Effect.gen(function* () { + // index.ts re-exports everything from ./lib/used via export *, but + // ./lib/orphan.ts is never re-exported or imported by anyone. + const pkg = makePackage('@test/utils', '/test/utils', ['./src/index.ts']); + + const allExports = new Map([ + [ + '/test/utils/src/index.ts', + [makeExport('*', '/test/utils/src/index.ts', 1, false, true, './lib/used')], + ], + [ + '/test/utils/src/lib/used.ts', + [ + makeExport('a', '/test/utils/src/lib/used.ts'), + makeExport('b', '/test/utils/src/lib/used.ts'), + ], + ], + ['/test/utils/src/lib/orphan.ts', [makeExport('dead', '/test/utils/src/lib/orphan.ts')]], + ]); + + const allImports = new Map(); + + const result = yield* analyze([pkg], allExports, allImports); + + const deadNames = result.deadExports.map((d) => d.symbol.name); + // a and b are protected by the star re-export + expect(deadNames).not.toContain('a'); + expect(deadNames).not.toContain('b'); + // dead is in a file nobody re-exports or imports + expect(deadNames).toContain('dead'); + expect(result.deadExports).toHaveLength(1); + }), +); + +it.effect('cross-package named import marks export as consumed', () => + Effect.gen(function* () { + // @test/app imports { helperFn } from '@test/utils' by package name. + // helperFn lives in a non-entry-point file, but the package-specifier + // import should mark it as consumed. + const utilsPkg = makePackage('@test/utils', '/test/utils', ['./src/index.ts']); + const appPkg = makePackage('@test/app', '/test/app', ['./src/index.ts']); + + const allExports = new Map([ + ['/test/utils/src/index.ts', []], + ['/test/utils/src/internal.ts', [makeExport('helperFn', '/test/utils/src/internal.ts')]], + ['/test/app/src/index.ts', []], + ]); + + const allImports = new Map([ + ['/test/app/src/index.ts', [makeImport('helperFn', '/test/app/src/index.ts', '@test/utils')]], + ]); + + const result = yield* analyze([utilsPkg, appPkg], allExports, allImports); + + const deadNames = result.deadExports.map((d) => d.symbol.name); + expect(deadNames).not.toContain('helperFn'); + }), +); + +it.effect('cross-package named import does not protect unrelated exports', () => + Effect.gen(function* () { + // @test/app imports { used } from '@test/utils', but @test/utils also + // exports { unused } from a different file. Only unused should be dead. + const utilsPkg = makePackage('@test/utils', '/test/utils', ['./src/index.ts']); + const appPkg = makePackage('@test/app', '/test/app', ['./src/index.ts']); + + const allExports = new Map([ + ['/test/utils/src/index.ts', []], + ['/test/utils/src/used.ts', [makeExport('used', '/test/utils/src/used.ts')]], + ['/test/utils/src/unused.ts', [makeExport('unused', '/test/utils/src/unused.ts')]], + ['/test/app/src/index.ts', []], + ]); + + const allImports = new Map([ + ['/test/app/src/index.ts', [makeImport('used', '/test/app/src/index.ts', '@test/utils')]], + ]); + + const result = yield* analyze([utilsPkg, appPkg], allExports, allImports); + + const deadNames = result.deadExports.map((d) => d.symbol.name); + expect(deadNames).not.toContain('used'); + expect(deadNames).toContain('unused'); + }), +); + it.effect('silently skips files that cannot be attributed to any package', () => Effect.gen(function* () { const pkg = makePackage('@test/utils', '/test/utils', ['./src/index.ts']); diff --git a/packages/dead-export-finder/src/lib/file-scanner.test.ts b/packages/dead-export-finder/src/lib/file-scanner.test.ts index 66b71cf..f5f9875 100644 --- a/packages/dead-export-finder/src/lib/file-scanner.test.ts +++ b/packages/dead-export-finder/src/lib/file-scanner.test.ts @@ -112,6 +112,64 @@ layer(NodeContext.layer)('FileScanner', (it) => { ), ); + it.scoped('inherits .gitignore from workspace root', () => + withScanner( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tmpDir = yield* fs.makeTempDirectoryScoped(); + + // Workspace root has .gitignore excluding dist/ + yield* fs.writeFileString(path.join(tmpDir, '.gitignore'), 'dist/\n'); + + // Package is nested under packages/my-lib + const pkgDir = path.join(tmpDir, 'packages', 'my-lib'); + const srcDir = path.join(pkgDir, 'src'); + const distDir = path.join(pkgDir, 'dist'); + yield* fs.makeDirectory(srcDir, { recursive: true }); + yield* fs.makeDirectory(distDir, { recursive: true }); + + yield* fs.writeFileString(path.join(srcDir, 'index.ts'), ''); + yield* fs.writeFileString(path.join(distDir, 'index.js'), ''); + yield* fs.writeFileString(path.join(distDir, 'index.d.ts'), ''); + + const scanner = yield* FileScanner; + // Pass workspaceRoot so parent .gitignore is found + const files = yield* scanner.scan(pkgDir, [], tmpDir); + + const names = files.map((f) => path.basename(f)); + expect(names).toContain('index.ts'); + expect(names).not.toContain('index.js'); + expect(names).not.toContain('index.d.ts'); + }), + ), + ); + + it.scoped('excludes config files by default', () => + withScanner( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tmpDir = yield* fs.makeTempDirectoryScoped(); + + yield* fs.writeFileString(path.join(tmpDir, 'index.ts'), ''); + yield* fs.writeFileString(path.join(tmpDir, 'vite.config.ts'), ''); + yield* fs.writeFileString(path.join(tmpDir, 'eslint.config.mjs'), ''); + yield* fs.writeFileString(path.join(tmpDir, 'vitest.config.ts'), ''); + yield* fs.writeFileString(path.join(tmpDir, 'tsconfig.json'), ''); + + const scanner = yield* FileScanner; + const files = yield* scanner.scan(tmpDir, []); + + const names = files.map((f) => path.basename(f)); + expect(names).toContain('index.ts'); + expect(names).not.toContain('vite.config.ts'); + expect(names).not.toContain('eslint.config.mjs'); + expect(names).not.toContain('vitest.config.ts'); + }), + ), + ); + it.scoped('respects custom ignore globs', () => withScanner( Effect.gen(function* () { diff --git a/packages/dead-export-finder/src/lib/file-scanner.ts b/packages/dead-export-finder/src/lib/file-scanner.ts index 2eef326..32cbe14 100644 --- a/packages/dead-export-finder/src/lib/file-scanner.ts +++ b/packages/dead-export-finder/src/lib/file-scanner.ts @@ -11,13 +11,11 @@ class GlobError extends Data.TaggedError('GlobError')<{ // ─── Pure helpers ──────────────────────────────────────────────────────────── -const loadGitignorePatterns = ( +const loadGitignoreAt = ( fs: FileSystem.FileSystem, - pathSvc: Path.Path, - root: string, -): Effect.Effect> => { - const gitignorePath = pathSvc.join(root, '.gitignore'); - return pipe( + gitignorePath: string, +): Effect.Effect> => + pipe( fs.exists(gitignorePath), Effect.orElseSucceed(() => false), Effect.flatMap((exists) => @@ -30,17 +28,45 @@ const loadGitignorePatterns = ( : Effect.succeed([] as ReadonlyArray), ), ); + +const loadGitignorePatterns = ( + fs: FileSystem.FileSystem, + pathSvc: Path.Path, + root: string, + workspaceRoot: string | undefined, +): Effect.Effect> => { + const dirs = [root]; + if (workspaceRoot !== undefined && workspaceRoot !== root) { + // Walk from package root up to workspace root, collecting .gitignore files + let current = pathSvc.dirname(root); + while (current.length >= workspaceRoot.length && current !== pathSvc.dirname(current)) { + dirs.push(current); + if (current === workspaceRoot) break; + current = pathSvc.dirname(current); + } + } + + return pipe( + dirs, + Arr.map((dir) => loadGitignoreAt(fs, pathSvc.join(dir, '.gitignore'))), + (effects) => Effect.all(effects), + Effect.map(Arr.flatten), + ); }; +const DEFAULT_IGNORE: ReadonlyArray = [ + 'node_modules', + '*.config.ts', + '*.config.mjs', + '*.config.cjs', + '*.config.js', +]; + const buildIgnorePatterns = ( gitignorePatterns: ReadonlyArray, customGlobs: readonly string[], ): ReadonlyArray => - pipe( - ['node_modules'] as ReadonlyArray, - Arr.appendAll(gitignorePatterns), - Arr.appendAll(customGlobs), - ); + pipe(DEFAULT_IGNORE, Arr.appendAll(gitignorePatterns), Arr.appendAll(customGlobs)); const discoverFiles = (root: string): Effect.Effect, GlobError> => Effect.tryPromise({ @@ -78,6 +104,7 @@ export interface FileScannerShape { readonly scan: ( root: string, ignoreGlobs: readonly string[], + workspaceRoot?: string, ) => Effect.Effect; } @@ -96,9 +123,10 @@ export const FileScannerLive = Layer.effect( const scan = ( root: string, ignoreGlobs: readonly string[], + workspaceRoot?: string, ): Effect.Effect => pipe( - loadGitignorePatterns(fs, pathSvc, root), + loadGitignorePatterns(fs, pathSvc, root, workspaceRoot), Effect.map((gitignorePatterns) => buildIgnorePatterns(gitignorePatterns, ignoreGlobs)), Effect.flatMap((patterns) => pipe( diff --git a/packages/dead-export-finder/src/lib/integration.test.ts b/packages/dead-export-finder/src/lib/integration.test.ts index d5bb919..a455740 100644 --- a/packages/dead-export-finder/src/lib/integration.test.ts +++ b/packages/dead-export-finder/src/lib/integration.test.ts @@ -152,4 +152,113 @@ layer(NodeContext.layer)('integration', (it) => { }).pipe(Effect.provide(ServicesLayer)), { timeout: 30_000 }, ); + + it.scoped( + 'end-to-end: flags export in file not covered by barrel re-export', + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + + const tmpDir = yield* fs.makeTempDirectoryScoped(); + + // Root: pnpm-workspace.yaml + yield* fs.writeFileString( + path.join(tmpDir, 'pnpm-workspace.yaml'), + 'packages:\n - "packages/*"\n', + ); + + // Package: @test/types — barrel re-exports from schema.ts but NOT orphan.ts + const typesDir = path.join(tmpDir, 'packages', 'types'); + const typesSrcDir = path.join(typesDir, 'src', 'lib'); + yield* fs.makeDirectory(typesSrcDir, { recursive: true }); + + yield* fs.writeFileString( + path.join(typesDir, 'package.json'), + JSON.stringify({ + name: '@test/types', + exports: { '.': './src/index.ts' }, + }), + ); + + yield* fs.writeFileString( + path.join(typesDir, 'src', 'index.ts'), + "export * from './lib/schema.js';\n", + ); + + yield* fs.writeFileString( + path.join(typesSrcDir, 'schema.ts'), + 'export const MySchema = {};\nexport type MyType = string;\n', + ); + + // orphan.ts exports something but is never imported or re-exported + yield* fs.writeFileString( + path.join(typesSrcDir, 'orphan.ts'), + 'export const DeadExport = {};\n', + ); + + // Package: @test/app — consumes @test/types + const appDir = path.join(tmpDir, 'packages', 'app'); + const appSrcDir = path.join(appDir, 'src'); + yield* fs.makeDirectory(appSrcDir, { recursive: true }); + + yield* fs.writeFileString( + path.join(appDir, 'package.json'), + JSON.stringify({ + name: '@test/app', + exports: { '.': './src/index.ts' }, + }), + ); + + yield* fs.writeFileString( + path.join(appSrcDir, 'index.ts'), + "import { MySchema } from '@test/types';\nexport const app = MySchema;\n", + ); + + // ── Run analysis ────────────────────────────────────────────────────── + + const detector = yield* WorkspaceDetector; + const scanner = yield* FileScanner; + const exportParser = yield* ExportParser; + const importParser = yield* ImportParser; + const graph = yield* ExportGraph; + + const workspace = yield* detector.detect(tmpDir); + + const allExports = new Map(); + const allImports = new Map(); + + for (const pkg of workspace.packages) { + const files = yield* scanner.scan(pkg.root, []); + for (const filePath of files) { + const source = yield* fs.readFileString(filePath, 'utf-8'); + const exports = yield* exportParser + .parse(filePath, source) + .pipe(Effect.catchTag('ParseError', () => Effect.succeed([]))); + const imports = yield* importParser + .parse(filePath, source) + .pipe(Effect.catchTag('ParseError', () => Effect.succeed([]))); + allExports.set(filePath, exports); + allImports.set(filePath, imports); + } + } + + // Analyze with ALL packages (the full-workspace scenario) + const result = yield* graph.analyze(workspace.packages, allExports, allImports); + + // ── Assertions ──────────────────────────────────────────────────────── + + const deadNames = result.deadExports.map((d) => d.symbol.name); + + // DeadExport is in orphan.ts which nobody imports or re-exports + expect(deadNames).toContain('DeadExport'); + + // MySchema is consumed by @test/app via package specifier + expect(deadNames).not.toContain('MySchema'); + + // MyType is protected by the star re-export from index.ts + expect(deadNames).not.toContain('MyType'); + }).pipe(Effect.provide(ServicesLayer)), + { timeout: 30_000 }, + ); });