Skip to content

Commit 453a768

Browse files
committed
fix(resolver): require invocation evidence for object-literal value-ref liveness
Object-literal property-value references (`{ resolve: someFn }` bare identifiers and `{ someFn }` shorthand) previously granted a function liveness merely by existing as a property value, regardless of whether that property was ever actually invoked (`table.resolve(...)`) anywhere in the codebase. A dispatch-table entry that was wired up but never read escaped `roles --role dead` detection as a false negative. Value-ref calls originating from an object-literal property now carry the property's key name (Call.keyExpr / CallInfo.key_expr), distinct from the referenced value's own name. The resolver only honors the value-ref as a `calls` edge when that key is independently confirmed to be invoked via member-call syntax somewhere in the files being processed (full build: the whole codebase; incremental: the current file), mirrored identically in both the WASM/TS and native Rust engines. Impact: 10 functions changed, 32 affected
1 parent a45e74d commit 453a768

8 files changed

Lines changed: 536 additions & 11 deletions

File tree

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

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,18 @@ struct EdgeContext<'a> {
153153
nodes_by_file: HashMap<&'a str, Vec<&'a NodeInfo>>,
154154
builtin_set: HashSet<&'a str>,
155155
receiver_kinds: HashSet<&'a str>,
156+
/// Property/method names ever invoked via member-call syntax
157+
/// (`x.name(...)`) across every file in this build pass — see
158+
/// `collect_invoked_property_names` for the #1895 liveness rationale.
159+
invoked_property_names: HashSet<&'a str>,
156160
}
157161

