mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(dead-code): a method a framework calls by decorator or by name is not dead (#2405)
The viewer's Dead code screen listed two kinds of framework-called methods. A method whose decorator resolves outside the project (`@HostListener`, `@Cron`, `@OnEvent`, `@Process`, `@BeforeInsert`, `@InitBinder`) has no `decorates` edge and no `node.decorators`, only the resolver's unresolved `decorates` row. That row now counts as a decorator, the same evidence the opaque-ancestor rule reads for an `extends`/`implements` row, under the existing `decorated` count. A lifecycle hook on a class that skips the `implements` clause (`ngOnInit` on a `@Component` without `implements OnInit`) had no evidence at all. A new `hooks` rule drops a TypeScript method when its class carries a decorator and an interface outside the index carries the name. What the interface carries is read off the classes that write it out: every one has the method, declared or inherited, at least two and more than half declare it, and more than half of them carry a decorator. TypeScript only, because `implements` is erased there and a framework can only be finding the hook by name. The ancestor walk moves out of overrideCandidates into ancestorsOf, shared by both rules. Declarations are read from the candidate list rather than searched by name: a hook is never called from inside the index, and a name some declaration of which is called is the ambiguous-name rule's anyway. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -532,3 +532,234 @@ function reallyUnused() {}
|
||||
expect(names(report)).toContain('neverCalledMember');
|
||||
});
|
||||
});
|
||||
|
||||
describe('a framework that calls members by name', () => {
|
||||
let root: string;
|
||||
let graph: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-deadcode-by-name-'));
|
||||
// Three components that write the interface out: together they are the
|
||||
// index's own record of what Angular's `OnInit` carries. The third inherits
|
||||
// `ngOnInit` rather than declaring it, and lacks the `refresh` the other
|
||||
// two happen to share.
|
||||
write(
|
||||
root,
|
||||
'src/app/first.component.ts',
|
||||
`import { Component, OnInit } from '@angular/core';
|
||||
|
||||
@Component({ selector: 'app-first', template: '' })
|
||||
export class FirstComponent implements OnInit {
|
||||
ngOnInit(): void {}
|
||||
refresh(): void {}
|
||||
}
|
||||
`
|
||||
);
|
||||
write(
|
||||
root,
|
||||
'src/app/second.component.ts',
|
||||
`import { AfterViewInit, Component, OnInit } from '@angular/core';
|
||||
|
||||
@Component({ selector: 'app-second', template: '' })
|
||||
export class SecondComponent implements OnInit, AfterViewInit {
|
||||
ngOnInit(): void {}
|
||||
ngAfterViewInit(): void {}
|
||||
reload(): void {}
|
||||
refresh(): void {}
|
||||
}
|
||||
`
|
||||
);
|
||||
write(
|
||||
root,
|
||||
'src/app/base-page.ts',
|
||||
`import { Directive } from '@angular/core';
|
||||
|
||||
@Directive()
|
||||
export abstract class BasePage {
|
||||
ngOnInit(): void {
|
||||
void 0;
|
||||
}
|
||||
}
|
||||
`
|
||||
);
|
||||
write(
|
||||
root,
|
||||
'src/app/third.component.ts',
|
||||
`import { Component, OnInit } from '@angular/core';
|
||||
import { BasePage } from './base-page';
|
||||
|
||||
@Component({ selector: 'app-third', template: '' })
|
||||
export class ThirdComponent extends BasePage implements OnInit {}
|
||||
`
|
||||
);
|
||||
// Writes no `implements` clause. Angular calls `ngOnInit` all the same.
|
||||
write(
|
||||
root,
|
||||
'src/app/demo.component.ts',
|
||||
`import { Component, HostListener } from '@angular/core';
|
||||
|
||||
@Component({ selector: 'app-demo', template: '' })
|
||||
export class DemoComponent {
|
||||
ngOnInit(): void {
|
||||
void 0;
|
||||
}
|
||||
|
||||
@HostListener('window:keyup', ['$event'])
|
||||
keyEvent(event: KeyboardEvent): void {
|
||||
void event;
|
||||
}
|
||||
|
||||
reload(): void {
|
||||
void 0;
|
||||
}
|
||||
|
||||
refresh(): void {
|
||||
void 0;
|
||||
}
|
||||
|
||||
unusedHelper(): number {
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
`
|
||||
);
|
||||
// No decorator, so no framework registers it: the name alone is no evidence.
|
||||
write(root, 'src/app/plain.ts', `export class Plain {\n ngOnInit(): void {\n void 0;\n }\n}\n`);
|
||||
write(
|
||||
root,
|
||||
'src/api/cron.service.ts',
|
||||
`import { Injectable } from '@nestjs/common';
|
||||
import { OnEvent } from '@nestjs/event-emitter';
|
||||
import { Cron } from '@nestjs/schedule';
|
||||
|
||||
@Injectable()
|
||||
export class CronService {
|
||||
@Cron('0 * * * *')
|
||||
runEveryHour(): void {
|
||||
void 0;
|
||||
}
|
||||
|
||||
@OnEvent('portfolio.changed')
|
||||
handlePortfolioChanged(): void {
|
||||
void 0;
|
||||
}
|
||||
|
||||
hasCurrencyPair(): boolean {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
`
|
||||
);
|
||||
write(
|
||||
root,
|
||||
'src/main.ts',
|
||||
`import { FirstComponent } from './app/first.component';
|
||||
import { SecondComponent } from './app/second.component';
|
||||
import { ThirdComponent } from './app/third.component';
|
||||
import { DemoComponent } from './app/demo.component';
|
||||
import { Plain } from './app/plain';
|
||||
import { CronService } from './api/cron.service';
|
||||
|
||||
export const declarations = [FirstComponent, SecondComponent, ThirdComponent, DemoComponent, Plain, CronService];
|
||||
`
|
||||
);
|
||||
// Java checks the interface itself: Spring calls `afterPropertiesSet` on a
|
||||
// bean that implements InitializingBean, never on one that only has the name.
|
||||
for (const bean of ['FirstBean', 'SecondBean']) {
|
||||
write(
|
||||
root,
|
||||
`src/main/java/demo/${bean}.java`,
|
||||
`package demo;
|
||||
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
@Component
|
||||
public class ${bean} implements InitializingBean {
|
||||
public void afterPropertiesSet() {}
|
||||
}
|
||||
`
|
||||
);
|
||||
}
|
||||
write(
|
||||
root,
|
||||
'src/main/java/demo/LooseBean.java',
|
||||
`package demo;
|
||||
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
@Component
|
||||
public class LooseBean {
|
||||
public void afterPropertiesSet() {}
|
||||
}
|
||||
`
|
||||
);
|
||||
write(
|
||||
root,
|
||||
'src/main/java/demo/App.java',
|
||||
`package demo;
|
||||
|
||||
public class App {
|
||||
public static void main(String[] args) {
|
||||
new FirstBean();
|
||||
new SecondBean();
|
||||
new LooseBean();
|
||||
}
|
||||
}
|
||||
`
|
||||
);
|
||||
graph = CodeGraph.initSync(root, {
|
||||
config: { include: ['src/**/*.ts', 'src/**/*.java'], exclude: [] },
|
||||
});
|
||||
await graph.indexAll();
|
||||
}, 60_000);
|
||||
|
||||
afterAll(() => {
|
||||
graph?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const listed = (report: ReturnType<typeof buildDeadCodeReport>, qualifiedName: string): boolean =>
|
||||
report.entries.some((entry) => entry.node.qualifiedName === qualifiedName);
|
||||
|
||||
it('does not list a method whose decorator resolves outside the index', () => {
|
||||
for (const readSource of [undefined, null] as const) {
|
||||
const report = buildDeadCodeReport(graph, readSource === null ? { readSource } : {});
|
||||
expect(listed(report, 'DemoComponent::keyEvent')).toBe(false);
|
||||
expect(listed(report, 'CronService::runEveryHour')).toBe(false);
|
||||
expect(listed(report, 'CronService::handlePortfolioChanged')).toBe(false);
|
||||
expect(report.excluded.decorated).toBeGreaterThanOrEqual(3);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not list a hook its class fills by name, without writing the interface', () => {
|
||||
for (const readSource of [undefined, null] as const) {
|
||||
const report = buildDeadCodeReport(graph, readSource === null ? { readSource } : {});
|
||||
expect(listed(report, 'DemoComponent::ngOnInit')).toBe(false);
|
||||
// The base a component inherits the hook from is reached the same way.
|
||||
expect(listed(report, 'BasePage::ngOnInit')).toBe(false);
|
||||
expect(report.excluded.hooks).toBe(2);
|
||||
}
|
||||
});
|
||||
|
||||
it('still lists what nothing reaches in a class a framework registers', () => {
|
||||
const report = buildDeadCodeReport(graph);
|
||||
// A decorator on the class is not evidence for each of its members.
|
||||
expect(listed(report, 'DemoComponent::unusedHelper')).toBe(true);
|
||||
expect(listed(report, 'CronService::hasCurrencyPair')).toBe(true);
|
||||
// One implementer is not a contract: `reload` is only SecondComponent's.
|
||||
expect(listed(report, 'DemoComponent::reload')).toBe(true);
|
||||
// Two implementers sharing a method is no contract while a third lacks it.
|
||||
expect(listed(report, 'DemoComponent::refresh')).toBe(true);
|
||||
// Nothing registers an undecorated class.
|
||||
expect(listed(report, 'Plain::ngOnInit')).toBe(true);
|
||||
});
|
||||
|
||||
it('infers a contract only where the interface has no runtime effect', () => {
|
||||
// Java has no export marker in the index, so ask for the exported list,
|
||||
// where every other rule still runs.
|
||||
const report = buildDeadCodeReport(graph, { includeExported: true });
|
||||
expect(listed(report, 'demo::LooseBean::afterPropertiesSet')).toBe(true);
|
||||
expect(listed(report, 'DemoComponent::ngOnInit')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -378,8 +378,10 @@ modules are not dimmed — a module with one `.pb.go` in it is still one somebod
|
||||
|
||||
**What the list refuses to claim** is the whole design. Behind it, `src/graph/dead-code.ts` starts from "no incoming edge
|
||||
other than `contains`" and subtracts every candidate there is any reason to believe something reaches: exported symbols and
|
||||
header declarations, test and generated files, abstract and interface members, anything carrying a `decorates` edge, overrides
|
||||
of an ancestor's member, names the language calls by itself, vendored directories, files nothing in the index reaches (those are
|
||||
header declarations, test and generated files, abstract and interface members, anything carrying a decorator (a `decorates`
|
||||
edge, or a decorator from outside the index the resolver could not follow), overrides of an ancestor's member, a TypeScript
|
||||
hook its framework calls by name on a decorated class that skips the `implements` clause (which names are hooks is read off the
|
||||
classes that do write the interface out), names the language calls by itself, vendored directories, files nothing in the index reaches (those are
|
||||
islands — the Map's job, not this list's), names the resolver failed to resolve somewhere, names shared with a symbol that IS
|
||||
referenced, and — the only rule that reads a file — names written more than once in a file that can reach them.
|
||||
|
||||
|
||||
@@ -175,6 +175,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel
|
||||
|
||||
## Fixes — Symbols, tests and the viewer
|
||||
|
||||
- **Dead code no longer lists methods a framework calls for you.** A method with a framework's decorator on it — Angular's `@HostListener` or `@HostBinding`, NestJS's `@Cron`, `@OnEvent` or `@Process`, TypeORM's `@BeforeInsert`, Spring's `@InitBinder` — was listed as unused whenever the decorator came from a package outside your project. So was a lifecycle hook like Angular's `ngOnInit` or NestJS's `onModuleInit` on a class that skips the `implements` clause, though the framework calls it by name all the same. Both are now left off and counted under the list. What counts as a hook is learned from your own classes that do write the interface out, not from a built-in list, so an unused helper on the same component is still listed. Nothing needs a re-index.
|
||||
|
||||
- **One slow file can no longer freeze the viewer.** Code is highlighted with the same grammars the index uses. The COBOL grammar never finishes a line like ` .`, which free-format COBOL paragraphs end with, so opening a Flow through cobolcraft froze `codegraph ui`, and every page after it waited forever. Highlighting now runs where it can be stopped: a piece of code that takes too long is shown unhighlighted, and everything else keeps working.
|
||||
|
||||
- **The Map opens on a picture, not a single box.** A project whose code sits in one flat folder, like Express's `lib/`, an R package's `R/`, an Erlang app's `src/` or fmt's `include/fmt/`, opened as one box with nothing to say. It now opens on the whole repository when that draws more. Files at the repository's top level also count when the Map picks where the program lives: git's hundreds of top-level `.c` files had made `builtin/` look like most of the code.
|
||||
|
||||
+55
-5
@@ -2426,19 +2426,69 @@ export class QueryBuilder {
|
||||
* Chunked probe over `idx_unresolved_from_node`.
|
||||
*/
|
||||
getUnresolvedSupertypeSourcesAmong(nodeIds: Iterable<string>): Set<string> {
|
||||
return new Set(this.getUnresolvedReferenceNamesFrom(nodeIds, ['extends', 'implements']).keys());
|
||||
}
|
||||
|
||||
/**
|
||||
* The names each of `nodeIds` holds an unresolved reference of `kinds` to.
|
||||
*
|
||||
* A reference into code outside the index leaves no edge, only this row: a
|
||||
* decorator imported from a framework (`@HostListener`, `@Cron`) or an
|
||||
* interface named in an `implements` clause. Pending rows count with failed
|
||||
* ones, as in {@link getUnresolvedSupertypeSourcesAmong}. Ids with no such row
|
||||
* are absent from the map. Chunked probe over `idx_unresolved_from_node`.
|
||||
*/
|
||||
getUnresolvedReferenceNamesFrom(nodeIds: Iterable<string>, kinds: readonly string[]): Map<string, string[]> {
|
||||
const unique = [...new Set(nodeIds)];
|
||||
const found = new Set<string>();
|
||||
const found = new Map<string, string[]>();
|
||||
if (unique.length === 0 || kinds.length === 0) return found;
|
||||
const kindPlaceholders = kinds.map(() => '?').join(',');
|
||||
for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) {
|
||||
const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE);
|
||||
const placeholders = chunk.map(() => '?').join(',');
|
||||
const rows = this.db
|
||||
.prepare(
|
||||
`SELECT DISTINCT from_node_id AS id FROM unresolved_refs
|
||||
`SELECT DISTINCT from_node_id AS id, reference_name AS name FROM unresolved_refs
|
||||
WHERE from_node_id IN (${placeholders})
|
||||
AND reference_kind IN ('extends', 'implements')`
|
||||
AND reference_kind IN (${kindPlaceholders})`
|
||||
)
|
||||
.all(...chunk) as Array<{ id: string }>;
|
||||
for (const row of rows) found.add(row.id);
|
||||
.all(...chunk, ...kinds) as Array<{ id: string; name: string }>;
|
||||
for (const row of rows) {
|
||||
const names = found.get(row.id);
|
||||
if (names) names.push(row.name);
|
||||
else found.set(row.id, [row.name]);
|
||||
}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every node holding an unresolved reference of `kinds` to each of `names` —
|
||||
* the mirror of {@link getUnresolvedReferenceNamesFrom}, keyed by the name:
|
||||
* for `implements`, every class that names an interface outside the index.
|
||||
* Names nothing refers to are absent from the map. Chunked probe over
|
||||
* `idx_unresolved_name`.
|
||||
*/
|
||||
getUnresolvedReferenceSourcesNamed(names: Iterable<string>, kinds: readonly string[]): Map<string, Set<string>> {
|
||||
const unique = [...new Set(names)].filter((name) => name.length > 0);
|
||||
const found = new Map<string, Set<string>>();
|
||||
if (unique.length === 0 || kinds.length === 0) return found;
|
||||
const kindPlaceholders = kinds.map(() => '?').join(',');
|
||||
for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) {
|
||||
const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE);
|
||||
const placeholders = chunk.map(() => '?').join(',');
|
||||
const rows = this.db
|
||||
.prepare(
|
||||
`SELECT DISTINCT reference_name AS name, from_node_id AS id FROM unresolved_refs
|
||||
WHERE reference_name IN (${placeholders})
|
||||
AND reference_kind IN (${kindPlaceholders})`
|
||||
)
|
||||
.all(...chunk, ...kinds) as Array<{ name: string; id: string }>;
|
||||
for (const row of rows) {
|
||||
const ids = found.get(row.name);
|
||||
if (ids) ids.add(row.id);
|
||||
else found.set(row.name, new Set([row.id]));
|
||||
}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
+253
-51
@@ -25,11 +25,18 @@
|
||||
* - it is **decorated** (`@app.route`, `@Component`, `@EventHandler`) — a
|
||||
* decorator is a registration, and the framework that reads it is not in the
|
||||
* graph. Seen as the symbol's own outgoing `decorates` edge, which is where
|
||||
* the engine records it; `node.decorators` is only populated by a couple of
|
||||
* languages and is checked as well rather than instead;
|
||||
* the engine records it, or, for a decorator imported from outside the index
|
||||
* (`@HostListener`, `@Cron`), as the resolver's unresolved `decorates` row —
|
||||
* the same record the ancestor rule below reads for a supertype it could not
|
||||
* follow. `node.decorators` is only populated by a couple of languages and is
|
||||
* checked as well rather than instead;
|
||||
* - it **overrides** a member an ancestor declares (calls land on the ancestor;
|
||||
* see {@link overrideCandidates} for why an ancestor we cannot read counts
|
||||
* the same way);
|
||||
* - it is a **hook its framework calls by name**: `ngOnInit` on a component
|
||||
* that never writes `implements OnInit`. Which names are hooks is not kept in
|
||||
* a table here — the index already says it, in what the classes that DO
|
||||
* write the interface out declare (see {@link byNameHookCandidates});
|
||||
* - it has a name the language calls by itself (`constructor`, `__enter__`,
|
||||
* `main`);
|
||||
* - it sits in a **vendored** directory (`vendor/`, `third_party/`,
|
||||
@@ -90,7 +97,7 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import type CodeGraph from '../index';
|
||||
import type { Node, NodeKind } from '../types';
|
||||
import type { EdgeKind, Node, NodeKind } from '../types';
|
||||
import { isTestFile } from '../search/query-utils';
|
||||
|
||||
// =============================================================================
|
||||
@@ -184,6 +191,18 @@ const OVERRIDABLE_KINDS: ReadonlySet<NodeKind> = new Set<NodeKind>([
|
||||
'field',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Languages whose `implements` clause is erased before the program runs, so a
|
||||
* framework that calls an interface's member can only be finding it by name —
|
||||
* the premise of {@link byNameHookCandidates}. Java, C#, Kotlin and the rest
|
||||
* check the interface itself: Spring never calls `afterPropertiesSet` on a
|
||||
* bean that merely has a method of that name.
|
||||
*/
|
||||
const BY_NAME_DISPATCH_LANGUAGES: ReadonlySet<string> = new Set(['typescript', 'tsx']);
|
||||
|
||||
/** Member kinds a framework calls as a hook. */
|
||||
const HOOK_KINDS: readonly NodeKind[] = ['method', 'function'];
|
||||
|
||||
/**
|
||||
* Names a language or a runtime calls without anything in the source naming
|
||||
* them.
|
||||
@@ -193,7 +212,8 @@ const OVERRIDABLE_KINDS: ReadonlySet<NodeKind> = new Set<NodeKind>([
|
||||
* stale, and it would hide real dead code behind a name coincidence. The
|
||||
* entries below are the ones where the *language itself* does the calling, so
|
||||
* no source file could name them even in principle. Framework hooks are caught
|
||||
* by the decorator and override rules instead, which are structural.
|
||||
* by the decorator, override and by-name hook rules instead, which are
|
||||
* structural.
|
||||
*/
|
||||
const IMPLICIT_ENTRY_NAMES: ReadonlySet<string> = new Set([
|
||||
'constructor',
|
||||
@@ -346,6 +366,12 @@ export interface DeadCodeExclusions {
|
||||
decorated: number;
|
||||
/** Overrides a member an ancestor declares, or an ancestor we cannot read. */
|
||||
overriding: number;
|
||||
/**
|
||||
* A hook its framework calls by name, on a class that skips the interface:
|
||||
* the class carries a decorator, and the name is one an interface outside
|
||||
* the index carries, going by the classes that write it out.
|
||||
*/
|
||||
hooks: number;
|
||||
/** Named something the language calls by itself. */
|
||||
implicit: number;
|
||||
/** In a vendored directory — carried code, reached by something outside the index. */
|
||||
@@ -437,6 +463,7 @@ export function buildDeadCodeReport(cg: CodeGraph, query: DeadCodeQuery = {}): D
|
||||
declarations: 0,
|
||||
decorated: 0,
|
||||
overriding: 0,
|
||||
hooks: 0,
|
||||
implicit: 0,
|
||||
vendored: 0,
|
||||
testScope: 0,
|
||||
@@ -508,14 +535,7 @@ export function buildDeadCodeReport(cg: CodeGraph, query: DeadCodeQuery = {}): D
|
||||
// A `decorates` edge runs FROM the decorated symbol to the decorator, so
|
||||
// this is an outgoing-edge question, not something the candidate query could
|
||||
// have answered.
|
||||
const decorated = new Set(
|
||||
cg
|
||||
.getOutgoingEdgesFrom(
|
||||
surviving.map((row) => row.node.id),
|
||||
['decorates']
|
||||
)
|
||||
.map((edge) => edge.source)
|
||||
);
|
||||
const decorated = carriesDecorator(cg, surviving.map((row) => row.node));
|
||||
const containers = containersOf(cg, surviving.map((row) => row.node));
|
||||
const overriding = overrideCandidates(cg, surviving.map((row) => row.node), containers);
|
||||
// One batched count for every file still in play. A file nothing reaches is
|
||||
@@ -532,10 +552,19 @@ export function buildDeadCodeReport(cg: CodeGraph, query: DeadCodeQuery = {}): D
|
||||
const names = surviving.map((row) => row.node.name);
|
||||
const unresolved = cg.getUnresolvedNamesAmong(names);
|
||||
const ambiguousNames = cg.getAmbiguousReferencedNames(names);
|
||||
// Asked only about the members no earlier rule in the loop below drops.
|
||||
const hooks = byNameHookCandidates(
|
||||
cg,
|
||||
surviving
|
||||
.map((row) => row.node)
|
||||
.filter((node) => !decorated.has(node.id) && !overriding.has(node.id)),
|
||||
containers,
|
||||
candidates.map((row) => row.node)
|
||||
);
|
||||
|
||||
const kept: Array<{ node: Node; generated: boolean }> = [];
|
||||
for (const row of surviving) {
|
||||
if (decorated.has(row.node.id) || (row.node.decorators?.length ?? 0) > 0) {
|
||||
if (decorated.has(row.node.id)) {
|
||||
excluded.decorated += 1;
|
||||
continue;
|
||||
}
|
||||
@@ -548,6 +577,10 @@ export function buildDeadCodeReport(cg: CodeGraph, query: DeadCodeQuery = {}): D
|
||||
excluded.overriding += 1;
|
||||
continue;
|
||||
}
|
||||
if (hooks.has(row.node.id)) {
|
||||
excluded.hooks += 1;
|
||||
continue;
|
||||
}
|
||||
if (unresolved.has(row.node.name)) {
|
||||
excluded.unresolvedName += 1;
|
||||
continue;
|
||||
@@ -814,44 +847,8 @@ function overrideCandidates(
|
||||
const containerIds = [...new Set([...containers.values()].map((node) => node.id))];
|
||||
if (containerIds.length === 0) return dropped;
|
||||
|
||||
// Level-by-level upward walk over EVERY container at once: one query per
|
||||
// level rather than one per container. `reach` maps an ancestor back to the
|
||||
// containers it is an ancestor of.
|
||||
const reach = new Map<string, Set<string>>();
|
||||
const seen = new Map(containerIds.map((id) => [id, new Set([id])]));
|
||||
let frontier = containerIds.map((id) => ({ id, roots: new Set<string>([id]) }));
|
||||
|
||||
for (let depth = 0; depth < MAX_OVERRIDE_ANCESTOR_DEPTH && frontier.length > 0; depth++) {
|
||||
const rootsOf = new Map(frontier.map((item) => [item.id, item.roots]));
|
||||
const edges = cg.getOutgoingEdgesFrom(
|
||||
frontier.map((item) => item.id),
|
||||
['extends', 'implements']
|
||||
);
|
||||
const next = new Map<string, Set<string>>();
|
||||
for (const edge of edges) {
|
||||
if (edge.target === edge.source) continue;
|
||||
const roots = rootsOf.get(edge.source);
|
||||
if (!roots) continue;
|
||||
const merged = next.get(edge.target) ?? new Set<string>();
|
||||
const visited = seen.get(edge.target) ?? new Set<string>();
|
||||
const known = reach.get(edge.target) ?? new Set<string>();
|
||||
// An ancestor may already be a candidate root. Only skip pairs we have
|
||||
// propagated, so later descendants still reach all of its ancestors.
|
||||
for (const root of roots) {
|
||||
if (visited.has(root)) continue;
|
||||
visited.add(root);
|
||||
merged.add(root);
|
||||
known.add(root);
|
||||
}
|
||||
seen.set(edge.target, visited);
|
||||
if (merged.size > 0) next.set(edge.target, merged);
|
||||
if (known.size > 0) reach.set(edge.target, known);
|
||||
}
|
||||
frontier = [];
|
||||
for (const [id, roots] of next) {
|
||||
frontier.push({ id, roots });
|
||||
}
|
||||
}
|
||||
// `reach` maps an ancestor back to the containers it is an ancestor of.
|
||||
const reach = ancestorsOf(cg, containerIds, ['extends', 'implements']);
|
||||
|
||||
const opaqueContainers = new Set<string>();
|
||||
const externalBased = cg.getUnresolvedSupertypeSourcesAmong([...containerIds, ...reach.keys()]);
|
||||
@@ -901,3 +898,208 @@ function overrideCandidates(
|
||||
}
|
||||
return dropped;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every ancestor of `ids` along `kinds` edges, mapped back to the ids it is an
|
||||
* ancestor of. A level-by-level upward walk over EVERY id at once — one query
|
||||
* per level rather than one per id — up to {@link MAX_OVERRIDE_ANCESTOR_DEPTH}.
|
||||
*/
|
||||
function ancestorsOf(
|
||||
cg: CodeGraph,
|
||||
ids: readonly string[],
|
||||
kinds: EdgeKind[]
|
||||
): Map<string, Set<string>> {
|
||||
const reach = new Map<string, Set<string>>();
|
||||
const seen = new Map(ids.map((id) => [id, new Set([id])]));
|
||||
let frontier = ids.map((id) => ({ id, roots: new Set<string>([id]) }));
|
||||
|
||||
for (let depth = 0; depth < MAX_OVERRIDE_ANCESTOR_DEPTH && frontier.length > 0; depth++) {
|
||||
const rootsOf = new Map(frontier.map((item) => [item.id, item.roots]));
|
||||
const edges = cg.getOutgoingEdgesFrom(
|
||||
frontier.map((item) => item.id),
|
||||
kinds
|
||||
);
|
||||
const next = new Map<string, Set<string>>();
|
||||
for (const edge of edges) {
|
||||
if (edge.target === edge.source) continue;
|
||||
const roots = rootsOf.get(edge.source);
|
||||
if (!roots) continue;
|
||||
const merged = next.get(edge.target) ?? new Set<string>();
|
||||
const visited = seen.get(edge.target) ?? new Set<string>();
|
||||
const known = reach.get(edge.target) ?? new Set<string>();
|
||||
// An ancestor may already be a root. Only skip pairs we have propagated,
|
||||
// so later descendants still reach all of its ancestors.
|
||||
for (const root of roots) {
|
||||
if (visited.has(root)) continue;
|
||||
visited.add(root);
|
||||
merged.add(root);
|
||||
known.add(root);
|
||||
}
|
||||
seen.set(edge.target, visited);
|
||||
if (merged.size > 0) next.set(edge.target, merged);
|
||||
if (known.size > 0) reach.set(edge.target, known);
|
||||
}
|
||||
frontier = [];
|
||||
for (const [id, roots] of next) {
|
||||
frontier.push({ id, roots });
|
||||
}
|
||||
}
|
||||
return reach;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of `nodes` carry a decorator: their own outgoing `decorates` edge, an
|
||||
* unresolved `decorates` row (the only trace a decorator imported from outside
|
||||
* the index, like `@HostListener` or `@Cron`, leaves), or the `decorators`
|
||||
* list a couple of languages record on the node itself.
|
||||
*/
|
||||
function carriesDecorator(cg: CodeGraph, nodes: readonly Node[]): Set<string> {
|
||||
const ids = nodes.map((node) => node.id);
|
||||
const found = new Set(cg.getOutgoingEdgesFrom(ids, ['decorates']).map((edge) => edge.source));
|
||||
for (const id of cg.getUnresolvedReferenceNamesFrom(ids, ['decorates']).keys()) found.add(id);
|
||||
for (const node of nodes) if ((node.decorators?.length ?? 0) > 0) found.add(node.id);
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which members are hooks their framework calls by name — the ids to drop.
|
||||
*
|
||||
* Angular calls `ngOnInit` on every component it creates, and NestJS calls
|
||||
* `onModuleInit` on every provider, whether or not the class writes
|
||||
* `implements OnInit`: TypeScript erases the clause, so the framework can only
|
||||
* be finding the method by its name. A class that writes the clause is already
|
||||
* off the list, because its ancestor is outside the index
|
||||
* ({@link overrideCandidates}). One that skips it is tied to the framework by
|
||||
* nothing but the name.
|
||||
*
|
||||
* Which names are hooks is not a table kept here; a table would go stale and
|
||||
* hide real dead code behind a name coincidence. The index says it already:
|
||||
* what `OnInit` carries is what the classes that DO write `implements OnInit`
|
||||
* have. A method is dropped when all of this holds:
|
||||
*
|
||||
* - **its class carries a decorator**, so a framework creates it
|
||||
* (`@Component`, `@Injectable`). Nothing creates an undecorated class, so
|
||||
* nothing calls its hooks. And the class's decorator says nothing about its
|
||||
* other methods: an unused helper on a component stays on the list;
|
||||
* - **an interface outside the index carries the name**. Every TypeScript class
|
||||
* that writes the interface into an `implements` clause has a method of that
|
||||
* name, declared or inherited from an ancestor that declares it, which is
|
||||
* what TypeScript itself checks; a class that inherits from outside the index
|
||||
* may have it from there, so it is no evidence either way. At least two of
|
||||
* them, and more than half, declare it themselves: one class's methods are
|
||||
* that class, not a contract, and a name that only a few declare while the
|
||||
* rest are unreadable is not evidence of one either;
|
||||
* - **more than half of those classes carry a decorator** as well: the
|
||||
* interface is one that framework-created classes fill, not a library type
|
||||
* whose method happens to share the name.
|
||||
*
|
||||
* TypeScript only — see {@link BY_NAME_DISPATCH_LANGUAGES}.
|
||||
*/
|
||||
function byNameHookCandidates(
|
||||
cg: CodeGraph,
|
||||
nodes: readonly Node[],
|
||||
containers: ReadonlyMap<string, Node>,
|
||||
unreferenced: readonly Node[]
|
||||
): Set<string> {
|
||||
const dropped = new Set<string>();
|
||||
const isHookShaped = (node: Node): boolean =>
|
||||
HOOK_KINDS.includes(node.kind) && BY_NAME_DISPATCH_LANGUAGES.has(node.language);
|
||||
const methods = nodes.filter(
|
||||
(node) => isHookShaped(node) && containers.get(node.id)?.kind === 'class'
|
||||
);
|
||||
if (methods.length === 0) return dropped;
|
||||
|
||||
const classes = new Map<string, Node>();
|
||||
for (const node of methods) {
|
||||
const container = containers.get(node.id);
|
||||
if (container) classes.set(container.id, container);
|
||||
}
|
||||
const registered = carriesDecorator(cg, [...classes.values()]);
|
||||
const eligible = methods.filter((node) => registered.has(containers.get(node.id)?.id ?? ''));
|
||||
if (eligible.length === 0) return dropped;
|
||||
|
||||
// Every class declaring a method of each name. A hook is called by its
|
||||
// framework and never by the code in the index, so each class's own
|
||||
// declaration of it is unreferenced too: it is in `unreferenced` already, and
|
||||
// the index need not be searched by name. (A name with a declaration that
|
||||
// something does call is the ambiguous-name rule's, which drops every
|
||||
// candidate of that name.)
|
||||
const names = new Set(eligible.map((node) => node.name));
|
||||
const declarations = unreferenced.filter((node) => names.has(node.name) && isHookShaped(node));
|
||||
const declaredIn = containersOf(
|
||||
cg,
|
||||
declarations.filter((node) => !containers.has(node.id))
|
||||
);
|
||||
const declaring = new Map<string, Set<string>>();
|
||||
for (const node of declarations) {
|
||||
const container = containers.get(node.id) ?? declaredIn.get(node.id);
|
||||
if (container?.kind !== 'class') continue;
|
||||
const ids = declaring.get(node.name);
|
||||
if (ids) ids.add(container.id);
|
||||
else declaring.set(node.name, new Set([container.id]));
|
||||
}
|
||||
|
||||
// The interfaces outside the index each of those classes writes out.
|
||||
const declarers = new Set([...declaring.values()].flatMap((ids) => [...ids]));
|
||||
const interfacesOf = cg.getUnresolvedReferenceNamesFrom(declarers, ['implements']);
|
||||
const interfaces = new Set([...interfacesOf.values()].flat());
|
||||
if (interfaces.size === 0) return dropped;
|
||||
|
||||
// Every TypeScript class writing each of those interfaces out, kept where
|
||||
// more than half of them are framework-created.
|
||||
const writers = cg.getUnresolvedReferenceSourcesNamed(interfaces, ['implements']);
|
||||
const writerIds = [...new Set([...writers.values()].flatMap((ids) => [...ids]))];
|
||||
const typescript = [...cg.getNodesByIds(writerIds).values()].filter((node) =>
|
||||
BY_NAME_DISPATCH_LANGUAGES.has(node.language)
|
||||
);
|
||||
const typescriptIds = new Set(typescript.map((node) => node.id));
|
||||
const registeredWriters = carriesDecorator(cg, typescript);
|
||||
const implementers = new Map<string, string[]>();
|
||||
for (const [name, ids] of writers) {
|
||||
const own = [...ids].filter((id) => typescriptIds.has(id));
|
||||
const registeredCount = own.filter((id) => registeredWriters.has(id)).length;
|
||||
if (registeredCount > own.length / 2) implementers.set(name, own);
|
||||
}
|
||||
if (implementers.size === 0) return dropped;
|
||||
|
||||
// What each implementer inherits from, and which inherit from outside the
|
||||
// index somewhere up the chain.
|
||||
const implementerIds = [...new Set([...implementers.values()].flat())];
|
||||
const ancestors = ancestorsOf(cg, implementerIds, ['extends']);
|
||||
const inherits = new Map<string, string[]>();
|
||||
for (const [ancestor, roots] of ancestors) {
|
||||
for (const root of roots) {
|
||||
const bucket = inherits.get(root);
|
||||
if (bucket) bucket.push(ancestor);
|
||||
else inherits.set(root, [ancestor]);
|
||||
}
|
||||
}
|
||||
const outside = cg.getUnresolvedReferenceNamesFrom([...implementerIds, ...ancestors.keys()], ['extends']);
|
||||
const opaque = new Set(implementerIds.filter((id) => outside.has(id)));
|
||||
for (const [ancestor, roots] of ancestors) {
|
||||
if (outside.has(ancestor)) for (const root of roots) opaque.add(root);
|
||||
}
|
||||
|
||||
const hookNames = new Set<string>();
|
||||
for (const name of names) {
|
||||
const declared = declaring.get(name);
|
||||
if (!declared) continue;
|
||||
const has = (id: string): boolean =>
|
||||
declared.has(id) ||
|
||||
opaque.has(id) ||
|
||||
(inherits.get(id) ?? []).some((ancestor) => declared.has(ancestor));
|
||||
const contracts = new Set([...declared].flatMap((id) => interfacesOf.get(id) ?? []));
|
||||
for (const contract of contracts) {
|
||||
const own = implementers.get(contract);
|
||||
if (!own) continue;
|
||||
const declaredCount = own.filter((id) => declared.has(id)).length;
|
||||
if (declaredCount >= 2 && declaredCount > own.length / 2 && own.every(has)) {
|
||||
hookNames.add(name);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const node of eligible) if (hookNames.has(node.name)) dropped.add(node.id);
|
||||
return dropped;
|
||||
}
|
||||
|
||||
@@ -1651,6 +1651,24 @@ export class CodeGraph {
|
||||
return this.queries.getUnresolvedSupertypeSourcesAmong(nodeIds);
|
||||
}
|
||||
|
||||
/**
|
||||
* The names each of the given symbols refers to, by reference kind, where
|
||||
* the resolver could not follow the reference — a decorator or an interface
|
||||
* from outside the index, which leaves no edge. Ids with none are absent.
|
||||
*/
|
||||
getUnresolvedReferenceNamesFrom(nodeIds: Iterable<string>, kinds: readonly Edge['kind'][]): Map<string, string[]> {
|
||||
return this.queries.getUnresolvedReferenceNamesFrom(nodeIds, kinds);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every symbol that refers to each of the given names, by reference kind,
|
||||
* where the resolver could not follow it: for `implements`, every class that
|
||||
* names a given interface from outside the index.
|
||||
*/
|
||||
getUnresolvedReferenceSourcesNamed(names: Iterable<string>, kinds: readonly Edge['kind'][]): Map<string, Set<string>> {
|
||||
return this.queries.getUnresolvedReferenceSourcesNamed(names, kinds);
|
||||
}
|
||||
|
||||
/**
|
||||
* The symbols with the most distinct dependents, most first — the index's
|
||||
* hubs. Distinct dependents, not edges: a helper called forty times from one
|
||||
|
||||
@@ -100,6 +100,7 @@ const EXCLUSION_LABELS: Record<keyof DeadCodeExclusions, string> = {
|
||||
declarations: 'abstract, or declared on an interface',
|
||||
decorated: 'carrying a decorator, so a framework registers them',
|
||||
overriding: 'overriding a member declared further up',
|
||||
hooks: 'framework hooks, called by name on classes that skip the interface',
|
||||
implicit: 'named something the language calls by itself',
|
||||
vendored: 'in vendored directories',
|
||||
testScope: 'inside a test module',
|
||||
|
||||
Reference in New Issue
Block a user