Skip to content

Commit d250f8d

Browse files
committed
fix: credit destructured dynamic import() bindings as exports consumers
const { X, Y } = (await import('./mod.js')) as {...} — a dynamic import() whose awaited result is wrapped in redundant parentheses and/or a TypeScript `as` type assertion before being destructured — never resolved to a `calls` edge. extractDynamicImportNames (TS) and extract_dynamic_import_names (Rust) only skipped a single optional await_expression before requiring the immediate parent to be a variable_declarator, so the extra parenthesized_expression / as_expression layers broke the walk-up and silently returned an empty names list. Without names, importedNames never gained an entry for the destructured bindings, so calls through them never resolved, and `codegraph exports` reported the target functions as zero-consumer dead exports despite being called from production code (e.g. buildDataflowVerticesFromMap from native-orchestrator.ts). Generalize the walk-up in both engines to skip any combination or nesting of await_expression, parenthesized_expression, and as_expression wrappers before checking for the enclosing variable_declarator. Scoped to static string-literal import specifiers, the only case that is statically resolvable at all; computed/variable specifiers remain unresolvable by design. Verified on both engines against the exact repro (codegraph exports src/features/dataflow.ts -T --json now credits buildDataflowVerticesFromMap with its real consumer) and against the resolution-benchmark suite (no change: 206/206 tests, 63.8% aggregate recall before and after). No language/architecture/command surface changed (docs check acknowledged). Fixes #1781 Impact: 1 functions changed, 8 affected
1 parent aec5d2b commit d250f8d

4 files changed

Lines changed: 404 additions & 11 deletions

File tree

crates/codegraph-core/src/extractors/javascript.rs

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3031,14 +3031,29 @@ fn find_parent_class_no_fn_boundary(node: &Node, source: &[u8]) -> Option<String
30313031
None
30323032
}
30333033

