Fix incorrectly replacing references in macro invocation in "Convert to named struct" assist
Fixes#15630.
Complements #13647 (same assist but missed this one), #14920 (inverse action assist).
fix: resolve Self type references in delegate method assist
This PR makes the delegate method assist resolve any `Self` type references in the parameters or return type. It also works across macros such as the `uint_impl!` macro used for `saturating_mul` in the issue example.
Closes#14485
internal: Migrate assists to the structured snippet API, part 4
Continuing from #15260
Migrates the following assists:
- `add_turbo_fish`
- `add_type_ascription`
- `destructure_tuple_binding`
- `destructure_tuple_binding_in_subpattern`
I did this a while ago, but forgot to make a PR for the changes until now. 😅
Due to the way the current tree mutation api works, we need to collect
changes before we can apply them to the real syntax tree, and also can only
switch to a file once.
`destructure_tuple_binding_in_sub_pattern` also gets migrated even
though can't be used.
feat: implement tuple return type to tuple struct assist
This PR implements the `convert_tuple_return_type_to_struct` assist, for converting the return type of a function or method from a tuple to a tuple struct. Additionally, it moves the `to_camel_case` and `char_has_case` functions from `case_conv` to `stdx` so that they can be used similar to `to_lower_snake_case`.
[tuple_return_type_to_tuple_struct.webm](https://github.com/rust-lang/rust-analyzer/assets/52933714/2803ff58-fde3-4144-9495-7c7c7e139075)
Currently, the assist puts the struct definition above the function, or above the nearest `impl` or `trait` if applicable and only rewrites literal tuples that are returned in the body of the function. Additionally, it only attempts to rewrite simple tuple pattern usages with the corresponding tuple struct pattern but does so across files and modules.
I think that this is sufficient for the majority of use cases but I could be wrong. One thing I'm still not sure how to approach is handling `Self` and generics/lifetimes in the tuple type to be extracted. I was thinking of either manually figuring out what lifetimes and generics are in scope and using them (sort of similar to the `generate_function` assist) or maybe using `ctx.sema.resolve_type` and `generic_params` on `hir::Type` but this seems to not deal with lifetimes.
Closes#14293
fix: make bool_to_enum assist create enum at top-level
This pr makes the `bool_to_enum` assist create the `enum` at the next closest module block or at top-level, which fixes a few tricky cases such as with an associated `const` in a trait or module:
```rust
trait Foo {
const $0BOOL: bool;
}
impl Foo for usize {
const BOOL: bool = true;
}
fn main() {
if <usize as Foo>::BOOL {
println!("foo");
}
}
```
Which now properly produces:
```rust
#[derive(PartialEq, Eq)]
enum Bool { True, False }
trait Foo {
const BOOL: Bool;
}
impl Foo for usize {
const BOOL: Bool = Bool::True;
}
fn main() {
if <usize as Foo>::BOOL == Bool::True {
println!("foo");
}
}
```
I also think it's a bit nicer, especially for local variables, but didn't really know to do it in the first PR :)
fix: panic with wrapping/unwrapping result return type assists
With the `wrap_return_type_in_result` assist, the following code results in a panic (note the lack of a semicolon):
```rust
fn foo(num: i32) -> $0i32 {
return num
}
=>
thread 'handlers::wrap_return_type_in_result::tests::wrap_return_in_tail_position' panicked at crates/syntax/src/ted.rs:137:41:
called `Option::unwrap()` on a `None` value
```
I think this is because it first walks the body expression to change any `return` expressions and then walks all tail expressions, resulting in the `return num` being changed twice since it is both a `return` and in tail position. This can also happen when a `match` is in tail position and `return` is used in a branch for example. Not really sure how big of an issue this is in practice though since this seems to be the only case that is impacted and can be reduced to just `num` instead of `return num`.
This also occurs with the `unwrap_result_return_type` assist but panics with the following instead:
```
thread 'handlers::unwrap_result_return_type::tests::wrap_return_in_tail_position' panicked at /rustc/3223b0b5e8dadda3f76c3fd1a8d6c5addc09599e/library/alloc/src/string.rs:1766:29:
assertion failed: self.is_char_boundary(n)
```
minor : Deunwrap convert_comment_block and desugar_doc_comment
Closes subtask 13 of #15398 . I still don't know a more idiomatic way for the for loops I added, any suggestion would make me happy.
Although it doesn't panic now, further changes to how we recover from incomplete syntax
may cause this assist to panic. To mitigate this a test case has been added.
feat: Bool to enum assist
This adds the `bool_to_enum` assist, which converts the type of boolean local variables, fields, constants and statics to a new `enum` type, making it easier to distinguish the meaning of `true` and `false` by renaming the variants.
Closes#14779
Field shorthand overwritten in promote local to const assist
Currently, running `promote_local_to_const` on the following:
```rust
struct Foo {
bar: usize,
}
fn main() {
let $0bar = 0;
let foo = Foo { bar };
}
```
Results in:
```rust
struct Foo {
bar: usize,
}
fn main() {
const BAR: usize = 0;
let foo = Foo { BAR };
}
```
But instead should be something like:
```rust
struct Foo {
bar: usize,
}
fn main() {
const BAR: usize = 0;
let foo = Foo { bar: BAR };
}
```
Bind unused parameter assistant
This PR introduces a new **Bind unused parameter assistant**.
While we do have a QuickFix from `rustc` (prefixing the parameter with an underscore), it's sometimes more convenient to suppress the warning using the following approach:
```rust
fn some_function(unused: i32) {}
```
->
```rust
fn some_function(unused: i32) {
let _ = unused;
}
```
minor : Deunwrap generate_derive
#15398 subtask 1. Since the editing closure has arms, I did something *experimental* ( in this case just a clever term for bad code ) to bypass creating an `Option` but I am ready to change this.
the "add missing members" assists: implemented substitution of default values of const params
To achieve this, I've made `hir::ConstParamData` store the default values
internal : rewrite DeMorgan assist
fixes#15239 , #15240 . This PR is a rewrite of the DeMorgan assist that essentially rids of all the string manipulation and modifies syntax trees to apply demorgan on a binary expr. The main reason for the rewrite is that I wanted to use `Expr::needs_parens_in` method to see if the expr on which the assist is applied would still need the parens it had once the parent expression's operator had equal precedence with that of the expression. I used `.clone_(subtree|for_update)` left and right and probably more than I should have, so I would also be happy to hear how I could have prevented redundant cloning.
internal: Turn unresolved proc macro expansions into missing expressions
Reduces the amount of type related errors one gets when proc macro expansion is disabled.
Add ExternCrateDecl to HIR
Adding these doesn't really require much design effort as they represent a single import, unlike use trees which are one item that represent 0 or more imports.
We only resolve to this definition when actually resolving on the name or alias of an `extern crate name as alias` item, not usages yet as that requires far more changes that won't lead anywhere without giving it more thought. Nevertheless the changes slightly improve IDE things, an example being hover on the decl showing the merged doc comments for example.
cc https://github.com/rust-lang/rust-analyzer/issues/14079
Added remove unused imports assist
This resolves the most important part of #5131. I needed to make a couple of cosmetic changes to the search infrastructure to do this.
A few open questions:
* Should imports that don't resolve to anything be considered unused? I figured probably not, but it would be a trivial change to make if we want it.
* Is there a cleaner way to make the edits to the use list?
* Is there a cleaner way to get the list of uses that intersect the current selection?
* Is the performance acceptable? When testing this on itself, it takes a good couple seconds to perform the assist.
* Is there a way to hide the rustc diagnostics that overlap with this functionality?
internal: Defer structured snippet rendering to allow escaping snippet bits
Since we know exactly where snippets are, we can transparently escape snippet bits to the exact text edits that need it, and not have to do it for anything other text edits.
Also will eventually fix#11006 once all assists are migrated. This comes as a side-effect of text edits that don't have snippets get marked as having no insert formatting at all.
Don't provide `add_missing_match_arms` assist when upmapping match arm list failed
Fixes#15310
We shouldn't provide the assist when we fail to find the original match arm list.
Note that this PR will temporarily make the assist not applicable when attribute macro operates on the match expression in question, just like the case in #15310, for most of the current stable toolchain users. This is because the sysroot-abi proc-macro-srv on the current stable [discards] spans for `Group` delimiters in some code paths, which the popular `proc-macro2` crate almost always calls, and it makes the identity of match arm list's brackets lost, leading to the upmapping failure. This has been fixed by #14960, which will land in the next stable, 1.71.
[discards]: 8ede3aae28/src/tools/rust-analyzer/crates/proc-macro-srv/src/abis/abi_sysroot/ra_server.rs (L231)
bugfix : skip doc(hidden) default members
fixes #14957 . I have two questions :
1. I am definitely looking for a more idiomatic way for the things I added in `crates/ide-assists/src/utils.rs`. See `FIXME` in that file.
2. Would it be actually better to change `DefaultMethods` to something like
```rust
enum DefaultMethods {
Only( IgnoreHidden ( bool ) ) ,
None
}
```
instead of adding a boolean to every function that calls `crates/ide-assists/src/utils.rs::filter_assoc_items`
internal: Migrate assists to the structured snippet API, part 3
Continuing from #15231
Migrates the following assists:
- `add_missing_match_arms`
- `fix_visibility`
- `promote_local_to_const`
The `add_missing_match_arms` changes are best reviewed commit-by-commit since they're relatively big changes compared to the rest of the commits.
`clone_for_update` is relatively cheap in comparison, since making a
node require parsing an entire source text
Adds a test to make sure that it doesn't crash when multiple uses are
present.
internal: Migrate more assists to use the structured snippet API
Continuing from #14979
Migrates the following assists:
- `generate_derive`
- `wrap_return_type_in_result`
- `generate_delegate_methods`
As a bonus, `generate_delegate_methods` now generates the function and impl block at the correct indentation 🎉.
`does_not_fill_wildcard_with_wildcard`
and `does_not_fill_wildcard_with_partial_wildcard_and_wildcard`
both made no modifications to the code,
which is a problem for mutable ast porting as it generates a best-effort
minimal set of text edits,
and assists require at least one text edit.
# Overview
Extracting a match arm value that has type unit into a function, when a
comma already follows the match arm value, results in an invalid (syntax
error) semicolon added between the newly generated function's generated
call and the comma.
# Example
Running this extraction
```rust
fn main() {
match () {
_ => $0()$0,
};
}
```
would lead to
```rust
fn main() {
match () {
_ => fun_name();,
};
}
fn fun_name() {
}
```
# Issue / Fix details
This happens because when there is no comma, rust-analyzer would simply
add the comma and wouldn't even try to evaluate whether it needs to add
a semicolon. But when the comma is there, it proceeds to evaluate
whether it needs to add a semicolon and it looks like the evaluation
logic erroneously ignores the possibility that we're in a match arm.
IIUC it never makes sense to add a semicolon when we're extracting from
a match arm value, so I've adjusted the logic to always decide against
adding a semicolon when we're in a match arm
Can actually split out adding the functions from getting the impl to
update or create thanks to being able to refer to the impl ast node.
FIXME Context:
Unfortunately we can't adjust the indentation of the newly added function
inside of `ast::AssocItemList::add_item` since for some reason the `todo!()`
placeholder generated by `add_missing_impl_members` and
`replace_derive_with_manual_impl` gets indented weirdly.
Unify getter and setter assists
This PR combines what previously have been two different files into a single file. I want to talk about the reasons why I did this. The issue that prompted this PR ( and before I forget : this pr fixes#15080 ) mentions an interesting behavior. We combine these two assists into an assist group and the order in which the assists are listed in this group changes depending on the text range of the selected area. The reason for that is that VSCode prioritizes actions that have a bigger impact in a smaller area and until now generate setter assist was only possible to be invoked for a single field whereas you could generate multiple getters for the getter assist. So I used the latter's infra to make former applicable to multiple fields, hence the unification. So this PR solves in essence
1. Make `generate setter` applicable to multiple fields
2. Provide a consistent order of the said assists in listing.
assist : generate trait from impl
fixes#14987 . As the name suggests this assist is used to generate traits from inherent impls while adapting the original impl to fit to the newly generated trait. I made some decisions regarding when the assist should be applicable. These are surely open to discussion. I looking forward to any feedback.
