From 751ca9ec0db3e027ad77cdfe93a46e0625092744 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 28 May 2025 11:12:28 +0200 Subject: [PATCH] feat: Desugar assist for `let pat = expr?;` -> `let else` --- .../src/handlers/desugar_try_expr.rs | 281 ++++++++++++++++++ .../handlers/replace_try_expr_with_match.rs | 148 --------- crates/ide-assists/src/lib.rs | 4 +- crates/ide-assists/src/tests/generated.rs | 62 ++-- .../src/ast/syntax_factory/constructors.rs | 12 + 5 files changed, 336 insertions(+), 171 deletions(-) create mode 100644 crates/ide-assists/src/handlers/desugar_try_expr.rs delete mode 100644 crates/ide-assists/src/handlers/replace_try_expr_with_match.rs diff --git a/crates/ide-assists/src/handlers/desugar_try_expr.rs b/crates/ide-assists/src/handlers/desugar_try_expr.rs new file mode 100644 index 0000000000..efadde9e36 --- /dev/null +++ b/crates/ide-assists/src/handlers/desugar_try_expr.rs @@ -0,0 +1,281 @@ +use std::iter; + +use ide_db::{ + assists::{AssistId, ExprFillDefaultMode}, + ty_filter::TryEnum, +}; +use syntax::{ + AstNode, T, + ast::{ + self, + edit::{AstNodeEdit, IndentLevel}, + make, + syntax_factory::SyntaxFactory, + }, +}; + +use crate::assist_context::{AssistContext, Assists}; + +// Assist: desugar_try_expr_match +// +// Replaces a `try` expression with a `match` expression. +// +// ``` +// # //- minicore: try, option +// fn handle() { +// let pat = Some(true)$0?; +// } +// ``` +// -> +// ``` +// fn handle() { +// let pat = match Some(true) { +// Some(it) => it, +// None => return None, +// }; +// } +// ``` + +// Assist: desugar_try_expr_let_else +// +// Replaces a `try` expression with a `let else` statement. +// +// ``` +// # //- minicore: try, option +// fn handle() { +// let pat = Some(true)$0?; +// } +// ``` +// -> +// ``` +// fn handle() { +// let Some(pat) = Some(true) else { +// return None; +// }; +// } +// ``` +pub(crate) fn desugar_try_expr(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let question_tok = ctx.find_token_syntax_at_offset(T![?])?; + let try_expr = question_tok.parent().and_then(ast::TryExpr::cast)?; + + let expr = try_expr.expr()?; + let expr_type_info = ctx.sema.type_of_expr(&expr)?; + + let try_enum = TryEnum::from_ty(&ctx.sema, &expr_type_info.original)?; + + let target = try_expr.syntax().text_range(); + acc.add( + AssistId::refactor_rewrite("desugar_try_expr_match"), + "Replace try expression with match", + target, + |edit| { + let sad_pat = match try_enum { + TryEnum::Option => make::path_pat(make::ext::ident_path("None")), + TryEnum::Result => make::tuple_struct_pat( + make::ext::ident_path("Err"), + iter::once(make::path_pat(make::ext::ident_path("err"))), + ) + .into(), + }; + let sad_expr = match try_enum { + TryEnum::Option => { + make::expr_return(Some(make::expr_path(make::ext::ident_path("None")))) + } + TryEnum::Result => make::expr_return(Some( + make::expr_call( + make::expr_path(make::ext::ident_path("Err")), + make::arg_list(iter::once(make::expr_path(make::ext::ident_path("err")))), + ) + .into(), + )), + }; + + let happy_arm = make::match_arm( + try_enum.happy_pattern(make::ident_pat(false, false, make::name("it")).into()), + None, + make::expr_path(make::ext::ident_path("it")), + ); + let sad_arm = make::match_arm(sad_pat, None, sad_expr); + + let match_arm_list = make::match_arm_list([happy_arm, sad_arm]); + + let expr_match = make::expr_match(expr.clone(), match_arm_list) + .indent(IndentLevel::from_node(try_expr.syntax())); + + edit.replace_ast::(try_expr.clone().into(), expr_match.into()); + }, + ); + + if let Some(let_stmt) = try_expr.syntax().parent().and_then(ast::LetStmt::cast) { + if let_stmt.let_else().is_none() { + let pat = let_stmt.pat()?; + acc.add( + AssistId::refactor_rewrite("desugar_try_expr_let_else"), + "Replace try expression with let else", + target, + |builder| { + let make = SyntaxFactory::with_mappings(); + let mut editor = builder.make_editor(let_stmt.syntax()); + + let indent_level = IndentLevel::from_node(let_stmt.syntax()); + let new_let_stmt = make.let_else_stmt( + try_enum.happy_pattern(pat), + let_stmt.ty(), + expr, + make.block_expr( + iter::once( + make.expr_stmt( + make.expr_return(Some(match try_enum { + TryEnum::Option => make.expr_path(make.ident_path("None")), + TryEnum::Result => make + .expr_call( + make.expr_path(make.ident_path("Err")), + make.arg_list(iter::once( + match ctx.config.expr_fill_default { + ExprFillDefaultMode::Todo => make + .expr_macro( + make.ident_path("todo"), + make.token_tree( + syntax::SyntaxKind::L_PAREN, + [], + ), + ) + .into(), + ExprFillDefaultMode::Underscore => { + make.expr_underscore().into() + } + ExprFillDefaultMode::Default => make + .expr_macro( + make.ident_path("todo"), + make.token_tree( + syntax::SyntaxKind::L_PAREN, + [], + ), + ) + .into(), + }, + )), + ) + .into(), + })) + .indent(indent_level + 1) + .into(), + ) + .into(), + ), + None, + ) + .indent(indent_level), + ); + editor.replace(let_stmt.syntax(), new_let_stmt.syntax()); + editor.add_mappings(make.finish_with_mappings()); + builder.add_file_edits(ctx.vfs_file_id(), editor); + }, + ); + } + } + Some(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::tests::{check_assist, check_assist_by_label, check_assist_not_applicable}; + + #[test] + fn test_desugar_try_expr_not_applicable() { + check_assist_not_applicable( + desugar_try_expr, + r#" + fn test() { + let pat: u32 = 25$0; + } + "#, + ); + } + + #[test] + fn test_desugar_try_expr_option() { + check_assist( + desugar_try_expr, + r#" +//- minicore: try, option +fn test() { + let pat = Some(true)$0?; +} + "#, + r#" +fn test() { + let pat = match Some(true) { + Some(it) => it, + None => return None, + }; +} + "#, + ); + } + + #[test] + fn test_desugar_try_expr_result() { + check_assist( + desugar_try_expr, + r#" +//- minicore: try, from, result +fn test() { + let pat = Ok(true)$0?; +} + "#, + r#" +fn test() { + let pat = match Ok(true) { + Ok(it) => it, + Err(err) => return Err(err), + }; +} + "#, + ); + } + + #[test] + fn test_desugar_try_expr_option_let_else() { + check_assist_by_label( + desugar_try_expr, + r#" +//- minicore: try, option +fn test() { + let pat = Some(true)$0?; +} + "#, + r#" +fn test() { + let Some(pat) = Some(true) else { + return None; + }; +} + "#, + "Replace try expression with let else", + ); + } + + #[test] + fn test_desugar_try_expr_result_let_else() { + check_assist_by_label( + desugar_try_expr, + r#" +//- minicore: try, from, result +fn test() { + let pat = Ok(true)$0?; +} + "#, + r#" +fn test() { + let Ok(pat) = Ok(true) else { + return Err(todo!()); + }; +} + "#, + "Replace try expression with let else", + ); + } +} diff --git a/crates/ide-assists/src/handlers/replace_try_expr_with_match.rs b/crates/ide-assists/src/handlers/replace_try_expr_with_match.rs deleted file mode 100644 index c6e864fcfd..0000000000 --- a/crates/ide-assists/src/handlers/replace_try_expr_with_match.rs +++ /dev/null @@ -1,148 +0,0 @@ -use std::iter; - -use ide_db::{assists::AssistId, ty_filter::TryEnum}; -use syntax::{ - AstNode, T, - ast::{ - self, - edit::{AstNodeEdit, IndentLevel}, - make, - }, -}; - -use crate::assist_context::{AssistContext, Assists}; - -// Assist: replace_try_expr_with_match -// -// Replaces a `try` expression with a `match` expression. -// -// ``` -// # //- minicore: try, option -// fn handle() { -// let pat = Some(true)$0?; -// } -// ``` -// -> -// ``` -// fn handle() { -// let pat = match Some(true) { -// Some(it) => it, -// None => return None, -// }; -// } -// ``` -pub(crate) fn replace_try_expr_with_match( - acc: &mut Assists, - ctx: &AssistContext<'_>, -) -> Option<()> { - let qm_kw = ctx.find_token_syntax_at_offset(T![?])?; - let qm_kw_parent = qm_kw.parent().and_then(ast::TryExpr::cast)?; - - let expr = qm_kw_parent.expr()?; - let expr_type_info = ctx.sema.type_of_expr(&expr)?; - - let try_enum = TryEnum::from_ty(&ctx.sema, &expr_type_info.original)?; - - let target = qm_kw_parent.syntax().text_range(); - acc.add( - AssistId::refactor_rewrite("replace_try_expr_with_match"), - "Replace try expression with match", - target, - |edit| { - let sad_pat = match try_enum { - TryEnum::Option => make::path_pat(make::ext::ident_path("None")), - TryEnum::Result => make::tuple_struct_pat( - make::ext::ident_path("Err"), - iter::once(make::path_pat(make::ext::ident_path("err"))), - ) - .into(), - }; - let sad_expr = match try_enum { - TryEnum::Option => { - make::expr_return(Some(make::expr_path(make::ext::ident_path("None")))) - } - TryEnum::Result => make::expr_return(Some( - make::expr_call( - make::expr_path(make::ext::ident_path("Err")), - make::arg_list(iter::once(make::expr_path(make::ext::ident_path("err")))), - ) - .into(), - )), - }; - - let happy_arm = make::match_arm( - try_enum.happy_pattern(make::ident_pat(false, false, make::name("it")).into()), - None, - make::expr_path(make::ext::ident_path("it")), - ); - let sad_arm = make::match_arm(sad_pat, None, sad_expr); - - let match_arm_list = make::match_arm_list([happy_arm, sad_arm]); - - let expr_match = make::expr_match(expr, match_arm_list) - .indent(IndentLevel::from_node(qm_kw_parent.syntax())); - edit.replace_ast::(qm_kw_parent.into(), expr_match.into()); - }, - ) -} - -#[cfg(test)] -mod tests { - use super::*; - - use crate::tests::{check_assist, check_assist_not_applicable}; - - #[test] - fn test_replace_try_expr_with_match_not_applicable() { - check_assist_not_applicable( - replace_try_expr_with_match, - r#" - fn test() { - let pat: u32 = 25$0; - } - "#, - ); - } - - #[test] - fn test_replace_try_expr_with_match_option() { - check_assist( - replace_try_expr_with_match, - r#" -//- minicore: try, option -fn test() { - let pat = Some(true)$0?; -} - "#, - r#" -fn test() { - let pat = match Some(true) { - Some(it) => it, - None => return None, - }; -} - "#, - ); - } - - #[test] - fn test_replace_try_expr_with_match_result() { - check_assist( - replace_try_expr_with_match, - r#" -//- minicore: try, from, result -fn test() { - let pat = Ok(true)$0?; -} - "#, - r#" -fn test() { - let pat = match Ok(true) { - Ok(it) => it, - Err(err) => return Err(err), - }; -} - "#, - ); - } -} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 2395091b6f..c260443203 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -139,6 +139,7 @@ mod handlers { mod destructure_struct_binding; mod destructure_tuple_binding; mod desugar_doc_comment; + mod desugar_try_expr; mod expand_glob_import; mod expand_rest_pattern; mod extract_expressions_from_format_string; @@ -214,7 +215,6 @@ mod handlers { mod replace_named_generic_with_impl; mod replace_qualified_name_with_use; mod replace_string_with_char; - mod replace_try_expr_with_match; mod replace_turbofish_with_explicit_type; mod sort_items; mod split_import; @@ -273,6 +273,7 @@ mod handlers { destructure_struct_binding::destructure_struct_binding, destructure_tuple_binding::destructure_tuple_binding, desugar_doc_comment::desugar_doc_comment, + desugar_try_expr::desugar_try_expr, expand_glob_import::expand_glob_import, expand_glob_import::expand_glob_reexport, expand_rest_pattern::expand_rest_pattern, @@ -354,7 +355,6 @@ mod handlers { replace_method_eager_lazy::replace_with_lazy_method, replace_named_generic_with_impl::replace_named_generic_with_impl, replace_qualified_name_with_use::replace_qualified_name_with_use, - replace_try_expr_with_match::replace_try_expr_with_match, replace_turbofish_with_explicit_type::replace_turbofish_with_explicit_type, sort_items::sort_items, split_import::split_import, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 76134acb36..72f7195cbd 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -929,6 +929,47 @@ comment"] ) } +#[test] +fn doctest_desugar_try_expr_let_else() { + check_doc_test( + "desugar_try_expr_let_else", + r#####" +//- minicore: try, option +fn handle() { + let pat = Some(true)$0?; +} +"#####, + r#####" +fn handle() { + let Some(pat) = Some(true) else { + return None; + }; +} +"#####, + ) +} + +#[test] +fn doctest_desugar_try_expr_match() { + check_doc_test( + "desugar_try_expr_match", + r#####" +//- minicore: try, option +fn handle() { + let pat = Some(true)$0?; +} +"#####, + r#####" +fn handle() { + let pat = match Some(true) { + Some(it) => it, + None => return None, + }; +} +"#####, + ) +} + #[test] fn doctest_expand_glob_import() { check_doc_test( @@ -3096,27 +3137,6 @@ fn main() { ) } -#[test] -fn doctest_replace_try_expr_with_match() { - check_doc_test( - "replace_try_expr_with_match", - r#####" -//- minicore: try, option -fn handle() { - let pat = Some(true)$0?; -} -"#####, - r#####" -fn handle() { - let pat = match Some(true) { - Some(it) => it, - None => return None, - }; -} -"#####, - ) -} - #[test] fn doctest_replace_turbofish_with_explicit_type() { check_doc_test( diff --git a/crates/syntax/src/ast/syntax_factory/constructors.rs b/crates/syntax/src/ast/syntax_factory/constructors.rs index 8dee3964d4..429e51ba36 100644 --- a/crates/syntax/src/ast/syntax_factory/constructors.rs +++ b/crates/syntax/src/ast/syntax_factory/constructors.rs @@ -585,6 +585,18 @@ impl SyntaxFactory { ast } + pub fn expr_underscore(&self) -> ast::UnderscoreExpr { + let ast::Expr::UnderscoreExpr(ast) = make::ext::expr_underscore().clone_for_update() else { + unreachable!() + }; + + if let Some(mut mapping) = self.mappings() { + SyntaxMappingBuilder::new(ast.syntax().clone()).finish(&mut mapping); + } + + ast + } + pub fn expr_if( &self, condition: ast::Expr,