Skip to content

Commit 7a47ce2

Browse files
feat(js-extractor): resolve named function references passed as arguments (#947)
* feat(js-extractor): resolve named function references passed as arguments When a named function is passed as a callback argument — e.g. `router.use(handleToken)`, `arr.map(transform)`, or `promise.then(onSuccess)` — the extractor now creates a dynamic call edge to the referenced function. Previously only the method call itself (e.g. `router.use`) was recorded, leaving the handler with zero callers and a `dead-unresolved` classification. Additionally, destructured `const` bindings such as `const { handleToken, checkPermissions } = initAuth(config)` are now emitted as function definitions so the edge resolver can match them as call targets. Both the query-based and walk-based extraction paths are covered. Tested on a 1 895-file / 10-service Express + TypeScript monorepo: - Before: handleToken 0 callers, checkPermissions 0 callers - After: handleToken 21 callers across 6 services, checkPermissions 18 callers across 3 services - Graph edges: 29 465 → 30 768 (+1 303) - Graph nodes: 18 674 → 18 984 (+310 from destructured bindings) * fix(js-extractor): restrict destructured binding extraction to const Review feedback from Greptile: the previous implementation emitted function definitions for destructured bindings regardless of whether the declaration used `const`, `let`, or `var`. A transient binding like `let { userId } = parseRequest(req)` would be treated as a stable function reference, producing spurious dynamic call edges if `userId` was later passed as an argument. Restrict extraction to `const` declarations, matching the semantics of a stable re-export pattern. Same guard applied in both the query path (`handleVariableDecl`) and the walk path (`extractDestructuredBindingsWalk`). * feat(rust-extractor): port callback reference and destructured binding extraction Port two JS extractor features from WASM/TS to the Rust native engine for engine parity: 1. Callback reference extraction — identifier and member_expression arguments in call expressions are emitted as dynamic call edges (e.g. router.use(handleToken) produces a call to handleToken) 2. Destructured const binding extraction — const { a, b } = init() creates function definitions for each destructured property, restricted to const (not let/var) Impact: 15 functions changed, 3 affected * fix(rust-extractor): skip callback reference extraction on dynamic import() (#947) The Rust handle_call_expr was calling extract_callback_reference_calls unconditionally, including when fn.kind() == "import". The TS walk-path equivalent (handleCallExpr) only runs callback-reference extraction in the else branch, so import(modulePath) emitted a spurious dynamic call to modulePath in the Rust engine but not in TS — violating dual-engine parity. Restructured handle_call_expr to early-return after handle_dynamic_import, mirroring the TS if/else. Added unit test asserting import() arguments are not emitted as dynamic calls. Impact: 2 functions changed, 1 affected * test(js-extractor): add TS test for renamed destructured binding (#947) The Rust suite includes extracts_renamed_destructured_binding covering const { original: renamed } = initAuth() — but the TS side had no equivalent test. If the pair_pattern branch in extractDestructuredBindings were accidentally broken, none of the existing TS tests would catch it. Mirrors the Rust test: asserts the local alias is emitted as a function definition and the original property name is not. * fix(rust-extractor): add function-scope guard to destructured bindings (#947) The Rust walk path's handle_var_decl emitted destructured const bindings from any scope — including inside function bodies — while the TS query path's extractDestructuredBindingsWalk skips FUNCTION_SCOPE_TYPES. For function setup() { const { handleToken } = initAuth(); } the Rust engine emitted handleToken as a definition but the TS WASM engine did not, violating the dual-engine parity requirement. Added find_parent_of_types guard to the object_pattern branch, mirroring the guard already present on the constant-extraction branch. Added unit test skips_destructured_bindings_inside_function_scope asserting the parity. Impact: 2 functions changed, 1 affected * fix(js-extractor): guard TS walk-path destructured bindings by function scope (#947) The walk-path handleVariableDecl emitted destructured const bindings from any scope — including inside function bodies — while the query-path extractDestructuredBindingsWalk already skips FUNCTION_SCOPE_TYPES. When the walk path is used as fallback, function-internal const destructurings were incorrectly registered as definitions, diverging from the query path. Added hasFunctionScopeAncestor helper and gated the object_pattern branch on it, mirroring the Rust handle_var_decl find_parent_of_types guard. Added regression test asserting function-internal destructured bindings are not emitted. Impact: 2 functions changed, 4 affected * fix(ci): rebuild native addon from PR source for test and parity jobs The test and parity CI jobs were running against the last-published native binary (@optave/codegraph-*@3.9.3) installed via npm. When a PR includes Rust extractor changes, the WASM side picks up the new TS changes while the native side stays on the old published binary, producing false parity failures. Add a `scripts/ci-rebuild-native.mjs` helper that runs `napi build --release` and copies the resulting .node file over the published binary in node_modules/@optave/<platform-pkg>/. Wire it into the `test` and `parity` jobs after `npm install` so parity is compared against the Rust source under review. Also reformat the destructured-binding guard in src/extractors/javascript.ts to a single line to satisfy biome (the lint job was failing on this). * refactor(ci): share native build between test and parity jobs Extract the napi build into a dedicated `native-host-build` matrix job that uploads the `.node` per OS as an artifact. `test` and `parity` download that artifact and install it over the published platform binary. Replaces the per-job rebuild (compiled Rust twice) with one build shared by both downstream jobs. `ci-rebuild-native.mjs` becomes `ci-install-native.mjs` — copy-only, no build invocation. * fix(ci): add native-host-build to ci-pipeline needs (#947) Without this, a Rust compile failure in native-host-build causes test and parity to be skipped (not failed), which the ci-pipeline check treats as success since it only matches 'failure' and 'cancelled'. Adding native-host-build to the needs list propagates its failure directly so the required status check correctly turns red. --------- Co-authored-by: carlos-alm <127798846+carlos-alm@users.noreply.github.com>
1 parent b961d1c commit 7a47ce2

5 files changed

Lines changed: 608 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,45 @@ jobs:
3838
- name: Run Biome
3939
run: npx @biomejs/biome check src/ tests/
4040

41+
native-host-build:
42+
strategy:
43+
fail-fast: false
44+
matrix:
45+
os: [ubuntu-latest, macos-latest, windows-latest]
46+
runs-on: ${{ matrix.os }}
47+
name: Native host build (${{ matrix.os }})
48+
steps:
49+
- uses: actions/checkout@v6
50+
51+
- name: Setup Node.js
52+
uses: actions/setup-node@v6
53+
with:
54+
node-version: 22
55+
56+
- name: Setup Rust
57+
uses: dtolnay/rust-toolchain@stable
58+
59+
- name: Rust cache
60+
uses: Swatinem/rust-cache@v2
61+
with:
62+
workspaces: crates/codegraph-core
63+
64+
- name: Install napi-rs CLI
65+
run: npm install -g @napi-rs/cli@3
66+
67+
- name: Build native addon
68+
working-directory: crates/codegraph-core
69+
run: napi build --release
70+
71+
- name: Upload artifact
72+
uses: actions/upload-artifact@v7
73+
with:
74+
name: native-host-${{ matrix.os }}
75+
path: crates/codegraph-core/*.node
76+
if-no-files-found: error
77+
4178
test:
79+
needs: native-host-build
4280
strategy:
4381
fail-fast: false
4482
matrix:
@@ -70,6 +108,16 @@ jobs:
70108
fi
71109
done
72110
111+
- name: Download PR-built native addon
112+
uses: actions/download-artifact@v8
113+
with:
114+
name: native-host-${{ matrix.os }}
115+
path: crates/codegraph-core
116+
117+
- name: Install native addon over published binary
118+
shell: bash
119+
run: node scripts/ci-install-native.mjs
120+
73121
- name: Run tests
74122
run: npm test
75123

@@ -135,6 +183,7 @@ jobs:
135183
node $STRIP_FLAG scripts/verify-imports.ts
136184
137185
parity:
186+
needs: native-host-build
138187
strategy:
139188
fail-fast: false
140189
matrix:
@@ -165,6 +214,16 @@ jobs:
165214
fi
166215
done
167216
217+
- name: Download PR-built native addon
218+
uses: actions/download-artifact@v8
219+
with:
220+
name: native-host-${{ matrix.os }}
221+
path: crates/codegraph-core
222+
223+
- name: Install native addon over published binary
224+
shell: bash
225+
run: node scripts/ci-install-native.mjs
226+
168227
- name: Verify native addon is available
169228
shell: bash
170229
run: |
@@ -224,7 +283,7 @@ jobs:
224283

225284
ci-pipeline:
226285
if: always()
227-
needs: [lint, test, typecheck, audit, verify-imports, rust-check, parity]
286+
needs: [lint, native-host-build, test, typecheck, audit, verify-imports, rust-check, parity]
228287
runs-on: ubuntu-latest
229288
name: CI Testing Pipeline
230289
steps:

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

Lines changed: 216 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,17 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
280280
cfg: build_function_cfg(&value_n, "javascript", source),
281281
children: opt_children(children),
282282
});
283+
} else if is_const && name_n.kind() == "object_pattern"
284+
&& find_parent_of_types(node, &[
285+
"function_declaration", "arrow_function",
286+
"function_expression", "method_definition",
287+
"generator_function_declaration", "generator_function",
288+
]).is_none()
289+
{
290+
// Parity with TS query path (extractDestructuredBindingsWalk):
291+
// skip destructured const bindings inside function scopes so the
292+
// Rust walk path matches FUNCTION_SCOPE_TYPES behaviour.
293+
extract_destructured_bindings(&name_n, source, start_line(node), end_line(node), &mut symbols.definitions);
283294
} else if is_const && is_js_literal(&value_n)
284295
&& find_parent_of_types(node, &[
285296
"function_declaration", "arrow_function",
@@ -302,16 +313,18 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
302313
}
303314

304315
fn handle_call_expr(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
305-
if let Some(fn_node) = node.child_by_field_name("function") {
306-
if fn_node.kind() == "import" {
307-
handle_dynamic_import(node, &fn_node, source, symbols);
308-
} else if let Some(call_info) = extract_call_info(&fn_node, node, source) {
309-
symbols.calls.push(call_info);
310-
}
316+
let Some(fn_node) = node.child_by_field_name("function") else { return };
317+
if fn_node.kind() == "import" {
318+
handle_dynamic_import(node, &fn_node, source, symbols);
319+
return;
320+
}
321+
if let Some(call_info) = extract_call_info(&fn_node, node, source) {
322+
symbols.calls.push(call_info);
311323
}
312324
if let Some(cb_def) = extract_callback_definition(node, source) {
313325
symbols.definitions.push(cb_def);
314326
}
327+
extract_callback_reference_calls(node, source, &mut symbols.calls);
315328
}
316329

317330
fn handle_new_expr(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
@@ -865,6 +878,85 @@ fn extract_implements_depth(node: &Node, source: &[u8], result: &mut Vec<String>
865878
}
866879
}
867880

881+
fn extract_callback_reference_calls(call_node: &Node, source: &[u8], calls: &mut Vec<Call>) {
882+
let args = call_node.child_by_field_name("arguments")
883+
.or_else(|| find_child(call_node, "arguments"));
884+
let Some(args) = args else { return };
885+
let call_line = start_line(call_node);
886+
887+
for i in 0..args.child_count() {
888+
let Some(child) = args.child(i) else { continue };
889+
match child.kind() {
890+
"identifier" => {
891+
calls.push(Call {
892+
name: node_text(&child, source).to_string(),
893+
line: call_line,
894+
dynamic: Some(true),
895+
receiver: None,
896+
});
897+
}
898+
"member_expression" => {
899+
if let Some(prop) = child.child_by_field_name("property") {
900+
let receiver = child.child_by_field_name("object")
901+
.map(|obj| node_text(&obj, source).to_string());
902+
calls.push(Call {
903+
name: node_text(&prop, source).to_string(),
904+
line: call_line,
905+
dynamic: Some(true),
906+
receiver,
907+
});
908+
}
909+
}
910+
_ => {}
911+
}
912+
}
913+
}
914+
915+
fn extract_destructured_bindings(
916+
pattern: &Node,
917+
source: &[u8],
918+
line: u32,
919+
end_line: u32,
920+
definitions: &mut Vec<Definition>,
921+
) {
922+
for i in 0..pattern.child_count() {
923+
let Some(child) = pattern.child(i) else { continue };
924+
match child.kind() {
925+
"shorthand_property_identifier_pattern" | "shorthand_property_identifier" => {
926+
definitions.push(Definition {
927+
name: node_text(&child, source).to_string(),
928+
kind: "function".to_string(),
929+
line,
930+
end_line: Some(end_line),
931+
decorators: None,
932+
complexity: None,
933+
cfg: None,
934+
children: None,
935+
});
936+
}
937+
"pair_pattern" | "pair" => {
938+
if let Some(value) = child.child_by_field_name("value") {
939+
if value.kind() == "identifier"
940+
|| value.kind() == "shorthand_property_identifier_pattern"
941+
{
942+
definitions.push(Definition {
943+
name: node_text(&value, source).to_string(),
944+
kind: "function".to_string(),
945+
line,
946+
end_line: Some(end_line),
947+
decorators: None,
948+
complexity: None,
949+
cfg: None,
950+
children: None,
951+
});
952+
}
953+
}
954+
}
955+
_ => {}
956+
}
957+
}
958+
}
959+
868960
fn extract_call_info(fn_node: &Node, call_node: &Node, source: &[u8]) -> Option<Call> {
869961
match fn_node.kind() {
870962
"identifier" => Some(Call {
@@ -1584,4 +1676,122 @@ mod tests {
15841676
assert!(dyn_imports[0].names.contains(&"foo".to_string()));
15851677
assert!(!dyn_imports[0].names.contains(&"nested".to_string()));
15861678
}
1679+
1680+
#[test]
1681+
fn extracts_callback_reference_in_router_use() {
1682+
let s = parse_js("router.use(handleToken);");
1683+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1684+
assert!(dynamic_calls.iter().any(|c| c.name == "handleToken"), "should extract handleToken as dynamic call");
1685+
}
1686+
1687+
#[test]
1688+
fn extracts_multiple_callback_references() {
1689+
let s = parse_js("app.get('/api', authenticate, validate, handler);");
1690+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1691+
assert!(dynamic_calls.iter().any(|c| c.name == "authenticate"));
1692+
assert!(dynamic_calls.iter().any(|c| c.name == "validate"));
1693+
assert!(dynamic_calls.iter().any(|c| c.name == "handler"));
1694+
}
1695+
1696+
#[test]
1697+
fn extracts_member_expression_callback() {
1698+
let s = parse_js("app.use(auth.validate);");
1699+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1700+
let cb = dynamic_calls.iter().find(|c| c.name == "validate");
1701+
assert!(cb.is_some(), "should extract validate as dynamic call");
1702+
assert_eq!(cb.unwrap().receiver.as_deref(), Some("auth"));
1703+
}
1704+
1705+
#[test]
1706+
fn extracts_callback_in_array_method() {
1707+
let s = parse_js("items.map(transform);");
1708+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1709+
assert!(dynamic_calls.iter().any(|c| c.name == "transform"));
1710+
}
1711+
1712+
#[test]
1713+
fn extracts_callback_in_settimeout() {
1714+
let s = parse_js("setTimeout(tick, 1000);");
1715+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1716+
assert!(dynamic_calls.iter().any(|c| c.name == "tick"));
1717+
}
1718+
1719+
#[test]
1720+
fn no_dynamic_calls_for_non_identifiers() {
1721+
let s = parse_js("app.get('/path', {key: 1}, [], 42);");
1722+
let dynamic_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1723+
assert!(dynamic_calls.is_empty());
1724+
}
1725+
1726+
#[test]
1727+
fn no_duplicate_call_for_call_expression_arg() {
1728+
let s = parse_js("router.use(checkPermissions(['admin']));");
1729+
let cp_calls: Vec<_> = s.calls.iter().filter(|c| c.name == "checkPermissions").collect();
1730+
assert_eq!(cp_calls.len(), 1);
1731+
}
1732+
1733+
#[test]
1734+
fn no_dynamic_call_for_dynamic_import_arg() {
1735+
// Parity with TS walk path: callback-reference extraction must be skipped
1736+
// when the call is a dynamic `import()`. Otherwise `import(modulePath)`
1737+
// would emit a spurious dynamic call to `modulePath`.
1738+
let s = parse_js("const mod = await import(modulePath);");
1739+
let dyn_calls: Vec<_> = s.calls.iter().filter(|c| c.dynamic == Some(true)).collect();
1740+
assert!(
1741+
!dyn_calls.iter().any(|c| c.name == "modulePath"),
1742+
"import() argument must not be emitted as a dynamic call"
1743+
);
1744+
}
1745+
1746+
#[test]
1747+
fn extracts_destructured_const_bindings() {
1748+
let s = parse_js("const { handleToken, checkPermissions } = initAuth(config);");
1749+
let names: Vec<&str> = s.definitions.iter().map(|d| d.name.as_str()).collect();
1750+
assert!(names.contains(&"handleToken"), "should extract handleToken definition");
1751+
assert!(names.contains(&"checkPermissions"), "should extract checkPermissions definition");
1752+
let ht = s.definitions.iter().find(|d| d.name == "handleToken").unwrap();
1753+
assert_eq!(ht.kind, "function");
1754+
}
1755+
1756+
#[test]
1757+
fn extracts_exported_destructured_const_bindings() {
1758+
let s = parse_js("export const { handleToken, checkPermissions } = initAuth(config);");
1759+
let names: Vec<&str> = s.definitions.iter().map(|d| d.name.as_str()).collect();
1760+
assert!(names.contains(&"handleToken"));
1761+
assert!(names.contains(&"checkPermissions"));
1762+
}
1763+
1764+
#[test]
1765+
fn skips_let_var_destructured_bindings() {
1766+
let s = parse_js("let { userId, email } = parseRequest(req);");
1767+
assert!(!s.definitions.iter().any(|d| d.name == "userId"));
1768+
assert!(!s.definitions.iter().any(|d| d.name == "email"));
1769+
1770+
let s2 = parse_js("var { foo, bar } = getConfig();");
1771+
assert!(!s2.definitions.iter().any(|d| d.name == "foo"));
1772+
assert!(!s2.definitions.iter().any(|d| d.name == "bar"));
1773+
}
1774+
1775+
#[test]
1776+
fn skips_destructured_bindings_inside_function_scope() {
1777+
// Parity with TS query path (extractDestructuredBindingsWalk), which
1778+
// skips FUNCTION_SCOPE_TYPES. Function-internal destructured const
1779+
// bindings must not be emitted as definitions in the Rust walk path.
1780+
let s = parse_js("function setup() { const { handleToken, checkPermissions } = initAuth(config); }");
1781+
assert!(
1782+
!s.definitions.iter().any(|d| d.name == "handleToken"),
1783+
"function-nested destructured binding must not be emitted"
1784+
);
1785+
assert!(
1786+
!s.definitions.iter().any(|d| d.name == "checkPermissions"),
1787+
"function-nested destructured binding must not be emitted"
1788+
);
1789+
}
1790+
1791+
#[test]
1792+
fn extracts_renamed_destructured_binding() {
1793+
let s = parse_js("const { original: renamed } = initAuth();");
1794+
assert!(s.definitions.iter().any(|d| d.name == "renamed"), "should use the local alias");
1795+
assert!(!s.definitions.iter().any(|d| d.name == "original"), "should not use the original key");
1796+
}
15871797
}

0 commit comments

Comments
 (0)