From 651518638d26c38c1654b11feecbd8d177792477 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Mon, 6 Jul 2026 10:05:22 -0600 Subject: [PATCH] fix: emit reference edges for function identifiers used as object-literal values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codegraph roles --role dead inconsistently flagged dispatch-table handler functions (`{ matches, resolve: someFunction }`-style arrays) as dead, depending on an unrelated, incidental property of the referenced function: whether it happened to call another tracked symbol internally (fanOut > 0). Root cause: codegraph created no edge at all for a bare function identifier used as an object-literal property VALUE. The role classifier's `node.kind === 'function' && node.fanOut > 0` heuristic in classifyUnreferencedNode incidentally rescued whichever handlers happened to have nonzero fanOut, leaving the rest misclassified dead-unresolved — a false positive that risks deleting live dispatch-table handlers during dead-code cleanup. Fix: both engines now extract a dynamic `calls` edge (dynamicKind / dynamic_kind = 'value-ref') for every bare-identifier object-literal property value and shorthand property (`{ resolve: fn }` / `{ fn }`), attributed to the enclosing scope via the existing findCaller machinery (falls back to the widest enclosing constant/variable binding for top-level dispatch tables, same as any other call). Resolution is restricted to function/method-kind targets only, so plain data references (`{ name: SOME_CONSTANT }`) are silently dropped rather than fabricating nonsensical edges to constants. Reuses the existing `calls` edge kind + dynamic flag (per ADR-002's "no new edge kind" precedent, and the existing extractCallbackReferenceCalls mechanism from #1741) rather than inventing a new edge kind — fan-in/fan-out (scoped to `kind IN ('calls','imports-type')` / `kind = 'calls'`) already account for it with zero changes to structure.ts/roles.rs's median computations. The `fanOut > 0` heuristic in classifyUnreferencedNode is kept, narrowed by comment: it's no longer load-bearing for the object-literal case but remains the only rescue for sibling value-reference patterns not yet extracted as edges (logical-or fallback defaults, ternary defaults, array-of-functions elements). Applied to both engines: - WASM/TS: src/extractors/javascript.ts (collectObjectLiteralValueRefCall, wired into the shared runCollectorWalk used by both the walk and query extraction paths), src/domain/graph/builder/stages/build-edges.ts (resolveFallbackTargets kind filter), src/domain/graph/builder/incremental.ts (mirrored filter for the incremental/watch-mode path), src/types.ts (new 'value-ref' DynamicKind variant). - Native: crates/codegraph-core/src/extractors/javascript.rs (handle_object_literal_pair_value_ref / handle_object_literal_shorthand_value_ref, wired into match_js_node), crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs (matching kind filter in process_file). Verified against the exact repro (this repo's own PARAM_NODE_HANDLERS dispatch table in src/ast-analysis/visitor-utils.ts): all 5 handlers now show totalDependents=1 and role=core on both engines, with zero dead-flagged symbols in that file. Resolution-benchmark precision/recall is unchanged across all 34 language fixtures (javascript 100/100, typescript 95.7/93.6, aggregate 63.8% recall, identical before and after). Fixes #1771 docs check acknowledged — internal edge-emission/resolver bug fix, no new commands, languages, or architecture to document. Impact: 9 functions changed, 19 affected --- .../graph/builder/stages/build_edges.rs | 11 + .../src/extractors/javascript.rs | 139 +++++++++++ .../src/graph/classifiers/roles.rs | 11 + .../decisions/002-dynamic-call-resolution.md | 11 +- src/domain/graph/builder/incremental.ts | 9 +- .../graph/builder/stages/build-edges.ts | 16 ++ src/extractors/javascript.ts | 54 +++++ src/graph/classifiers/roles.ts | 11 + src/types.ts | 3 +- ...ssue-1771-dispatch-table-value-ref.test.ts | 228 ++++++++++++++++++ tests/parsers/javascript.test.ts | 69 ++++++ 11 files changed, 559 insertions(+), 3 deletions(-) create mode 100644 tests/integration/issue-1771-dispatch-table-value-ref.test.ts diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index 94b35cc06..01a4fbaed 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -785,6 +785,17 @@ fn process_file<'a>( let mut targets = resolve_call_targets( ctx, call, fc.rel_path, imported_from, &fc.type_map, caller_name, imported_original_name, ); + // #1771: object-literal property-value references resolve against + // function/method-kind targets only — a bare identifier there is as + // likely to be a plain data reference (`{ name: SOME_CONSTANT }`) as + // a function reference, so drop any non-callable match rather than + // fabricating a "calls" edge to a constant/class/etc. Applied once + // here (after all resolve_call_targets tiers), mirroring the + // `dynamicKind === 'value-ref'` filter in resolveFallbackTargets + // (stages/build-edges.ts). + if call.dynamic_kind.as_deref() == Some("value-ref") { + targets.retain(|t| t.kind == "function" || t.kind == "method"); + } sort_targets_by_confidence(&mut targets, fc.rel_path, imported_from); emit_call_edges(&targets, caller_id, is_dynamic, fc.rel_path, imported_from, &mut seen_edges, &mut pts_edge_map, edges); diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index d273edb2e..2168a30d6 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1075,6 +1075,12 @@ fn match_js_node(node: &Node, source: &[u8], symbols: &mut FileSymbols, _depth: "import_statement" => handle_import_stmt(node, source, symbols), "export_statement" => handle_export_stmt(node, source, symbols), "expression_statement" => handle_expr_stmt(node, source, symbols), + // #1771: dispatch-table-style object-literal property values + // (`{ resolve: someFunction }` / shorthand `{ someFunction }`). + "pair" => handle_object_literal_pair_value_ref(node, source, &mut symbols.calls), + "shorthand_property_identifier" => { + handle_object_literal_shorthand_value_ref(node, source, &mut symbols.calls) + } _ => {} } } @@ -2440,6 +2446,62 @@ fn extract_callback_reference_calls(call_node: &Node, source: &[u8], calls: &mut } } +/// Collect a dynamic value-ref `Call` for an object-literal `pair` node whose +/// value is a bare identifier — e.g. `{ resolve: someFunction }`, the +/// "dispatch table" pattern (`{ matches, resolve }`-style handler arrays, +/// issue #1771). Restricted to plain `identifier` values: call expressions, +/// member expressions, and inline function/arrow values are handled by their +/// own extraction paths (regular call resolution, `seed_objlit_type_map_entries` +/// / `match_js_objlit_qualified_method_defs`) and must not be double-counted here. +/// +/// Emitted unconditionally for every bare-identifier property value in the +/// file — `dynamic_kind: "value-ref"` is resolved downstream (build_edges.rs) +/// against function/method-kind targets ONLY, so plain data references +/// (`{ name: SOME_CONSTANT }`) naturally fail to resolve into an edge rather +/// than needing a structural allowlist gate here. +/// +/// Mirrors `collectObjectLiteralValueRefCall` in `src/extractors/javascript.ts`. +fn handle_object_literal_pair_value_ref(node: &Node, source: &[u8], calls: &mut Vec) { + let Some(value_n) = node.child_by_field_name("value") else { return }; + if value_n.kind() != "identifier" { + return; + } + let text = node_text(&value_n, source); + if JS_BUILTIN_GLOBALS.contains(&text) { + return; + } + calls.push(Call { + name: text.to_string(), + line: start_line(&value_n), + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); +} + +/// Collect a dynamic value-ref `Call` for an object-literal shorthand property +/// (`{ someFunction }`) — semantically identical to `{ someFunction: someFunction }`. +/// `shorthand_property_identifier` only appears inside object-literal +/// EXPRESSIONS in this grammar (destructuring patterns use the distinct +/// `shorthand_property_identifier_pattern` kind), so this can't misfire on +/// destructuring targets. +/// +/// Mirrors the walk path's `shorthand_property_identifier` handling in +/// `src/extractors/javascript.ts`'s `runCollectorWalk` (issue #1771). +fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls: &mut Vec) { + let text = node_text(node, source); + if JS_BUILTIN_GLOBALS.contains(&text) { + return; + } + calls.push(Call { + name: text.to_string(), + line: start_line(node), + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); +} + fn extract_destructured_bindings( pattern: &Node, source: &[u8], @@ -4171,6 +4233,83 @@ mod tests { assert!(dynamic_calls.is_empty()); } + // ── #1771: object-literal value-ref extraction ────────────────────────── + + #[test] + fn extracts_value_ref_call_for_object_literal_property() { + let s = parse_js("const table = { resolve: resolveWrapperParam };"); + let value_refs: Vec<_> = s + .calls + .iter() + .filter(|c| c.dynamic_kind.as_deref() == Some("value-ref")) + .collect(); + assert!(value_refs.iter().any(|c| c.name == "resolveWrapperParam")); + assert!(value_refs.iter().all(|c| c.dynamic == Some(true))); + } + + #[test] + fn extracts_value_ref_calls_for_every_handler_in_dispatch_table_array() { + // Mirrors this repo's own PARAM_NODE_HANDLERS pattern (issue #1771). + let s = parse_js( + "const HANDLERS = [\n\ + { matches: isA, resolve: resolveA },\n\ + { matches: isB, resolve: resolveB },\n\ + ];", + ); + let names: Vec<&str> = s + .calls + .iter() + .filter(|c| c.dynamic_kind.as_deref() == Some("value-ref")) + .map(|c| c.name.as_str()) + .collect(); + for expected in ["isA", "resolveA", "isB", "resolveB"] { + assert!(names.contains(&expected), "missing value-ref call for {}", expected); + } + } + + #[test] + fn extracts_value_ref_call_for_shorthand_property() { + let s = parse_js("const table = { someFunction };"); + let value_refs: Vec<_> = s + .calls + .iter() + .filter(|c| c.dynamic_kind.as_deref() == Some("value-ref")) + .collect(); + assert!(value_refs.iter().any(|c| c.name == "someFunction")); + } + + #[test] + fn no_value_ref_call_for_call_expression_value() { + let s = parse_js("const table = { resolve: someFunction() };"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_member_expression_value() { + let s = parse_js("const table = { resolve: obj.someFunction };"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_inline_function_value() { + let s = parse_js("const table = { resolve: () => {}, other: function () {} };"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_literal_or_data_shaped_values() { + let s = parse_js( + "const config = { name: 'literal', count: 42, active: true, empty: null, list: [1, 2] };", + ); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_builtin_globals() { + let s = parse_js("const table = { log: console, Ctor: Object };"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + #[test] fn no_duplicate_call_for_call_expression_arg() { let s = parse_js("router.use(checkPermissions(['admin']));"); diff --git a/crates/codegraph-core/src/graph/classifiers/roles.rs b/crates/codegraph-core/src/graph/classifiers/roles.rs index 1b45f03bc..b4572e93d 100644 --- a/crates/codegraph-core/src/graph/classifiers/roles.rs +++ b/crates/codegraph-core/src/graph/classifiers/roles.rs @@ -237,6 +237,17 @@ fn classify_node( // value references, not call sites, so no call edge is produced. We // require `fan_out > 0` as evidence that the function is non-trivial // (i.e. it calls something), ruling out truly inert dead helpers. + // + // NOTE (#1771): this used to also be the only thing rescuing functions + // referenced as object-literal property values (dispatch tables, e.g. + // `{ resolve: someFunction }`) — and only by coincidence, for whichever + // of those functions happened to have fan_out > 0 themselves. That + // pattern now gets a real `calls` edge (dynamic_kind "value-ref") at + // extraction time, so it no longer depends on this heuristic. Kept + // here as a fallback for value-reference shapes that still produce no + // edge at all — the logical-or default above, and others (ternary + // defaults, array-of-functions elements, default parameter values) + // that aren't extracted as edges yet. if kind == "function" && fan_out > 0 { return "leaf"; } diff --git a/docs/architecture/decisions/002-dynamic-call-resolution.md b/docs/architecture/decisions/002-dynamic-call-resolution.md index 0e70c3422..5159f7b99 100644 --- a/docs/architecture/decisions/002-dynamic-call-resolution.md +++ b/docs/architecture/decisions/002-dynamic-call-resolution.md @@ -76,11 +76,20 @@ export type DynamicKind = | 'computed-key' // obj[k]() — resolvable iff k is a const literal, else flag | 'reflection' // .call/.apply/.bind, getattr, Method.invoke, $obj->$m() | 'eval' // eval(), new Function() — undecidable - | 'unresolved-dynamic'; // detected dynamic call we cannot resolve + | 'unresolved-dynamic' // detected dynamic call we cannot resolve + | 'value-ref'; // bare identifier used as an object-literal property value + // (dispatch tables, e.g. `{ resolve: someFn }`) — resolvable + // against function/method-kind targets only (#1771) ``` `dynamic?: boolean` is kept to avoid churning every `call.dynamic ? 1 : 0` site. +`value-ref` is Track A (resolvable) but deliberately **not** added to the flag-only +sink-edge set: when the identifier doesn't resolve to a function/method (e.g. a +plain data reference like `{ name: SOME_CONSTANT }`), that's the common case, not +an undecidable dynamic call site — so it's silently dropped rather than flagged, +unlike `eval`/`computed-key`/`unresolved-dynamic`. + ### Sink edges reuse `kind='calls'`, not a new edge kind A new `EdgeKind` would ripple through every edge-kind switch, role classifier, exporter, MCP tool, and the viewer — high blast radius. Instead: DB migration adds `dynamic_kind TEXT` column to `edges`; sink edges use `kind='calls'`, `dynamic=1`, `dynamic_kind=`, `confidence=0.0`. Confidence below `DEFAULT_MIN_CONFIDENCE=0.5` means they never pollute normal queries or exports but remain queryable when explicitly requested. diff --git a/src/domain/graph/builder/incremental.ts b/src/domain/graph/builder/incremental.ts index 5f3492837..6c5b1e7cc 100644 --- a/src/domain/graph/builder/incremental.ts +++ b/src/domain/graph/builder/incremental.ts @@ -758,7 +758,7 @@ function buildCallEdges( importedOriginalNames, ); - const targets = applyCallFallbacks( + let targets = applyCallFallbacks( call, caller.callerName, relPath, @@ -768,6 +768,13 @@ function buildCallEdges( initialTargets, ); + // #1771: object-literal property-value references resolve against + // function/method-kind targets only — mirrors the same filter in + // resolveFallbackTargets (stages/build-edges.ts, full-build path). + if (call.dynamicKind === 'value-ref') { + targets = targets.filter((t) => t.kind === 'function' || t.kind === 'method'); + } + edgesAdded += emitIncrementalCallEdges( call, caller, diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index a67814c00..03ab97c63 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -1368,6 +1368,22 @@ function resolveFallbackTargets( if (qualified.length > 0) targets = qualified; } + // #1771: object-literal property-value references (`{ resolve: someFn }`) + // resolve against function/method-kind targets only — a bare identifier + // there is as likely to be a plain data reference (`{ name: SOME_CONSTANT }`) + // as a function, so drop any non-callable match rather than fabricating a + // "calls" edge to a constant/class/etc. Applied once here, after every + // fallback tier above, so it covers whichever tier produced the match. + if (call.dynamicKind === 'value-ref') { + // `targets` is typed without `kind` when it flows straight through from + // resolveCallTargets (call-resolver.ts's declared return type omits it), + // but every underlying CallNodeLookup method actually populates it — the + // same gap the preQualifiedTargets cast above already works around. + targets = (targets as ReadonlyArray<{ id: number; file: string; kind?: string }>).filter( + (t) => t.kind === 'function' || t.kind === 'method', + ); + } + return { targets, importedFrom }; } diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 097c1f127..f780290b2 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -459,6 +459,7 @@ function extractSymbolsQuery(tree: TreeSitterTree, query: TreeSitterQuery): Extr objectPropBindings, newExpressions, definePropertyReceivers, + valueRefCalls: calls, imports, calls, thisCallBindings, @@ -884,6 +885,7 @@ function extractSymbolsWalk(tree: TreeSitterTree): ExtractorOutput { objectPropBindings: ctx.objectPropBindings!, newExpressions, definePropertyReceivers, + valueRefCalls: ctx.calls, funcPropDefs: ctx.definitions, }); ctx.newExpressions = newExpressions; @@ -3138,6 +3140,32 @@ function collectObjectPropBindings(node: TreeSitterNode, bindings: ObjectPropBin } } +/** + * Collect a dynamic value-ref `Call` for an object-literal `pair` node whose + * value is a bare identifier — e.g. `{ resolve: someFunction }`, the + * "dispatch table" pattern (`{ matches, resolve }`-style handler arrays, + * issue #1771). Restricted to plain `identifier` values: call expressions, + * member expressions, and inline function/arrow values are handled by their + * own extraction paths (regular call resolution, `extractObjectLiteralFunctions`) + * and must not be double-counted here. + * + * Emitted unconditionally for every bare-identifier property value in the + * file — `dynamicKind: 'value-ref'` is resolved downstream (build-edges.ts / + * incremental.ts) against function/method-kind targets ONLY, so plain data + * references (`{ name: SOME_CONSTANT }`) naturally fail to resolve into an + * edge rather than needing a structural allowlist gate here. + */ +function collectObjectLiteralValueRefCall(pairNode: TreeSitterNode, calls: Call[]): void { + const valueNode = pairNode.childForFieldName('value'); + if (valueNode?.type !== 'identifier' || BUILTIN_GLOBALS.has(valueNode.text)) return; + calls.push({ + name: valueNode.text, + line: nodeStartLine(valueNode), + dynamic: true, + dynamicKind: 'value-ref', + }); +} + function extractReceiverName(objNode: TreeSitterNode | null): string | undefined { if (!objNode) return undefined; const t = objNode.type; @@ -3687,6 +3715,15 @@ function collectThisCallAndBindings( * (the walk path's walkJavaScriptNode covers those node types itself). * - `funcPropDefs` — walk path only (the query path captures `fn.method = …` * assignments via the `assign_left`/`assign_right` query pattern). + * + * `valueRefCalls` is REQUIRED (unlike `calls`) — both paths route + * object-literal value-ref extraction through this single field, since + * neither `walkJavaScriptNode` (walk path) nor the compiled query patterns + * (query path) visit `pair`/`shorthand_property_identifier` nodes on their + * own (#1771). Both callers pass their own `calls` array here; it's a + * separate field from the optional `calls` above purely so this collector + * isn't accidentally gated off by the walk path's "don't double-collect + * call_expression" omission. */ interface CollectorWalkTargets { definitions: Definition[]; @@ -3696,6 +3733,7 @@ interface CollectorWalkTargets { objectPropBindings: ObjectPropBinding[]; newExpressions: string[]; definePropertyReceivers: Map; + valueRefCalls: Call[]; imports?: Import[]; calls?: Call[]; thisCallBindings?: ThisCallBinding[]; @@ -3774,6 +3812,22 @@ function runCollectorWalk(rootNode: TreeSitterNode, targets: CollectorWalkTarget case 'class_static_block': if (targets.classMemberDefs) handleStaticBlock(node, targets.classMemberDefs); break; + case 'pair': + // #1771: dispatch-table-style object-literal property values, e.g. + // `{ resolve: someFunction }`. + collectObjectLiteralValueRefCall(node, targets.valueRefCalls); + break; + case 'shorthand_property_identifier': + // #1771: shorthand form of the same pattern, e.g. `{ someFunction }`. + if (!BUILTIN_GLOBALS.has(node.text)) { + targets.valueRefCalls.push({ + name: node.text, + line: nodeStartLine(node), + dynamic: true, + dynamicKind: 'value-ref', + }); + } + break; } for (let i = 0; i < node.childCount; i++) { walk(node.child(i)!, depth + 1, childInDynamicImport); diff --git a/src/graph/classifiers/roles.ts b/src/graph/classifiers/roles.ts index fe278ff31..632037aa2 100644 --- a/src/graph/classifiers/roles.ts +++ b/src/graph/classifiers/roles.ts @@ -219,6 +219,17 @@ function classifyUnreferencedNode(node: RoleClassificationNode): Role { // value references, not call sites, so no call edge is produced. We // require `fanOut > 0` as evidence that the function is non-trivial // (i.e. it calls something), ruling out truly inert dead helpers. + // + // NOTE (#1771): this used to also be the only thing rescuing functions + // referenced as object-literal property values (dispatch tables, e.g. + // `{ resolve: someFunction }`) — and only by coincidence, for whichever + // of those functions happened to have fanOut > 0 themselves. That + // pattern now gets a real `calls` edge (dynamicKind 'value-ref') at + // extraction time, so it no longer depends on this heuristic. Kept + // here as a fallback for value-reference shapes that still produce no + // edge at all — the logical-or default above, and others (ternary + // defaults, array-of-functions elements, default parameter values) + // that aren't extracted as edges yet. return 'leaf'; } } diff --git a/src/types.ts b/src/types.ts index 954b9fb5a..c352bb0b5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -487,7 +487,8 @@ export type DynamicKind = | 'computed-key' // obj[k]() — potentially resolvable via pts; else flagged | 'reflection' // .call/.apply/.bind / Reflect.* / callable-ref — resolved when target is in codebase; sink edge emitted if unresolved | 'eval' // eval() / new Function() — undecidable; always flagged - | 'unresolved-dynamic'; // any other detected dynamic pattern; flagged + | 'unresolved-dynamic' // any other detected dynamic pattern; flagged + | 'value-ref'; // bare identifier used as an object-literal property value (dispatch-table pattern, e.g. `{ resolve: someFn }`) — resolved only against function/method-kind targets; unresolved (e.g. plain data references) are dropped silently, NOT flagged /** A function/method call detected by an extractor. */ export interface Call { diff --git a/tests/integration/issue-1771-dispatch-table-value-ref.test.ts b/tests/integration/issue-1771-dispatch-table-value-ref.test.ts new file mode 100644 index 000000000..5a92bd3cc --- /dev/null +++ b/tests/integration/issue-1771-dispatch-table-value-ref.test.ts @@ -0,0 +1,228 @@ +/** + * Integration test for #1771: dispatch-table function references + * (`{ matches, resolve }`-style handler arrays) inconsistently flagged + * dead-unresolved depending on an unrelated, incidental property of the + * referenced function (whether it happens to call another tracked symbol + * internally, giving it fanOut > 0). + * + * Root cause: codegraph created no edge at all for a bare function + * identifier used as an object-literal property VALUE (e.g. + * `{ resolve: someFunction }`) — only a `fanOut > 0` heuristic in the role + * classifier incidentally rescued whichever handlers happened to call + * another tracked symbol internally. + * + * Fix: both engines now emit a real `calls` edge (dynamic=1, + * dynamicKind/dynamic_kind = 'value-ref') from the enclosing scope to the + * referenced function/method, so fan-in reflects the reference directly and + * role classification no longer depends on the referenced function's own + * unrelated fanOut. + */ + +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 { isNativeAvailable } from '../../src/infrastructure/native.js'; + +// Mirrors this repo's own src/ast-analysis/visitor-utils.ts PARAM_NODE_HANDLERS +// dispatch table: an array of `{ matches, resolve }` objects where `resolve` +// is a bare function identifier dispatched at runtime via `handler.resolve(...)`. +// `resolveB` deliberately calls another tracked function (`helper`) so it has +// fanOut > 0 — the exact "coincidental rescue" condition from the bug report — +// while `resolveA`/`resolveC` do not, so they exercise the actually-fixed path. +const FIXTURE = { + 'dispatch.js': ` +function isA(node) { return node.type === 'a'; } +function resolveA(node) { return { kind: 'a', value: node }; } +function isB(node) { return node.type === 'b'; } +function resolveB(node) { return helper(node); } +function isC(node) { return node.type === 'c'; } +function resolveC(node) { return { kind: 'c', value: node }; } +function helper(node) { return node; } + +const HANDLERS = [ + { matches: isA, resolve: resolveA }, + { matches: isB, resolve: resolveB }, + { matches: isC, resolve: resolveC }, +]; + +function dispatch(node) { + for (const h of HANDLERS) { + if (h.matches(node)) return h.resolve(node); + } + return null; +} + +module.exports = { dispatch }; +`, + 'data.js': ` +const SOME_CONSTANT = 'hello'; +const dataConfig = { name: SOME_CONSTANT, label: 'literal', count: 42 }; +module.exports = { dataConfig }; +`, +}; + +const HANDLER_NAMES = ['isA', 'resolveA', 'isB', 'resolveB', 'isC', 'resolveC']; +const DEAD_ROLES = new Set(['dead-unresolved', 'dead-leaf', 'dead-entry', 'dead-ffi']); + +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 countCallEdgesTo(dbPath: string, targetName: string): number { + const db = new Database(dbPath, { readonly: true }); + try { + const row = db + .prepare( + `SELECT COUNT(*) AS cnt + FROM edges e + JOIN nodes t ON e.target_id = t.id + WHERE e.kind = 'calls' AND t.name = ?`, + ) + .get(targetName) as { cnt: number }; + return row.cnt; + } finally { + db.close(); + } +} + +// `dynamic_kind` is only persisted on UNRESOLVED sink edges (confidence=0, +// the flag-only fallback for eval/computed-key/reflection/unresolved-dynamic +// call sites that never found a target) — by existing, consistent design +// across every dynamicKind category, a call site that DOES resolve (as +// value-ref calls into a real function/method target always do here) is +// persisted as a plain `dynamic=1` `calls` edge with `dynamic_kind = NULL`. +// So a resolved value-ref edge is identified by `dynamic = 1`, not by the +// `dynamic_kind` column (see resolveFallbackTargets / emitDirectCallEdgesForCall). +function readDynamicCallEdges(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT s.name AS src, t.name AS tgt + FROM edges e + JOIN nodes s ON e.source_id = s.id + JOIN nodes t ON e.target_id = t.id + WHERE e.kind = 'calls' AND e.dynamic = 1 + ORDER BY s.name, t.name`, + ) + .all() as Array<{ src: string; tgt: string }>; + } finally { + db.close(); + } +} + +describe('dispatch-table value-ref edges (#1771) — WASM', () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1771-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(tmpDir, rel), content); + } + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); + }); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('emits a value-ref calls edge from the dispatch table to every handler', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readDynamicCallEdges(dbPath); + for (const name of HANDLER_NAMES) { + expect( + edges.some((e) => e.src === 'HANDLERS' && e.tgt === name), + `Expected a value-ref edge HANDLERS -> ${name}; got: ${JSON.stringify(edges)}`, + ).toBe(true); + } + }); + + it('gives every handler at least one inbound calls edge (fan-in >= 1)', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + for (const name of HANDLER_NAMES) { + expect(countCallEdgesTo(dbPath, name), `${name} has no inbound calls edge`).toBeGreaterThan( + 0, + ); + } + }); + + it('does not classify any dispatch-table handler as dead, regardless of its own fanOut', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const name of HANDLER_NAMES) { + const node = nodes.find((n) => n.name === name && n.kind === 'function'); + expect(node, `${name} node not found`).toBeDefined(); + expect( + node!.role, + `${name} was classified as ${node!.role} — expected a non-dead role now that a real edge exists`, + ).not.toBe(undefined); + expect(DEAD_ROLES.has(node!.role ?? '')).toBe(false); + } + }); + + it('does not fabricate a value-ref edge to a plain data constant', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readDynamicCallEdges(dbPath); + expect(edges.some((e) => e.tgt === 'SOME_CONSTANT')).toBe(false); + expect(countCallEdgesTo(dbPath, 'SOME_CONSTANT')).toBe(0); + }); +}); + +// ── Native engine parity ──────────────────────────────────────────────────── +// Skipped when the native addon is not installed. + +describe.skipIf(!isNativeAvailable())('dispatch-table value-ref edges (#1771) — native', () => { + let nativeTmpDir: string; + + beforeAll(async () => { + nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1771-native-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(nativeTmpDir, rel), content); + } + await buildGraph(nativeTmpDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(nativeTmpDir, { recursive: true, force: true }); + }); + + it('emits a value-ref calls edge from the dispatch table to every handler', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = readDynamicCallEdges(dbPath); + for (const name of HANDLER_NAMES) { + expect( + edges.some((e) => e.src === 'HANDLERS' && e.tgt === name), + `Expected a native value-ref edge HANDLERS -> ${name}; got: ${JSON.stringify(edges)}`, + ).toBe(true); + } + }); + + it('does not classify any dispatch-table handler as dead', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const name of HANDLER_NAMES) { + 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); + } + }); + + it('does not fabricate a value-ref edge to a plain data constant', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = readDynamicCallEdges(dbPath); + expect(edges.some((e) => e.tgt === 'SOME_CONSTANT')).toBe(false); + expect(countCallEdgesTo(dbPath, 'SOME_CONSTANT')).toBe(0); + }); +}); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index c785eb02c..79174055f 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1108,6 +1108,75 @@ describe('JavaScript parser', () => { }); }); + describe('object-literal value-ref extraction (#1771)', () => { + it('extracts a value-ref call for a bare-identifier property value', () => { + const symbols = parseJS(`const table = { resolve: resolveWrapperParam };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ + name: 'resolveWrapperParam', + dynamic: true, + dynamicKind: 'value-ref', + }), + ); + }); + + it('extracts value-ref calls for every handler in a dispatch-table array', () => { + // Mirrors this repo's own PARAM_NODE_HANDLERS pattern (issue #1771): + // an array of `{ matches, resolve }` objects where `resolve` is a bare + // function identifier dispatched at runtime via `handler.resolve(...)`. + const symbols = parseJS(` + const HANDLERS = [ + { matches: isA, resolve: resolveA }, + { matches: isB, resolve: resolveB }, + { matches: isC, resolve: resolveC }, + ]; + `); + for (const name of ['isA', 'resolveA', 'isB', 'resolveB', 'isC', 'resolveC']) { + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name, dynamic: true, dynamicKind: 'value-ref' }), + ); + } + }); + + it('extracts a value-ref call for a shorthand property', () => { + const symbols = parseJS(`const table = { someFunction };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'someFunction', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + + it('does not extract a value-ref call for a call-expression value', () => { + const symbols = parseJS(`const table = { resolve: someFunction() };`); + expect(symbols.calls).not.toContainEqual( + expect.objectContaining({ name: 'someFunction', dynamicKind: 'value-ref' }), + ); + }); + + it('does not extract a value-ref call for a member-expression value', () => { + const symbols = parseJS(`const table = { resolve: obj.someFunction };`); + expect(symbols.calls).not.toContainEqual( + expect.objectContaining({ dynamicKind: 'value-ref', name: 'someFunction' }), + ); + }); + + it('does not extract a value-ref call for an inline function/arrow value', () => { + const symbols = parseJS(`const table = { resolve: () => {}, other: function () {} };`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('does not extract a value-ref call for literal or data-shaped values', () => { + const symbols = parseJS(` + const config = { name: 'literal', count: 42, active: true, empty: null, list: [1, 2] }; + `); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('excludes builtin globals from value-ref extraction', () => { + const symbols = parseJS(`const table = { log: console, Ctor: Object };`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + }); + describe('Phase 8.3f: object-destructuring rest parameter binding extraction', () => { function parseJS(code) { const parser = parsers.get('javascript');