From 3973d1aaa75b684d2ebd3a77ca6a0845928954a6 Mon Sep 17 00:00:00 2001 From: Mathew Horner Date: Sat, 22 Oct 2022 16:49:37 -0500 Subject: [PATCH 1/4] Add assist to convert nested function to closure --- .../convert_nested_function_to_closure.rs | 185 ++++++++++++++++++ crates/ide-assists/src/lib.rs | 4 +- crates/ide-assists/src/tests/generated.rs | 25 +++ 3 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs diff --git a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs new file mode 100644 index 0000000000..0524c27890 --- /dev/null +++ b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs @@ -0,0 +1,185 @@ +use ide_db::assists::{AssistId, AssistKind}; +use syntax::ast::{self, HasGenericParams, HasName}; +use syntax::{AstNode, SyntaxKind}; + +use crate::assist_context::{AssistContext, Assists}; + +// Assist: convert_nested_function_to_closure +// +// Converts a function that is defined within the body of another function into a closure. +// +// ``` +// fn main() { +// fn fo$0o(label: &str, number: u64) { +// println!("{}: {}", label, number); +// } +// +// foo("Bar", 100); +// } +// ``` +// -> +// ``` +// fn main() { +// let foo = |label: &str, number: u64| { +// println!("{}: {}", label, number); +// }; +// +// foo("Bar", 100); +// } +// ``` +pub(crate) fn convert_nested_function_to_closure( + acc: &mut Assists, + ctx: &AssistContext<'_>, +) -> Option<()> { + let name = ctx.find_node_at_offset::()?; + let function = name.syntax().parent().and_then(ast::Fn::cast)?; + + if !is_nested_function(&function) || is_generic(&function) { + return None; + } + + let target = function.syntax().text_range(); + let body = function.body()?; + let name = function.name()?; + let params = function.param_list()?; + + acc.add( + AssistId("convert_nested_function_to_closure", AssistKind::RefactorRewrite), + "Convert nested function to closure", + target, + |edit| { + let has_semicolon = has_semicolon(&function); + let params_text = params.syntax().text().to_string(); + let params_text_trimmed = + params_text.strip_prefix("(").and_then(|p| p.strip_suffix(")")); + + if let Some(closure_params) = params_text_trimmed { + let body = body.to_string(); + let body = if has_semicolon { body } else { format!("{};", body) }; + edit.replace(target, format!("let {} = |{}| {}", name, closure_params, body)); + } + }, + ) +} + +/// Returns whether the given function is nested within the body of another function. +fn is_nested_function(function: &ast::Fn) -> bool { + function + .syntax() + .parent() + .map(|p| p.ancestors().any(|a| a.kind() == SyntaxKind::FN)) + .unwrap_or(false) +} + +/// Returns whether the given nested function has generic parameters. +fn is_generic(function: &ast::Fn) -> bool { + function.generic_param_list().is_some() +} + +/// Returns whether the given nested function has a trailing semicolon. +fn has_semicolon(function: &ast::Fn) -> bool { + function + .syntax() + .next_sibling_or_token() + .map(|t| t.kind() == SyntaxKind::SEMICOLON) + .unwrap_or(false) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::convert_nested_function_to_closure; + + #[test] + fn convert_nested_function_to_closure_works() { + check_assist( + convert_nested_function_to_closure, + r#" +fn main() { + fn $0foo(a: u64, b: u64) -> u64 { + 2 * (a + b) + } + + _ = foo(3, 4); +} + "#, + r#" +fn main() { + let foo = |a: u64, b: u64| { + 2 * (a + b) + }; + + _ = foo(3, 4); +} + "#, + ); + } + + #[test] + fn convert_nested_function_to_closure_works_with_existing_semicolon() { + check_assist( + convert_nested_function_to_closure, + r#" +fn main() { + fn foo$0(a: u64, b: u64) -> u64 { + 2 * (a + b) + }; + + _ = foo(3, 4); +} + "#, + r#" +fn main() { + let foo = |a: u64, b: u64| { + 2 * (a + b) + }; + + _ = foo(3, 4); +} + "#, + ); + } + + #[test] + fn convert_nested_function_to_closure_does_not_work_on_top_level_function() { + check_assist_not_applicable( + convert_nested_function_to_closure, + r#" +fn ma$0in() {} + "#, + ); + } + + #[test] + fn convert_nested_function_to_closure_does_not_work_when_cursor_off_name() { + check_assist_not_applicable( + convert_nested_function_to_closure, + r#" +fn main() { + fn foo(a: u64, $0b: u64) -> u64 { + 2 * (a + b) + }; + + _ = foo(3, 4); +} + "#, + ); + } + + #[test] + fn convert_nested_function_to_closure_does_not_work_if_function_has_generic_params() { + check_assist_not_applicable( + convert_nested_function_to_closure, + r#" +fn main() { + fn fo$0o>(s: S) -> String { + s.into() + }; + + _ = foo("hello"); +} + "#, + ); + } +} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 8b07e29a58..fc03903e59 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -122,6 +122,7 @@ mod handlers { mod convert_iter_for_each_to_for; mod convert_let_else_to_match; mod convert_match_to_let_else; + mod convert_nested_function_to_closure; mod convert_tuple_struct_to_named_struct; mod convert_named_struct_to_tuple_struct; mod convert_to_guarded_return; @@ -228,8 +229,9 @@ mod handlers { convert_iter_for_each_to_for::convert_iter_for_each_to_for, convert_iter_for_each_to_for::convert_for_loop_with_for_each, convert_let_else_to_match::convert_let_else_to_match, - convert_named_struct_to_tuple_struct::convert_named_struct_to_tuple_struct, convert_match_to_let_else::convert_match_to_let_else, + convert_named_struct_to_tuple_struct::convert_named_struct_to_tuple_struct, + convert_nested_function_to_closure::convert_nested_function_to_closure, convert_to_guarded_return::convert_to_guarded_return, convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, convert_two_arm_bool_match_to_matches_macro::convert_two_arm_bool_match_to_matches_macro, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index dd8705d797..993e7c371f 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -494,6 +494,31 @@ impl Point { ) } +#[test] +fn doctest_convert_nested_function_to_closure() { + check_doc_test( + "convert_nested_function_to_closure", + r#####" +fn main() { + fn fo$0o(label: &str, number: u64) { + println!("{}: {}", label, number); + } + + foo("Bar", 100); +} +"#####, + r#####" +fn main() { + let foo = |label: &str, number: u64| { + println!("{}: {}", label, number); + }; + + foo("Bar", 100); +} +"#####, + ) +} + #[test] fn doctest_convert_to_guarded_return() { check_doc_test( From 0e11d507e10f64b1575cb2b65e0b0632eb1c3c8b Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Fri, 31 Mar 2023 12:20:08 +0200 Subject: [PATCH 2/4] Address review comment --- .../src/handlers/convert_nested_function_to_closure.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs index 0524c27890..a9fd12922a 100644 --- a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs +++ b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs @@ -66,9 +66,10 @@ pub(crate) fn convert_nested_function_to_closure( fn is_nested_function(function: &ast::Fn) -> bool { function .syntax() - .parent() - .map(|p| p.ancestors().any(|a| a.kind() == SyntaxKind::FN)) - .unwrap_or(false) + .ancestors() + .skip(1) + .find_map(ast::Item::cast) + .map_or(false, |it| matches!(it, ast::Item::Fn(_))) } /// Returns whether the given nested function has generic parameters. From d01a38ce3f10158f8ecb2f15278c0b0d711fcf27 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Fri, 31 Mar 2023 13:53:57 +0200 Subject: [PATCH 3/4] Address second round of review comments --- .../convert_nested_function_to_closure.rs | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs index a9fd12922a..fb79ec3e50 100644 --- a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs +++ b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs @@ -34,7 +34,7 @@ pub(crate) fn convert_nested_function_to_closure( let name = ctx.find_node_at_offset::()?; let function = name.syntax().parent().and_then(ast::Fn::cast)?; - if !is_nested_function(&function) || is_generic(&function) { + if !is_nested_function(&function) || is_generic(&function) || has_modifiers(&function) { return None; } @@ -43,21 +43,21 @@ pub(crate) fn convert_nested_function_to_closure( let name = function.name()?; let params = function.param_list()?; + let params_text = params.syntax().text().to_string(); + let closure_params = params_text.strip_prefix("(").and_then(|p| p.strip_suffix(")"))?; + acc.add( AssistId("convert_nested_function_to_closure", AssistKind::RefactorRewrite), "Convert nested function to closure", target, |edit| { let has_semicolon = has_semicolon(&function); - let params_text = params.syntax().text().to_string(); - let params_text_trimmed = - params_text.strip_prefix("(").and_then(|p| p.strip_suffix(")")); - if let Some(closure_params) = params_text_trimmed { - let body = body.to_string(); - let body = if has_semicolon { body } else { format!("{};", body) }; - edit.replace(target, format!("let {} = |{}| {}", name, closure_params, body)); + let mut body = body.to_string(); + if !has_semicolon { + body.push(';'); } + edit.replace(target, format!("let {} = |{}| {}", name, closure_params, body)); }, ) } @@ -77,6 +77,17 @@ fn is_generic(function: &ast::Fn) -> bool { function.generic_param_list().is_some() } +/// Returns whether the given nested function has any modifiers: +/// +/// - `async`, +/// - `const` or +/// - `unsafe` +fn has_modifiers(function: &ast::Fn) -> bool { + function.async_token().is_some() + || function.const_token().is_some() + || function.unsafe_token().is_some() +} + /// Returns whether the given nested function has a trailing semicolon. fn has_semicolon(function: &ast::Fn) -> bool { function @@ -143,7 +154,7 @@ fn main() { } #[test] - fn convert_nested_function_to_closure_does_not_work_on_top_level_function() { + fn convert_nested_function_to_closure_is_not_suggested_on_top_level_function() { check_assist_not_applicable( convert_nested_function_to_closure, r#" @@ -153,14 +164,14 @@ fn ma$0in() {} } #[test] - fn convert_nested_function_to_closure_does_not_work_when_cursor_off_name() { + fn convert_nested_function_to_closure_is_not_suggested_when_cursor_off_name() { check_assist_not_applicable( convert_nested_function_to_closure, r#" fn main() { fn foo(a: u64, $0b: u64) -> u64 { 2 * (a + b) - }; + } _ = foo(3, 4); } @@ -169,14 +180,30 @@ fn main() { } #[test] - fn convert_nested_function_to_closure_does_not_work_if_function_has_generic_params() { + fn convert_nested_function_to_closure_is_not_suggested_if_function_has_generic_params() { check_assist_not_applicable( convert_nested_function_to_closure, r#" fn main() { fn fo$0o>(s: S) -> String { s.into() - }; + } + + _ = foo("hello"); +} + "#, + ); + } + + #[test] + fn convert_nested_function_to_closure_is_not_suggested_if_function_has_modifier() { + check_assist_not_applicable( + convert_nested_function_to_closure, + r#" +fn main() { + const fn fo$0o(s: String) -> String { + s + } _ = foo("hello"); } From bc704e127de2b9ffce384b63812dae6b1d268024 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 5 Apr 2023 18:48:21 +0200 Subject: [PATCH 4/4] Address another round of review comments --- .../convert_nested_function_to_closure.rs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs index fb79ec3e50..399f87c8f5 100644 --- a/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs +++ b/crates/ide-assists/src/handlers/convert_nested_function_to_closure.rs @@ -41,35 +41,31 @@ pub(crate) fn convert_nested_function_to_closure( let target = function.syntax().text_range(); let body = function.body()?; let name = function.name()?; - let params = function.param_list()?; - - let params_text = params.syntax().text().to_string(); - let closure_params = params_text.strip_prefix("(").and_then(|p| p.strip_suffix(")"))?; + let param_list = function.param_list()?; acc.add( AssistId("convert_nested_function_to_closure", AssistKind::RefactorRewrite), "Convert nested function to closure", target, |edit| { - let has_semicolon = has_semicolon(&function); + let params = ¶m_list.syntax().text().to_string(); + let params = params.strip_prefix("(").unwrap_or(params); + let params = params.strip_suffix(")").unwrap_or(params); let mut body = body.to_string(); - if !has_semicolon { + if !has_semicolon(&function) { body.push(';'); } - edit.replace(target, format!("let {} = |{}| {}", name, closure_params, body)); + edit.replace(target, format!("let {name} = |{params}| {body}")); }, ) } /// Returns whether the given function is nested within the body of another function. fn is_nested_function(function: &ast::Fn) -> bool { - function - .syntax() - .ancestors() - .skip(1) - .find_map(ast::Item::cast) - .map_or(false, |it| matches!(it, ast::Item::Fn(_))) + function.syntax().ancestors().skip(1).find_map(ast::Item::cast).map_or(false, |it| { + matches!(it, ast::Item::Fn(_) | ast::Item::Static(_) | ast::Item::Const(_)) + }) } /// Returns whether the given nested function has generic parameters.