Skip to content

Commit 1cff13a

Browse files
TheGreatAxiosTheGreatAxios
committed
fix(deadcode): scope dead-export exemptions and warn on stale entries
Whole-file exemptions hide new dead exports in the covered modules, so the guard can only see what it already knows. Exact-name entries keep the exemption set precise, and stale-entry warnings keep exemptions from outliving the code they cover.
1 parent bc47a37 commit 1cff13a

3 files changed

Lines changed: 222 additions & 30 deletions

File tree

‎scripts/check-dead-exports.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import {
4+
evaluateGuard,
5+
isAllowlisted,
6+
loadAllowlist,
7+
parseAllowlistText,
8+
parseTsPruneLine,
9+
} from "./check-dead-exports.js";
10+
11+
// A probe dead export in one of the scoped files must fail the guard: the
12+
// exact-name exemptions cover only the five deferred-cleanup flags, never
13+
// the whole module.
14+
describe("scoped exemptions", () => {
15+
test("the real allowlist covers the named flags but not a sibling probe", () => {
16+
const rules = loadAllowlist();
17+
expect(
18+
isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexUsage"),
19+
).toBe(true);
20+
expect(
21+
isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexModels"),
22+
).toBe(true);
23+
expect(isAllowlisted(rules, "src/auth/xai/usage.ts", "fetchXaiUsage")).toBe(
24+
true,
25+
);
26+
expect(
27+
isAllowlisted(
28+
rules,
29+
"src/auth/codex/constants.ts",
30+
"CODEX_REFRESH_SKEW_MS",
31+
),
32+
).toBe(true);
33+
expect(
34+
isAllowlisted(
35+
rules,
36+
"src/auth/codex/constants.ts",
37+
"CODEX_HEADLESS_REFRESH_INTERVAL_MS",
38+
),
39+
).toBe(true);
40+
expect(
41+
isAllowlisted(rules, "src/auth/codex/usage.ts", "someNewDeadExport"),
42+
).toBe(false);
43+
});
44+
45+
test("a probe dead export in a scoped file is a violation", () => {
46+
const rules = parseAllowlistText(
47+
"src/auth/codex/usage.ts: fetchCodexUsage\n",
48+
);
49+
const outcome = evaluateGuard(
50+
rules,
51+
"src/auth/codex/usage.ts:114 - fetchCodexUsage\n" +
52+
"src/auth/codex/usage.ts:200 - someNewDeadExport\n",
53+
);
54+
expect(outcome.dead).toBe(2);
55+
expect(outcome.violations).toEqual([
56+
"src/auth/codex/usage.ts: someNewDeadExport",
57+
]);
58+
expect(outcome.unused).toEqual([]);
59+
});
60+
61+
test("exports used only inside their own module do not count", () => {
62+
const outcome = evaluateGuard(
63+
[],
64+
"src/inference-abort.ts:5 - InferenceAbortReason (used in module)\n",
65+
);
66+
expect(outcome.dead).toBe(0);
67+
expect(outcome.violations).toEqual([]);
68+
});
69+
70+
test("parseTsPruneLine skips blank and unparseable lines", () => {
71+
expect(parseTsPruneLine("")).toBeUndefined();
72+
expect(parseTsPruneLine("not a ts-prune line")).toBeUndefined();
73+
expect(
74+
parseTsPruneLine("src/auth/xai/usage.ts:89 - fetchXaiUsage"),
75+
).toEqual({ file: "src/auth/xai/usage.ts", name: "fetchXaiUsage" });
76+
});
77+
});
78+
79+
// Allowlist entries that match no current ts-prune flag are stale: report
80+
// them so the exemption is removed with the code it covered, without
81+
// failing the gate on their own.
82+
describe("stale allowlist entries", () => {
83+
test("an entry matching nothing is reported as unused, not a violation", () => {
84+
const rules = parseAllowlistText(
85+
"src/auth/xai/usage.ts: fetchXaiUsage\n" +
86+
"src/gone.ts: vanishedExport\n",
87+
);
88+
const outcome = evaluateGuard(
89+
rules,
90+
"src/auth/xai/usage.ts:89 - fetchXaiUsage\n",
91+
);
92+
expect(outcome.violations).toEqual([]);
93+
expect(outcome.unused).toEqual(["src/gone.ts: vanishedExport"]);
94+
});
95+
96+
test("a fully fresh allowlist reports no unused entries", () => {
97+
const rules = parseAllowlistText(
98+
"vendor/\nsrc/auth/xai/usage.ts: fetchXaiUsage\n",
99+
);
100+
const outcome = evaluateGuard(
101+
rules,
102+
"vendor/intx-types/src/index.ts:3 - ErrorResponse\n" +
103+
"src/auth/xai/usage.ts:89 - fetchXaiUsage\n",
104+
);
105+
expect(outcome.violations).toEqual([]);
106+
expect(outcome.unused).toEqual([]);
107+
});
108+
109+
test("a prefix entry counts as used when any flag falls under it", () => {
110+
const rules = parseAllowlistText("vendor/\n");
111+
const outcome = evaluateGuard(
112+
rules,
113+
"vendor/intx-types/src/index.ts:3 - ErrorResponse\n",
114+
);
115+
expect(outcome.violations).toEqual([]);
116+
expect(outcome.unused).toEqual([]);
117+
});
118+
});

