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 8eb1d7f4e..71c80163c 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,16 +785,22 @@ 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). + // #1771/#1784: value-ref references (object-literal property values, + // Lua builtin reassignment, `instanceof ClassName`) resolve against + // function/method/class-kind targets only. A bare identifier in one + // of these positions is as likely to be a plain data reference + // (`{ name: SOME_CONSTANT }`) as a real function/class, so drop any + // other-kind match rather than fabricating a "calls" edge to a + // constant. `class` was added because `instanceof`'s right operand + // is always a class/constructor (#1784). The filter is keyed on + // `dynamic_kind`, not on which site produced the call, so the #1771 + // object-literal and #1776 Lua sites also gain class-kind + // resolution as a side effect — not because either idiom commonly + // names a class. 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"); + targets.retain(|t| t.kind == "function" || t.kind == "method" || t.kind == "class"); } 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 6c2d1f973..2c8a72da1 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1081,6 +1081,8 @@ fn match_js_node(node: &Node, source: &[u8], symbols: &mut FileSymbols, _depth: "shorthand_property_identifier" => { handle_object_literal_shorthand_value_ref(node, source, &mut symbols.calls) } + // #1784: `instanceof ClassName` checks, e.g. `err instanceof CodegraphError`. + "binary_expression" => handle_instanceof_value_ref(node, source, &mut symbols.calls), _ => {} } } @@ -2507,6 +2509,47 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls: }); } +/// Collect a dynamic value-ref `Call` for the right-hand operand of an +/// `instanceof` binary expression when it's a bare identifier — e.g. +/// `err instanceof CodegraphError` (issue #1784). `instanceof` reads its +/// right operand as a value (a prototype-chain check), never calls it, so +/// this is the same "referenced as a value, not a call site" shape as the +/// object-literal (#1771) and Lua builtin-reassignment (#1776) sites — +/// reused rather than given its own `dynamic_kind` (see ADR-002). +/// +/// Restricted to plain `identifier` right operands: `a instanceof B.C` +/// (`member_expression`) and `a instanceof (foo())` (parenthesized/call +/// expressions) are left unresolved rather than guessing — same +/// "restrict to the simplest syntactic shape" precedent as #1771. +/// +/// Unlike the function/method-only value-ref sites, `instanceof`'s operand +/// is always a class/constructor — the resolver-side kind filter in +/// `build_edges.rs` accepts `class`-kind targets in addition to +/// function/method for this reason. +/// +/// Mirrors `collectInstanceofValueRefCall` in `src/extractors/javascript.ts`. +fn handle_instanceof_value_ref(node: &Node, source: &[u8], calls: &mut Vec) { + let Some(operator_n) = node.child_by_field_name("operator") else { return }; + if node_text(&operator_n, source) != "instanceof" { + return; + } + let Some(right_n) = node.child_by_field_name("right") else { return }; + if right_n.kind() != "identifier" { + return; + } + let text = node_text(&right_n, source); + if JS_BUILTIN_GLOBALS.contains(&text) { + return; + } + calls.push(Call { + name: text.to_string(), + line: start_line(&right_n), + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); +} + /// Extract definitions from destructured object bindings: `const { handleToken, /// checkPermissions } = initAuth(...)` creates definitions for `handleToken` /// and `checkPermissions`, kind `constant` — matching the convention for plain @@ -4445,6 +4488,69 @@ mod tests { assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); } + // ── #1784: instanceof value-ref extraction ────────────────────────────── + + #[test] + fn extracts_value_ref_call_for_instanceof_class_name() { + let s = parse_js( + "function handle(err) { if (err instanceof CodegraphError) { report(err); } }", + ); + 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 == "CodegraphError")); + assert!(value_refs.iter().all(|c| c.dynamic == Some(true))); + } + + #[test] + fn extracts_value_ref_call_for_instanceof_as_expression_value() { + let s = parse_js("const isConfig = (err) => err instanceof ConfigError;"); + 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 == "ConfigError")); + } + + #[test] + fn no_value_ref_call_for_instanceof_member_expression_operand() { + let s = parse_js("const check = (a) => a instanceof ns.SomeClass;"); + assert!( + s.calls + .iter() + .all(|c| !(c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "SomeClass")) + ); + } + + #[test] + fn no_value_ref_call_for_instanceof_call_expression_operand() { + let s = parse_js("const check = (a) => a instanceof getClass();"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_instanceof_builtin_globals() { + let s = parse_js( + "function isBuiltin(x) { return x instanceof Error || x instanceof Array || x instanceof Map; }", + ); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_in_operator() { + let s = parse_js("const has = (obj) => 'key' in obj;"); + assert!(s.calls.iter().all(|c| c.dynamic_kind.as_deref() != Some("value-ref"))); + } + + #[test] + fn no_value_ref_call_for_other_binary_operators() { + let s = parse_js("const sum = (a, b) => a + b === Total;"); + 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/docs/architecture/decisions/002-dynamic-call-resolution.md b/docs/architecture/decisions/002-dynamic-call-resolution.md index c017742e5..e69641c73 100644 --- a/docs/architecture/decisions/002-dynamic-call-resolution.md +++ b/docs/architecture/decisions/002-dynamic-call-resolution.md @@ -79,31 +79,45 @@ export type DynamicKind = | 'unresolved-dynamic' // detected dynamic call we cannot resolve | 'value-ref'; // bare identifier used as a value reference, not a call site — // object-literal property value (dispatch tables, e.g. - // `{ resolve: someFn }`, #1771) or assignment to a Lua + // `{ resolve: someFn }`, #1771), assignment to a Lua // global/builtin identifier (e.g. `require = tracedRequire`, - // #1776) — resolvable against function/method-kind targets - // only + // #1776), or the right operand of an `instanceof` check + // (e.g. `err instanceof CodegraphError`, #1784) — + // resolvable against function/method/class-kind targets; + // class was added for instanceof, but the filter is + // per-kind rather than per-site, so all three sites + // share the same allow-list ``` `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 +sink-edge set: when the identifier doesn't resolve to a function/method/class (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`. `value-ref` is deliberately syntax-position-agnostic: any bare identifier that -names a known function/method and appears somewhere other than a call site -qualifies, regardless of which language or grammar shape produced it. -Object-literal property values (#1771) and Lua assignment to a -global/builtin identifier (#1776, `require = tracedRequire` — a builtin name -isn't a locally-scoped variable that alias/points-to resolution could ever -observe, so this is the narrow, language-specific case where a plain -reference edge is the correct substitute for real alias tracking) are two -independent extraction sites feeding the same resolution/filtering logic -downstream; new languages/positions can add a third without touching -`build-edges.ts` / `incremental.ts` / `build_edges.rs` again. +names a known function/method/class and appears somewhere other than a call +site qualifies, regardless of which language or grammar shape produced it. +Object-literal property values (#1771), Lua assignment to a global/builtin +identifier (#1776, `require = tracedRequire` — a builtin name isn't a +locally-scoped variable that alias/points-to resolution could ever observe, +so this is the narrow, language-specific case where a plain reference edge +is the correct substitute for real alias tracking), and the right operand of +an `instanceof` check (#1784, `err instanceof CodegraphError` — `instanceof` +evaluates its right operand as a value, never calls it) are three independent +extraction sites feeding the same resolution/filtering logic downstream. The +`instanceof` site is the motivation for adding `class` to the allowed +target kinds, since its operand is always a class/constructor — but the +resolver-side filter is keyed on `dynamicKind`, not on which site produced +the call, so this is a per-kind allowed-target-kind set rather than a +per-site one: object-literal and Lua value-ref sites also gain class-kind +resolution as a side effect of this change, not because either idiom +commonly names a class. New languages/positions can add a fourth +without touching `build-edges.ts` / `incremental.ts` / `build_edges.rs` +again, beyond widening the allowed-target-kind set if the new site's operand +isn't a function/method/class. ### Sink edges reuse `kind='calls'`, not a new edge kind diff --git a/src/domain/graph/builder/incremental.ts b/src/domain/graph/builder/incremental.ts index 396885322..4ddd0a05c 100644 --- a/src/domain/graph/builder/incremental.ts +++ b/src/domain/graph/builder/incremental.ts @@ -759,11 +759,14 @@ 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). + // #1771/#1784: value-ref references resolve against function/method/ + // class-kind targets only (class included for `instanceof ClassName`, + // #1784) — 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'); + targets = targets.filter( + (t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class', + ); } edgesAdded += emitIncrementalCallEdges( diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 9c5f5f7b8..7ac1f6817 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -1372,11 +1372,17 @@ 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 + // #1771/#1784: value-ref references (object-literal property values, + // Lua builtin reassignment, `instanceof ClassName`) resolve against + // function/method/class-kind targets only. A bare identifier in one of + // these positions is as likely to be a plain data reference + // (`{ name: SOME_CONSTANT }`) as a real function/class, so drop any + // other-kind match rather than fabricating a "calls" edge to a constant. + // `class` was added because `instanceof`'s right operand is always a + // class/constructor (#1784). The filter is keyed on `dynamicKind`, not on + // which site produced the call, so the #1771 object-literal and #1776 Lua + // sites also gain class-kind resolution as a side effect — not because + // either idiom commonly names a class. 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 @@ -1386,7 +1392,9 @@ function resolveFallbackTargets( // as its own step (not folded into the filter callback) so the type-gap // workaround and the actual filtering decision stay visually distinct. const typedTargets = targets as ReadonlyArray<{ id: number; file: string; kind?: string }>; - targets = typedTargets.filter((t) => t.kind === 'function' || t.kind === 'method'); + targets = typedTargets.filter( + (t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class', + ); } return { targets, importedFrom }; diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 9f5259aee..b5236c3bf 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -3185,6 +3185,37 @@ function collectObjectLiteralValueRefCall(pairNode: TreeSitterNode, calls: Call[ }); } +/** + * Collect a dynamic value-ref `Call` for the right-hand operand of an + * `instanceof` binary expression when it's a bare identifier — e.g. + * `err instanceof CodegraphError` (issue #1784). `instanceof` reads its + * right operand as a value (a prototype-chain check), never calls it, so + * this is the same "referenced as a value, not a call site" shape as the + * object-literal (#1771) and Lua builtin-reassignment (#1776) sites — reused + * rather than given its own DynamicKind (see ADR-002). + * + * Restricted to plain `identifier` right operands: `a instanceof B.C` + * (`member_expression`) and `a instanceof (foo())` (parenthesized/call + * expressions) are left unresolved rather than guessing — same + * "restrict to the simplest syntactic shape" precedent as #1771. + * + * Unlike the function/method-only value-ref sites, `instanceof`'s operand is + * always a class/constructor — the resolver-side kind filter + * (`resolveFallbackTargets` / `build_edges.rs`) accepts `class`-kind targets + * in addition to function/method for this reason. + */ +function collectInstanceofValueRefCall(binaryNode: TreeSitterNode, calls: Call[]): void { + if (binaryNode.childForFieldName('operator')?.text !== 'instanceof') return; + const rightNode = binaryNode.childForFieldName('right'); + if (rightNode?.type !== 'identifier' || BUILTIN_GLOBALS.has(rightNode.text)) return; + calls.push({ + name: rightNode.text, + line: nodeStartLine(rightNode), + dynamic: true, + dynamicKind: 'value-ref', + }); +} + function extractReceiverName(objNode: TreeSitterNode | null): string | undefined { if (!objNode) return undefined; const t = objNode.type; @@ -3745,11 +3776,11 @@ function collectThisCallAndBindings( * `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. + * (query path) visit `pair`/`shorthand_property_identifier`/`binary_expression` + * nodes on their own (#1771, #1784). 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[]; @@ -3854,6 +3885,10 @@ function runCollectorWalk(rootNode: TreeSitterNode, targets: CollectorWalkTarget }); } break; + case 'binary_expression': + // #1784: `instanceof ClassName` checks, e.g. `err instanceof CodegraphError`. + collectInstanceofValueRefCall(node, targets.valueRefCalls); + break; } for (let i = 0; i < node.childCount; i++) { walk(node.child(i)!, depth + 1, childInDynamicImport); diff --git a/src/types.ts b/src/types.ts index 7b90058ee..a3ae69639 100644 --- a/src/types.ts +++ b/src/types.ts @@ -488,7 +488,7 @@ export type DynamicKind = | '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 - | '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 + | '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 against function/method/class-kind targets; class was added for instanceof, but the filter is per-kind rather than per-site, so all three sites share the same allow-list; 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-1784-instanceof-consumer-credit.test.ts b/tests/integration/issue-1784-instanceof-consumer-credit.test.ts new file mode 100644 index 000000000..63148b1b5 --- /dev/null +++ b/tests/integration/issue-1784-instanceof-consumer-credit.test.ts @@ -0,0 +1,278 @@ +/** + * Integration test for #1784: `codegraph exports` did not credit `instanceof + * ClassName` checks (or other bare-reference, no-call-site usages) as + * consumers — a base/parent class whose primary cross-file use is + * `instanceof` narrowing falsely presented as dead/unused. + * + * Repro (this repo's own source): `src/shared/errors.ts`'s `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(...)`. + * + * 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 emit a dynamic `calls` edge (dynamicKind/dynamic_kind + * = 'value-ref', reusing the #1771/#1776 taxonomy entry per ADR-002) from the + * enclosing scope to the referenced symbol when `instanceof`'s right operand + * is a bare identifier. Unlike the function/method-only #1771/#1776 sites, + * the resolver-side kind filter for `instanceof` additionally accepts + * `class`-kind targets (and still accepts `function`-kind, covering the + * pre-ES6 "constructor function" `instanceof` idiom) since `instanceof`'s + * operand is never a plain data reference in valid JS. + */ + +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 { exportsData } from '../../src/domain/queries.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const DEAD_ROLES = new Set(['dead-unresolved', 'dead-leaf', 'dead-entry', 'dead-ffi']); + +const TARGET_FILE = 'shared/errors.ts'; + +// `BaseError` mirrors this repo's own `CodegraphError`: a class consumed +// ONLY via `instanceof`, never `new`'d, in another file. `LegacyCtor` +// exercises the pre-ES6 "constructor function" `instanceof` idiom +// (`x instanceof SomeFunction` is valid JS, checking the prototype chain) — +// the resolver-side kind filter must keep accepting function-kind targets, +// not just add class. `ERROR_LABEL` is a plain data constant used as +// `instanceof`'s right operand (invalid at runtime, but syntactically legal) +// to prove the resolver does not fabricate an edge to a non-callable target, +// mirroring #1771's "does not fabricate a value-ref edge to a plain data +// constant" guard. +const FIXTURE = { + [TARGET_FILE]: ` +export class BaseError extends Error { + code: string; + constructor(message: string, code: string) { + super(message); + this.code = code; + } +} + +export function LegacyCtor(this: { legacy: boolean }) { + this.legacy = true; +} + +export const ERROR_LABEL = 'base-error'; +`, + 'cli/handler.ts': ` +import { BaseError, ERROR_LABEL, LegacyCtor } from '../shared/errors.js'; + +export function handle(err: unknown): string { + if (err instanceof BaseError) { + return err.code; + } + return String(err); +} + +export function isLegacy(x: unknown): boolean { + return x instanceof LegacyCtor; +} + +export function describe(err: unknown): string { + return err instanceof ERROR_LABEL ? 'label' : 'other'; +} +`, +}; + +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 getCallEdges(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n2.name AS tgt, n2.kind AS tgt_kind + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name`, + ) + .all() as Array<{ src: string; tgt: string; tgt_kind: string }>; + } 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(); + } +} + +function writeFixture(rootDir: string) { + for (const [rel, content] of Object.entries(FIXTURE)) { + const abs = path.join(rootDir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } +} + +describe('instanceof ClassName consumer crediting (#1784) — WASM', () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1784-wasm-')); + writeFixture(tmpDir); + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); + }); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('creates a calls edge from the instanceof check to the class', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect( + edges.some((e) => e.src === 'handle' && e.tgt === 'BaseError' && e.tgt_kind === 'class'), + `Expected handle -> BaseError (class); got: ${JSON.stringify(edges)}`, + ).toBe(true); + }); + + it('creates a calls edge from instanceof against a plain constructor function', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect( + edges.some( + (e) => e.src === 'isLegacy' && e.tgt === 'LegacyCtor' && e.tgt_kind === 'function', + ), + `Expected isLegacy -> LegacyCtor (function); got: ${JSON.stringify(edges)}`, + ).toBe(true); + }); + + it('does not fabricate an edge from instanceof against a plain data constant', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect(edges.some((e) => e.tgt === 'ERROR_LABEL')).toBe(false); + expect(countCallEdgesTo(dbPath, 'ERROR_LABEL')).toBe(0); + }); + + it('codegraph exports credits BaseError with a consumer from handle', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const data = exportsData(TARGET_FILE, dbPath); + + const baseError = data.results.find((r: { name: string }) => r.name === 'BaseError'); + expect(baseError).toBeDefined(); + expect(baseError.consumerCount).toBeGreaterThanOrEqual(1); + expect(baseError.consumers.map((c: { name: string }) => c.name)).toContain('handle'); + }); + + it('does not classify BaseError or LegacyCtor as dead', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const [name, kind] of [ + ['BaseError', 'class'], + ['LegacyCtor', 'function'], + ] as const) { + const node = nodes.find((n) => n.name === name && n.kind === kind); + expect(node, `${name} node not found`).toBeDefined(); + expect( + DEAD_ROLES.has(node!.role ?? ''), + `${name} was classified as ${node!.role} — expected a non-dead role now that a real edge exists`, + ).toBe(false); + } + }); +}); + +// ── Native engine parity ──────────────────────────────────────────────────── +// Skipped when the native addon is not installed. + +describe.skipIf(!isNativeAvailable())( + 'instanceof ClassName consumer crediting (#1784) — native', + () => { + let nativeTmpDir: string; + + beforeAll(async () => { + nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1784-native-')); + writeFixture(nativeTmpDir); + await buildGraph(nativeTmpDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(nativeTmpDir, { recursive: true, force: true }); + }); + + it('creates a calls edge from the instanceof check to the class', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect( + edges.some((e) => e.src === 'handle' && e.tgt === 'BaseError' && e.tgt_kind === 'class'), + `Expected native handle -> BaseError (class); got: ${JSON.stringify(edges)}`, + ).toBe(true); + }); + + it('creates a calls edge from instanceof against a plain constructor function', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect( + edges.some( + (e) => e.src === 'isLegacy' && e.tgt === 'LegacyCtor' && e.tgt_kind === 'function', + ), + `Expected native isLegacy -> LegacyCtor (function); got: ${JSON.stringify(edges)}`, + ).toBe(true); + }); + + it('does not fabricate an edge from instanceof against a plain data constant', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const edges = getCallEdges(dbPath); + expect(edges.some((e) => e.tgt === 'ERROR_LABEL')).toBe(false); + expect(countCallEdgesTo(dbPath, 'ERROR_LABEL')).toBe(0); + }); + + it('codegraph exports credits BaseError with a consumer from handle', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const data = exportsData(TARGET_FILE, dbPath); + + const baseError = data.results.find((r: { name: string }) => r.name === 'BaseError'); + expect(baseError).toBeDefined(); + expect(baseError.consumerCount).toBeGreaterThanOrEqual(1); + expect(baseError.consumers.map((c: { name: string }) => c.name)).toContain('handle'); + }); + + it('does not classify BaseError or LegacyCtor as dead', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + for (const [name, kind] of [ + ['BaseError', 'class'], + ['LegacyCtor', 'function'], + ] as const) { + const node = nodes.find((n) => n.name === name && n.kind === kind); + expect(node, `${name} node not found (native)`).toBeDefined(); + expect(DEAD_ROLES.has(node!.role ?? '')).toBe(false); + } + }); + }, +); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index b104c12e9..d48aafed9 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1302,6 +1302,61 @@ describe('JavaScript parser', () => { }); }); + describe('instanceof value-ref extraction (#1784)', () => { + it('extracts a value-ref call for `instanceof ClassName`', () => { + const symbols = parseJS(` + function handle(err) { + if (err instanceof CodegraphError) { report(err); } + } + `); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ + name: 'CodegraphError', + dynamic: true, + dynamicKind: 'value-ref', + }), + ); + }); + + it('extracts a value-ref call for `instanceof` used as an expression value', () => { + const symbols = parseJS(`const isConfig = (err) => err instanceof ConfigError;`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'ConfigError', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + + it('does not extract a value-ref call for a member-expression right operand', () => { + const symbols = parseJS(`const check = (a) => a instanceof ns.SomeClass;`); + expect(symbols.calls).not.toContainEqual( + expect.objectContaining({ dynamicKind: 'value-ref', name: 'SomeClass' }), + ); + }); + + it('does not extract a value-ref call for a call-expression right operand', () => { + const symbols = parseJS(`const check = (a) => a instanceof getClass();`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('excludes builtin globals from instanceof value-ref extraction', () => { + const symbols = parseJS(` + function isBuiltin(x) { + return x instanceof Error || x instanceof Array || x instanceof Map; + } + `); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('does not extract a value-ref call for the unrelated `in` operator', () => { + const symbols = parseJS(`const has = (obj) => 'key' in obj;`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('does not extract a value-ref call for other binary operators', () => { + const symbols = parseJS(`const sum = (a, b) => a + b === Total;`); + 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');