From 44c76d6550081552c3c5106b0535a7e5bf265aec Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 5 Dec 2020 15:41:36 +0100 Subject: Add replace_match_with_if_let assist --- crates/assists/src/handlers/early_return.rs | 2 +- crates/assists/src/handlers/move_guard.rs | 1 + .../src/handlers/replace_if_let_with_match.rs | 277 ++++++++++++++++++++- .../src/handlers/replace_let_with_if_let.rs | 2 +- crates/assists/src/lib.rs | 1 + crates/assists/src/tests/generated.rs | 28 +++ 6 files changed, 308 insertions(+), 3 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/early_return.rs b/crates/assists/src/handlers/early_return.rs index 7fd78e9d4..7bcc318a9 100644 --- a/crates/assists/src/handlers/early_return.rs +++ b/crates/assists/src/handlers/early_return.rs @@ -112,7 +112,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) let then_branch = make::block_expr(once(make::expr_stmt(early_expression).into()), None); let cond = invert_boolean_expression(cond_expr); - make::expr_if(make::condition(cond, None), then_branch) + make::expr_if(make::condition(cond, None), then_branch, None) .indent(if_indent_level) }; replace(new_expr.syntax(), &then_block, &parent_block, &if_expr) diff --git a/crates/assists/src/handlers/move_guard.rs b/crates/assists/src/handlers/move_guard.rs index e1855b63d..eaffd80ce 100644 --- a/crates/assists/src/handlers/move_guard.rs +++ b/crates/assists/src/handlers/move_guard.rs @@ -42,6 +42,7 @@ pub(crate) fn move_guard_to_arm_body(acc: &mut Assists, ctx: &AssistContext) -> let if_expr = make::expr_if( make::condition(guard_condition, None), make::block_expr(None, Some(arm_expr.clone())), + None, ) .indent(arm_expr.indent_level()); diff --git a/crates/assists/src/handlers/replace_if_let_with_match.rs b/crates/assists/src/handlers/replace_if_let_with_match.rs index 9a49c48c1..4a355c66f 100644 --- a/crates/assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/assists/src/handlers/replace_if_let_with_match.rs @@ -1,3 +1,6 @@ +use std::iter; + +use ide_db::{ty_filter::TryEnum, RootDatabase}; use syntax::{ ast::{ self, @@ -8,7 +11,6 @@ use syntax::{ }; use crate::{utils::unwrap_trivial_block, AssistContext, AssistId, AssistKind, Assists}; -use ide_db::ty_filter::TryEnum; // Assist: replace_if_let_with_match // @@ -79,6 +81,91 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) ) } +// Assist: replace_match_with_if_let +// +// Replaces a binary `match` with a wildcard pattern and no guards with an `if let` expression. +// +// ``` +// enum Action { Move { distance: u32 }, Stop } +// +// fn handle(action: Action) { +// <|>match action { +// Action::Move { distance } => foo(distance), +// _ => bar(), +// } +// } +// ``` +// -> +// ``` +// enum Action { Move { distance: u32 }, Stop } +// +// fn handle(action: Action) { +// if let Action::Move { distance } = action { +// foo(distance) +// } else { +// bar() +// } +// } +// ``` +pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let match_expr: ast::MatchExpr = ctx.find_node_at_offset()?; + let mut arms = match_expr.match_arm_list()?.arms(); + let first_arm = arms.next()?; + let second_arm = arms.next()?; + if arms.next().is_some() || first_arm.guard().is_some() || second_arm.guard().is_some() { + return None; + } + let condition_expr = match_expr.expr()?; + let (if_let_pat, then_expr, else_expr) = if is_pat_wildcard_or_sad(&ctx.sema, &first_arm.pat()?) + { + (second_arm.pat()?, second_arm.expr()?, first_arm.expr()?) + } else if is_pat_wildcard_or_sad(&ctx.sema, &second_arm.pat()?) { + (first_arm.pat()?, first_arm.expr()?, second_arm.expr()?) + } else { + return None; + }; + + let target = match_expr.syntax().text_range(); + acc.add( + AssistId("replace_match_with_if_let", AssistKind::RefactorRewrite), + "Replace with if let", + target, + move |edit| { + let condition = make::condition(condition_expr, Some(if_let_pat)); + let then_block = match then_expr.reset_indent() { + ast::Expr::BlockExpr(block) => block, + expr => make::block_expr(iter::empty(), Some(expr)), + }; + let else_expr = match else_expr { + ast::Expr::BlockExpr(block) + if block.statements().count() == 0 && block.expr().is_none() => + { + None + } + ast::Expr::TupleExpr(tuple) if tuple.fields().count() == 0 => None, + expr => Some(expr), + }; + let if_let_expr = make::expr_if( + condition, + then_block, + else_expr.map(|else_expr| { + ast::ElseBranch::Block(make::block_expr(iter::empty(), Some(else_expr))) + }), + ) + .indent(IndentLevel::from_node(match_expr.syntax())); + + edit.replace_ast::(match_expr.into(), if_let_expr); + }, + ) +} + +fn is_pat_wildcard_or_sad(sema: &hir::Semantics, pat: &ast::Pat) -> bool { + sema.type_of_pat(&pat) + .and_then(|ty| TryEnum::from_ty(sema, &ty)) + .map(|it| it.sad_pattern().syntax().text() == pat.syntax().text()) + .unwrap_or_else(|| matches!(pat, ast::Pat::WildcardPat(_))) +} + #[cfg(test)] mod tests { use super::*; @@ -249,6 +336,194 @@ fn main() { } } } +"#, + ) + } + + #[test] + fn test_replace_match_with_if_let_unwraps_simple_expressions() { + check_assist( + replace_match_with_if_let, + r#" +impl VariantData { + pub fn is_struct(&self) -> bool { + <|>match *self { + VariantData::Struct(..) => true, + _ => false, + } + } +} "#, + r#" +impl VariantData { + pub fn is_struct(&self) -> bool { + if let VariantData::Struct(..) = *self { + true + } else { + false + } + } +} "#, + ) + } + + #[test] + fn test_replace_match_with_if_let_doesnt_unwrap_multiline_expressions() { + check_assist( + replace_match_with_if_let, + r#" +fn foo() { + <|>match a { + VariantData::Struct(..) => { + bar( + 123 + ) + } + _ => false, + } +} "#, + r#" +fn foo() { + if let VariantData::Struct(..) = a { + bar( + 123 + ) + } else { + false + } +} "#, + ) + } + + #[test] + fn replace_match_with_if_let_target() { + check_assist_target( + replace_match_with_if_let, + r#" +impl VariantData { + pub fn is_struct(&self) -> bool { + <|>match *self { + VariantData::Struct(..) => true, + _ => false, + } + } +} "#, + r#"match *self { + VariantData::Struct(..) => true, + _ => false, + }"#, + ); + } + + #[test] + fn special_case_option_match_to_if_let() { + check_assist( + replace_match_with_if_let, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + <|>match x { + Some(x) => println!("{}", x), + None => println!("none"), + } +} + "#, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + if let Some(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + ); + } + + #[test] + fn special_case_result_match_to_if_let() { + check_assist( + replace_match_with_if_let, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + <|>match x { + Ok(x) => println!("{}", x), + Err(_) => println!("none"), + } +} + "#, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + if let Ok(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + ); + } + + #[test] + fn nested_indent_match_to_if_let() { + check_assist( + replace_match_with_if_let, + r#" +fn main() { + if true { + <|>match path.strip_prefix(root_path) { + Ok(rel_path) => { + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) + } + _ => None, + } + } +} +"#, + r#" +fn main() { + if true { + if let Ok(rel_path) = path.strip_prefix(root_path) { + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) + } else { + None + } + } +} +"#, + ) + } + + #[test] + fn replace_match_with_if_let_empty_wildcard_expr() { + check_assist( + replace_match_with_if_let, + r#" +fn main() { + <|>match path.strip_prefix(root_path) { + Ok(rel_path) => println!("{}", rel_path), + _ => (), + } +} +"#, + r#" +fn main() { + if let Ok(rel_path) = path.strip_prefix(root_path) { + println!("{}", rel_path) + } +} "#, ) } diff --git a/crates/assists/src/handlers/replace_let_with_if_let.rs b/crates/assists/src/handlers/replace_let_with_if_let.rs index 69d3b08d3..5970e283c 100644 --- a/crates/assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/assists/src/handlers/replace_let_with_if_let.rs @@ -60,7 +60,7 @@ pub(crate) fn replace_let_with_if_let(acc: &mut Assists, ctx: &AssistContext) -> }; let block = make::block_expr(None, None).indent(IndentLevel::from_node(let_stmt.syntax())); - let if_ = make::expr_if(make::condition(init, Some(with_placeholder)), block); + let if_ = make::expr_if(make::condition(init, Some(with_placeholder)), block, None); let stmt = make::expr_stmt(if_); let placeholder = stmt.syntax().descendants().find_map(ast::WildcardPat::cast).unwrap(); diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index dfe6c2729..b8ce7418d 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -209,6 +209,7 @@ mod handlers { 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_if_let_with_match::replace_match_with_if_let, replace_impl_trait_with_generic::replace_impl_trait_with_generic, replace_let_with_if_let::replace_let_with_if_let, replace_qualified_name_with_use::replace_qualified_name_with_use, diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 8d50c8791..853bde09c 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -889,6 +889,34 @@ fn compute() -> Option { None } ) } +#[test] +fn doctest_replace_match_with_if_let() { + check_doc_test( + "replace_match_with_if_let", + r#####" +enum Action { Move { distance: u32 }, Stop } + +fn handle(action: Action) { + <|>match action { + Action::Move { distance } => foo(distance), + _ => bar(), + } +} +"#####, + r#####" +enum Action { Move { distance: u32 }, Stop } + +fn handle(action: Action) { + if let Action::Move { distance } = action { + foo(distance) + } else { + bar() + } +} +"#####, + ) +} + #[test] fn doctest_replace_qualified_name_with_use() { check_doc_test( -- cgit v1.2.3