mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
* fix(dead-code): treat an ancestor outside the index as opaque (#1973) An override is excluded from the dead-code report when its ancestor is in the graph, or is present but declares no members we can read. An ancestor outside the project (React.Component, stream.Transform, a NestJS interface) creates no edge at all, so a lifecycle method the framework calls looked like it overrode nothing and was listed as unreferenced. The resolver does record such an ancestor, as a failed extends / implements row. A container with one, or a project ancestor of one that has it, now counts as having an opaque ancestor, and its members are excluded under "overriding". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dead-code): propagate external ancestor uncertainty to descendants (#1973) External ancestors leave unresolved inheritance references rather than graph edges. Treat those ancestors as opaque and propagate uncertainty through every descendant root. Track visited ancestor/root pairs while preserving cycle protection and the depth bound. Cover direct and three-level inheritance, unused controls, and batched query behavior. Co-authored-by: danusha2345 <ewidusoc498@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: danusha2345 <ewidusoc498@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@
|
||||
* Every one of those must be OFF the list, and the reason must be counted.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
|
||||
import * as http from 'http';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
@@ -483,3 +483,52 @@ describe('GET /api/map — generated files and islands', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('an ancestor outside the index (#1973)', () => {
|
||||
let root: string;
|
||||
let graph: CodeGraph;
|
||||
|
||||
beforeEach(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-deadcode-external-'));
|
||||
write(
|
||||
root,
|
||||
'src/clock.tsx',
|
||||
`import React from 'react';
|
||||
import { Transform } from 'stream';
|
||||
import { OnModuleInit } from '@nestjs/common';
|
||||
export class Clock extends React.Component { componentDidMount() {} render() { return null; } }
|
||||
export class Upper extends Transform { _transform(c, e, cb) { cb(null, c); } }
|
||||
export class Boot implements OnModuleInit { onModuleInit() {} }
|
||||
export class Ticker extends Clock { componentDidUpdate() {} }
|
||||
export class Third extends Ticker { componentWillUnmount() {} componentDidCatch() {} }
|
||||
export class Plain { neverCalledMember() {} }
|
||||
function reallyUnused() {}
|
||||
`
|
||||
);
|
||||
write(root, 'src/main.ts', `import { Clock, Upper, Boot, Ticker, Third, Plain } from './clock';\nexport const all = [Clock, Upper, Boot, Ticker, Third, Plain];\n`);
|
||||
graph = CodeGraph.initSync(root, { config: { include: ['src/**/*.ts', 'src/**/*.tsx'], exclude: [] } });
|
||||
await graph.indexAll();
|
||||
}, 60_000);
|
||||
|
||||
afterEach(() => {
|
||||
graph?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('does not list members a framework base class calls', () => {
|
||||
const report = buildDeadCodeReport(graph);
|
||||
for (const name of ['componentDidMount', 'componentWillUnmount', 'render', '_transform', 'onModuleInit']) {
|
||||
expect(names(report)).not.toContain(name);
|
||||
}
|
||||
// A project class whose own ancestor extends an external base inherits the doubt.
|
||||
expect(names(report)).not.toContain('componentDidUpdate');
|
||||
expect(names(report)).not.toContain('componentDidCatch');
|
||||
expect(report.excluded.overriding).toBeGreaterThanOrEqual(7);
|
||||
});
|
||||
|
||||
it('still lists what nothing reaches outside such a class', () => {
|
||||
const report = buildDeadCodeReport(graph);
|
||||
expect(names(report)).toContain('reallyUnused');
|
||||
expect(names(report)).toContain('neverCalledMember');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { DatabaseConnection } from '../src/db';
|
||||
import { QueryBuilder } from '../src/db/queries';
|
||||
import type { Node } from '../src/types';
|
||||
|
||||
function node(id: string): Node {
|
||||
return { id, kind: 'class', name: id, qualifiedName: id, filePath: 'fixture.ts',
|
||||
language: 'typescript', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: Date.now() };
|
||||
}
|
||||
|
||||
describe('getUnresolvedSupertypeSourcesAmong (#1973)', () => {
|
||||
let root: string;
|
||||
let db: DatabaseConnection;
|
||||
let queries: QueryBuilder;
|
||||
|
||||
beforeEach(() => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-supertype-query-'));
|
||||
db = DatabaseConnection.initialize(path.join(root, 'test.db'));
|
||||
queries = new QueryBuilder(db.getDb());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
db?.close();
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('selects only requested inheritance sources, including pending and failed refs', () => {
|
||||
for (const [id, kind] of [['base', 'extends'], ['contract', 'implements'], ['call', 'calls'], ['other', 'extends']] as const) {
|
||||
queries.insertNode(node(id));
|
||||
queries.insertUnresolvedRef({ fromNodeId: id, referenceName: 'External', referenceKind: kind, line: 1, column: 0 });
|
||||
}
|
||||
queries.markReferencesFailed([{ fromNodeId: 'contract', referenceName: 'External', referenceKind: 'implements' }]);
|
||||
expect(queries.getUnresolvedSupertypeSourcesAmong(['base', 'contract', 'call', 'missing', 'base']))
|
||||
.toEqual(new Set(['base', 'contract']));
|
||||
expect(queries.getUnresolvedSupertypeSourcesAmong([])).toEqual(new Set());
|
||||
});
|
||||
|
||||
it('keeps sources across chunk boundaries', () => {
|
||||
const ids = Array.from({ length: 1001 }, (_, i) => `class-${i}`);
|
||||
for (const id of [ids[0], ids[500], ids[1000]]) {
|
||||
queries.insertNode(node(id));
|
||||
queries.insertUnresolvedRef({ fromNodeId: id, referenceName: 'External', referenceKind: 'extends', line: 1, column: 0 });
|
||||
}
|
||||
expect(queries.getUnresolvedSupertypeSourcesAmong(ids)).toEqual(new Set([ids[0], ids[500], ids[1000]]));
|
||||
});
|
||||
});
|
||||
@@ -2124,6 +2124,31 @@ export class QueryBuilder {
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of `nodeIds` extend or implement a type the resolver could not follow.
|
||||
* An ancestor outside the index (`React.Component`, `stream.Transform`, a
|
||||
* framework interface) leaves no edge, only this `extends` / `implements`
|
||||
* row, so it is the one record that such an ancestor exists (#1973).
|
||||
* Chunked probe over `idx_unresolved_from_node`.
|
||||
*/
|
||||
getUnresolvedSupertypeSourcesAmong(nodeIds: Iterable<string>): Set<string> {
|
||||
const unique = [...new Set(nodeIds)];
|
||||
const found = new Set<string>();
|
||||
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
|
||||
WHERE from_node_id IN (${placeholders})
|
||||
AND reference_kind IN ('extends', 'implements')`
|
||||
)
|
||||
.all(...chunk) as Array<{ id: string }>;
|
||||
for (const row of rows) found.add(row.id);
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of `names` are carried by MORE THAN ONE symbol, at least one of which
|
||||
* something points at.
|
||||
|
||||
+26
-9
@@ -797,6 +797,13 @@ function containersOf(cg: CodeGraph, nodes: readonly Node[]): Map<string, Node>
|
||||
* only choice that keeps the list's promise; the alternative puts every
|
||||
* implementation of every signature-only interface at the top of a screen that
|
||||
* says "nothing reaches this".
|
||||
*
|
||||
* An ancestor **outside the index** is opaque in the same way, and more so: a
|
||||
* class that extends `React.Component` or `stream.Transform`, or implements a
|
||||
* framework interface, has lifecycle methods the framework calls, and the
|
||||
* ancestor leaves no edge to walk at all — only the resolver's failed
|
||||
* `extends` / `implements` row. A container with such a row, or a project
|
||||
* ancestor of one that has it, counts as having an opaque ancestor (#1973).
|
||||
*/
|
||||
function overrideCandidates(
|
||||
cg: CodeGraph,
|
||||
@@ -811,7 +818,7 @@ function overrideCandidates(
|
||||
// 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 Set<string>(containerIds);
|
||||
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++) {
|
||||
@@ -826,21 +833,32 @@ function overrideCandidates(
|
||||
const roots = rootsOf.get(edge.source);
|
||||
if (!roots) continue;
|
||||
const merged = next.get(edge.target) ?? new Set<string>();
|
||||
for (const root of roots) merged.add(root);
|
||||
next.set(edge.target, merged);
|
||||
const visited = seen.get(edge.target) ?? new Set<string>();
|
||||
const known = reach.get(edge.target) ?? new Set<string>();
|
||||
for (const root of roots) known.add(root);
|
||||
reach.set(edge.target, known);
|
||||
// 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) {
|
||||
if (seen.has(id)) continue;
|
||||
seen.add(id);
|
||||
frontier.push({ id, roots });
|
||||
}
|
||||
}
|
||||
|
||||
if (reach.size === 0) return dropped;
|
||||
const opaqueContainers = new Set<string>();
|
||||
const externalBased = cg.getUnresolvedSupertypeSourcesAmong([...containerIds, ...reach.keys()]);
|
||||
for (const id of containerIds) if (externalBased.has(id)) opaqueContainers.add(id);
|
||||
for (const [ancestorId, roots] of reach) {
|
||||
if (externalBased.has(ancestorId)) for (const root of roots) opaqueContainers.add(root);
|
||||
}
|
||||
|
||||
// What each ancestor declares, and whether it declares anything at all.
|
||||
const ancestorIds = [...reach.keys()];
|
||||
@@ -855,7 +873,6 @@ function overrideCandidates(
|
||||
|
||||
/** Member names an ancestor declares, and whether it declares none we can read. */
|
||||
const namesByContainer = new Map<string, Set<string>>();
|
||||
const opaqueContainers = new Set<string>();
|
||||
for (const [ancestorId, roots] of reach) {
|
||||
const names: string[] = [];
|
||||
for (const memberId of memberIdsByAncestor.get(ancestorId) ?? []) {
|
||||
|
||||
@@ -1479,6 +1479,14 @@ export class CodeGraph {
|
||||
return this.queries.getUnresolvedNamesAmong(names);
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of the given symbols extend or implement a type outside the index —
|
||||
* an ancestor the resolver could not follow, so it has no edge (#1973).
|
||||
*/
|
||||
getUnresolvedSupertypeSourcesAmong(nodeIds: Iterable<string>): Set<string> {
|
||||
return this.queries.getUnresolvedSupertypeSourcesAmong(nodeIds);
|
||||
}
|
||||
|
||||
/**
|
||||
* The symbols with the most distinct dependents, most first — the index's
|
||||
* hubs. Distinct dependents, not edges: a helper called forty times from one
|
||||
|
||||
Reference in New Issue
Block a user