Change single_let() and is_pattern_cond() to free functions

Author: ChayimFriedman2

crates/ide_assists/src/handlers/convert_bool_then.rs

@@ -2,7 +2,7 @@ use hir::{known, AsAssocItem, Semantics};
 use ide_db::{
     helpers::{
         for_each_tail_expr,
-        node_ext::{block_as_lone_tail, preorder_expr},
+        node_ext::{block_as_lone_tail, is_pattern_cond, preorder_expr},
         FamousDefs,
     },
     RootDatabase,
@@ -45,7 +45,7 @@ pub(crate) fn convert_if_to_bool_then(acc: &mut Assists, ctx: &AssistContext) ->
         return None;
     }
 
-    let cond = expr.condition().filter(|cond| !cond.is_pattern_cond())?;
+    let cond = expr.condition().filter(|cond| !is_pattern_cond(cond.clone()))?;
     let then = expr.then_branch()?;
     let else_ = match expr.else_branch()? {
         ast::ElseBranch::Block(b) => b,

crates/ide_assists/src/handlers/convert_to_guarded_return.rs

@@ -1,5 +1,6 @@
 use std::iter::once;
 
+use ide_db::helpers::node_ext::{is_pattern_cond, single_let};
 use syntax::{
     ast::{
         self,
@@ -48,8 +49,8 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext)
     let cond = if_expr.condition()?;
 
     // Check if there is an IfLet that we can handle.
-    let (if_let_pat, cond_expr) = if cond.is_pattern_cond() {
-        let let_ = cond.single_let()?;
+    let (if_let_pat, cond_expr) = if is_pattern_cond(cond.clone()) {
+        let let_ = single_let(cond)?;
         match let_.pat() {
             Some(ast::Pat::TupleStructPat(pat)) if pat.fields().count() == 1 => {
                 let path = pat.path()?;

crates/ide_assists/src/handlers/convert_while_to_loop.rs

@@ -1,5 +1,6 @@
 use std::iter::once;
 
+use ide_db::helpers::node_ext::is_pattern_cond;
 use syntax::{
     ast::{
         self,
@@ -54,7 +55,7 @@ pub(crate) fn convert_while_to_loop(acc: &mut Assists, ctx: &AssistContext) -> O
             let break_block =
                 make::block_expr(once(make::expr_stmt(make::expr_break(None)).into()), None)
                     .indent(while_indent_level);
-            let block_expr = if while_cond.is_pattern_cond() {
+            let block_expr = if is_pattern_cond(while_cond.clone()) {
                 let if_expr = make::expr_if(while_cond, while_body, Some(break_block.into()));
                 let stmts = once(make::expr_stmt(if_expr).into());
                 make::block_expr(stmts, None)

crates/ide_assists/src/handlers/invert_if.rs

@@ -1,3 +1,4 @@
+use ide_db::helpers::node_ext::is_pattern_cond;
 use syntax::{
     ast::{self, AstNode},
     T,
@@ -36,7 +37,7 @@ pub(crate) fn invert_if(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
 
     let cond = expr.condition()?;
     // This assist should not apply for if-let.
-    if cond.is_pattern_cond() {
+    if is_pattern_cond(cond.clone()) {
         return None;
     }
 

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,
@@ -61,7 +66,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
         }
     });
     let scrutinee_to_be_expr = if_expr.condition()?;
-    let scrutinee_to_be_expr = match scrutinee_to_be_expr.single_let() {
+    let scrutinee_to_be_expr = match single_let(scrutinee_to_be_expr.clone()) {
         Some(cond) => cond.expr()?,
         None => scrutinee_to_be_expr,
     };
@@ -70,7 +75,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
     let mut cond_bodies = Vec::new();
     for if_expr in if_exprs {
         let cond = if_expr.condition()?;
-        let cond = match cond.single_let() {
+        let cond = match single_let(cond.clone()) {
             Some(let_) => {
                 let pat = let_.pat()?;
                 let expr = let_.expr()?;
@@ -84,7 +89,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext)
                 Either::Left(pat)
             }
             // Multiple `let`, unsupported.
-            None if cond.is_pattern_cond() => return None,
+            None if is_pattern_cond(cond.clone()) => return None,
             None => Either::Right(cond),
         };
         let body = if_expr.then_branch()?;

crates/ide_db/src/helpers/node_ext.rs

@@ -216,3 +216,29 @@ pub fn vis_eq(this: &ast::Visibility, other: &ast::Visibility) -> bool {
         _ => false,
     }
 }
+
+/// Returns the `let` only if there is exactly one (that is, `let pat = expr`
+/// or `((let pat = expr))`, but not `let pat = expr && expr` or `non_let_expr`).
+pub fn single_let(expr: ast::Expr) -> Option<ast::LetExpr> {
+    match expr {
+        ast::Expr::ParenExpr(expr) => expr.expr().and_then(single_let),
+        ast::Expr::LetExpr(expr) => Some(expr),
+        _ => None,
+    }
+}
+
+pub fn is_pattern_cond(expr: ast::Expr) -> bool {
+    match expr {
+        ast::Expr::BinExpr(expr)
+            if expr.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And)) =>
+        {
+            expr.lhs()
+                .map(is_pattern_cond)
+                .or_else(|| expr.rhs().map(is_pattern_cond))
+                .unwrap_or(false)
+        }
+        ast::Expr::ParenExpr(expr) => expr.expr().map_or(false, is_pattern_cond),
+        ast::Expr::LetExpr(_) => true,
+        _ => false,
+    }
+}

crates/syntax/src/ast/node_ext.rs

@@ -528,42 +528,6 @@ impl ast::Item {
     }
 }
 
-impl ast::Expr {
-    /// Returns the `let` only if there is exactly one (that is, `let pat = expr`
-    /// or `((let pat = expr))`, but not `let pat = expr && expr` or `non_let_expr`).
-    pub fn single_let(&self) -> Option<ast::LetExpr> {
-        return get_pat(self.clone());
-
-        fn get_pat(expr: ast::Expr) -> Option<ast::LetExpr> {
-            match expr {
-                ast::Expr::ParenExpr(expr) => expr.expr().and_then(get_pat),
-                ast::Expr::LetExpr(expr) => Some(expr),
-                _ => None,
-            }
-        }
-    }
-
-    pub fn is_pattern_cond(&self) -> bool {
-        return contains_let(self.clone());
-
-        fn contains_let(expr: ast::Expr) -> bool {
-            match expr {
-                ast::Expr::BinExpr(expr)
-                    if expr.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And)) =>
-                {
-                    expr.lhs()
-                        .map(contains_let)
-                        .or_else(|| expr.rhs().map(contains_let))
-                        .unwrap_or(false)
-                }
-                ast::Expr::ParenExpr(expr) => expr.expr().map_or(false, contains_let),
-                ast::Expr::LetExpr(_) => true,
-                _ => false,
-            }
-        }
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum FieldKind {
     Name(ast::NameRef),