diff --git a/crates/assists/src/handlers/change_return_type_to_result.rs b/crates/assists/src/handlers/change_return_type_to_result.rs deleted file mode 100644 index 76f33a5b6d..0000000000 --- a/crates/assists/src/handlers/change_return_type_to_result.rs +++ /dev/null @@ -1,1091 +0,0 @@ -use std::iter; - -use syntax::{ - ast::{self, make, BlockExpr, Expr, LoopBodyOwner}, - match_ast, AstNode, SyntaxNode, -}; -use test_utils::mark; - -use crate::{AssistContext, AssistId, AssistKind, Assists}; - -// Assist: change_return_type_to_result -// -// Change the function's return type to Result. -// -// ``` -// fn foo() -> i32<|> { 42i32 } -// ``` -// -> -// ``` -// fn foo() -> Result { Ok(42i32) } -// ``` -pub(crate) fn change_return_type_to_result(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let ret_type = ctx.find_node_at_offset::()?; - let parent = ret_type.syntax().parent()?; - let block_expr = match_ast! { - match parent { - ast::Fn(func) => func.body()?, - ast::ClosureExpr(closure) => match closure.body()? { - Expr::BlockExpr(block) => block, - // closures require a block when a return type is specified - _ => return None, - }, - _ => return None, - } - }; - - let type_ref = &ret_type.ty()?; - let ret_type_str = type_ref.syntax().text().to_string(); - let first_part_ret_type = ret_type_str.splitn(2, '<').next(); - if let Some(ret_type_first_part) = first_part_ret_type { - if ret_type_first_part.ends_with("Result") { - mark::hit!(change_return_type_to_result_simple_return_type_already_result); - return None; - } - } - - acc.add( - AssistId("change_return_type_to_result", AssistKind::RefactorRewrite), - "Wrap return type in Result", - type_ref.syntax().text_range(), - |builder| { - let mut tail_return_expr_collector = TailReturnCollector::new(); - tail_return_expr_collector.collect_jump_exprs(&block_expr, false); - tail_return_expr_collector.collect_tail_exprs(&block_expr); - - for ret_expr_arg in tail_return_expr_collector.exprs_to_wrap { - let ok_wrapped = make::expr_call( - make::expr_path(make::path_unqualified(make::path_segment(make::name_ref( - "Ok", - )))), - make::arg_list(iter::once(ret_expr_arg.clone())), - ); - builder.replace_ast(ret_expr_arg, ok_wrapped); - } - - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = format!("Result<{}, ${{0:_}}>", type_ref); - builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet) - } - None => builder - .replace(type_ref.syntax().text_range(), format!("Result<{}, _>", type_ref)), - } - }, - ) -} - -struct TailReturnCollector { - exprs_to_wrap: Vec, -} - -impl TailReturnCollector { - fn new() -> Self { - Self { exprs_to_wrap: vec![] } - } - /// Collect all`return` expression - fn collect_jump_exprs(&mut self, block_expr: &BlockExpr, collect_break: bool) { - let statements = block_expr.statements(); - for stmt in statements { - let expr = match &stmt { - ast::Stmt::ExprStmt(stmt) => stmt.expr(), - ast::Stmt::LetStmt(stmt) => stmt.initializer(), - ast::Stmt::Item(_) => continue, - }; - if let Some(expr) = &expr { - self.handle_exprs(expr, collect_break); - } - } - - // Browse tail expressions for each block - if let Some(expr) = block_expr.expr() { - if let Some(last_exprs) = get_tail_expr_from_block(&expr) { - for last_expr in last_exprs { - let last_expr = match last_expr { - NodeType::Node(expr) => expr, - NodeType::Leaf(expr) => expr.syntax().clone(), - }; - - if let Some(last_expr) = Expr::cast(last_expr.clone()) { - self.handle_exprs(&last_expr, collect_break); - } else if let Some(expr_stmt) = ast::Stmt::cast(last_expr) { - let expr_stmt = match &expr_stmt { - ast::Stmt::ExprStmt(stmt) => stmt.expr(), - ast::Stmt::LetStmt(stmt) => stmt.initializer(), - ast::Stmt::Item(_) => None, - }; - if let Some(expr) = &expr_stmt { - self.handle_exprs(expr, collect_break); - } - } - } - } - } - } - - fn handle_exprs(&mut self, expr: &Expr, collect_break: bool) { - match expr { - Expr::BlockExpr(block_expr) => { - self.collect_jump_exprs(&block_expr, collect_break); - } - Expr::ReturnExpr(ret_expr) => { - if let Some(ret_expr_arg) = &ret_expr.expr() { - self.exprs_to_wrap.push(ret_expr_arg.clone()); - } - } - Expr::BreakExpr(break_expr) if collect_break => { - if let Some(break_expr_arg) = &break_expr.expr() { - self.exprs_to_wrap.push(break_expr_arg.clone()); - } - } - Expr::IfExpr(if_expr) => { - for block in if_expr.blocks() { - self.collect_jump_exprs(&block, collect_break); - } - } - Expr::LoopExpr(loop_expr) => { - if let Some(block_expr) = loop_expr.loop_body() { - self.collect_jump_exprs(&block_expr, collect_break); - } - } - Expr::ForExpr(for_expr) => { - if let Some(block_expr) = for_expr.loop_body() { - self.collect_jump_exprs(&block_expr, collect_break); - } - } - Expr::WhileExpr(while_expr) => { - if let Some(block_expr) = while_expr.loop_body() { - self.collect_jump_exprs(&block_expr, collect_break); - } - } - Expr::MatchExpr(match_expr) => { - if let Some(arm_list) = match_expr.match_arm_list() { - arm_list.arms().filter_map(|match_arm| match_arm.expr()).for_each(|expr| { - self.handle_exprs(&expr, collect_break); - }); - } - } - _ => {} - } - } - - fn collect_tail_exprs(&mut self, block: &BlockExpr) { - if let Some(expr) = block.expr() { - self.handle_exprs(&expr, true); - self.fetch_tail_exprs(&expr); - } - } - - fn fetch_tail_exprs(&mut self, expr: &Expr) { - if let Some(exprs) = get_tail_expr_from_block(expr) { - for node_type in &exprs { - match node_type { - NodeType::Leaf(expr) => { - self.exprs_to_wrap.push(expr.clone()); - } - NodeType::Node(expr) => { - if let Some(last_expr) = Expr::cast(expr.clone()) { - self.fetch_tail_exprs(&last_expr); - } - } - } - } - } - } -} - -#[derive(Debug)] -enum NodeType { - Leaf(ast::Expr), - Node(SyntaxNode), -} - -/// Get a tail expression inside a block -fn get_tail_expr_from_block(expr: &Expr) -> Option> { - match expr { - Expr::IfExpr(if_expr) => { - let mut nodes = vec![]; - for block in if_expr.blocks() { - if let Some(block_expr) = block.expr() { - if let Some(tail_exprs) = get_tail_expr_from_block(&block_expr) { - nodes.extend(tail_exprs); - } - } else if let Some(last_expr) = block.syntax().last_child() { - nodes.push(NodeType::Node(last_expr)); - } else { - nodes.push(NodeType::Node(block.syntax().clone())); - } - } - Some(nodes) - } - Expr::LoopExpr(loop_expr) => { - loop_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) - } - Expr::ForExpr(for_expr) => { - for_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) - } - Expr::WhileExpr(while_expr) => { - while_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) - } - Expr::BlockExpr(block_expr) => { - block_expr.expr().map(|lc| vec![NodeType::Node(lc.syntax().clone())]) - } - Expr::MatchExpr(match_expr) => { - let arm_list = match_expr.match_arm_list()?; - let arms: Vec = arm_list - .arms() - .filter_map(|match_arm| match_arm.expr()) - .map(|expr| match expr { - Expr::ReturnExpr(ret_expr) => NodeType::Node(ret_expr.syntax().clone()), - Expr::BreakExpr(break_expr) => NodeType::Node(break_expr.syntax().clone()), - _ => match expr.syntax().last_child() { - Some(last_expr) => NodeType::Node(last_expr), - None => NodeType::Node(expr.syntax().clone()), - }, - }) - .collect(); - - Some(arms) - } - Expr::BreakExpr(expr) => expr.expr().map(|e| vec![NodeType::Leaf(e)]), - Expr::ReturnExpr(ret_expr) => Some(vec![NodeType::Node(ret_expr.syntax().clone())]), - - Expr::CallExpr(_) - | Expr::Literal(_) - | Expr::TupleExpr(_) - | Expr::ArrayExpr(_) - | Expr::ParenExpr(_) - | Expr::PathExpr(_) - | Expr::RecordExpr(_) - | Expr::IndexExpr(_) - | Expr::MethodCallExpr(_) - | Expr::AwaitExpr(_) - | Expr::CastExpr(_) - | Expr::RefExpr(_) - | Expr::PrefixExpr(_) - | Expr::RangeExpr(_) - | Expr::BinExpr(_) - | Expr::MacroCall(_) - | Expr::BoxExpr(_) => Some(vec![NodeType::Leaf(expr.clone())]), - _ => None, - } -} - -#[cfg(test)] -mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; - - use super::*; - - #[test] - fn change_return_type_to_result_simple() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i3<|>2 { - let test = "test"; - return 42i32; - }"#, - r#"fn foo() -> Result { - let test = "test"; - return Ok(42i32); - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_closure() { - check_assist( - change_return_type_to_result, - r#"fn foo() { - || -> i32<|> { - let test = "test"; - return 42i32; - }; - }"#, - r#"fn foo() { - || -> Result { - let test = "test"; - return Ok(42i32); - }; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_return_type_bad_cursor() { - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() -> i32 { - let test = "test";<|> - return 42i32; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_return_type_bad_cursor_closure() { - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() { - || -> i32 { - let test = "test";<|> - return 42i32; - }; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_closure_non_block() { - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() { - || -> i<|>32 3; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_return_type_already_result_std() { - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() -> std::result::Result, String> { - let test = "test"; - return 42i32; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_return_type_already_result() { - mark::check!(change_return_type_to_result_simple_return_type_already_result); - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() -> Result, String> { - let test = "test"; - return 42i32; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_return_type_already_result_closure() { - check_assist_not_applicable( - change_return_type_to_result, - r#"fn foo() { - || -> Result, String> { - let test = "test"; - return 42i32; - }; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_cursor() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> <|>i32 { - let test = "test"; - return 42i32; - }"#, - r#"fn foo() -> Result { - let test = "test"; - return Ok(42i32); - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail() { - check_assist( - change_return_type_to_result, - r#"fn foo() -><|> i32 { - let test = "test"; - 42i32 - }"#, - r#"fn foo() -> Result { - let test = "test"; - Ok(42i32) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_closure() { - check_assist( - change_return_type_to_result, - r#"fn foo() { - || -><|> i32 { - let test = "test"; - 42i32 - }; - }"#, - r#"fn foo() { - || -> Result { - let test = "test"; - Ok(42i32) - }; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_only() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - 42i32 - }"#, - r#"fn foo() -> Result { - Ok(42i32) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_block_like() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - if true { - 42i32 - } else { - 24i32 - } - }"#, - r#"fn foo() -> Result { - if true { - Ok(42i32) - } else { - Ok(24i32) - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_without_block_closure() { - check_assist( - change_return_type_to_result, - r#"fn foo() { - || -> i32<|> { - if true { - 42i32 - } else { - 24i32 - } - }; - }"#, - r#"fn foo() { - || -> Result { - if true { - Ok(42i32) - } else { - Ok(24i32) - } - }; - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_nested_if() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - if true { - if false { - 1 - } else { - 2 - } - } else { - 24i32 - } - }"#, - r#"fn foo() -> Result { - if true { - if false { - Ok(1) - } else { - Ok(2) - } - } else { - Ok(24i32) - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_await() { - check_assist( - change_return_type_to_result, - r#"async fn foo() -> i<|>32 { - if true { - if false { - 1.await - } else { - 2.await - } - } else { - 24i32.await - } - }"#, - r#"async fn foo() -> Result { - if true { - if false { - Ok(1.await) - } else { - Ok(2.await) - } - } else { - Ok(24i32.await) - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_array() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> [i32;<|> 3] { - [1, 2, 3] - }"#, - r#"fn foo() -> Result<[i32; 3], ${0:_}> { - Ok([1, 2, 3]) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_cast() { - check_assist( - change_return_type_to_result, - r#"fn foo() -<|>> i32 { - if true { - if false { - 1 as i32 - } else { - 2 as i32 - } - } else { - 24 as i32 - } - }"#, - r#"fn foo() -> Result { - if true { - if false { - Ok(1 as i32) - } else { - Ok(2 as i32) - } - } else { - Ok(24 as i32) - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_block_like_match() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = 5; - match my_var { - 5 => 42i32, - _ => 24i32, - } - }"#, - r#"fn foo() -> Result { - let my_var = 5; - match my_var { - 5 => Ok(42i32), - _ => Ok(24i32), - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_loop_with_tail() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = 5; - loop { - println!("test"); - 5 - } - - my_var - }"#, - r#"fn foo() -> Result { - let my_var = 5; - loop { - println!("test"); - 5 - } - - Ok(my_var) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_loop_in_let_stmt() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = let x = loop { - break 1; - }; - - my_var - }"#, - r#"fn foo() -> Result { - let my_var = let x = loop { - break 1; - }; - - Ok(my_var) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_block_like_match_return_expr() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = 5; - let res = match my_var { - 5 => 42i32, - _ => return 24i32, - }; - - res - }"#, - r#"fn foo() -> Result { - let my_var = 5; - let res = match my_var { - 5 => 42i32, - _ => return Ok(24i32), - }; - - Ok(res) - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = 5; - let res = if my_var == 5 { - 42i32 - } else { - return 24i32; - }; - - res - }"#, - r#"fn foo() -> Result { - let my_var = 5; - let res = if my_var == 5 { - 42i32 - } else { - return Ok(24i32); - }; - - Ok(res) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_block_like_match_deeper() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let my_var = 5; - match my_var { - 5 => { - if true { - 42i32 - } else { - 25i32 - } - }, - _ => { - let test = "test"; - if test == "test" { - return bar(); - } - 53i32 - }, - } - }"#, - r#"fn foo() -> Result { - let my_var = 5; - match my_var { - 5 => { - if true { - Ok(42i32) - } else { - Ok(25i32) - } - }, - _ => { - let test = "test"; - if test == "test" { - return Ok(bar()); - } - Ok(53i32) - }, - } - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_tail_block_like_early_return() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i<|>32 { - let test = "test"; - if test == "test" { - return 24i32; - } - 53i32 - }"#, - r#"fn foo() -> Result { - let test = "test"; - if test == "test" { - return Ok(24i32); - } - Ok(53i32) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_closure() { - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -><|> u32 { - let true_closure = || { - return true; - }; - if the_field < 5 { - let mut i = 0; - - - if true_closure() { - return 99; - } else { - return 0; - } - } - - the_field - }"#, - r#"fn foo(the_field: u32) -> Result { - let true_closure = || { - return true; - }; - if the_field < 5 { - let mut i = 0; - - - if true_closure() { - return Ok(99); - } else { - return Ok(0); - } - } - - Ok(the_field) - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -> u32<|> { - let true_closure = || { - return true; - }; - if the_field < 5 { - let mut i = 0; - - - if true_closure() { - return 99; - } else { - return 0; - } - } - let t = None; - - t.unwrap_or_else(|| the_field) - }"#, - r#"fn foo(the_field: u32) -> Result { - let true_closure = || { - return true; - }; - if the_field < 5 { - let mut i = 0; - - - if true_closure() { - return Ok(99); - } else { - return Ok(0); - } - } - let t = None; - - Ok(t.unwrap_or_else(|| the_field)) - }"#, - ); - } - - #[test] - fn change_return_type_to_result_simple_with_weird_forms() { - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let test = "test"; - if test == "test" { - return 24i32; - } - let mut i = 0; - loop { - if i == 1 { - break 55; - } - i += 1; - } - }"#, - r#"fn foo() -> Result { - let test = "test"; - if test == "test" { - return Ok(24i32); - } - let mut i = 0; - loop { - if i == 1 { - break Ok(55); - } - i += 1; - } - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo() -> i32<|> { - let test = "test"; - if test == "test" { - return 24i32; - } - let mut i = 0; - loop { - loop { - if i == 1 { - break 55; - } - i += 1; - } - } - }"#, - r#"fn foo() -> Result { - let test = "test"; - if test == "test" { - return Ok(24i32); - } - let mut i = 0; - loop { - loop { - if i == 1 { - break Ok(55); - } - i += 1; - } - } - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo() -> i3<|>2 { - let test = "test"; - let other = 5; - if test == "test" { - let res = match other { - 5 => 43, - _ => return 56, - }; - } - let mut i = 0; - loop { - loop { - if i == 1 { - break 55; - } - i += 1; - } - } - }"#, - r#"fn foo() -> Result { - let test = "test"; - let other = 5; - if test == "test" { - let res = match other { - 5 => 43, - _ => return Ok(56), - }; - } - let mut i = 0; - loop { - loop { - if i == 1 { - break Ok(55); - } - i += 1; - } - } - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -> u32<|> { - if the_field < 5 { - let mut i = 0; - loop { - if i > 5 { - return 55u32; - } - i += 3; - } - - match i { - 5 => return 99, - _ => return 0, - }; - } - - the_field - }"#, - r#"fn foo(the_field: u32) -> Result { - if the_field < 5 { - let mut i = 0; - loop { - if i > 5 { - return Ok(55u32); - } - i += 3; - } - - match i { - 5 => return Ok(99), - _ => return Ok(0), - }; - } - - Ok(the_field) - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -> u3<|>2 { - if the_field < 5 { - let mut i = 0; - - match i { - 5 => return 99, - _ => return 0, - } - } - - the_field - }"#, - r#"fn foo(the_field: u32) -> Result { - if the_field < 5 { - let mut i = 0; - - match i { - 5 => return Ok(99), - _ => return Ok(0), - } - } - - Ok(the_field) - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -> u32<|> { - if the_field < 5 { - let mut i = 0; - - if i == 5 { - return 99 - } else { - return 0 - } - } - - the_field - }"#, - r#"fn foo(the_field: u32) -> Result { - if the_field < 5 { - let mut i = 0; - - if i == 5 { - return Ok(99) - } else { - return Ok(0) - } - } - - Ok(the_field) - }"#, - ); - - check_assist( - change_return_type_to_result, - r#"fn foo(the_field: u32) -> <|>u32 { - if the_field < 5 { - let mut i = 0; - - if i == 5 { - return 99; - } else { - return 0; - } - } - - the_field - }"#, - r#"fn foo(the_field: u32) -> Result { - if the_field < 5 { - let mut i = 0; - - if i == 5 { - return Ok(99); - } else { - return Ok(0); - } - } - - Ok(the_field) - }"#, - ); - } -} diff --git a/crates/assists/src/handlers/add_custom_impl.rs b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs similarity index 74% rename from crates/assists/src/handlers/add_custom_impl.rs rename to crates/assists/src/handlers/replace_derive_with_manual_impl.rs index c13493fd87..82625516c2 100644 --- a/crates/assists/src/handlers/add_custom_impl.rs +++ b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs @@ -16,24 +16,31 @@ use crate::{ AssistId, AssistKind, }; -// Assist: add_custom_impl +// Assist: replace_derive_with_manual_impl // -// Adds impl block for derived trait. +// Converts a `derive` impl into a manual one. // // ``` +// # trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } // #[derive(Deb<|>ug, Display)] // struct S; // ``` // -> // ``` +// # trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } // #[derive(Display)] // struct S; // // impl Debug for S { -// $0 +// fn fmt(&self, f: &mut Formatter) -> Result<()> { +// ${0:todo!()} +// } // } // ``` -pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { +pub(crate) fn replace_derive_with_manual_impl( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { let attr = ctx.find_node_at_offset::()?; let attr_name = attr @@ -90,43 +97,49 @@ fn add_assist( ) -> Option<()> { let target = attr.syntax().text_range(); let input = attr.token_tree()?; - let label = format!("Add custom impl `{}` for `{}`", trait_path, annotated_name); + let label = format!("Convert to manual `impl {} for {}`", trait_path, annotated_name); let trait_name = trait_path.segment().and_then(|seg| seg.name_ref())?; - acc.add(AssistId("add_custom_impl", AssistKind::Refactor), label, target, |builder| { - let impl_def_with_items = - impl_def_from_trait(&ctx.sema, annotated_name, trait_, trait_path); - update_attribute(builder, &input, &trait_name, &attr); - match (ctx.config.snippet_cap, impl_def_with_items) { - (None, _) => builder.insert( - insert_pos, - format!("\n\nimpl {} for {} {{\n\n}}", trait_path, annotated_name), - ), - (Some(cap), None) => builder.insert_snippet( - cap, - insert_pos, - format!("\n\nimpl {} for {} {{\n $0\n}}", trait_path, annotated_name), - ), - (Some(cap), Some((impl_def, first_assoc_item))) => { - let mut cursor = Cursor::Before(first_assoc_item.syntax()); - let placeholder; - if let ast::AssocItem::Fn(ref func) = first_assoc_item { - if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) { - if m.syntax().text() == "todo!()" { - placeholder = m; - cursor = Cursor::Replace(placeholder.syntax()); - } - } - } - - builder.insert_snippet( + acc.add( + AssistId("replace_derive_with_manual_impl", AssistKind::Refactor), + label, + target, + |builder| { + let impl_def_with_items = + impl_def_from_trait(&ctx.sema, annotated_name, trait_, trait_path); + update_attribute(builder, &input, &trait_name, &attr); + match (ctx.config.snippet_cap, impl_def_with_items) { + (None, _) => builder.insert( + insert_pos, + format!("\n\nimpl {} for {} {{\n\n}}", trait_path, annotated_name), + ), + (Some(cap), None) => builder.insert_snippet( cap, insert_pos, - format!("\n\n{}", render_snippet(cap, impl_def.syntax(), cursor)), - ) - } - }; - }) + format!("\n\nimpl {} for {} {{\n $0\n}}", trait_path, annotated_name), + ), + (Some(cap), Some((impl_def, first_assoc_item))) => { + let mut cursor = Cursor::Before(first_assoc_item.syntax()); + let placeholder; + if let ast::AssocItem::Fn(ref func) = first_assoc_item { + if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) + { + if m.syntax().text() == "todo!()" { + placeholder = m; + cursor = Cursor::Replace(placeholder.syntax()); + } + } + } + + builder.insert_snippet( + cap, + insert_pos, + format!("\n\n{}", render_snippet(cap, impl_def.syntax(), cursor)), + ) + } + }; + }, + ) } fn impl_def_from_trait( @@ -192,7 +205,7 @@ mod tests { #[test] fn add_custom_impl_debug() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " mod fmt { pub struct Error; @@ -233,7 +246,7 @@ impl fmt::Debug for Foo { #[test] fn add_custom_impl_all() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " mod foo { pub trait Bar { @@ -282,7 +295,7 @@ impl foo::Bar for Foo { #[test] fn add_custom_impl_for_unique_input() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debu<|>g)] struct Foo { @@ -304,7 +317,7 @@ impl Debug for Foo { #[test] fn add_custom_impl_for_with_visibility_modifier() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debug<|>)] pub struct Foo { @@ -326,7 +339,7 @@ impl Debug for Foo { #[test] fn add_custom_impl_when_multiple_inputs() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Display, Debug<|>, Serialize)] struct Foo {} @@ -345,7 +358,7 @@ impl Debug for Foo { #[test] fn test_ignore_derive_macro_without_input() { check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(<|>)] struct Foo {} @@ -356,7 +369,7 @@ struct Foo {} #[test] fn test_ignore_if_cursor_on_param() { check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive<|>(Debug)] struct Foo {} @@ -364,7 +377,7 @@ struct Foo {} ); check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debug)<|>] struct Foo {} @@ -375,7 +388,7 @@ struct Foo {} #[test] fn test_ignore_if_not_derive() { check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[allow(non_camel_<|>case_types)] struct Foo {} diff --git a/crates/assists/src/handlers/wrap_return_type_in_result.rs b/crates/assists/src/handlers/wrap_return_type_in_result.rs new file mode 100644 index 0000000000..59e5debb13 --- /dev/null +++ b/crates/assists/src/handlers/wrap_return_type_in_result.rs @@ -0,0 +1,1158 @@ +use std::iter; + +use syntax::{ + ast::{self, make, BlockExpr, Expr, LoopBodyOwner}, + match_ast, AstNode, SyntaxNode, +}; +use test_utils::mark; + +use crate::{AssistContext, AssistId, AssistKind, Assists}; + +// Assist: wrap_return_type_in_result +// +// Wrap the function's return type into Result. +// +// ``` +// fn foo() -> i32<|> { 42i32 } +// ``` +// -> +// ``` +// fn foo() -> Result { Ok(42i32) } +// ``` +pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let ret_type = ctx.find_node_at_offset::()?; + let parent = ret_type.syntax().parent()?; + let block_expr = match_ast! { + match parent { + ast::Fn(func) => func.body()?, + ast::ClosureExpr(closure) => match closure.body()? { + Expr::BlockExpr(block) => block, + // closures require a block when a return type is specified + _ => return None, + }, + _ => return None, + } + }; + + let type_ref = &ret_type.ty()?; + let ret_type_str = type_ref.syntax().text().to_string(); + let first_part_ret_type = ret_type_str.splitn(2, '<').next(); + if let Some(ret_type_first_part) = first_part_ret_type { + if ret_type_first_part.ends_with("Result") { + mark::hit!(wrap_return_type_in_result_simple_return_type_already_result); + return None; + } + } + + acc.add( + AssistId("wrap_return_type_in_result", AssistKind::RefactorRewrite), + "Wrap return type in Result", + type_ref.syntax().text_range(), + |builder| { + let mut tail_return_expr_collector = TailReturnCollector::new(); + tail_return_expr_collector.collect_jump_exprs(&block_expr, false); + tail_return_expr_collector.collect_tail_exprs(&block_expr); + + for ret_expr_arg in tail_return_expr_collector.exprs_to_wrap { + let ok_wrapped = make::expr_call( + make::expr_path(make::path_unqualified(make::path_segment(make::name_ref( + "Ok", + )))), + make::arg_list(iter::once(ret_expr_arg.clone())), + ); + builder.replace_ast(ret_expr_arg, ok_wrapped); + } + + match ctx.config.snippet_cap { + Some(cap) => { + let snippet = format!("Result<{}, ${{0:_}}>", type_ref); + builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet) + } + None => builder + .replace(type_ref.syntax().text_range(), format!("Result<{}, _>", type_ref)), + } + }, + ) +} + +struct TailReturnCollector { + exprs_to_wrap: Vec, +} + +impl TailReturnCollector { + fn new() -> Self { + Self { exprs_to_wrap: vec![] } + } + /// Collect all`return` expression + fn collect_jump_exprs(&mut self, block_expr: &BlockExpr, collect_break: bool) { + let statements = block_expr.statements(); + for stmt in statements { + let expr = match &stmt { + ast::Stmt::ExprStmt(stmt) => stmt.expr(), + ast::Stmt::LetStmt(stmt) => stmt.initializer(), + ast::Stmt::Item(_) => continue, + }; + if let Some(expr) = &expr { + self.handle_exprs(expr, collect_break); + } + } + + // Browse tail expressions for each block + if let Some(expr) = block_expr.expr() { + if let Some(last_exprs) = get_tail_expr_from_block(&expr) { + for last_expr in last_exprs { + let last_expr = match last_expr { + NodeType::Node(expr) => expr, + NodeType::Leaf(expr) => expr.syntax().clone(), + }; + + if let Some(last_expr) = Expr::cast(last_expr.clone()) { + self.handle_exprs(&last_expr, collect_break); + } else if let Some(expr_stmt) = ast::Stmt::cast(last_expr) { + let expr_stmt = match &expr_stmt { + ast::Stmt::ExprStmt(stmt) => stmt.expr(), + ast::Stmt::LetStmt(stmt) => stmt.initializer(), + ast::Stmt::Item(_) => None, + }; + if let Some(expr) = &expr_stmt { + self.handle_exprs(expr, collect_break); + } + } + } + } + } + } + + fn handle_exprs(&mut self, expr: &Expr, collect_break: bool) { + match expr { + Expr::BlockExpr(block_expr) => { + self.collect_jump_exprs(&block_expr, collect_break); + } + Expr::ReturnExpr(ret_expr) => { + if let Some(ret_expr_arg) = &ret_expr.expr() { + self.exprs_to_wrap.push(ret_expr_arg.clone()); + } + } + Expr::BreakExpr(break_expr) if collect_break => { + if let Some(break_expr_arg) = &break_expr.expr() { + self.exprs_to_wrap.push(break_expr_arg.clone()); + } + } + Expr::IfExpr(if_expr) => { + for block in if_expr.blocks() { + self.collect_jump_exprs(&block, collect_break); + } + } + Expr::LoopExpr(loop_expr) => { + if let Some(block_expr) = loop_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::ForExpr(for_expr) => { + if let Some(block_expr) = for_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::WhileExpr(while_expr) => { + if let Some(block_expr) = while_expr.loop_body() { + self.collect_jump_exprs(&block_expr, collect_break); + } + } + Expr::MatchExpr(match_expr) => { + if let Some(arm_list) = match_expr.match_arm_list() { + arm_list.arms().filter_map(|match_arm| match_arm.expr()).for_each(|expr| { + self.handle_exprs(&expr, collect_break); + }); + } + } + _ => {} + } + } + + fn collect_tail_exprs(&mut self, block: &BlockExpr) { + if let Some(expr) = block.expr() { + self.handle_exprs(&expr, true); + self.fetch_tail_exprs(&expr); + } + } + + fn fetch_tail_exprs(&mut self, expr: &Expr) { + if let Some(exprs) = get_tail_expr_from_block(expr) { + for node_type in &exprs { + match node_type { + NodeType::Leaf(expr) => { + self.exprs_to_wrap.push(expr.clone()); + } + NodeType::Node(expr) => { + if let Some(last_expr) = Expr::cast(expr.clone()) { + self.fetch_tail_exprs(&last_expr); + } + } + } + } + } + } +} + +#[derive(Debug)] +enum NodeType { + Leaf(ast::Expr), + Node(SyntaxNode), +} + +/// Get a tail expression inside a block +fn get_tail_expr_from_block(expr: &Expr) -> Option> { + match expr { + Expr::IfExpr(if_expr) => { + let mut nodes = vec![]; + for block in if_expr.blocks() { + if let Some(block_expr) = block.expr() { + if let Some(tail_exprs) = get_tail_expr_from_block(&block_expr) { + nodes.extend(tail_exprs); + } + } else if let Some(last_expr) = block.syntax().last_child() { + nodes.push(NodeType::Node(last_expr)); + } else { + nodes.push(NodeType::Node(block.syntax().clone())); + } + } + Some(nodes) + } + Expr::LoopExpr(loop_expr) => { + loop_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::ForExpr(for_expr) => { + for_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::WhileExpr(while_expr) => { + while_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)]) + } + Expr::BlockExpr(block_expr) => { + block_expr.expr().map(|lc| vec![NodeType::Node(lc.syntax().clone())]) + } + Expr::MatchExpr(match_expr) => { + let arm_list = match_expr.match_arm_list()?; + let arms: Vec = arm_list + .arms() + .filter_map(|match_arm| match_arm.expr()) + .map(|expr| match expr { + Expr::ReturnExpr(ret_expr) => NodeType::Node(ret_expr.syntax().clone()), + Expr::BreakExpr(break_expr) => NodeType::Node(break_expr.syntax().clone()), + _ => match expr.syntax().last_child() { + Some(last_expr) => NodeType::Node(last_expr), + None => NodeType::Node(expr.syntax().clone()), + }, + }) + .collect(); + + Some(arms) + } + Expr::BreakExpr(expr) => expr.expr().map(|e| vec![NodeType::Leaf(e)]), + Expr::ReturnExpr(ret_expr) => Some(vec![NodeType::Node(ret_expr.syntax().clone())]), + + Expr::CallExpr(_) + | Expr::Literal(_) + | Expr::TupleExpr(_) + | Expr::ArrayExpr(_) + | Expr::ParenExpr(_) + | Expr::PathExpr(_) + | Expr::RecordExpr(_) + | Expr::IndexExpr(_) + | Expr::MethodCallExpr(_) + | Expr::AwaitExpr(_) + | Expr::CastExpr(_) + | Expr::RefExpr(_) + | Expr::PrefixExpr(_) + | Expr::RangeExpr(_) + | Expr::BinExpr(_) + | Expr::MacroCall(_) + | Expr::BoxExpr(_) => Some(vec![NodeType::Leaf(expr.clone())]), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn wrap_return_type_in_result_simple() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i3<|>2 { + let test = "test"; + return 42i32; +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + return Ok(42i32); +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_closure() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() { + || -> i32<|> { + let test = "test"; + return 42i32; + }; +} +"#, + r#" +fn foo() { + || -> Result { + let test = "test"; + return Ok(42i32); + }; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_return_type_bad_cursor() { + check_assist_not_applicable( + wrap_return_type_in_result, + r#" +fn foo() -> i32 { + let test = "test";<|> + return 42i32; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_return_type_bad_cursor_closure() { + check_assist_not_applicable( + wrap_return_type_in_result, + r#" +fn foo() { + || -> i32 { + let test = "test";<|> + return 42i32; + }; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_closure_non_block() { + check_assist_not_applicable(wrap_return_type_in_result, r#"fn foo() { || -> i<|>32 3; }"#); + } + + #[test] + fn wrap_return_type_in_result_simple_return_type_already_result_std() { + check_assist_not_applicable( + wrap_return_type_in_result, + r#" +fn foo() -> std::result::Result, String> { + let test = "test"; + return 42i32; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_return_type_already_result() { + mark::check!(wrap_return_type_in_result_simple_return_type_already_result); + check_assist_not_applicable( + wrap_return_type_in_result, + r#" +fn foo() -> Result, String> { + let test = "test"; + return 42i32; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_return_type_already_result_closure() { + check_assist_not_applicable( + wrap_return_type_in_result, + r#" +fn foo() { + || -> Result, String> { + let test = "test"; + return 42i32; + }; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_cursor() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> <|>i32 { + let test = "test"; + return 42i32; +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + return Ok(42i32); +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -><|> i32 { + let test = "test"; + 42i32 +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + Ok(42i32) +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_closure() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() { + || -><|> i32 { + let test = "test"; + 42i32 + }; +} +"#, + r#" +fn foo() { + || -> Result { + let test = "test"; + Ok(42i32) + }; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_only() { + check_assist( + wrap_return_type_in_result, + r#"fn foo() -> i32<|> { 42i32 }"#, + r#"fn foo() -> Result { Ok(42i32) }"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_block_like() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + if true { + 42i32 + } else { + 24i32 + } +} +"#, + r#" +fn foo() -> Result { + if true { + Ok(42i32) + } else { + Ok(24i32) + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_without_block_closure() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() { + || -> i32<|> { + if true { + 42i32 + } else { + 24i32 + } + }; +} +"#, + r#" +fn foo() { + || -> Result { + if true { + Ok(42i32) + } else { + Ok(24i32) + } + }; +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_nested_if() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + if true { + if false { + 1 + } else { + 2 + } + } else { + 24i32 + } +} +"#, + r#" +fn foo() -> Result { + if true { + if false { + Ok(1) + } else { + Ok(2) + } + } else { + Ok(24i32) + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_await() { + check_assist( + wrap_return_type_in_result, + r#" +async fn foo() -> i<|>32 { + if true { + if false { + 1.await + } else { + 2.await + } + } else { + 24i32.await + } +} +"#, + r#" +async fn foo() -> Result { + if true { + if false { + Ok(1.await) + } else { + Ok(2.await) + } + } else { + Ok(24i32.await) + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_array() { + check_assist( + wrap_return_type_in_result, + r#"fn foo() -> [i32;<|> 3] { [1, 2, 3] }"#, + r#"fn foo() -> Result<[i32; 3], ${0:_}> { Ok([1, 2, 3]) }"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_cast() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -<|>> i32 { + if true { + if false { + 1 as i32 + } else { + 2 as i32 + } + } else { + 24 as i32 + } +} +"#, + r#" +fn foo() -> Result { + if true { + if false { + Ok(1 as i32) + } else { + Ok(2 as i32) + } + } else { + Ok(24 as i32) + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_block_like_match() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = 5; + match my_var { + 5 => 42i32, + _ => 24i32, + } +} +"#, + r#" +fn foo() -> Result { + let my_var = 5; + match my_var { + 5 => Ok(42i32), + _ => Ok(24i32), + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_loop_with_tail() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = 5; + loop { + println!("test"); + 5 + } + my_var +} +"#, + r#" +fn foo() -> Result { + let my_var = 5; + loop { + println!("test"); + 5 + } + Ok(my_var) +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_loop_in_let_stmt() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = let x = loop { + break 1; + }; + my_var +} +"#, + r#" +fn foo() -> Result { + let my_var = let x = loop { + break 1; + }; + Ok(my_var) +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_block_like_match_return_expr() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = 5; + let res = match my_var { + 5 => 42i32, + _ => return 24i32, + }; + res +} +"#, + r#" +fn foo() -> Result { + let my_var = 5; + let res = match my_var { + 5 => 42i32, + _ => return Ok(24i32), + }; + Ok(res) +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = 5; + let res = if my_var == 5 { + 42i32 + } else { + return 24i32; + }; + res +} +"#, + r#" +fn foo() -> Result { + let my_var = 5; + let res = if my_var == 5 { + 42i32 + } else { + return Ok(24i32); + }; + Ok(res) +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_block_like_match_deeper() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let my_var = 5; + match my_var { + 5 => { + if true { + 42i32 + } else { + 25i32 + } + }, + _ => { + let test = "test"; + if test == "test" { + return bar(); + } + 53i32 + }, + } +} +"#, + r#" +fn foo() -> Result { + let my_var = 5; + match my_var { + 5 => { + if true { + Ok(42i32) + } else { + Ok(25i32) + } + }, + _ => { + let test = "test"; + if test == "test" { + return Ok(bar()); + } + Ok(53i32) + }, + } +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_tail_block_like_early_return() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i<|>32 { + let test = "test"; + if test == "test" { + return 24i32; + } + 53i32 +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + Ok(53i32) +} +"#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_closure() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo(the_field: u32) -><|> u32 { + let true_closure = || { return true; }; + if the_field < 5 { + let mut i = 0; + if true_closure() { + return 99; + } else { + return 0; + } + } + the_field +} +"#, + r#" +fn foo(the_field: u32) -> Result { + let true_closure = || { return true; }; + if the_field < 5 { + let mut i = 0; + if true_closure() { + return Ok(99); + } else { + return Ok(0); + } + } + Ok(the_field) +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" + fn foo(the_field: u32) -> u32<|> { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return 99; + } else { + return 0; + } + } + let t = None; + + t.unwrap_or_else(|| the_field) + } + "#, + r#" + fn foo(the_field: u32) -> Result { + let true_closure = || { + return true; + }; + if the_field < 5 { + let mut i = 0; + + + if true_closure() { + return Ok(99); + } else { + return Ok(0); + } + } + let t = None; + + Ok(t.unwrap_or_else(|| the_field)) + } + "#, + ); + } + + #[test] + fn wrap_return_type_in_result_simple_with_weird_forms() { + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let test = "test"; + if test == "test" { + return 24i32; + } + let mut i = 0; + loop { + if i == 1 { + break 55; + } + i += 1; + } +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + let mut i = 0; + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i32<|> { + let test = "test"; + if test == "test" { + return 24i32; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break 55; + } + i += 1; + } + } +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + if test == "test" { + return Ok(24i32); + } + let mut i = 0; + loop { + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } + } +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo() -> i3<|>2 { + let test = "test"; + let other = 5; + if test == "test" { + let res = match other { + 5 => 43, + _ => return 56, + }; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break 55; + } + i += 1; + } + } +} +"#, + r#" +fn foo() -> Result { + let test = "test"; + let other = 5; + if test == "test" { + let res = match other { + 5 => 43, + _ => return Ok(56), + }; + } + let mut i = 0; + loop { + loop { + if i == 1 { + break Ok(55); + } + i += 1; + } + } +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo(the_field: u32) -> u32<|> { + if the_field < 5 { + let mut i = 0; + loop { + if i > 5 { + return 55u32; + } + i += 3; + } + match i { + 5 => return 99, + _ => return 0, + }; + } + the_field +} +"#, + r#" +fn foo(the_field: u32) -> Result { + if the_field < 5 { + let mut i = 0; + loop { + if i > 5 { + return Ok(55u32); + } + i += 3; + } + match i { + 5 => return Ok(99), + _ => return Ok(0), + }; + } + Ok(the_field) +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo(the_field: u32) -> u3<|>2 { + if the_field < 5 { + let mut i = 0; + match i { + 5 => return 99, + _ => return 0, + } + } + the_field +} +"#, + r#" +fn foo(the_field: u32) -> Result { + if the_field < 5 { + let mut i = 0; + match i { + 5 => return Ok(99), + _ => return Ok(0), + } + } + Ok(the_field) +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo(the_field: u32) -> u32<|> { + if the_field < 5 { + let mut i = 0; + if i == 5 { + return 99 + } else { + return 0 + } + } + the_field +} +"#, + r#" +fn foo(the_field: u32) -> Result { + if the_field < 5 { + let mut i = 0; + if i == 5 { + return Ok(99) + } else { + return Ok(0) + } + } + Ok(the_field) +} +"#, + ); + + check_assist( + wrap_return_type_in_result, + r#" +fn foo(the_field: u32) -> <|>u32 { + if the_field < 5 { + let mut i = 0; + if i == 5 { + return 99; + } else { + return 0; + } + } + the_field +} +"#, + r#" +fn foo(the_field: u32) -> Result { + if the_field < 5 { + let mut i = 0; + if i == 5 { + return Ok(99); + } else { + return Ok(0); + } + } + Ok(the_field) +} +"#, + ); + } +} diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index af88b34374..e8d81b33d7 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -120,13 +120,11 @@ mod handlers { pub(crate) type Handler = fn(&mut Assists, &AssistContext) -> Option<()>; - mod add_custom_impl; mod add_explicit_type; mod add_missing_impl_members; mod add_turbo_fish; mod apply_demorgan; mod auto_import; - mod change_return_type_to_result; mod change_visibility; mod convert_integer_literal; mod early_return; @@ -157,6 +155,7 @@ mod handlers { mod remove_mut; mod remove_unused_param; mod reorder_fields; + mod replace_derive_with_manual_impl; mod replace_if_let_with_match; mod replace_impl_trait_with_generic; mod replace_let_with_if_let; @@ -165,16 +164,15 @@ mod handlers { mod replace_unwrap_with_match; mod split_import; mod unwrap_block; + mod wrap_return_type_in_result; pub(crate) fn all() -> &'static [Handler] { &[ // These are alphabetic for the foolish consistency - add_custom_impl::add_custom_impl, add_explicit_type::add_explicit_type, add_turbo_fish::add_turbo_fish, apply_demorgan::apply_demorgan, auto_import::auto_import, - change_return_type_to_result::change_return_type_to_result, change_visibility::change_visibility, convert_integer_literal::convert_integer_literal, early_return::convert_to_guarded_return, @@ -208,6 +206,7 @@ mod handlers { remove_mut::remove_mut, remove_unused_param::remove_unused_param, reorder_fields::reorder_fields, + replace_derive_with_manual_impl::replace_derive_with_manual_impl, replace_if_let_with_match::replace_if_let_with_match, replace_impl_trait_with_generic::replace_impl_trait_with_generic, replace_let_with_if_let::replace_let_with_if_let, @@ -215,6 +214,7 @@ mod handlers { replace_unwrap_with_match::replace_unwrap_with_match, split_import::split_import, unwrap_block::unwrap_block, + wrap_return_type_in_result::wrap_return_type_in_result, // These are manually sorted for better priorities add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 168e1626ab..dbf4f21aa5 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -2,25 +2,6 @@ use super::check_doc_test; -#[test] -fn doctest_add_custom_impl() { - check_doc_test( - "add_custom_impl", - r#####" -#[derive(Deb<|>ug, Display)] -struct S; -"#####, - r#####" -#[derive(Display)] -struct S; - -impl Debug for S { - $0 -} -"#####, - ) -} - #[test] fn doctest_add_explicit_type() { check_doc_test( @@ -177,19 +158,6 @@ pub mod std { pub mod collections { pub struct HashMap { } } } ) } -#[test] -fn doctest_change_return_type_to_result() { - check_doc_test( - "change_return_type_to_result", - r#####" -fn foo() -> i32<|> { 42i32 } -"#####, - r#####" -fn foo() -> Result { Ok(42i32) } -"#####, - ) -} - #[test] fn doctest_change_visibility() { check_doc_test( @@ -831,6 +799,29 @@ const test: Foo = Foo {foo: 1, bar: 0} ) } +#[test] +fn doctest_replace_derive_with_manual_impl() { + check_doc_test( + "replace_derive_with_manual_impl", + r#####" +trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +#[derive(Deb<|>ug, Display)] +struct S; +"#####, + r#####" +trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +#[derive(Display)] +struct S; + +impl Debug for S { + fn fmt(&self, f: &mut Formatter) -> Result<()> { + ${0:todo!()} + } +} +"#####, + ) +} + #[test] fn doctest_replace_if_let_with_match() { check_doc_test( @@ -985,3 +976,16 @@ fn foo() { "#####, ) } + +#[test] +fn doctest_wrap_return_type_in_result() { + check_doc_test( + "wrap_return_type_in_result", + r#####" +fn foo() -> i32<|> { 42i32 } +"#####, + r#####" +fn foo() -> Result { Ok(42i32) } +"#####, + ) +} diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index 4c7db8405e..99652e76b6 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -214,9 +214,6 @@ fn check_todo(path: &Path, text: &str) { // This file itself obviously needs to use todo (<- like this!). "tests/cli.rs", // Some of our assists generate `todo!()`. - "tests/generated.rs", - "handlers/add_custom_impl.rs", - "handlers/add_missing_impl_members.rs", "handlers/add_turbo_fish.rs", "handlers/generate_function.rs", // To support generating `todo!()` in assists, we have `expr_todo()` in @@ -229,6 +226,11 @@ fn check_todo(path: &Path, text: &str) { return; } if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") { + // Generated by an assist + if text.contains("${0:todo!()}") { + return; + } + panic!( "\nTODO markers or todo! macros should not be committed to the master branch,\n\ use FIXME instead\n\