‎scripts/check-dead-exports.ts‎

Lines changed: 93 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,37 +8,68 @@ import { fileURLToPath } from "node:url";
88
// Exports used only inside their own module ("(used in module)") are live
99
// enough and do not count. New dead exports must be deleted, not allowlisted:
1010
// the allowlist covers entry points, cross-lane ownership, plugin surfaces
11-
// loaded by path, and ts-prune parser false positives only.
11+
// loaded by path, and ts-prune parser false positives only. Allowlist entries
12+
// that match no current ts-prune flag are reported as stale warnings so dead
13+
// exemptions cannot linger after the code they cover is gone.
1214

1315
const here = dirname(fileURLToPath(import.meta.url));
1416
const repoRoot = dirname(here);
1517
const allowlistPath = join(here, "dead-export-allowlist.txt");
1618
const tsPruneBin = join(repoRoot, "node_modules", ".bin", "ts-prune");
1719

18-
type AllowRule =
19-
| { readonly kind: "prefix"; readonly prefix: string }
20-
| { readonly kind: "exact"; readonly file: string; readonly name: string };
20+
export type AllowRule =
21+
| {
22+
readonly kind: "prefix";
23+
readonly prefix: string;
24+
readonly source: string;
25+
}
26+
| {
27+
readonly kind: "exact";
28+
readonly file: string;
29+
readonly name: string;
30+
readonly source: string;
31+
};
32+
33+
export interface DeadExport {
34+
readonly file: string;
35+
readonly name: string;
36+
}
2137

