diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-02-05 02:55:56 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2021-02-05 02:55:56 +0000 |
commit | ac5958485e5e77b020b3ebe0c100df5a5fe6ca08 (patch) | |
tree | fd76eca27628f746269d3d7478226d88e595e3aa /crates/syntax | |
parent | 842033b15055eba9aabfc730468cd076a30a5f29 (diff) | |
parent | 7eaa3e56a01e9a275129c76817232559b0e20f2b (diff) |
Merge #7535
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 <[email protected]>
Diffstat (limited to 'crates/syntax')
-rw-r--r-- | crates/syntax/src/ast/make.rs | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index b755c9692..1da5a125e 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs | |||
@@ -487,7 +487,7 @@ pub mod tokens { | |||
487 | use crate::{ast, AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken}; | 487 | use crate::{ast, AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken}; |
488 | 488 | ||
489 | pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> = | 489 | pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> = |
490 | Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true)\n;\n\n")); | 490 | Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true, *p)\n;\n\n")); |
491 | 491 | ||
492 | pub fn single_space() -> SyntaxToken { | 492 | pub fn single_space() -> SyntaxToken { |
493 | SOURCE_FILE | 493 | SOURCE_FILE |