diff --git a/CHANGELOG.md b/CHANGELOG.md index ba875b7..cce5174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,14 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Members that redeclare something from a type above are marked in the outline ("overrides Base", or "satisfies Clock" for an interface), so a 40-member class shows at a glance which parts are its own and which are a contract it is filling. The number of implementations shown here is the same number `codegraph_explore` reports to your agent when it announces an interface dispatch. +- **Find the code nothing reaches, in `codegraph ui`.** A new **Dead code** tab lists the symbols no import, call or reference anywhere in your project reaches — biggest first, grouped by the file they live in, with the number of lines each one would take with it. A class nobody uses brings its methods along as a single finding rather than eleven. Every row opens the code. + + The screen is built to be believed rather than to look impressive. A line above the list says, and keeps saying, that this means "no static reference in the index" and not "unused" — reflection, a framework registry and a template can all reach code a graph cannot follow. Under the list, every reason a candidate was left off is printed with its count, so you can see the list is twenty findings out of two and a half thousand candidates rather than twenty out of twenty-one. + + Those exclusions are the feature. Anything exported, or declared in a header, is off the list by default, because something outside your repository can import it — one switch adds them back, with a warning band. So are test and generated files, abstract and interface declarations, anything a decorator registers, members that override something further up, names the language calls for you (`constructor`, `__enter__`, `main`), vendored directories, and files nothing in your project reaches at all. Two more rules catch what the graph itself missed: a name CodeGraph failed to resolve somewhere is never called unreferenced, and neither is a name shared with a symbol that *is* used — the twin may simply have been picked instead. Last, before any row is shown, the files that could reach it are read and the identifier counted: written down twice, something uses it and we did not see it. + + On the **Map**, a module nothing depends on now says so in its own count line instead of counting itself — usually your entry points, sometimes something you forgot to delete. Tool-generated files and modules are dimmed wherever they appear: on the map, in its file list, in search results and on the file screen. + ### Fixes - Fixed a long-running `codegraph ui` session serving a symbol that a sync had already deleted. The viewer keeps one connection to your index open, and its in-memory lookup didn't notice when another process — your agent's sync, or `codegraph sync` — rewrote the file underneath it, so a symbol screen could keep showing a body with no callers while search correctly reported it had moved. Because a symbol's identity includes the line it starts on, this happened after almost any edit above it. diff --git a/CLAUDE.md b/CLAUDE.md index c1d97af..862e7d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,8 @@ The public API surface is `src/index.ts` — the `CodeGraph` class wires all the - `src/db/` — `DatabaseConnection`, `QueryBuilder` (prepared statements), `schema.sql`, `sqlite-adapter.ts`. Backed by Node's built-in **`node:sqlite`** (`DatabaseSync`) — real SQLite with WAL + FTS5, exposed through a thin better-sqlite3-shaped adapter. The bundled runtime always ships Node ≥22.5, so `node:sqlite` is always available: **no native build step and no wasm fallback**. (Running from source needs Node ≥22.5.) `codegraph status` reports the live backend (`node-sqlite`, the sole backend). - `src/extraction/` — `ExtractionOrchestrator`, tree-sitter wrappers, per-language extractors under `languages/` (one file per language), plus standalone extractors for non-tree-sitter formats (`svelte-extractor.ts`, `vue-extractor.ts`, `liquid-extractor.ts`, `dfm-extractor.ts` for Delphi). `parse-worker.ts` runs heavy parsing off the main thread. - `src/resolution/` — `ReferenceResolver` orchestrates `import-resolver.ts` (with `path-aliases.ts` for tsconfig path aliases + cargo workspace member globs), `name-matcher.ts`, and `frameworks/` (Express, Laravel, Rails, FastAPI, Django, Flask, Spring, Gin, Axum, ASP.NET, Vapor, React Router, SvelteKit, Vue/Nuxt, Cargo workspaces). Frameworks emit `route` nodes and `references` edges. -- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. +- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws), + `dead-code.ts` (unreferenced symbols, and every reason a candidate is NOT claimed). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. - `src/context/` — `ContextBuilder` + formatter for markdown/JSON output. - `src/search/` — full-text query parser and helpers for FTS5. - `src/sync/` — `FileWatcher` (native FSEvents/inotify/RDCW) with debounce + filter, and git-hook helpers. diff --git a/__tests__/dead-code.test.ts b/__tests__/dead-code.test.ts new file mode 100644 index 0000000..d946c46 --- /dev/null +++ b/__tests__/dead-code.test.ts @@ -0,0 +1,485 @@ +/** + * Dead code and islands (CG-59). + * + * Two halves, both against a real indexed fixture: the derivation in + * `src/graph/dead-code.ts`, and the `/api/deadcode` endpoint that renders it + * over a real loopback server, like the rest of the viewer's API suite. + * + * The fixture is shaped to produce, deliberately, one of each thing the report + * has to get RIGHT BY NOT CLAIMING IT: + * + * - a genuinely unreferenced helper (the only row that should survive); + * - a same-name pair where the resolver attaches the call to the wrong one — + * the mis-resolution that makes a used method look unreached; + * - a method that overrides a base's, reached only through the base; + * - a decorated method, registered by a framework the graph cannot see; + * - a helper only a template mentions, so no edge records the use but the file + * text does; + * - an exported function nothing here calls, which an outside caller may. + * + * Every one of those must be OFF the list, and the reason must be counted. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as http from 'http'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { + buildDeadCodeReport, + isHeaderFile, + isImplicitEntryName, + isTestScope, + isVendoredPath, + mentionCount, + DEAD_CODE_KINDS, +} from '../src/graph/dead-code'; +import { createGraphApi, startUiServer, type GraphApi, type UiServerHandle } from '../src/ui-server'; + +let server: UiServerHandle; +let api: GraphApi; +let tempDir: string; +let projectRoot: string; +let cg: CodeGraph; + +function write(root: string, rel: string, body: string): void { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, body); +} + +function request(requestPath: string): Promise<{ status: number; body: string; type?: string }> { + return new Promise((resolve, reject) => { + const req = http.request( + { + host: '127.0.0.1', + port: server.port, + path: requestPath, + method: 'GET', + headers: { Host: `127.0.0.1:${server.port}` }, + setHost: false, + }, + (res) => { + const chunks: Buffer[] = []; + res.on('data', (c: Buffer) => chunks.push(c)); + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + body: Buffer.concat(chunks).toString('utf-8'), + type: res.headers['content-type'], + }) + ); + } + ); + req.on('error', reject); + req.end(); + }); +} + +async function getDeadCode(query = ''): Promise { + const res = await request(`/api/deadcode${query}`); + expect(res.type).toBe('application/json; charset=utf-8'); + expect(res.status).toBe(200); + return JSON.parse(res.body); +} + +const names = (report: { entries: Array<{ node: { name: string } }> }): string[] => + report.entries.map((entry) => entry.node.name); + +beforeAll(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-deadcode-')); + projectRoot = path.join(tempDir, 'project'); + + // The one genuinely dead symbol, plus a live one beside it so the file is + // reached and the island rule does not swallow the whole thing. + write( + projectRoot, + 'src/util.ts', + `export function used(value: string): string { + return value.trim(); +} + +function neverCalledAnywhere(value: string): string { + return value.toUpperCase(); +} + +function alsoDeadButSmaller(): number { + return 1; +} + +// Exported and never called here — an outside caller may import it, so the +// default list must not claim it. It lives in a REACHED file on purpose: an +// unreached file is an island, which is a different exclusion. +export function publicEntryPoint(): string { + return 'hello'; +} +` + ); + + // The mis-resolution: \`Facade.load\` calls \`this.inner.load()\`, and the + // resolver prefers a same-name definition in the call site's own file. One of + // the two ends up with no incoming edge and neither is unreferenced. + write( + projectRoot, + 'src/inner.ts', + `export class Inner { + load(): string { + return 'inner'; + } +} +` + ); + + // A base and an override: calls land on \`Base.run\`, never on \`Child.run\`. + write( + projectRoot, + 'src/base.ts', + `export class Base { + run(): string { + return 'base'; + } +} +` + ); + write( + projectRoot, + 'src/child.ts', + `import { Base } from './base'; + +export class Child extends Base { + run(): string { + return 'child'; + } +} +` + ); + + write( + projectRoot, + 'src/facade.ts', + `import { Inner } from './inner'; +import { Base } from './base'; +import { Child } from './child'; +import { used } from './util'; + +function register(target: unknown, key: string): void { + void target; + void key; +} + +export class Facade { + inner = new Inner(); + child = new Child(); + + load(): string { + return this.inner.load(); + } + + go(): string { + const base: Base = this.child; + return used(base.run()) + this.load(); + } + + @register + onEvent(): void { + void 0; + } +} +` + ); + + // Mentioned in a template but never called anywhere the graph can see: the + // corroboration pass has to find the second mention in this file's own text. + write( + projectRoot, + 'src/handlers.ts', + `export function mountHandlers(): string { + return TEMPLATE; +} + +function onSubmit(): void { + void 0; +} + +const TEMPLATE = '
'; +` + ); + + // Nothing imports this file at all: its symbols' zero fan-in describes the + // file, not the symbol. That is the island rule, and it is the Map's job. + write( + projectRoot, + 'src/orphan.ts', + `function strandedHelper(): string { + return 'nobody imports this file'; +} + +function alsoStranded(): number { + return strandedHelper().length; +} +` + ); + + write( + projectRoot, + 'src/index.ts', + `import { Facade } from './facade'; +import { mountHandlers } from './handlers'; + +export function start(): string { + return new Facade().go() + mountHandlers(); +} +` + ); + + // A test helper file with a dependent, so `includeTests` is what decides + // whether its dead symbol shows — not the island rule. + write( + projectRoot, + 'tests/helpers.ts', + `export function sharedHelper(): string { + return 'shared'; +} + +function helperNothingCalls(): void { + void 0; +} +` + ); + write( + projectRoot, + 'tests/facade.test.ts', + `import { Facade } from '../src/facade'; +import { sharedHelper } from './helpers'; + +export function testFacade(): string { + return new Facade().go() + sharedHelper(); +} +` + ); + + const init = CodeGraph.initSync(projectRoot, { + config: { include: ['src/**/*.ts', 'tests/**/*.ts'], exclude: [] }, + }); + await init.indexAll(); + init.resolveReferences(); + init.close(); + + cg = CodeGraph.openSync(projectRoot); + + const viewerDir = path.join(tempDir, 'viewer'); + fs.mkdirSync(viewerDir, { recursive: true }); + fs.writeFileSync(path.join(viewerDir, 'index.html'), '
'); + + api = createGraphApi({ projectRoot }); + server = await startUiServer({ projectRoot, viewerDir, port: 0, api: api.handler }); +}, 120_000); + +afterAll(async () => { + cg?.close(); + api?.close(); + await server?.close(); + if (tempDir) fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +describe('buildDeadCodeReport', () => { + it('finds the symbol nothing references', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).toContain('neverCalledAnywhere'); + }); + + it('leaves nothing on the list that anything reaches', () => { + const report = buildDeadCodeReport(cg); + // `used`, `start`, `go` and `mountHandlers` are all called; `Inner.load` + // and `Facade.load` are the same-name pair; `Child.run` is an override. + for (const name of ['used', 'start', 'go', 'mountHandlers', 'load', 'run']) { + expect(names(report)).not.toContain(name); + } + }); + + it('excludes a symbol only its own file mentions, and counts it', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).not.toContain('onSubmit'); + expect(report.excluded.mentioned).toBeGreaterThan(0); + expect(report.corroborated).toBe(true); + }); + + it('makes the claim when corroboration is switched off', () => { + // The rule that catches `onSubmit` is the only one that reads a file, so + // turning it off has to be visible in BOTH the list and the flag. + const report = buildDeadCodeReport(cg, { readSource: null }); + expect(report.corroborated).toBe(false); + expect(report.excluded.mentioned).toBe(0); + expect(names(report)).toContain('onSubmit'); + }); + + it('excludes exported symbols by default and includes them on request', () => { + const strict = buildDeadCodeReport(cg); + expect(names(strict)).not.toContain('publicEntryPoint'); + expect(strict.excluded.exported).toBeGreaterThan(0); + expect(strict.includeExported).toBe(false); + + const wide = buildDeadCodeReport(cg, { includeExported: true }); + expect(names(wide)).toContain('publicEntryPoint'); + expect(wide.includeExported).toBe(true); + expect(wide.excluded.exported).toBe(0); + }); + + it('excludes test files by default and includes them on request', () => { + expect(names(buildDeadCodeReport(cg))).not.toContain('helperNothingCalls'); + expect(buildDeadCodeReport(cg).excluded.tests).toBeGreaterThan(0); + expect(names(buildDeadCodeReport(cg, { includeTests: true }))).toContain( + 'helperNothingCalls' + ); + }); + + it('says nothing about a file nothing in the index reaches', () => { + // An island's symbols have zero fan-in because the FILE is unreached, which + // is a fact about the file — the Map draws it, this list does not claim it. + const report = buildDeadCodeReport(cg, { includeExported: true }); + expect(names(report)).not.toContain('strandedHelper'); + expect(report.excluded.unreachableFile).toBeGreaterThan(0); + }); + + it('excludes a decorated member — a framework registers it', () => { + const report = buildDeadCodeReport(cg); + expect(names(report)).not.toContain('onEvent'); + expect(report.excluded.decorated).toBeGreaterThan(0); + }); + + it('ranks by size and reports the real total when capped', () => { + const full = buildDeadCodeReport(cg); + const sizes = full.entries.map((entry) => entry.lines); + expect([...sizes].sort((a, b) => b - a)).toEqual(sizes); + + const capped = buildDeadCodeReport(cg, { limit: 1 }); + expect(capped.entries).toHaveLength(1); + expect(capped.total).toBe(full.total); + // The cap trims the tail, not the head: the biggest finding survives. + expect(capped.entries[0]?.node.name).toBe(full.entries[0]?.node.name); + }); + + it('every exclusion count is a number of candidates, and they add up', () => { + const report = buildDeadCodeReport(cg); + const excluded = Object.values(report.excluded).reduce((sum, n) => sum + n, 0); + expect(report.candidates).toBeGreaterThan(0); + expect(excluded + report.entries.length).toBeLessThanOrEqual(report.candidates); + expect(report.bounded).toBe(false); + }); + + it('restricts to the kinds asked for, and ignores nonsense', () => { + const classesOnly = buildDeadCodeReport(cg, { kinds: ['class'] }); + expect(classesOnly.kinds).toEqual(['class']); + for (const entry of classesOnly.entries) expect(entry.node.kind).toBe('class'); + + // An unknown kind is not a 500 and not an empty list: it falls back to the + // default set, which is the answer the caller meant. + const nonsense = buildDeadCodeReport(cg, { kinds: ['banana' as never] }); + expect(nonsense.kinds).toEqual([...DEAD_CODE_KINDS]); + }); +}); + +describe('the rules that are pure', () => { + it('counts whole-identifier mentions only', () => { + expect(mentionCount('const load = 1; loader(); reload();', 'load')).toBe(1); + expect(mentionCount('a.load(); load();', 'load')).toBe(2); + expect(mentionCount('nothing here', 'load')).toBe(0); + // Stops early: the caller only ever needs to know "one, or more than one". + expect(mentionCount('x x x x x', 'x', 2)).toBe(2); + }); + + it('matches vendored directories as whole segments', () => { + expect(isVendoredPath('vendor/lib/a.go')).toBe(true); + expect(isVendoredPath('a/node_modules/b/c.js')).toBe(true); + expect(isVendoredPath('src/vendored-parser.ts')).toBe(false); + }); + + it('recognises headers as declaration surfaces', () => { + expect(isHeaderFile('src/tree_sitter/parser.h')).toBe(true); + expect(isHeaderFile('types/global.d.ts')).toBe(true); + expect(isHeaderFile('src/parser.c')).toBe(false); + }); + + it('recognises a test scope inside a file', () => { + expect(isTestScope('tests::row_sizes_match')).toBe(true); + expect(isTestScope('Fixtures.Tests.Helper')).toBe(true); + expect(isTestScope('Latest.value')).toBe(false); + }); + + it('recognises names the language calls by itself', () => { + expect(isImplicitEntryName('constructor')).toBe(true); + expect(isImplicitEntryName('__enter__')).toBe(true); + expect(isImplicitEntryName('ToString')).toBe(true); + expect(isImplicitEntryName('mainHandler')).toBe(false); + }); +}); + +describe('GET /api/deadcode', () => { + it('groups the rows by file and keeps the totals honest', async () => { + const payload = await getDeadCode(); + expect(payload.rows.total).toBe(payload.rows.items.length); + expect(payload.rows.shown).toBe(payload.rows.items.length); + + // Every count equals a list length in the same payload. + const grouped = payload.groups.reduce((sum: number, g: any) => sum + g.rows.length, 0); + expect(grouped).toBe(payload.rows.shown); + + const files = payload.groups.map((g: any) => g.file); + expect(new Set(files).size).toBe(files.length); + expect(files).toContain('src/util.ts'); + }); + + it('carries the exclusions with their own wording', async () => { + const payload = await getDeadCode(); + expect(payload.excluded.length).toBeGreaterThan(0); + for (const entry of payload.excluded) { + expect(entry.count).toBeGreaterThan(0); + expect(typeof entry.label).toBe('string'); + expect(entry.label.length).toBeGreaterThan(0); + } + const sum = payload.excluded.reduce((n: number, e: any) => n + e.count, 0); + expect(payload.excludedTotal).toBe(sum); + expect(payload.candidates).toBeGreaterThanOrEqual(payload.excludedTotal); + expect(payload.corroborated).toBe(true); + }); + + it('widens on ?exported=1 and says which list it answered', async () => { + const strict = await getDeadCode(); + const wide = await getDeadCode('?exported=1'); + expect(strict.includeExported).toBe(false); + expect(wide.includeExported).toBe(true); + expect(wide.rows.total).toBeGreaterThan(strict.rows.total); + expect(wide.rows.items.some((r: any) => r.name === 'publicEntryPoint')).toBe(true); + }); + + it('honours ?limit= without lying about the total', async () => { + const full = await getDeadCode(); + const capped = await getDeadCode('?limit=1'); + expect(capped.rows.items).toHaveLength(1); + expect(capped.rows.total).toBe(full.rows.total); + expect(capped.rows.truncated).toBe(full.rows.total > 1); + }); + + it('is listed on the API index', async () => { + const res = await request('/api'); + const body = JSON.parse(res.body); + expect(body.endpoints.some((e: any) => e.path === '/api/deadcode')).toBe(true); + }); +}); + +describe('GET /api/map — generated files and islands', () => { + it('reports how many of a module’s files are tool-generated', async () => { + const res = await request('/api/map'); + const payload = JSON.parse(res.body); + for (const module of payload.modules) { + expect(typeof module.generated).toBe('number'); + expect(module.generated).toBeLessThanOrEqual(module.files); + // The dimmed rows are drawn from `fileList.items`, so the generated + // subset has to be a subset of exactly that list. + for (const file of module.generatedFiles) { + expect(module.fileList.items).toContain(file); + } + } + }); +}); diff --git a/__tests__/ui-package.test.ts b/__tests__/ui-package.test.ts index 46042a0..abd90c5 100644 --- a/__tests__/ui-package.test.ts +++ b/__tests__/ui-package.test.ts @@ -391,6 +391,21 @@ function mockAdapter(): { adapter: GraphAdapter; calls: string[] } { index: { lastIndexedAt: null, files: 3 }, timing: { elapsedMs: 1, cached: false }, }), + deadCode: () => + seen('deadCode', { + rows: { total: 0, shown: 0, truncated: false, items: [] }, + groups: [], + candidates: 0, + excluded: [], + excludedTotal: 0, + kinds: ['function'], + includeExported: false, + includeTests: false, + includeGenerated: false, + bounded: false, + corroborated: true, + timing: { elapsedMs: 1 }, + }), // Deliberately no `events`: a host without a live channel is the normal // case, and nothing may poll in its absence. }; diff --git a/docs/design/codegraph-ui-design-spec.md b/docs/design/codegraph-ui-design-spec.md index c2ef3d6..5407609 100644 --- a/docs/design/codegraph-ui-design-spec.md +++ b/docs/design/codegraph-ui-design-spec.md @@ -337,6 +337,46 @@ Nothing in the engine emits an `overrides` edge, so this is a NAME match inside says so. It is deliberately blind to signatures — an overload set would need type resolution the graph does not have. Layout is arithmetic (row height × index): no `ResizeObserver`, no measurement, same payload → same picture. +### 3.11 Dead code and islands (CG-59) +A screen (`#/dead`, `?exported=1`) and a mark on the Map. + +**The list.** Symbols no import, call or reference in the index reaches, ranked largest first and grouped by file with the +Symbol view's `.filegroup` / `.row` shapes (design spec §3.2) — file path 11px mono `--ink-3` with the group's +"N symbols · M lines" opposite it, then rows of kind glyph + 12.5px mono name + 11px mono `file:line` + an 11px `--ink-3` meta +line ("method · 51 lines"). A dead container folds its unreachable members into a wrapped strip of 11px mono links under it +rather than listing them as siblings — one finding, not eleven. Column max-width **760px**, 40px gutters, exactly like the +entry-points panel. + +**The caveat is part of the screen, not a note on it.** A persistent 11.5px `--ink-3` line sits above the rows, between two +hairline rules, and never collapses or dismisses: *"No static reference in the index — dynamic use is possible."* Under the list, +every reason a candidate was left off is printed with its count ("1 677 in test files", "378 exported, or declared in a header", +"40 overriding a member declared further up"), preceded by the scale — *"2 494 symbols in this index carry no incoming reference +at all; 2 474 of them were left off this list."* Twenty rows drawn from twenty candidates and twenty drawn from two and a half +thousand are different screens and only that sentence tells them apart. + +**One switch**, an 11px mono chip on the right of the caveat bar: `Internal only` (default) ↔ `Including exported`, carried in +the URL. Turning it on adds symbols something outside the repository could import, and the screen grows an `--accent-soft` band +with an `--accent-line` border saying so; each such row also carries an `exported` chip. Exported rows are never on the default +list, because the index cannot check a caller it does not contain. + +**Islands, on the Map.** A module no link in the payload arrives at keeps its normal 1px `--ink` stroke — it is not a lesser +module, it is an unreached one — and its 11px count line reads **"nothing depends on this"** in `--ink-2` *instead of* the +symbol/file counts, which stay in the side panel. The island verdict is computed from the whole link set, so hiding test modules +cannot manufacture one. Selecting the module adds a sentence in the panel. Note the box is sized from whichever string it will +show, so the layout and the node must be given the same verdict. + +**Generated files recede everywhere** (`files.generated`, §2.6): a module whose files are *all* tool-generated draws in +`--ink-4` with a `--rule-soft` stroke; a generated file in the Map panel's file list, a generated group on the dead code list, +a generated result in the search palette and a generated file's title in the File view are all `--ink-4`. Partly-generated +modules are not dimmed — a module with one `.pb.go` in it is still one somebody writes by hand. + +**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 +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. + ## 4. Libraries and versions - Svelte 5 (≥ 5.25) + Vite (workspace `ui/`), Svelte Flow `@xyflow/svelte` ^1.6 for the Map and Flow canvases only (custom nodes/edges, hidden handles for port spreading, local selection state — the pattern in docker-app's `StackGraph.svelte`); `@dagrejs/dagre` only as a diff --git a/scripts/check-ui-package.mjs b/scripts/check-ui-package.mjs index 6a0a04f..399c4ea 100644 --- a/scripts/check-ui-package.mjs +++ b/scripts/check-ui-package.mjs @@ -141,6 +141,7 @@ for (const name of [ 'TypeHierarchy', 'FlowStrip', 'ArchitectureMap', + 'DeadCodeView', 'TrailBar', 'SearchPalette', 'CodegraphUi', diff --git a/src/db/queries.ts b/src/db/queries.ts index 2de4d9c..f462637 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -1979,6 +1979,173 @@ export class QueryBuilder { return out; } + /** + * Symbols nothing in the index points at — the candidate set behind the dead + * code list (`src/graph/dead-code.ts`). + * + * "Points at" is every edge kind EXCEPT `contains`: a class containing a + * method is structure, not use, and counting it would make every member look + * reached by its own container. A self-edge is excluded for the same reason + * a recursive function is not its own caller. + * + * One scan, one index probe per candidate. `NOT EXISTS` over + * `idx_edges_target_kind` is what keeps it that way — the alternative + * (`LEFT JOIN edges … GROUP BY`) builds a row per edge for the whole table + * before discarding all but the empty groups. Ordered by position so the + * answer is stable across runs and groups by file without a second sort. + * + * The result is deliberately NOT called dead code: an unreferenced symbol is + * a symbol with no STATIC reference, and the caller applies the exclusions + * (tests, generated files, overrides, unresolved names) that turn the + * candidate set into a claim worth making. + */ + getUnreferencedNodes( + kinds: readonly string[], + limit: number + ): Array<{ node: Node; generated: boolean }> { + if (kinds.length === 0 || limit <= 0) return []; + const placeholders = kinds.map(() => '?').join(','); + const rows = this.db + .prepare( + `SELECT n.*, COALESCE(f.generated, 0) AS file_generated + FROM nodes n + LEFT JOIN files f ON f.path = n.file_path + WHERE n.kind IN (${placeholders}) + AND NOT EXISTS ( + SELECT 1 FROM edges e + WHERE e.target = n.id + AND e.kind != 'contains' + AND e.source != n.id + ) + ORDER BY n.file_path, n.start_line, n.name + LIMIT ?` + ) + .all(...kinds, limit) as Array; + return rows.map((row) => ({ node: rowToNode(row), generated: row.file_generated === 1 })); + } + + /** + * Which of `names` the index holds an UNRESOLVED reference to. + * + * The point is honesty about our own blind spots. A `failed` row in + * `unresolved_refs` records that some file referenced a name and the resolver + * could not decide what it meant — so a symbol with that name cannot be + * called unreferenced, whatever the edge table says. It is deliberately + * matched loosely, on the reference name AND on its tail (`util.greet` → + * `greet`), because the question being asked is "could this name be the one + * we failed to follow", and a maybe has to count as a yes. + * + * Bounded-lookup like {@link getGeneratedPathsAmong}: the caller holds a + * candidate list, so this is a chunked probe over `idx_unresolved_name`, not + * a scan of the table. + */ + getUnresolvedNamesAmong(names: Iterable): Set { + const unique = [...new Set(names)].filter((name) => name.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + 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 unresolved_refs + WHERE reference_name IN (${placeholders}) + UNION + SELECT DISTINCT name_tail AS name FROM unresolved_refs + WHERE name_tail IN (${placeholders})` + ) + .all(...chunk, ...chunk) as Array<{ name: string }>; + for (const row of rows) found.add(row.name); + } + return found; + } + + /** + * Which of `names` are carried by MORE THAN ONE symbol, at least one of which + * something points at. + * + * The false positive this exists to kill: `CodeGraph.getTopRouteFile` calls + * `this.queries.getTopRouteFile()`, and the resolver — which prefers a + * same-name definition in the call site's own file — attaches that edge to + * the *calling* method. One of the two ends up with a self-edge and the other + * with nothing at all, and neither is unreferenced. From the edge table the + * mis-resolution and a genuinely unused twin are the same picture, so the + * claim is not made about either. + * + * Both halves of the condition are load-bearing. **More than one symbol**: + * a uniquely-named function that only calls itself is genuinely dead, and + * excluding every recursive function would gut the list. **Self-edges + * counted**: the self-edge IS the fingerprint of the mis-resolution above, so + * it has to count as evidence that this name resolves somewhere. + * + * Chunked probe over `idx_nodes_name`, bounded by the caller's candidate list. + */ + getAmbiguousReferencedNames(names: Iterable): Set { + const unique = [...new Set(names)].filter((name) => name.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + 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 name FROM ( + SELECT n.name AS name, + EXISTS ( + SELECT 1 FROM edges e + WHERE e.target = n.id AND e.kind != 'contains' + ) AS referenced + FROM nodes n + WHERE n.name IN (${placeholders}) + ) + GROUP BY name + HAVING COUNT(*) > 1 AND SUM(referenced) > 0` + ) + .all(...chunk) as Array<{ name: string }>; + for (const row of rows) found.add(row.name); + } + return found; + } + + /** + * Which of the given languages the index records an EXPORT marker for. + * + * A self-measurement, and the honest basis for a whole class of exclusion. + * The dead code report's strongest filter is "exported symbols may be reached + * from outside this repository" — and that filter silently does nothing for a + * language whose exports are not recorded, either because the extractor does + * not record them (Rust `pub`) or because the language has no such concept at + * all (Python, C, Ruby: the header or the module IS the surface). Rather than + * carry a table of which is which, ask the index: if nothing in this language + * is marked exported, the filter did not run, and no claim about outside + * reachability can be made for it. + * + * `idx_nodes_language` covers the grouping; the caller passes the handful of + * languages its candidates are actually in. + */ + getLanguagesWithExports(languages: Iterable): Set { + const unique = [...new Set(languages)].filter((language) => language.length > 0); + const found = new Set(); + if (unique.length === 0) return found; + + 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 language, MAX(is_exported) AS any_exported + FROM nodes + WHERE language IN (${placeholders}) + GROUP BY language` + ) + .all(...chunk) as Array<{ language: string; any_exported: number }>; + for (const row of rows) if (row.any_exported === 1) found.add(row.language); + } + return found; + } + /** * The nodes with the most DISTINCT dependents, most first. * diff --git a/src/graph/dead-code.ts b/src/graph/dead-code.ts new file mode 100644 index 0000000..1d724c5 --- /dev/null +++ b/src/graph/dead-code.ts @@ -0,0 +1,886 @@ +/** + * Dead code and islands — one derivation of "nothing in this repository + * reaches here". + * + * The graph can answer that question exactly, and that is the problem: the + * exact answer is *no incoming edge*, and a symbol with no incoming edge is not + * the same thing as a symbol nobody uses. Reflection calls it. A framework + * registers it by name. A test file that was never indexed imports it. The + * resolver saw the name and could not follow it. So the honest product of this + * module is two things at once — a list, and everything the list could not see. + * + * ## The shape of the claim + * + * `unreferenced` is a fact: no edge in the index, other than the `contains` + * edge from whatever holds it, points at this symbol. `dead` is an inference on + * top of that fact, and every step of the inference is subtractive — a + * candidate is dropped from the list the moment there is any reason to believe + * something outside the graph reaches it: + * + * - it is **exported** (something outside this repository may import it); + * - it lives in a **test** or a **generated** file (not code anyone deletes by + * hand); + * - it is **abstract** or declared on an interface (a declaration is dispatched + * to, never called); + * - 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; + * - 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 has a name the language calls by itself (`constructor`, `__enter__`, + * `main`); + * - it sits in a **vendored** directory (`vendor/`, `third_party/`, + * `node_modules/` — code the repository carries but does not own); + * - it is in a **test scope** the path does not reveal — a Rust + * `#[cfg(test)] mod tests`, a nested `Tests` namespace; + * - it is in a **component file** whose markup the index reads for calls but + * not for references, so a handler passed as `{onkeydown}` is invisible; + * - it is in a file **nothing in the index reaches**. Then "nothing references + * this symbol" is a restatement of "we cannot see how this file is wired", + * not a finding about the symbol — and it is the map, not this list, that + * says so: a file no one reaches is an island, and islands are drawn there; + * - the index holds an **unresolved reference** to its name. A `failed` row in + * `unresolved_refs` is the resolver's own record of a reference it could not + * follow, and a symbol whose name we failed to follow cannot be called + * unreferenced. + * - **another symbol of the same name IS referenced.** This is the one that + * matters most and the one nothing else would catch. `CodeGraph.getTopRouteFile` + * calls `this.queries.getTopRouteFile()`; the resolver prefers a same-name + * definition in the call site's own file, so the edge lands on the caller + * itself and the real target is left with nothing. From the edge table, + * "nobody calls this" and "the resolver picked the twin" are the same + * picture — so the claim is not made about either; + * - it is declared in a **header** (`.h`, `.hpp`, `.d.ts`, `.pyi`): a header IS + * the export surface, and the reference to it is an `#include` the resolver + * does not follow to the declaration; + * - it is in a language this index records **no export marker** for. The + * exported filter is the strongest one here, and for Rust (`pub` is not + * recorded) or Python and C (no such concept at all) it silently does + * nothing — so the index is asked, per language, whether it ran; + * - **its own file writes the name more than once.** The last rule, and the + * only one that is not a graph query. Everything above assumes the edge + * table is complete; it is not, and the gaps do not announce themselves — + * `this.handleMessage.bind(this)` is a value reference the extractor does not + * record, and a call inside an object-literal initialiser is another. Both + * leave the name written twice in one file and no edge at all. So before the + * claim is made, the identifier is counted in the file itself AND in every + * file the index says depends on it — written once, in its own declaration, + * nothing that can reach it writes it down; written twice, we simply did not + * see the second one. + * + * Every subtraction is counted. {@link DeadCodeReport.excluded} is not + * diagnostics — it is the sentence under the list ("47 exported, 12 overriding + * an ancestor…"), because a list of eight rows drawn from four thousand + * candidates means something different from a list of eight drawn from nine. + * + * ## Islands + * + * The other half of the task, a *module* nothing depends on, is not computed + * here: it falls straight out of the map's own link set (a module with no + * incoming link), and the map's layout is already a pure function in the + * viewer. Computing it a second time on this side would be a second answer to + * a question the map has already answered. See `ui/src/lib/map-model.ts`. + * + * Everything here is query-time and read-only. + */ + +import fs from 'fs'; +import path from 'path'; +import type CodeGraph from '../index'; +import type { Node, NodeKind } from '../types'; +import { isTestFile } from '../search/query-utils'; + +// ============================================================================= +// Caps and defaults +// ============================================================================= + +/** + * Kinds asked about by default. + * + * Callables and types, and nothing else. `variable`/`constant`/`field` are out + * deliberately: a value's uses are recorded as `references` edges, and that + * coverage is the most language-dependent thing in the resolver — a default + * that included them would produce a list whose truthfulness varied by which + * language the reader happened to be looking at. + */ +export const DEAD_CODE_KINDS: readonly NodeKind[] = [ + 'function', + 'method', + 'class', + 'component', + 'interface', + 'struct', + 'trait', + 'protocol', + 'enum', + 'union', + 'type_alias', +]; + +/** Kinds a `kinds=` request may ask for. Anything else is a caller bug. */ +export const DEAD_CODE_ALLOWED_KINDS: ReadonlySet = new Set([ + ...DEAD_CODE_KINDS, + 'variable', + 'constant', + 'property', + 'field', + 'enum_member', + 'namespace', + 'module', +]); + +/** + * Candidates pulled out of SQL before any exclusion runs. + * + * High enough that no real repository reaches it with the default kinds (this + * index produces ~1 400), and bounded so that a half-indexed monorepo cannot + * turn one screen into a scan of a million rows. When it bites, + * {@link DeadCodeReport.bounded} says so. + */ +export const MAX_DEAD_CODE_CANDIDATES = 20000; + +/** Levels walked up looking for an ancestor that declares the same member. */ +export const MAX_OVERRIDE_ANCESTOR_DEPTH = 8; + +/** + * Files read for the corroboration pass, and the biggest one read. + * + * The pass runs over the survivors only — everything cheap has already fired — + * so on this index it reads a few dozen files. The caps are a backstop against + * a repository whose survivors span a thousand files or include a generated + * megabyte. A file skipped for either reason counts as NOT corroborated, which + * drops the row: the safe direction is always the one that says less. + */ +export const MAX_CORROBORATION_FILES = 600; +export const MAX_CORROBORATION_BYTES = 2_000_000; + +/** Kinds that can carry members, i.e. whose ancestors are worth walking. */ +const CONTAINER_KINDS: ReadonlySet = new Set([ + 'class', + 'interface', + 'struct', + 'trait', + 'protocol', + 'enum', + 'union', + 'type_alias', +]); + +/** Container kinds whose members are declarations, never call targets. */ +const DECLARATION_CONTAINER_KINDS: ReadonlySet = new Set([ + 'interface', + 'trait', + 'protocol', +]); + +/** Member kinds an override can be declared on. */ +const OVERRIDABLE_KINDS: ReadonlySet = new Set([ + 'method', + 'function', + 'property', + 'field', +]); + +/** + * Names a language or a runtime calls without anything in the source naming + * them. + * + * Kept short on purpose. The temptation is a per-language table of every + * lifecycle hook ever written, which would be wrong twice over — it would go + * 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. + */ +const IMPLICIT_ENTRY_NAMES: ReadonlySet = new Set([ + 'constructor', + 'main', + 'init', + 'deinit', + 'finalize', + 'destructor', + 'dispose', + 'drop', + 'default', + 'tostring', + 'equals', + 'gethashcode', + 'hashcode', +]); + +/** + * Qualified-name segments that mean "inside a test scope the file path does not + * reveal" — a Rust `#[cfg(test)] mod tests`, a nested `Tests` class in C#, a + * Go `TestMain` helper block. `isTestFile` only reads paths, and an in-file test + * module is invisible to it. + */ +const TEST_SCOPE_SEGMENTS: ReadonlySet = new Set([ + 'test', + 'tests', + '__tests__', + 'spec', + 'specs', + 'testing', +]); + +/** + * Languages whose files are markup with a script block inside them. + * + * The extractors for these read the ` + +
+
+

