From a45e74d58f6834549414411c01ad631b0ded191a Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 8 Jul 2026 05:50:48 -0600 Subject: [PATCH] fix(extractors): attribute same-file ES6 getter/setter property reads as call edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/extractors/javascript.rs | 286 ++++++++++++++++++ src/extractors/javascript.ts | 158 ++++++++++ ...e-1893-getter-setter-property-read.test.ts | 231 ++++++++++++++ tests/parsers/javascript.test.ts | 120 ++++++++ 4 files changed, 795 insertions(+) create mode 100644 tests/integration/issue-1893-getter-setter-property-read.test.ts diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index fd1a0f8b..91849303 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -45,6 +45,15 @@ impl SymbolExtractor for JsExtractor { walk_tree(&tree.root_node(), source, &mut symbols, match_js_prototype_methods); // call_assignments runs after type_map is populated (needs receiver types) walk_tree(&tree.root_node(), source, &mut symbols, match_js_call_assignments); + // #1893: same-file get/set accessor property reads/writes → calls edges. + // Runs after type_map is populated (needs receiver types for the + // `varName.prop` case) and after handle_method_def has run (the + // registry re-derives accessor names directly from the AST, so source + // order relative to match_js_node doesn't matter for correctness). + let local_accessors = collect_local_accessors(&tree.root_node(), source); + walk_tree(&tree.root_node(), source, &mut symbols, |node, source, symbols, _depth| { + handle_accessor_property_read(node, source, symbols, &local_accessors) + }); // Phase 8.3c–8.3f: points-to bindings (params, this-rebinding, arrays, // spread, for-of, object rest/props) for the pts constraint solver. 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) { } } +// ── ES6 getter/setter property-read call attribution (#1893) ──────────────── +// +// A bare (non-call) property read/write on an ES6 `get`/`set` class accessor +// (`obj.isReady`, no call parens) invokes the accessor function just as surely +// as `obj.isReady()` would if written explicitly — but call-site extraction +// only ever looked at `member_expression` nodes used as a call_expression's +// callee, so accessor reads/writes never produced a `calls` edge at all. +// Mirrors `collectAccessorPropertyRead` in `src/extractors/javascript.ts`. +// +// 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 this file's +// own type_map) is a class also declared in this file. Cross-file accessor +// reads (the accessor's class declared in a different file than the read +// site) are not yet covered — see #2030. + +/// Per-property record of which accessor kinds a same-file class declares. +#[derive(Default, Clone, Copy)] +struct LocalAccessorInfo { + get: bool, + set: bool, +} + +/// `ClassName.propName` → which accessor kinds are declared, for this file only. +type LocalAccessorRegistry = HashMap; + +/// True when `meth_node` (a method_definition) carries a `get` or `set` +/// accessor modifier — an unnamed token child preceding the `name` field +/// (tree-sitter represents `get`/`set`/`static`/`async` as literal unnamed +/// children, not a dedicated field). Returns `None` for a plain method. +fn get_method_accessor_kind(meth_node: &Node) -> Option<&'static str> { + let name_node = meth_node.child_by_field_name("name"); + for i in 0..meth_node.child_count() { + let Some(child) = meth_node.child(i) else { continue }; + if Some(child.id()) == name_node.map(|n| n.id()) { + break; + } + match child.kind() { + "get" => return Some("get"), + "set" => return Some("set"), + _ => {} + } + } + None +} + +/// Pre-scan pass: collect every ES6 get/set class-accessor declared in this +/// file, keyed by its qualified `ClassName.propName` name — the same +/// qualification `handle_method_def` already gives the accessor's own +/// Definition entry. Must run before the property-read pass so the registry +/// is complete regardless of source order. +fn collect_local_accessors(root: &Node, source: &[u8]) -> LocalAccessorRegistry { + let mut registry = LocalAccessorRegistry::new(); + + fn walk(node: &Node, source: &[u8], registry: &mut LocalAccessorRegistry, depth: usize) { + if depth >= MAX_WALK_DEPTH { + return; + } + if node.kind() == "method_definition" { + if let Some(kind) = get_method_accessor_kind(node) { + if let (Some(class_name), Some(prop_name)) = + (find_parent_class(node, source), resolve_method_def_name(node, source)) + { + let key = format!("{}.{}", class_name, prop_name); + let entry = registry.entry(key).or_default(); + if kind == "get" { + entry.get = true; + } else { + entry.set = true; + } + } + } + } + for i in 0..node.child_count() { + if let Some(child) = node.child(i) { + walk(&child, source, registry, depth + 1); + } + } + } + + walk(root, source, &mut registry, 0); + registry +} + +/// Detect a bare (non-call) `this.prop` / `varName.prop` member-expression +/// that reads or writes a same-file accessor property, and record it as an +/// ordinary `Call` — indistinguishable from a real +/// `this.prop()`/`varName.prop()` call site, so it flows through the existing +/// (unchanged) call-resolution cascade. +/// +/// A plain assignment (`obj.prop = value`) invokes the setter; every other +/// bare usage (reads, compound-assignment targets, etc.) invokes the getter. +/// When a property declares *both* a getter and a setter, the two accessors +/// share the same qualified name and resolution has no way to tell them +/// apart — rather than risk an edge to the wrong one, that case is skipped +/// entirely (mirrors the "ambiguous → drop rather than fan out" precedent +/// used elsewhere in call resolution). +fn handle_accessor_property_read( + node: &Node, + source: &[u8], + symbols: &mut FileSymbols, + local_accessors: &LocalAccessorRegistry, +) { + if node.kind() != "member_expression" { + return; + } + // obj.method() — already a real call, handled by the regular call path + // regardless of whether `method` also happens to be an accessor. + if let Some(parent) = node.parent() { + if parent.kind() == "call_expression" + && parent.child_by_field_name("function").map(|f| f.id()) == Some(node.id()) + { + return; + } + } + + let Some(obj) = node.child_by_field_name("object") else { return }; + let Some(prop_node) = node.child_by_field_name("property") else { return }; + if prop_node.kind() != "property_identifier" { + return; + } + let prop_name = node_text(&prop_node, source); + + let (receiver, class_name): (String, Option) = match obj.kind() { + "this" => ("this".to_string(), find_parent_class(node, source)), + "identifier" => { + let obj_name = node_text(&obj, source).to_string(); + let type_name = symbols + .type_map + .iter() + .find(|e| e.name == obj_name) + .map(|e| e.type_name.clone()); + (obj_name, type_name) + } + _ => return, + }; + let Some(class_name) = class_name else { return }; + + let key = format!("{}.{}", class_name, prop_name); + let Some(accessor_info) = local_accessors.get(&key) else { return }; + if accessor_info.get && accessor_info.set { + return; + } + + let is_plain_assign_target = node + .parent() + .filter(|p| p.kind() == "assignment_expression") + .and_then(|p| p.child_by_field_name("left")) + .map(|l| l.id()) + == Some(node.id()); + let needed_get = !is_plain_assign_target; + if needed_get && !accessor_info.get { + return; + } + if !needed_get && !accessor_info.set { + return; + } + + symbols.calls.push(Call { + name: prop_name.to_string(), + line: start_line(node), + receiver: Some(receiver), + ..Default::default() + }); +} + /// Create a synthetic `ClassName.` definition for a class static block /// so that calls inside the block are attributed to a method-kind node and /// `super.method()` dispatch can walk up to the parent class. @@ -6529,4 +6703,116 @@ mod tests { let dt_call = s.calls.iter().find(|c| c.name.starts_with("[*]")); assert!(dt_call.is_some(), "dispatch-table call missing for parenthesized object; got: {:?}", s.calls); } + + // ── ES6 getter/setter same-file property-read call attribution (#1893) ── + + #[test] + fn attributes_bare_this_prop_read_to_same_class_getter() { + let s = parse_js( + "class Session {\n\ + get isReady() { return this._ready; }\n\ + check() { if (this.isReady) { report(); } }\n\ + }", + ); + assert!( + s.calls.iter().any(|c| c.name == "isReady" && c.receiver.as_deref() == Some("this")), + "expected a call to isReady via this; got: {:?}", + s.calls + ); + } + + #[test] + fn attributes_bare_varname_prop_read_to_same_file_class_getter_via_type_map() { + let s = parse_ts( + "class Repo {\n\ + get db() { return this._db; }\n\ + }\n\ + function useRepo(repo: Repo) {\n\ + return repo.db;\n\ + }", + ); + assert!( + s.calls.iter().any(|c| c.name == "db" && c.receiver.as_deref() == Some("repo")), + "expected a call to db via repo; got: {:?}", + s.calls + ); + } + + #[test] + fn attributes_plain_assignment_write_to_same_class_setter() { + let s = parse_js( + "class Toggle {\n\ + set flag(v) { this._f = v; }\n\ + reset() { this.flag = false; }\n\ + }", + ); + assert!( + s.calls.iter().any(|c| c.name == "flag" && c.receiver.as_deref() == Some("this")), + "expected a call to flag via this; got: {:?}", + s.calls + ); + } + + #[test] + fn skips_property_with_both_getter_and_setter() { + let s = parse_js( + "class Toggle {\n\ + get flag() { return this._f; }\n\ + set flag(v) { this._f = v; }\n\ + flip() { this.flag = !this.flag; }\n\ + }", + ); + assert!( + !s.calls.iter().any(|c| c.name == "flag"), + "ambiguous get+set accessor must not produce a call; got: {:?}", + s.calls + ); + } + + #[test] + fn does_not_duplicate_a_real_call_to_an_accessor_name() { + let s = parse_js( + "class Widget {\n\ + get value() { return this._v; }\n\ + }\n\ + function useWidget(w) {\n\ + return w.value();\n\ + }", + ); + let matches = s.calls.iter().filter(|c| c.name == "value" && c.receiver.as_deref() == Some("w")).count(); + assert_eq!(matches, 1, "expected exactly one call to w.value(); got: {:?}", s.calls); + } + + #[test] + fn does_not_attribute_plain_method_reference_as_call() { + let s = parse_js( + "class Widget {\n\ + render() { return 1; }\n\ + }\n\ + function useWidget(w) {\n\ + const fn = w.render;\n\ + return fn;\n\ + }", + ); + assert!( + !s.calls.iter().any(|c| c.name == "render" && c.receiver.as_deref() == Some("w")), + "plain method reference (no accessor) must not produce a call; got: {:?}", + s.calls + ); + } + + #[test] + fn recognizes_static_accessor_same_as_instance() { + let s = parse_js( + "class Config {\n\ + static get version() { return Config._v; }\n\ + static describe() { return this.version; }\n\ + }", + ); + assert!( + s.calls.iter().any(|c| c.name == "version" && c.receiver.as_deref() == Some("this")), + "expected a call to version via this; got: {:?}", + s.calls + ); + } } diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 9b7a19cc..539e48f6 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -458,6 +458,10 @@ function extractSymbolsQuery(tree: TreeSitterTree, query: TreeSitterQuery): Extr // which query patterns don't capture), and this()/call/apply bindings. const newExpressions: string[] = []; const definePropertyReceivers: Map = new Map(); + // #1893: same-file get/set accessor registry, needed before the collector + // walk below so bare property reads can be recognized regardless of + // whether the accessing code appears before or after the class declaration. + const localAccessors = collectLocalAccessors(tree.rootNode); runCollectorWalk(tree.rootNode, { definitions, typeMap, @@ -467,6 +471,7 @@ function extractSymbolsQuery(tree: TreeSitterTree, query: TreeSitterQuery): Extr newExpressions, definePropertyReceivers, valueRefCalls: calls, + localAccessors, imports, calls, thisCallBindings, @@ -572,6 +577,144 @@ function buildMethodDefinition(node: TreeSitterNode, name: string): Definition { }; } +// ── ES6 getter/setter property-read call attribution (#1893) ──────────────── +// +// A bare (non-call) property read/write on an ES6 `get`/`set` class accessor +// (`obj.isReady`, no call parens) invokes the accessor function just as surely +// as `obj.isReady()` would if written explicitly — but call-site extraction +// only ever looked at `member_expression` nodes used as a call_expression's +// callee, so accessor reads/writes never produced a `calls` edge at all. +// +// 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 this file's +// own typeMap) is a class also declared in this file. Cross-file accessor +// reads (the accessor's class declared in a different file than the read +// site) are not yet covered — see #2030. + +/** Per-property record of which accessor kinds a same-file class declares. */ +interface LocalAccessorInfo { + get: boolean; + set: boolean; +} + +/** `ClassName.propName` → which accessor kinds are declared, for this file only. */ +type LocalAccessorRegistry = Map; + +/** + * True when `methNode` (a method_definition) carries a `get` or `set` accessor + * modifier — an unnamed token child preceding the `name` field (tree-sitter + * represents `get`/`set`/`static`/`async` as literal unnamed children, not a + * dedicated field). Returns null for a plain (non-accessor) method. + */ +function getMethodAccessorKind(methNode: TreeSitterNode): 'get' | 'set' | null { + const nameNode = methNode.childForFieldName('name'); + for (let i = 0; i < methNode.childCount; i++) { + const child = methNode.child(i); + if (!child || child === nameNode) break; + if (child.type === 'get' || child.type === 'set') return child.type; + } + return null; +} + +/** + * Pre-scan pass: collect every ES6 get/set class-accessor declared in this + * file, keyed by its qualified `ClassName.propName` name — the same + * qualification `buildMethodDefinition`'s caller already gives the accessor's + * own Definition entry. Must run before the property-read walk below so the + * registry is complete regardless of source order (a class can be declared + * after code that reads its instances' accessors). + */ +function collectLocalAccessors(rootNode: TreeSitterNode): LocalAccessorRegistry { + const registry: LocalAccessorRegistry = new Map(); + const walk = (node: TreeSitterNode, depth: number): void => { + if (depth >= MAX_WALK_DEPTH) return; + if (node.type === 'method_definition') { + const accessorKind = getMethodAccessorKind(node); + if (accessorKind) { + const nameNode = node.childForFieldName('name'); + const className = findParentClass(node); + const propName = nameNode ? resolveMethodDefinitionName(nameNode) : ''; + if (className && propName) { + const key = `${className}.${propName}`; + const entry = registry.get(key) ?? { get: false, set: false }; + entry[accessorKind] = true; + registry.set(key, entry); + } + } + } + for (let i = 0; i < node.childCount; i++) { + walk(node.child(i)!, depth + 1); + } + }; + walk(rootNode, 0); + return registry; +} + +/** Unwrap a typeMap entry (always `{type, confidence}` in this file's own typeMap) to its type name. */ +function localTypeMapTypeName(typeMap: Map, varName: string): string | null { + return typeMap.get(varName)?.type ?? null; +} + +/** + * Detect a bare (non-call) `this.prop` / `varName.prop` member-expression that + * reads or writes a same-file accessor property, and record it as an ordinary + * `Call` — indistinguishable from a real `this.prop()`/`varName.prop()` call + * site, so it flows through the existing (unchanged) call-resolution cascade. + * + * A plain assignment (`obj.prop = value`) invokes the setter; every other bare + * usage (reads, compound-assignment targets, etc.) invokes the getter. When a + * property declares *both* a getter and a setter, the two accessors share the + * same qualified name and resolution has no way to tell them apart — rather + * than risk an edge to the wrong one, that case is skipped entirely (mirrors + * resolveExactGlobalMatch's "ambiguous → drop rather than fan out" precedent + * in resolver/strategy.ts). + */ +function collectAccessorPropertyRead( + node: TreeSitterNode, + localAccessors: LocalAccessorRegistry, + typeMap: Map, + valueRefCalls: Call[], +): void { + const parent = node.parent; + // obj.method() — already a real call, handled by the regular call path + // regardless of whether `method` also happens to be an accessor. Node + // identity must be compared via `.id` — tree-sitter (WASM) mints a fresh + // wrapper object on every childForFieldName()/parent access, so `===` + // between two independently-fetched references to the same AST node is + // always false. + if (parent?.type === 'call_expression' && parent.childForFieldName('function')?.id === node.id) { + return; + } + + const obj = node.childForFieldName('object'); + const propNode = node.childForFieldName('property'); + if (!obj || !propNode || propNode.type !== 'property_identifier') return; + const propName = propNode.text; + + let receiver: string; + let className: string | null; + if (obj.type === 'this') { + receiver = 'this'; + className = findParentClass(node); + } else if (obj.type === 'identifier') { + receiver = obj.text; + className = localTypeMapTypeName(typeMap, obj.text); + } else { + return; + } + if (!className) return; + + const accessorInfo = localAccessors.get(`${className}.${propName}`); + if (!accessorInfo || (accessorInfo.get && accessorInfo.set)) return; + + const isPlainAssignTarget = + parent?.type === 'assignment_expression' && parent.childForFieldName('left')?.id === node.id; + const neededKind = isPlainAssignTarget ? 'set' : 'get'; + if (!accessorInfo[neededKind]) return; + + valueRefCalls.push({ name: propName, receiver, line: nodeStartLine(node) }); +} + /** * Recursively walk the AST to extract `const x = ` as constants. * Skips nodes inside function scopes so only file-level / block-level constants @@ -942,6 +1085,9 @@ function extractSymbolsWalk(tree: TreeSitterTree): ExtractorOutput { // walkJavaScriptNode already covers those node types on this path. const newExpressions: string[] = []; const definePropertyReceivers: Map = new Map(); + // #1893: same-file get/set accessor registry — see the query-path call site + // of collectLocalAccessors for why this must be computed up front. + const localAccessors = collectLocalAccessors(tree.rootNode); runCollectorWalk(tree.rootNode, { definitions: ctx.definitions, typeMap: ctx.typeMap!, @@ -951,6 +1097,7 @@ function extractSymbolsWalk(tree: TreeSitterTree): ExtractorOutput { newExpressions, definePropertyReceivers, valueRefCalls: ctx.calls, + localAccessors, funcPropDefs: ctx.definitions, }); ctx.newExpressions = newExpressions; @@ -4057,6 +4204,8 @@ interface CollectorWalkTargets { newExpressions: string[]; definePropertyReceivers: Map; valueRefCalls: Call[]; + /** #1893: same-file `ClassName.propName` → declared get/set accessor kinds. */ + localAccessors: LocalAccessorRegistry; imports?: Import[]; calls?: Call[]; thisCallBindings?: ThisCallBinding[]; @@ -4155,6 +4304,15 @@ function runCollectorWalk(rootNode: TreeSitterNode, targets: CollectorWalkTarget // #1784: `instanceof ClassName` checks, e.g. `err instanceof CodegraphError`. collectInstanceofValueRefCall(node, targets.valueRefCalls); break; + case 'member_expression': + // #1893: bare (non-call) reads/writes of a same-file get/set class accessor. + collectAccessorPropertyRead( + node, + targets.localAccessors, + targets.typeMap, + targets.valueRefCalls, + ); + break; } for (let i = 0; i < node.childCount; i++) { walk(node.child(i)!, depth + 1, childInDynamicImport); diff --git a/tests/integration/issue-1893-getter-setter-property-read.test.ts b/tests/integration/issue-1893-getter-setter-property-read.test.ts new file mode 100644 index 00000000..b5cdfd7d --- /dev/null +++ b/tests/integration/issue-1893-getter-setter-property-read.test.ts @@ -0,0 +1,231 @@ +/** + * Regression test for #1893: a bare (non-call) property read/write on an ES6 + * `get`/`set` class accessor (`obj.isReady`, no call parens) never produced a + * `calls` edge — call-site extraction only recognized `member_expression` + * nodes used as a call_expression's callee. + * + * Scoped fix: same-file accessor recognition — `this.prop` inside one of the + * accessor's own class's methods, or `varName.prop` where `varName`'s type is + * a class also declared in this file. Verifies: + * - the getter-read and setter-write edges appear in a full build + * - both engines (wasm/native) produce identical edges + * - full build and an incremental single-file rebuild agree + * - a property with both a getter and setter (ambiguous target) produces + * no edge, on both engines + */ +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 { getNodeId as getNodeIdQuery, initSchema, openDb } from '../../src/db/index.js'; +import { rebuildFile } from '../../src/domain/graph/builder/incremental.js'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { EngineMode } from '../../src/types.js'; + +function writeFixture(dir: string): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'accessor.ts'), + `export class Session { + get isReady(): boolean { + return this._ready; + } + + check(): void { + if (this.isReady) { + report(); + } + } +} + +export class Toggle { + get flag(): boolean { + return this._f; + } + set flag(v: boolean) { + this._f = v; + } + private _f = false; + + flip(): void { + this.flag = !this.flag; + } +} + +export class Repo { + get db(): unknown { + return this._db; + } + private _db: unknown; +} + +export function useRepo(repo: Repo): unknown { + return repo.db; +} + +function report(): void {} +`, + ); +} + +interface CallEdgeRow { + src: string; + srcKind: string; + tgt: string; + tgtKind: string; + kind: string; +} + +function readCallEdges(dbPath: string): CallEdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n1.kind AS srcKind, n2.name AS tgt, n2.kind AS tgtKind, e.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, n1.kind`, + ) + .all() as CallEdgeRow[]; + } finally { + db.close(); + } +} + +function runScenario(engine: EngineMode): void { + describe(`ES6 getter/setter property-read attribution (#1893) — ${engine}`, () => { + let projDir: string; + + beforeAll(async () => { + projDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1893-${engine}-`)); + writeFixture(projDir); + await buildGraph(projDir, { engine, incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(projDir, { recursive: true, force: true }); + }); + + it('attributes a bare `this.prop` read to the same-class getter', () => { + const edges = readCallEdges(path.join(projDir, '.codegraph', 'graph.db')); + expect(edges).toContainEqual({ + src: 'Session.check', + srcKind: 'method', + tgt: 'Session.isReady', + tgtKind: 'method', + kind: 'calls', + }); + }); + + it('attributes a bare `varName.prop` read to a same-file class getter', () => { + const edges = readCallEdges(path.join(projDir, '.codegraph', 'graph.db')); + expect(edges).toContainEqual({ + src: 'useRepo', + srcKind: 'function', + tgt: 'Repo.db', + tgtKind: 'method', + kind: 'calls', + }); + }); + + it('does not attribute either accessor of an ambiguous get+set property pair', () => { + const edges = readCallEdges(path.join(projDir, '.codegraph', 'graph.db')); + expect(edges.filter((e) => e.tgt === 'Toggle.flag')).toHaveLength(0); + }); + }); +} + +runScenario('wasm'); +describe.skipIf(!isNativeAvailable())('native engine coverage', () => { + runScenario('native'); +}); + +function makeStmts(db: ReturnType) { + return { + insertNode: db.prepare( + 'INSERT OR IGNORE INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)', + ), + getNodeId: { + get: (name: string, kind: string, file: string, line: number) => { + const id = getNodeIdQuery(db, name, kind, file, line); + return id != null ? { id } : undefined; + }, + }, + insertEdge: db.prepare( + 'INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?, ?, ?, ?, ?)', + ), + countNodes: db.prepare('SELECT COUNT(*) as c FROM nodes WHERE file = ?'), + countEdges: db.prepare( + 'SELECT COUNT(*) as c FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)', + ), + findNodeInFile: db.prepare( + "SELECT id, kind, file FROM nodes WHERE name = ? AND kind IN ('function', 'method', 'class', 'interface', 'type', 'struct', 'enum', 'trait', 'record', 'module', 'constant') AND file = ?", + ), + findNodeByName: db.prepare( + "SELECT id, file, kind FROM nodes WHERE name = ? AND kind IN ('function', 'method', 'class', 'interface', 'type', 'struct', 'enum', 'trait', 'record', 'module', 'constant')", + ), + listSymbols: db.prepare("SELECT name, kind, line FROM nodes WHERE file = ? AND kind != 'file'"), + upsertFileHash: db.prepare( + 'INSERT OR REPLACE INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)', + ), + deleteFileHash: db.prepare('DELETE FROM file_hashes WHERE file = ?'), + }; +} + +function runIncrementalParityScenario(engine: EngineMode): void { + describe(`incremental rebuild matches full build for accessor reads (#1893) — ${engine}`, () => { + let incDir: string; + let refDir: string; + + beforeAll(async () => { + incDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1893-inc-${engine}-`)); + writeFixture(incDir); + await buildGraph(incDir, { engine, incremental: false, skipRegistry: true }); + + // Touch the file and rebuild it through the single-file incremental path. + const filePath = path.join(incDir, 'accessor.ts'); + fs.appendFileSync(filePath, '\n// touched\n'); + const dbPath = path.join(incDir, '.codegraph', 'graph.db'); + const db = openDb(dbPath); + try { + initSchema(db); + const stmts = makeStmts(db); + await rebuildFile(db, incDir, filePath, stmts, { engine }, null); + } finally { + db.close(); + } + + refDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1893-ref-${engine}-`)); + writeFixture(refDir); + fs.appendFileSync(path.join(refDir, 'accessor.ts'), '\n// touched\n'); + await buildGraph(refDir, { engine, incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(incDir, { recursive: true, force: true }); + fs.rmSync(refDir, { recursive: true, force: true }); + }); + + it('produces the same accessor-read call edges as a full rebuild', () => { + const incremental = readCallEdges(path.join(incDir, '.codegraph', 'graph.db')); + const reference = readCallEdges(path.join(refDir, '.codegraph', 'graph.db')); + expect(incremental).toEqual(reference); + expect(incremental).toContainEqual({ + src: 'Session.check', + srcKind: 'method', + tgt: 'Session.isReady', + tgtKind: 'method', + kind: 'calls', + }); + }); + }); +} + +runIncrementalParityScenario('wasm'); +describe.skipIf(!isNativeAvailable())('native engine incremental parity coverage', () => { + runIncrementalParityScenario('native'); +}); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index a7288292..6c3ee6ff 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -2407,4 +2407,124 @@ function runDemo(users: string[]): void { ); }); }); + + describe('ES6 getter/setter same-file property-read call attribution (#1893)', () => { + function parseJS(code) { + const parser = parsers.get('javascript'); + const tree = parser.parse(code); + return extractSymbols(tree, 'test.js'); + } + + function parseTS(code) { + const parser = parsers.get('typescript'); + const tree = parser.parse(code); + return extractSymbols(tree, 'test.ts'); + } + + it('attributes a bare `this.prop` read to the same-class getter', () => { + const symbols = parseJS(` + class Session { + get isReady() { return this._ready; } + check() { + if (this.isReady) { report(); } + } + } + `); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'isReady', receiver: 'this' }), + ); + }); + + it('attributes a bare `varName.prop` read to a same-file class getter via typeMap', () => { + const symbols = parseTS(` + class Repo { + get db() { return this._db; } + } + function useRepo(repo: Repo) { + return repo.db; + } + `); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'db', receiver: 'repo' }), + ); + }); + + it('attributes a plain-assignment write to the same-class setter', () => { + const symbols = parseJS(` + class Toggle { + set flag(v) { this._f = v; } + reset() { this.flag = false; } + } + `); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'flag', receiver: 'this' }), + ); + }); + + it('does not attribute a getter-only read as a call to the setter (no setter declared)', () => { + const symbols = parseJS(` + class Repo { + get db() { return this._db; } + check() { return this.db; } + } + `); + const dbCalls = symbols.calls.filter((c) => c.name === 'db'); + expect(dbCalls).toHaveLength(1); + }); + + it('skips a property with both a getter and a setter (ambiguous target)', () => { + const symbols = parseJS(` + class Toggle { + get flag() { return this._f; } + set flag(v) { this._f = v; } + flip() { this.flag = !this.flag; } + } + `); + expect(symbols.calls.filter((c) => c.name === 'flag')).toHaveLength(0); + }); + + it('does not attribute a real method call to a bare-read even when the name matches an accessor', () => { + const symbols = parseJS(` + class Widget { + get value() { return this._v; } + } + function useWidget(w) { + return w.value(); + } + `); + // The call-callee occurrence must still be handled by the regular call + // path (name='value', receiver='w') exactly once — not duplicated by + // the accessor-read collector. + expect(symbols.calls.filter((c) => c.name === 'value' && c.receiver === 'w')).toHaveLength(1); + }); + + it('does not attribute a plain (non-accessor) same-name method reference as a call', () => { + const symbols = parseJS(` + class Widget { + render() { return 1; } + } + function useWidget(w) { + const fn = w.render; + return fn; + } + `); + expect(symbols.calls).not.toContainEqual( + expect.objectContaining({ name: 'render', receiver: 'w' }), + ); + }); + + it('recognizes a static get/set accessor the same way as an instance accessor', () => { + const symbols = parseJS(` + class Config { + static get version() { return Config._v; } + static describe() { + return this.version; + } + } + `); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'version', receiver: 'this' }), + ); + }); + }); });