fix(resolve): stop leaking a sibling class's member across a JAR's synthetic file boundary by Hessesian · Pull Request #315 · Hessesian/kmp-lsp

Hessesian · 2026-09-09T16:51:06Z

Summary

Real bug behind navController.navigate(route = "...") never resolving on a real Android corpus: androidx.navigation's NavController.navigate is a real overloaded JVM member (not an extension), and NavHostController extends NavController without declaring its own navigate. Both classes compile into one synthetic per-JAR FileData.

Two compounding bugs in the JAR-derived member lookup:

find_name_in_uri_after_line's final fallback matched the closest same-named symbol "at or after" a container's line with no container check at all — for a JAR packing several classes into one file, this mis-attributed NavController's own navigate overloads to NavHostController purely by file position, corrupting resolve_qualified into treating an inherited member as an own, wrong-arity one.
resolve_from_class_hierarchy_scoped's ancestor walk called the single-result, arity-blind find_name_in_uri per ancestor instead of the container- and overload-aware find_all_names_scoped_to_container, so even reaching the right ancestor only ever returned one arbitrary overload.

find_name_in_uri_after_line now takes an optional container name and only trusts a symbol's real container tag over file position; a new find_all_names_with_container_in_uri helper lets the hierarchy walk look up every overload scoped to the ancestor's class name directly, without first needing to resolve that ancestor's own declaration Location.

Full research trail (including a shelved, independently-reviewed LatencyClass/JAR-promotion-budget architecture effort that turned out to be aimed at the wrong target for this specific gap) is documented at lsp_tasks/2026-09-09-navhostcontroller-navigate-container-scoping-and-latency-budget-research.md for anyone who wants the context.

Test plan

