Fix declaration export regression and document (resolve #1722)

webpro · webpro · commit 3a2c22b52cda · 2026-05-06T09:43:34.000+02:00
diff --git a/.agents/EXPORTS.md b/.agents/EXPORTS.md
@@ -1,55 +1,70 @@
 # Identifiers, Symbols & Matching
 
-How knip resolves "is this export used?" across an AST. Matching is name-based;
-the sections below cover the cases that need extra care.
+How knip resolves "is this export used?" across an AST. Matching is name-based.
 
 ## Shadow detection
 
-Name-based matching produces false negatives when a local binding shadows an
-exported name of the same spelling. `isShadowed(name, pos)` returns true if the
-reference at `pos` falls inside a scope that shadows `name`. Shadows are
-registered via:
+A local binding can shadow an exported name, producing false negatives.
+`isShadowed(name, pos)` returns true if the reference at `pos` falls inside a
+scope that shadows `name`. Shadows registered via:
 
-- `_addShadow`: block-scoped variables and nested function declarations
-  (uses current `scopeDepth` range from `BlockStatement`)
+- `_addShadow`: block-scoped variables and nested function declarations (uses
+  current `scopeDepth` range from `BlockStatement`)
 - `_addParamShadows`: function/arrow/method parameters (uses function body
   range directly, since params are visited before the body's `BlockStatement`)
 - `CatchClause` handler: catch binding (uses catch body range)
 - `ForInStatement`/`ForOfStatement` handlers: loop variable (uses loop body
   range, since the `VariableDeclaration` fires before the body's `BlockStatement`)
 
 `_collectBindingNames` recurses into destructuring patterns (ObjectPattern,
-ArrayPattern, AssignmentPattern, RestElement) to extract all bound Identifiers.
+ArrayPattern, AssignmentPattern, RestElement) to extract bound Identifiers.
 
 ## `ignoreExportsUsedInFile`
 
 Opt-in config (default `false`).
 
 **Default semantics match tsc/tsgo.** An export only referenced in its own file
-is reported as unused, because removing the `export` keyword leaves the program
-and `.d.ts` valid. Types are structurally inlined: a consumer importing
+is reported as unused; removing the `export` keyword leaves the program and
+`.d.ts` valid. Type aliases inline structurally: a consumer importing
 `UserInfo = { address: Address }` does not require `Address` to be exported.
-Same for `typeof X` references inside type aliases. So `Address` is correctly
-flagged. Opting `true` is a code-organization preference, not a correctness
-concern.
+Same for `typeof X` references inside type aliases. Opting `true` is a
+code-organization preference, not a correctness concern.
+
+**Does not apply to types in value signatures.** Types referenced in exported
+value signatures (function params/returns, variable annotations, `typeof` of an
+exported value) stay alive unconditionally. `tsc --declaration` cannot inline
+an interface or class type into a `.d.ts` and errors TS4023 if the name is not
+exported. That path is intentionally not gated by `ignoreExportsUsedInFile`.
 
 **With the config on.** `localRefsVisitorObject` populates `localRefs` during
 AST traversal. Exports present in `localRefs` get `hasRefsInFile = true`.
 `shouldCountRefs` gates eligible types. Computed member access
 (`obj[EXPORTED_KEY]`) is handled.
 
 `analyze.ts` reads this via `isReferencedInUsedExport` for exports not directly
-imported: returns true only when a containing export has `hasRefsInFile` and is
-a type/interface (recursively checked). Alive-ness does not cascade through
-external imports; inner refs stay scoped to the in-file relationship.
+imported: returns true when a containing export has `hasRefsInFile` and is a
+type/interface (recursively checked). Alive-ness does not cascade through
+external imports; chains stay in-file.
+
+## `referencedInExport` (chain refs)
+
+Maps exported identifier to set of export names whose declarations reference
+it. Populated for type aliases, interfaces (body and `extends`), and the
+signature parts of exported values: variable type annotations, function/arrow
+expression params and return types, function/method declarations.
+Function/arrow bodies are skipped via `signatureOnly`.
+
+`isReferencedInUsedExport` walks the chain, classifying each hop:
 
-## `referencedInExport` (type-chain refs)
+- **Type→type hop** (containing export is `type`/`interface`/`enum`):
+  propagates only when `ignoreExportsUsedInFile` is on. Otherwise the chain
+  breaks here, since tsc structurally inlines the inner type and its export is
+  removable.
+- **Type→value hop** (containing export is a function/class/variable):
+  propagates unconditionally. Needed for `tsc --declaration`; removing the
+  inner type's export would TS4023.
 
-Maps exported identifier → set of export names whose type annotations reference
-it. Type-level only, not function signatures. Type→type chains are followed;
-type→function chains do not keep types alive. Interface `extends` clauses
-captured via `addRefInExport`. Feeds the recursive type/interface check in
-`isReferencedInUsedExport` above.
+Interface `extends` clauses captured via `addRefInExport`.
 
 ## Namespace/enum member `hasRefsInFile`
 
@@ -75,14 +90,20 @@ If neither, reported as unused.
 
 **Edge case:** when a namespace/enum has export-level `hasRefsInFile = true`
 but is NOT externally imported, `analyze.ts` skips the member check entirely.
-Unused members silently pass. By design (the export itself is considered
-"used").
+Unused members silently pass. By design: the export itself is considered "used".
 
 ## E2E
 
-`packages/knip/test/e2e/fix-tsgo.test.ts` is the safety net for the resolution
-paths above. Each fixture builds clean under tsgo, has `knip --fix` run on it,
-then must build clean under tsgo again. A failing post-fix build means knip
-removed something tsc/tsgo still needs: a false positive in one of these
-mechanisms. The `e2e-lib-*` variants extend the round-trip to a consumer so
-type-visibility regressions also fail.
+`packages/knip/test/e2e/fix-tsgo.test.ts` covers the resolution paths above.
+Each fixture builds clean under tsgo, has `knip --fix` run on it, then must
+build clean under tsgo again. A failing post-fix build means knip removed
+something tsc/tsgo still needs: a false positive in one of these mechanisms.
+The `e2e-lib-*` variants extend the round-trip to a consumer with
+`declaration: true` so type-visibility regressions (TS4023 etc.) also fail.
+
+When adding a fixture for a value-to-type-visibility scenario, do not also
+re-export the type explicitly from the public entry. That path masks
+type-chain bugs: the type stays alive via the re-export regardless of whether
+knip tracks the value's signature. Test the implicit case (type only
+referenced in a function signature, never named at the entry) so the chain is
+the only thing keeping the export alive.
diff --git a/.github/workflows/snapshots/astro.txt b/.github/workflows/snapshots/astro.txt
@@ -1,9 +1,2 @@
 Unused exports (1)
 .flue/lib/github.ts: issueDetailsSchema, repoLabelSchema
-Unused exported types (6)
-packages/create-astro/test/test-utils.ts: Context
-packages/integrations/cloudflare/test/test-utils.ts: AstroInlineConfig
-packages/integrations/netlify/test/test-utils.ts: AstroInlineConfig
-packages/integrations/node/test/test-utils.ts: AstroInlineConfig
-packages/integrations/sitemap/test/test-utils.ts: AstroInlineConfig
-packages/integrations/vercel/test/test-utils.ts: VercelOutputConfig
diff --git a/packages/knip/fixtures/e2e-lib-public-surface/_index.ts b/packages/knip/fixtures/e2e-lib-public-surface/_index.ts
@@ -1,3 +1,5 @@
-import { pickFruit, type Fruit } from '@fixtures/e2e-lib-public-surface';
+import { pickFruit, fetchApi, type Fruit } from '@fixtures/e2e-lib-public-surface';
 const x: Fruit = pickFruit('apple');
+const r = fetchApi();
 x;
+r.status;
diff --git a/packages/knip/fixtures/e2e-lib-public-surface/src/index.ts b/packages/knip/fixtures/e2e-lib-public-surface/src/index.ts
@@ -1,2 +1,2 @@
 export type { Fruit } from './types.ts';
-export { pickFruit } from './types.ts';
+export { pickFruit, fetchApi } from './types.ts';
diff --git a/packages/knip/fixtures/e2e-lib-public-surface/src/types.ts b/packages/knip/fixtures/e2e-lib-public-surface/src/types.ts
@@ -8,3 +8,9 @@ export const pickFruit = (name: string): Fruit => ({ name, color: 'red' });
 export const internalUnused = 1;
 
 export type DraftFruit = { name: string };
+
+export interface ApiResult {
+  readonly status: 'ok';
+}
+
+export const fetchApi = (): ApiResult => ({ status: 'ok' });
diff --git a/packages/knip/src/graph/analyze.ts b/packages/knip/src/graph/analyze.ts
@@ -59,10 +59,14 @@ export const analyze = async ({
     for (const containingExport of exportedItem.referencedIn) {
       const inExport = file.exports.get(containingExport);
       if (!inExport) continue;
-      if (shouldCountRefs(ignoreExportsUsedInFile, inExport.type)) {
-        if (inExport.hasRefsInFile) return true;
-        if (explorer.isReferenced(filePath, containingExport, { includeEntryExports })[0]) return true;
+      if (
+        (inExport.type === 'type' || inExport.type === 'interface' || inExport.type === 'enum') &&
+        !shouldCountRefs(ignoreExportsUsedInFile, inExport.type)
+      ) {
+        continue;
       }
+      if (inExport.hasRefsInFile) return true;
+      if (explorer.isReferenced(filePath, containingExport, { includeEntryExports })[0]) return true;
       if (inExport.referencedIn) {
         const v = visited ?? new Set();
         if (!v.has(containingExport)) {
diff --git a/packages/knip/src/typescript/visitors/exports.ts b/packages/knip/src/typescript/visitors/exports.ts
@@ -140,6 +140,11 @@ export function handleExportNamed(node: ExportNamedDeclaration, s: WalkState) {
 
           s.addExport(name, SYMBOL_TYPE.UNKNOWN, declarator.id.start, [], fix, isReExport, jsDocTags);
 
+          if (declarator.id.typeAnnotation) s.collectRefsInType(declarator.id.typeAnnotation, name, true);
+          if (declarator.init?.type === 'ArrowFunctionExpression' || declarator.init?.type === 'FunctionExpression') {
+            s.collectRefsInType(declarator.init, name, true);
+          }
+
           if (!jsDocTags.has(ALIAS_TAG) && declarator.init?.type === 'Identifier') {
             const initName = declarator.init.name;
             const existingExport = s.exports.get(initName);
@@ -161,6 +166,7 @@ export function handleExportNamed(node: ExportNamedDeclaration, s: WalkState) {
     } else if ((decl.type === 'FunctionDeclaration' || decl.type === 'TSDeclareFunction') && decl.id) {
       const fix = s.getFix(exportStart, exportStart + 7);
       s.addExport(decl.id.name, SYMBOL_TYPE.FUNCTION, decl.id.start, [], fix, false, s.getJSDocTags(exportStart));
+      s.collectRefsInType(decl, decl.id.name, true);
     } else if (decl.type === 'ClassDeclaration' && decl.id) {
       const fix = s.getFix(exportStart, exportStart + 7);
       s.addExport(decl.id.name, SYMBOL_TYPE.CLASS, decl.id.start, [], fix, false, s.getJSDocTags(exportStart));
@@ -245,6 +251,7 @@ export function handleExportDefault(node: ExportDefaultDeclaration, s: WalkState
   if (decl.type === 'FunctionDeclaration') {
     type = SYMBOL_TYPE.FUNCTION;
     pos = decl.id?.start ?? decl.start;
+    s.collectRefsInType(decl, 'default', true);
   } else if (decl.type === 'ClassDeclaration') {
     type = SYMBOL_TYPE.CLASS;
     pos = decl.id?.start ?? decl.start;
diff --git a/packages/knip/test/ignore-exports-used-in-file-typeof-class.test.ts b/packages/knip/test/ignore-exports-used-in-file-typeof-class.test.ts
@@ -12,15 +12,15 @@ test('Ignore exports used in file for typeof function and class references', asy
   const { issues, counters } = await main(options);
 
   assert(issues.exports['src/api.ts']['TreeLeaf']);
-  assert(issues.exports['src/api.ts']['TreeNode']);
-  assert(issues.exports['src/api.ts']['logger']);
 
+  assert(!issues.exports['src/api.ts']?.['TreeNode']);
+  assert(!issues.exports['src/api.ts']?.['logger']);
   assert(!issues.exports['src/api.ts']?.['Leaf']);
   assert(!issues.exports['src/api.ts']?.['Node']);
 
   assert.deepEqual(counters, {
     ...baseCounters,
-    exports: 3,
+    exports: 1,
     processed: 2,
     total: 2,
   });
diff --git a/packages/knip/test/type-in-value-export.test.ts b/packages/knip/test/type-in-value-export.test.ts
@@ -12,12 +12,10 @@ test('Find unused types in value exports', async () => {
   const { issues, counters } = await main(options);
 
   assert(issues.types['src/api.ts']['GetPointsResponse']);
-  assert(issues.types['src/api.ts']['GetPoints']);
-  assert(issues.types['src/api.ts']['GetPointsParams']);
 
   assert.deepEqual(counters, {
     ...baseCounters,
-    types: 3,
+    types: 1,
     processed: 2,
     total: 2,
   });
diff --git a/packages/knip/test/type-visibility.test.ts b/packages/knip/test/type-visibility.test.ts
@@ -7,19 +7,21 @@ import { resolve } from './helpers/resolve.ts';
 
 const cwd = resolve('fixtures/type-visibility');
 
-test('Report types only externally unused (per tsc: structural inlining does not keep inner exports alive)', async () => {
+test('Keep types referenced in exported value signatures alive (declaration emit / TS4023)', async () => {
   const options = await createOptions({ cwd });
   const { issues, counters } = await main(options);
 
-  // Type → function: reported (nobody imports the type)
-  assert(issues.types['src/lib.ts']['ParamConfig']);
-  assert(issues.types['src/lib.ts']['ResponseData']);
+  // Type → exported value signature: required for .d.ts emit
+  assert(!issues.types['src/lib.ts']?.['ParamConfig']);
+  assert(!issues.types['src/lib.ts']?.['ResponseData']);
+  assert(!issues.types['src/lib.ts']?.['LogLevel']);
+  assert(!issues.exports['src/lib.ts']?.['Connection']);
+  assert(!issues.exports['src/lib.ts']?.['defaultHandler']);
+
+  // Used only in function body (not signature): still flagged
   assert(issues.types['src/lib.ts']['InternalState']);
-  assert(issues.types['src/lib.ts']['LogLevel']);
-  assert(issues.exports['src/lib.ts']['Connection']);
-  assert(issues.exports['src/lib.ts']['defaultHandler']);
 
-  // Type → type (imported): inner type is structurally inlined — flagged
+  // Type → type (structurally inlined): flagged
   assert(issues.types['src/lib.ts']['SuccessResult']);
   assert(issues.types['src/lib.ts']['ErrorResult']);
   assert(issues.types['src/lib.ts']['BaseEntity']);
@@ -28,19 +30,21 @@ test('Report types only externally unused (per tsc: structural inlining does not
   // Directly imported: kept
   assert(!issues.types['src/lib.ts']?.['DirectlyUsed']);
 
-  // Type → type → function chain: reported (chain ends at function)
+  // Chain: FilterRule → FilterSet (type) → applyFilters (function).
+  // FilterSet is in a value signature → kept; FilterRule structurally inlined into FilterSet → flagged.
+  assert(!issues.types['src/lib.ts']?.['FilterSet']);
   assert(issues.types['src/lib.ts']['FilterRule']);
-  assert(issues.types['src/lib.ts']['FilterSet']);
 
-  // Unused
-  assert(issues.types['src/lib.ts']['CompletelyUnused']);
-  assert(issues.types['src/lib.ts']['SharedConfig']);
+  // SharedConfig used in both function param and unused type alias → kept (via function path)
+  assert(!issues.types['src/lib.ts']?.['SharedConfig']);
   assert(issues.types['src/lib.ts']['AppConfig']);
 
+  // Genuinely unused
+  assert(issues.types['src/lib.ts']['CompletelyUnused']);
+
   assert.deepEqual(counters, {
     ...baseCounters,
-    exports: 2,
-    types: 13,
+    types: 8,
     processed: 2,
     total: 2,
   });

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`export type { Fruit } from './types.ts';`
`2`		`-export { pickFruit } from './types.ts';`
	`2`	`+export { pickFruit, fetchApi } from './types.ts';`