Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 231 additions & 0 deletions __tests__/dead-code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
6 changes: 4 additions & 2 deletions docs/design/codegraph-ui-design-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 2 additions & 0 deletions docs/viewer-launch-changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
60 changes: 55 additions & 5 deletions src/db/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...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) as Array<{ id: string }>;
for (const row of rows) found.add(row.id);
.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;
}
Expand Down
Loading