Skip to content

Commit 98113e6

Browse files
committed
Add e2e tests w/ tsgo defs + lib consumption
1 parent a71ead1 commit 98113e6

38 files changed

Lines changed: 266 additions & 57 deletions

File tree

.agents/EXPORTS.md

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
# Identifiers, Symbols & Matching
22

3-
## The v5→v6 shift
4-
5-
v5 used TypeScript's typechecker for binding/symbol identity — two identifiers
6-
with the same name in different scopes were distinct symbols. v6 migrated to OXC
7-
(no typechecker), so matching is name-based. This creates false negatives when
8-
local bindings shadow exported names.
3+
How knip resolves "is this export used?" across an AST. Matching is name-based;
4+
the sections below cover the cases that need extra care.
95

106
## Shadow detection
117

12-
`isShadowed(name, pos)` checks whether a reference at a given position falls
13-
inside a scope that shadows that name. Shadows are registered via:
8+
Name-based matching produces false negatives when a local binding shadows an
9+
exported name of the same spelling. `isShadowed(name, pos)` returns true if the
10+
reference at `pos` falls inside a scope that shadows `name`. Shadows are
11+
registered via:
1412

1513
- `_addShadow`: block-scoped variables and nested function declarations
1614
(uses current `scopeDepth` range from `BlockStatement`)
@@ -23,55 +21,68 @@ inside a scope that shadows that name. Shadows are registered via:
2321
`_collectBindingNames` recurses into destructuring patterns (ObjectPattern,
2422
ArrayPattern, AssignmentPattern, RestElement) to extract all bound Identifiers.
2523

26-
## `referencedInExport`
27-
28-
Maps exported identifier → set of export names whose type annotations reference
29-
it. Only for type-level exports, NOT function signatures. Type→type chains are
30-
followed; type→function chains do not keep types alive. Interface `extends`
31-
clauses captured via `addRefInExport`.
24+
## `ignoreExportsUsedInFile`
3225

33-
## `isReferencedInUsedExport`
26+
Opt-in config (default `false`).
3427

35-
Called in `analyze.ts` for exports NOT directly imported. Returns true only
36-
through the `ignoreExportsUsedInFile` chain: a containing export has
37-
`hasRefsInFile` and is a type/interface (checked recursively).
28+
**Default semantics match tsc/tsgo.** An export only referenced in its own file
29+
is reported as unused, because removing the `export` keyword leaves the program
30+
and `.d.ts` valid. Types are structurally inlined: a consumer importing
31+
`UserInfo = { address: Address }` does not require `Address` to be exported.
32+
Same for `typeof X` references inside type aliases. So `Address` is correctly
33+
flagged. Opting `true` is a code-organization preference, not a correctness
34+
concern.
3835

39-
Does *not* keep inner exports alive just because the containing export is
40-
imported externally. Per tsc semantics, types are structurally inlined — a
41-
consumer importing `UserInfo = { address: Address }` does not require `Address`
42-
to be exported (tsc will emit a `declare`-private decl in the `.d.ts`). So
43-
`Address` is correctly flagged as an unused export. Same applies to `typeof X`
44-
references inside type aliases.
36+
**With the config on.** `localRefsVisitorObject` populates `localRefs` during
37+
AST traversal. Exports present in `localRefs` get `hasRefsInFile = true`.
38+
`shouldCountRefs` gates eligible types. Computed member access
39+
(`obj[EXPORTED_KEY]`) is handled.
4540

46-
## `ignoreExportsUsedInFile`
41+
`analyze.ts` reads this via `isReferencedInUsedExport` for exports not directly
42+
imported: returns true only when a containing export has `hasRefsInFile` and is
43+
a type/interface (recursively checked). Alive-ness does not cascade through
44+
external imports; inner refs stay scoped to the in-file relationship.
4745

48-
Opt-in config. When enabled, `localRefsVisitorObject` populates `localRefs`
49-
during AST traversal. Exports in `localRefs` get `hasRefsInFile = true`.
50-
`shouldCountRefs` gates eligible types. Handles computed member access
51-
(`obj[EXPORTED_KEY]`).
46+
## `referencedInExport` (type-chain refs)
5247

53-
Note: when a namespace/enum has export-level `hasRefsInFile = true` but is NOT
54-
externally imported, `analyze.ts` skips the member check entirely — unused
55-
members silently pass. This is by design (the export is considered "used").
48+
Maps exported identifier → set of export names whose type annotations reference
49+
it. Type-level only, not function signatures. Type→type chains are followed;
50+
type→function chains do not keep types alive. Interface `extends` clauses
51+
captured via `addRefInExport`. Feeds the recursive type/interface check in
52+
`isReferencedInUsedExport` above.
5653

