Skip to content

Commit d0b9bf2

Browse files
committed
fix: address code review findings
- graph: replace silent `if (existing)` guard with invariant assertion - graph: extract merge values as consts for order-safe mutation - test: assert `main` is dead in same-file call test (negative case) - test: add call-graph-only class test (no import edge) - test: verify all graphology attrs in edge merge sync test
1 parent 18727fd commit d0b9bf2

2 files changed

Lines changed: 35 additions & 9 deletions

File tree

src/analyzer/index.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ describe("dead export detection", () => {
327327

328328
const dead = result.fileMetrics.get("module.ts")?.deadExports;
329329
expect(dead).not.toContain("helper");
330+
// main is exported but never called by anyone — should be dead
331+
expect(dead).toContain("main");
330332
});
331333

332334
it("class method calls mark the class as consumed", () => {
@@ -346,6 +348,23 @@ describe("dead export detection", () => {
346348
expect(dead).not.toContain("AuthService");
347349
});
348350

351+
it("class consumed via call graph only (no import edge)", () => {
352+
const files = [
353+
makeFile("service.ts", {
354+
exports: [exp("MyClass", "class")],
355+
}),
356+
makeFile("caller.ts", {
357+
// No import edge — only a call site references the class
358+
callSites: [callSite("caller.ts", "init", "service.ts", "MyClass.create")],
359+
}),
360+
];
361+
const built = buildGraph(files);
362+
const result = analyzeGraph(built, files);
363+
364+
const dead = result.fileMetrics.get("service.ts")?.deadExports;
365+
expect(dead).not.toContain("MyClass");
366+
});
367+
349368
it("mixed dead and alive exports only flags dead ones", () => {
350369
const files = [
351370
makeFile("mixed.ts", {
@@ -393,8 +412,12 @@ describe("dead export detection", () => {
393412

394413
// graphology edge attrs should match
395414
const graphSymbols = built.graph.getEdgeAttribute("source.ts", "target.ts", "symbols") as string[];
415+
const graphWeight = built.graph.getEdgeAttribute("source.ts", "target.ts", "weight") as number;
416+
const graphIsTypeOnly = built.graph.getEdgeAttribute("source.ts", "target.ts", "isTypeOnly") as boolean;
396417
expect(graphSymbols).toContain("valFn");
397418
expect(graphSymbols).toContain("TypeB");
419+
expect(graphWeight).toBe(2);
420+
expect(graphIsTypeOnly).toBe(false);
398421
});
399422
});
400423

src/graph/index.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,19 @@ export function buildGraph(files: ParsedFile[]): BuiltGraph {
7979
} else {
8080
// Merge symbols from duplicate import into existing edge
8181
const existing = edges.find((e) => e.source === file.relativePath && e.target === target);
82-
if (existing) {
83-
const merged = [...new Set([...existing.symbols, ...imp.symbols])];
84-
existing.symbols = merged;
85-
existing.weight = merged.length || 1;
86-
existing.isTypeOnly = existing.isTypeOnly && imp.isTypeOnly;
87-
// Sync graphology edge attributes
88-
graph.setEdgeAttribute(file.relativePath, target, "symbols", merged);
89-
graph.setEdgeAttribute(file.relativePath, target, "weight", merged.length || 1);
90-
graph.setEdgeAttribute(file.relativePath, target, "isTypeOnly", existing.isTypeOnly);
82+
if (!existing) {
83+
throw new Error(`Invariant violation: edge ${file.relativePath}${target} exists in graph but missing from edges[]`);
9184
}
85+
const merged = [...new Set([...existing.symbols, ...imp.symbols])];
86+
const mergedWeight = merged.length || 1;
87+
const mergedIsTypeOnly = existing.isTypeOnly && imp.isTypeOnly;
88+
existing.symbols = merged;
89+
existing.weight = mergedWeight;
90+
existing.isTypeOnly = mergedIsTypeOnly;
91+
// Sync graphology edge attributes
92+
graph.setEdgeAttribute(file.relativePath, target, "symbols", merged);
93+
graph.setEdgeAttribute(file.relativePath, target, "weight", mergedWeight);
94+
graph.setEdgeAttribute(file.relativePath, target, "isTypeOnly", mergedIsTypeOnly);
9295
}
9396
}
9497
}

0 commit comments

Comments
 (0)