Improved reference <> definition support#1165
Merged
sbillig merged 33 commits intoargotorg:masterfrom Dec 8, 2025
Merged
Conversation
f5f8ff9 to
a5b3c2f
Compare
419f840 to
b424f43
Compare
14 tasks
2ee316f to
06913bb
Compare
micahscopes
commented
Dec 6, 2025
sbillig
reviewed
Dec 6, 2025
Comment on lines
225
to
240
| fn local_target(&self, db: &'db dyn HirAnalysisDb) -> Option<Target<'db>> { | ||
| let body = self.scope.body()?; | ||
| let func = body.containing_func(db)?; | ||
| let expr_id = body.find_expr_for_path(db, self.path)?; | ||
| let (_, typed_body) = check_func_body(db, func); | ||
|
|
||
| let span = typed_body.expr_binding_def_span(func, expr_id)?; | ||
| let ty = typed_body.expr_ty(db, expr_id); | ||
|
|
||
| Some(Target::Local { | ||
| span, | ||
| ty, | ||
| func, | ||
| expr: expr_id, | ||
| }) | ||
| } |
Collaborator
There was a problem hiding this comment.
PathView as it's defined, and this function (and find_expr_for_path) don't suffice for variable names in function bodies, where scoping rules rule.
Collaborator
Author
There was a problem hiding this comment.
I'm working on this, still not satisfied though
Collaborator
Author
There was a problem hiding this comment.
should be addressed now, not in the most elegant way, but effectively
5c0409f to
ab1bf80
Compare
Collaborator
Author
|
@sbillig I think this is much better now, though not the most elegant |
590065a to
87768e6
Compare
Local variables and parameters should shadow module-level items with the same name. Modified PathView::target() and target_at() to check local bindings first before falling back to module-level resolution. Updated test snapshots to reflect correct behavior where parameters and local bindings now resolve as locals instead of incorrectly resolving to module paths or value references. Added locals.fe test case to verify local shadowing works correctly, including parameter shadowing by let bindings.
When renaming Bar in Foo::Bar, the entire path was being replaced with just the new name instead of producing Foo::Baz. Added PathView::last_segment_span() to get only the identifier span of the last segment. Added ReferenceView::rename_span() that delegates to last_segment_span for paths while returning normal spans for other reference types. Updated rename handler to use rename_span() instead of span() for text edits. This preserves goto-definition behavior (which needs full path spans to detect intermediate segment clicks) while fixing rename to only replace the actual referenced item.
Added goto_path_segments.fe to verify all intermediate path segments are resolvable for goto-definition (e.g. clicking on 'stuff' in 'stuff::calculations::ambiguous' resolves correctly). Added goto_where_clause.fe to verify trait bounds in where clauses are resolvable (e.g. clicking on 'MyTrait' in 'where T: MyTrait' resolves to the trait definition). Both tests cover inline module resolution. Cross-ingot resolution is blocked pending PR argotorg#1146 which adds ingot initialization when opening .fe files.
When use statements with braces like `use foo::{ bar, baz }` are
decomposed into separate Use HIR items, they share identical item spans
pointing to the original source. This caused find_enclosing_item to
return only the first matching use, breaking goto for subsequent items.
Changed find_enclosing_item to find_enclosing_items (returning all
items with the smallest span) and reference_at now tries each candidate
until finding one whose references match the cursor position.
This handles decomposed uses naturally without special-casing.
Scope graphs don't include edges to child file modules, only to items declared in the file itself. This caused use statements like `use stuff::` to fail when `stuff` is a child file module (stuff.fe). Added fallback in UsePathView::target to check the module tree for child file modules when normal name resolution fails. This allows resolving the first segment of use paths to child file modules.
Extended the semantic traversal API to handle ambiguous references where a path can resolve to multiple items (e.g., both a module and function with the same name). API design: - target() / target_at() - returns first/best match (existing behavior) - target_candidates() / target_candidates_at() - returns all candidates This provides a coherent API where: - Simple cases use the singular methods - Ambiguous resolution uses the plural methods - LSP handlers can return arrays for multiple goto targets - All ambiguity handling is encapsulated in the HIR semantic layer The goto handler now returns GotoDefinitionResponse::Array when multiple targets exist, allowing editors to present all options to the user.
When navigating to TopLevelMod (file modules), we were incorrectly using file.path(db) which returns a relative path, then formatting it as a file:// URL. This created invalid URLs like 'file://src/stuff.fe' instead of valid absolute file:// URLs. Fixed by using file.url(db) which returns the proper absolute file:// URL. This fixes goto-definition not working when clicking on root path segments that resolve to file modules (e.g., clicking 'stuff' in 'use stuff::calculations'). crates/language-server/src/util.rs:66
- Fix UsePathView to start resolution from the use item's scope instead of its parent scope. This fixes resolution of file module references in use paths. - Add UsePathView::target_at for cursor-aware segment resolution in use paths - Update ReferenceView::target_at to delegate to UsePathView::target_at for use paths - Improve hover display for ambiguous references: - Change header from 'Ambiguous reference' to 'Multiple definitions' - Remove candidate numbering for cleaner display - Add horizontal rules between candidates - Include docstrings for all candidates - Remove duplicate goto_path_segments test (covered by goto_ambiguous) crates/hir/src/core/semantic/reference/mod.rs:372 crates/hir/src/core/semantic/reference/mod.rs:420 crates/hir/src/core/semantic/reference/mod.rs:498 crates/language-server/src/functionality/hover.rs:39
646ba91 to
1101302
Compare
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.
Builds on #1162 (traversal API) to add a reference view API at the HIR semantic layer, then uses it to power a bunch of LSP functionality.
Changes
HIR semantic layer (
hir::semantic::reference):LSP handlers (
language-server):Fixes along the way:
What's left for followup