diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index edac9ceed..6c2d1f973 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -3036,14 +3036,35 @@ fn find_parent_class_no_fn_boundary(node: &Node, source: &[u8]) -> Option Vec { - // Walk up: call_expression → await_expression? → variable_declarator + // Walk up through any combination/nesting of await/parenthesized/as-cast + // wrappers to reach the variable_declarator. let mut current = call_node.parent(); - if let Some(parent) = current { - if parent.kind() == "await_expression" { + while let Some(parent) = current { + if DYNAMIC_IMPORT_WRAPPER_KINDS.contains(&parent.kind()) { current = parent.parent(); + } else { + break; } } let declarator = match current { @@ -4205,6 +4226,58 @@ mod tests { assert!(!dyn_imports[0].names.contains(&"nested".to_string())); } + // Regression tests for #1781: `codegraph exports` failed to credit consumers + // reached via `const { X } = (await import('./mod.js')) as {...}` — the + // walk-up from the import() call to its enclosing variable_declarator only + // skipped a single optional await_expression, so the extra + // parenthesized_expression / as_expression layers introduced by wrapping + // parens and a TS type-assertion caused name extraction to bail out with + // an empty list, exactly as if the destructured names couldn't be + // determined at all. + + #[test] + fn finds_dynamic_import_with_parenthesized_destructuring() { + let s = parse_ts("const { a, b } = (await import('./foo.js'));"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert!(dyn_imports[0].names.contains(&"a".to_string())); + assert!(dyn_imports[0].names.contains(&"b".to_string())); + } + + #[test] + fn finds_dynamic_import_with_as_cast_destructuring() { + let s = parse_ts("const { a, b } = await import('./foo.js') as { a: Fn; b: Fn };"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert!(dyn_imports[0].names.contains(&"a".to_string())); + assert!(dyn_imports[0].names.contains(&"b".to_string())); + } + + #[test] + fn finds_dynamic_import_with_satisfies_cast_destructuring() { + // TS 4.9+ `satisfies` is structurally identical to `as` here (Greptile + // follow-up to #1781) — same walk-up gap would otherwise reproduce. + let s = parse_ts("const { a, b } = await import('./foo.js') satisfies { a: Fn; b: Fn };"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert!(dyn_imports[0].names.contains(&"a".to_string())); + assert!(dyn_imports[0].names.contains(&"b".to_string())); + } + + #[test] + fn finds_dynamic_import_with_parenthesized_as_cast_destructuring() { + // Exact repro shape from #1781 (native-orchestrator.ts): + // `const { X, Y } = (await import('../mod.js')) as { X: Fn; Y: Fn };` + let s = parse_ts( + "const { buildDataflowVerticesFromMap, buildDataflowEdges } = (await import('../../../../features/dataflow.js')) as { buildDataflowVerticesFromMap: Fn; buildDataflowEdges: Fn };", + ); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert_eq!(dyn_imports[0].source, "../../../../features/dataflow.js"); + assert!(dyn_imports[0].names.contains(&"buildDataflowVerticesFromMap".to_string())); + assert!(dyn_imports[0].names.contains(&"buildDataflowEdges".to_string())); + } + #[test] fn extracts_callback_reference_in_router_use() { let s = parse_js("router.use(handleToken);"); diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 61a4867b2..9f5259aee 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -4051,22 +4051,44 @@ function extractImportNames( return names; } +/** + * Wrapper node types that can sit between a dynamic `import()` call and its + * enclosing `variable_declarator` without changing which value gets bound — + * `await`, redundant parentheses, and TypeScript `as`/`satisfies` casts. + * Real-world dynamic-import call sites often combine several of these, e.g. + * `const { X } = (await import('./mod.js')) as { X: Fn }` nests + * await_expression → parenthesized_expression → as_expression before + * reaching the declarator (#1781). `satisfies_expression` (TS 4.9+ + * `... satisfies { X: Fn }`) is structurally identical to `as_expression` + * here — same Greptile follow-up as the native mirror. + */ +const DYNAMIC_IMPORT_WRAPPER_TYPES = new Set([ + 'await_expression', + 'parenthesized_expression', + 'as_expression', + 'satisfies_expression', +]); + /** * Extract destructured names from a dynamic import() call expression. * * Handles: - * const { a, b } = await import('./foo.js') → ['a', 'b'] - * const mod = await import('./foo.js') → ['mod'] - * import('./foo.js') → [] (no names extractable) + * const { a, b } = await import('./foo.js') → ['a', 'b'] + * const mod = await import('./foo.js') → ['mod'] + * const { a } = (await import('./foo.js')) as { a: Fn } → ['a'] + * import('./foo.js') → [] (no names extractable) * - * Walks up the AST from the call_expression to find the enclosing + * Walks up the AST from the call_expression — through any nesting of + * await/parenthesized/as-cast wrappers — to find the enclosing * variable_declarator and reads the name/object_pattern. */ function extractDynamicImportNames(callNode: TreeSitterNode): string[] { - // Walk up: call_expression → await_expression → variable_declarator + // Walk up through await_expression / parenthesized_expression / as_expression + // wrappers, in any combination or order, to reach the variable_declarator. let current = callNode.parent; - // Skip await_expression wrapper if present - if (current && current.type === 'await_expression') current = current.parent; + while (current && DYNAMIC_IMPORT_WRAPPER_TYPES.has(current.type)) { + current = current.parent; + } // We should now be at a variable_declarator (or not, if standalone import()) if (current?.type !== 'variable_declarator') return []; diff --git a/tests/integration/issue-1781-dynamic-import-destructure-consumer.test.ts b/tests/integration/issue-1781-dynamic-import-destructure-consumer.test.ts new file mode 100644 index 000000000..ce63c08d0 --- /dev/null +++ b/tests/integration/issue-1781-dynamic-import-destructure-consumer.test.ts @@ -0,0 +1,257 @@ +/** + * Integration test for #1781: `codegraph exports` did not credit consumers + * reached via a dynamic `import()` expression whose destructured result is + * wrapped in redundant parentheses and/or a TypeScript `as {...}` type + * assertion — the exact shape used throughout + * `src/domain/graph/builder/stages/native-orchestrator.ts`: + * + * const { buildDataflowVerticesFromMap, ... } = + * (await import('../../../../features/dataflow.js')) as {...}; + * + * Root cause: `extractDynamicImportNames` (TS) / `extract_dynamic_import_names` + * (Rust) walked up from the `import()` call through at most one optional + * `await_expression` before requiring the immediate parent to be a + * `variable_declarator`. The extra `parenthesized_expression` and/or + * `as_expression` wrapper layers introduced by parens and a type assertion + * broke that walk-up, so no destructured names were ever extracted — the + * import was recorded with `names: []`, `importedNames` never gained an + * entry for the destructured bindings, and the later call through the + * destructured local name never resolved to a `calls` edge. Both `codegraph + * exports` (consumer count) and `codegraph roles --role dead` therefore + * treated a genuinely-consumed export as unreferenced. + * + * Fix: the walk-up now skips any nesting/combination of `await_expression`, + * `parenthesized_expression`, and `as_expression` wrappers before checking + * for the enclosing `variable_declarator`, in both engines. + * + * Two consumer shapes are exercised against the same target module: + * - `usesPlain`: bare `const { X } = await import(...)` (no cast) — the + * pre-existing baseline that must keep working. + * - `usesCast`: `const { Y } = (await import(...)) as {...}` — the exact + * regression shape from native-orchestrator.ts. + * + * The target module and its consumer are deliberately placed in *different* + * directories (`features/` vs `domain/stages/`, mirroring the real repo + * layout) rather than side by side. A same-directory fixture does not + * discriminate this bug at all: the resolver's directory-scoped fallback + * tier (a same-directory, name-only match — one of several confidence tiers + * below the import-aware match) coincidentally "rescues" the call even when + * `extractDynamicImportNames` returns no names, producing a `calls` edge + * regardless of whether the fix is present. Cross-directory placement is + * required so the only way to resolve `usesCast`'s call is through the + * import-aware path this bug actually breaks. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { exportsData } from '../../src/domain/queries.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const DEAD_ROLES = new Set(['dead-unresolved', 'dead-leaf', 'dead-entry', 'dead-ffi']); + +const TARGET_FILE = 'features/dataflow.ts'; + +const FIXTURE = { + [TARGET_FILE]: ` +export function buildDataflowVerticesFromMap(): number { + return 42; +} + +export function buildDataflowP4ForNative(): number { + return 7; +} +`, + 'domain/stages/consumer.ts': ` +export async function usesPlain() { + const { buildDataflowVerticesFromMap } = await import('../../features/dataflow.js'); + return buildDataflowVerticesFromMap(); +} + +export async function usesCast() { + const { buildDataflowP4ForNative } = (await import('../../features/dataflow.js')) as { + buildDataflowP4ForNative: () => number; + }; + return buildDataflowP4ForNative(); +} +`, +}; + +function readNodesWithRoles(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db.prepare('SELECT name, kind, role FROM nodes ORDER BY name').all() as Array<{ + name: string; + kind: string; + role: string | null; + }>; + } finally { + db.close(); + } +} + +function getCallEdges(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n2.name AS tgt, n2.file AS tgt_file + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name`, + ) + .all() as Array<{ src: string; tgt: string; tgt_file: string }>; + } finally { + db.close(); + } +} + +function writeFixture(rootDir: string) { + for (const [rel, content] of Object.entries(FIXTURE)) { + const abs = path.join(rootDir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } +} + +describe('dynamic import() + destructure consumer crediting (#1781) — WASM', () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1781-wasm-')); + writeFixture(tmpDir); + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('creates a calls edge for the bare (uncast) destructured binding', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + const edge = edges.find( + (e) => e.src === 'usesPlain' && e.tgt === 'buildDataflowVerticesFromMap', + ); + expect( + edge, + `Expected usesPlain -> buildDataflowVerticesFromMap; got: ${JSON.stringify(edges)}`, + ).toBeDefined(); + expect(edge?.tgt_file).toBe(TARGET_FILE); + }); + + it('creates a calls edge for the parenthesized + `as`-cast destructured binding', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + const edge = edges.find((e) => e.src === 'usesCast' && e.tgt === 'buildDataflowP4ForNative'); + expect( + edge, + `Expected usesCast -> buildDataflowP4ForNative; got: ${JSON.stringify(edges)}`, + ).toBeDefined(); + expect(edge?.tgt_file).toBe(TARGET_FILE); + }); + + it('codegraph exports credits both dynamically-imported functions with real consumers', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const data = exportsData(TARGET_FILE, dbPath); + + const vertices = data.results.find( + (r: { name: string }) => r.name === 'buildDataflowVerticesFromMap', + ); + expect(vertices).toBeDefined(); + expect(vertices.consumerCount).toBeGreaterThanOrEqual(1); + expect(vertices.consumers.map((c: { name: string }) => c.name)).toContain('usesPlain'); + + const p4 = data.results.find((r: { name: string }) => r.name === 'buildDataflowP4ForNative'); + expect(p4).toBeDefined(); + expect(p4.consumerCount).toBeGreaterThanOrEqual(1); + expect(p4.consumers.map((c: { name: string }) => c.name)).toContain('usesCast'); + }); + + it('does not classify either dynamically-imported export as dead', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const name of ['buildDataflowVerticesFromMap', 'buildDataflowP4ForNative']) { + const node = nodes.find((n) => n.name === name && n.kind === 'function'); + expect(node, `${name} node not found`).toBeDefined(); + expect( + DEAD_ROLES.has(node!.role ?? ''), + `${name} was classified as ${node!.role} — expected a non-dead role now that a real edge exists`, + ).toBe(false); + } + }); +}); + +// ── Native engine parity ──────────────────────────────────────────────────── +// Skipped when the native addon is not installed. + +describe.skipIf(!isNativeAvailable())( + 'dynamic import() + destructure consumer crediting (#1781) — native', + () => { + let nativeTmpDir: string; + + beforeAll(async () => { + nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1781-native-')); + writeFixture(nativeTmpDir); + await buildGraph(nativeTmpDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(nativeTmpDir, { recursive: true, force: true }); + }); + + it('creates a calls edge for the bare (uncast) destructured binding', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + const edge = edges.find( + (e) => e.src === 'usesPlain' && e.tgt === 'buildDataflowVerticesFromMap', + ); + expect( + edge, + `Expected native usesPlain -> buildDataflowVerticesFromMap; got: ${JSON.stringify(edges)}`, + ).toBeDefined(); + expect(edge?.tgt_file).toBe(TARGET_FILE); + }); + + it('creates a calls edge for the parenthesized + `as`-cast destructured binding', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + const edge = edges.find((e) => e.src === 'usesCast' && e.tgt === 'buildDataflowP4ForNative'); + expect( + edge, + `Expected native usesCast -> buildDataflowP4ForNative; got: ${JSON.stringify(edges)}`, + ).toBeDefined(); + expect(edge?.tgt_file).toBe(TARGET_FILE); + }); + + it('codegraph exports credits both dynamically-imported functions with real consumers', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const data = exportsData(TARGET_FILE, dbPath); + + const vertices = data.results.find( + (r: { name: string }) => r.name === 'buildDataflowVerticesFromMap', + ); + expect(vertices).toBeDefined(); + expect(vertices.consumerCount).toBeGreaterThanOrEqual(1); + + const p4 = data.results.find((r: { name: string }) => r.name === 'buildDataflowP4ForNative'); + expect(p4).toBeDefined(); + expect(p4.consumerCount).toBeGreaterThanOrEqual(1); + }); + + it('does not classify either dynamically-imported export as dead', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const name of ['buildDataflowVerticesFromMap', 'buildDataflowP4ForNative']) { + const node = nodes.find((n) => n.name === name && n.kind === 'function'); + expect(node, `${name} node not found (native)`).toBeDefined(); + expect(DEAD_ROLES.has(node!.role ?? '')).toBe(false); + } + }); + }, +); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index dd53cd001..b104c12e9 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -176,6 +176,77 @@ describe('JavaScript parser', () => { }); }); + describe('dynamic import() destructuring through parens/as-cast wrappers (#1781)', () => { + function parseTS(code) { + const parser = parsers.get('typescript'); + const tree = parser.parse(code); + return extractSymbols(tree, 'test.ts'); + } + + // Before the fix, extractDynamicImportNames walked up from the import() + // call through at most one optional await_expression before requiring the + // immediate parent to be a variable_declarator. Wrapping the awaited call + // in redundant parens and/or a TS `as {...}` cast — exactly the pattern + // used throughout native-orchestrator.ts — inserted extra + // parenthesized_expression/as_expression layers that broke the walk-up, + // so `names` came back empty and the destructured bindings never got + // credited as real consumers of the target module's exports (#1781). + + it('extracts destructured names from a bare dynamic import (no wrapper)', () => { + const symbols = parseJS(`const { a, b } = await import('./foo.js');`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'b']); + expect(symbols.imports[0].dynamicImport).toBe(true); + }); + + it('extracts destructured names when the awaited import is wrapped in redundant parens', () => { + const symbols = parseTS(`const { a, b } = (await import('./foo.js'));`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'b']); + }); + + it('extracts destructured names through a TypeScript `as {...}` type assertion (no parens)', () => { + const symbols = parseTS(`const { a, b } = await import('./foo.js') as { a: Fn; b: Fn };`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'b']); + }); + + it('extracts destructured names through a TypeScript `satisfies {...}` assertion', () => { + // TS 4.9+ `satisfies` is structurally identical to `as` here (Greptile + // follow-up to #1781) — same walk-up gap would otherwise reproduce. + const symbols = parseTS( + `const { a, b } = await import('./foo.js') satisfies { a: Fn; b: Fn };`, + ); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'b']); + }); + + it('extracts destructured names through parens + `as`-cast combined (exact repro shape)', () => { + // Matches native-orchestrator.ts's actual production pattern: + // const { X, Y } = (await import('./mod.js')) as { X: Fn; Y: Fn }; + const symbols = parseTS(` + const { buildDataflowVerticesFromMap, buildDataflowEdges } = + (await import('../../../../features/dataflow.js')) as { + buildDataflowVerticesFromMap: (db: unknown) => number; + buildDataflowEdges: (db: unknown) => Promise; + }; + `); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].source).toBe('../../../../features/dataflow.js'); + expect(symbols.imports[0].names).toEqual([ + 'buildDataflowVerticesFromMap', + 'buildDataflowEdges', + ]); + expect(symbols.imports[0].dynamicImport).toBe(true); + }); + + it('still extracts a single namespace-style binding through parens + as-cast', () => { + const symbols = parseTS(`const mod = (await import('./foo.js')) as { a: number };`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['mod']); + }); + }); + it('extracts call expressions', () => { const symbols = parseJS(`import { foo } from './bar'; foo(); baz();`); expect(symbols.calls).toContainEqual(expect.objectContaining({ name: 'foo' }));