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 18b812e7..ac76aeba 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 @@ -153,10 +153,18 @@ struct EdgeContext<'a> { nodes_by_file: HashMap<&'a str, Vec<&'a NodeInfo>>, builtin_set: HashSet<&'a str>, receiver_kinds: HashSet<&'a str>, + /// Property/method names ever invoked via member-call syntax + /// (`x.name(...)`) across every file in this build pass — see + /// `collect_invoked_property_names` for the #1895 liveness rationale. + invoked_property_names: HashSet<&'a str>, } impl<'a> EdgeContext<'a> { - fn new(all_nodes: &'a [NodeInfo], builtin_receivers: &'a [String]) -> Self { + fn new( + all_nodes: &'a [NodeInfo], + builtin_receivers: &'a [String], + files: &'a [FileEdgeInput], + ) -> Self { let mut nodes_by_name: HashMap<&str, Vec<&NodeInfo>> = HashMap::new(); let mut nodes_by_name_and_file: HashMap<(&str, &str), Vec<&NodeInfo>> = HashMap::new(); let mut nodes_by_file: HashMap<&str, Vec<&NodeInfo>> = HashMap::new(); @@ -177,8 +185,41 @@ impl<'a> EdgeContext<'a> { nodes_by_file, builtin_set, receiver_kinds, + invoked_property_names: collect_invoked_property_names(files), + } + } +} + +/// Collect the set of property/method names ever invoked via member-call +/// syntax (`x.name(...)`) across every file currently being processed — +/// regardless of whether the receiver `x` itself resolves to anything. +/// +/// Used as the "one hop further" liveness check for object-literal-property +/// value-refs (#1895): a function referenced as `{ resolve: someFn }` should +/// only be credited with a `calls` edge from that reference when something, +/// somewhere, actually invokes a `.resolve(...)`-shaped call — otherwise the +/// property is wired up but never read, and `someFn` is genuinely dead. +/// +/// Scope matches whatever `files` the caller passes to `build_call_edges`: +/// the full codebase for a full build, or just the changed file(s) on an +/// incremental one. The incremental case is a narrower, same-build-pass view +/// (a cross-file consumer added in an untouched file won't be seen until the +/// next full rebuild) — the same scoping trade-off already accepted +/// elsewhere in this codebase's incremental classification +/// (`has_active_file_siblings`, exported-via-reexport, and median fan-in/out +/// all recompute from the affected file set only, not the whole graph, in +/// `graph/classifiers/roles.rs`'s incremental path). Mirrors +/// `collectInvokedPropertyNames` in `src/domain/graph/builder/call-resolver.ts`. +fn collect_invoked_property_names(files: &[FileEdgeInput]) -> HashSet<&str> { + let mut names = HashSet::new(); + for file in files { + for call in &file.calls { + if call.receiver.is_some() { + names.insert(call.name.as_str()); + } } } + names } // ── Phase 8.3: points-to analysis ───────────────────────────────────────── @@ -467,7 +508,7 @@ pub fn build_call_edges( builtin_receivers: Vec, max_iterations: u32, ) -> Vec { - let ctx = EdgeContext::new(&all_nodes, &builtin_receivers); + let ctx = EdgeContext::new(&all_nodes, &builtin_receivers, &files); let mut edges = Vec::new(); for file_input in &files { @@ -799,6 +840,17 @@ fn process_file<'a>( // (stages/build-edges.ts). if call.dynamic_kind.as_deref() == Some("value-ref") { targets.retain(|t| t.kind == "function" || t.kind == "method" || t.kind == "class"); + // #1895: object-literal-property value-refs (key_expr set — see + // handle_object_literal_pair_value_ref / shorthand handler) + // additionally require independent evidence that the property is + // actually invoked somewhere (`x.key_expr(...)`) — merely being + // wired into an object literal is not liveness. instanceof/Lua + // value-refs never set key_expr, so they are unaffected. + if let Some(key_expr) = call.key_expr.as_deref() { + if !ctx.invoked_property_names.contains(key_expr) { + targets.clear(); + } + } } 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); @@ -2904,6 +2956,72 @@ mod call_edge_tests { assert_eq!(re.target_id, 2, "receiver edge target should be Calculator (id=2)"); } + /// Issue #1895: an object-literal-property value-ref call whose property + /// key (`key_expr`) is never independently confirmed to be invoked + /// anywhere (`x.resolve(...)`) must NOT produce a `calls` edge — merely + /// being wired into the object literal is not liveness. A sibling + /// property whose key IS invoked elsewhere (`table.reject(...)`) keeps + /// its edge. + #[test] + fn value_ref_edge_requires_key_invoked_elsewhere() { + let all_nodes = vec![ + node(1, "makeTable", "function", "factory.js", 1), + node(2, "neverRead", "function", "factory.js", 2), + node(3, "isRead", "function", "factory.js", 3), + node(4, "run", "function", "consumer.js", 1), + ]; + + let mut resolve_call = call("neverRead", 5, None); + resolve_call.dynamic = Some(true); + resolve_call.dynamic_kind = Some("value-ref".to_string()); + resolve_call.key_expr = Some("resolve".to_string()); + + let mut reject_call = call("isRead", 6, None); + reject_call.dynamic = Some(true); + reject_call.dynamic_kind = Some("value-ref".to_string()); + reject_call.key_expr = Some("reject".to_string()); + + let factory_file = make_file( + "factory.js", + 10, + vec![def("makeTable", "function", 1, 8)], + vec![resolve_call, reject_call], + vec![], + vec![], + ); + + // Evidence that `.reject(...)` is genuinely invoked somewhere, but + // `.resolve(...)` never is. + let consumer_file = make_file( + "consumer.js", + 20, + vec![def("run", "function", 1, 3)], + vec![call("reject", 2, Some("table"))], + vec![], + vec![], + ); + + let edges = build_call_edges( + vec![factory_file, consumer_file], + all_nodes, + vec![], + MAX_SOLVER_ITERATIONS, + ); + + let calls_never_read = edges.iter().any(|e| e.kind == "calls" && e.target_id == 2); + let calls_is_read = edges.iter().any(|e| e.kind == "calls" && e.target_id == 3); + assert!( + !calls_never_read, + "expected no calls edge to neverRead (key 'resolve' never invoked); got: {:?}", + edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::>() + ); + assert!( + calls_is_read, + "expected a calls edge to isRead (key 'reject' invoked in consumer.js); got: {:?}", + edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::>() + ); + } + /// Regression: when the same file has a `kind="function"` node for the /// effective receiver created by a destructured import (e.g. /// `const { Calculator } = require('./utils')`), that import artifact must diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 91849303..58839901 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -2874,6 +2874,12 @@ fn extract_callback_reference_calls( /// (`{ name: SOME_CONSTANT }`) naturally fail to resolve into an edge rather /// than needing a structural allowlist gate here. /// +/// `key_expr` carries the property KEY (e.g. `resolve`), distinct from `name` +/// (the referenced value's own identifier, e.g. `someFunction`) — the +/// downstream "is this property ever invoked" liveness check (#1895) needs +/// the key, since that's the name a dispatch consumer would actually call +/// (`table.resolve(...)`), not the function's own declared name. +/// /// 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 }; @@ -2884,11 +2890,13 @@ fn handle_object_literal_pair_value_ref(node: &Node, source: &[u8], calls: &mut if JS_BUILTIN_GLOBALS.contains(&text) { return; } + let key_expr = node.child_by_field_name("key").and_then(|k| resolve_pair_key_name(&k, source)); calls.push(Call { name: text.to_string(), line: start_line(&value_n), dynamic: Some(true), dynamic_kind: Some("value-ref".to_string()), + key_expr, ..Default::default() }); } @@ -2900,6 +2908,9 @@ fn handle_object_literal_pair_value_ref(node: &Node, source: &[u8], calls: &mut /// `shorthand_property_identifier_pattern` kind), so this can't misfire on /// destructuring targets. /// +/// `key_expr` equals `name` here — the property key and the referenced value +/// are the same identifier in shorthand form (#1895). +/// /// 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) { @@ -2912,6 +2923,7 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls: line: start_line(node), dynamic: Some(true), dynamic_kind: Some("value-ref".to_string()), + key_expr: Some(text.to_string()), ..Default::default() }); } @@ -5068,6 +5080,75 @@ mod tests { assert!(value_refs.iter().any(|c| c.name == "someFunction")); } + // #1895: key_expr capture — the property key, distinct from the + // referenced value's own name, is what a dispatch consumer would + // actually call (`table.resolve(...)`). + #[test] + fn value_ref_captures_property_key_distinct_from_referenced_name() { + let s = parse_js("const table = { resolve: someFunction };"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr.as_deref(), Some("resolve")); + } + + #[test] + fn value_ref_captures_string_literal_key_with_quotes_stripped() { + let s = parse_js("const table = { 'resolve': someFunction };"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr.as_deref(), Some("resolve")); + } + + #[test] + fn value_ref_captures_computed_string_literal_key() { + let s = parse_js("const table = { ['resolve']: someFunction };"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr.as_deref(), Some("resolve")); + } + + #[test] + fn value_ref_leaves_key_expr_unset_for_non_string_computed_key() { + let s = parse_js("const table = { [Symbol.iterator]: someFunction };"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr, None); + } + + #[test] + fn value_ref_key_expr_equals_name_for_shorthand_property() { + let s = parse_js("const table = { someFunction };"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr.as_deref(), Some("someFunction")); + } + + #[test] + fn instanceof_value_ref_leaves_key_expr_unset() { + let s = parse_js("if (err instanceof CodegraphError) {}"); + let call = s + .calls + .iter() + .find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "CodegraphError") + .expect("expected a value-ref call"); + assert_eq!(call.key_expr, None); + } + #[test] fn no_value_ref_call_for_call_expression_value() { let s = parse_js("const table = { resolve: someFunction() };"); diff --git a/src/domain/graph/builder/call-resolver.ts b/src/domain/graph/builder/call-resolver.ts index bfbd3e56..039d0d35 100644 --- a/src/domain/graph/builder/call-resolver.ts +++ b/src/domain/graph/builder/call-resolver.ts @@ -45,6 +45,41 @@ export const RECEIVER_KINDS = new Set(['class', 'struct', 'interface', 'type', ' // continue to work without changes (build-edges.ts, etc.). export { isModuleScopedLanguage }; +/** + * Collect the set of property/method names ever invoked via member-call + * syntax (`x.name(...)`) across every file currently being processed — + * regardless of whether the receiver `x` itself resolves to anything. + * + * Used as the "one hop further" liveness check for object-literal-property + * value-refs (#1895): a function referenced as `{ resolve: someFn }` should + * only be credited with a `calls` edge from that reference when something, + * somewhere, actually invokes a `.resolve(...)`-shaped call — otherwise the + * property is wired up but never read, and `someFn` is genuinely dead. + * + * Scope matches whatever set of files the caller passes in: the full + * codebase for a full build (build-edges.ts's `buildCallEdgesJS`, from + * `ctx.fileSymbols`), or just the single file being rebuilt on an incremental + * update (incremental.ts's `buildCallEdges`, from that file's own `calls`). + * The incremental case is a narrower, same-file view — a cross-file consumer + * added in a different, untouched file won't be seen until the next full + * rebuild — the same scoping trade-off already accepted elsewhere in this + * codebase's incremental classification (`hasActiveFileSiblings`, + * exported-via-reexport, and median fan-in/out all recompute from an affected + * subset, not the whole graph, in `graph/classifiers/roles.rs`'s incremental + * path). + */ +export function collectInvokedPropertyNames( + callsList: Iterable>, +): Set { + const names = new Set(); + for (const calls of callsList) { + for (const call of calls) { + if (call.receiver) names.add(call.name); + } + } + return names; +} + /** * Shared by both the full-build (build-edges.ts) and incremental (incremental.ts) * same-class fallback strategies: derive the enclosing class name from the diff --git a/src/domain/graph/builder/incremental.ts b/src/domain/graph/builder/incremental.ts index 7282eeea..504023e7 100644 --- a/src/domain/graph/builder/incremental.ts +++ b/src/domain/graph/builder/incremental.ts @@ -32,6 +32,7 @@ import { } from '../resolver/points-to.js'; import { type CallNodeLookup, + collectInvokedPropertyNames, findCaller, isModuleScopedLanguage, resolveCallTargets, @@ -1055,6 +1056,10 @@ function buildCallEdges( // JS path uses (buildPointsToMapForFile, shared via resolver/points-to.js). const ptsMap = buildPointsToMapForFile(symbols, importedNames); const fnRefBindingLhs = new Set(symbols.fnRefBindings?.map((b) => b.lhs) ?? []); + // #1895: scoped to this file's own calls only — see collectInvokedPropertyNames + // doc comment (call-resolver.ts) for why incremental rebuilds use a narrower, + // same-file view rather than a full-codebase one. + const invokedPropertyNames = collectInvokedPropertyNames([symbols.calls]); let edgesAdded = 0; for (const call of symbols.calls) { @@ -1089,6 +1094,12 @@ function buildCallEdges( targets = targets.filter( (t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class', ); + // #1895: object-literal-property value-refs additionally require + // independent evidence the property is actually invoked somewhere — + // mirrors the same check in resolveFallbackTargets (stages/build-edges.ts). + if (call.keyExpr && !invokedPropertyNames.has(call.keyExpr)) { + targets = []; + } } edgesAdded += emitIncrementalCallEdges( diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 62924736..08e4894e 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -40,6 +40,7 @@ import { unwrapTypeEntry } from '../../resolver/strategy.js'; import { enrichTypeMapWithTsc } from '../../resolver/ts-resolver.js'; import { type CallNodeLookup, + collectInvokedPropertyNames, findCaller, isModuleScopedLanguage, resolveCallTargets, @@ -951,6 +952,12 @@ function buildCallEdgesJS( ): void { const { fileSymbols, barrelOnlyFiles, rootDir } = ctx; const lookup = makeContextLookup(ctx, getNodeIdStmt); + // #1895: computed once from every file in this build pass (all files on a + // full build, just the affected files on an incremental one) — see + // collectInvokedPropertyNames doc comment for the scoping rationale. + const invokedPropertyNames = collectInvokedPropertyNames( + Array.from(fileSymbols.values(), (s) => s.calls), + ); for (const [relPath, symbols] of fileSymbols) { if (barrelOnlyFiles.has(relPath)) continue; @@ -1019,6 +1026,7 @@ function buildCallEdgesJS( lookup, allEdgeRows, typeMap, + invokedPropertyNames, ptsMap, chaCtx, importArtifactNames, @@ -1289,6 +1297,7 @@ function resolveFallbackTargets( lookup: CallNodeLookup, typeMap: Map, definePropertyReceivers: Map | undefined, + invokedPropertyNames: ReadonlySet, importedOriginalNames?: ReadonlyMap, ): { targets: ReadonlyArray<{ id: number; file: string; kind?: string }>; @@ -1368,6 +1377,15 @@ function resolveFallbackTargets( targets = (targets as ReadonlyArray<{ id: number; file: string; kind?: string }>).filter( (t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class', ); + // #1895: object-literal-property value-refs (keyExpr set — see + // collectObjectLiteralValueRefCall / shorthand collector) additionally + // require independent evidence that the property is actually invoked + // somewhere (`x.keyExpr(...)`) — merely being wired into an object + // literal is not liveness. instanceof/Lua value-refs never set keyExpr, + // so they are unaffected by this stricter check. + if (call.keyExpr && !invokedPropertyNames.has(call.keyExpr)) { + targets = []; + } } return { targets, importedFrom }; @@ -1719,6 +1737,7 @@ function buildFileCallEdges( lookup: CallNodeLookup, allEdgeRows: EdgeRowTuple[], typeMap: Map, + invokedPropertyNames: ReadonlySet, ptsMap?: PointsToMap | null, chaCtx?: ChaContext, importArtifactNames?: ReadonlyMap, @@ -1760,6 +1779,7 @@ function buildFileCallEdges( lookup, typeMap, symbols.definePropertyReceivers, + invokedPropertyNames, importedOriginalNames, ); diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 539e48f6..d76ae210 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -1499,6 +1499,20 @@ function handleConstObjectPatternAssignment( } } +/** + * Resolve an object-literal `pair`'s key node to its plain string name. + * Computed string-literal keys (e.g. `['foo']: fn`) are unwrapped the same way as + * method_definition's name field; non-string computed keys (e.g. `[Symbol.iterator]: fn`) + * resolve to '' (no resolvable name), mirroring the method_definition branch. + */ +function resolveObjectLiteralKeyName(keyNode: TreeSitterNode): string { + return keyNode.type === 'string' + ? keyNode.text.replace(/^['"]|['"]$/g, '') + : keyNode.type === 'computed_property_name' + ? resolveComputedKeyName(keyNode) + : keyNode.text; +} + /** * Phase 8.3f: extract function/arrow function properties from an object literal as standalone * definitions so that `this.method()` calls inside Object.defineProperty accessor functions can @@ -1531,15 +1545,7 @@ function extractObjectLiteralFunctions( const keyNode = child.childForFieldName('key'); const valueNode = child.childForFieldName('value'); if (!keyNode || !valueNode) continue; - // Computed string-literal keys (e.g. `['foo']: fn`) are unwrapped the same way as - // method_definition's name field; non-string computed keys (e.g. `[Symbol.iterator]: fn`) - // resolve to '' and are skipped below, mirroring the method_definition branch. - const keyName = - keyNode.type === 'string' - ? keyNode.text.replace(/^['"]|['"]$/g, '') - : keyNode.type === 'computed_property_name' - ? resolveComputedKeyName(keyNode) - : keyNode.text; + const keyName = resolveObjectLiteralKeyName(keyNode); if (!keyName) continue; if ( valueNode.type === 'arrow_function' || @@ -3413,15 +3419,24 @@ function collectObjectPropBindings(node: TreeSitterNode, bindings: ObjectPropBin * 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. + * + * `keyExpr` carries the property KEY (e.g. `resolve`), distinct from `name` + * (the referenced value's own identifier, e.g. `someFunction`) — the + * downstream "is this property ever invoked" liveness check (#1895) needs the + * key, since that's the name a dispatch consumer would actually call + * (`table.resolve(...)`), not the function's own declared name. */ function collectObjectLiteralValueRefCall(pairNode: TreeSitterNode, calls: Call[]): void { const valueNode = pairNode.childForFieldName('value'); if (valueNode?.type !== 'identifier' || BUILTIN_GLOBALS.has(valueNode.text)) return; + const keyNode = pairNode.childForFieldName('key'); + const keyExpr = keyNode ? resolveObjectLiteralKeyName(keyNode) || undefined : undefined; calls.push({ name: valueNode.text, line: nodeStartLine(valueNode), dynamic: true, dynamicKind: 'value-ref', + keyExpr, }); } @@ -4291,12 +4306,15 @@ function runCollectorWalk(rootNode: TreeSitterNode, targets: CollectorWalkTarget break; case 'shorthand_property_identifier': // #1771: shorthand form of the same pattern, e.g. `{ someFunction }`. + // keyExpr equals name here — the property key and the referenced + // value are the same identifier in shorthand form (#1895). if (!BUILTIN_GLOBALS.has(node.text)) { targets.valueRefCalls.push({ name: node.text, line: nodeStartLine(node), dynamic: true, dynamicKind: 'value-ref', + keyExpr: node.text, }); } break; diff --git a/tests/integration/issue-1895-value-ref-invocation-check.test.ts b/tests/integration/issue-1895-value-ref-invocation-check.test.ts new file mode 100644 index 00000000..9a307cf5 --- /dev/null +++ b/tests/integration/issue-1895-value-ref-invocation-check.test.ts @@ -0,0 +1,191 @@ +/** + * Integration test for #1895: `roles --role dead` treated a function + * referenced as an object-literal property VALUE as sufficient evidence of + * liveness on its own, without checking whether the property is ever + * actually accessed and invoked (`table.propName(...)`) anywhere. + * + * Root cause: the #1771 value-ref mechanism created a `calls` edge from the + * enclosing scope to the referenced function for every bare-identifier + * object-literal property value, purely because the reference existed — with + * no check that the property key is ever member-called. A dispatch-table + * property that is wired up but genuinely never read (`table.propName(...)` + * appears nowhere in the codebase) still got fan-in > 0 and was never + * flagged dead. + * + * Fix: value-ref calls originating from an object-literal property now carry + * the property's key name (`keyExpr`); the resolver only honors the + * reference as a `calls` edge when that key is independently confirmed to be + * invoked via member-call syntax (`x.keyExpr(...)`) somewhere in the files + * being processed. `matches`/`resolve`-style dispatch tables that ARE read + * through a real `handler.resolve(...)` call site (the #1771 fixture) keep + * their edges; a property that's wired up but never read does not. + */ + +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'; + +// `neverRead` is wired into the dispatch table under the `resolve` key but +// `.resolve(...)` never appears anywhere in the fixture — genuinely dead. +// `isRead` is wired under `reject` and IS invoked via `table.reject(...)` in +// `run()` — genuinely live. Both are referenced identically as object-literal +// property values, so only the new invocation check can tell them apart. +const FIXTURE = { + 'factory.js': ` +function neverRead(x) { return x + 1; } +function isRead(x) { return x + 2; } +function shorthandNeverRead(x) { return x + 3; } + +function makeTable() { + return { + resolve: neverRead, + reject: isRead, + shorthandNeverRead, + }; +} + +module.exports = { makeTable }; +`, + 'consumer.js': ` +const { makeTable } = require('./factory.js'); + +function run() { + const table = makeTable(); + return table.reject(1); +} + +module.exports = { run }; +`, +}; + +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(); + } +} + +describe('object-literal value-ref requires invocation evidence (#1895) — WASM', () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1895-')); + 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('does not create a value-ref calls edge for a property key that is never invoked', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + expect(countCallEdgesTo(dbPath, 'neverRead')).toBe(0); + expect(countCallEdgesTo(dbPath, 'shorthandNeverRead')).toBe(0); + }); + + it('creates a value-ref calls edge for a property key that IS invoked elsewhere', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + expect(countCallEdgesTo(dbPath, 'isRead')).toBeGreaterThan(0); + }); + + it('classifies the never-invoked dispatch-table entry as dead', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + const neverRead = nodes.find((n) => n.name === 'neverRead' && n.kind === 'function'); + expect(neverRead, 'neverRead node not found').toBeDefined(); + expect(DEAD_ROLES.has(neverRead!.role ?? '')).toBe(true); + + const shorthand = nodes.find((n) => n.name === 'shorthandNeverRead' && n.kind === 'function'); + expect(shorthand, 'shorthandNeverRead node not found').toBeDefined(); + expect(DEAD_ROLES.has(shorthand!.role ?? '')).toBe(true); + }); + + it('does not classify the actually-invoked dispatch-table entry as dead', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + const isRead = nodes.find((n) => n.name === 'isRead' && n.kind === 'function'); + expect(isRead, 'isRead node not found').toBeDefined(); + expect(DEAD_ROLES.has(isRead!.role ?? '')).toBe(false); + }); +}); + +// ── Native engine parity ──────────────────────────────────────────────────── +// Skipped when the native addon is not installed. + +describe.skipIf(!isNativeAvailable())( + 'object-literal value-ref requires invocation evidence (#1895) — native', + () => { + let nativeTmpDir: string; + + beforeAll(async () => { + nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1895-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('does not create a value-ref calls edge for a property key that is never invoked', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + expect(countCallEdgesTo(dbPath, 'neverRead')).toBe(0); + expect(countCallEdgesTo(dbPath, 'shorthandNeverRead')).toBe(0); + }); + + it('creates a value-ref calls edge for a property key that IS invoked elsewhere', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + expect(countCallEdgesTo(dbPath, 'isRead')).toBeGreaterThan(0); + }); + + it('classifies the never-invoked dispatch-table entry as dead', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + const neverRead = nodes.find((n) => n.name === 'neverRead' && n.kind === 'function'); + expect(neverRead, 'neverRead node not found (native)').toBeDefined(); + expect(DEAD_ROLES.has(neverRead!.role ?? '')).toBe(true); + }); + + it('does not classify the actually-invoked dispatch-table entry as dead', () => { + const dbPath = path.join(nativeTmpDir, '.codegraph', 'graph.db'); + const nodes = readNodesWithRoles(dbPath); + const isRead = nodes.find((n) => n.name === 'isRead' && n.kind === 'function'); + expect(isRead, 'isRead node not found (native)').toBeDefined(); + expect(DEAD_ROLES.has(isRead!.role ?? '')).toBe(false); + }); + }, +); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 6c3ee6ff..790bf0c5 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1504,6 +1504,57 @@ function runDemo(users: string[]): void { }); }); + describe('object-literal value-ref keyExpr capture (#1895)', () => { + it('captures the property key, distinct from the referenced value name', () => { + const symbols = parseJS(`const table = { resolve: someFunction };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ + name: 'someFunction', + dynamicKind: 'value-ref', + keyExpr: 'resolve', + }), + ); + }); + + it('captures a string-literal key with quotes stripped', () => { + const symbols = parseJS(`const table = { 'resolve': someFunction };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'someFunction', keyExpr: 'resolve' }), + ); + }); + + it('captures a computed string-literal key', () => { + const symbols = parseJS(`const table = { ['resolve']: someFunction };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'someFunction', keyExpr: 'resolve' }), + ); + }); + + it('leaves keyExpr unset for a non-string computed key', () => { + const symbols = parseJS(`const table = { [Symbol.iterator]: someFunction };`); + const call = symbols.calls.find( + (c) => c.dynamicKind === 'value-ref' && c.name === 'someFunction', + ); + expect(call?.keyExpr).toBeUndefined(); + }); + + it('sets keyExpr equal to name for a shorthand property', () => { + const symbols = parseJS(`const table = { someFunction };`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'someFunction', keyExpr: 'someFunction' }), + ); + }); + + it('leaves keyExpr unset for instanceof value-refs (no property key exists)', () => { + const symbols = parseJS(`if (err instanceof CodegraphError) {}`); + const call = symbols.calls.find( + (c) => c.dynamicKind === 'value-ref' && c.name === 'CodegraphError', + ); + expect(call).toBeDefined(); + expect(call?.keyExpr).toBeUndefined(); + }); + }); + describe('instanceof value-ref extraction (#1784)', () => { it('extracts a value-ref call for `instanceof ClassName`', () => { const symbols = parseJS(`