Merge #9962

9962: Add empty-body check to replace_match_with_if_let and re-prioritize choices r=elkowar a=elkowar

This PR changes some behaviour of the `replace_match_with_if_let` ide-assist.
Concretely, it makes two changes:

it introduces a check for empty expression bodies. This means that checks of the shape
```rs
match x {
  A => {}
  B => {
    println!("hi");
  }
}
```
will prefer to use the B branch as the first (and only) variant.

It also reprioritizes the importance of "happy" and "sad" patterns.
Concretely, if there are reasons to prefer having the sad pattern be the first (/only) pattern,
it will follow these.
This means that in the case of 
```rs
match x {
  Ok(_) => {
    println!("Success");
  }
  Err(e) => {
    println!("Failure: {}", e);
  }
}
```
the `Err` variant will correctly be used as the first expression in the generated if.
Up until now, the generated code was actually invalid, as it would generate
```rs
if let Ok(_) = x {
  println!("Success");
} else {
  println!("Failure: {}", e);
}
```
where `e` in the else branch is not defined.


Co-authored-by: elkowar <[email protected]>

Author: bors[bot]

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,
 };
@@ -235,22 +235,34 @@ 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, pat2) => match (binds_name(&pat), binds_name(&pat2)) {
+        (pat, _) if is_empty_expr(&expr2) => (pat, expr, expr2),
+        (_, pat) if is_empty_expr(&expr) => (pat, expr2, expr),
+        (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),
+            _ if is_sad_pat(sema, &pat) => (pat2, expr2, expr),
             (false, false) => (pat, expr, expr2),
         },
     };
     Some(res)
 }
 
-fn binds_name(pat: &ast::Pat) -> bool {
-    let binds_name_v = |pat| binds_name(&pat);
+fn is_empty_expr(expr: &ast::Expr) -> bool {
+    match expr {
+        ast::Expr::BlockExpr(expr) => expr.is_empty(),
+        ast::Expr::TupleExpr(expr) => expr.fields().next().is_none(),
+        _ => false,
+    }
+}
+
+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(_) => 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),
@@ -702,6 +714,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(
@@ -762,6 +796,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(