feat: Allow crate authors to control completion of their things #19375
Conversation
rustbot
added
the
S-waiting-on-review
There was a problem hiding this comment.
Not a fan of this infesting all of hir-def and the like with ide specific things (though I do like the changing of flags to all use bitflags).
Given this only affects flyimport (with the exception of traits), I feel like this would be more fitting to be recorded as a flag in the ImportMap ImportInfo entries and then either be filtered immediately in search_for_imports or have that flag propagate out of search_for_imports.
crates/hir/src/lib.rs
Outdated
| @@ -6343,3 +6483,42 @@ where | |||
| self(item) | |||
| } | |||
| } | |||
|
|
|||
| pub fn resolve_absolute_path( | |||
There was a problem hiding this comment.
This method does not make sense, a crate name only exists as a name of a dependency edge. That is we can only resolve paths from within the context of some crate.
| scope | ||
| .resolve_mod_path(&ModPath::from_segments( | ||
| hir::PathKind::Plain, | ||
| path.split("::").map(Symbol::intern).map(Name::new_symbol_root), | ||
| )) | ||
| .find_map(|it| match it { | ||
| hir::resolve_absolute_path(db, path.split("::").map(Symbol::intern)).find_map( | ||
| |it| match it { | ||
| hir::ItemInNs::Types(ModuleDef::Trait(t)) => Some(t), | ||
| _ => None, | ||
| }) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Ah I see, that is probably regarding
Also fix a bug with existing settings for that where the paths wouldn't resolve correctly. @Veykril it seems you just blessed the tests incorrectly in #18179?
No that was intentional, we need to resolve the paths from the given scope. Though I suppose that hir::PathKind::Plain really should've been hir::PathKind::Absolute?
There was a problem hiding this comment.
The test blessing was probably not intentional, as it rendered those tests meaningless.
About
This method does not make sense, a crate name only exists as a name of a dependency edge. That is we can only resolve paths from within the context of some crate.
Generally I agree, but this is a global setting, so it doesn't depend at the current crate. At the extreme what could happen is that in different crates it will resolve to different things. More problematic, this prevents marking an item from the crate itself, as seen by the tests (without hacks). Also, if some crate reexports another crate, you will have to either write both or let the setting be ignored in some places.
What about traits? Should they remain part of hir-def or should we lookup the attribute in |
|
We should probably lookup the attribute in |
ChayimFriedman2
force-pushed
the
do-not-complete
branch
from
31a9f20 to
c15a595
Compare
|
I changed this to check the attribute in the import and symbols map. We can't filter it in |
|
Design wise, I wonder if we should go for something more generic like |
ChayimFriedman2
force-pushed
the
do-not-complete
branch
from
c15a595 to
81bd267
Compare
|
Fixed the multi crates problem and removed legacy macros. |
ChayimFriedman2
force-pushed
the
do-not-complete
branch
from
81bd267 to
ee2efbc
Compare
|
Maybe something more generic would be appropriate. Two questions:
|
I guess yes, if they want? We don't have a
Yes, but e.g. |
|
I'm not objected to a more general attribute, but I also think we shouldn't hang this indefinitely on the perfect design choice. At worst, we can declare no stability. |
ChayimFriedman2
force-pushed
the
do-not-complete
branch
from
ee2efbc to
cbae3be
Compare
Via the new `#[rust_analyzer::completions(...)]` attribute. Also fix a bug with existing settings for that where the paths wouldn't resolve correctly.
ChayimFriedman2
force-pushed
the
do-not-complete
branch
from
cbae3be to
7b584ef
Compare
|
Forgot to hit the button. |
Via the new
#[rust_analyzer::completions(...)]attribute.Closes #17477.
Also fix a bug with existing settings for that where the paths wouldn't resolve correctly. @Veykril it seems you just blessed the tests incorrectly in #18179?