mirror of
https://github.com/rust-lang/rust-analyzer
synced 2024-12-26 13:03:31 +00:00
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 <cpud47@gmail.com>
This commit is contained in:
commit
ac5958485e
4 changed files with 2189 additions and 1 deletions
2159
crates/assists/src/handlers/extract_function.rs
Normal file
2159
crates/assists/src/handlers/extract_function.rs
Normal file
File diff suppressed because it is too large
Load diff
|
@ -117,6 +117,7 @@ mod handlers {
|
||||||
mod convert_integer_literal;
|
mod convert_integer_literal;
|
||||||
mod early_return;
|
mod early_return;
|
||||||
mod expand_glob_import;
|
mod expand_glob_import;
|
||||||
|
mod extract_function;
|
||||||
mod extract_struct_from_enum_variant;
|
mod extract_struct_from_enum_variant;
|
||||||
mod extract_variable;
|
mod extract_variable;
|
||||||
mod fill_match_arms;
|
mod fill_match_arms;
|
||||||
|
@ -174,6 +175,7 @@ mod handlers {
|
||||||
early_return::convert_to_guarded_return,
|
early_return::convert_to_guarded_return,
|
||||||
expand_glob_import::expand_glob_import,
|
expand_glob_import::expand_glob_import,
|
||||||
move_module_to_file::move_module_to_file,
|
move_module_to_file::move_module_to_file,
|
||||||
|
extract_function::extract_function,
|
||||||
extract_struct_from_enum_variant::extract_struct_from_enum_variant,
|
extract_struct_from_enum_variant::extract_struct_from_enum_variant,
|
||||||
extract_variable::extract_variable,
|
extract_variable::extract_variable,
|
||||||
fill_match_arms::fill_match_arms,
|
fill_match_arms::fill_match_arms,
|
||||||
|
|
|
@ -256,6 +256,33 @@ fn qux(bar: Bar, baz: Baz) {}
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn doctest_extract_function() {
|
||||||
|
check_doc_test(
|
||||||
|
"extract_function",
|
||||||
|
r#####"
|
||||||
|
fn main() {
|
||||||
|
let n = 1;
|
||||||
|
$0let m = n + 2;
|
||||||
|
let k = m + n;$0
|
||||||
|
let g = 3;
|
||||||
|
}
|
||||||
|
"#####,
|
||||||
|
r#####"
|
||||||
|
fn main() {
|
||||||
|
let n = 1;
|
||||||
|
fun_name(n);
|
||||||
|
let g = 3;
|
||||||
|
}
|
||||||
|
|
||||||
|
fn $0fun_name(n: i32) {
|
||||||
|
let m = n + 2;
|
||||||
|
let k = m + n;
|
||||||
|
}
|
||||||
|
"#####,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn doctest_extract_struct_from_enum_variant() {
|
fn doctest_extract_struct_from_enum_variant() {
|
||||||
check_doc_test(
|
check_doc_test(
|
||||||
|
|
|
@ -487,7 +487,7 @@ pub mod tokens {
|
||||||
use crate::{ast, AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken};
|
use crate::{ast, AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken};
|
||||||
|
|
||||||
pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> =
|
pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> =
|
||||||
Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true)\n;\n\n"));
|
Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true, *p)\n;\n\n"));
|
||||||
|
|
||||||
pub fn single_space() -> SyntaxToken {
|
pub fn single_space() -> SyntaxToken {
|
||||||
SOURCE_FILE
|
SOURCE_FILE
|
||||||
|
|
Loading…
Reference in a new issue