Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #21787 from A4-Tacks/replace-is-some-and-fallback
Fix other predicate for replace_is_method_with_if_let_method
3 files changed, 92 insertions, 29 deletions
diff --git a/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs b/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs index d22e951b5d..38d8c38ef2 100644 --- a/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs +++ b/crates/ide-assists/src/handlers/replace_is_method_with_if_let_method.rs @@ -1,8 +1,11 @@ use either::Either; use ide_db::syntax_helpers::suggest_name; -use syntax::ast::{self, AstNode, HasArgList, syntax_factory::SyntaxFactory}; +use syntax::ast::{self, AstNode, HasArgList, prec::ExprPrecedence, syntax_factory::SyntaxFactory}; -use crate::{AssistContext, AssistId, Assists, utils::cover_let_chain}; +use crate::{ + AssistContext, AssistId, Assists, + utils::{cover_let_chain, wrap_paren, wrap_paren_in_call}, +}; // Assist: replace_is_some_with_if_let_some // @@ -39,6 +42,7 @@ pub(crate) fn replace_is_method_with_if_let_method( match method_kind { "is_some" | "is_ok" => { let receiver = call_expr.receiver()?; + let make = SyntaxFactory::with_mappings(); let mut name_generator = suggest_name::NameGenerator::new_from_scope_locals( ctx.sema.scope(has_cond.syntax()), @@ -48,7 +52,7 @@ pub(crate) fn replace_is_method_with_if_let_method( } else { name_generator.for_variable(&receiver, &ctx.sema) }; - let (pat, predicate) = method_predicate(&call_expr).unzip(); + let (pat, predicate) = method_predicate(&call_expr, &var_name, &make); let (assist_id, message, text) = if method_kind == "is_some" { ("replace_is_some_with_if_let_some", "Replace `is_some` with `let Some`", "Some") @@ -61,13 +65,9 @@ pub(crate) fn replace_is_method_with_if_let_method( message, call_expr.syntax().text_range(), |edit| { - let make = SyntaxFactory::with_mappings(); let mut editor = edit.make_editor(call_expr.syntax()); - let var_pat = pat.unwrap_or_else(|| { - make.ident_pat(false, false, make.name(&var_name)).into() - }); - let pat = make.tuple_struct_pat(make.ident_path(text), [var_pat]).into(); + let pat = make.tuple_struct_pat(make.ident_path(text), [pat]).into(); let let_expr = make.expr_let(pat, receiver); if let Some(cap) = ctx.config.snippet_cap @@ -81,6 +81,7 @@ pub(crate) fn replace_is_method_with_if_let_method( let new_expr = if let Some(predicate) = predicate { let op = ast::BinaryOp::LogicOp(ast::LogicOp::And); + let predicate = wrap_paren(predicate, &make, ExprPrecedence::LAnd); make.expr_bin(let_expr.into(), op, predicate).into() } else { ast::Expr::from(let_expr) @@ -96,14 +97,23 @@ pub(crate) fn replace_is_method_with_if_let_method( } } -fn method_predicate(call_expr: &ast::MethodCallExpr) -> Option<(ast::Pat, ast::Expr)> { - let argument = call_expr.arg_list()?.args().next()?; - match argument { - ast::Expr::ClosureExpr(it) => { - let pat = it.param_list()?.params().next()?.pat()?; - Some((pat, it.body()?)) - } - _ => None, +fn method_predicate( + call_expr: &ast::MethodCallExpr, + name: &str, + make: &SyntaxFactory, +) -> (ast::Pat, Option<ast::Expr>) { + let argument = call_expr.arg_list().and_then(|it| it.args().next()); + if let Some(ast::Expr::ClosureExpr(it)) = argument.clone() + && let Some(pat) = it.param_list().and_then(|it| it.params().next()?.pat()) + { + (pat, it.body()) + } else { + let pat = make.ident_pat(false, false, make.name(name)); + let expr = argument.map(|expr| { + let arg_list = make.arg_list([make.expr_path(make.ident_path(name))]); + make.expr_call(wrap_paren_in_call(expr, make), arg_list).into() + }); + (pat.into(), expr) } } @@ -234,6 +244,54 @@ fn main() { } "#, ); + + check_assist( + replace_is_method_with_if_let_method, + r#" +fn main() { + let x = Some(1); + if x.is_som$0e_and(|it| it != 3 || it > 10) {} +} +"#, + r#" +fn main() { + let x = Some(1); + if let Some(it) = x && (it != 3 || it > 10) {} +} +"#, + ); + + check_assist( + replace_is_method_with_if_let_method, + r#" +fn main() { + let x = Some(1); + if x.is_som$0e_and(predicate) {} +} +"#, + r#" +fn main() { + let x = Some(1); + if let Some(x1) = x && predicate(x1) {} +} +"#, + ); + + check_assist( + replace_is_method_with_if_let_method, + r#" +fn main() { + let x = Some(1); + if x.is_som$0e_and(func.f) {} +} +"#, + r#" +fn main() { + let x = Some(1); + if let Some(x1) = x && (func.f)(x1) {} +} +"#, + ); } #[test] diff --git a/crates/ide-assists/src/handlers/replace_method_eager_lazy.rs b/crates/ide-assists/src/handlers/replace_method_eager_lazy.rs index 6ca3e26ca0..6e4dd8cb73 100644 --- a/crates/ide-assists/src/handlers/replace_method_eager_lazy.rs +++ b/crates/ide-assists/src/handlers/replace_method_eager_lazy.rs @@ -2,10 +2,10 @@ use hir::Semantics; use ide_db::{RootDatabase, assists::AssistId, defs::Definition}; use syntax::{ AstNode, - ast::{self, Expr, HasArgList, make}, + ast::{self, Expr, HasArgList, make, syntax_factory::SyntaxFactory}, }; -use crate::{AssistContext, Assists}; +use crate::{AssistContext, Assists, utils::wrap_paren_in_call}; // Assist: replace_with_lazy_method // @@ -177,11 +177,7 @@ fn into_call(param: &Expr, sema: &Semantics<'_, RootDatabase>) -> Expr { } })() .unwrap_or_else(|| { - let callable = if needs_parens_in_call(param) { - make::expr_paren(param.clone()).into() - } else { - param.clone() - }; + let callable = wrap_paren_in_call(param.clone(), &SyntaxFactory::without_mappings()); make::expr_call(callable, make::arg_list(Vec::new())).into() }) } @@ -200,12 +196,6 @@ fn ends_is(name: &str, end: &str) -> bool { name.strip_suffix(end).is_some_and(|s| s.is_empty() || s.ends_with('_')) } -fn needs_parens_in_call(param: &Expr) -> bool { - let call = make::expr_call(make::ext::expr_unit(), make::arg_list(Vec::new())); - let callable = call.expr().expect("invalid make call"); - param.needs_parens_in_place_of(call.syntax(), callable.syntax()) -} - #[cfg(test)] mod tests { use crate::tests::check_assist; diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 27788a70eb..ef178cdc70 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -25,6 +25,7 @@ use syntax::{ edit::{AstNodeEdit, IndentLevel}, edit_in_place::AttrsOwnerEdit, make, + prec::ExprPrecedence, syntax_factory::SyntaxFactory, }, syntax_editor::{Element, Removable, SyntaxEditor}, @@ -97,6 +98,20 @@ pub(crate) fn wrap_block(expr: &ast::Expr, make: &SyntaxFactory) -> ast::BlockEx } } +pub(crate) fn wrap_paren(expr: ast::Expr, make: &SyntaxFactory, prec: ExprPrecedence) -> ast::Expr { + if expr.precedence().needs_parentheses_in(prec) { make.expr_paren(expr).into() } else { expr } +} + +pub(crate) fn wrap_paren_in_call(expr: ast::Expr, make: &SyntaxFactory) -> ast::Expr { + if needs_parens_in_call(&expr) { make.expr_paren(expr).into() } else { expr } +} + +fn needs_parens_in_call(param: &ast::Expr) -> bool { + let call = make::expr_call(make::ext::expr_unit(), make::arg_list(Vec::new())); + let callable = call.expr().expect("invalid make call"); + param.needs_parens_in_place_of(call.syntax(), callable.syntax()) +} + /// This is a method with a heuristics to support test methods annotated with custom test annotations, such as /// `#[test_case(...)]`, `#[tokio::test]` and similar. /// Also a regular `#[test]` annotation is supported. |