From f70512cc1710656628b4173557795c1e60f0317d Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 21 Feb 2022 08:15:21 +0200 Subject: [PATCH] Change `single_let()` and `is_pattern_cond()` to free functions --- .../src/handlers/convert_bool_then.rs | 4 +-- .../src/handlers/convert_to_guarded_return.rs | 5 +-- .../src/handlers/convert_while_to_loop.rs | 3 +- crates/ide_assists/src/handlers/invert_if.rs | 3 +- .../src/handlers/replace_if_let_with_match.rs | 13 ++++--- crates/ide_db/src/helpers/node_ext.rs | 26 ++++++++++++++ crates/syntax/src/ast/node_ext.rs | 36 ------------------- 7 files changed, 44 insertions(+), 46 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_bool_then.rs b/crates/ide_assists/src/handlers/convert_bool_then.rs index 0bf2abe493..274718e6ea 100644 --- a/crates/ide_assists/src/handlers/convert_bool_then.rs +++ b/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, diff --git a/crates/ide_assists/src/handlers/convert_to_guarded_return.rs b/crates/ide_assists/src/handlers/convert_to_guarded_return.rs index 9a80db6a83..193d1cdfb2 100644 --- a/crates/ide_assists/src/handlers/convert_to_guarded_return.rs +++ 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()?; diff --git a/crates/ide_assists/src/handlers/convert_while_to_loop.rs b/crates/ide_assists/src/handlers/convert_while_to_loop.rs index 30b4daec9d..0fa2dcfbde 100644 --- a/crates/ide_assists/src/handlers/convert_while_to_loop.rs +++ b/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) diff --git a/crates/ide_assists/src/handlers/invert_if.rs b/crates/ide_assists/src/handlers/invert_if.rs index 8f99c4db67..46f11f4af3 100644 --- a/crates/ide_assists/src/handlers/invert_if.rs +++ b/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; } 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 2f35f74c5a..b594c64c41 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,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()?; diff --git a/crates/ide_db/src/helpers/node_ext.rs b/crates/ide_db/src/helpers/node_ext.rs index 82178ed749..5df3ed1366 100644 --- a/crates/ide_db/src/helpers/node_ext.rs +++ b/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 { + 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, + } +} diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index 2915e7aab1..5ff6519c9c 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/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 { - return get_pat(self.clone()); - - fn get_pat(expr: ast::Expr) -> Option { - 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),