Add guard support for replace_if_let_with_match
- Fix loses comments
- Fix bad indentation
Example
---
```rust
fn main() {
if $0let true = true
&& true
&& false
{
code()
}
}
```
**Before this PR**
Assist not applicable
**After this PR**
```rust
fn main() {
match true {
true if true
&& false => code(),
_ => (),
}
}
```
---
```rust
pub fn foo(foo: i32) {
$0if let 1 = foo {
// some comment
self.foo();
} else if let 2 = foo {
// some comment 2
self.bar()
}
}
```
**Before this PR**
```rust
pub fn foo(foo: i32) {
match foo {
1 => {
self.foo();
}
2 => self.bar(),
_ => (),
}
}
```
**After this PR**
```rust
pub fn foo(foo: i32) {
match foo {
1 => {
// some comment
self.foo();
}
2 => {
// some comment 2
self.bar()
},
_ => (),
}
}
```
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 3b815a4..b7e5344 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,15 +1,10 @@
use std::iter::successors;
-use either::Either;
-use ide_db::{
- RootDatabase,
- defs::NameClass,
- syntax_helpers::node_ext::{is_pattern_cond, single_let},
- ty_filter::TryEnum,
-};
+use ide_db::{RootDatabase, defs::NameClass, ty_filter::TryEnum};
use syntax::{
- AstNode, Edition, T, TextRange,
+ AstNode, Edition, SyntaxKind, T, TextRange,
ast::{self, HasName, edit::IndentLevel, edit_in_place::Indent, syntax_factory::SyntaxFactory},
+ syntax_editor::SyntaxEditor,
};
use crate::{
@@ -54,42 +49,46 @@
return None;
}
let mut else_block = None;
+ let indent = if_expr.indent_level();
let if_exprs = successors(Some(if_expr.clone()), |expr| match expr.else_branch()? {
ast::ElseBranch::IfExpr(expr) => Some(expr),
ast::ElseBranch::Block(block) => {
+ let block = unwrap_trivial_block(block).clone_for_update();
+ block.reindent_to(IndentLevel(1));
else_block = Some(block);
None
}
});
let scrutinee_to_be_expr = if_expr.condition()?;
- let scrutinee_to_be_expr = match single_let(scrutinee_to_be_expr.clone()) {
- Some(cond) => cond.expr()?,
- None => scrutinee_to_be_expr,
+ let scrutinee_to_be_expr = match let_and_guard(&scrutinee_to_be_expr) {
+ (Some(let_expr), _) => let_expr.expr()?,
+ (None, cond) => cond?,
};
let mut pat_seen = false;
let mut cond_bodies = Vec::new();
for if_expr in if_exprs {
let cond = if_expr.condition()?;
- let cond = match single_let(cond.clone()) {
- Some(let_) => {
+ let (cond, guard) = match let_and_guard(&cond) {
+ (None, guard) => (None, Some(guard?)),
+ (Some(let_), guard) => {
let pat = let_.pat()?;
let expr = let_.expr()?;
- // FIXME: If one `let` is wrapped in parentheses and the second is not,
- // we'll exit here.
if scrutinee_to_be_expr.syntax().text() != expr.syntax().text() {
// Only if all condition expressions are equal we can merge them into a match
return None;
}
pat_seen = true;
- Either::Left(pat)
+ (Some(pat), guard)
}
- // Multiple `let`, unsupported.
- None if is_pattern_cond(cond.clone()) => return None,
- None => Either::Right(cond),
};
- let body = if_expr.then_branch()?;
- cond_bodies.push((cond, body));
+ if let Some(guard) = &guard {
+ guard.dedent(indent);
+ guard.indent(IndentLevel(1));
+ }
+ let body = if_expr.then_branch()?.clone_for_update();
+ body.indent(IndentLevel(1));
+ cond_bodies.push((cond, guard, body));
}
if !pat_seen && cond_bodies.len() != 1 {
@@ -106,27 +105,25 @@
available_range,
move |builder| {
let make = SyntaxFactory::with_mappings();
- let match_expr = {
+ let match_expr: ast::Expr = {
let else_arm = make_else_arm(ctx, &make, else_block, &cond_bodies);
- let make_match_arm = |(pat, body): (_, ast::BlockExpr)| {
- let body = make.block_expr(body.statements(), body.tail_expr());
- body.indent(IndentLevel::from(1));
- let body = unwrap_trivial_block(body);
- match pat {
- Either::Left(pat) => make.match_arm(pat, None, body),
- Either::Right(_) if !pat_seen => {
- make.match_arm(make.literal_pat("true").into(), None, body)
+ let make_match_arm =
+ |(pat, guard, body): (_, Option<ast::Expr>, ast::BlockExpr)| {
+ body.reindent_to(IndentLevel::single());
+ let body = unwrap_trivial_block(body);
+ match (pat, guard.map(|it| make.match_guard(it))) {
+ (Some(pat), guard) => make.match_arm(pat, guard, body),
+ (None, _) if !pat_seen => {
+ make.match_arm(make.literal_pat("true").into(), None, body)
+ }
+ (None, guard) => {
+ make.match_arm(make.wildcard_pat().into(), guard, body)
+ }
}
- Either::Right(expr) => make.match_arm(
- make.wildcard_pat().into(),
- Some(make.match_guard(expr)),
- body,
- ),
- }
- };
+ };
let arms = cond_bodies.into_iter().map(make_match_arm).chain([else_arm]);
let match_expr = make.expr_match(scrutinee_to_be_expr, make.match_arm_list(arms));
- match_expr.indent(IndentLevel::from_node(if_expr.syntax()));
+ match_expr.indent(indent);
match_expr.into()
};
@@ -134,7 +131,11 @@
if_expr.syntax().parent().is_some_and(|it| ast::IfExpr::can_cast(it.kind()));
let expr = if has_preceding_if_expr {
// make sure we replace the `else if let ...` with a block so we don't end up with `else expr`
- make.block_expr([], Some(match_expr)).into()
+ match_expr.dedent(indent);
+ match_expr.indent(IndentLevel(1));
+ let block_expr = make.block_expr([], Some(match_expr));
+ block_expr.indent(indent);
+ block_expr.into()
} else {
match_expr
};
@@ -150,13 +151,13 @@
fn make_else_arm(
ctx: &AssistContext<'_>,
make: &SyntaxFactory,
- else_block: Option<ast::BlockExpr>,
- conditionals: &[(Either<ast::Pat, ast::Expr>, ast::BlockExpr)],
+ else_expr: Option<ast::Expr>,
+ conditionals: &[(Option<ast::Pat>, Option<ast::Expr>, ast::BlockExpr)],
) -> ast::MatchArm {
- let (pattern, expr) = if let Some(else_block) = else_block {
+ let (pattern, expr) = if let Some(else_expr) = else_expr {
let pattern = match conditionals {
- [(Either::Right(_), _)] => make.literal_pat("false").into(),
- [(Either::Left(pat), _)] => match ctx
+ [(None, Some(_), _)] => make.literal_pat("false").into(),
+ [(Some(pat), _, _)] => match ctx
.sema
.type_of_pat(pat)
.and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty.adjusted()))
@@ -174,10 +175,10 @@
},
_ => make.wildcard_pat().into(),
};
- (pattern, unwrap_trivial_block(else_block))
+ (pattern, else_expr)
} else {
let pattern = match conditionals {
- [(Either::Right(_), _)] => make.literal_pat("false").into(),
+ [(None, Some(_), _)] => make.literal_pat("false").into(),
_ => make.wildcard_pat().into(),
};
(pattern, make.expr_unit())
@@ -266,7 +267,10 @@
// wrap them in another BlockExpr.
match expr {
ast::Expr::BlockExpr(block) if block.modifier().is_none() => block,
- expr => make.block_expr([], Some(expr)),
+ expr => {
+ expr.indent(IndentLevel(1));
+ make.block_expr([], Some(expr))
+ }
}
};
@@ -289,7 +293,9 @@
condition
};
let then_expr = then_expr.clone_for_update();
+ let else_expr = else_expr.clone_for_update();
then_expr.reindent_to(IndentLevel::single());
+ else_expr.reindent_to(IndentLevel::single());
let then_block = make_block_expr(then_expr);
let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) };
let if_let_expr = make.expr_if(
@@ -382,6 +388,48 @@
.is_some_and(|it| does_pat_match_variant(pat, &it.sad_pattern()))
}
+fn let_and_guard(cond: &ast::Expr) -> (Option<ast::LetExpr>, Option<ast::Expr>) {
+ if let ast::Expr::ParenExpr(expr) = cond
+ && let Some(sub_expr) = expr.expr()
+ {
+ let_and_guard(&sub_expr)
+ } else if let ast::Expr::LetExpr(let_expr) = cond {
+ (Some(let_expr.clone()), None)
+ } else if let ast::Expr::BinExpr(bin_expr) = cond
+ && let Some(ast::Expr::LetExpr(let_expr)) = and_bin_expr_left(bin_expr).lhs()
+ {
+ let new_expr = bin_expr.clone_subtree();
+ let mut edit = SyntaxEditor::new(new_expr.syntax().clone());
+
+ let left_bin = and_bin_expr_left(&new_expr);
+ if let Some(rhs) = left_bin.rhs() {
+ edit.replace(left_bin.syntax(), rhs.syntax());
+ } else {
+ if let Some(next) = left_bin.syntax().next_sibling_or_token()
+ && next.kind() == SyntaxKind::WHITESPACE
+ {
+ edit.delete(next);
+ }
+ edit.delete(left_bin.syntax());
+ }
+
+ let new_expr = edit.finish().new_root().clone();
+ (Some(let_expr), ast::Expr::cast(new_expr))
+ } else {
+ (None, Some(cond.clone()))
+ }
+}
+
+fn and_bin_expr_left(expr: &ast::BinExpr) -> ast::BinExpr {
+ if expr.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And))
+ && let Some(ast::Expr::BinExpr(left)) = expr.lhs()
+ {
+ and_bin_expr_left(&left)
+ } else {
+ expr.clone()
+ }
+}
+
#[cfg(test)]
mod tests {
use super::*;
@@ -453,6 +501,45 @@
}
#[test]
+ fn test_if_with_match_comments() {
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+pub fn foo(foo: i32) {
+ $0if let 1 = foo {
+ // some comment
+ self.foo();
+ } else if let 2 = foo {
+ // some comment 2
+ self.bar()
+ } else {
+ // some comment 3
+ self.baz();
+ }
+}
+"#,
+ r#"
+pub fn foo(foo: i32) {
+ match foo {
+ 1 => {
+ // some comment
+ self.foo();
+ }
+ 2 => {
+ // some comment 2
+ self.bar()
+ }
+ _ => {
+ // some comment 3
+ self.baz();
+ }
+ }
+}
+"#,
+ )
+ }
+
+ #[test]
fn test_if_let_with_match_no_else() {
check_assist(
replace_if_let_with_match,
@@ -514,14 +601,151 @@
#[test]
fn test_if_let_with_match_let_chain() {
- check_assist_not_applicable(
+ check_assist(
replace_if_let_with_match,
r#"
+#![feature(if_let_guard)]
+fn main() {
+ if $0let true = true && let Some(1) = None {} else { other() }
+}
+"#,
+ r#"
+#![feature(if_let_guard)]
+fn main() {
+ match true {
+ true if let Some(1) = None => {}
+ _ => other(),
+ }
+}
+"#,
+ );
+
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+#![feature(if_let_guard)]
+fn main() {
+ if true {
+ $0if let ParenExpr(expr) = cond
+ && let Some(sub_expr) = expr.expr()
+ {
+ branch1(
+ "..."
+ )
+ } else if let LetExpr(let_expr) = cond {
+ branch2(
+ "..."
+ )
+ } else if let BinExpr(bin_expr) = cond
+ && let Some(kind) = bin_expr.op_kind()
+ && let Some(LetExpr(let_expr)) = foo(bin_expr)
+ {
+ branch3()
+ } else {
+ branch4(
+ "..."
+ )
+ }
+ }
+}
+"#,
+ r#"
+#![feature(if_let_guard)]
+fn main() {
+ if true {
+ match cond {
+ ParenExpr(expr) if let Some(sub_expr) = expr.expr() => {
+ branch1(
+ "..."
+ )
+ }
+ LetExpr(let_expr) => {
+ branch2(
+ "..."
+ )
+ }
+ BinExpr(bin_expr) if let Some(kind) = bin_expr.op_kind()
+ && let Some(LetExpr(let_expr)) = foo(bin_expr) => branch3(),
+ _ => {
+ branch4(
+ "..."
+ )
+ }
+ }
+ }
+}
+"#,
+ );
+
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+fn main() {
+ if $0let true = true
+ && true
+ && false
+ {
+ code()
+ } else {
+ other()
+ }
+}
+"#,
+ r#"
+fn main() {
+ match true {
+ true if true
+ && false => code(),
+ _ => other(),
+ }
+}
+"#,
+ );
+ }
+
+ #[test]
+ fn test_if_let_with_match_let_chain_no_else() {
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+#![feature(if_let_guard)]
fn main() {
if $0let true = true && let Some(1) = None {}
}
"#,
- )
+ r#"
+#![feature(if_let_guard)]
+fn main() {
+ match true {
+ true if let Some(1) = None => {}
+ _ => (),
+ }
+}
+"#,
+ );
+
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+fn main() {
+ if $0let true = true
+ && true
+ && false
+ {
+ code()
+ }
+}
+"#,
+ r#"
+fn main() {
+ match true {
+ true if true
+ && false => code(),
+ _ => (),
+ }
+}
+"#,
+ );
}
#[test]
@@ -553,10 +777,10 @@
VariantData::Tuple(..) => false,
_ if cond() => true,
_ => {
- bar(
- 123
- )
- }
+ bar(
+ 123
+ )
+ }
}
}
}
@@ -587,11 +811,11 @@
if let VariantData::Struct(..) = *self {
true
} else {
- match *self {
- VariantData::Tuple(..) => false,
- _ => false,
+ match *self {
+ VariantData::Tuple(..) => false,
+ _ => false,
+ }
}
-}
}
}
"#,
@@ -706,9 +930,12 @@
fn main() {
if true {
$0if let Ok(rel_path) = path.strip_prefix(root_path) {
- let rel_path = RelativePathBuf::from_path(rel_path).ok()?;
+ let rel_path = RelativePathBuf::from_path(rel_path)
+ .ok()?;
Some((*id, rel_path))
} else {
+ let _ = some_code()
+ .clone();
None
}
}
@@ -719,10 +946,52 @@
if true {
match path.strip_prefix(root_path) {
Ok(rel_path) => {
- let rel_path = RelativePathBuf::from_path(rel_path).ok()?;
+ let rel_path = RelativePathBuf::from_path(rel_path)
+ .ok()?;
Some((*id, rel_path))
}
- _ => None,
+ _ => {
+ let _ = some_code()
+ .clone();
+ None
+ }
+ }
+ }
+}
+"#,
+ );
+
+ check_assist(
+ replace_if_let_with_match,
+ r#"
+fn main() {
+ if true {
+ $0if let Ok(rel_path) = path.strip_prefix(root_path) {
+ Foo {
+ x: 1
+ }
+ } else {
+ Foo {
+ x: 2
+ }
+ }
+ }
+}
+"#,
+ r#"
+fn main() {
+ if true {
+ match path.strip_prefix(root_path) {
+ Ok(rel_path) => {
+ Foo {
+ x: 1
+ }
+ }
+ _ => {
+ Foo {
+ x: 2
+ }
+ }
}
}
}
@@ -1583,11 +1852,12 @@
fn main() {
if true {
$0match path.strip_prefix(root_path) {
- Ok(rel_path) => {
- let rel_path = RelativePathBuf::from_path(rel_path).ok()?;
- Some((*id, rel_path))
+ Ok(rel_path) => Foo {
+ x: 2
}
- _ => None,
+ _ => Foo {
+ x: 3
+ },
}
}
}
@@ -1596,15 +1866,55 @@
fn main() {
if true {
if let Ok(rel_path) = path.strip_prefix(root_path) {
- let rel_path = RelativePathBuf::from_path(rel_path).ok()?;
+ Foo {
+ x: 2
+ }
+ } else {
+ Foo {
+ x: 3
+ }
+ }
+ }
+}
+"#,
+ );
+
+ check_assist(
+ replace_match_with_if_let,
+ r#"
+fn main() {
+ if true {
+ $0match path.strip_prefix(root_path) {
+ Ok(rel_path) => {
+ let rel_path = RelativePathBuf::from_path(rel_path)
+ .ok()?;
+ Some((*id, rel_path))
+ }
+ _ => {
+ let _ = some_code()
+ .clone();
+ None
+ },
+ }
+ }
+}
+"#,
+ r#"
+fn main() {
+ if true {
+ if let Ok(rel_path) = path.strip_prefix(root_path) {
+ let rel_path = RelativePathBuf::from_path(rel_path)
+ .ok()?;
Some((*id, rel_path))
} else {
+ let _ = some_code()
+ .clone();
None
}
}
}
"#,
- )
+ );
}
#[test]
diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs
index e43516f..fbdd066 100644
--- a/crates/ide-assists/src/utils.rs
+++ b/crates/ide-assists/src/utils.rs
@@ -57,6 +57,14 @@
});
non_trivial_children.next().is_some()
};
+ if stmt_list
+ .syntax()
+ .children_with_tokens()
+ .filter_map(NodeOrToken::into_token)
+ .any(|token| token.kind() == syntax::SyntaxKind::COMMENT)
+ {
+ return None;
+ }
if let Some(expr) = stmt_list.tail_expr() {
if has_anything_else(expr.syntax()) {
