Deferred from PR #2031 review (fix(extractors): attribute same-file ES6 getter/setter property reads as call edges).
Original reviewer comment: Greptile summary body (no separate inline comment) — #2031 (comment) ("Confidence Score: 5/5... The only open issue is that the registry does not distinguish static vs instance accessors, which can produce a spurious call edge in an unusual code pattern... worth addressing in a follow-up alongside the cross-file accessor work tracked in #2030.")
The bug
collectLocalAccessors/collect_local_accessors's registry keys purely by ClassName.propName -> {get, set}, with no static/instance distinction. this inside an instance method never refers to the class/constructor object (where static members live) — only this inside a static method does. If a class declares only a static get X() (no instance accessor of that name), a bare this.X read inside an instance method gets incorrectly matched against the static accessor and produces a false-positive calls edge (and the mirror case — an instance-only accessor read from a static method context — has the same gap in reverse).
Confirmed empirically, identically on both engines:
export class Config {
static get version(): string { return Config._v; }
static _v = '1.0';
// this refers to the Config INSTANCE here, not the class object — this.version
// does not invoke the static getter at runtime (no instance accessor/property
// named `version` exists), yet codegraph reports Config.describe -> Config.version.
describe(): string {
return this.version;
}
}
codegraph query Config.version (both --engine wasm and --engine native) shows a Config.describe -> Config.version call edge, which is wrong.
Note: the correct static-to-static case already has passing coverage (recognizes_static_accessor_same_as_instance in both tests/parsers/javascript.test.ts and crates/codegraph-core/src/extractors/javascript.rs's test module) — that test uses a static describe() caller, where this legitimately refers to the class object. The bug is specifically the instance-caller-reads-static-only-accessor (and reverse) case, which has no coverage.
Why deferred rather than fixed inline in #2031
Fixing this requires:
- Extending
LocalAccessorInfo/the registry to track static and instance accessor kinds separately ({get, set, staticGet, staticSet} or equivalent), which in turn requires getMethodAccessorKind/get_method_accessor_kind to also detect the static modifier token (tree-sitter represents static as another literal unnamed child, same pattern as get/set).
- New logic at the read site to determine whether the enclosing method containing a
this.prop access is itself static or instance (walk up to the nearest enclosing method_definition and check its modifiers) — the varName.prop (non-this) branch can be assumed instance-only for now, since typeMap only ever records "variable holds an instance of type X", never "variable holds the class object X itself".
- Mirrored in both engines, plus new tests for all four static/instance × get/set combinations.
This is a real, self-contained increment to the registry's precision (not a cross-cutting change like the separately-filed #2085 this-binding scope issue), but it's still a distinct enough chunk of new logic + new engine-parity surface that it doesn't belong bolted onto #2031's already-large diff — consistent with Greptile's own recommendation to bundle it with the cross-file accessor work already tracked in #2030.
Suggested fix direction
- Track static-ness per accessor kind in the registry (4-state: instance-get, instance-set, static-get, static-set).
- At the
this.prop read site, resolve whether the containing method is static (walk to nearest enclosing method_definition, check for a static modifier child) and match only against the corresponding static/instance bucket.
- Add regression tests: instance-caller reading a static-only accessor (should produce no edge), static-caller reading an instance-only accessor (should produce no edge), and confirm the existing static-to-static and instance-to-instance cases keep passing.
Deferred from PR #2031 review (fix(extractors): attribute same-file ES6 getter/setter property reads as call edges).
Original reviewer comment: Greptile summary body (no separate inline comment) — #2031 (comment) ("Confidence Score: 5/5... The only open issue is that the registry does not distinguish static vs instance accessors, which can produce a spurious call edge in an unusual code pattern... worth addressing in a follow-up alongside the cross-file accessor work tracked in #2030.")
The bug
collectLocalAccessors/collect_local_accessors's registry keys purely byClassName.propName -> {get, set}, with no static/instance distinction.thisinside an instance method never refers to the class/constructor object (wherestaticmembers live) — onlythisinside a static method does. If a class declares only astatic get X()(no instance accessor of that name), a barethis.Xread inside an instance method gets incorrectly matched against the static accessor and produces a false-positivecallsedge (and the mirror case — an instance-only accessor read from a static method context — has the same gap in reverse).Confirmed empirically, identically on both engines:
codegraph query Config.version(both--engine wasmand--engine native) shows aConfig.describe -> Config.versioncall edge, which is wrong.Note: the correct static-to-static case already has passing coverage (
recognizes_static_accessor_same_as_instancein bothtests/parsers/javascript.test.tsandcrates/codegraph-core/src/extractors/javascript.rs's test module) — that test uses astatic describe()caller, wherethislegitimately refers to the class object. The bug is specifically the instance-caller-reads-static-only-accessor (and reverse) case, which has no coverage.Why deferred rather than fixed inline in #2031
Fixing this requires:
LocalAccessorInfo/the registry to track static and instance accessor kinds separately ({get, set, staticGet, staticSet}or equivalent), which in turn requiresgetMethodAccessorKind/get_method_accessor_kindto also detect thestaticmodifier token (tree-sitter representsstaticas another literal unnamed child, same pattern asget/set).this.propaccess is itself static or instance (walk up to the nearest enclosingmethod_definitionand check its modifiers) — thevarName.prop(non-this) branch can be assumed instance-only for now, since typeMap only ever records "variable holds an instance of type X", never "variable holds the class object X itself".This is a real, self-contained increment to the registry's precision (not a cross-cutting change like the separately-filed #2085
this-binding scope issue), but it's still a distinct enough chunk of new logic + new engine-parity surface that it doesn't belong bolted onto #2031's already-large diff — consistent with Greptile's own recommendation to bundle it with the cross-file accessor work already tracked in #2030.Suggested fix direction
this.propread site, resolve whether the containing method is static (walk to nearest enclosingmethod_definition, check for astaticmodifier child) and match only against the corresponding static/instance bucket.