Skip to content

fix(references): discover class-member references through inferred receiver types - #324

Merged
Hessesian merged 15 commits into
mainfrom
worktree-inferred-receiver-refs
Sep 23, 2026
Merged

Hessesian merged 15 commits into
mainfrom
worktree-inferred-receiver-refs

Conversation

@Hessesian

Copy link
Copy Markdown
Owner

Summary

textDocument/references on a field, doubly-nested method, or interface method returned zero results when every real usage reached the member through a variable whose type is only known via transitive inference (val x = repository.openBody(); x.isOnline), because candidate-file discovery required the file to textually mention the owning class/interface name before it was even considered.

Real-world repro: a data-class field used across 6 files in a large Android monorepo, none of which imported or named the declaring class — find-references from the field's declaration returned nothing.

Fix

Adds a bounded "producer hop" to src/rg.rs's discovery stage: after the existing owner-name text scan, also look in those files for members whose declared return type is the owner class (a "producer"), then scan for callers of those producer names. verify_candidates (the existing receiver-type-based post-hoc filter) is untouched and is the correctness backstop for anything discovery over-includes. Two bounded caps ensure the mechanism degrades to today's exact behavior — never a wider unscoped scan — when it can't confidently narrow.

Applied to all three affected discovery paths: field_scoped_reference_locations, owner_scoped_reference_locations (doubly-nested methods), parent_scoped_reference_locations (interface methods).

Deliberately revises the 6b find-references design's "never touch rg's discovery scope" non-goal — that was scoped to one verification-layer slice, never declared permanent, and 6b's own verify machinery is exactly what makes this safe (it needs zero changes to absorb a wider candidate set).

Full design + rationale: docs/superpowers/plans/2026-09-22-inferred-receiver-reference-discovery-plan.md

Verification

  • cargo test: 1984 passed / 0 failed / 3 ignored (main binary) + all integration-test binaries green
  • cargo clippy --all-targets -- -D warnings: clean
  • Real-world measurement against /home/ocel/Work/Moneta/android: isOnline find-references → 9 references in 66.9ms
  • Built via subagent-driven-development: 2 implementer tasks + 2 task reviews (both Approved) + 1 whole-branch review (found 3 Important findings — uppercase-nested false-positive surface, missed Owner.Nested producer shape, perf deviations from the plan's specified mechanism) + 1 fix round + clean scoped re-review

Known follow-up (out of scope, not fixed here)

Indexer::enclosing_class_at (src/indexer/scope.rs) can't resolve the enclosing class for a primary-constructor property (data class Foo(val bar: Baz)) — only body-declared members — so verify_candidates's type-based rejection silently never fires for find-references/rename launched from that common declaration shape. Verified not to affect this PR's fix (discovery uses a different, shape-agnostic check), but real and worth its own investigation.

Test plan

  • field_reference_found_through_inferred_receiver_type — the repro, with 3 competing decoys
  • top_level_function_reference_stays_package_scoped — locks the ruled-out widening decision
  • usage_site_field_reference_finds_sibling_usages — characterizes the separate unscoped usage-site path
  • owner_scoped_method_reference_found_through_inferred_receiver_type
  • interface_method_reference_found_without_importing_the_interface
  • Regression + unit tests added during the fix round (uppercase-nested gate, dotted-segment producer matching, multi-producer alternation search)

🤖 Generated with Claude Code

https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF

Hessesian and others added 6 commits September 22, 2026 15:28
Task 1/Task 2 headings (matching the repo's SDD task-brief convention)
and a Global Constraints section, split out of the prose.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
The task-brief script extracts only the contiguous text under each
"### Task N" heading; the standalone Test plan section (after both
task headings) was invisible to both briefs. Moved each task's test
specs inline, left a pointer where the full section was.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…types

textDocument/references on a data-class field returned zero results when
every real usage reached it through a variable whose type is only known via
transitive inference (val x = repository.openBody(); x.isOnline), because
field_scoped_reference_locations only considered files that textually
mention the owning class name.

Add a hop-2 discovery pass: scan hop-1 candidate files for a producer
declaration (a fun/val/Java method whose explicit type is the owner class),
then widen the candidate file set to files that call that producer's member
name. Scoped exactly like every other rg pass here, capped at 8 producer
names / 256 additional files, and degrades to today's exact hop-1-only
behavior (never wider) when either cap is exceeded or no producer is found.

verify_candidates (the CST-based correctness backstop) is unchanged --
widening discovery is safe because it already filters candidates by receiver
type, independent of file text.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…-scoped discovery

Widens owner_scoped_reference_locations and parent_scoped_reference_locations
with the same hop-2 producer-file discovery Task 1 added for field/property
declaration-site queries, so a doubly-nested method (Reducer.Factory.create)
or an interface method (Repo.openBody) is found even when every caller
reaches it only through a variable whose type is known via transitive
inference, never naming the owning class/interface textually.

owner_scoped_reference_locations needed per-file provenance (a new
ProducerDiscoveredFiles set) because it applies qualifier_hints_owner, which
would otherwise reject every hop-2 hit (the call-site qualifier is named
after the intermediate producer, not the owner class) — the heuristic stays
applied for any file hop 1 already found, so an unrelated same-named call in
a hop-1 file stays excluded. parent_scoped_reference_locations needed no such
tracking: its bare-name pass only gates on qualifier for uppercase type
names, and verify_candidates remains the backstop, same as every other hop-2
widening in this module.

Both caps introduced in Task 1 (MAX_OWNER_PRODUCING_MEMBER_NAMES,
MAX_PRODUCER_CANDIDATE_FILES) continue to degrade to today's exact hop-1
behavior when exceeded — no unscoped workspace scan is introduced here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
- Gate parent_scoped_reference_locations's producer hop on
  !is_uppercase_nested: an uppercase nested type can never be referenced
  bare without an import, so widening the bare-name candidate set for one
  could only ever waste verify_candidates's IO budget.
- Teach type_annotation_matches_owner the Owner.Nested and pkg.Owner dotted
  shapes, so a member declared to return a nested/fully-qualified type is
  recognized as a producer of the outer/short owner class. Fix test B's
  fixture, which previously passed by an unrelated accidental
  self-production coincidence rather than exercising this shape.
- producer_scoped_candidate_files: skip per-line parsing when the line
  doesn't even contain owner_class, and issue one alternation-pattern rg
  call for all producer names instead of one call per name.

Verified Moneta ground-truth measurement (isOnline field, real
~/Work/Moneta/android checkout): 9 references found in 66.9ms.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…CST data

Follow-up to PR #324 per review: declared_member_name_returning read raw
source and hand-scanned it line-by-line, duplicating what SymbolEntry.detail
(extracted from the CST once at parse time) already provides.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved correctness, filtering, parsing, and bounded-discovery issues remain in src/rg.rs.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 5 Medium severity

Open (6)
What changed in this PR

This PR expands reference discovery for members accessed through inferred receiver types.

Changes:

  • Adds bounded producer-hop discovery for fields, nested methods, and interface methods.
  • Adds parser, regression, gating, and end-to-end tests.
  • Documents the design and rationale.
File Summary
src/​rg.rs Implements producer-hop discovery. Requires changes for parsing coverage, false-positive filtering, cap enforcement, path normalization, and declaration identity.
src/​rg_tests.rs Adds producer parsing and discovery unit tests.
src/​features/​references_tests.rs Adds end-to-end inferred-receiver reference tests.
docs/​superpowers/​plans/​2026-09-22-inferred-receiver-reference-discovery-plan.md Documents the implementation plan and tradeoffs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/rg.rs Outdated
Comment thread src/rg.rs Outdated
Comment thread src/rg.rs Outdated
Comment thread src/rg.rs Outdated
Comment thread src/rg.rs Outdated
Comment thread src/rg.rs Outdated
Hessesian and others added 4 commits September 22, 2026 17:03
…d CST data

PR #324's hand-rolled `declared_member_name_returning` and its three
shape-scanners (`kotlin_function_returning`, `kotlin_property_typed`,
`java_method_returning`) read raw file content and hand-scanned it line by
line to detect whether a declaration's return/declared type equals the
owner class -- exactly the string-parsing-where-the-CST-should-be pattern
AGENTS.md warns against. The data was already available, pre-extracted:
`SymbolEntry::detail` (computed once at parse time via CST-bounded
`extract_detail_from_node`) plus `Indexer::files` for per-file symbol
enumeration.

Replace the scanners with `declared_type_from_detail`, which extracts a
type token from an already-normalized `detail` string, gated on
`SymbolKind` rather than re-sniffing the shape. `Indexer` access isn't
available inside the `spawn_blocking` rg pass, so `(file_uri, member_name)`
producer candidates are pre-computed in `rg_locations` before the blocking
closure (same pattern already used for `index_candidates`), then
intersected against hop-1 files inside `producer_scoped_candidate_files`.

Found empirically (a real fixture failure, not anticipated by the plan):
`SymbolKind::METHOD` covers BOTH a Kotlin member function nested inside a
class/interface/object (nesting demotes `FUNCTION` to `METHOD`) and a
genuine Java method -- disambiguated by whether `detail` carries the
literal `fun` keyword, since both land on the same `SymbolKind`.

Two existing integration tests had incomplete fixtures (only the
declaration file was indexed, not the hop-1 producer file) that the old
filesystem-read implementation silently papered over; fixed to index
every file, mirroring a real workspace scan -- their expected assertions
are unchanged.

Re-verified against a real Moneta checkout: byte-for-byte identical
find-references output (8 locations) before and after this rework on the
same harness -- PR #324's own "9" figure was against a harness mechanism
that wasn't recoverable from git history (never committed), so the
apples-to-apples comparison is before/after on this session's harness,
not the literal historical number.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
Four findings from task + independent review of commit c807eb6, all fixed
in one commit:

1. Unscoped full-index (+ JAR) scan on a hot reference-search path.
   `rg_locations`'s producer_candidates precompute walked EVERY indexed
   file via `for_each_indexed_file`, including `jar_files` -- a JAR symbol
   can never be a hop-1 candidate (hop1_files only ever comes from an rg
   scan over workspace source files), so visiting it was pure wasted cost
   with zero payoff, and on a JAR-heavy Android project that map can be
   very large. Added `Indexer::for_each_indexed_workspace_file` (files
   only, no jar_files) plus `file_uri_under_source_paths` to further scope
   iteration to configured source roots when set, matching every other rg
   pass's own scoping.

2. `MAX_DETAIL_CHARS` (120) truncation silently drops the return-type tail
   for a long-enough Dagger/Hilt-shaped producer signature. Rather than
   changing `SymbolEntry::detail`'s own truncation (which other consumers
   depend on in its exact current shape -- confirmed by 4 existing tests
   that broke under an earlier attempt to smarten the truncation globally,
   reverted), added a narrow fallback (`declared_type_from_raw_lines`)
   that re-derives the type from the symbol's own raw source lines only
   when `detail` was truncated and the primary extraction failed.

3. `parent_scoped_reference_locations`'s hop-2 widening didn't track
   per-file provenance (unlike `owner_scoped_reference_locations`, which
   already does via `ProducerDiscoveredFiles`), so a file reached ONLY
   through hop 2 could have its own unrelated same-named top-level
   declaration counted as a false-positive reference --
   `verify_candidates` can't catch this since a top-level declaration has
   no enclosing class to check against. Applied the same
   `ProducerDiscoveredFiles` provenance tracking.

4. `MAX_PRODUCER_CANDIDATE_FILES` was compared against the raw rg match
   count, before `filter_candidate_files` and before subtracting hop-1
   overlap -- could trip the cap and disable widening entirely even when
   the real new-and-usable file count was comfortably under it. Now
   compared against the filtered, hop-1-subtracted count.

Also: `producer_candidates` is now a named `ProducerCandidate` struct
(file_uri, member_name) instead of a `(String, String)` tuple, and the
Kotlin/Java `METHOD` disambiguation's theoretical misfire (a Java
method/param literally named `fun`) is now documented.

Every fix has a dedicated red/green test (verified via a temporary revert
of just that fix, confirming failure, then restoring it). Re-verified
against the real Moneta checkout: byte-for-byte identical output (8
locations) before and after this fix round, on top of the earlier CST
rework's own already-established baseline.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…discovery

The actual reported bug (find-references on a data-class field like
ApplicationOpenResponseBody.isOnline) was still broken after every prior
fix round in this branch, because every verification step substituted a
different, similarly-shaped field (NetUtil.kt's NetworkInfo.isOnline)
instead of re-running the literal original repro.

Root cause: synthesize_data_class_copy generates a copy(): Self SymbolEntry
for every Kotlin data class. Producer discovery treated it as a legitimate
producer signal, but "copy" as a bare-word rg pattern matches over a
thousand files in a real ~18k-file Android monorepo — enough alone to
exceed MAX_PRODUCER_CANDIDATE_FILES and discard hop 2's entire merged
alternation search, including the real producer name found alongside it.

None of this branch's existing fixtures used a data class (they used plain
`class` to sidestep an unrelated enclosing_class_at gap), so no test ever
exercised a real copy() synthesis until this fix's own regression test.

Verified against the real external project the bug was originally reported
against: 0 -> 8 locations for the exact repro (ApplicationOpenResponseBody
.isOnline), matching the original manual investigation exactly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…ynthesis

Same-session review of the copy() exclusion found the identical mechanism
unaddressed for enum-synthesized values()/valueOf()/entries: measured on
the real Moneta corpus, that three-name alternation alone matches 557
files (2.2x MAX_PRODUCER_CANDIDATE_FILES), silently discarding hop 2 for
every field on any enum class. Also found a live gap in the copy()
exclusion itself: it was gated to SymbolKind::FUNCTION, but nesting
demotes FUNCTION to METHOD, so a hand-written class member named `copy`
(a real instance exists in the corpus) reopened the original hole.

