Skip to content

Commit a45e74d

Browse files
committed
fix(extractors): attribute same-file ES6 getter/setter property reads as call edges
A bare (non-call) property read or write on an ES6 get/set class accessor (`obj.isReady`, no call parens) invokes the accessor just as surely as `obj.isReady()` would — but call-site extraction only ever recognized member_expression nodes used as a call_expression's callee, so accessor reads/writes never produced a `calls` edge in either engine. Scoped to the same-file case: `this.prop` inside one of the accessor's own class's methods, or `varName.prop` where `varName`'s type (from the file's own typeMap) is a class also declared in the same file. A same-file accessor registry gates which bare property reads become synthetic Call entries, so they flow through the existing (unchanged) call-resolution cascade in both engines with no new edge kind, DB schema change, or serialization wiring. A property declaring both a getter and a setter is skipped (the two accessors share a qualified name and resolution can't tell them apart). Cross-file accessor recognition (the accessor's class declared in a different file than the read site) is filed as a follow-up: #2030. Fixes #1893 Impact: 12 functions changed, 11 affected
1 parent 801d290 commit a45e74d

4 files changed

Lines changed: 795 additions & 0 deletions

File tree

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

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ impl SymbolExtractor for JsExtractor {
4545
walk_tree(&tree.root_node(), source, &mut symbols, match_js_prototype_methods);
4646
// call_assignments runs after type_map is populated (needs receiver types)
4747
walk_tree(&tree.root_node(), source, &mut symbols, match_js_call_assignments);
48+
// #1893: same-file get/set accessor property reads/writes → calls edges.
49+
// Runs after type_map is populated (needs receiver types for the
50+
// `varName.prop` case) and after handle_method_def has run (the
51+
// registry re-derives accessor names directly from the AST, so source
52+
// order relative to match_js_node doesn't matter for correctness).
53+
let local_accessors = collect_local_accessors(&tree.root_node(), source);
54+
walk_tree(&tree.root_node(), source, &mut symbols, |node, source, symbols, _depth| {
55+
handle_accessor_property_read(node, source, symbols, &local_accessors)
56+
});
4857
// Phase 8.3c–8.3f: points-to bindings (params, this-rebinding, arrays,
4958
// spread, for-of, object rest/props) for the pts constraint solver.
5059
walk_tree(&tree.root_node(), source, &mut symbols, match_js_pts_bindings);
@@ -1175,6 +1184,171 @@ fn handle_method_def(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
11751184
}
11761185
}
11771186

1187+
// ── ES6 getter/setter property-read call attribution (#1893) ────────────────
1188+
//
1189+
// A bare (non-call) property read/write on an ES6 `get`/`set` class accessor
1190+
// (`obj.isReady`, no call parens) invokes the accessor function just as surely
1191+
// as `obj.isReady()` would if written explicitly — but call-site extraction
1192+
// only ever looked at `member_expression` nodes used as a call_expression's
1193+
// callee, so accessor reads/writes never produced a `calls` edge at all.
1194+
// Mirrors `collectAccessorPropertyRead` in `src/extractors/javascript.ts`.
1195+
//
1196+
// Scoped to the *same-file* case: `this.prop` inside one of the accessor's own
1197+
// class's methods, or `varName.prop` where `varName`'s type (from this file's
1198+
// own type_map) is a class also declared in this file. Cross-file accessor
1199+
// reads (the accessor's class declared in a different file than the read
1200+
// site) are not yet covered — see #2030.
1201+
1202+
/// Per-property record of which accessor kinds a same-file class declares.
1203+
#[derive(Default, Clone, Copy)]
1204+
struct LocalAccessorInfo {
1205+
get: bool,
1206+
set: bool,
1207+
}
1208+
1209+
/// `ClassName.propName` → which accessor kinds are declared, for this file only.
1210+
type LocalAccessorRegistry = HashMap<String, LocalAccessorInfo>;
1211+
1212+
/// True when `meth_node` (a method_definition) carries a `get` or `set`
1213+
/// accessor modifier — an unnamed token child preceding the `name` field
1214+
/// (tree-sitter represents `get`/`set`/`static`/`async` as literal unnamed
1215+
/// children, not a dedicated field). Returns `None` for a plain method.
1216+
fn get_method_accessor_kind(meth_node: &Node) -> Option<&'static str> {
1217+
let name_node = meth_node.child_by_field_name("name");
1218+
for i in 0..meth_node.child_count() {
1219+
let Some(child) = meth_node.child(i) else { continue };
1220+
if Some(child.id()) == name_node.map(|n| n.id()) {
1221+
break;
1222+
}
1223+
match child.kind() {
1224+
"get" => return Some("get"),
1225+
"set" => return Some("set"),
1226+
_ => {}
1227+
}
1228+
}
1229+
None
1230+
}
1231+
1232+
/// Pre-scan pass: collect every ES6 get/set class-accessor declared in this
1233+
/// file, keyed by its qualified `ClassName.propName` name — the same
1234+
/// qualification `handle_method_def` already gives the accessor's own
1235+
/// Definition entry. Must run before the property-read pass so the registry
1236+
/// is complete regardless of source order.
1237+
fn collect_local_accessors(root: &Node, source: &[u8]) -> LocalAccessorRegistry {
1238+
let mut registry = LocalAccessorRegistry::new();
1239+
1240+
fn walk(node: &Node, source: &[u8], registry: &mut LocalAccessorRegistry, depth: usize) {
1241+
if depth >= MAX_WALK_DEPTH {
1242+
return;
1243+
}
1244+
if node.kind() == "method_definition" {
1245+
if let Some(kind) = get_method_accessor_kind(node) {
1246+
if let (Some(class_name), Some(prop_name)) =
1247+
(find_parent_class(node, source), resolve_method_def_name(node, source))
1248+
{
1249+
let key = format!("{}.{}", class_name, prop_name);
1250+
let entry = registry.entry(key).or_default();
1251+
if kind == "get" {
1252+
entry.get = true;
1253+
} else {
1254+
entry.set = true;
1255+
}
1256+
}
1257+
}
1258+
}
1259+
for i in 0..node.child_count() {
1260+
if let Some(child) = node.child(i) {
1261+
walk(&child, source, registry, depth + 1);
1262+
}
1263+
}
1264+
}
1265+
1266+
walk(root, source, &mut registry, 0);
1267+
registry
1268+
}
1269+
1270+
/// Detect a bare (non-call) `this.prop` / `varName.prop` member-expression
1271+
/// that reads or writes a same-file accessor property, and record it as an
1272+
/// ordinary `Call` — indistinguishable from a real
1273+
/// `this.prop()`/`varName.prop()` call site, so it flows through the existing
1274+
/// (unchanged) call-resolution cascade.
1275+
///
1276+
/// A plain assignment (`obj.prop = value`) invokes the setter; every other
1277+
/// bare usage (reads, compound-assignment targets, etc.) invokes the getter.
1278+
/// When a property declares *both* a getter and a setter, the two accessors
1279+
/// share the same qualified name and resolution has no way to tell them
1280+
/// apart — rather than risk an edge to the wrong one, that case is skipped
1281+
/// entirely (mirrors the "ambiguous → drop rather than fan out" precedent
1282+
/// used elsewhere in call resolution).
1283+
fn handle_accessor_property_read(
1284+
node: &Node,
1285+
source: &[u8],
1286+
symbols: &mut FileSymbols,
1287+
local_accessors: &LocalAccessorRegistry,
1288+
) {
1289+
if node.kind() != "member_expression" {
1290+
return;
1291+
}
1292+
// obj.method() — already a real call, handled by the regular call path
1293+
// regardless of whether `method` also happens to be an accessor.
1294+
if let Some(parent) = node.parent() {
1295+
if parent.kind() == "call_expression"
1296+
&& parent.child_by_field_name("function").map(|f| f.id()) == Some(node.id())
1297+
{
1298+
return;
1299+
}
1300+
}
1301+
1302+
let Some(obj) = node.child_by_field_name("object") else { return };
1303+
let Some(prop_node) = node.child_by_field_name("property") else { return };
1304+
if prop_node.kind() != "property_identifier" {
1305+
return;
1306+
}
1307+
let prop_name = node_text(&prop_node, source);
1308+
1309+
let (receiver, class_name): (String, Option<String>) = match obj.kind() {
1310+
"this" => ("this".to_string(), find_parent_class(node, source)),
1311+
"identifier" => {
1312+
let obj_name = node_text(&obj, source).to_string();
1313+
let type_name = symbols
1314+
.type_map
1315+
.iter()
1316+
.find(|e| e.name == obj_name)
1317+
.map(|e| e.type_name.clone());
1318+
(obj_name, type_name)
1319+
}
1320+
_ => return,
1321+
};
1322+
let Some(class_name) = class_name else { return };
1323+
1324+
let key = format!("{}.{}", class_name, prop_name);
1325+
let Some(accessor_info) = local_accessors.get(&key) else { return };
1326+
if accessor_info.get && accessor_info.set {
1327+
return;
1328+
}
1329+
1330+
let is_plain_assign_target = node
1331+
.parent()
1332+
.filter(|p| p.kind() == "assignment_expression")
1333+
.and_then(|p| p.child_by_field_name("left"))
1334+
.map(|l| l.id())
1335+
== Some(node.id());
1336+
let needed_get = !is_plain_assign_target;
1337+
if needed_get && !accessor_info.get {
1338+
return;
1339+
}
1340+
if !needed_get && !accessor_info.set {
1341+
return;
1342+
}
1343+
1344+
symbols.calls.push(Call {
1345+
name: prop_name.to_string(),
1346+
line: start_line(node),
1347+
receiver: Some(receiver),
1348+
..Default::default()
1349+
});
1350+
}
1351+
11781352
/// Create a synthetic `ClassName.<static:L:C>` definition for a class static block
11791353
/// so that calls inside the block are attributed to a method-kind node and
11801354
/// `super.method()` dispatch can walk up to the parent class.
@@ -6529,4 +6703,116 @@ mod tests {
65296703
let dt_call = s.calls.iter().find(|c| c.name.starts_with("<dt_") && c.name.ends_with(">[*]"));
65306704
assert!(dt_call.is_some(), "dispatch-table call missing for parenthesized object; got: {:?}", s.calls);
65316705
}
6706+
6707+
// ── ES6 getter/setter same-file property-read call attribution (#1893) ──
6708+
6709+
#[test]
6710+
fn attributes_bare_this_prop_read_to_same_class_getter() {
6711+
let s = parse_js(
6712+
"class Session {\n\
6713+
get isReady() { return this._ready; }\n\
6714+
check() { if (this.isReady) { report(); } }\n\
6715+
}",
6716+
);
6717+
assert!(
6718+
s.calls.iter().any(|c| c.name == "isReady" && c.receiver.as_deref() == Some("this")),
6719+
"expected a call to isReady via this; got: {:?}",
6720+
s.calls
6721+
);
6722+
}
6723+
6724+
#[test]
6725+
fn attributes_bare_varname_prop_read_to_same_file_class_getter_via_type_map() {
6726+
let s = parse_ts(
6727+
"class Repo {\n\
6728+
get db() { return this._db; }\n\
6729+
}\n\
6730+
function useRepo(repo: Repo) {\n\
6731+
return repo.db;\n\
6732+
}",
6733+
);
6734+
assert!(
6735+
s.calls.iter().any(|c| c.name == "db" && c.receiver.as_deref() == Some("repo")),
6736+
"expected a call to db via repo; got: {:?}",
6737+
s.calls
6738+
);
6739+
}
6740+
6741+
#[test]
6742+
fn attributes_plain_assignment_write_to_same_class_setter() {
6743+
let s = parse_js(
6744+
"class Toggle {\n\
6745+
set flag(v) { this._f = v; }\n\
6746+
reset() { this.flag = false; }\n\
6747+
}",
6748+
);
6749+
assert!(
6750+
s.calls.iter().any(|c| c.name == "flag" && c.receiver.as_deref() == Some("this")),
6751+
"expected a call to flag via this; got: {:?}",
6752+
s.calls
6753+
);
6754+
}
6755+
6756+
#[test]
6757+
fn skips_property_with_both_getter_and_setter() {
6758+
let s = parse_js(
6759+
"class Toggle {\n\
6760+
get flag() { return this._f; }\n\
6761+
set flag(v) { this._f = v; }\n\
6762+
flip() { this.flag = !this.flag; }\n\
6763+
}",
6764+
);
6765+
assert!(
6766+
!s.calls.iter().any(|c| c.name == "flag"),
6767+
"ambiguous get+set accessor must not produce a call; got: {:?}",
6768+
s.calls
6769+
);
6770+
}
6771+
6772+
#[test]
6773+
fn does_not_duplicate_a_real_call_to_an_accessor_name() {
6774+
let s = parse_js(
6775+
"class Widget {\n\
6776+
get value() { return this._v; }\n\
6777+
}\n\
6778+
function useWidget(w) {\n\
6779+
return w.value();\n\
6780+
}",
6781+
);
6782+
let matches = s.calls.iter().filter(|c| c.name == "value" && c.receiver.as_deref() == Some("w")).count();
6783+
assert_eq!(matches, 1, "expected exactly one call to w.value(); got: {:?}", s.calls);
6784+
}
6785+
6786+
#[test]
6787+
fn does_not_attribute_plain_method_reference_as_call() {
6788+
let s = parse_js(
6789+
"class Widget {\n\
6790+
render() { return 1; }\n\
6791+
}\n\
6792+
function useWidget(w) {\n\
6793+
const fn = w.render;\n\
6794+
return fn;\n\
6795+
}",
6796+
);
6797+
assert!(
6798+
!s.calls.iter().any(|c| c.name == "render" && c.receiver.as_deref() == Some("w")),
6799+
"plain method reference (no accessor) must not produce a call; got: {:?}",
6800+
s.calls
6801+
);
6802+
}
6803+
6804+
#[test]
6805+
fn recognizes_static_accessor_same_as_instance() {
6806+
let s = parse_js(
6807+
"class Config {\n\
6808+
static get version() { return Config._v; }\n\
6809+
static describe() { return this.version; }\n\
6810+
}",
6811+
);
6812+
assert!(
6813+
s.calls.iter().any(|c| c.name == "version" && c.receiver.as_deref() == Some("this")),
6814+
"expected a call to version via this; got: {:?}",
6815+
s.calls
6816+
);
6817+
}
65326818
}

0 commit comments

Comments
 (0)