internal: Follow rustfmt's algorithm for ordering imports when ordering and merging use trees
Updates use tree ordering and merging utilities to follow rustfmt's algorithm for ordering imports.
The [rustfmt implementation](6356fca675/src/imports.rs) was used as reference.
Show which roots are being scanned in progress messages
This changes the `Roots Scanned` message to include the directory being scanned.
Before: `Roots Scanned 206/210 (98%)`
After: `Roots Scanned 206/210: .direnv (98%)`
This makes it a lot easier to tell that `rust-analyzer` isn't crashed, it's just trying to scan a huge directory.
See: #12613
By making it an `EscapeError` instead of a `LitError`. This makes it
like the other errors produced when checking string literals contents,
e.g. for invalid escape sequences or bare CR chars.
NOTE: this means these errors are issued earlier, before expansion,
which changes behaviour. It will be possible to move the check back to
the later point if desired. If that happens, it's likely that all the
string literal contents checks will be delayed together.
One nice thing about this: the old approach had some code in
`report_lit_error` to calculate the span of the nul char from a range.
This code used a hardwired `+2` to account for the `c"` at the start of
a C string literal, but this should have changed to a `+3` for raw C
string literals to account for the `cr"`, which meant that the caret in
`cr"` nul error messages was one short of where it should have been. The
new approach doesn't need any of this and avoids the off-by-one error.
fix: Acknowledge `pub(crate)` imports in import suggestions
rust-analyzer has logic that discounts suggesting `use`s for private imports, but that logic is unnecessarily strict - for instance given this code:
```rust
mod foo {
pub struct Foo;
}
pub(crate) use self::foo::*;
mod bar {
fn main() {
Foo$0;
}
}
```
... RA will suggest to add `use crate::foo::Foo;`, which not only makes the code overly verbose (especially in larger code bases), but also is disjoint with what rustc itself suggests.
This commit adjusts the logic, so that `pub(crate)` imports are taken into account when generating the suggestions; considering rustc's behavior, I think this change doesn't warrant any extra configuration flag.
Note that this is my first commit to RA, so I guess the approach taken here might be suboptimal - certainly feels somewhat hacky, maybe there's some better way of finding out the optimal import path 😅
rust-analyzer has logic that discounts suggesting `use`s for private
imports, but that logic is unnecessarily strict - for instance given
this code:
```rust
mod foo {
pub struct Foo;
}
pub(crate) use self::foo::*;
mod bar {
fn main() {
Foo$0;
}
}
```
... RA will suggest to add `use crate::foo::Foo;`, which not only makes
the code overly verbose (especially in larger code bases), but also is
disjoint with what rustc itself suggests.
This commit adjusts the logic, so that `pub(crate)` imports are taken
into account when generating the suggestions; considering rustc's
behavior, I think this change doesn't warrant any extra configuration
flag.
Note that this is my first commit to RA, so I guess the approach taken
here might be suboptimal - certainly feels somewhat hacky, maybe there's
some better way of finding out the optimal import path 😅
minor: Mark unresolved associated item diagnostic as experimental
Per #16327 unresolved associated item has false positives. Mark the diagnostic as experimental until this is more dependable.
Resolve panic in `generate_delegate_methods`
Fixes#16276
This PR addresses two issues:
1. When using `PathTransform`, it searches for the node corresponding to the `path` in the `source_scope` during `make::fn_`. Therefore, we need to perform the transform before `make::fn_` (similar to the problem in issue #15804). Otherwise, even though the tokens are the same, their offsets (i.e., `span`) differ, resulting in the error "Can't find CONST_ARG@xxx."
2. As mentioned in the first point, `PathTransform` searches for the node corresponding to the `path` in the `source_scope`. Thus, when transforming paths, we should update nodes from right to left (i.e., use **reverse of preorder** (right -> left -> root) instead of **postorder** (left -> right -> root)). Reasons are as follows:
In the red-green tree (rowan), we do not store absolute ranges but instead store the length of each node and dynamically calculate offsets (spans). Therefore, when modifying the left-side node (such as nodes are inserted or deleted), it causes all right-side nodes' spans to change. This, in turn, leads to PathTransform being unable to find nodes with the same paths (due to different spans), resulting in errors.
fix: Fix `ast::Path::segments` implementation
calling `ast::Path::segments` on a qualifier currently returns all the segments of the top path instead of just the segments of the qualifier.
The issue can be summarized by the simple failing test below:
```rust
#[test]
fn path_segments() {
//use ra_ap_syntax::ast;
let path: ast::Path = ...; // e.g. `ast::Path` for "foo::bar::item".
let path_segments: Vec<_> = path.segments().collect();
let qualifier_segments: Vec<_> = path.qualifier().unwrap().segments().collect();
assert_eq!(path_segments.len(), qualifier_segments.len() + 1); // Fails because `LHS = RHS`.
}
```
This PR:
- Fixes the implementation of `ast::Path::segments`
- Fixes `ast::Path::segments` callers that either implicitly relied on behavior of previous implementation or exhibited other "wrong" behavior directly related to the result of `ast::Path::segments` (all callers have been reviewed, only one required modification)
- Removes unnecessary (and now unused) `ast::Path::segments` alternatives
fix: Differentiate between vfs config load and file changed events
Kind of fixes https://github.com/rust-lang/rust-analyzer/issues/14730 in a pretty bad way. We need to rethink the vfs-notify layer entirely. For a decent fix.
internal: Only compare relevant parts in `ide::{runnables,inlay_hints}` tests
This PR limits the data being compared. Therefore the tests should be more readable, as well as being more robust to changes to the data structure.
Part of https://github.com/rust-lang/rust-analyzer/issues/14268.
internal: clean and enhance readability for `generate_delegate_trait`
Continue from #16112
This PR primarily involves some cleanup and simple refactoring work, including:
- Adding numerous comments to layer the code and explain the behavior of each step.
- Renaming some variables to make them more sensible.
- Simplify certain operations using a more elegant approach.
The goal is to make this intricate implementation clearer and facilitate future maintenance.
In addition to this, the PR also removes redundant `path_transform` operations for `type_gen_args`.
Taking the example of `impl Trait<T1> for S<S1>`, where `S1` is considered. The struct `S` must be in the file where the user triggers code actions, so there's no need for the `path_transform`. Furthermore, before performing the transform, we've already renamed `S1`, ensuring it won't clash with existing generics parameters. Therefore, there's no need to transform it.
internal: Speed up import searching some more
Pushes the sorting to the caller, meaning additional filtering can be done pre-sorting. Similarly a collect call was pushed to the caller for allowing some other filters to run pre-collecting.
Remove completion limit for trait importing method completions
Fixes https://github.com/rust-lang/rust-analyzer/issues/16075
The < 3 char limit never applied to methods and the amount of completions generated due this is not absolutely massive as not all traits in a project are ever applicable so there is little reason to employ the limit here. Especially as it limits the number of traits we consider, not items (after my changes yesterday), and the number of traits is not the slowing factor here. Tested this in r-a where we have ~800 traits project wide and even when ~260 are applicable there was no noticable slow down from it.
internal: Move query limits to the caller
Prior we calculated up to `limit` entries from a query, then filtered from that leaving us with less entries than the limit in some cases (which might give odd completion behavior due to items disappearing). This changes it so we filter before checking the limit.
Give a userful error when rustc cannot be found in explicit sysroot
Somehow r-a believed that my sysroot was something weird with no rustc. Probably a me issue, but it was impossible to diagnose since r-a just gave me a plain "No such file or directory". Adding this error makes it clear what happened and allows diagnosing the problem.
Somehow r-a believed that my sysroot was something weird with no rustc.
Probably a me issue, but it was impossible to diagnose since r-a just
gave me a plain "No such file or directory". Adding this error makes it
clear what happened and allows diagnosing the problem.
feat: resolve inherent and implemented associated items in docs
This partially fixes#9694.
Supported:
- Trait methods and constants.
* Due to resolution differences pointed out during the review of the PR, trait associated types are _not_ supported.
- Inherent methods, constants and associated types.
* Inherent associated types are a [nightly feature](https://github.com/rust-lang/rust/issues/8995), and are supported with no additional work in this PR.
Screenshot of VS Code running with the change:
<img width="513" alt="image" src="https://github.com/rust-lang/rust-analyzer/assets/7189784/c37ed8b7-b572-4684-8e81-2a817b0027c4">
You can see that the items are resolved (excl. trait associated types) since they are semantically highlighted in the doc comment.
fix: try obligation of `IndexMut` when infer
Closes#15842.
This issue arises because `K` is ambiguous if only inferred from `Index` trait, but is unique if inferred from `IndexMut`, but r-a doesn't use this info.
SymbolInformation::kind is finer-grained than the SCIP symbol suffix.
This also fixes a bug where all type aliases where treated like type
parameters.
```
trait SomeTrait {
type AssociatedType; // ← this is SomeTrait#[AssociatedType]
}
type MyTypeAlias = u8; // ← this used to be [MyTypeAlias]
// and now is MyTypeAlias#
```
To build the SymbolInformation::signature_documentation we need access
to the “label” when building the TokenStaticData, preferably without
any markdown markup.
Therefore this refactors ide::hover::render::definition and its helper
functions to give easier access to the label alone.
For local variables, this gets the moniker from the enclosing
definition and stores it into the TokenStaticData.
Then it builds the scip symbol for that moniker when building the
SymbolInformation.
This is meant to implement SymbolInformation::enclosing_symbol, so we
can build the enclosing symbol from the enclosing moniker without
having the full enclosing token's TokenStaticData.
fix: pick up new names when the name conflicts in 'introduce_named_generic'
Improve generation of names for generic parameters in `introduce_named_generics`.
fix#15731.
### Changes
- Modified `for_generic_parameter` function in `suggest_name.rs` to handle conflicts with existing generic parameters and generate unique names accordingly.
- Update `introduce_named_generic` function and pass existing params to `for_generic_parameter`, enabling the detection and handling of name collisions.
* Extracted the function `for_unique_generic_name` that handling generics with identical names for reusability.
* Renamed `for_generic_params` to `for_impl_trait_as_generic` for clarity
* Added documentations for `for_impl_trait_as_generic` and `for_unique_generic_name`
This commit changes how the expected type is calculated when working
with Fn pointers, making the parenthesis stop vanishing when completing
the function name.
I've been bugged by the behaviour on parenthesis completion for a long
while now. R-a assumes that the `LetStmt` type is the same as the
function type I've just written. Worse is that all parenthesis vanish,
even from functions that have completely different signatures. It will
now verify if the signature is the same.
While working on this, I noticed that record fields behave the same, so
I also made it prioritize the field type instead of the current
expression when possible, but I'm unsure if this is OK, so input is
appreciated.
ImplTraits as return types will still behave weirdly because lowering is
disallowed at the time it resolves the function types.
fix: rewrite code_action `generate_delegate_trait`
I've made substantial enhancements to the "generate delegate trait" code action in rust-analyzer. Here's a summary of the changes:
#### Resolved the "Can’t find CONST_ARG@158..159 in AstIdMap" error
Fix#15804, fix#15968, fix#15108
The issue stemmed from an incorrect application of PathTransform in the original code. Previously, a new 'impl' was generated first and then transformed, causing PathTransform to fail in locating the correct AST node, resulting in an error. I rectified this by performing the transformation before generating the new 'impl' (using make::impl_trait), ensuring a step-by-step transformation of associated items.
#### Rectified generation of `Self` type
`generate_delegate_trait` is unable to properly handle trait with `Self` type.
Let's take the following code as an example:
```rust
trait Trait {
fn f() -> Self;
}
struct B {}
impl Trait for B {
fn f() -> B { B{} }
}
struct S {
b: B,
}
```
Here, if we implement `Trait` for `S`, the type of `f` should be `() -> Self`, i.e. `() -> S`. However we cannot automatically generate a function that constructs `S`.
To ensure that the code action doesn't generate delegate traits for traits with Self types, I add a function named `has_self_type` to handle it.
#### Extended support for generics in structs and fields within this code action
The former version of `generate_delegate_trait` cannot handle structs with generics properly. Here's an example:
```rust
struct B<T> {
a: T
}
trait Trait<T> {
fn f(a: T);
}
impl<T1, T2> Trait<T1> for B<T2> {
fn f(a: T1) -> T2 { self.a }
}
struct A {}
struct S {
b$0 : B<A>,
}
```
The former version will generates improper code:
```rust
impl<T1, T2> Trait<T1, T2> for S {
fn f(&self, a: T1) -> T1 {
<B as Trait<T1, T2>>::f( &self.b , a)
}
}
```
The rewritten version can handle generics properly:
```rust
impl<T1> Trait<T1> for S {
fn f(&self, a: T1) -> T1 {
<B<A> as Trait<T1>>::f(&self.b, a)
}
}
```
See more examples in added unit tests.
I enabled support for generic structs in `generate_delegate_trait` through the following steps (using the code example provided):
1. Initially, to prevent conflicts between the generic parameters in struct `S` and the ones in the impl of `B`, I renamed the generic parameters of `S`.
2. Then, since `B`'s parameters are instantiated within `S`, the original generic parameters of `B` needed removal within `S` (to avoid errors from redundant parameters). An important consideration here arises when Trait and B share parameters in `B`'s impl. In such cases, these shared generic parameters cannot be removed.
3. Next, I addressed the matching of types between `B`'s type in `S` and its type in the impl. Given that some generic parameters in the impl are instantiated in `B`, I replaced these parameters with their instantiated results using PathTransform. For instance, in the example provided, matching `B<A>` and `B<T2>`, where `T2` is instantiated as `A`, I replaced all occurrences of `T2` in the impl with `A` (i.e. apply the instantiated generic arguments to the params).
4. Finally, I performed transformations on each assoc item (also to prevent the initial issue) and handled redundant where clauses.
For a more detailed explanation, please refer to the code and comments. I welcome suggestions and any further questions!
fix: self type replacement in inline-function
Fix#16113, fix#16091
The problem described in this issue actually involves three bugs.
Firstly, when using `ted` to modify the syntax tree, the offset of nodes on the tree changes, which causes the syntax range information from `hir` to become invalid. Therefore, we need to edit the AST after the last usage for `usages_for_locals`.
The second issue is that when inserting nodes, it's necessary to use `clone_subtree` for duplication because the `ted::replace` operation essentially moves a node.
The third issue is that we should use `ancestors_with_macros` instead of `ancestors` to handle impl definition in macros.
I have fixed the three bugs mentioned above and added unit tests.
internal: Migrate assists to the structured snippet API, part 5
Continuing from #15874
Migrates the following assists:
- `extract_variable`
- `generate_function`
- `replace_is_some_with_if_let_some`
- `replace_is_ok_with_if_let_ok`
Don't trim trailing whitespace from doc comments
Don't trim trailing whitespace from doc comments as multiple trailing spaces indicates a hard line break in Markdown.
I'd have liked to add a unit test for `docs_from_attrs`, but couldn't find a reasonable way to get an `&Attrs` object for use in the test.
Fixes#15877.
fix: make callable fields not complete in method access no parens case
Follow up PR for #15879
Fixes the callable field completion appearing in the method access with no parens case.
fix: no code action 'introduce_named_generic' for impl inside types
Fix#15734.
### Changes Made
- Find params in `ancestors` instead of just `parent`
- Added tests (`replace_impl_with_mut` and `replace_impl_inside`)
fix: Correct references from `rust-analyzer.cargo.check` to `rust-analyzer.check`
When reading the manual, I noticed that the documentation referenced configurations that have since been renamed. This PR updates those references to their new names.
