Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #20758 from A4-Tacks/guarded-if-let-else
Add else-block support for convert_to_guarded_return
| -rw-r--r-- | crates/ide-assists/src/handlers/convert_to_guarded_return.rs | 89 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils.rs | 16 |
2 files changed, 90 insertions, 15 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 7bbe13bedd..ae13f83fbc 100644 --- a/crates/ide-assists/src/handlers/convert_to_guarded_return.rs +++ b/crates/ide-assists/src/handlers/convert_to_guarded_return.rs @@ -17,7 +17,7 @@ use syntax::{ use crate::{ AssistId, assist_context::{AssistContext, Assists}, - utils::invert_boolean_expression_legacy, + utils::{invert_boolean_expression_legacy, is_never_block}, }; // Assist: convert_to_guarded_return @@ -54,9 +54,13 @@ fn if_expr_to_guarded_return( acc: &mut Assists, ctx: &AssistContext<'_>, ) -> Option<()> { - if if_expr.else_branch().is_some() { - return None; - } + let else_block = match if_expr.else_branch() { + Some(ast::ElseBranch::Block(block_expr)) if is_never_block(&ctx.sema, &block_expr) => { + Some(block_expr) + } + Some(_) => return None, + _ => None, + }; let cond = if_expr.condition()?; @@ -96,7 +100,11 @@ fn if_expr_to_guarded_return( let parent_container = parent_block.syntax().parent()?; - let early_expression: ast::Expr = early_expression(parent_container, &ctx.sema)?; + let early_expression = else_block + .or_else(|| { + early_expression(parent_container, &ctx.sema).map(ast::make::tail_only_block_expr) + })? + .reset_indent(); then_block.syntax().first_child_or_token().map(|t| t.kind() == T!['{'])?; @@ -123,21 +131,14 @@ fn if_expr_to_guarded_return( && let (Some(pat), Some(expr)) = (let_expr.pat(), let_expr.expr()) { // If-let. - let let_else_stmt = make::let_else_stmt( - pat, - None, - expr, - ast::make::tail_only_block_expr(early_expression.clone()), - ); + let let_else_stmt = + make::let_else_stmt(pat, None, expr, early_expression.clone()); let let_else_stmt = let_else_stmt.indent(if_indent_level); let_else_stmt.syntax().clone() } else { // If. let new_expr = { - let then_branch = make::block_expr( - once(make::expr_stmt(early_expression.clone()).into()), - None, - ); + let then_branch = clean_stmt_block(&early_expression); let cond = invert_boolean_expression_legacy(expr); make::expr_if(cond, then_branch, None).indent(if_indent_level) }; @@ -272,6 +273,17 @@ fn flat_let_chain(mut expr: ast::Expr) -> Vec<ast::Expr> { chains } +fn clean_stmt_block(block: &ast::BlockExpr) -> ast::BlockExpr { + if block.statements().next().is_none() + && let Some(tail_expr) = block.tail_expr() + && block.modifier().is_none() + { + make::block_expr(once(make::expr_stmt(tail_expr).into()), None) + } else { + block.clone() + } +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; @@ -422,6 +434,53 @@ fn main() { } #[test] + fn convert_if_let_has_never_type_else_block() { + check_assist( + convert_to_guarded_return, + r#" +fn main() { + if$0 let Ok(x) = Err(92) { + foo(x); + } else { + // needless comment + return; + } +} +"#, + r#" +fn main() { + let Ok(x) = Err(92) else { + // needless comment + return; + }; + foo(x); +} +"#, + ); + + check_assist( + convert_to_guarded_return, + r#" +fn main() { + if$0 let Ok(x) = Err(92) { + foo(x); + } else { + return + } +} +"#, + r#" +fn main() { + let Ok(x) = Err(92) else { + return + }; + foo(x); +} +"#, + ); + } + + #[test] fn convert_if_let_result_inside_let() { check_assist( convert_to_guarded_return, diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 6ed4467aa3..5a3c5a39da 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -1165,3 +1165,19 @@ pub fn is_body_const(sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr) -> bo }); is_const } + +// FIXME: #20460 When hir-ty can analyze the `never` statement at the end of block, remove it +pub(crate) fn is_never_block( + sema: &Semantics<'_, RootDatabase>, + block_expr: &ast::BlockExpr, +) -> bool { + if let Some(tail_expr) = block_expr.tail_expr() { + sema.type_of_expr(&tail_expr).is_some_and(|ty| ty.original.is_never()) + } else if let Some(ast::Stmt::ExprStmt(expr_stmt)) = block_expr.statements().last() + && let Some(expr) = expr_stmt.expr() + { + sema.type_of_expr(&expr).is_some_and(|ty| ty.original.is_never()) + } else { + false + } +} |