is_data_class_synthetic_copy -> is_unusable_producer_name: name-only
match (copy/values/valueOf/entries), no kind gating, on the same
reasoning that already justified excluding copy - a name this common as
a bare rg pattern is an unusable discovery signal regardless of origin.

New regression test mirrors the data-class one for the enum case
(260 decoy files calling .values(), confirmed red without the fix).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/rg.rs Outdated
Comment thread src/features/references.rs
Comment thread src/rg.rs Outdated
Comment thread src/rg.rs
Hessesian and others added 4 commits September 23, 2026 16:23
…ing CST parser

Fixes 4 real findings from Copilot's automated review of PR #324:
1. kotlin_function_return_type/java_method_return_type broke on a leading
   annotation with its own parenthesized argument list
   (@nAmed("body") fun provideBody(): Body) - the first `(` in `detail`
   landed on the annotation's own parens, not the parameter list's.
2. ProducerDiscoveredFiles compared a percent-encoded Url::path()/as_str()
   against raw filesystem paths - silently failed the membership check on
   workspace paths with spaces or non-ASCII characters, wrongly rejecting
   genuine hop-2 references via qualifier_hints_owner.
3. file_uri_under_source_paths had no fallback for the case build_command
   already handles (every configured sourceRoots entry missing/stale) -
   rg itself falls back to the whole workspace root in that scenario, but
   the producer precompute silently found nothing, disabling hop 2.
4. (unscoped-precompute-cost finding) - investigated via an Opus brainstorm
   rather than patched inline; see below.

While fixing (1), user pushback ("why aren't we using CST again") led to
finding this codebase ALREADY has a CST-derived return-type parser
(extract_return_type_from_detail / extract_property_type_from_detail,
src/resolver/infer_lines.rs) used by the Resolver trait's own
function_return_type/method_return_type across ~10 call sites - and it's
MORE correct than what rg.rs had (correctly handles a `)` inside a string
default value, e.g. `fun split(separator: String = ")"): Body`, which the
rg.rs version did not and could not without the same fix applied twice).
Deleted kotlin_function_return_type and take_type_token entirely; rg.rs's
declared_type_from_detail now delegates to the existing resolver parsers
for the Kotlin shapes. java_method_return_type (Java has no existing
equivalent) stays, with the same annotation-skipping fix applied via a new
balanced_paren_open_backward helper.

Also dispatched an Opus brainstorm on whether SymbolEntry should gain a
real indexed return_type field (the "actual CST-first" alternative to
string-parsing detail at all) - verdict: no. Cold-bundling would break the
99.1% joint-sparsity invariant PR #212's memory diet depends on; hot-adding
costs ~18-26MB on a 740k-symbol corpus; JAR-sourced symbols wouldn't
populate it (the sidecar emits no return type), making it a third source
of truth instead of a replacement; and it forces a CACHE_VERSION bump
(full reindex for every user) for one consumer's benefit. Full analysis
not committed here, reported to the user directly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
Review of the CST-delegation commit found a real regression: delegating
PROPERTY/VARIABLE detail parsing straight to extract_property_type_from_detail
silently dropped producer detection for any property with a modifier that
function doesn't strip - override, lateinit, const, or a leading annotation.
@Inject lateinit var repository: Repository (the canonical Dagger/Hilt
field-injection shape this whole feature targets) returned None.

Fixed narrowly in rg.rs's own PROPERTY|VARIABLE arm by anchoring on the
val /var  keyword itself before delegating - same strategy already used
for the FUNCTION arm's `fun ` anchor - rather than touching the shared
resolver function, which has ~10 other existing callers with their own
established behavior. That function has the identical blind spot today;
flagged as a separate, out-of-scope follow-up, not fixed here.

Also: resolve_effective_source_paths moved inside the producer_owner guard
(was computed unconditionally, wasted work when no producer scan runs).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
…nder_source_paths tests

These 4 tests (added in an earlier fix-round commit on this branch)
hardcoded /workspace/... absolute paths and passed them to
Url::from_file_path, which panics on Windows for a driveless path -
this branch's first real Windows CI run caught it. Switched to
tempfile::tempdir(), matching every other cross-platform test in this
file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
@Hessesian
Hessesian merged commit faaf08a into main Sep 23, 2026
4 checks passed
@Hessesian
Hessesian deleted the worktree-inferred-receiver-refs branch September 23, 2026 14:47
@Hessesian Hessesian mentioned this pull request Sep 23, 2026
2 of 3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants