Skip to content

Commit

Permalink
Merge #7535
Browse files Browse the repository at this point in the history
7535: Extract function assist r=cpud36 a=cpud36

This PR adds `extract function/method` assist. closes #5409.

# Supported features
Assist should support extracting from expressions(`1`, `2 + 2`, `loop { }`) and from a series of statements, e.g.:
```rust
foo();
$0bar();
baz();$0
quix();
```
Assist also supports extracting parameters, like:
```rust
fn foo() -> i32 {
  let n = 1;
  $0n + 1$0
}

// -
fn foo() -> i32 {
  let n = 1;
  fun_name(n)
}

fn fun_name(n: i32) -> i32 {
  n + 1
}
```

Extracting methods also generally works.

Assist allows referencing outer variables, both mutably and immutably, and handles handles access to variables local to extracted function:
```rust
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
  $0
  n += 1;
  let k = 1;
  moved_v.push(n);
  let r = &mut m;
  ref_mut_v.push(*r);
  let h = 3;
  $0
  n = ref_mut_v.len() + k;
  n -= h + m;
}

// -
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
 
  let (k, h) =  fun_name(&mut n, moved_v, &mut m, &mut ref_mut_v);

  n = ref_mut_v.len() + k;
  n -= h + m;
}

fn fun_name(n: &mut i32, mut moved_v: Vec<i32>, m: &mut i32, ref_mut_v: &mut Vec<i32>) -> (i32, i32) {
  *n += 1;
  let k = 1;
  moved_v.push(*n);
  let r = m;
  ref_mut_v.push(*r);
  let h = 3;
  (k, h)
}
```

So we handle both input and output paramters

# Showcase

![extract_cursor_in_range_3](https://user-images.githubusercontent.com/4218373/106980190-c9870800-6770-11eb-83d9-3d36b2550ff6.gif)
![fill_match_arms_discard_wildcard](https://user-images.githubusercontent.com/4218373/106980197-cbe96200-6770-11eb-96b0-14c27894fac0.gif)
![ide_db_helpers_handle_kind](https://user-images.githubusercontent.com/4218373/106980201-cdb32580-6770-11eb-9e6e-6ac8155d65ac.gif)
![ide_db_imports_location_local_query](https://user-images.githubusercontent.com/4218373/106980205-cf7ce900-6770-11eb-8516-653c8fcca807.gif)

# Working with non-`Copy` types

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0
  let n = v.len();
  $0
  let is_empty = v.is_empty();
}
```
`v` must be a parameter to extracted function. 
The question is, what type should it have.
It could be `v: Vec<i32>`, or `v: &Vec<i32>`. 
The former is incorrect for `Vec<i32>`, but the later is silly for `i32`.

To resolve this we need to know if the type implements `Copy` trait.

I didn't find any api available from assists to query this. 
`hir_ty::method_resolution::implements` seems relevant, but is isn't publicly re-exported from `hir`.

# Star(`*`) token and pointer dereference

If I understand correctly, in order to create expression like `*p`, one should use `ast::make::expr_prefix(T![*], ...)`, which
in turn calls `token(T![*])`.

`token` does not have star in `tokens::SOURCE_FILE`, so this panics.
I had to add `*` to `SOURCE_FILE` to make it work.

Correct me if this is not intended way to do this.

# Lowering access `value -> mut ref -> shared ref`

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0 let n = v.len(); $0
}
```
`v` is not used after extracted function body, so both `v: &Vec<i32>` and `v: Vec<i32>` would work.
Currently the later would be chosen.

We can however check the body of extracted function and conclude that `v: &Vec<i32>` is sufficient.
Using `v: &Vec<i32>`(that is a minimal required access level) might be a better default.
I am unsure.

# Cleanup
The assist seems to be reasonably handling most of common cases.
If there are no concerns with code it produces(i.e. with test cases), I will start cleaning up

[edit]
added showcase


Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
  • Loading branch information
bors[bot] and cpud36 authored Feb 5, 2021
2 parents 842033b + 7eaa3e5 commit ac59584
Show file tree
Hide file tree
Showing 4 changed files with 2,189 additions and 1 deletion.
Loading

0 comments on commit ac59584

Please sign in to comment.