158162
impl<'a> EdgeContext<'a> {
159-
fn new(all_nodes: &'a [NodeInfo], builtin_receivers: &'a [String]) -> Self {
163+
fn new(
164+
all_nodes: &'a [NodeInfo],
165+
builtin_receivers: &'a [String],
166+
files: &'a [FileEdgeInput],
167+
) -> Self {
160168
let mut nodes_by_name: HashMap<&str, Vec<&NodeInfo>> = HashMap::new();
161169
let mut nodes_by_name_and_file: HashMap<(&str, &str), Vec<&NodeInfo>> = HashMap::new();
162170
let mut nodes_by_file: HashMap<&str, Vec<&NodeInfo>> = HashMap::new();
@@ -177,8 +185,41 @@ impl<'a> EdgeContext<'a> {
177185
nodes_by_file,
178186
builtin_set,
179187
receiver_kinds,
188+
invoked_property_names: collect_invoked_property_names(files),
189+
}
190+
}
191+
}
192+
193+
/// Collect the set of property/method names ever invoked via member-call
194+
/// syntax (`x.name(...)`) across every file currently being processed —
195+
/// regardless of whether the receiver `x` itself resolves to anything.
196+
///
197+
/// Used as the "one hop further" liveness check for object-literal-property
198+
/// value-refs (#1895): a function referenced as `{ resolve: someFn }` should
199+
/// only be credited with a `calls` edge from that reference when something,
200+
/// somewhere, actually invokes a `.resolve(...)`-shaped call — otherwise the
201+
/// property is wired up but never read, and `someFn` is genuinely dead.
202+
///
203+
/// Scope matches whatever `files` the caller passes to `build_call_edges`:
204+
/// the full codebase for a full build, or just the changed file(s) on an
205+
/// incremental one. The incremental case is a narrower, same-build-pass view
206+
/// (a cross-file consumer added in an untouched file won't be seen until the
207+
/// next full rebuild) — the same scoping trade-off already accepted
208+
/// elsewhere in this codebase's incremental classification
209+
/// (`has_active_file_siblings`, exported-via-reexport, and median fan-in/out
210+
/// all recompute from the affected file set only, not the whole graph, in
211+
/// `graph/classifiers/roles.rs`'s incremental path). Mirrors
212+
/// `collectInvokedPropertyNames` in `src/domain/graph/builder/call-resolver.ts`.
213+
fn collect_invoked_property_names(files: &[FileEdgeInput]) -> HashSet<&str> {
214+
let mut names = HashSet::new();
215+
for file in files {
216+
for call in &file.calls {
217+
if call.receiver.is_some() {
218+
names.insert(call.name.as_str());
219+
}
180220
}
181221
}
222+
names
182223
}
183224

184225
// ── Phase 8.3: points-to analysis ─────────────────────────────────────────
@@ -467,7 +508,7 @@ pub fn build_call_edges(
467508
builtin_receivers: Vec<String>,
468509
max_iterations: u32,
469510
) -> Vec<ComputedEdge> {
470-
let ctx = EdgeContext::new(&all_nodes, &builtin_receivers);
511+
let ctx = EdgeContext::new(&all_nodes, &builtin_receivers, &files);
471512
let mut edges = Vec::new();
472513

473514
for file_input in &files {
@@ -799,6 +840,17 @@ fn process_file<'a>(
799840
// (stages/build-edges.ts).
800841
if call.dynamic_kind.as_deref() == Some("value-ref") {
801842
targets.retain(|t| t.kind == "function" || t.kind == "method" || t.kind == "class");
843+
// #1895: object-literal-property value-refs (key_expr set — see
844+
// handle_object_literal_pair_value_ref / shorthand handler)
845+
// additionally require independent evidence that the property is
846+
// actually invoked somewhere (`x.key_expr(...)`) — merely being
847+
// wired into an object literal is not liveness. instanceof/Lua
848+
// value-refs never set key_expr, so they are unaffected.
849+
if let Some(key_expr) = call.key_expr.as_deref() {
850+
if !ctx.invoked_property_names.contains(key_expr) {
851+
targets.clear();
852+
}
853+
}
802854
}
803855
sort_targets_by_confidence(&mut targets, fc.rel_path, imported_from);
804856
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 {
29042956
assert_eq!(re.target_id, 2, "receiver edge target should be Calculator (id=2)");
29052957
}
29062958

2959+
/// Issue #1895: an object-literal-property value-ref call whose property
2960+
/// key (`key_expr`) is never independently confirmed to be invoked
2961+
/// anywhere (`x.resolve(...)`) must NOT produce a `calls` edge — merely
2962+
/// being wired into the object literal is not liveness. A sibling
2963+
/// property whose key IS invoked elsewhere (`table.reject(...)`) keeps
2964+
/// its edge.
2965+
#[test]
2966+
fn value_ref_edge_requires_key_invoked_elsewhere() {
2967+
let all_nodes = vec![
2968+
node(1, "makeTable", "function", "factory.js", 1),
2969+
node(2, "neverRead", "function", "factory.js", 2),
2970+
node(3, "isRead", "function", "factory.js", 3),
2971+
node(4, "run", "function", "consumer.js", 1),
2972+
];
2973+
2974+
let mut resolve_call = call("neverRead", 5, None);
2975+
resolve_call.dynamic = Some(true);
2976+
resolve_call.dynamic_kind = Some("value-ref".to_string());
2977+
resolve_call.key_expr = Some("resolve".to_string());
2978+
2979+
let mut reject_call = call("isRead", 6, None);
2980+
reject_call.dynamic = Some(true);
2981+
reject_call.dynamic_kind = Some("value-ref".to_string());
2982+
reject_call.key_expr = Some("reject".to_string());
2983+
2984+
let factory_file = make_file(
2985+
"factory.js",
2986+
10,
2987+
vec![def("makeTable", "function", 1, 8)],
2988+
vec![resolve_call, reject_call],
2989+
vec![],
2990+
vec![],
2991+
);
2992+
2993+
// Evidence that `.reject(...)` is genuinely invoked somewhere, but
2994+
// `.resolve(...)` never is.
2995+
let consumer_file = make_file(
2996+
"consumer.js",
2997+
20,
2998+
vec![def("run", "function", 1, 3)],
2999+
vec![call("reject", 2, Some("table"))],
3000+
vec![],
3001+
vec![],
3002+
);
3003+
3004+
let edges = build_call_edges(
3005+
vec![factory_file, consumer_file],
3006+
all_nodes,
3007+
vec![],
3008+
MAX_SOLVER_ITERATIONS,
3009+
);
3010+
3011+
let calls_never_read = edges.iter().any(|e| e.kind == "calls" && e.target_id == 2);
3012+
let calls_is_read = edges.iter().any(|e| e.kind == "calls" && e.target_id == 3);
3013+
assert!(
3014+
!calls_never_read,
3015+
"expected no calls edge to neverRead (key 'resolve' never invoked); got: {:?}",
3016+
edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::<Vec<_>>()
3017+
);
3018+
assert!(
3019+
calls_is_read,
3020+
"expected a calls edge to isRead (key 'reject' invoked in consumer.js); got: {:?}",
3021+
edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::<Vec<_>>()
3022+
);
3023+
}
3024+
29073025
/// Regression: when the same file has a `kind="function"` node for the
29083026
/// effective receiver created by a destructured import (e.g.
29093027
/// `const { Calculator } = require('./utils')`), that import artifact must

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,6 +2874,12 @@ fn extract_callback_reference_calls(
28742874
/// (`{ name: SOME_CONSTANT }`) naturally fail to resolve into an edge rather
28752875
/// than needing a structural allowlist gate here.
28762876
///
2877+
/// `key_expr` carries the property KEY (e.g. `resolve`), distinct from `name`
2878+
/// (the referenced value's own identifier, e.g. `someFunction`) — the
2879+
/// downstream "is this property ever invoked" liveness check (#1895) needs
2880+
/// the key, since that's the name a dispatch consumer would actually call
2881+
/// (`table.resolve(...)`), not the function's own declared name.
2882+
///
28772883
/// Mirrors `collectObjectLiteralValueRefCall` in `src/extractors/javascript.ts`.
28782884
fn handle_object_literal_pair_value_ref(node: &Node, source: &[u8], calls: &mut Vec<Call>) {
28792885
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
28842890
if JS_BUILTIN_GLOBALS.contains(&text) {
28852891
return;
28862892
}
2893+
let key_expr = node.child_by_field_name("key").and_then(|k| resolve_pair_key_name(&k, source));
28872894
calls.push(Call {
28882895
name: text.to_string(),
28892896
line: start_line(&value_n),
28902897
dynamic: Some(true),
28912898
dynamic_kind: Some("value-ref".to_string()),
2899+
key_expr,
28922900
..Default::default()
28932901
});
28942902
}
@@ -2900,6 +2908,9 @@ fn handle_object_literal_pair_value_ref(node: &Node, source: &[u8], calls: &mut
29002908
/// `shorthand_property_identifier_pattern` kind), so this can't misfire on
29012909
/// destructuring targets.
29022910
///
2911+
/// `key_expr` equals `name` here — the property key and the referenced value
2912+
/// are the same identifier in shorthand form (#1895).
2913+
///
29032914
/// Mirrors the walk path's `shorthand_property_identifier` handling in
29042915
/// `src/extractors/javascript.ts`'s `runCollectorWalk` (issue #1771).
29052916
fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls: &mut Vec<Call>) {
@@ -2912,6 +2923,7 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls:
29122923
line: start_line(node),
29132924
dynamic: Some(true),
29142925
dynamic_kind: Some("value-ref".to_string()),
2926+
key_expr: Some(text.to_string()),
29152927
..Default::default()
29162928
});
29172929
}
@@ -5068,6 +5080,75 @@ mod tests {
50685080
assert!(value_refs.iter().any(|c| c.name == "someFunction"));
50695081
}
50705082

5083+
// #1895: key_expr capture — the property key, distinct from the
5084+
// referenced value's own name, is what a dispatch consumer would
5085+
// actually call (`table.resolve(...)`).
5086+
#[test]
5087+
fn value_ref_captures_property_key_distinct_from_referenced_name() {
5088+
let s = parse_js("const table = { resolve: someFunction };");
5089+
let call = s
5090+
.calls
5091+
.iter()
5092+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction")
5093+
.expect("expected a value-ref call");
5094+
assert_eq!(call.key_expr.as_deref(), Some("resolve"));
5095+
}
5096+
5097+
#[test]
5098+
fn value_ref_captures_string_literal_key_with_quotes_stripped() {
5099+
let s = parse_js("const table = { 'resolve': someFunction };");
5100+
let call = s
5101+
.calls
5102+
.iter()
5103+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction")
5104+
.expect("expected a value-ref call");
5105+
assert_eq!(call.key_expr.as_deref(), Some("resolve"));
5106+
}
5107+
5108+
#[test]
5109+
fn value_ref_captures_computed_string_literal_key() {
5110+
let s = parse_js("const table = { ['resolve']: someFunction };");
5111+
let call = s
5112+
.calls
5113+
.iter()
5114+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction")
5115+
.expect("expected a value-ref call");
5116+
assert_eq!(call.key_expr.as_deref(), Some("resolve"));
5117+
}
5118+
5119+
#[test]
5120+
fn value_ref_leaves_key_expr_unset_for_non_string_computed_key() {
5121+
let s = parse_js("const table = { [Symbol.iterator]: someFunction };");
5122+
let call = s
5123+
.calls
5124+
.iter()
5125+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction")
5126+
.expect("expected a value-ref call");
5127+
assert_eq!(call.key_expr, None);
5128+
}
5129+
5130+
#[test]
5131+
fn value_ref_key_expr_equals_name_for_shorthand_property() {
5132+
let s = parse_js("const table = { someFunction };");
5133+
let call = s
5134+
.calls
5135+
.iter()
5136+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "someFunction")
5137+
.expect("expected a value-ref call");
5138+
assert_eq!(call.key_expr.as_deref(), Some("someFunction"));
5139+
}
5140+
5141+
#[test]
5142+
fn instanceof_value_ref_leaves_key_expr_unset() {
5143+
let s = parse_js("if (err instanceof CodegraphError) {}");
5144+
let call = s
5145+
.calls
5146+
.iter()
5147+
.find(|c| c.dynamic_kind.as_deref() == Some("value-ref") && c.name == "CodegraphError")
5148+
.expect("expected a value-ref call");
5149+
assert_eq!(call.key_expr, None);
5150+
}
5151+
50715152
#[test]
50725153
fn no_value_ref_call_for_call_expression_value() {
50735154
let s = parse_js("const table = { resolve: someFunction() };");

src/domain/graph/builder/call-resolver.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,41 @@ export const RECEIVER_KINDS = new Set(['class', 'struct', 'interface', 'type', '
4545
// continue to work without changes (build-edges.ts, etc.).
4646
export { isModuleScopedLanguage };
4747

48+
/**
49+
* Collect the set of property/method names ever invoked via member-call
50+
* syntax (`x.name(...)`) across every file currently being processed —
51+
* regardless of whether the receiver `x` itself resolves to anything.
52+
*
53+
* Used as the "one hop further" liveness check for object-literal-property
54+
* value-refs (#1895): a function referenced as `{ resolve: someFn }` should
55+
* only be credited with a `calls` edge from that reference when something,
56+
* somewhere, actually invokes a `.resolve(...)`-shaped call — otherwise the
57+
* property is wired up but never read, and `someFn` is genuinely dead.
58+
*
59+
* Scope matches whatever set of files the caller passes in: the full
60+
* codebase for a full build (build-edges.ts's `buildCallEdgesJS`, from
61+
* `ctx.fileSymbols`), or just the single file being rebuilt on an incremental
62+
* update (incremental.ts's `buildCallEdges`, from that file's own `calls`).
63+
* The incremental case is a narrower, same-file view — a cross-file consumer
64+
* added in a different, untouched file won't be seen until the next full
65+
* rebuild — the same scoping trade-off already accepted elsewhere in this
66+
* codebase's incremental classification (`hasActiveFileSiblings`,
67+
* exported-via-reexport, and median fan-in/out all recompute from an affected
68+
* subset, not the whole graph, in `graph/classifiers/roles.rs`'s incremental
69+
* path).
70+
*/
71+
export function collectInvokedPropertyNames(
72+
callsList: Iterable<Iterable<{ name: string; receiver?: string }>>,
73+
): Set<string> {
74+
const names = new Set<string>();
75+
for (const calls of callsList) {
76+
for (const call of calls) {
77+
if (call.receiver) names.add(call.name);
78+
}
79+
}
80+
return names;
81+
}
82+
4883
/**
4984
* Shared by both the full-build (build-edges.ts) and incremental (incremental.ts)
5085
* same-class fallback strategies: derive the enclosing class name from the

src/domain/graph/builder/incremental.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
} from '../resolver/points-to.js';
3333
import {
3434
type CallNodeLookup,
35+
collectInvokedPropertyNames,
3536
findCaller,
3637
isModuleScopedLanguage,
3738
resolveCallTargets,
@@ -1055,6 +1056,10 @@ function buildCallEdges(
10551056
// JS path uses (buildPointsToMapForFile, shared via resolver/points-to.js).
10561057
const ptsMap = buildPointsToMapForFile(symbols, importedNames);
10571058
const fnRefBindingLhs = new Set(symbols.fnRefBindings?.map((b) => b.lhs) ?? []);
1059+
// #1895: scoped to this file's own calls only — see collectInvokedPropertyNames
1060+
// doc comment (call-resolver.ts) for why incremental rebuilds use a narrower,
1061+
// same-file view rather than a full-codebase one.
1062+
const invokedPropertyNames = collectInvokedPropertyNames([symbols.calls]);
10581063
let edgesAdded = 0;
10591064

10601065
for (const call of symbols.calls) {
@@ -1089,6 +1094,12 @@ function buildCallEdges(
10891094
targets = targets.filter(
10901095
(t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class',
10911096
);
1097+
// #1895: object-literal-property value-refs additionally require
1098+
// independent evidence the property is actually invoked somewhere —
1099+
// mirrors the same check in resolveFallbackTargets (stages/build-edges.ts).
1100+
if (call.keyExpr && !invokedPropertyNames.has(call.keyExpr)) {
1101+
targets = [];
1102+
}
10921103
}
10931104

10941105
edgesAdded += emitIncrementalCallEdges(

0 commit comments

Comments
 (0)