diff --git a/__tests__/dead-code.test.ts b/__tests__/dead-code.test.ts index 7b446caa8f..b87ad8a466 100644 --- a/__tests__/dead-code.test.ts +++ b/__tests__/dead-code.test.ts @@ -763,3 +763,106 @@ public class App { expect(listed(report, 'DemoComponent::ngOnInit')).toBe(false); }); }); + +describe('a declaration that merges into a type outside the index', () => { + let root: string; + let graph: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-deadcode-ambient-')); + // ghostfolio's chart.registry.ts: chart.js reads both interfaces through + // the types they extend, and nothing in the repository names either. + write( + root, + 'src/chart.registry.ts', + `import { Chart, Tooltip, type ChartType } from 'chart.js'; + +interface VerticalHoverLinePluginOptions { + color?: string; +} + +declare module 'chart.js' { + interface PluginOptionsByType { + verticalHoverLine: TType extends 'line' ? VerticalHoverLinePluginOptions : never; + } + interface TooltipPositionerMap { + top: (items: unknown[]) => { x: number; y: number }; + } +} + +export function registerChartConfiguration(): void { + Chart.register(Tooltip); +} + +function unusedChartHelper(): void {} +` + ); + // angular-realworld's app.config.ts, with a Node.js augmentation beside it. + write( + root, + 'src/app.config.ts', + `declare global { + interface Window { + __conduit_debug__?: { token(): string | null }; + } + namespace NodeJS { + interface ProcessEnv { + API_URL?: string; + } + } +} + +// In a module file this namespace is the file's own: it merges with nothing. +declare namespace Settings { + interface NotMergedAnywhere { + retries: number; + } +} + +export const appConfig = { providers: [] }; +` + ); + write( + root, + 'src/main.ts', + `import { registerChartConfiguration } from './chart.registry'; +import { appConfig } from './app.config'; + +registerChartConfiguration(); +export const config = appConfig; +` + ); + graph = CodeGraph.initSync(root, { config: { include: ['src/**/*.ts'], exclude: [] } }); + await graph.indexAll(); + }, 60_000); + + afterAll(() => { + graph?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + const merged = ['PluginOptionsByType', 'TooltipPositionerMap', 'Window', 'ProcessEnv']; + + it('does not list what a declare module or declare global block declares', () => { + for (const readSource of [undefined, null] as const) { + const report = buildDeadCodeReport(graph, readSource === null ? { readSource } : {}); + for (const name of merged) expect(names(report)).not.toContain(name); + // TypeScript exports them without the keyword, so the exported rule + // takes them, and counts them. + expect(report.excluded.exported).toBe(merged.length); + } + }); + + it('lists them with the outside-reach caveat when exported symbols are asked for', () => { + const report = buildDeadCodeReport(graph, { includeExported: true }); + for (const name of merged) { + expect(report.entries.find((entry) => entry.node.name === name)?.exported).toBe(true); + } + }); + + it('still lists what nothing reaches beside them', () => { + const report = buildDeadCodeReport(graph); + expect(names(report)).toContain('unusedChartHelper'); + expect(names(report)).toContain('NotMergedAnywhere'); + }); +}); diff --git a/__tests__/ts-ambient-exports.test.ts b/__tests__/ts-ambient-exports.test.ts new file mode 100644 index 0000000000..a77a2f4d8e --- /dev/null +++ b/__tests__/ts-ambient-exports.test.ts @@ -0,0 +1,208 @@ +/** + * A declaration inside `declare module 'x' { … }` or `declare global { … }` is + * exported, though nothing in it writes `export`: + * + * declare module 'chart.js' { // ghostfolio's chart.registry.ts + * interface PluginOptionsByType { … } + * } + * declare global { // angular-realworld's app.config.ts + * interface Window { __conduit_debug__?: ConduitDebug; } + * } + * + * Both indexed as unexported file-level interfaces, so the dead-code report + * listed them: nothing in the repository names them, because chart.js and the + * browser read them through the type they merge into. TypeScript's binder + * makes an ambient module body an export context, and a namespace nested in + * one is ambient too, unless the body holds an export declaration of its own + * (`export {}`, `export =`, `export default x`). Both extractors now say what + * the compiler says. `declare namespace X` is unchanged: in a module file it + * is that file's own namespace and merges with nothing outside. Runs against + * the native kernel (when built) and the wasm extractor, which must agree. + */ +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult, Language } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +/** A module file: augmentations, a global block, and the namespaces that are neither. */ +const MODULE_SOURCE = `import type { ChartType } from 'chart.js'; + +declare module 'chart.js' { + interface PluginOptionsByType { + verticalHoverLine: TType; + } + export interface WrittenOut {} + namespace Plugins { + interface Nested {} + } + type Alias = string; + enum Position { Top } + class Positioner {} + const defaults: Alias; +} + +declare global { + interface Window { + __conduit_debug__?: unknown; + } + namespace NodeJS { + interface ProcessEnv { + API_URL?: string; + } + } + var bitwardenContainerService: unknown; +} + +declare namespace Settings { + interface OwnNamespace {} +} + +namespace Plain { + interface Hidden {} + export interface Shown {} +} + +interface FileLocal {} + +export const appConfig = {}; +`; + +/** + * A script file declaring a module whole, the way an untyped library's typings + * do. \`export =\` is an export declaration, so the body exports only that. + */ +const SCRIPT_SOURCE = `declare module 'legacy-lib' { + interface Options {} + const main: (options: Options) => void; + export = main; +} + +declare module 'listed-lib' { + interface Unlisted {} + export interface Listed {} + export { Unlisted as Renamed }; +} + +declare module 'open-lib' { + interface Open {} +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +/** Kinds a statement declares — the nodes that carry an export flag of their own. */ +const DECLARATION_KINDS = new Set(['interface', 'class', 'enum', 'type_alias', 'function', 'variable', 'constant', 'component']); + +/** `name` → isExported, for every declaration. */ +function exportFlags(result: ExtractionResult): Record { + const flags: Record = {}; + for (const node of result.nodes) { + if (DECLARATION_KINDS.has(node.kind)) flags[node.name] = node.isExported === true; + } + return flags; +} + +describe('TypeScript ambient module bodies export what they declare', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['typescript', 'tsx']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', file: string, source: string, language: Language): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource(file, source, language); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract(file, source, language); + expect(result, `kernel extraction of ${file}`).not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + + for (const crlf of [false, true]) { + const eol = (s: string) => (crlf ? s.replace(/\n/g, '\r\n') : s); + const label = crlf ? ' (CRLF)' : ''; + + it.each(backends)(`declare module and declare global export their declarations: %s${label}`, (backend) => { + const result = extract(backend, 'src/chart.registry.ts', eol(MODULE_SOURCE), 'typescript'); + expect(exportFlags(result)).toEqual({ + // declare module 'chart.js', and a namespace nested in it. + PluginOptionsByType: true, + WrittenOut: true, + Nested: true, + Alias: true, + Position: true, + Positioner: true, + defaults: true, + // declare global, and a namespace nested in it. + Window: true, + ProcessEnv: true, + bitwardenContainerService: true, + // Neither is an outside scope: a module file's own namespaces. + OwnNamespace: false, + Hidden: false, + Shown: true, + FileLocal: false, + appConfig: true, + }); + }); + + it.each(backends)(`a body with an export declaration exports only what it names: %s${label}`, (backend) => { + const result = extract(backend, 'types/legacy.ts', eol(SCRIPT_SOURCE), 'typescript'); + expect(exportFlags(result)).toEqual({ + Options: false, + main: false, + Unlisted: false, + Listed: true, + Open: true, + }); + }); + + it.each(backends)(`tsx reads the same blocks: %s${label}`, (backend) => { + const source = eol(`declare module '@tanstack/react-router' {\n interface Register {\n router: unknown;\n }\n}\nexport const App = () =>
;\n`); + const result = extract(backend, 'src/main.tsx', source, 'tsx'); + expect(exportFlags(result).Register).toBe(true); + }); + } + + it.runIf(kernelAvailable)('the kernel and the wasm extractor agree on every node', () => { + for (const [file, source] of [['src/chart.registry.ts', MODULE_SOURCE], ['types/legacy.ts', SCRIPT_SOURCE]] as const) { + const canon = (r: ExtractionResult) => + r.nodes.map(({ updatedAt: _u, ...n }) => JSON.stringify(n, Object.keys(n).sort())).sort(); + expect(canon(extract('kernel', file, source, 'typescript'))).toEqual( + canon(extract('wasm', file, source, 'typescript')) + ); + } + }); +}); diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index 2658226bab..dc5d333524 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -1030,7 +1030,8 @@ impl<'t> Walker<'t> { None } - /// isExported: walk the parent chain for an export_statement. + /// isExported: walk the parent chain for an export_statement, then ask + /// whether a `declare module '…'` / `declare global` body exports it. fn is_exported(&self, node: Node) -> bool { let mut cur = node.parent(); while let Some(p) = cur { @@ -1039,7 +1040,7 @@ impl<'t> Walker<'t> { } cur = p.parent(); } - false + is_ambient_export(node) } fn has_keyword_child(&self, node: Node, kw: &str) -> bool { @@ -1136,6 +1137,48 @@ fn body_of(node: Node) -> Option { resolve_field_body(node).or_else(|| node.child_by_field_name("body")) } +/// isAmbientExport (languages/typescript.ts): declared in a `declare module +/// 'x' { … }` or `declare global { … }` body, which TypeScript exports without +/// an `export` keyword unless the body holds an export declaration of its own. +fn is_ambient_export(node: Node) -> bool { + let mut cur = node.parent(); + while let Some(p) = cur { + if p.kind() == "statement_block" && has_export_declaration(p) { + return false; + } + if p.kind() == "ambient_declaration" { + // `declare global` is the keyword and a bare block, with no field names. + for i in 0..p.child_count() { + if p.child(i).is_some_and(|c| c.kind() == "global") { + return true; + } + } + if let Some(declared) = p.named_child(0) { + if declared.kind() == "module" + && declared.child_by_field_name("name").is_some_and(|n| n.kind() == "string") + { + return true; + } + } + } + cur = p.parent(); + } + false +} + +/// hasExportDeclaration: a statement that exports by name or by assignment +/// (`export {…}`, `export =`, `export default x`), not by declaring. +fn has_export_declaration(block: Node) -> bool { + for i in 0..block.named_child_count() { + if let Some(statement) = block.named_child(i) { + if statement.kind() == "export_statement" && statement.child_by_field_name("declaration").is_none() { + return true; + } + } + } + false +} + fn opt_str(arena: &mut Arena, s: Option<&str>) -> StrRef { match s { Some(s) => arena.put(s), diff --git a/docs/design/codegraph-ui-design-spec.md b/docs/design/codegraph-ui-design-spec.md index ac8ec2d9b2..1c41a0d85c 100644 --- a/docs/design/codegraph-ui-design-spec.md +++ b/docs/design/codegraph-ui-design-spec.md @@ -355,7 +355,7 @@ 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", +every reason a candidate was left off is printed with its count ("1 677 in test files", "378 exported, declared in a header, or merged into a library or global type", "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. diff --git a/docs/viewer-launch-changelog.md b/docs/viewer-launch-changelog.md index 4afa667d68..6e9049b852 100644 --- a/docs/viewer-launch-changelog.md +++ b/docs/viewer-launch-changelog.md @@ -179,6 +179,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel - **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. +- **Dead code no longer lists the types you add to a library's or the browser's.** An interface written inside `declare module 'chart.js' { … }` or `declare global { … }`, like chart.js's `PluginOptionsByType`, TanStack Router's `Register`, the browser's `Window`, or a custom element's entry in `HTMLElementTagNameMap`, merges into a type declared outside your project. The library or the browser reads it from there, so nothing in your code names it, and every one was listed as unused. TypeScript exports what these blocks declare without an `export` keyword, and CodeGraph now records them the same way, so they are left off and counted with the exported symbols. A `declare namespace` in an ordinary module file still belongs to that file and is checked as before. Re-index TypeScript projects after upgrading. + - **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. diff --git a/src/extraction/languages/typescript.ts b/src/extraction/languages/typescript.ts index a56e020574..15ef823981 100644 --- a/src/extraction/languages/typescript.ts +++ b/src/extraction/languages/typescript.ts @@ -57,6 +57,43 @@ export function blankFlowSyntax(source: string, filePath?: string): string { .replace(/\.\.\.(?=\s*[},])/g, ' '); } +/** + * Whether `node` is declared in a `declare module 'x' { … }` or `declare + * global { … }` body, which exports it without an `export` keyword. + * + * This is TypeScript's own rule: the binder makes an ambient module body an + * export context unless the body holds an export declaration (`export {…}`, + * `export * from`, `export =`, `export default x`), and a namespace nested in + * one is ambient too. What such a body declares belongs to that module or to + * the global scope, not to this file (`interface Window`, chart.js's + * `PluginOptionsByType`): other files reach it through them, and so does the + * library whose type it extends. `declare namespace X` is not one: in a module + * file, X belongs to that file. Mirrored in the kernel (tsjs/mod.rs + * `is_ambient_export`). + */ +export function isAmbientExport(node: SyntaxNode): boolean { + for (let current = node.parent; current; current = current.parent) { + if (current.type === 'statement_block' && hasExportDeclaration(current)) return false; + if (current.type !== 'ambient_declaration') continue; + // `declare global` is the keyword and a bare block, with no field names. + for (let i = 0; i < current.childCount; i++) { + if (current.child(i)?.type === 'global') return true; + } + const declared = current.namedChild(0); + if (declared?.type === 'module' && getChildByField(declared, 'name')?.type === 'string') return true; + } + return false; +} + +/** A statement in `block` that exports by name or by assignment, not by declaring. */ +function hasExportDeclaration(block: SyntaxNode): boolean { + for (let i = 0; i < block.namedChildCount; i++) { + const statement = block.namedChild(i); + if (statement?.type === 'export_statement' && !getChildByField(statement, 'declaration')) return true; + } + return false; +} + export const typescriptExtractor: LanguageExtractor = { preParse: blankFlowSyntax, functionTypes: ['function_declaration', 'generator_function_declaration', 'arrow_function', 'function_expression', 'generator_function'], @@ -143,7 +180,7 @@ export const typescriptExtractor: LanguageExtractor = { if (current.type === 'export_statement') return true; current = current.parent; } - return false; + return isAmbientExport(node); }, isAsync: (node) => { for (let i = 0; i < node.childCount; i++) { diff --git a/src/graph/dead-code.ts b/src/graph/dead-code.ts index c3a2fd1b4a..d712a00009 100644 --- a/src/graph/dead-code.ts +++ b/src/graph/dead-code.ts @@ -17,7 +17,12 @@ * 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 is **exported** (something outside this repository may import it). That + * includes whatever a TypeScript `declare module 'x'` or `declare global` + * block declares, which TypeScript exports without the keyword: `interface + * Window` or chart.js's `PluginOptionsByType` merges into a type declared + * outside the index, and the runtime or the library reads it through that + * type, so nothing in the repository names 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 @@ -353,7 +358,11 @@ export interface DeadCodeExclusions { tests: number; /** In a tool-generated file. */ generated: number; - /** Exported, or declared in a header — reachable from outside this index. */ + /** + * Exported, or declared in a header — reachable from outside this index. A + * TypeScript `declare module` / `declare global` declaration is exported + * implicitly: it merges into a library's or the global scope's type. + */ exported: number; /** * In a language this index records no export marker for, so nothing here can diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 69bebef53b..7d148d80da 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1373,8 +1373,9 @@ const HAS_IMPORT_STATEMENT = /^[ \t]*import[\s{*'"]/m; * CommonJS shapes cover files that never use ESM syntax at all, in both the dot * and the bracket form; and `declare global` contributes names to every file * whether or not the module exports anything of its own. Kept as a source test - * rather than a node scan precisely because `isExported` is set only from an - * `export_statement` ancestor, so `const x = …; export { x }` and + * rather than a node scan precisely because `isExported` is set only where a + * declaration is written (an `export_statement` around it, or a `declare + * module` / `declare global` body), so `const x = …; export { x }` and * `module.exports = { x }` both read as unexported on the node. */ const HAS_ESM_EXPORT = /^[ \t]*export[\s{*]|^[ \t]*declare\s+global\b/m; diff --git a/src/ui-server/api/deadcode.ts b/src/ui-server/api/deadcode.ts index 67b687f066..fb31e2ea82 100644 --- a/src/ui-server/api/deadcode.ts +++ b/src/ui-server/api/deadcode.ts @@ -95,7 +95,7 @@ export interface WireDeadCode { const EXCLUSION_LABELS: Record = { tests: 'in test files', generated: 'in generated files', - exported: 'exported, or declared in a header', + exported: 'exported, declared in a header, or merged into a library or global type', exportsUnknown: 'in languages this index records no exports for', declarations: 'abstract, or declared on an interface', decorated: 'carrying a decorator, so a framework registers them',