Dead code

+

+ Symbols no import, call or reference in this index reaches, largest first. Everything below + is what the graph can see; the notes under the list are what it cannot. +

+
+ +
+

{DEAD_CODE_CAVEAT}

+ { + event.preventDefault(); + navigate(deadHref({ exported: !exported })); + }}>{exported ? 'Including exported' : 'Internal only'} +
+ + {#if failure} +

Could not read the list — {failure}

+ {:else if loading && payload === null} +

Reading the graph…

+ {:else if payload} + {#if exported} +

+ Exported symbols are on this list. Nothing in this repository references them, but anything + outside it can — a published package, another service, a script. Read each one before you + believe it. +

+ {/if} + + {#if payload.groups.length === 0} +

{emptyMessage(payload)}

+ {:else} +

{headline}

+
+ {#each payload.groups as group (group.file)} +
+
+ {group.file} + {groupMeta(group)} +
+ {#each group.rows as row (row.id)} +
+ +
+
+ + {row.file}:{row.line} + {#if row.exported}exported{/if} +
+
{deadCodeRowMeta(row)}
+ {#if row.members.items.length > 0} +
+ {#each row.members.items as member (member.id)} + {member.name} + {/each} + {#if row.members.truncated} + +{row.members.total - row.members.shown} more + {/if} +
+ {/if} +
+
+ {/each} +
+ {/each} +
+ {/if} + +
+

{scale}

+ {#if phrases.length > 0} +
    + {#each phrases as phrase (phrase)} +
  • {phrase}
    • + {/each} +
+ {/if} + {#if !payload.corroborated} +

+ The rows were not checked against the text of the files that can reach them, so a + reference the extractor did not record would not have been caught. +

+ {/if} + {#if payload.bounded} +

+ The scan stopped at its cap — this index holds more unreferenced symbols than were + considered. +

+ {/if} + {#if payload.rows.truncated} +

+ Showing {payload.rows.shown} of {payload.rows.total} — the rest are in the index, not on + this list. +

+ {/if} +
+ {/if} +
+ + diff --git a/ui/src/views/FileView.svelte b/ui/src/views/FileView.svelte index 1344e3e..7fb365d 100644 --- a/ui/src/views/FileView.svelte +++ b/ui/src/views/FileView.svelte @@ -276,7 +276,8 @@
-

{basename(payload.file.path)}

+ +

{basename(payload.file.path)}

{fileMetaLine(payload)} {payload.file.path}
@@ -381,6 +382,10 @@ letter-spacing: -0.01em; } + .card-h h1.gen { + color: var(--ink-4); + } + .spacer { flex: 1 1 auto; }