3034+
/// Wrapper node kinds that can sit between a dynamic `import()` call and its
3035+
/// enclosing `variable_declarator` without changing which value gets bound —
3036+
/// `await`, redundant parentheses, and TypeScript `as` casts. Real-world call
3037+
/// sites often combine several of these, e.g.
3038+
/// `const { X } = (await import('./mod.js')) as { X: Fn }` nests
3039+
/// await_expression → parenthesized_expression → as_expression before
3040+
/// reaching the declarator (#1781).
3041+
const DYNAMIC_IMPORT_WRAPPER_KINDS: &[&str] =
3042+
&["await_expression", "parenthesized_expression", "as_expression"];
3043+
30343044
/// Extract named bindings from a dynamic `import()` call expression.
3035-
/// Handles: `const { a, b } = await import(...)` and `const mod = await import(...)`
3045+
/// Handles: `const { a, b } = await import(...)`, `const mod = await import(...)`,
3046+
/// and casts/parens wrapping the awaited call, e.g.
3047+
/// `const { a } = (await import(...)) as { a: Fn }`.
30363048
fn extract_dynamic_import_names(call_node: &Node, source: &[u8]) -> Vec<String> {
3037-
// Walk up: call_expression → await_expression? → variable_declarator
3049+
// Walk up through any combination/nesting of await/parenthesized/as-cast
3050+
// wrappers to reach the variable_declarator.
30383051
let mut current = call_node.parent();
3039-
if let Some(parent) = current {
3040-
if parent.kind() == "await_expression" {
3052+
while let Some(parent) = current {
3053+
if DYNAMIC_IMPORT_WRAPPER_KINDS.contains(&parent.kind()) {
30413054
current = parent.parent();
3055+
} else {
3056+
break;
30423057
}
30433058
}
30443059
let declarator = match current {
@@ -4200,6 +4215,47 @@ mod tests {
42004215
assert!(!dyn_imports[0].names.contains(&"nested".to_string()));
42014216
}
42024217

4218+
// Regression tests for #1781: `codegraph exports` failed to credit consumers
4219+
// reached via `const { X } = (await import('./mod.js')) as {...}` — the
4220+
// walk-up from the import() call to its enclosing variable_declarator only
4221+
// skipped a single optional await_expression, so the extra
4222+
// parenthesized_expression / as_expression layers introduced by wrapping
4223+
// parens and a TS type-assertion caused name extraction to bail out with
4224+
// an empty list, exactly as if the destructured names couldn't be
4225+
// determined at all.
4226+
4227+
#[test]
4228+
fn finds_dynamic_import_with_parenthesized_destructuring() {
4229+
let s = parse_ts("const { a, b } = (await import('./foo.js'));");
4230+
let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect();
4231+
assert_eq!(dyn_imports.len(), 1);
4232+
assert!(dyn_imports[0].names.contains(&"a".to_string()));
4233+
assert!(dyn_imports[0].names.contains(&"b".to_string()));
4234+
}
4235+
4236+
#[test]
4237+
fn finds_dynamic_import_with_as_cast_destructuring() {
4238+
let s = parse_ts("const { a, b } = await import('./foo.js') as { a: Fn; b: Fn };");
4239+
let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect();
4240+
assert_eq!(dyn_imports.len(), 1);
4241+
assert!(dyn_imports[0].names.contains(&"a".to_string()));
4242+
assert!(dyn_imports[0].names.contains(&"b".to_string()));
4243+
}
4244+
4245+
#[test]
4246+
fn finds_dynamic_import_with_parenthesized_as_cast_destructuring() {
4247+
// Exact repro shape from #1781 (native-orchestrator.ts):
4248+
// `const { X, Y } = (await import('../mod.js')) as { X: Fn; Y: Fn };`
4249+
let s = parse_ts(
4250+
"const { buildDataflowVerticesFromMap, buildDataflowEdges } = (await import('../../../../features/dataflow.js')) as { buildDataflowVerticesFromMap: Fn; buildDataflowEdges: Fn };",
4251+
);
4252+
let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect();
4253+
assert_eq!(dyn_imports.len(), 1);
4254+
assert_eq!(dyn_imports[0].source, "../../../../features/dataflow.js");
4255+
assert!(dyn_imports[0].names.contains(&"buildDataflowVerticesFromMap".to_string()));
4256+
assert!(dyn_imports[0].names.contains(&"buildDataflowEdges".to_string()));
4257+
}
4258+
42034259
#[test]
42044260
fn extracts_callback_reference_in_router_use() {
42054261
let s = parse_js("router.use(handleToken);");

src/extractors/javascript.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4046,22 +4046,41 @@ function extractImportNames(
40464046
return names;
40474047
}
40484048

4049+
/**
4050+
* Wrapper node types that can sit between a dynamic `import()` call and its
4051+
* enclosing `variable_declarator` without changing which value gets bound —
4052+
* `await`, redundant parentheses, and TypeScript `as` casts. Real-world
4053+
* dynamic-import call sites often combine several of these, e.g.
4054+
* `const { X } = (await import('./mod.js')) as { X: Fn }` nests
4055+
* await_expression → parenthesized_expression → as_expression before
4056+
* reaching the declarator (#1781).
4057+
*/
4058+
const DYNAMIC_IMPORT_WRAPPER_TYPES = new Set([
4059+
'await_expression',
4060+
'parenthesized_expression',
4061+
'as_expression',
4062+
]);
4063+
40494064
/**
40504065
* Extract destructured names from a dynamic import() call expression.
40514066
*
40524067
* Handles:
4053-
* const { a, b } = await import('./foo.js') → ['a', 'b']
4054-
* const mod = await import('./foo.js') → ['mod']
4055-
* import('./foo.js') → [] (no names extractable)
4068+
* const { a, b } = await import('./foo.js') → ['a', 'b']
4069+
* const mod = await import('./foo.js') → ['mod']
4070+
* const { a } = (await import('./foo.js')) as { a: Fn } → ['a']
4071+
* import('./foo.js') → [] (no names extractable)
40564072
*
4057-
* Walks up the AST from the call_expression to find the enclosing
4073+
* Walks up the AST from the call_expression — through any nesting of
4074+
* await/parenthesized/as-cast wrappers — to find the enclosing
40584075
* variable_declarator and reads the name/object_pattern.
40594076
*/
40604077
function extractDynamicImportNames(callNode: TreeSitterNode): string[] {
4061-
// Walk up: call_expression → await_expression → variable_declarator
4078+
// Walk up through await_expression / parenthesized_expression / as_expression
4079+
// wrappers, in any combination or order, to reach the variable_declarator.
40624080
let current = callNode.parent;
4063-
// Skip await_expression wrapper if present
4064-
if (current && current.type === 'await_expression') current = current.parent;
4081+
while (current && DYNAMIC_IMPORT_WRAPPER_TYPES.has(current.type)) {
4082+
current = current.parent;
4083+
}
40654084
// We should now be at a variable_declarator (or not, if standalone import())
40664085
if (current?.type !== 'variable_declarator') return [];
40674086

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
/**
2+
* Integration test for #1781: `codegraph exports` did not credit consumers
3+
* reached via a dynamic `import()` expression whose destructured result is
4+
* wrapped in redundant parentheses and/or a TypeScript `as {...}` type
5+
* assertion — the exact shape used throughout
6+
* `src/domain/graph/builder/stages/native-orchestrator.ts`:
7+
*
8+
* const { buildDataflowVerticesFromMap, ... } =
9+
* (await import('../../../../features/dataflow.js')) as {...};
10+
*
11+
* Root cause: `extractDynamicImportNames` (TS) / `extract_dynamic_import_names`
12+
* (Rust) walked up from the `import()` call through at most one optional
13+
* `await_expression` before requiring the immediate parent to be a
14+
* `variable_declarator`. The extra `parenthesized_expression` and/or
15+
* `as_expression` wrapper layers introduced by parens and a type assertion
16+
* broke that walk-up, so no destructured names were ever extracted — the
17+
* import was recorded with `names: []`, `importedNames` never gained an
18+
* entry for the destructured bindings, and the later call through the
19+
* destructured local name never resolved to a `calls` edge. Both `codegraph
20+
* exports` (consumer count) and `codegraph roles --role dead` therefore
21+
* treated a genuinely-consumed export as unreferenced.
22+
*
23+
* Fix: the walk-up now skips any nesting/combination of `await_expression`,
24+
* `parenthesized_expression`, and `as_expression` wrappers before checking
25+
* for the enclosing `variable_declarator`, in both engines.
26+
*
27+
* Two consumer shapes are exercised against the same target module:
28+
* - `usesPlain`: bare `const { X } = await import(...)` (no cast) — the
29+
* pre-existing baseline that must keep working.
30+
* - `usesCast`: `const { Y } = (await import(...)) as {...}` — the exact
31+
* regression shape from native-orchestrator.ts.
32+
*
33+
* The target module and its consumer are deliberately placed in *different*
34+
* directories (`features/` vs `domain/stages/`, mirroring the real repo
35+
* layout) rather than side by side. A same-directory fixture does not
36+
* discriminate this bug at all: the resolver's directory-scoped fallback
37+
* tier (a same-directory, name-only match — one of several confidence tiers
38+
* below the import-aware match) coincidentally "rescues" the call even when
39+
* `extractDynamicImportNames` returns no names, producing a `calls` edge
40+
* regardless of whether the fix is present. Cross-directory placement is
41+
* required so the only way to resolve `usesCast`'s call is through the
42+
* import-aware path this bug actually breaks.
43+
*/
44+
45+
import fs from 'node:fs';
46+
import os from 'node:os';
47+
import path from 'node:path';
48+
import Database from 'better-sqlite3';
49+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
50+
import { buildGraph } from '../../src/domain/graph/builder.js';
51+
import { exportsData } from '../../src/domain/queries.js';
52+
import { isNativeAvailable } from '../../src/infrastructure/native.js';
53+
54+
const DEAD_ROLES = new Set(['dead-unresolved', 'dead-leaf', 'dead-entry', 'dead-ffi']);
55+
56+
const TARGET_FILE = 'features/dataflow.ts';
57+
58+
const FIXTURE = {
59+
[TARGET_FILE]: `
60+
export function buildDataflowVerticesFromMap(): number {
61+
return 42;
62+
}
63+
64+
export function buildDataflowP4ForNative(): number {
65+
return 7;
66+
}
67+
`,
68+
'domain/stages/consumer.ts': `
69+
export async function usesPlain() {
70+
const { buildDataflowVerticesFromMap } = await import('../../features/dataflow.js');
71+
return buildDataflowVerticesFromMap();
72+
}
73+
74+
export async function usesCast() {
75+
const { buildDataflowP4ForNative } = (await import('../../features/dataflow.js')) as {
76+
buildDataflowP4ForNative: () => number;
77+
};
78+
return buildDataflowP4ForNative();
79+
}
80+
`,
81+
};
82+
83+
function readNodesWithRoles(dbPath: string) {
84+
const db = new Database(dbPath, { readonly: true });
85+
try {
86+
return db.prepare('SELECT name, kind, role FROM nodes ORDER BY name').all() as Array<{
87+
name: string;
88+
kind: string;
89+
role: string | null;
90+
}>;
91+
} finally {
92+
db.close();
93+
}
94+
}
95+
96+
function getCallEdges(dbPath: string) {
97+
const db = new Database(dbPath, { readonly: true });
98+
try {
99+
return db
100+
.prepare(
101+
`SELECT n1.name AS src, n2.name AS tgt, n2.file AS tgt_file
102+
FROM edges e
103+
JOIN nodes n1 ON e.source_id = n1.id
104+
JOIN nodes n2 ON e.target_id = n2.id
105+
WHERE e.kind = 'calls'
106+
ORDER BY n1.name, n2.name`,
107+
)
108+
.all() as Array<{ src: string; tgt: string; tgt_file: string }>;
109+
} finally {
110+
db.close();
111+
}
112+
}
113+
114+
function writeFixture(rootDir: string) {
115+
for (const [rel, content] of Object.entries(FIXTURE)) {
116+
const abs = path.join(rootDir, rel);
117+
fs.mkdirSync(path.dirname(abs), { recursive: true });
118+
fs.writeFileSync(abs, content);
119+
}
120+
}
121+
122+
describe('dynamic import() + destructure consumer crediting (#1781) — WASM', () => {
123+
let tmpDir: string;
124+
125+
beforeAll(async () => {
126+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1781-wasm-'));
127+
writeFixture(tmpDir);
128+
await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true });
129+
});
130+
131+
afterAll(() => {
132+
fs.rmSync(tmpDir, { recursive: true, force: true });
133+
});
134+
135+
it('creates a calls edge for the bare (uncast) destructured binding', () => {
136+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
137+
const edges = getCallEdges(dbPath);
138+
const edge = edges.find(
139+
(e) => e.src === 'usesPlain' && e.tgt === 'buildDataflowVerticesFromMap',
140+
);
141+
expect(
142+
edge,
143+
`Expected usesPlain -> buildDataflowVerticesFromMap; got: ${JSON.stringify(edges)}`,
144+
).toBeDefined();
145+
expect(edge?.tgt_file).toBe(TARGET_FILE);
146+
});
147+
148+
it('creates a calls edge for the parenthesized + `as`-cast destructured binding', () => {
149+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
150+
const edges = getCallEdges(dbPath);
151+
const edge = edges.find((e) => e.src === 'usesCast' && e.tgt === 'buildDataflowP4ForNative');
152+
expect(
153+
edge,
154+
`Expected usesCast -> buildDataflowP4ForNative; got: ${JSON.stringify(edges)}`,
155+
).toBeDefined();
156+
expect(edge?.tgt_file).toBe(TARGET_FILE);
157+
});
158+
159+
it('codegraph exports credits both dynamically-imported functions with real consumers', () => {
160+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
161+
const data = exportsData(TARGET_FILE, dbPath);
162+
163+
const vertices = data.results.find(
164+
(r: { name: string }) => r.name === 'buildDataflowVerticesFromMap',
165+
);
166+
expect(vertices).toBeDefined();
167+
expect(vertices.consumerCount).toBeGreaterThanOrEqual(1);
168+
expect(vertices.consumers.map((c: { name: string }) => c.name)).toContain('usesPlain');
169+
170+
const p4 = data.results.find((r: { name: string }) => r.name === 'buildDataflowP4ForNative');
171+
expect(p4).toBeDefined();
172+
expect(p4.consumerCount).toBeGreaterThanOrEqual(1);
173+
expect(p4.consumers.map((c: { name: string }) => c.name)).toContain('usesCast');
174+
});
175+
176+
it('does not classify either dynamically-imported export as dead', () => {
177+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
178+
const nodes = readNodesWithRoles(dbPath);
179+
for (const name of ['buildDataflowVerticesFromMap', 'buildDataflowP4ForNative']) {
180+
const node = nodes.find((n) => n.name === name && n.kind === 'function');
181+
expect(node, `${name} node not found`).toBeDefined();
182+
expect(
183+
DEAD_ROLES.has(node!.role ?? ''),
184+
`${name} was classified as ${node!.role} — expected a non-dead role now that a real edge exists`,
185+
).toBe(false);
186+
}
187+
});
188+
});
189+
190+
// ── Native engine parity ────────────────────────────────────────────────────
191+
// Skipped when the native addon is not installed.
192+
193+
describe.skipIf(!isNativeAvailable())(
194+
'dynamic import() + destructure consumer crediting (#1781) — native',
195+
() => {
196+
let nativeTmpDir: string;
197+
198+
beforeAll(async () => {
199+
nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1781-native-'));
200+
writeFixture(nativeTmpDir);
201+
await buildGraph(nativeTmpDir, { engine: 'native', incremental: false, skipRegistry: true });
202+
}, 60_000);
203+
204+
afterAll(() => {
205+
fs.rmSync(nativeTmpDir, { recursive: true, force: true });
206+
});
207+
208+
it('creates a calls edge for the bare (uncast) destructured binding', () => {
209+
const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db');
210+
const edges = getCallEdges(dbPath);
211+
const edge = edges.find(
212+
(e) => e.src === 'usesPlain' && e.tgt === 'buildDataflowVerticesFromMap',
213+
);
214+
expect(
215+
edge,
216+
`Expected native usesPlain -> buildDataflowVerticesFromMap; got: ${JSON.stringify(edges)}`,
217+
).toBeDefined();
218+
expect(edge?.tgt_file).toBe(TARGET_FILE);
219+
});
220+
221+
it('creates a calls edge for the parenthesized + `as`-cast destructured binding', () => {
222+
const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db');
223+
const edges = getCallEdges(dbPath);
224+
const edge = edges.find((e) => e.src === 'usesCast' && e.tgt === 'buildDataflowP4ForNative');
225+
expect(
226+
edge,
227+
`Expected native usesCast -> buildDataflowP4ForNative; got: ${JSON.stringify(edges)}`,
228+
).toBeDefined();
229+
expect(edge?.tgt_file).toBe(TARGET_FILE);
230+
});
231+
232+
it('codegraph exports credits both dynamically-imported functions with real consumers', () => {
233+
const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db');
234+
const data = exportsData(TARGET_FILE, dbPath);
235+
236+
const vertices = data.results.find(
237+
(r: { name: string }) => r.name === 'buildDataflowVerticesFromMap',
238+
);
239+
expect(vertices).toBeDefined();
240+
expect(vertices.consumerCount).toBeGreaterThanOrEqual(1);
241+
242+
const p4 = data.results.find((r: { name: string }) => r.name === 'buildDataflowP4ForNative');
243+
expect(p4).toBeDefined();
244+
expect(p4.consumerCount).toBeGreaterThanOrEqual(1);
245+
});
246+
247+
it('does not classify either dynamically-imported export as dead', () => {
248+
const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db');
249+
const nodes = readNodesWithRoles(dbPath);
250+
for (const name of ['buildDataflowVerticesFromMap', 'buildDataflowP4ForNative']) {
251+
const node = nodes.find((n) => n.name === name && n.kind === 'function');
252+
expect(node, `${name} node not found (native)`).toBeDefined();
253+
expect(DEAD_ROLES.has(node!.role ?? '')).toBe(false);
254+
}
255+
});
256+
},
257+
);

0 commit comments

Comments
 (0)