New regression test modeling the exact NavHostController/NavController same-JAR-file shape with a wrong-arity decoy + matching overload
Full suite: 1902/1902 passing (this branch's base), 0 failures
cargo clippy --all-targets --all-features -- -D warnings: clean
Verified on the real Moneta corpus: navigate — 220-224 occurrences in the resolution-accuracy Gap top-20 → 0, gone from the list entirely

🤖 Generated with Claude Code

https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF

Copilot

🟢 Approval recommended

The changes are targeted to the reported resolution failure mode and are backed by a dedicated regression test, with only a minor test-comment mismatch noted.

Pull request overview

Fixes JAR-derived member resolution where multiple classes compile into a single synthetic per-JAR FileData, preventing sibling-class members (and wrong overloads) from being incorrectly attributed during inherited-member lookup.

Changes:

Add container-aware, overload-preserving ancestor lookup during resolve_from_class_hierarchy_scoped by using a new find_all_names_with_container_in_uri helper.
Harden find_name_in_uri_after_line to optionally respect a known container name and avoid position-only sibling leakage.
Add a regression test modeling the NavHostController → NavController.navigate(...) same-JAR-file overload scenario.

File summaries

File	Description
src/resolver/tests.rs	Adds a regression test reproducing the same-JAR-file inherited overload selection bug.
src/resolver/resolve.rs	Updates hierarchy walk collection to prefer container-scoped overload sets for ancestors.
src/resolver/find.rs	Adds `find_all_names_with_container_in_uri` and extends `find_name_in_uri_after_line` with optional container scoping.
src/resolver/find_tests.rs	Updates test helper call to match the new `find_name_in_uri_after_line` signature.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

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

+                // Wrong-arity decoy overload -- real `navigate(Int)` shape
+                // (androidx's own 0-required-arg-past-receiver overload
+                // family). Declared BEFORE the matching overload, same as
+                // real ASM-derived JAR output can produce in either order.


… fall back to the root Copilot review finding on #317: `Outer.Missing.member` -- `Outer` resolves but `Missing` names no real nested type of it -- fell through to the declaration-less anchor fallback keyed on `root` ("Outer") whenever the nested walk produced zero anchors, regardless of why it was empty. That silently resolved `member` against `Outer` itself, as if `.Missing` had never been written. The fallback is now reserved for a genuinely unresolved root (no indexed declaration at all); a resolved root whose nested segment failed yields no anchors. Also fixes a Copilot finding on #315: a decoy-overload test comment said `navigate(Int)` but the fixture encodes a 0-arg overload via `param_counts: (0, 0)` -- text now matches what's actually asserted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF

…easured gap fixes (#317) * fix(resolve): stop leaking a sibling class's member across a JAR's synthetic file boundary Real bug behind navController.navigate(route = "...") never resolving on a real Android corpus: androidx.navigation's NavController.navigate is a real overloaded JVM member (not an extension), and NavHostController extends it without declaring its own navigate. Both classes compile into one synthetic per-JAR FileData. Two compounding bugs in the JAR-derived member lookup: 1. find_name_in_uri_after_line's final fallback matched the closest same-named symbol "at or after" a container's line with no container check at all -- for a JAR packing several classes into one file, this mis-attributed NavController's own navigate overloads to NavHostController purely by file position, corrupting resolve_qualified into treating an inherited member as an own, wrong-arity one. 2. resolve_from_class_hierarchy_scoped's ancestor walk used the single- result, arity-blind find_name_in_uri per ancestor instead of the container- and overload-aware find_all_names_scoped_to_container, so even reaching the right ancestor only ever returned one arbitrary overload. find_name_in_uri_after_line now takes an optional container name and only trusts a symbol's real container tag over position; a new find_all_names_with_container_in_uri helper lets the hierarchy walk look up every overload scoped to the ancestor's class name directly, without first needing to resolve that ancestor's own declaration Location. Verified on the real Moneta corpus: navigate goes from 220-224 occurrences in the resolution-accuracy Gap top-20 to zero. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * fix(resolve): resolve a Java getter as the Kotlin synthetic property it exposes Kotlin's Java-interop synthetic-property rule: `obj.foo` retries as `obj.getFoo()` when `foo` names no real member and the declaring file is Java. Implements the read-only, getFoo-only slice of that rule inside resolve_qualified's uppercase-branch member tier, after the real member/ inherited-member lookups (now unified behind member_or_inherited_member) return empty and before the supertype-extension tier. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * fix(resolve): match the JAR-extension probe's fallback by declaration, not name alone, and de-duplicate the star-import package list and default-import constant resolve_qualified's item-1 JAR-extension fallback materialized its Location by finding the first symbol in the declaring file whose bare name matched, not the actual extension declaration -- so a same-named, unrelated symbol elsewhere in the file could win, returning the wrong range. Fixed by matching through extension_declaration_matches (receiver + container), the same helper the sibling in-scope lookup already uses. The fallback stays deliberately fail-open (no extension_is_in_scope check) -- that behavior is load-bearing for resolve_extension_fn_on_uppercase_qualifier and is now guarded by its own regression test. Also de-duplicates two small helpers that straddle the resolve.rs/ package_scope.rs boundary planned for the next task: a single star_import_packages() replaces three copies of the same star-import filter, and is_default_import_package() now reads KOTLIN_DEFAULT_IMPORT_PACKAGES instead of carrying its own identical 10-entry list. Corpus measurement (resolution-accuracy /home/ocel/Work/Moneta/android): member recall 90.8% (150626/165876), no regression from the most recent known baseline (90.3%, 149718/165795) -- expected, since item 1 is a goto-definition precision fix the benchmark doesn't score and items 2-3 are behavior-preserving deduplications. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * refactor(resolver): split resolve.rs into a spine and nine named siblings Pure mechanical move, zero behavior change: resolve.rs shrinks from 2753 to 686 lines. Moved by resolution-order taxonomy into container.rs, package.rs, tie_break.rs, package_scope.rs, imports.rs, extension.rs, scope_check.rs, qualified.rs, and platform_types.rs, per the task-2 brief's module boundary table (moved innermost-first, compiling and testing after each module). Includes the brief's one documented exception: corrects the wrong comment in resolve_qualified (now in qualified.rs) claiming an "exact-key extension" had already been ruled out before the supertype-extension fallback — it hadn't; member_or_inherited_member's inherited-member half is a pure member lookup. 1910 tests pass before and after (0 failed, 3 ignored), clippy and fmt clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * refactor(resolve): give resolve_qualified one parse, one anchor, and one precedence ladder `resolve_qualified` implemented the same three-stage sequence twice -- parse the qualifier, normalize it to a receiver anchor, run a member/extension precedence ladder -- once for an uppercase (`Outer.Inner`) root and once for a lowercase (`variable.field`) root. Because the sequence was written twice with no shared boundary, the two ladders had drifted: a tier that exists on one branch and not the other, and an anchor re-derived per branch so an extension probe could stay keyed on the ROOT while the anchor had already moved to the leaf. The four stages are now named and separately testable: parse_qualifier -> QualifierRoot (no Indexer, no IO) anchors_for -> ReceiverAnchor (qualifier -> receiver, nothing else) candidates_on -> QualifiedCandidates (the four tiers) companion_member_on (a different question, kept separate) `ReceiverAnchor` carries the leaf type's full declaration `Location` -- file and range -- because `find_all_names_scoped_to_container` scopes its member search by the container's declaration range, not merely its file. It also carries the LEAF type's simple name, which makes "probe keyed on the root while the anchor has moved to `Inner`" unrepresentable rather than merely fixed. Deliberate, named behaviour correction (not a regression): the two branches did not merely differ in which tiers existed, they checked them in a DIFFERENT ORDER. The uppercase branch probed the own-type extension FIRST and returned early, before its member tier was ever computed; the lowercase branch ran members first and reached its extension tier last. One ladder can encode only one order, and the lowercase branch's is the correct one -- Kotlin resolves a real member over a same-named extension. The uppercase branch's extension-first check was the outlier and is corrected, not averaged. `resolve_qualified_uppercase_receiver_own_member_now_wins_over_own_type_extension` is the test for exactly this, and is red against the previous code. `resolve_imported_extension_preferred_over_member` encoded the old order and is inverted and renamed to `resolve_member_preferred_over_imported_extension`. Precedence is carried by candidate ORDER, not by refusing to compute a tier: a same-named member does not always satisfy the call's arity, so the own-type extension is appended below a winning member rather than dropped, matching what the supertype-extension tier has always done. Measured: short-circuiting it instead cost 791 `IMockProvider.loadJSONFromAssets` call sites (a 1-arg extension shadowed by a 2-arg member), which fell out of resolution into the ambiguous bucket. Moneta corpus (`resolution-accuracy`, before -> after): member recall 90.9% (150924/166021) -> 90.9% (150553/165558) Gap (member) 8214 -> 8121 FilteredCandidate 6883 -> 6884 No new name in either the Gap or FilteredCandidate top-20; remaining per-name movement is single-digit and within the tool's run-to-run variance, with `collectEmit` (-13) and `collectLatest` (-9) leaving the ambiguous bucket. Tests: 1957 -> 1964 (7 added, 0 failures). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * test(resolve): lock in the own-type extension tier's coverage post-Task-2b Task 2b's decomposition of resolve_qualified (parse_qualifier -> anchors_for -> candidates_on) already populates own_type_extension unconditionally, for both TypePath and ValuePath roots and every nesting depth, since both families normalize through the same ReceiverAnchor before the ladder runs. Task 3's planned ~20-line fix (jar-promotion-latency-budget-plan) is fully absorbed: all six cases from its brief (lowercase-root wrong-arity member, uppercase-root regression guard, member-wins-on-arity-match, supertype fallback still reached, and both finding-11 nested-type-qualifier exits) are green against today's code with no production change. These tests are the proof. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * fix(resolver): stop dropping the hierarchy walk's budget at the ambiguity-safe tail `supertype_targets` threads its own `sidecar_budget: &mut usize` into the per-hop `ensure_jar_definitions_for` promotion, but its two calls into `resolve_symbol_hierarchy_ambiguity_safe` (the nested-type "outer" resolution and the final leaf-name tail) never received it. That function's `resolve_chain` tail (`ResolveIo::HierarchyAmbiguitySafe`) reads via `indexer.lookup_definitions`, which internally promotes with a hardcoded zero budget -- so a supertype name resolvable only through this ambiguity- safe path, and living in a not-yet-materialized JAR, silently fell through to a zero-budget promotion even mid-walk with real budget remaining. Adds `sidecar_budget: &mut usize` to `resolve_symbol_hierarchy_ambiguity_safe` and spends it on a pre-promotion of `name` before falling into `resolve_chain`, threaded through from both `supertype_targets` call sites. Plain `&mut usize`, matching every other budget site in this codebase -- independent of the still-parked `LatencyClass`/`JarPromotionBudget` work. New test: hierarchy_walk_shares_its_budget_with_the_ambiguity_safe_tail, modeled on the promotion-counting harness at tests.rs:7568. Verified red against pre-fix code, green after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * test(resolve): split a two-assertion test to satisfy the no-"and" naming rule Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * fix(resolve): a missing nested-type segment must yield no anchor, not fall back to the root Copilot review finding on #317: `Outer.Missing.member` -- `Outer` resolves but `Missing` names no real nested type of it -- fell through to the declaration-less anchor fallback keyed on `root` ("Outer") whenever the nested walk produced zero anchors, regardless of why it was empty. That silently resolved `member` against `Outer` itself, as if `.Missing` had never been written. The fallback is now reserved for a genuinely unresolved root (no indexed declaration at all); a resolved root whose nested segment failed yields no anchors. Also fixes a Copilot finding on #315: a decoy-overload test comment said `navigate(Int)` but the fixture encodes a 0-arg overload via `param_counts: (0, 0)` -- text now matches what's actually asserted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF * fix(resolve): address PR #317 Copilot review findings Triaged all 10 findings from the automated review (1 posted inline, 9 suppressed but listed in the review body). Fixed 6, each with a red-before-fix-verified regression test: - companion_applies was gated on nested.is_empty(), so Outer.Inner.member() never reached Inner's own companion object even though anchors_for had already normalized the anchor down to Inner itself. - The last-resort JAR-extension fallback in resolve_qualified keyed on the qualifier's literal ROOT spelling ("Outer") instead of the leaf ("Inner") — extension_by_receiver keys by the receiver's own simple name. - supertype_extension was skipped whenever inherited_members was non-empty, with no regard for arity — the same bug own_type_extension was already fixed for a few lines above, on its sibling field. - The Java-getter synthetic-property retry checked the RECEIVER's own declaring-file language, rejecting a getter genuinely inherited from a Java ancestor whenever the receiver's own class is Kotlin-declared. Now checks each getter candidate's own declaring file. - resolve_from_class_hierarchy_scoped's per-ancestor closure fell through to an unscoped find_name_in_uri whenever the container-tagged lookup came up empty — but an empty result there usually just means "this ancestor has no member of this name" (normal while walking up), not "no container metadata exists". Reintroduced the exact cross-container leak this file's own container-tag scoping exists to prevent. - The denylist prefix check only matched a DESCENDANT package (trailing-dot starts_with), never the bare prefix package itself. Also fixed: default_kotlin_import_tie_break now gated on the origin file's own language — it was applying Kotlin's default-import set even from a Java origin, which can never implicitly import kotlin.*. Investigated but NOT changed (documented, not silently dropped): - package.rs's coarse whole-JAR package fallback (Copilot flagged it as imprecise for a multi-package JAR) — tried removing it, but that broke three real existing tests that specifically depend on it for JARs with no per-symbol package table. Reverted; the imprecision risk is real but this codebase's own tests show the fallback is the tested, intentional trade-off, not an oversight. - The line-scan fallback in find_name_in_uri_after_line isn't container-bounded even when container_name is known (only matters for un-indexed names, e.g. constructor params, colliding with a same-named sibling elsewhere in a degenerate JAR-stub file) — a correct fix needs the container's own end-line threaded through, which isn't available at every call site; left for its own investigation. - The Java-getter retry doesn't filter candidates to zero-arg overloads — needs arity data threaded through member_tiers's Vec<Location>, which currently loses it; left for its own investigation. Verified: cargo test (1933 passed), cargo clippy -D warnings clean, and a full resolution-accuracy scan against the real Moneta corpus shows no new anomalies in the Gap/FilteredCandidate lists (a small aggregate recall delta vs. the prior measurement is fully explained by this branch not yet containing PR #318's separate enum-intrinsics fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7ZonwYUh94VsuQphiHykF --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Hessesian · 2026-09-14T09:00:25Z

Closed without merging (auto-closed after the rebase): PR #317's own split/decomposition of resolve.rs already absorbed and improved on this fix (the same NavController/NavHostController container-scoping closure, now in qualified.rs, plus this session's additional cross-container-leak hardening). Rebasing this branch onto post-#317 main produced a zero-diff branch — nothing left to merge. Verified on the rebased branch before closing: cargo test (1933 passed), cargo clippy -D warnings clean.

Hessesian requested a lite review from Copilot September 10, 2026 09:31

Copilot started reviewing on behalf of Hessesian September 10, 2026 09:32 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Hessesian mentioned this pull request Sep 11, 2026

resolve.rs: split into modules, decompose resolve_qualified, plus 3 measured gap fixes #317

Merged

5 tasks

Hessesian closed this Sep 14, 2026

Hessesian force-pushed the fix/jar-container-scoping-navigate-leak branch from 94f1283 to 4c28243 Compare September 14, 2026 09:00

Navigation Menu

fix(resolve): stop leaking a sibling class's member across a JAR's synthetic file boundary - #315

fix(resolve): stop leaking a sibling class's member across a JAR's synthetic file boundary#315
Hessesian wants to merge 0 commit into
mainfrom
fix/jar-container-scoping-navigate-leak

Hessesian commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Hessesian commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Hessesian commented Sep 9, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Hessesian commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants