10440: Fix Clippy warnings and replace some `if let`s with `match` r=Veykril a=arzg
I decided to try fixing a bunch of Clippy warnings. I am aware of this project’s opinion of Clippy (I have read both [rust-lang/clippy#5537](https://github.com/rust-lang/rust-clippy/issues/5537) and [rust-analyzer/rowan#57 (comment)](https://github.com/rust-analyzer/rowan/pull/57#discussion_r415676159)), so I totally understand if part of or the entirety of this PR is rejected. In particular, I can see how the semicolons and `if let` vs `match` commits provide comparatively little benefit when compared to the ensuing churn.
I tried to separate each kind of change into its own commit to make it easier to discard certain changes. I also only applied Clippy suggestions where I thought they provided a definite improvement to the code (apart from semicolons, which is IMO more of a formatting/consistency question than a linting question). In the end I accumulated a list of 28 Clippy lints I ignored entirely.
Sidenote: I should really have asked about this on Zulip before going through all 1,555 `if let`s in the codebase to decide which ones definitely look better as `match` :P
Co-authored-by: Aramis Razzaghipour <aramisnoah@gmail.com>
Consider these expples
{ 92 }
async { 92 }
'a: { 92 }
#[a] { 92 }
Previously the tree for them were
BLOCK_EXPR
{ ... }
EFFECT_EXPR
async
BLOCK_EXPR
{ ... }
EFFECT_EXPR
'a:
BLOCK_EXPR
{ ... }
BLOCK_EXPR
#[a]
{ ... }
As you see, it gets progressively worse :) The last two items are
especially odd. The last one even violates the balanced curleys
invariant we have (#10357) The new approach is to say that the stuff in
`{}` is stmt_list, and the block is stmt_list + optional modifiers
BLOCK_EXPR
STMT_LIST
{ ... }
BLOCK_EXPR
async
STMT_LIST
{ ... }
BLOCK_EXPR
'a:
STMT_LIST
{ ... }
BLOCK_EXPR
#[a]
STMT_LIST
{ ... }
9944: internal: introduce in-place indenting API r=matklad a=iDawer
Introduce `edit_in_place::Indent` that uses mutable tree API and intended to replace `edit::AstNodeEdit`.
Closes#9903
Co-authored-by: Dawer <7803845+iDawer@users.noreply.github.com>
10001: Sort enum variant r=Veykril a=vsrs
A small fix to the problem noted by `@lnicola` :
> 
>
> (note the slight inconsistency here: to sort the variants of `Animal` I have to select the enum name, but to sort the fields of `Cat` I have to select the fields themselves)
Co-authored-by: vsrs <vit@conrlab.com>
9972: refactor : function generation assists r=Veykril a=mahdi-frms
Separated code generation from finding position for generated code. This will be ground work for introducing static associated function generation.
Co-authored-by: mahdi-frms <mahdif1380@outlook.com>
9962: Add empty-body check to replace_match_with_if_let and re-prioritize choices r=elkowar a=elkowar
This PR changes some behaviour of the `replace_match_with_if_let` ide-assist.
Concretely, it makes two changes:
it introduces a check for empty expression bodies. This means that checks of the shape
```rs
match x {
A => {}
B => {
println!("hi");
}
}
```
will prefer to use the B branch as the first (and only) variant.
It also reprioritizes the importance of "happy" and "sad" patterns.
Concretely, if there are reasons to prefer having the sad pattern be the first (/only) pattern,
it will follow these.
This means that in the case of
```rs
match x {
Ok(_) => {
println!("Success");
}
Err(e) => {
println!("Failure: {}", e);
}
}
```
the `Err` variant will correctly be used as the first expression in the generated if.
Up until now, the generated code was actually invalid, as it would generate
```rs
if let Ok(_) = x {
println!("Success");
} else {
println!("Failure: {}", e);
}
```
where `e` in the else branch is not defined.
Co-authored-by: elkowar <5300871+elkowar@users.noreply.github.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:
* 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>
