9955: fix: Rename fails on renaming definitions created by macros instead of renaming the macro invocation r=Veykril a=Veykril
bors r+
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
9855: feature: Destructure Tuple Assist r=Veykril a=Booksbaum
Part of #8673. This PR only handles tuples, not TupleStruct and RecordStruct.
Code Assist to destructure a tuple into its items:
![Destructure_Tuple_Assist](https://user-images.githubusercontent.com/15612932/129020107-775d7c94-dca7-4d1f-a0a2-cd63cabf4132.gif)
* Should work in nearly all pattern positions, like let assignment, function parameters, match arms, for loops, and nested variables (`if let Some($0t) = Some((1,2))`)
-> everywhere `IdentPat` is allowed
* Exception: If there's a sub-pattern (``@`):`
```rust
if let t @ (1..=3, 1..=3) = ... {}
// ^
```
-> `t` must be a `Name`; `TuplePat` (`(_0, _1)`) isn't allowed
* inside subpattern is ok:
```rust
let t @ (a, _) = ((1,2), 3);
// ^
```
->
```rust
let t @ ((_0, _1), _) = ((1,2), 3);
```
* Assist triggers only at tuple declaration, not tuple usage.
(might be useful especially when it creates a sub-pattern (after ``@`)` and only changes the usage under cursor -- but not part of this PR).
### References
References can be destructured:
```rust
let t = &(1,2);
// ^
let v = t.0;
```
->
```rust
let (_0, _1) = &(1,2);
let v = _0;
```
BUT: `t.0` and `_0` have different types (`i32` vs. `&i32`) -> `v` has now a different type.
I think that's acceptable: I think the destructure assist is mostly used in simple, immediate scopes and not huge existing code.
Additional Notes:
* `ref` has same behaviour (-> `ref` is kept for items)
```rust
let ref t = (1,2);
// ^
```
->
```rust
let (ref _0, ref _1) = (1,2);
```
* Rust IntelliJ Plugin: doesn't trigger with `&` or `ref` at all
### mutable
```rust
let mut t = (1,2);
// ^
```
->
```rust
let (mut _0, mut _1) = (1,2);
```
and
```rust
let t = &mut (1,2);
// ^
```
->
```rust
let (_0, _1) = &mut (1,2);
```
Again: with reference (`&mut`), `t.0` and `_0` have different types (`i32` vs `&mut i32`).
And there's an additional issue with `&mut` and assignment:
```rust
let t = &mut (1,2);
// ^
t.0 = 9;
```
->
```rust
let (_0, _1) = &mut (1,2);
_0 = 9;
// ^
// mismatched types
// expected `&mut {integer}`, found integer
// consider dereferencing here to assign to the mutable borrowed piece of memory
```
But I think that's quite a niche use case, so I don't catch that (`*_0 = 9;`)
Additional Notes:
* Rust IntelliJ Plugin: removes the `mut` (`let mut t = ...` -> `let (_0, _1) = ...`), doesn't trigger with `&mut`
### Binding after ``@``
Destructure tuple in sub-pattern is implemented:
```rust
let t = (1,2);
// ^
let v = t.0;
let f = t.into();
```
->
```rust
let t @ (_0, _1) = (1,2);
let v = _0;
let f = t.into();
```
BUT: Bindings after ``@`` aren't currently in stable and require `#![feature(bindings_after_at)]` (though should be generally [available quite soon](https://github.com/rust-lang/rust/pull/85305#event-5072889913) (with `1.56.0`)).
But I don't know how to check for an enabled feature -> Destructure tuple in sub-pattern [isn't enabled](a4ee6c7954/crates/ide_assists/src/handlers/destructure_tuple_binding.rs (L32)) yet.
* When Destructure in sub-pattern is enabled there are two assists:
* `Destructure tuple in place`:
```rust
let t = (1,2);
// ^
```
->
```rust
let (_0, _1) = (1,2);
let v = _0;
let f = /*t*/.into();
```
* `Destructure tuple in sub-pattern`:
```rust
let t = (1,2);
// ^
let v = t.0;
let f = t.into();
```
->
```rust
let t @ (_0, _1) = (1,2);
let v = _0;
let f = t.into();
```
* When Destructure in sub-pattern is disabled, only the first one is available and just named `Destructure tuple`
<br/>
<br/>
### Caveats
* Unlike in #8673 or IntelliJ rust plugin, I'm not leaving the previous tuple name at function calls.
**Reasoning**: It's not too unlikely the tuple variable shadows another variable. Destructuring the tuple while leaving the function call untouched, results in still a valid function call -- but now with another variable:
```rust
let t = (8,9);
let t = (1,2);
// ^
t.into()
```
=> Destructure Tuple
```rust
let t = (8,9);
let (_0, _1) = (1,2);
t.into()
```
`t.into()` is still valid -- using the first tuple.
Instead I comment out the tuple usage, which results in invalid code -> must be handled by user:
```rust
/*t*/.into()
```
* (though that might be a biased decision: For testing I just declared a lot of `t`s and quite ofen in lines next to each other...)
* Issue: there are some cases that results in still valid code:
* macro that accept the tuple as well as no arguments:
```rust
macro_rules! m {
() => { "foo" };
($e:expr) => { $e; "foo" };
}
let t = (1,2);
m!(t);
m!(/*t*/);
```
-> both calls are valid ([test](a4ee6c7954/crates/ide_assists/src/handlers/destructure_tuple_binding.rs (L1474)))
* Probably with tuple as return value. Changing the return value most likely results in an error -- but in another place; not where the tuple usage was.
-> not sure that's the best way....
Additional the tuple name surrounded by comment is more difficult to edit than just the name.
* Code Assists don't support snippet placeholder, and rust analyzer just the first `$0` -> unfortunately no editing of generated tuple item variables. Cursor (`$0`) is placed on first generated item.
<br/>
<br/>
### Issues
* Tuple index usage in macro calls aren't converted:
```rust
let t = (1,2);
// ^
let v = t.0;
println!("{}", t.0);
```
->
```rust
let (_0, _1) = (1,2);
let v = _0;
println!("{}", /*t*/.0);
```
([tests](a4ee6c7954/crates/ide_assists/src/handlers/destructure_tuple_binding.rs (L1294)))
* Issue is:
[name.syntax()](a4ee6c7954/crates/ide_assists/src/handlers/destructure_tuple_binding.rs (L242-L244)) in each [usage](a4ee6c7954/crates/ide_assists/src/handlers/destructure_tuple_binding.rs (L108-L113)) of a tuple is syntax & text_range in its file.
EXCEPT when tuple usage is in a macro call (`m!(t.0)`), the macro is expanded and syntax (and range) is based on that expanded macro, not in actual file.
That leads to several things:
* I cannot differentiate between calling the macro with the tuple or with tuple item:
```rust
macro_rules! m {
($t:expr, $i:expr) => { $t.0 + $i };
}
let t = (1,2);
m!(t, t.0);
```
-> both `t` usages are resolved as tuple index usage
* Range of resolved tuple index usage is in expanded macro, not in actual file
-> don't know where to replace index usage
-> tuple items passed into a macro are ignored, and only the tuple name itself is handled (uncommented)
* I'm not checking if the generated names conflict with already existing variables.
```rust
let _0 = 42; // >-|
let t = (1,2); // |
let v = _0; // <-|
// ^ 42
```
=> deconstruct tuple
```rust
let _0 = 42;
let (_0, _1) = (1,2); // >-|
let v = _0; // <-|
// ^ now 1
```
* I tried to get the scope at tuple declaration and its usages. And then iterate all names with [`process_all_names`](145b51f9da/crates/hir/src/semantics.rs (L935)). But that doesn't find all local names for declarations (`let t = (1,2)`) (for usages it does)
* This isn't unique to this Code Assist, but happen in others too (like `extract into variable` or `extract into function`). But here a name conflict is more likely (when destructuring multiple tuples, for examples nested ones (`let t = ((1,2),3)` -> `let (_0, _1) = ...` -> `let ((_0, _1), _1) = ...` -> error))
* IntelliJ rust plugin does handle this (-> name is `_00`)
Co-authored-by: BooksBaum <15612932+Booksbaum@users.noreply.github.com>
Note:
2nd Assist description is moved down: generated doc tests extracts now
all tests (previously only the first one). But it uses the first
`Assist` name -- which is the wrong one for the 2nd test. And 2nd assist
is currently disabled -> would fail anyway.
9921: Only add entries to SourceToDef dynmaps when they come from the same file r=matklad a=Veykril
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/9919
Running the test as described in the issue I do not get any eprintln output at all anymore.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>