feat: Allow excluding specific traits from completion #18179
Conversation
rustbot
added
the
S-waiting-on-review
ChayimFriedman2
force-pushed
the
omit-trait-completion
branch
2 times, most recently
from
61667ba
to
1878d26
Compare
| complete_semicolon, | ||
| }; | ||
| Some((ctx, analysis)) | ||
| } | ||
| } | ||
|
|
||
| fn resolve_exclude_traits_list(db: &RootDatabase, traits: &[String]) -> FxHashSet<hir::Trait> { |
There was a problem hiding this comment.
This should be doable in a more simple manner using semantics, Semantics::scope (we likely already have that computed at this point) and then expose a method to that that either resolves a string path or a mod path using similar logic of doc_modpath_from_str function in crates\hir\src\attrs.rs
There was a problem hiding this comment.
Or actually, do what the snippet stuff is doing by turning the strings into green nodes https://github.com/rust-lang/rust-analyzer/blob/2f55a9155211a9c4a7e061ed4c01bacfa2ebc3d4/crates/ide-completion/src/snippet.rs#L197-L222
it already is doing something like this for paths, creating fake ast nodes for them and then resolving those via the scope at the use site
There was a problem hiding this comment.
I somewhat dislike this approach since it means the names are not universal: they can be shadowed and will be impacted by dependency renames. This also makes caching them harder, if we will ever want to do that. Not strongly opposed, though.
Veykril
added
S-waiting-on-author
| /// - [`core::any::Any`](https://doc.rust-lang.org/nightly/core/any/trait.Any.html) | ||
| /// - All operator traits (in [`core::ops`](https://doc.rust-lang.org/nightly/core/ops)) | ||
| /// | ||
| /// Note that if you override this setting, those traits won't be automatically inserted, so you may want to insert them manually. |
There was a problem hiding this comment.
| /// Note that if you override this setting, those traits won't be automatically inserted, so you may want to insert them manually. | |
| /// Note that if this setting is overridden, the above traits must be re-inserted manually override. |
There was a problem hiding this comment.
This implies that the user needs to insert those traits, while the reality is that we don't include them when overridden so they can be removed if wanted.
ChayimFriedman2
force-pushed
the
omit-trait-completion
branch
2 times, most recently
from
3699f48
to
39d3441
Compare
|
☔ The latest upstream changes (presumably #18167) made this pull request unmergeable. Please resolve the merge conflicts. |
To be accurate, only their methods are excluded, the trait themselves are still available. I also excluded a bunch of std traits by default. Some less opinionated, like `AsRef`, which should never be used directly except in generic scenarios (and won't be excluded there), some more opinionated, like the ops traits, which I know some users sometimes want to use directly. Either way it's configurable. It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently. Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.
… the name I.e. with `fn foo()`, don't complete at `x.fo|`, but complete (with imports) for `x.foo|`, since this is less likely to have false positives. I opted to only do that for flyimport, even though for basic imports there can also be snippet completion (completing the params list for a method), since this is less universally applicable and seems not so useful.
ChayimFriedman2
force-pushed
the
omit-trait-completion
branch
from
addd4fd
to
1d9543f
Compare
|
☔ The latest upstream changes (presumably #18291) made this pull request unmergeable. Please resolve the merge conflicts. |
To be accurate, only their methods are excluded, the trait themselves are still available.
I also excluded a bunch of std traits by default. Some less opinionated, like
AsRef, which should never be used directly except in generic scenarios (and won't be excluded there), some more opinionated, like the ops traits, which I know some users sometimes want to use directly. Either way it's configurable.It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently.
Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.
Leave #17477 open, as per @dtolnay request.