Repository navigation
fix(references): discover class-member references through inferred receiver types - #324
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF
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
There was a problem hiding this comment.
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
Open (6)
Filter hop-2 declaration hits by provenance · New Recognize producers with receivers and type parameters · New Exclude local typed variables from producer discovery · New Parse multiline producer declarations · New Stop processing after the producer-name cap is exceeded · New Apply candidate filtering before enforcing the file cap · New
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.
…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
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings affect correctness, performance, and language/source coverage.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (4)
Resolved since last review (6)
Filter hop-2 declaration hits by provenance Apply candidate filtering before enforcing the file cap Stop processing after the producer-name cap is exceeded Parse multiline producer declarations Exclude local typed variables from producer discovery Recognize producers with receivers and type parameters
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
textDocument/referenceson 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.mdVerification
cargo test: 1984 passed / 0 failed / 3 ignored (main binary) + all integration-test binaries greencargo clippy --all-targets -- -D warnings: clean/home/ocel/Work/Moneta/android:isOnlinefind-references → 9 references in 66.9msOwner.Nestedproducer shape, perf deviations from the plan's specified mechanism) + 1 fix round + clean scoped re-reviewKnown 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 — soverify_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 decoystop_level_function_reference_stays_package_scoped— locks the ruled-out widening decisionusage_site_field_reference_finds_sibling_usages— characterizes the separate unscoped usage-site pathowner_scoped_method_reference_found_through_inferred_receiver_typeinterface_method_reference_found_without_importing_the_interface🤖 Generated with Claude Code
https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF