Skip to content

Commit dd447bc

Browse files
committed
fix: credit instanceof ClassName checks as exports consumers
codegraph exports did not credit `instanceof ClassName` checks (or other bare-reference, no-call-site usages) as consumers. CodegraphError showed consumerCount:0 via `codegraph exports src/shared/errors.ts --json` despite two real production usages (src/cli.ts, src/mcp/server.ts) that only ever reference it via `err instanceof CodegraphError` — never `new CodegraphError(...)`. Subclasses consumed via constructor calls were correctly credited (ConfigError, consumerCount:23); only bare-reference usages like instanceof were invisible to the consumer counter, so any base/parent class whose primary cross-file use is instanceof narrowing falsely presented as dead/unused. Root cause: no edge at all was created for the right-hand operand of an `instanceof` binary expression — the same "no edge for a bare-identifier value reference" gap as #1771 (object-literal property values) and #1776 (Lua builtin reassignment), just at a different syntactic position. Fix: both engines now extract a dynamic `calls` edge (dynamicKind/ dynamic_kind = 'value-ref') when instanceof's right operand is a bare identifier, reusing the #1771/#1776 taxonomy entry rather than introducing a new DynamicKind — ADR-002 explicitly anticipated this as "syntax-position- agnostic" and invited a third extraction site to reuse it. Unlike the function/method-only #1771/#1776 sites, the resolver-side kind filter for value-ref now also accepts class-kind targets, since instanceof's operand is always a class/constructor (never a plain data reference) — it still accepts function-kind too, correctly covering the pre-ES6 "constructor function" instanceof idiom (`x instanceof SomeFunction`). Applied to both engines: - WASM/TS: src/extractors/javascript.ts (collectInstanceofValueRefCall, wired into the shared runCollectorWalk's binary_expression case), src/domain/graph/builder/stages/build-edges.ts (resolveFallbackTargets kind filter extended to accept 'class'), src/domain/graph/builder/ incremental.ts (mirrored filter for the incremental/watch-mode path). - Native: crates/codegraph-core/src/extractors/javascript.rs (handle_instanceof_value_ref, wired into match_js_node's binary_expression arm — dispatches for JavaScript/TypeScript/Tsx alike), crates/ codegraph-core/src/domain/graph/builder/stages/build_edges.rs (matching kind filter in process_file). - src/types.ts / docs/architecture/decisions/002-dynamic-call-resolution.md: documented instanceof as the third value-ref extraction site and the class-kind target-set extension. Verified against the exact repro on both engines: CodegraphError now shows consumerCount:2 (src/cli.ts, createCallToolHandler in src/mcp/server.ts), identical between native and WASM. codegraph roles --role dead does not (and did not) flag the CodegraphError class itself — exported classes are never classified dead by role, independent of this fix; the fix's measurable effect is the consumer-count/fan-in correction. Total dead-symbol count across the repo's own graph dropped from 816 to 810 as a side effect of crediting other instanceof-only-referenced symbols codebase-wide. Resolution-benchmark: no fixture exercises instanceof, so precision/recall is byte-identical before and after (javascript 100/100, typescript 95.7/93.6, aggregate 63.8% recall) — confirmed empirically, not assumed. Fixes #1784 docs check acknowledged — internal edge-emission/resolver bug fix, no new commands, languages, or architecture to document. Impact: 5 functions changed, 17 affected
1 parent af17b6d commit dd447bc

9 files changed

Lines changed: 535 additions & 38 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -785,16 +785,21 @@ fn process_file<'a>(
785785
let mut targets = resolve_call_targets(
786786
ctx, call, fc.rel_path, imported_from, &fc.type_map, caller_name, imported_original_name,
787787
);
788-
// #1771: object-literal property-value references resolve against
789-
// function/method-kind targets only — a bare identifier there is as
790-
// likely to be a plain data reference (`{ name: SOME_CONSTANT }`) as
791-
// a function reference, so drop any non-callable match rather than
792-
// fabricating a "calls" edge to a constant/class/etc. Applied once
793-
// here (after all resolve_call_targets tiers), mirroring the
788+
// #1771/#1784: value-ref references (object-literal property values,
789+
// Lua builtin reassignment, `instanceof ClassName`) resolve against
790+
// function/method/class-kind targets only. A bare identifier in one
791+
// of these positions is as likely to be a plain data reference
792+
// (`{ name: SOME_CONSTANT }`) as a real function/class, so drop any
793+
// other-kind match rather than fabricating a "calls" edge to a
794+
// constant. `class` is included alongside function/method because
795+
// `instanceof`'s right operand is always a class/constructor
796+
// (#1784) — unlike the original #1771 object-literal case, which is
797+
// function/method only. Applied once here (after all
798+
// resolve_call_targets tiers), mirroring the
794799
// `dynamicKind === 'value-ref'` filter in resolveFallbackTargets
795800
// (stages/build-edges.ts).
796801
if call.dynamic_kind.as_deref() == Some("value-ref") {
797-
targets.retain(|t| t.kind == "function" || t.kind == "method");
802+
targets.retain(|t| t.kind == "function" || t.kind == "method" || t.kind == "class");
798803
}
799804
sort_targets_by_confidence(&mut targets, fc.rel_path, imported_from);
800805
emit_call_edges(&targets, caller_id, is_dynamic, fc.rel_path, imported_from, &mut seen_edges, &mut pts_edge_map, edges);

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,8 @@ fn match_js_node(node: &Node, source: &[u8], symbols: &mut FileSymbols, _depth:
10811081
"shorthand_property_identifier" => {
10821082
handle_object_literal_shorthand_value_ref(node, source, &mut symbols.calls)
10831083
}
1084+
// #1784: `instanceof ClassName` checks, e.g. `err instanceof CodegraphError`.
1085+
"binary_expression" => handle_instanceof_value_ref(node, source, &mut symbols.calls),
10841086
_ => {}
10851087
}
10861088
}
@@ -2502,6 +2504,47 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls:
25022504
});
25032505
}
25042506

2507+
/// Collect a dynamic value-ref `Call` for the right-hand operand of an
2508+
/// `instanceof` binary expression when it's a bare identifier — e.g.
2509+
/// `err instanceof CodegraphError` (issue #1784). `instanceof` reads its
2510+
/// right operand as a value (a prototype-chain check), never calls it, so
2511+
/// this is the same "referenced as a value, not a call site" shape as the
2512+
/// object-literal (#1771) and Lua builtin-reassignment (#1776) sites —
2513+
/// reused rather than given its own `dynamic_kind` (see ADR-002).
2514+
///
2515+
/// Restricted to plain `identifier` right operands: `a instanceof B.C`
2516+
/// (`member_expression`) and `a instanceof (foo())` (parenthesized/call
2517+
/// expressions) are left unresolved rather than guessing — same
2518+
/// "restrict to the simplest syntactic shape" precedent as #1771.
2519+
///
2520+
/// Unlike the function/method-only value-ref sites, `instanceof`'s operand
2521+
/// is always a class/constructor — the resolver-side kind filter in
2522+
/// `build_edges.rs` accepts `class`-kind targets in addition to
2523+
/// function/method for this reason.
2524+
///
2525+
/// Mirrors `collectInstanceofValueRefCall` in `src/extractors/javascript.ts`.
2526+
fn handle_instanceof_value_ref(node: &Node, source: &[u8], calls: &mut Vec<Call>) {
2527+
let Some(operator_n) = node.child_by_field_name("operator") else { return };
2528+
if node_text(&operator_n, source) != "instanceof" {
2529+
return;
2530+
}
2531+
let Some(right_n) = node.child_by_field_name("right") else { return };
2532+
if right_n.kind() != "identifier" {
2533+
return;
2534+
}
2535+
let text = node_text(&right_n, source);
2536+
if JS_BUILTIN_GLOBALS.contains(&text) {
2537+
return;
2538+
}
2539+
calls.push(Call {
2540+
name: text.to_string(),
2541+
line: start_line(&right_n),
2542+
dynamic: Some(true),
2543+
dynamic_kind: Some("value-ref".to_string()),
2544+
..Default::default()
2545+
});
2546+
}
2547+
25052548
/// Extract definitions from destructured object bindings: `const { handleToken,
25062549
/// checkPermissions } = initAuth(...)` creates definitions for `handleToken`
25072550
/// and `checkPermissions`, kind `constant` — matching the convention for plain
@@ -4423,6 +4466,69 @@ mod tests {
44234466
assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref")));
44244467
}
44254468

