From 1f20804b6e54d90226e1d82253676648ca8be8da Mon Sep 17 00:00:00 2001
From: Jonas Schievink <jonas.schievink@ferrous-systems.com>
Date: Mon, 30 Jan 2023 17:50:02 +0100
Subject: [PATCH] Improve "match to let-else" assist

---
 .../src/handlers/convert_match_to_let_else.rs | 75 +++++++++++++++----
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/crates/ide-assists/src/handlers/convert_match_to_let_else.rs b/crates/ide-assists/src/handlers/convert_match_to_let_else.rs
index fbd81c8015..7896d3a66d 100644
--- a/crates/ide-assists/src/handlers/convert_match_to_let_else.rs
+++ b/crates/ide-assists/src/handlers/convert_match_to_let_else.rs
@@ -30,7 +30,7 @@ use crate::{
 // ```
 pub(crate) fn convert_match_to_let_else(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let let_stmt: ast::LetStmt = ctx.find_node_at_offset()?;
-    let binding = find_binding(let_stmt.pat()?)?;
+    let binding = let_stmt.pat()?;
 
     let initializer = match let_stmt.initializer() {
         Some(ast::Expr::MatchExpr(it)) => it,
@@ -47,7 +47,12 @@ pub(crate) fn convert_match_to_let_else(acc: &mut Assists, ctx: &AssistContext<'
         return None;
     }
 
-    let diverging_arm_expr = diverging_arm.expr()?;
+    let diverging_arm_expr = match diverging_arm.expr()? {
+        ast::Expr::BlockExpr(block) if block.modifier().is_none() && block.label().is_none() => {
+            block.to_string()
+        }
+        other => format!("{{ {other} }}"),
+    };
     let extracting_arm_pat = extracting_arm.pat()?;
     let extracted_variable = find_extracted_variable(ctx, &extracting_arm)?;
 
@@ -56,24 +61,16 @@ pub(crate) fn convert_match_to_let_else(acc: &mut Assists, ctx: &AssistContext<'
         "Convert match to let-else",
         let_stmt.syntax().text_range(),
         |builder| {
-            let extracting_arm_pat = rename_variable(&extracting_arm_pat, extracted_variable, binding);
+            let extracting_arm_pat =
+                rename_variable(&extracting_arm_pat, extracted_variable, binding);
             builder.replace(
                 let_stmt.syntax().text_range(),
-                format!("let {extracting_arm_pat} = {initializer_expr} else {{ {diverging_arm_expr} }};")
+                format!("let {extracting_arm_pat} = {initializer_expr} else {diverging_arm_expr};"),
             )
         },
     )
 }
 
-// Given a pattern, find the name introduced to the surrounding scope.
-fn find_binding(pat: ast::Pat) -> Option<ast::IdentPat> {
-    if let ast::Pat::IdentPat(ident) = pat {
-        Some(ident)
-    } else {
-        None
-    }
-}
-
 // Given a match expression, find extracting and diverging arms.
 fn find_arms(
     ctx: &AssistContext<'_>,
@@ -124,7 +121,7 @@ fn find_extracted_variable(ctx: &AssistContext<'_>, arm: &ast::MatchArm) -> Opti
 }
 
 // Rename `extracted` with `binding` in `pat`.
-fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::IdentPat) -> SyntaxNode {
+fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::Pat) -> SyntaxNode {
     let syntax = pat.syntax().clone_for_update();
     let extracted_syntax = syntax.covering_element(extracted.syntax().text_range());
 
@@ -136,7 +133,7 @@ fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::IdentPat)
         if let Some(name_ref) = record_pat_field.field_name() {
             ted::replace(
                 record_pat_field.syntax(),
-                ast::make::record_pat_field(ast::make::name_ref(&name_ref.text()), binding.into())
+                ast::make::record_pat_field(ast::make::name_ref(&name_ref.text()), binding)
                     .syntax()
                     .clone_for_update(),
             );
@@ -410,4 +407,52 @@ fn foo(opt: Option<i32>) -> Option<i32> {
     "#,
         );
     }
+
+    #[test]
+    fn complex_pattern() {
+        check_assist(
+            convert_match_to_let_else,
+            r#"
+//- minicore: option
+fn f() {
+    let (x, y) = $0match Some((0, 1)) {
+        Some(it) => it,
+        None => return,
+    };
+}
+"#,
+            r#"
+fn f() {
+    let Some((x, y)) = Some((0, 1)) else { return };
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn diverging_block() {
+        check_assist(
+            convert_match_to_let_else,
+            r#"
+//- minicore: option
+fn f() {
+    let x = $0match Some(()) {
+        Some(it) => it,
+        None => {//comment
+            println!("nope");
+            return
+        },
+    };
+}
+"#,
+            r#"
+fn f() {
+    let Some(x) = Some(()) else {//comment
+            println!("nope");
+            return
+        };
+}
+"#,
+        );
+    }
 }