From d6695286ee50c37c84294d97c71d3f0799f98a95 Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Fri, 20 Aug 2021 15:20:54 +0200
Subject: [PATCH 1/4] Add empty-body check to replace_match_with_if_let and
 re-prioritize choices

---
 .../src/handlers/replace_if_let_with_match.rs | 48 +++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

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 323f58acbbe7..d3269ef4d417 100644
--- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
@@ -235,12 +235,14 @@ fn pick_pattern_and_expr_order(
 ) -> Option<(ast::Pat, ast::Expr, ast::Expr)> {
     let res = match (pat, pat2) {
         (ast::Pat::WildcardPat(_), _) => return None,
-        (pat, sad_pat) if is_sad_pat(sema, &sad_pat) => (pat, expr, expr2),
-        (sad_pat, pat) if is_sad_pat(sema, &sad_pat) => (pat, expr2, expr),
+        (pat, _) if expr2.syntax().first_child().is_none() => (pat, expr, expr2),
+        (_, pat) if expr.syntax().first_child().is_none() => (pat, expr2, expr),
         (pat, pat2) => match (binds_name(&pat), binds_name(&pat2)) {
-            (true, true) => return None,
             (true, false) => (pat, expr, expr2),
             (false, true) => (pat2, expr2, expr),
+            _ if is_sad_pat(sema, &pat2) => (pat, expr, expr2),
+            _ if is_sad_pat(sema, &pat) => (pat2, expr2, expr),
+            (true, true) => return None,
             (false, false) => (pat, expr, expr2),
         },
     };
@@ -762,6 +764,46 @@ fn foo() {
         );
     }
 
