Add replace_match_with_if_let assist

Veykril · Veykril · commit 44c76d655008 · 2020-12-05T15:41:36.000+01:00
diff --git 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
@@ -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
@@ -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::<ast::Expr>(match_expr.into(), if_let_expr);
+        },
+    )
+}
+
+fn is_pat_wildcard_or_sad(sema: &hir::Semantics<RootDatabase>, 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<T> { Some(T), None }
+use Option::*;
+
+fn foo(x: Option<i32>) {
+    <|>match x {
+        Some(x) => println!("{}", x),
+        None => println!("none"),
+    }
+}
+           "#,
+            r#"
+enum Option<T> { Some(T), None }
+use Option::*;
+
+fn foo(x: Option<i32>) {
+    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<T, E> { Ok(T), Err(E) }
+use Result::*;
+
+fn foo(x: Result<i32, ()>) {
+    <|>match x {
+        Ok(x) => println!("{}", x),
+        Err(_) => println!("none"),
+    }
+}
+           "#,
+            r#"
+enum Result<T, E> { Ok(T), Err(E) }
+use Result::*;
+
+fn foo(x: Result<i32, ()>) {
+    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
@@ -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
@@ -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
@@ -889,6 +889,34 @@ fn compute() -> Option<i32> { 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(
diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs
@@ -171,8 +171,17 @@ pub fn expr_return() -> ast::Expr {
 pub fn expr_match(expr: ast::Expr, match_arm_list: ast::MatchArmList) -> ast::Expr {
     expr_from_text(&format!("match {} {}", expr, match_arm_list))
 }
-pub fn expr_if(condition: ast::Condition, then_branch: ast::BlockExpr) -> ast::Expr {
-    expr_from_text(&format!("if {} {}", condition, then_branch))
+pub fn expr_if(
+    condition: ast::Condition,
+    then_branch: ast::BlockExpr,
+    else_branch: Option<ast::ElseBranch>,
+) -> ast::Expr {
+    let else_branch = match else_branch {
+        Some(ast::ElseBranch::Block(block)) => format!("else {}", block),
+        Some(ast::ElseBranch::IfExpr(if_expr)) => format!("else {}", if_expr),
+        None => String::new(),
+    };
+    expr_from_text(&format!("if {} {} {}", condition, then_branch, else_branch))
 }
 pub fn expr_prefix(op: SyntaxKind, expr: ast::Expr) -> ast::Expr {
     let token = token(op);

Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ pub(crate) fn move_guard_to_arm_body(acc: &mut Assists, ctx: &AssistContext) ->`
`42`	`42`	`let if_expr = make::expr_if(`
`43`	`43`	`make::condition(guard_condition, None),`
`44`	`44`	`make::block_expr(None, Some(arm_expr.clone())),`
	`45`	`+ None,`
`45`	`46`	`)`
`46`	`47`	`.indent(arm_expr.indent_level());`
`47`	`48`