4469+
// ── #1784: instanceof value-ref extraction ──────────────────────────────
4470+
4471+
#[test]
4472+
fn extracts_value_ref_call_for_instanceof_class_name() {
4473+
let s = parse_js(
4474+
"function handle(err) { if (err instanceof CodegraphError) { report(err); } }",
4475+
);
4476+
let value_refs: Vec<_> = s
4477+
.calls
4478+
.iter()
4479+
.filter(|c| c.dynamic_kind.as_deref() == Some("value-ref"))
4480+
.collect();
4481+
assert!(value_refs.iter().any(|c| c.name == "CodegraphError"));
4482+
assert!(value_refs.iter().all(|c| c.dynamic == Some(true)));
4483+
}
4484+
4485+
#[test]
4486+
fn extracts_value_ref_call_for_instanceof_as_expression_value() {
4487+
let s = parse_js("const isConfig = (err) => err instanceof ConfigError;");
4488+
let value_refs: Vec<_> = s
4489+
.calls
4490+
.iter()
4491+
.filter(|c| c.dynamic_kind.as_deref() == Some("value-ref"))
4492+
.collect();
4493+
assert!(value_refs.iter().any(|c| c.name == "ConfigError"));
4494+
}
4495+
4496+
#[test]
4497+
fn no_value_ref_call_for_instanceof_member_expression_operand() {
4498+
let s = parse_js("const check = (a) => a instanceof ns.SomeClass;");
4499+
assert!(
4500+
s.calls
4501+
.iter()
4502+
.all(|c| !(c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "SomeClass"))
4503+
);
4504+
}
4505+
4506+
#[test]
4507+
fn no_value_ref_call_for_instanceof_call_expression_operand() {
4508+
let s = parse_js("const check = (a) => a instanceof getClass();");
4509+
assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref")));
4510+
}
4511+
4512+
#[test]
4513+
fn no_value_ref_call_for_instanceof_builtin_globals() {
4514+
let s = parse_js(
4515+
"function isBuiltin(x) { return x instanceof Error || x instanceof Array || x instanceof Map; }",
4516+
);
4517+
assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref")));
4518+
}
4519+
4520+
#[test]
4521+
fn no_value_ref_call_for_in_operator() {
4522+
let s = parse_js("const has = (obj) => 'key' in obj;");
4523+
assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref")));
4524+
}
4525+
4526+
#[test]
4527+
fn no_value_ref_call_for_other_binary_operators() {
4528+
let s = parse_js("const sum = (a, b) => a + b === Total;");
4529+
assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref")));
4530+
}
4531+
44264532
#[test]
44274533
fn no_duplicate_call_for_call_expression_arg() {
44284534
let s = parse_js("router.use(checkPermissions(['admin']));");

docs/architecture/decisions/002-dynamic-call-resolution.md

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,31 +79,41 @@ export type DynamicKind =
7979
| 'unresolved-dynamic' // detected dynamic call we cannot resolve
8080
| 'value-ref'; // bare identifier used as a value reference, not a call site —
8181
// object-literal property value (dispatch tables, e.g.
82-
// `{ resolve: someFn }`, #1771) or assignment to a Lua
82+
// `{ resolve: someFn }`, #1771), assignment to a Lua
8383
// global/builtin identifier (e.g. `require = tracedRequire`,
84-
// #1776) — resolvable against function/method-kind targets
85-
// only
84+
// #1776), or the right operand of an `instanceof` check
85+
// (e.g. `err instanceof CodegraphError`, #1784) —
86+
// resolvable against function/method-kind targets, plus
87+
// class-kind for the instanceof site
8688
```
8789

8890
`dynamic?: boolean` is kept to avoid churning every `call.dynamic ? 1 : 0` site.
8991

9092
`value-ref` is Track A (resolvable) but deliberately **not** added to the flag-only
91-
sink-edge set: when the identifier doesn't resolve to a function/method (e.g. a
93+
sink-edge set: when the identifier doesn't resolve to a function/method/class (e.g. a
9294
plain data reference like `{ name: SOME_CONSTANT }`), that's the common case, not
9395
an undecidable dynamic call site — so it's silently dropped rather than flagged,
9496
unlike `eval`/`computed-key`/`unresolved-dynamic`.
9597

9698
`value-ref` is deliberately syntax-position-agnostic: any bare identifier that
97-
names a known function/method and appears somewhere other than a call site
98-
qualifies, regardless of which language or grammar shape produced it.
99-
Object-literal property values (#1771) and Lua assignment to a
100-
global/builtin identifier (#1776, `require = tracedRequire` — a builtin name
101-
isn't a locally-scoped variable that alias/points-to resolution could ever
102-
observe, so this is the narrow, language-specific case where a plain
103-
reference edge is the correct substitute for real alias tracking) are two
104-
independent extraction sites feeding the same resolution/filtering logic
105-
downstream; new languages/positions can add a third without touching
106-
`build-edges.ts` / `incremental.ts` / `build_edges.rs` again.
99+
names a known function/method/class and appears somewhere other than a call
100+
site qualifies, regardless of which language or grammar shape produced it.
101+
Object-literal property values (#1771), Lua assignment to a global/builtin
102+
identifier (#1776, `require = tracedRequire` — a builtin name isn't a
103+
locally-scoped variable that alias/points-to resolution could ever observe,
104+
so this is the narrow, language-specific case where a plain reference edge
105+
is the correct substitute for real alias tracking), and the right operand of
106+
an `instanceof` check (#1784, `err instanceof CodegraphError``instanceof`
107+
evaluates its right operand as a value, never calls it) are three independent
108+
extraction sites feeding the same resolution/filtering logic downstream. The
109+
`instanceof` site is the first to resolve against `class`-kind targets (the
110+
other two are function/method only, since `instanceof`'s operand is always a
111+
class/constructor) — the resolver-side filter is keyed on `dynamicKind`, not
112+
on which site produced the call, so this is a per-kind allowed-target-kind
113+
set rather than a per-site one. New languages/positions can add a fourth
114+
without touching `build-edges.ts` / `incremental.ts` / `build_edges.rs`
115+
again, beyond widening the allowed-target-kind set if the new site's operand
116+
isn't a function/method/class.
107117

108118
### Sink edges reuse `kind='calls'`, not a new edge kind
109119

src/domain/graph/builder/incremental.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -768,11 +768,14 @@ function buildCallEdges(
768768
initialTargets,
769769
);
770770

771-
// #1771: object-literal property-value references resolve against
772-
// function/method-kind targets only — mirrors the same filter in
773-
// resolveFallbackTargets (stages/build-edges.ts, full-build path).
771+
// #1771/#1784: value-ref references resolve against function/method/
772+
// class-kind targets only (class included for `instanceof ClassName`,
773+
// #1784) — mirrors the same filter in resolveFallbackTargets
774+
// (stages/build-edges.ts, full-build path).
774775
if (call.dynamicKind === 'value-ref') {
775-
targets = targets.filter((t) => t.kind === 'function' || t.kind === 'method');
776+
targets = targets.filter(
777+
(t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class',
778+
);
776779
}
777780

778781
edgesAdded += emitIncrementalCallEdges(

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,19 +1380,24 @@ function resolveFallbackTargets(
13801380
if (qualified.length > 0) targets = qualified;
13811381
}
13821382

1383-
// #1771: object-literal property-value references (`{ resolve: someFn }`)
1384-
// resolve against function/method-kind targets only — a bare identifier
1385-
// there is as likely to be a plain data reference (`{ name: SOME_CONSTANT }`)
1386-
// as a function, so drop any non-callable match rather than fabricating a
1387-
// "calls" edge to a constant/class/etc. Applied once here, after every
1388-
// fallback tier above, so it covers whichever tier produced the match.
1383+
// #1771/#1784: value-ref references (object-literal property values,
1384+
// Lua builtin reassignment, `instanceof ClassName`) resolve against
1385+
// function/method/class-kind targets only. A bare identifier in one of
1386+
// these positions is as likely to be a plain data reference
1387+
// (`{ name: SOME_CONSTANT }`) as a real function/class, so drop any
1388+
// other-kind match rather than fabricating a "calls" edge to a constant.
1389+
// `class` is included alongside function/method because `instanceof`'s
1390+
// right operand is always a class/constructor (#1784) — unlike the
1391+
// original #1771 object-literal case, which is function/method only.
1392+
// Applied once here, after every fallback tier above, so it covers
1393+
// whichever tier produced the match.
13891394
if (call.dynamicKind === 'value-ref') {
13901395
// `targets` is typed without `kind` when it flows straight through from
13911396
// resolveCallTargets (call-resolver.ts's declared return type omits it),
13921397
// but every underlying CallNodeLookup method actually populates it — the
13931398
// same gap the preQualifiedTargets cast above already works around.
13941399
targets = (targets as ReadonlyArray<{ id: number; file: string; kind?: string }>).filter(
1395-
(t) => t.kind === 'function' || t.kind === 'method',
1400+
(t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class',
13961401
);
13971402
}
13981403

src/extractors/javascript.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3185,6 +3185,37 @@ function collectObjectLiteralValueRefCall(pairNode: TreeSitterNode, calls: Call[
31853185
});
31863186
}
31873187

3188+
/**
3189+
* Collect a dynamic value-ref `Call` for the right-hand operand of an
3190+
* `instanceof` binary expression when it's a bare identifier — e.g.
3191+
* `err instanceof CodegraphError` (issue #1784). `instanceof` reads its
3192+
* right operand as a value (a prototype-chain check), never calls it, so
3193+
* this is the same "referenced as a value, not a call site" shape as the
3194+
* object-literal (#1771) and Lua builtin-reassignment (#1776) sites — reused
3195+
* rather than given its own DynamicKind (see ADR-002).
3196+
*
3197+
* Restricted to plain `identifier` right operands: `a instanceof B.C`
3198+
* (`member_expression`) and `a instanceof (foo())` (parenthesized/call
3199+
* expressions) are left unresolved rather than guessing — same
3200+
* "restrict to the simplest syntactic shape" precedent as #1771.
3201+
*
3202+
* Unlike the function/method-only value-ref sites, `instanceof`'s operand is
3203+
* always a class/constructor — the resolver-side kind filter
3204+
* (`resolveFallbackTargets` / `build_edges.rs`) accepts `class`-kind targets
3205+
* in addition to function/method for this reason.
3206+
*/
3207+
function collectInstanceofValueRefCall(binaryNode: TreeSitterNode, calls: Call[]): void {
3208+
if (binaryNode.childForFieldName('operator')?.text !== 'instanceof') return;
3209+
const rightNode = binaryNode.childForFieldName('right');
3210+
if (rightNode?.type !== 'identifier' || BUILTIN_GLOBALS.has(rightNode.text)) return;
3211+
calls.push({
3212+
name: rightNode.text,
3213+
line: nodeStartLine(rightNode),
3214+
dynamic: true,
3215+
dynamicKind: 'value-ref',
3216+
});
3217+
}
3218+
31883219
function extractReceiverName(objNode: TreeSitterNode | null): string | undefined {
31893220
if (!objNode) return undefined;
31903221
const t = objNode.type;
@@ -3740,11 +3771,11 @@ function collectThisCallAndBindings(
37403771
* `valueRefCalls` is REQUIRED (unlike `calls`) — both paths route
37413772
* object-literal value-ref extraction through this single field, since
37423773
* neither `walkJavaScriptNode` (walk path) nor the compiled query patterns
3743-
* (query path) visit `pair`/`shorthand_property_identifier` nodes on their
3744-
* own (#1771). Both callers pass their own `calls` array here; it's a
3745-
* separate field from the optional `calls` above purely so this collector
3746-
* isn't accidentally gated off by the walk path's "don't double-collect
3747-
* call_expression" omission.
3774+
* (query path) visit `pair`/`shorthand_property_identifier`/`binary_expression`
3775+
* nodes on their own (#1771, #1784). Both callers pass their own `calls`
3776+
* array here; it's a separate field from the optional `calls` above purely
3777+
* so this collector isn't accidentally gated off by the walk path's "don't
3778+
* double-collect call_expression" omission.
37483779
*/
37493780
interface CollectorWalkTargets {
37503781
definitions: Definition[];
@@ -3849,6 +3880,10 @@ function runCollectorWalk(rootNode: TreeSitterNode, targets: CollectorWalkTarget
38493880
});
38503881
}
38513882
break;
3883+
case 'binary_expression':
3884+
// #1784: `instanceof ClassName` checks, e.g. `err instanceof CodegraphError`.
3885+
collectInstanceofValueRefCall(node, targets.valueRefCalls);
3886+
break;
38523887
}
38533888
for (let i = 0; i < node.childCount; i++) {
38543889
walk(node.child(i)!, depth + 1, childInDynamicImport);

src/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ export type DynamicKind =
488488
| 'reflection' // .call/.apply/.bind / Reflect.* / callable-ref — resolved when target is in codebase; sink edge emitted if unresolved
489489
| 'eval' // eval() / new Function() — undecidable; always flagged
490490
| 'unresolved-dynamic' // any other detected dynamic pattern; flagged
491-
| 'value-ref'; // bare identifier used as a value reference rather than a call site — object-literal property value (dispatch-table pattern, e.g. `{ resolve: someFn }`, #1771) or assignment to a Lua global/builtin identifier (e.g. `require = tracedRequire`, #1776) — resolved only against function/method-kind targets; unresolved (e.g. plain data references) are dropped silently, NOT flagged
491+
| 'value-ref'; // bare identifier used as a value reference rather than a call site — object-literal property value (dispatch-table pattern, e.g. `{ resolve: someFn }`, #1771), assignment to a Lua global/builtin identifier (e.g. `require = tracedRequire`, #1776), or the right operand of an `instanceof` check (e.g. `err instanceof CodegraphError`, #1784) — resolved only against function/method/class-kind targets (class only relevant for the instanceof site); unresolved (e.g. plain data references) are dropped silently, NOT flagged
492492

493493
/** A function/method call detected by an extractor. */
494494
export interface Call {

0 commit comments

Comments
 (0)