From 971a2718402dcf73c969490f51473b438f328dd9 Mon Sep 17 00:00:00 2001 From: Dezhi Wu Date: Wed, 22 Sep 2021 16:00:09 +0800 Subject: [PATCH 1/5] use `ControlFlow` in "extract function" assist --- .../src/handlers/extract_function.rs | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index deb98ad0836..59f0a786afd 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -1184,7 +1184,18 @@ impl FlowHandler { let action = action.make_result_handler(None); let stmt = make::expr_stmt(action); let block = make::block_expr(iter::once(stmt.into()), None); - let condition = make::condition(call_expr, None); + let controlflow_break_path = make::path_from_text("ControlFlow::Break"); + let tuple_pat = make::tuple_pat(iter::empty()); + let condition = make::condition( + call_expr, + Some( + make::tuple_struct_pat( + controlflow_break_path, + iter::once(tuple_pat.into()), + ) + .into(), + ), + ); make::expr_if(condition, block, None) } FlowHandler::IfOption { action } => { @@ -1326,7 +1337,7 @@ impl Function { .unwrap_or_else(make::ty_placeholder); make::ext::ty_result(fun_ty.make_ty(ctx, module), handler_ty) } - FlowHandler::If { .. } => make::ext::ty_bool(), + FlowHandler::If { .. } => make::ty("ControlFlow<()>"), FlowHandler::IfOption { action } => { let handler_ty = action .expr_ty(ctx) @@ -1461,8 +1472,11 @@ fn make_body( }) } FlowHandler::If { .. } => { - let lit_false = make::expr_literal("false"); - with_tail_expr(block, lit_false.into()) + let controlflow_continue = make::expr_call( + make::expr_path(make::path_from_text("ControlFlow::Continue")), + make::arg_list(iter::once(make::expr_unit())), + ); + with_tail_expr(block, controlflow_continue.into()) } FlowHandler::IfOption { .. } => { let none = make::expr_path(make::ext::ident_path("None")); @@ -1638,7 +1652,10 @@ fn update_external_control_flow(handler: &FlowHandler, syntax: &SyntaxNode) { fn make_rewritten_flow(handler: &FlowHandler, arg_expr: Option) -> Option { let value = match handler { FlowHandler::None | FlowHandler::Try { .. } => return None, - FlowHandler::If { .. } => make::expr_literal("true").into(), + FlowHandler::If { .. } => make::expr_call( + make::expr_path(make::path_from_text("ControlFlow::Break")), + make::arg_list(iter::once(make::expr_unit())), + ), FlowHandler::IfOption { .. } => { let expr = arg_expr.unwrap_or_else(|| make::expr_tuple(Vec::new())); let args = make::arg_list(iter::once(expr)); @@ -3284,18 +3301,18 @@ fn foo() { fn foo() { loop { let mut n = 1; - if fun_name(&mut n) { + if let ControlFlow::Break(()) = fun_name(&mut n) { break; } let h = 1 + n; } } -fn $0fun_name(n: &mut i32) -> bool { +fn $0fun_name(n: &mut i32) -> ControlFlow<()> { let m = *n + 1; - return true; + return ControlFlow::Break(()); *n += m; - false + ControlFlow::Continue(()) } "#, ); @@ -3321,19 +3338,19 @@ fn foo() { fn foo() { loop { let mut n = 1; - if fun_name(n) { + if let ControlFlow::Break(()) = fun_name(n) { break; } let h = 1; } } -fn $0fun_name(n: i32) -> bool { +fn $0fun_name(n: i32) -> ControlFlow<()> { let m = n + 1; if m == 42 { - return true; + return ControlFlow::Break(()); } - false + ControlFlow::Continue(()) } "#, ); From f888e85f79627cf05eca66b01aaf09c9d3906df5 Mon Sep 17 00:00:00 2001 From: Dezhi Wu Date: Wed, 22 Sep 2021 18:48:47 +0800 Subject: [PATCH 2/5] use `ControlFlow::Break(_)` pattern --- crates/ide_assists/src/handlers/extract_function.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 59f0a786afd..dfedc4a4870 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -1185,13 +1185,12 @@ impl FlowHandler { let stmt = make::expr_stmt(action); let block = make::block_expr(iter::once(stmt.into()), None); let controlflow_break_path = make::path_from_text("ControlFlow::Break"); - let tuple_pat = make::tuple_pat(iter::empty()); let condition = make::condition( call_expr, Some( make::tuple_struct_pat( controlflow_break_path, - iter::once(tuple_pat.into()), + iter::once(make::wildcard_pat().into()), ) .into(), ), @@ -3301,7 +3300,7 @@ fn foo() { fn foo() { loop { let mut n = 1; - if let ControlFlow::Break(()) = fun_name(&mut n) { + if let ControlFlow::Break(_) = fun_name(&mut n) { break; } let h = 1 + n; @@ -3338,7 +3337,7 @@ fn foo() { fn foo() { loop { let mut n = 1; - if let ControlFlow::Break(()) = fun_name(n) { + if let ControlFlow::Break(_) = fun_name(n) { break; } let h = 1; From 5818358bbfffceee842055a5c857b3437d59ee88 Mon Sep 17 00:00:00 2001 From: Dezhi Wu Date: Wed, 13 Oct 2021 09:01:30 +0800 Subject: [PATCH 3/5] import `ControlFlow` to the module --- .../src/handlers/extract_function.rs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index dfedc4a4870..0107e6a6132 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -5,6 +5,7 @@ use either::Either; use hir::{HirDisplay, InFile, Local, Semantics, TypeInfo}; use ide_db::{ defs::{Definition, NameRefClass}, + helpers::insert_use::{insert_use, ImportScope}, helpers::node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr}, search::{FileReference, ReferenceCategory, SearchScope}, RootDatabase, @@ -86,6 +87,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option let target_range = body.text_range(); + let scope = ImportScope::find_insert_use_container_with_macros(&node, &ctx.sema)?; + acc.add( AssistId("extract_function", crate::AssistKind::RefactorExtract), "Extract into function", @@ -118,10 +121,25 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option let fn_def = format_function(ctx, module, &fun, old_indent, new_indent); let insert_offset = insert_after.text_range().end(); + + if fn_def.contains("ControlFlow") { + let scope = match scope { + ImportScope::File(it) => ImportScope::File(builder.make_mut(it)), + ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)), + ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)), + }; + + insert_use( + &scope, + make::path_from_text("std::ops::ControlFlow"), + &ctx.config.insert_use, + ); + } + match ctx.config.snippet_cap { Some(cap) => builder.insert_snippet(cap, insert_offset, fn_def), None => builder.insert(insert_offset, fn_def), - } + }; }, ) } @@ -3297,6 +3315,8 @@ fn foo() { } "#, r#" +use std::ops::ControlFlow; + fn foo() { loop { let mut n = 1; @@ -3334,6 +3354,8 @@ fn foo() { } "#, r#" +use std::ops::ControlFlow; + fn foo() { loop { let mut n = 1; From 93ae993ec495233850720126deb0f6b6319645d9 Mon Sep 17 00:00:00 2001 From: Dezhi Wu Date: Wed, 13 Oct 2021 21:19:41 +0800 Subject: [PATCH 4/5] resolve `ControlFlow` ourself instead of hard coding. --- .../src/handlers/extract_function.rs | 37 ++++++++++++++----- crates/ide_db/src/helpers/famous_defs.rs | 4 ++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 0107e6a6132..8ae0a4a10ad 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -2,11 +2,17 @@ use std::{hash::BuildHasherDefault, iter}; use ast::make; use either::Either; -use hir::{HirDisplay, InFile, Local, Semantics, TypeInfo}; +use hir::{HirDisplay, InFile, Local, ModuleDef, Semantics, TypeInfo}; use ide_db::{ defs::{Definition, NameRefClass}, - helpers::insert_use::{insert_use, ImportScope}, - helpers::node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr}, + helpers::{ + insert_use::{insert_use, ImportScope}, + FamousDefs, + }, + helpers::{ + mod_path_to_ast, + node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr}, + }, search::{FileReference, ReferenceCategory, SearchScope}, RootDatabase, }; @@ -129,11 +135,20 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)), }; - insert_use( - &scope, - make::path_from_text("std::ops::ControlFlow"), - &ctx.config.insert_use, - ); + let control_flow_enum = + FamousDefs(&ctx.sema, Some(module.krate())).core_ops_ControlFlow(); + + if let Some(control_flow_enum) = control_flow_enum { + let mod_path = module.find_use_path_prefixed( + ctx.sema.db, + ModuleDef::from(control_flow_enum), + ctx.config.insert_use.prefix_kind, + ); + + if let Some(mod_path) = mod_path { + insert_use(&scope, mod_path_to_ast(&mod_path), &ctx.config.insert_use); + } + } } match ctx.config.snippet_cap { @@ -3304,6 +3319,7 @@ fn foo() { check_assist( extract_function, r#" +//- minicore: try fn foo() { loop { let mut n = 1; @@ -3315,7 +3331,7 @@ fn foo() { } "#, r#" -use std::ops::ControlFlow; +use core::ops::ControlFlow; fn foo() { loop { @@ -3342,6 +3358,7 @@ fn $0fun_name(n: &mut i32) -> ControlFlow<()> { check_assist( extract_function, r#" +//- minicore: try fn foo() { loop { let mut n = 1; @@ -3354,7 +3371,7 @@ fn foo() { } "#, r#" -use std::ops::ControlFlow; +use core::ops::ControlFlow; fn foo() { loop { diff --git a/crates/ide_db/src/helpers/famous_defs.rs b/crates/ide_db/src/helpers/famous_defs.rs index 08bd8e0cba6..18524986e2d 100644 --- a/crates/ide_db/src/helpers/famous_defs.rs +++ b/crates/ide_db/src/helpers/famous_defs.rs @@ -68,6 +68,10 @@ impl FamousDefs<'_, '_> { self.find_trait("core:ops:Deref") } + pub fn core_ops_ControlFlow(&self) -> Option { + self.find_enum("core:ops:ControlFlow") + } + pub fn alloc(&self) -> Option { self.find_crate("alloc") } From 214e7cc69d9f70c084bcc4950259c0ca4cec2764 Mon Sep 17 00:00:00 2001 From: Dezhi Wu Date: Wed, 13 Oct 2021 21:24:17 +0800 Subject: [PATCH 5/5] merge use statement --- crates/ide_assists/src/handlers/extract_function.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 8ae0a4a10ad..c97d75da939 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -7,11 +7,9 @@ use ide_db::{ defs::{Definition, NameRefClass}, helpers::{ insert_use::{insert_use, ImportScope}, - FamousDefs, - }, - helpers::{ mod_path_to_ast, node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr}, + FamousDefs, }, search::{FileReference, ReferenceCategory, SearchScope}, RootDatabase,