Commit Graph

553 Commits

Author SHA1 Message Date
Lukas Wirth
5c3a93ca9e Remove incorrect assertion in extract_function 2021-08-27 19:58:57 +02:00
bors[bot]
6287d388c0
Merge #9944
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>
2021-08-24 13:11:50 +00:00
bors[bot]
43021525e5
Merge #9994
9994: add static method generation assist r=matklad a=mahdi-frms

Adds feature: #9948

Will soon send a GIF for the changelog.

Co-authored-by: mahdi-frms <mahdif1380@outlook.com>
2021-08-24 09:26:52 +00:00
bors[bot]
ced65f77c4
Merge #10001
10001: Sort enum variant r=Veykril a=vsrs

A small fix to the problem noted by `@lnicola` :

> ![sort-fields](https://user-images.githubusercontent.com/308347/129513196-4ffc7937-be58-44d4-9ec7-ba8745dcb460.gif)
> 
> (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>
2021-08-23 22:19:50 +00:00
vsrs
7a8f881a09 Sort enum variant 2021-08-23 20:31:54 +03:00
mahdi-frms
cf5f1c102c add static method generation assist 2021-08-23 12:14:23 +04:30
bors[bot]
107b2f1890
Merge #9972
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>
2021-08-22 14:53:41 +00:00
mahdi-frms
c1edbfbacc better assist label 2021-08-22 12:31:16 +04:30
mahdi-frms
50923ad741 func gen: seperate generation form position(3) 2021-08-21 19:13:14 +04:30
mahdi-frms
1ac9400100 func gen: seperate generation form position(2) 2021-08-21 17:31:37 +04:30
bors[bot]
4aa2a44a55
Merge #9962
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>
2021-08-21 10:12:17 +00:00
elkowar
e47c9743cf
Fix smaller nitpicks 2021-08-21 12:02:21 +02:00
elkowar
7cff930393
Use NameClass::classify to check for ConstReference 2021-08-21 11:11:27 +02:00
elkowar
75f0701211
Add heuristic to determine type of IdentPat, make check for empty expressions correct 2021-08-21 11:00:43 +02:00
mahdi-frms
87439b1d6a func gen: seperate generation form position(1) 2021-08-21 13:29:15 +04:30
mahdi-frms
e2c233a42b simplify 2021-08-21 11:52:07 +04:30
elkowar
d6695286ee
Add empty-body check to replace_match_with_if_let and re-prioritize choices 2021-08-20 15:20:54 +02:00
bors[bot]
59aa091866
Merge #9855
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>
2021-08-19 15:19:06 +00:00
BooksBaum
8a9feeddd3
Remove match_ast usage 2021-08-19 14:17:23 +02:00
BooksBaum
2c27adc0a3
Remove match_ast! macro call
Add node about uncommenting tuple in macro call
2021-08-19 14:09:52 +02:00
Yoshua Wuyts
586d6fca8c Fix codegen for is_method documentation
Previously the generated paths were invalid. This fixes that.
2021-08-19 13:31:16 +02:00
Dawer
08694dc1ef minor: address review comments 2021-08-18 19:04:10 +05:00
Dawer
98847e4d5b internal: use the new edit_in_place::Indent API 2021-08-18 18:08:14 +05:00
BooksBaum
b1ebb82f32
Add deref (*) handling for usages of ref tuples 2021-08-17 19:23:07 +02:00
BooksBaum
384fae7fcd
Switch order of assists
Destructure in sub-pattern before Destructure in place to favor the
first one
2021-08-17 19:23:06 +02:00
BooksBaum
d0cf28322a
Add generated doctest 2021-08-17 19:23:06 +02:00
BooksBaum
5b9f8e7e8e
Fix errors from rebase master
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.
2021-08-17 19:23:06 +02:00
BooksBaum
b441aa2046
Cleanup according to style-requirements in tests 2021-08-17 19:23:06 +02:00
BooksBaum
45ef57bd23
Fix: different assist ids in doc and code 2021-08-17 19:23:05 +02:00
BooksBaum
0d2490f785
Handle tuple in macro call
Only tuple name is handled (uncommented), not tuple index
2021-08-17 19:23:05 +02:00
BooksBaum
450a9c5b12
Add destructure in sub-pattern (after @) 2021-08-17 19:23:05 +02:00
BooksBaum
3e4051938d
Handle mut & ref 2021-08-17 19:23:04 +02:00
BooksBaum
478b2988c0
Add Destructure Tuple 2021-08-17 19:22:57 +02:00
Laurențiu Nicola
c9f448a834 Fix some clippy lints 2021-08-16 22:04:26 +03:00
Yoshua Wuyts
067dc660ae implement feedback from review 2021-08-16 18:23:51 +02:00
Yoshua Wuyts
9374d526da fix debug tuple structs 2021-08-16 17:39:08 +02:00
Yoshua Wuyts
b67378f53e fix debug record structs 2021-08-16 12:58:06 +02:00
Aleksey Kladov
3c49a9f079 minor: move functionality to a better place 2021-08-14 20:43:28 +03:00
Aleksey Kladov
e300f58d2c internal: remove one more usage of old editing API. 2021-08-14 20:17:16 +03:00
Aleksey Kladov
a26b1c3923 internal: remove old editing API 2021-08-14 19:45:47 +03:00
bors[bot]
5d7ff6efe3
Merge #9894
9894: internal: use standard test style r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
2021-08-14 15:59:47 +00:00
Aleksey Kladov
c3bbeb947c internal: use standard test style 2021-08-14 18:53:21 +03:00
Lukas Wirth
37ad9cb2a5 Don't use non cache syntaxnodes in generate_function for lookups 2021-08-14 17:51:11 +02:00
Aleksey Kladov
a78decc466 internal: make assist naming consistent 2021-08-14 18:39:56 +03:00
Aleksey Kladov
dc17b35e62 internal: remove a remnant of old editing infra 2021-08-14 18:24:42 +03:00
Aleksey Kladov
90357a9090 internal: merge hir::BinaryOp and ast::BinOp 2021-08-14 18:10:01 +03:00
Aleksey Kladov
6df00f8495 internal: make naming consistent 2021-08-14 17:01:28 +03:00
Aleksey Kladov
beca92b245 internal: make invert binary op more robust
Previously, we only inverted comparison operators (< and the like) if
the type implemented Ord. This doesn't make sense: if `<` works, then
`>=` will work as well!

Extra semantic checks greatly reduce robustness and predictability of
the assist, it's better to keep things simple.
2021-08-14 16:40:00 +03:00
bors[bot]
43af7920af
Merge #9871
9871: Jump to generated func r=mahdi-frms a=rylev

Worked on this with `@yoshuawuyts.`

We thought we ran into an issue with the `generate_function` assist where the code was not being generated in a certain situations. However, it wasn't actually a bug just a very confusing implementation where the cursor is not moved to the newly generated function. This happened when the return type was successfully inferred (and not unit). The function would be generated, but selection would not be changed.

This can be very confusing: If the function is generated somewhat far from where the assist is being invoked, the user never sees that the code was generated (nor are they given the chance to actually implement the function body). 

This PR makes it so that the cursor is _always_ moved to somewhere in the newly generated function. In addition, if we can infer unit as the type, then we do not still generate `-> ()` as the return type. Instead, we simply omit the return type.

This means the selection will move to either one of two places:
* A generated `-> ()` return type when we cannot successfully infer the return type.
* The `todo!()` body when we can successfully infer the return type.

Co-authored-by: Ryan Levick <me@ryanlevick.com>
2021-08-13 11:19:16 +00:00
Ryan Levick
1cc43ab7f4 Add documentation on how whether a generated return type should be focused or not 2021-08-13 11:46:08 +02:00