refactor(resolution): widen ResolvedSymbol into a rich record; hover uses it#193
Merged
Hessesian merged 4 commits intoJun 30, 2026
Merged
Conversation
Remove unused items in the resolution subsystem so the upcoming trait-catalog refactor doesn't carry dead weight forward (all verified via reference-count + cargo test, not just cargo build): - `ResolutionChain` trait + impl (resolve.rs): #[allow(dead_code)], a TODO(G4) placeholder never wired up; consumed by nothing and duplicating the live `Resolver` catalog. The free fns it wrapped (resolve_local/.../resolve_qualified) remain — they back resolve_symbol_inner. - `impl Indexer` completion-wrapper block (complete.rs): 6 methods bypassed by every live call path, which uses the free functions directly. - 4 dead `WorkspaceRead` default methods (resolution.rs): enclosing_class_at, mem_lines_for, completions, is_indexing_in_progress — never invoked through the trait; callers reach the inherent `Indexer` via as_indexer. - 2 dead `Indexer` find-wrapper methods (find.rs): find_name_in_uri_after_line, find_local_declaration — no method-call sites; free fns remain. - Cascade: the now-orphaned free fn `complete_dot` is gated `#[cfg(test)]`, since its only remaining users are tests via the cfg(test) re-export. ~120 lines removed. cargo test --bin kmp-lsp: 1418 passed; clippy clean. Deliberately kept (not dead in spirit): the three `ResolvedSymbol` fields and `ResolveOptions::goto_def` / `SubstitutionContext::Precomputed` — they back test assertions and the planned `ResolvedSymbol` widening (Slice 3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7YFhRP8Eqwomdd1EzkDwv
`resolve_symbol_kind` (semantic highlighting) classified a reference by iterating `indexer.definition_locations(name)` — the raw global by-name map, with no import-awareness. For a name declared as different kinds in different packages (e.g. `class Foo` vs `object Foo`), it returned whichever the raw map yielded first, ignoring which `Foo` is actually in scope → wrong highlight token. Route it through scope-aware resolution instead: - add `from_uri` to `resolve_symbol_kind`; swap `definition_locations(name)` for `resolve_symbol_no_rg(name, from_uri)` (local → imports → same-package → star → qualified, no ripgrep fallback — correct for this hot per-token path). - thread `uri` through `walk_kotlin_references` (recursive), the `classify_kotlin_reference` call sites (type ref / top-level call / nav receiver) and `enum_entry_reference_token`. Behaviour preserved for unambiguous names (the no-rg chain still finds them); the only change is picking the in-scope symbol when same-named symbols span packages. New regression test `cross_package_same_name_type_uses_imported_kind` indexes the decoy object first to bias the raw map, then asserts the imported class wins. cargo test --bin kmp-lsp: 1419 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7YFhRP8Eqwomdd1EzkDwv
…thods) Folds the scattered call-return-type lookups behind two intent-named, documented catalog methods on `Resolver`, with a self-documenting `ReturnType` newtype replacing bare `Option<String>` on the trait surface: - `ReturnType(String)` — the as-written result type of a call (generics + `?` preserved); `Deref<str>` + `into_inner()` for the String-typed inference paths that haven't adopted the newtype yet. - `Resolver::function_return_type(fn, uri)` — import-aware: reachable bind first, workspace by-name fallback (the chain consumers were hand-rolling). - `Resolver::method_return_type(type, method, uri?)` — the single composite: member/extension methods then supertype inheritance, in one call. Routed the production consumers off the loose free functions onto the catalog: - `complete.rs` dot-receiver inference (function + method return types) — now import-aware (was the raw by-name scan). - `semantic_tokens/resolve.rs` `member_return_type` + the two call-callee function-return sites — now import-aware and gain the supertype fallback. - `indexer.rs::find_method_return_type_for_type` — collapsed: dropped the explicit `find_extension_fn_return_type` step (dead — `find_method_return_type` already probes extensions first) and the separate supertypes call; both now live inside `method_return_type`. Behaviour-preserving for unambiguous names; for same-named-cross-package functions the routed sites now pick the in-scope definition (same fix shape as the semantic-token symbol-kind slice). +2 catalog tests (`catalog_function_return_type_falls_back_to_by_name`, `catalog_method_return_type_folds_supertype_inheritance`). cargo test --bin kmp-lsp: 1421 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7YFhRP8Eqwomdd1EzkDwv
…uses it Implements the plan's data-boundary thesis for `ResolvedSymbol`: join the attributes a consumer needs into the one record at enrichment time, so it never reaches back into the raw index to read one more field. `ResolvedSymbol` gains (all populated in `enrich_symbol`, where `SymbolEntry` and `FileData` are already in hand — no extra lookups): - `deprecated: bool` ← `SymbolEntry::deprecated` - `container: Option<String>` ← `SymbolEntry::container` - `package: Option<String>` ← `FileData::package` `format_symbol_hover` now reads them straight off the record: - deprecated symbols get a `**⚠ Deprecated**` marker above the signature; - members get a `*in `package.Container`*` context footer (top-level symbols are unchanged — no container, no footer). Previously hover had no way to show either — the deprecation flag lived only on `SymbolEntry` (used by completion) and the enclosing context was discarded by the enrich pipeline. Now both ride along in the rich record. +test `resolve_symbol_info_widens_deprecated_and_qualified_context`. Existing hover tests unaffected (top-level symbols render identically). cargo test --bin kmp-lsp: 1422 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7YFhRP8Eqwomdd1EzkDwv
There was a problem hiding this comment.
Pull request overview
This PR widens ResolvedSymbol into a richer, fully-joined resolution record (pulling deprecated, container, and package during enrichment) so hover rendering can be done entirely from the resolved result without reaching back into raw index data.
Changes:
- Extended
ResolvedSymbolwithdeprecated,container, andpackage, populated inenrich_symbolfromSymbolEntryandFileData. - Updated hover Markdown formatting to show a deprecation marker and (for members) an
in package.Containercontext footer. - Added an end-to-end test covering the new joins and hover rendering output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/indexer/resolution.rs | Adds the new joined fields to ResolvedSymbol and populates them during enrichment. |
| src/backend/format.rs | Renders hover output using the enriched fields (deprecated marker + qualified context footer). |
| src/indexer/resolution_tests.rs | Adds a regression test asserting the joined fields and hover formatting behavior. |
| assert_eq!(r.container.as_deref(), Some("Widget")); | ||
| assert_eq!(r.package.as_deref(), Some("com.example.ui")); | ||
|
|
||
| let hover = format_symbol_hover(&r, file_uri); |
Hessesian
changed the base branch from
refactor/resolution-catalog-s4
to
refactor/unified-resolution
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.
Slice 3b — the plan's core data-boundary thesis: a resolved symbol should be a rich, complete record so a consumer gets everything from one resolve call and never reaches back into the raw index for one more attribute. Stacked on #192 (base
refactor/resolution-catalog-s4).What's joined
ResolvedSymbolgains three fields, all populated inenrich_symbolwhereSymbolEntryandFileDataare already in hand — zero extra lookups:deprecated: boolSymbolEntry::deprecatedcontainer: Option<String>SymbolEntry::containerpackage: Option<String>FileData::packageConsumer that reads them
format_symbol_hovernow renders straight off the record:**⚠ Deprecated**marker above the signature;*inpackage.Container*context footer.Top-level symbols are unchanged (no container ⇒ no footer), so existing hover output is preserved. Before this, hover had no way to show either: the deprecation flag lived only on
SymbolEntry(completion's copy) and the enclosing context was dropped by the enrich pipeline.This also makes the fields genuinely live rather than
#[allow(dead_code)]-reserved — the rich record earns its keep through a real consumer.Verification
+test
resolve_symbol_info_widens_deprecated_with_qualified_context(asserts the join + both renderings).cargo test --bin kmp-lsp: 1422 passed; clippy + fmt + guideline hooks clean.🤖 Generated with Claude Code