+    #[test]
+    fn replace_match_with_if_let_prefer_nonempty_body() {
+        check_assist(
+            replace_match_with_if_let,
+            r#"
+fn foo() {
+    match $0Ok(0) {
+        Ok(value) => {},
+        Err(err) => eprintln!("{}", err),
+    }
+}
+"#,
+            r#"
+fn foo() {
+    if let Err(err) = Ok(0) {
+        eprintln!("{}", err)
+    }
+}
+"#,
+        );
+        check_assist(
+            replace_match_with_if_let,
+            r#"
+fn foo() {
+    match $0Ok(0) {
+        Err(err) => eprintln!("{}", err),
+        Ok(value) => {},
+    }
+}
+"#,
+            r#"
+fn foo() {
+    if let Err(err) = Ok(0) {
+        eprintln!("{}", err)
+    }
+}
+"#,
+        );
+    }
+
     #[test]
     fn replace_match_with_if_let_rejects_double_name_bindings() {
         check_assist_not_applicable(

From 75f07012111972dcf38900b2d4347646fb89b75b Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Sat, 21 Aug 2021 11:00:43 +0200
Subject: [PATCH 2/4] Add heuristic to determine type of IdentPat, make check
 for empty expressions correct

---
 .../src/handlers/replace_if_let_with_match.rs | 42 +++++++++++++++++--
 1 file changed, 38 insertions(+), 4 deletions(-)

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 d3269ef4d417..025add6bc88a 100644
--- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
@@ -235,24 +235,36 @@ fn pick_pattern_and_expr_order(
 ) -> Option<(ast::Pat, ast::Expr, ast::Expr)> {
     let res = match (pat, pat2) {
         (ast::Pat::WildcardPat(_), _) => return None,
-        (pat, _) if expr2.syntax().first_child().is_none() => (pat, expr, expr2),
-        (_, pat) if expr.syntax().first_child().is_none() => (pat, expr2, expr),
+        (pat, _) if is_empty_expr(&expr2) => (pat, expr, expr2),
+        (_, pat) if is_empty_expr(&expr) => (pat, expr2, expr),
         (pat, pat2) => match (binds_name(&pat), binds_name(&pat2)) {
+            (true, true) => return None,
             (true, false) => (pat, expr, expr2),
             (false, true) => (pat2, expr2, expr),
             _ if is_sad_pat(sema, &pat2) => (pat, expr, expr2),
             _ if is_sad_pat(sema, &pat) => (pat2, expr2, expr),
-            (true, true) => return None,
             (false, false) => (pat, expr, expr2),
         },
     };
     Some(res)
 }
 
+fn is_empty_expr(expr: &ast::Expr) -> bool {
+    match expr {
+        ast::Expr::BlockExpr(expr) => {
+            expr.statements().next().is_none() && expr.tail_expr().is_none()
+        }
+        ast::Expr::TupleExpr(expr) => expr.fields().next().is_none(),
+        _ => false,
+    }
+}
+
 fn binds_name(pat: &ast::Pat) -> bool {
     let binds_name_v = |pat| binds_name(&pat);
     match pat {
-        ast::Pat::IdentPat(_) => true,
+        ast::Pat::IdentPat(pat) => {
+            pat.to_string().starts_with(|c: char| c.is_lowercase() && c != '_')
+        }
         ast::Pat::MacroPat(_) => true,
         ast::Pat::OrPat(pat) => pat.pats().any(binds_name_v),
         ast::Pat::SlicePat(pat) => pat.pats().any(binds_name_v),
@@ -704,6 +716,28 @@ fn main() {
         )
     }
 
+    #[test]
+    fn replace_match_with_if_let_number_body() {
+        check_assist(
+            replace_match_with_if_let,
+            r#"
+fn main() {
+    $0match Ok(()) {
+        Ok(()) => {},
+        Err(_) => 0,
+    }
+}
+"#,
+            r#"
+fn main() {
+    if let Err(_) = Ok(()) {
+        0
+    }
+}
+"#,
+        )
+    }
+
     #[test]
     fn replace_match_with_if_let_exhaustive() {
         check_assist(

From 7cff9303937dc5a292f246c5d1b99c3f5351841a Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Sat, 21 Aug 2021 11:11:27 +0200
Subject: [PATCH 3/4] Use NameClass::classify to check for ConstReference

---
 .../src/handlers/replace_if_let_with_match.rs     | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

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 025add6bc88a..43e8ad1c1286 100644
--- 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,12 +1,12 @@
 use std::iter::{self, successors};
 
 use either::Either;
-use ide_db::{ty_filter::TryEnum, RootDatabase};
+use ide_db::{defs::NameClass, ty_filter::TryEnum, RootDatabase};
 use syntax::{
     ast::{
         self,
         edit::{AstNodeEdit, IndentLevel},
-        make,
+        make, NameOwner,
     },
     AstNode,
 };
@@ -237,7 +237,7 @@ fn pick_pattern_and_expr_order(
         (ast::Pat::WildcardPat(_), _) => return None,
         (pat, _) if is_empty_expr(&expr2) => (pat, expr, expr2),
         (_, pat) if is_empty_expr(&expr) => (pat, expr2, expr),
-        (pat, pat2) => match (binds_name(&pat), binds_name(&pat2)) {
+        (pat, pat2) => match (binds_name(sema, &pat), binds_name(sema, &pat2)) {
             (true, true) => return None,
             (true, false) => (pat, expr, expr2),
             (false, true) => (pat2, expr2, expr),
@@ -259,11 +259,14 @@ fn is_empty_expr(expr: &ast::Expr) -> bool {
     }
 }
 
-fn binds_name(pat: &ast::Pat) -> bool {
-    let binds_name_v = |pat| binds_name(&pat);
+fn binds_name(sema: &hir::Semantics<RootDatabase>, pat: &ast::Pat) -> bool {
+    let binds_name_v = |pat| binds_name(&sema, &pat);
     match pat {
         ast::Pat::IdentPat(pat) => {
-            pat.to_string().starts_with(|c: char| c.is_lowercase() && c != '_')
+            match pat.name().and_then(|name| NameClass::classify(sema, &name)) {
+                Some(NameClass::ConstReference(_)) => false,
+                _ => true,
+            }
         }
         ast::Pat::MacroPat(_) => true,
         ast::Pat::OrPat(pat) => pat.pats().any(binds_name_v),

From e47c9743cfa7f6b3e271b4f5957d8c0913ce8520 Mon Sep 17 00:00:00 2001
From: elkowar <5300871+elkowar@users.noreply.github.com>
Date: Sat, 21 Aug 2021 12:02:21 +0200
Subject: [PATCH 4/4] Fix smaller nitpicks

---
 .../src/handlers/replace_if_let_with_match.rs     | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

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 43e8ad1c1286..5ce2ad3193e6 100644
--- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
@@ -241,7 +241,6 @@ fn pick_pattern_and_expr_order(
             (true, true) => return None,
             (true, false) => (pat, expr, expr2),
             (false, true) => (pat2, expr2, expr),
-            _ if is_sad_pat(sema, &pat2) => (pat, expr, expr2),
             _ if is_sad_pat(sema, &pat) => (pat2, expr2, expr),
             (false, false) => (pat, expr, expr2),
         },
@@ -251,9 +250,7 @@ fn pick_pattern_and_expr_order(
 
 fn is_empty_expr(expr: &ast::Expr) -> bool {
     match expr {
-        ast::Expr::BlockExpr(expr) => {
-            expr.statements().next().is_none() && expr.tail_expr().is_none()
-        }
+        ast::Expr::BlockExpr(expr) => expr.is_empty(),
         ast::Expr::TupleExpr(expr) => expr.fields().next().is_none(),
         _ => false,
     }
@@ -262,12 +259,10 @@ fn is_empty_expr(expr: &ast::Expr) -> bool {
 fn binds_name(sema: &hir::Semantics<RootDatabase>, pat: &ast::Pat) -> bool {
     let binds_name_v = |pat| binds_name(&sema, &pat);
     match pat {
-        ast::Pat::IdentPat(pat) => {
-            match pat.name().and_then(|name| NameClass::classify(sema, &name)) {
-                Some(NameClass::ConstReference(_)) => false,
-                _ => true,
-            }
-        }
+        ast::Pat::IdentPat(pat) => !matches!(
+            pat.name().and_then(|name| NameClass::classify(sema, &name)),
+            Some(NameClass::ConstReference(_))
+        ),
         ast::Pat::MacroPat(_) => true,
         ast::Pat::OrPat(pat) => pat.pats().any(binds_name_v),
         ast::Pat::SlicePat(pat) => pat.pats().any(binds_name_v),