22-
function loadAllowlist(): AllowRule[] {
38+
export interface GuardOutcome {
39+
readonly dead: number;
40+
readonly violations: string[];
41+
readonly unused: string[];
42+
}
43+
44+
export function parseAllowlistText(text: string): AllowRule[] {
2345
const rules: AllowRule[] = [];
24-
for (const raw of readFileSync(allowlistPath, "utf8").split("\n")) {
46+
for (const raw of text.split("\n")) {
2547
const line = raw.trim();
2648
if (line === "" || line.startsWith("#")) continue;
2749
const exact = line.match(/^(.+?): ([A-Za-z_$][\w$]*)$/);
2850
if (exact) {
2951
const file = exact[1];
3052
const name = exact[2];
3153
if (file !== undefined && name !== undefined) {
32-
rules.push({ kind: "exact", file: file.trim(), name: name.trim() });
54+
rules.push({
55+
kind: "exact",
56+
file: file.trim(),
57+
name: name.trim(),
58+
source: line,
59+
});
3360
}
3461
} else {
35-
rules.push({ kind: "prefix", prefix: line });
62+
rules.push({ kind: "prefix", prefix: line, source: line });
3663
}
3764
}
3865
return rules;
3966
}
4067

41-
function isAllowlisted(
68+
export function loadAllowlist(): AllowRule[] {
69+
return parseAllowlistText(readFileSync(allowlistPath, "utf8"));
70+
}
71+
72+
export function isAllowlisted(
4273
rules: AllowRule[],
4374
file: string,
4475
name: string,
@@ -50,35 +81,72 @@ function isAllowlisted(
5081
);
5182
}
5283

84+
// Parses one ts-prune output line. Returns undefined for blank lines,
85+
// unparseable lines, and exports used only inside their own module.
86+
export function parseTsPruneLine(raw: string): DeadExport | undefined {
87+
const match = raw.match(/^(.*?):\d+ - (\S+?)(\s+\(used in module\))?$/);
88+
if (!match) return undefined;
89+
if (match[3] !== undefined) return undefined;
90+
const file = match[1];
91+
const name = match[2];
92+
if (file === undefined || name === undefined) return undefined;
93+
return { file, name };
94+
}
95+
96+
export function evaluateGuard(
97+
rules: AllowRule[],
98+
tsPruneStdout: string,
99+
): GuardOutcome {
100+
const used = new Set<number>();
101+
const violations: string[] = [];
102+
let dead = 0;
103+
for (const raw of tsPruneStdout.split("\n")) {
104+
const parsed = parseTsPruneLine(raw);
105+
if (parsed === undefined) continue;
106+
dead += 1;
107+
const index = rules.findIndex((rule) =>
108+
rule.kind === "prefix"
109+
? parsed.file.startsWith(rule.prefix)
110+
: rule.file === parsed.file && rule.name === parsed.name,
111+
);
112+
if (index === -1) {
113+
violations.push(`${parsed.file}: ${parsed.name}`);
114+
} else {
115+
used.add(index);
116+
}
117+
}
118+
const unused = rules
119+
.filter((_, index) => !used.has(index))
120+
.map((r) => r.source);
121+
return { dead, violations, unused };
122+
}
123+
53124
function main(): void {
54125
const rules = loadAllowlist();
55126
const pruned = spawnSync(tsPruneBin, [], { cwd: repoRoot, encoding: "utf8" });
56127
if (pruned.status !== 0) {
57128
console.error(`ts-prune failed:\n${pruned.stderr || pruned.stdout}`);
58129
process.exit(1);
59130
}
60-
const violations: string[] = [];
61-
let dead = 0;
62-
for (const raw of String(pruned.stdout).split("\n")) {
63-
const match = raw.match(/^(.*?):\d+ - (\S+?)(\s+\(used in module\))?$/);
64-
if (!match) continue;
65-
if (match[3] !== undefined) continue;
66-
const file = match[1];
67-
const name = match[2];
68-
if (file === undefined || name === undefined) continue;
69-
dead += 1;
70-
if (!isAllowlisted(rules, file, name)) violations.push(`${file}: ${name}`);
71-
}
131+
const outcome = evaluateGuard(rules, String(pruned.stdout));
72132
console.log(
73-
`dead-export guard: ${dead} consumer-less exports, ${dead - violations.length} allowlisted, ${violations.length} violations`,
133+
`dead-export guard: ${outcome.dead} consumer-less exports, ${outcome.dead - outcome.violations.length} allowlisted, ${outcome.violations.length} violations`,
74134
);
75-
if (violations.length > 0) {
135+
if (outcome.unused.length > 0) {
136+
console.error(
137+
"Stale allowlist entries matching no dead export (remove them):\n" +
138+
outcome.unused.map((entry) => ` ${entry}`).join("\n"),
139+
);
140+
}
141+
if (outcome.violations.length > 0) {
76142
console.error(
77143
"New dead exports (delete them or justify an allowlist entry):\n" +
78-
violations.map((v) => ` ${v}`).join("\n"),
144+
outcome.violations.map((v) => ` ${v}`).join("\n"),
79145
);
80146
process.exit(1);
81147
}
82148
}
83149

84-
main();
150+
if (import.meta.main) {
151+
main();
152+
}

‎scripts/dead-export-allowlist.txt‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@
1313
# vendor workspaces on the next sync. Prune upstream, not here.
1414
vendor/
1515

16-
# CL-6815 lane owns the test-only consumer modules (Codex/xAI usage data
17-
# paths). That lane decides their fate; this lane must not delete them.
18-
src/auth/codex/usage.ts
19-
src/auth/xai/usage.ts
20-
src/auth/codex/constants.ts
16+
# Deferred Codex/xAI usage-data cleanup owned by the CL-6815 lane: these
17+
# modules are fully dead deferred code with no live importers (not test-only
18+
# helpers), and that lane decides their fate. Scoped to the exact ts-prune
19+
# flags so a new dead export in these files fails the guard instead of
20+
# hiding under a whole-file exemption. Remove each entry with the export it
21+
# names when the owning cleanup lands.
22+
src/auth/codex/usage.ts: fetchCodexUsage
23+
src/auth/codex/usage.ts: fetchCodexModels
24+
src/auth/xai/usage.ts: fetchXaiUsage
25+
src/auth/codex/constants.ts: CODEX_REFRESH_SKEW_MS
26+
src/auth/codex/constants.ts: CODEX_HEADLESS_REFRESH_INTERVAL_MS
2127

2228
# Plugin fixture entry point. Loaded by file path from the fixture's plugin
2329
# manifest by the plugin-registration tests, so it has no static importers.

0 commit comments

Comments
 (0)