Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #22618 from ada4a/replace_match_with_if_let-no-paren-if_let-guard
fix(assists/replace_match_with_if_let): don't parenthesize if-let guards
| -rw-r--r-- | crates/ide-assists/src/handlers/replace_if_let_with_match.rs | 73 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils.rs | 20 | ||||
| -rw-r--r-- | crates/syntax/src/ast/prec.rs | 23 |
3 files changed, 113 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 6959988db7..5225202177 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 @@ -17,7 +17,7 @@ use crate::{ AssistContext, AssistId, Assists, utils::{ does_pat_match_variant, does_pat_variant_nested_or_literal, unwrap_trivial_block, - wrap_paren, + wrap_paren_in_guard_chain, }, }; @@ -303,7 +303,7 @@ pub(crate) fn replace_match_with_if_let( _ => make.expr_let(if_let_pat, scrutinee).into(), }; let condition = if let Some(guard) = guard { - let guard = wrap_paren(guard, make, ast::prec::ExprPrecedence::LAnd); + let guard = wrap_paren_in_guard_chain(guard, make); make.expr_bin(condition, ast::BinaryOp::LogicOp(ast::LogicOp::And), guard).into() } else { condition @@ -2454,7 +2454,7 @@ fn main() { } #[test] - fn test_replace_match_with_if_let_chain() { + fn test_replace_match_with_if_let_with_simple_guard() { check_assist( replace_match_with_if_let, r#" @@ -2499,6 +2499,73 @@ fn main() { } #[test] + fn test_replace_match_with_if_let_with_if_let_guard() { + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 Some(0) { + Some(n) if let Some(m) = n.checked_add(1) => (), + _ => code(), + } +} +"#, + r#" +fn main() { + if let Some(n) = Some(0) && let Some(m) = n.checked_add(1) { + () + } else { + code() + } +} +"#, + ); + + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 Some(0) { + Some(n) if let Some(m) = n.checked_add(1) && m > 5 => (), + _ => code(), + } +} + "#, + r#" +fn main() { + if let Some(n) = Some(0) && let Some(m) = n.checked_add(1) && m > 5 { + () + } else { + code() + } +} + "#, + ); + + // what if the `let` expr is not the first one in the guard? + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 Some(0) { + Some(n) if n > 5 && let Some(m) = n.checked_add(1) => (), + _ => code(), + } +} + "#, + r#" +fn main() { + if let Some(n) = Some(0) && n > 5 && let Some(m) = n.checked_add(1) { + () + } else { + code() + } +} + "#, + ); + } + + #[test] fn test_replace_match_with_if_let_not_applicable_pat2_is_ident_pat() { check_assist_not_applicable( replace_match_with_if_let, diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 086f54ed17..2c4fb5f405 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -110,6 +110,26 @@ fn needs_parens_in_call(make: &SyntaxFactory, param: &ast::Expr) -> bool { param.needs_parens_in_place_of(call.syntax(), callable.syntax()) } +pub(crate) fn wrap_paren_in_guard_chain(guard: ast::Expr, make: &SyntaxFactory) -> ast::Expr { + if needs_parens_in_guard_chain(make, &guard) { make.expr_paren(guard).into() } else { guard } +} + +fn needs_parens_in_guard_chain(make: &SyntaxFactory, guard: &ast::Expr) -> bool { + let ast::Expr::BinExpr(if_let_and_guard) = make.expr_bin_op( + make.expr_unit(), + ast::BinaryOp::LogicOp(ast::LogicOp::And), + make.expr_unit(), + ) else { + stdx::never!("`SyntaxFactory::expr_bin_op` returns a `BinExpr`"); + return false; + }; + let Some(fake_guard) = if_let_and_guard.rhs() else { + stdx::never!("invalid make call"); + return false; + }; + guard.needs_parens_in_place_of(if_let_and_guard.syntax(), fake_guard.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. diff --git a/crates/syntax/src/ast/prec.rs b/crates/syntax/src/ast/prec.rs index 8411275350..2a50d233c3 100644 --- a/crates/syntax/src/ast/prec.rs +++ b/crates/syntax/src/ast/prec.rs @@ -264,6 +264,14 @@ impl Expr { return false; } + // Special-case `cond && <let-chain>` + if let ast::Expr::BinExpr(parent) = parent + && parent.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And)) + && self.contains_let_expr() + { + return false; + } + let (left, right, inv) = match self.is_ordered_before_parent_in_place_of(parent, place_of) { true => (self, parent, false), false => (parent, self, true), @@ -551,4 +559,19 @@ impl Expr { ForExpr(_) | IfExpr(_) | MatchExpr(_) | WhileExpr(_) | IncludeBytesExpr(_) => true, } } + + fn contains_let_expr(&self) -> bool { + use Expr::*; + + match self { + LetExpr(_) => true, + BinExpr(e) => { + // if we find something other than a `&&`, then this can't be a let chain + e.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And)) + && (e.lhs().is_none_or(|it| it.contains_let_expr()) + || e.rhs().is_none_or(|it| it.contains_let_expr())) + } + _ => false, + } + } } |