5754
## Namespace/enum member `hasRefsInFile`
5855

5956
`ExportMember.hasRefsInFile` (separate from export-level `Export.hasRefsInFile`)
60-
is set via deferred resolution: during the walk, same-file member references
57+
is set via deferred resolution. During the walk, same-file member references
6158
(e.g. `Bar.value`) are pushed as interleaved pairs to a flat
6259
`memberRefsInFile: string[]` on `WalkState`. After the walk, pairs are resolved
6360
against the final `exports` map.
6461

65-
Deferred because the AST visitor walks source-order forward references to
62+
Deferred because the AST visitor walks source-order; forward references to
6663
namespace members defined later in the file would miss inline resolution. Same
6764
pattern as `bareExprRefs`.
6865

6966
Collection: `handleMemberExpression` (3 depth levels) and
7067
`coreVisitorObject.TSQualifiedName` push pairs to `memberRefsInFile`.
7168
`localRefsVisitorObject.TSQualifiedName` only adds the namespace name to
72-
`localRefs` (no member pushcore handler covers it). Resolution in `walkAST`
73-
post-walk.
69+
`localRefs` (no member push, core handler covers it). Resolution happens in
70+
`walkAST` post-walk.
7471

7572
In `analyze.ts`: for referenced namespace/enum exports, members without
7673
`hasRefsInFile` are checked via `explorer.isReferenced(filePath, "Ns.member")`.
7774
If neither, reported as unused.
75+
76+
**Edge case:** when a namespace/enum has export-level `hasRefsInFile = true`
77+
but is NOT externally imported, `analyze.ts` skips the member check entirely.
78+
Unused members silently pass. By design (the export itself is considered
79+
"used").
80+
81+
## E2E
82+
83+
`packages/knip/test/e2e/fix-tsgo.test.ts` is the safety net for the resolution
84+
paths above. Each fixture builds clean under tsgo, has `knip --fix` run on it,
85+
then must build clean under tsgo again. A failing post-fix build means knip
86+
removed something tsc/tsgo still needs: a false positive in one of these
87+
mechanisms. The `e2e-lib-*` variants extend the round-trip to a consumer so
88+
type-visibility regressions also fail.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import * as Lib from '@fixtures/e2e-lib-namespace-subpaths';
2+
import { greet, type Greeting } from '@fixtures/e2e-lib-namespace-subpaths/utils';
3+
4+
const a: Greeting = greet('world');
5+
const b: Lib.Greeting = Lib.Utils.greet('hello');
6+
a;
7+
b;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"name": "@fixtures/e2e-lib-namespace-subpaths",
3+
"type": "module",
4+
"main": "./dist/index.js",
5+
"types": "./dist/index.d.ts",
6+
"exports": {
7+
".": {
8+
"types": "./dist/index.d.ts",
9+
"default": "./dist/index.js"
10+
},
11+
"./utils": {
12+
"types": "./dist/utils/index.d.ts",
13+
"default": "./dist/utils/index.js"
14+
}
15+
}
16+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * as Utils from './utils/index.js';
2+
export type { Greeting } from './utils/types.js';
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import type { Greeting } from './types';
2+
3+
export const greet = (name: string): Greeting => ({ name });
4+
5+
export const internalUnused = 1;
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { greet } from './greet.js';
2+
export type { Greeting } from './types.js';
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export type Greeting = {
2+
name: string;
3+
};
4+
5+
export type DraftGreeting = string;
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"compilerOptions": {
3+
"target": "ES2022",
4+
"module": "ESNext",
5+
"moduleResolution": "Bundler",
6+
"declaration": true,
7+
"outDir": "./dist",
8+
"rootDir": "./src",
9+
},
10+
"include": ["src/**/*.ts"]
11+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { pickFruit, type Fruit } from '@fixtures/e2e-lib-public-surface';
2+
const x: Fruit = pickFruit('apple');
3+
x;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"name": "@fixtures/e2e-lib-public-surface",
3+
"type": "module",
4+
"main": "./dist/index.js",
5+
"types": "./dist/index.d.ts",
6+
"exports": {
7+
".": {
8+
"types": "./dist/index.d.ts",
9+
"default": "./dist/index.js"
10+
}
11+
}
12+
}

0 commit comments

Comments
 (0)