Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
139 changes: 139 additions & 0 deletions crates/codegraph-core/src/extractors/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
_ => {}
}
}
Expand Down Expand Up @@ -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<Call>) {
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<Call>) {
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],
Expand Down Expand Up @@ -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']));");
Expand Down
11 changes: 11 additions & 0 deletions crates/codegraph-core/src/graph/classifiers/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down
11 changes: 10 additions & 1 deletion docs/architecture/decisions/002-dynamic-call-resolution.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=<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.
Expand Down
9 changes: 8 additions & 1 deletion src/domain/graph/builder/incremental.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ function buildCallEdges(
importedOriginalNames,
);

const targets = applyCallFallbacks(
let targets = applyCallFallbacks(
call,
caller.callerName,
relPath,
Expand All @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions src/domain/graph/builder/stages/build-edges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
Comment on lines +1377 to +1384

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unsafe kind field access on undeclared type

The cast targets as ReadonlyArray<{ id: number; file: string; kind?: string }> works around call-resolver.ts's return type omitting kind. If any resolution tier produces targets where kind is genuinely absent at runtime (not merely absent from the declared type), the filter silently drops those targets and no edge is emitted — with zero diagnostic signal. The PR comment and the preQualifiedTargets cast above confirm this is a known type gap in the codebase, but combining an unsafe cast with a functional gate in a single expression makes future regressions hard to detect.

Fix in Claude Code

}

return { targets, importedFrom };
}

Expand Down
54 changes: 54 additions & 0 deletions src/extractors/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ function extractSymbolsQuery(tree: TreeSitterTree, query: TreeSitterQuery): Extr
objectPropBindings,
newExpressions,
definePropertyReceivers,
valueRefCalls: calls,
imports,
calls,
thisCallBindings,
Expand Down Expand Up @@ -884,6 +885,7 @@ function extractSymbolsWalk(tree: TreeSitterTree): ExtractorOutput {
objectPropBindings: ctx.objectPropBindings!,
newExpressions,
definePropertyReceivers,
valueRefCalls: ctx.calls,
funcPropDefs: ctx.definitions,
});
ctx.newExpressions = newExpressions;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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[];
Expand All @@ -3696,6 +3733,7 @@ interface CollectorWalkTargets {
objectPropBindings: ObjectPropBinding[];
newExpressions: string[];
definePropertyReceivers: Map<string, string>;
valueRefCalls: Call[];
imports?: Import[];
calls?: Call[];
thisCallBindings?: ThisCallBinding[];
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions src/graph/classifiers/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading