Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #22199 from A4-Tacks/to-guarded-else-if
feat: offer on if-expr with else-if for convert_to_guarded_return
| -rw-r--r-- | crates/ide-assists/src/handlers/convert_to_guarded_return.rs | 200 | ||||
| -rw-r--r-- | crates/syntax/src/ast/make.rs | 7 |
2 files changed, 187 insertions, 20 deletions
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 791a6a26af..44ca57281e 100644 --- a/crates/ide-assists/src/handlers/convert_to_guarded_return.rs +++ b/crates/ide-assists/src/handlers/convert_to_guarded_return.rs @@ -72,12 +72,6 @@ fn if_expr_to_guarded_return( ctx: &AssistContext<'_>, ) -> Option<()> { let make = SyntaxFactory::without_mappings(); - let else_block = match if_expr.else_branch() { - Some(ast::ElseBranch::Block(block_expr)) => Some(block_expr), - Some(_) => return None, - _ => None, - }; - let cond = if_expr.condition()?; let if_token_range = if_expr.if_token()?.text_range(); @@ -102,7 +96,7 @@ fn if_expr_to_guarded_return( } let container = container_of(&parent_block)?; - let else_block = ElseBlock::new(&ctx.sema, else_block, &container)?; + let else_block = ElseBlock::new(&ctx.sema, if_expr.else_branch(), &container)?; if parent_block.tail_expr() != Some(if_expr.clone().into()) && !(else_block.is_never_block @@ -237,7 +231,7 @@ fn container_of(block: &ast::BlockExpr) -> Option<SyntaxNode> { } struct ElseBlock<'db> { - exist_else_block: Option<ast::BlockExpr>, + exist_else_branch: Option<ast::ElseBranch>, is_never_block: bool, kind: EarlyKind<'db>, } @@ -245,13 +239,23 @@ struct ElseBlock<'db> { impl<'db> ElseBlock<'db> { fn new( sema: &Semantics<'db, RootDatabase>, - exist_else_block: Option<ast::BlockExpr>, + exist_else_branch: Option<ast::ElseBranch>, parent_container: &SyntaxNode, ) -> Option<Self> { - let is_never_block = exist_else_block.as_ref().is_some_and(|it| is_never_block(sema, it)); + let is_never_block = + exist_else_branch.as_ref().is_some_and(|it| is_never_else_branch(sema, it)); let kind = EarlyKind::from_node(parent_container, sema)?; - Some(Self { exist_else_block, is_never_block, kind }) + Some(Self { exist_else_branch, is_never_block, kind }) + } + + fn make_else_block_from_exist_branch(&self, make: &SyntaxFactory) -> Option<ast::BlockExpr> { + match self.exist_else_branch.as_ref()? { + ast::ElseBranch::Block(block_expr) => Some(block_expr.reset_indent()), + ast::ElseBranch::IfExpr(if_expr) => { + Some(make.block_expr(None, Some(if_expr.reset_indent().indent(1.into()).into()))) + } + } } fn make_early_block( @@ -259,15 +263,15 @@ impl<'db> ElseBlock<'db> { sema: &Semantics<'_, RootDatabase>, make: &SyntaxFactory, ) -> ast::BlockExpr { - let Some(block_expr) = self.exist_else_block else { + let Some(block_expr) = self.make_else_block_from_exist_branch(make) else { return make.tail_only_block_expr(self.kind.make_early_expr(sema, make, None)); }; if self.is_never_block { - return block_expr.reset_indent(); + return block_expr; } - let (editor, block_expr) = SyntaxEditor::with_ast_node(&block_expr.reset_indent()); + let (editor, block_expr) = SyntaxEditor::with_ast_node(&block_expr); let make = editor.make(); let last_stmt = block_expr.statements().last().map(|it| it.syntax().clone()); @@ -284,8 +288,8 @@ impl<'db> ElseBlock<'db> { editor.replace(tail_expr.syntax(), early_expr.syntax()); } else { let last_stmt = match block_expr.tail_expr() { - Some(expr) => make.expr_stmt(expr).syntax().clone(), - None => last_element.clone(), + Some(expr) if !expr.is_block_like() => make.expr_stmt(expr).syntax().clone(), + _ => last_element.clone(), }; let whitespace = make.whitespace(&whitespace.map_or(String::new(), |it| it.to_string())); @@ -401,6 +405,27 @@ fn is_early_block(then_block: &ast::StmtList) -> bool { || then_block.statements().filter_map(into_expr).any(is_early_expr) } +fn is_never_else_branch(sema: &Semantics<'_, RootDatabase>, it: &ast::ElseBranch) -> bool { + match it { + ast::ElseBranch::Block(block_expr) => is_never_block(sema, block_expr), + ast::ElseBranch::IfExpr(if_expr) => { + let mut if_exprs = + std::iter::successors(Some(if_expr.clone()), |it| match it.else_branch()? { + ast::ElseBranch::Block(_) => None, + ast::ElseBranch::IfExpr(if_expr) => Some(if_expr), + }); + if_exprs.all(|it| { + let else_is_never = match it.else_branch() { + None => false, + Some(ast::ElseBranch::IfExpr(_)) => true, + Some(ast::ElseBranch::Block(block)) => is_never_block(sema, &block), + }; + else_is_never && it.then_branch().is_some_and(|it| is_never_block(sema, &it)) + }) + } + } +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; @@ -1025,6 +1050,42 @@ fn main() { } #[test] + fn convert_inside_loop_with_else_if() { + check_assist( + convert_to_guarded_return, + r#" +fn main() { + loop { + if$0 guard() { + foo(); + bar(); + } else if cond() { + break; + } else { + return + } + } +} +"#, + r#" +fn main() { + loop { + if !guard() { + if cond() { + break; + } else { + return + } + } + foo(); + bar(); + } +} +"#, + ); + } + + #[test] fn convert_let_inside_loop() { check_assist( convert_to_guarded_return, @@ -1055,6 +1116,7 @@ fn main() { check_assist( convert_to_guarded_return, r#" +//- minicore: iterator fn main() { for n in ns { if$0 let Some(n) = n { @@ -1107,6 +1169,7 @@ fn main() { check_assist( convert_to_guarded_return, r#" +//- minicore: iterator fn main() { for n in ns { if$0 let Some(n) = n { @@ -1134,6 +1197,109 @@ fn main() { } #[test] + fn convert_let_inside_for_with_else_if() { + check_assist( + convert_to_guarded_return, + r#" +//- minicore: iterator +fn main() { + for n in ns { + if$0 let Some(n) = n { + foo(n); + bar(); + } else if cond() { + return + } else { + baz() + } + } +} +"#, + r#" +fn main() { + for n in ns { + let Some(n) = n else { + if cond() { + return + } else { + baz() + } + continue + }; + foo(n); + bar(); + } +} +"#, + ); + + check_assist( + convert_to_guarded_return, + r#" +//- minicore: iterator +fn main() { + for n in ns { + if$0 let Some(n) = n { + foo(n); + bar(); + } else if cond() { + return + } + } +} +"#, + r#" +fn main() { + for n in ns { + let Some(n) = n else { + if cond() { + return + } + continue + }; + foo(n); + bar(); + } +} +"#, + ); + + check_assist( + convert_to_guarded_return, + r#" +//- minicore: iterator +fn main() { + for n in ns { + if$0 let Some(n) = n { + foo(n); + bar(); + } else if cond() { + return + } else { + break + } + } +} +"#, + r#" +fn main() { + for n in ns { + let Some(n) = n else { + if cond() { + return + } else { + break + } + }; + foo(n); + bar(); + } +} +"#, + ); + } + + #[test] fn convert_let_stmt_inside_fn() { check_assist( convert_to_guarded_return, @@ -1415,7 +1581,7 @@ fn main() { } #[test] - fn ignore_else_if() { + fn ignore_on_else_if() { check_assist_not_applicable( convert_to_guarded_return, r#" diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index ac02cc9e43..e8f5c9537d 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -541,9 +541,10 @@ pub fn block_expr( quote! { BlockExpr { StmtList { - ['{'] "\n" - #(" " #stmts "\n")* - #(" " #tail_expr "\n")* + ['{'] + #("\n " #stmts)* + #("\n " #tail_expr)* + "\n" ['}'] } } |