Merge #11481

[rust.git] / crates / ide_assists / src / handlers / replace_if_let_with_match.rs

diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
index 6f383ae8bba94e7450cf20874cce62df09f256e9..b594c64c412dac5eb2a794768645257e6414887f 100644 (file)
--- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
@@ -1,7 +1,12 @@
 use std::iter::{self, successors};
 
 use either::Either;
-use ide_db::{defs::NameClass, ty_filter::TryEnum, RootDatabase};
+use ide_db::{
+    defs::NameClass,
+    helpers::node_ext::{is_pattern_cond, single_let},
+    ty_filter::TryEnum,
+    RootDatabase,
+};
 use syntax::{
     ast::{
         self,
@@ -12,7 +17,7 @@
 };
 
 use crate::{
-    utils::{does_pat_match_variant, unwrap_trivial_block},
+    utils::{does_nested_pattern, does_pat_match_variant, unwrap_trivial_block},
     AssistContext, AssistId, AssistKind, Assists,
 };
 
@@ -48,7 +53,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
         if_expr.syntax().text_range().start(),
         if_expr.then_branch()?.syntax().text_range().start(),
     );
-    let cursor_in_range = available_range.contains_range(ctx.frange.range);
+    let cursor_in_range = available_range.contains_range(ctx.selection_trimmed());
     if !cursor_in_range {
         return None;
     }
@@ -60,15 +65,22 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
             None
         }
     });
-    let scrutinee_to_be_expr = if_expr.condition()?.expr()?;
+    let scrutinee_to_be_expr = if_expr.condition()?;
+    let scrutinee_to_be_expr = match single_let(scrutinee_to_be_expr.clone()) {
+        Some(cond) => cond.expr()?,
+        None => scrutinee_to_be_expr,
+    };
 
     let mut pat_seen = false;
     let mut cond_bodies = Vec::new();
     for if_expr in if_exprs {
         let cond = if_expr.condition()?;
-        let expr = cond.expr()?;
-        let cond = match cond.pat() {
-            Some(pat) => {
+        let cond = match single_let(cond.clone()) {
+            Some(let_) => {
+                let pat = let_.pat()?;
+                let expr = let_.expr()?;
+                // FIXME: If one `let` is wrapped in parentheses and the second is not,
+                // we'll exit here.
                 if scrutinee_to_be_expr.syntax().text() != expr.syntax().text() {
                     // Only if all condition expressions are equal we can merge them into a match
                     return None;
@@ -76,7 +88,9 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
                 pat_seen = true;
                 Either::Left(pat)
             }
-            None => Either::Right(expr),
+            // Multiple `let`, unsupported.
+            None if is_pattern_cond(cond.clone()) => return None,
+            None => Either::Right(cond),
         };
         let body = if_expr.then_branch()?;
         cond_bodies.push((cond, body));
@@ -143,6 +157,8 @@ fn make_else_arm(
             Some((it, pat)) => {
                 if does_pat_match_variant(pat, &it.sad_pattern()) {
                     it.happy_pattern_wildcard()
+                } else if does_nested_pattern(pat) {
+                    make::wildcard_pat().into()
                 } else {
                     it.sad_pattern()
                 }
@@ -205,21 +221,23 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext)
         "Replace match with if let",
         target,
         move |edit| {
-            let condition = make::condition(scrutinee, 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)),
-            };
+            fn make_block_expr(expr: ast::Expr) -> ast::BlockExpr {
+                // Blocks with modifiers (unsafe, async, etc.) are parsed as BlockExpr, but are
+                // formatted without enclosing braces. If we encounter such block exprs,
+                // wrap them in another BlockExpr.
+                match expr {
+                    ast::Expr::BlockExpr(block) if block.modifier().is_none() => block,
+                    expr => make::block_expr(iter::empty(), Some(expr)),
+                }
+            }
+
+            let condition = make::expr_let(if_let_pat, scrutinee);
+            let then_block = make_block_expr(then_expr.reset_indent());
             let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) };
             let if_let_expr = make::expr_if(
-                condition,
+                condition.into(),
                 then_block,
-                else_expr
-                    .map(|expr| match expr {
-                        ast::Expr::BlockExpr(block) => block,
-                        expr => (make::block_expr(iter::empty(), Some(expr))),
-                    })
-                    .map(ast::ElseBranch::Block),
+                else_expr.map(make_block_expr).map(ast::ElseBranch::Block),
             )
             .indent(IndentLevel::from_node(match_expr.syntax()));
 
@@ -368,6 +386,18 @@ pub fn foo(&self) {
         )
     }
 
+    #[test]
+    fn test_if_let_with_match_let_chain() {
+        check_assist_not_applicable(
+            replace_if_let_with_match,
+            r#"
+fn main() {
+    if $0let true = true && let Some(1) = None {}
+}
+"#,
+        )
+    }
+
     #[test]
     fn test_if_let_with_match_basic() {
         check_assist(
@@ -574,6 +604,33 @@ fn main() {
         )
     }
 
+    #[test]
+    fn nested_type() {
+        check_assist(
+            replace_if_let_with_match,
+            r#"
+//- minicore: result
+fn foo(x: Result<i32, ()>) {
+    let bar: Result<_, ()> = Ok(Some(1));
+    $0if let Ok(Some(_)) = bar {
+        ()
+    } else {
+        ()
+    }
+}
+"#,
+            r#"
+fn foo(x: Result<i32, ()>) {
+    let bar: Result<_, ()> = Ok(Some(1));
+    match bar {
+        Ok(Some(_)) => (),
+        _ => (),
+    }
+}
+"#,
+        );
+    }
+
     #[test]
     fn test_replace_match_with_if_let_unwraps_simple_expressions() {
         check_assist(
@@ -890,28 +947,28 @@ fn foo() {
     }
 
     #[test]
-    fn nested_type() {
+    fn test_replace_match_with_if_let_keeps_unsafe_block() {
         check_assist(
-            replace_if_let_with_match,
+            replace_match_with_if_let,
             r#"
-//- minicore: result
-fn foo(x: Result<i32, ()>) {
-    let bar: Result<_, ()> = Ok(Some(1));
-    $0if let Ok(Some(_)) = bar {
-        ()
-    } else {
-        ()
+impl VariantData {
+    pub fn is_struct(&self) -> bool {
+        $0match *self {
+            VariantData::Struct(..) => true,
+            _ => unsafe { unreachable_unchecked() },
+        }
     }
-}
-"#,
+}           "#,
             r#"
-fn foo(x: Result<i32, ()>) {
-    let bar: Result<_, ()> = Ok(Some(1));
-    match bar {
-        Ok(Some(_)) => (),
-        _ => (),
+impl VariantData {
+    pub fn is_struct(&self) -> bool {
+        if let VariantData::Struct(..) = *self {
+            true
+        } else {
+            unsafe { unreachable_unchecked() }
+        }
     }
-"#,
-        );
+}           "#,
+        )
     }
 }