From 317fc650d5c1267a8c192041efe6b591d900808f Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Thu, 16 Apr 2020 23:53:25 +0200 Subject: Make add_function generate functions in other modules via qualified path --- crates/ra_assists/src/handlers/add_function.rs | 219 ++++++++++++++++++++++--- 1 file changed, 195 insertions(+), 24 deletions(-) (limited to 'crates/ra_assists/src/handlers') diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index ad4ab66ed..a1261fe15 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -4,7 +4,7 @@ use ra_syntax::{ }; use crate::{Assist, AssistCtx, AssistId}; -use ast::{edit::IndentLevel, ArgListOwner, CallExpr, Expr}; +use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner}; use hir::HirDisplay; use rustc_hash::{FxHashMap, FxHashSet}; @@ -16,7 +16,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; // struct Baz; // fn baz() -> Baz { Baz } // fn foo() { -// bar<|>("", baz()); +// bar<|>("", baz()); // } // // ``` @@ -25,7 +25,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; // struct Baz; // fn baz() -> Baz { Baz } // fn foo() { -// bar("", baz()); +// bar("", baz()); // } // // fn bar(arg: &str, baz: Baz) { @@ -38,16 +38,24 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option { let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let path = path_expr.path()?; - if path.qualifier().is_some() { - return None; - } - if ctx.sema.resolve_path(&path).is_some() { // The function call already resolves, no need to add a function return None; } - let function_builder = FunctionBuilder::from_call(&ctx, &call)?; + let target_module = if let Some(qualifier) = path.qualifier() { + if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(resolved))) = + ctx.sema.resolve_path(&qualifier) + { + Some(resolved.definition_source(ctx.sema.db).value) + } else { + return None; + } + } else { + None + }; + + let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?; ctx.add_assist(AssistId("add_function"), "Add function", |edit| { edit.target(call.syntax().text_range()); @@ -66,26 +74,54 @@ struct FunctionTemplate { } struct FunctionBuilder { - append_fn_at: SyntaxNode, + target: GeneratedFunctionTarget, fn_name: ast::Name, type_params: Option, params: ast::ParamList, } impl FunctionBuilder { - fn from_call(ctx: &AssistCtx, call: &ast::CallExpr) -> Option { - let append_fn_at = next_space_for_fn(&call)?; - let fn_name = fn_name(&call)?; + /// Prepares a generated function that matches `call` in `generate_in` + /// (or as close to `call` as possible, if `generate_in` is `None`) + fn from_call( + ctx: &AssistCtx, + call: &ast::CallExpr, + path: &ast::Path, + generate_in: Option, + ) -> Option { + let target = if let Some(generate_in_module) = generate_in { + next_space_for_fn_in_module(generate_in_module)? + } else { + next_space_for_fn_after_call_site(&call)? + }; + let fn_name = fn_name(&path)?; let (type_params, params) = fn_args(ctx, &call)?; - Some(Self { append_fn_at, fn_name, type_params, params }) + Some(Self { target, fn_name, type_params, params }) } fn render(self) -> Option { let placeholder_expr = ast::make::expr_todo(); let fn_body = ast::make::block_expr(vec![], Some(placeholder_expr)); let fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body); - let fn_def = ast::make::add_newlines(2, fn_def); - let fn_def = IndentLevel::from_node(&self.append_fn_at).increase_indent(fn_def); - let insert_offset = self.append_fn_at.text_range().end(); + + let (fn_def, insert_offset) = match self.target { + GeneratedFunctionTarget::BehindItem(it) => { + let with_leading_blank_line = ast::make::add_leading_newlines(2, fn_def); + let indented = IndentLevel::from_node(&it).increase_indent(with_leading_blank_line); + (indented, it.text_range().end()) + } + GeneratedFunctionTarget::InEmptyItemList(it) => { + let with_leading_newline = ast::make::add_leading_newlines(1, fn_def); + let indent = IndentLevel::from_node(it.syntax()).indented(); + let mut indented = indent.increase_indent(with_leading_newline); + if !item_list_has_whitespace(&it) { + // In this case we want to make sure there's a newline between the closing + // function brace and the closing module brace (so it doesn't end in `}}`). + indented = ast::make::add_trailing_newlines(1, indented); + } + (indented, it.syntax().text_range().start() + TextUnit::from_usize(1)) + } + }; + let cursor_offset_from_fn_start = fn_def .syntax() .descendants() @@ -98,15 +134,25 @@ impl FunctionBuilder { } } -fn fn_name(call: &CallExpr) -> Option { - let name = call.expr()?.syntax().to_string(); +/// Returns true if the given ItemList contains whitespace. +fn item_list_has_whitespace(it: &ast::ItemList) -> bool { + it.syntax().descendants_with_tokens().find(|it| it.kind() == SyntaxKind::WHITESPACE).is_some() +} + +enum GeneratedFunctionTarget { + BehindItem(SyntaxNode), + InEmptyItemList(ast::ItemList), +} + +fn fn_name(call: &ast::Path) -> Option { + let name = call.segment()?.syntax().to_string(); Some(ast::make::name(&name)) } /// Computes the type variables and arguments required for the generated function fn fn_args( ctx: &AssistCtx, - call: &CallExpr, + call: &ast::CallExpr, ) -> Option<(Option, ast::ParamList)> { let mut arg_names = Vec::new(); let mut arg_types = Vec::new(); @@ -158,9 +204,9 @@ fn deduplicate_arg_names(arg_names: &mut Vec) { } } -fn fn_arg_name(fn_arg: &Expr) -> Option { +fn fn_arg_name(fn_arg: &ast::Expr) -> Option { match fn_arg { - Expr::CastExpr(cast_expr) => fn_arg_name(&cast_expr.expr()?), + ast::Expr::CastExpr(cast_expr) => fn_arg_name(&cast_expr.expr()?), _ => Some( fn_arg .syntax() @@ -172,7 +218,7 @@ fn fn_arg_name(fn_arg: &Expr) -> Option { } } -fn fn_arg_type(ctx: &AssistCtx, fn_arg: &Expr) -> Option { +fn fn_arg_type(ctx: &AssistCtx, fn_arg: &ast::Expr) -> Option { let ty = ctx.sema.type_of_expr(fn_arg)?; if ty.is_unknown() { return None; @@ -184,7 +230,7 @@ fn fn_arg_type(ctx: &AssistCtx, fn_arg: &Expr) -> Option { /// directly after the current block /// We want to write the generated function directly after /// fns, impls or macro calls, but inside mods -fn next_space_for_fn(expr: &CallExpr) -> Option { +fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option { let mut ancestors = expr.syntax().ancestors().peekable(); let mut last_ancestor: Option = None; while let Some(next_ancestor) = ancestors.next() { @@ -201,7 +247,26 @@ fn next_space_for_fn(expr: &CallExpr) -> Option { } last_ancestor = Some(next_ancestor); } - last_ancestor + last_ancestor.map(GeneratedFunctionTarget::BehindItem) +} + +fn next_space_for_fn_in_module(module: hir::ModuleSource) -> Option { + match module { + hir::ModuleSource::SourceFile(it) => { + if let Some(last_item) = it.items().last() { + Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) + } else { + Some(GeneratedFunctionTarget::BehindItem(it.syntax().clone())) + } + } + hir::ModuleSource::Module(it) => { + if let Some(last_item) = it.item_list().and_then(|it| it.items().last()) { + Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) + } else { + it.item_list().map(GeneratedFunctionTarget::InEmptyItemList) + } + } + } } #[cfg(test)] @@ -713,6 +778,112 @@ fn bar(baz_1: Baz, baz_2: Baz, arg_1: &str, arg_2: &str) { ) } + #[test] + fn add_function_in_module() { + check_assist( + add_function, + r" +mod bar {} + +fn foo() { + bar::my_fn<|>() +} +", + r" +mod bar { + fn my_fn() { + <|>todo!() + } +} + +fn foo() { + bar::my_fn() +} +", + ); + check_assist( + add_function, + r" +mod bar { +} + +fn foo() { + bar::my_fn<|>() +} +", + r" +mod bar { + fn my_fn() { + <|>todo!() + } +} + +fn foo() { + bar::my_fn() +} +", + ) + } + + #[test] + fn add_function_in_module_containing_other_items() { + check_assist( + add_function, + r" +mod bar { + fn something_else() {} +} + +fn foo() { + bar::my_fn<|>() +} +", + r" +mod bar { + fn something_else() {} + + fn my_fn() { + <|>todo!() + } +} + +fn foo() { + bar::my_fn() +} +", + ) + } + + #[test] + fn add_function_in_nested_module() { + check_assist( + add_function, + r" +mod bar { + mod baz { + } +} + +fn foo() { + bar::baz::my_fn<|>() +} +", + r" +mod bar { + mod baz { + fn my_fn() { + <|>todo!() + } + } +} + +fn foo() { + bar::baz::my_fn() +} +", + ) + } + #[test] fn add_function_not_applicable_if_function_already_exists() { check_assist_not_applicable( -- cgit v1.2.3 From ba8faf3efc7a3a373571f98569699bbe684779b3 Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Sat, 18 Apr 2020 16:12:21 +0200 Subject: Add target file information to AssistAction --- crates/ra_assists/src/handlers/add_function.rs | 71 +++++++++++++++++++++----- 1 file changed, 57 insertions(+), 14 deletions(-) (limited to 'crates/ra_assists/src/handlers') diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index a1261fe15..b65ded871 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -3,7 +3,7 @@ use ra_syntax::{ SyntaxKind, SyntaxNode, TextUnit, }; -use crate::{Assist, AssistCtx, AssistId}; +use crate::{Assist, AssistCtx, AssistFile, AssistId}; use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner}; use hir::HirDisplay; use rustc_hash::{FxHashMap, FxHashSet}; @@ -44,10 +44,10 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option { } let target_module = if let Some(qualifier) = path.qualifier() { - if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(resolved))) = + if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) = ctx.sema.resolve_path(&qualifier) { - Some(resolved.definition_source(ctx.sema.db).value) + Some(module.definition_source(ctx.sema.db)) } else { return None; } @@ -61,6 +61,7 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option { edit.target(call.syntax().text_range()); if let Some(function_template) = function_builder.render() { + edit.set_file(function_template.file); edit.set_cursor(function_template.cursor_offset); edit.insert(function_template.insert_offset, function_template.fn_def.to_string()); } @@ -71,6 +72,7 @@ struct FunctionTemplate { insert_offset: TextUnit, cursor_offset: TextUnit, fn_def: ast::SourceFile, + file: AssistFile, } struct FunctionBuilder { @@ -78,6 +80,7 @@ struct FunctionBuilder { fn_name: ast::Name, type_params: Option, params: ast::ParamList, + file: AssistFile, } impl FunctionBuilder { @@ -87,16 +90,19 @@ impl FunctionBuilder { ctx: &AssistCtx, call: &ast::CallExpr, path: &ast::Path, - generate_in: Option, + generate_in: Option>, ) -> Option { + let mut file = AssistFile::default(); let target = if let Some(generate_in_module) = generate_in { - next_space_for_fn_in_module(generate_in_module)? + let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, generate_in_module)?; + file = in_file; + target } else { next_space_for_fn_after_call_site(&call)? }; let fn_name = fn_name(&path)?; let (type_params, params) = fn_args(ctx, &call)?; - Some(Self { target, fn_name, type_params, params }) + Some(Self { target, fn_name, type_params, params, file }) } fn render(self) -> Option { let placeholder_expr = ast::make::expr_todo(); @@ -130,7 +136,7 @@ impl FunctionBuilder { .text_range() .start(); let cursor_offset = insert_offset + cursor_offset_from_fn_start; - Some(FunctionTemplate { insert_offset, cursor_offset, fn_def }) + Some(FunctionTemplate { insert_offset, cursor_offset, fn_def, file: self.file }) } } @@ -250,23 +256,29 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option Option { - match module { +fn next_space_for_fn_in_module( + db: &dyn hir::db::AstDatabase, + module: hir::InFile, +) -> Option<(AssistFile, GeneratedFunctionTarget)> { + let file = module.file_id.original_file(db); + let assist_file = AssistFile::TargetFile(file); + let assist_item = match module.value { hir::ModuleSource::SourceFile(it) => { if let Some(last_item) = it.items().last() { - Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) + GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()) } else { - Some(GeneratedFunctionTarget::BehindItem(it.syntax().clone())) + GeneratedFunctionTarget::BehindItem(it.syntax().clone()) } } hir::ModuleSource::Module(it) => { if let Some(last_item) = it.item_list().and_then(|it| it.items().last()) { - Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) + GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()) } else { - it.item_list().map(GeneratedFunctionTarget::InEmptyItemList) + GeneratedFunctionTarget::InEmptyItemList(it.item_list()?) } } - } + }; + Some((assist_file, assist_item)) } #[cfg(test)] @@ -884,6 +896,37 @@ fn foo() { ) } + #[test] + fn add_function_in_another_file() { + check_assist( + add_function, + r" +//- /main.rs +mod foo; + +fn main() { + foo::bar<|>() +} + +//- /foo.rs + +", + r" +//- /main.rs +mod foo; + +fn main() { + foo::bar() +} + +//- /foo.rs +fn bar() { + <|>todo!() +} +", + ) + } + #[test] fn add_function_not_applicable_if_function_already_exists() { check_assist_not_applicable( -- cgit v1.2.3 From 74780a15f65916d08942eb53c43b8e8c0b62cb48 Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Mon, 20 Apr 2020 18:02:36 +0200 Subject: Jump to sourceChanges in other files --- crates/ra_assists/src/handlers/add_function.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'crates/ra_assists/src/handlers') diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index b65ded871..9bd46f5dc 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -907,23 +907,14 @@ mod foo; fn main() { foo::bar<|>() } - //- /foo.rs - ", r" -//- /main.rs -mod foo; -fn main() { - foo::bar() -} -//- /foo.rs -fn bar() { +pub(crate) fn bar() { <|>todo!() -} -", +}", ) } -- cgit v1.2.3 From f2f882bc44a85eb13276a8fbda7533d94e92e3af Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Mon, 20 Apr 2020 18:36:12 +0200 Subject: Add `pub(crate)` to functions generated in other module --- crates/ra_assists/src/handlers/add_function.rs | 69 +++++++++----------------- 1 file changed, 23 insertions(+), 46 deletions(-) (limited to 'crates/ra_assists/src/handlers') diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index 9bd46f5dc..f185cffdb 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -81,6 +81,7 @@ struct FunctionBuilder { type_params: Option, params: ast::ParamList, file: AssistFile, + needs_pub: bool, } impl FunctionBuilder { @@ -90,11 +91,12 @@ impl FunctionBuilder { ctx: &AssistCtx, call: &ast::CallExpr, path: &ast::Path, - generate_in: Option>, + target_module: Option>, ) -> Option { + let needs_pub = target_module.is_some(); let mut file = AssistFile::default(); - let target = if let Some(generate_in_module) = generate_in { - let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, generate_in_module)?; + let target = if let Some(target_module) = target_module { + let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?; file = in_file; target } else { @@ -102,12 +104,16 @@ impl FunctionBuilder { }; let fn_name = fn_name(&path)?; let (type_params, params) = fn_args(ctx, &call)?; - Some(Self { target, fn_name, type_params, params, file }) + Some(Self { target, fn_name, type_params, params, file, needs_pub }) } + fn render(self) -> Option { let placeholder_expr = ast::make::expr_todo(); let fn_body = ast::make::block_expr(vec![], Some(placeholder_expr)); - let fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body); + let mut fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body); + if self.needs_pub { + fn_def = ast::make::add_pub_crate_modifier(fn_def); + } let (fn_def, insert_offset) = match self.target { GeneratedFunctionTarget::BehindItem(it) => { @@ -116,15 +122,14 @@ impl FunctionBuilder { (indented, it.text_range().end()) } GeneratedFunctionTarget::InEmptyItemList(it) => { - let with_leading_newline = ast::make::add_leading_newlines(1, fn_def); - let indent = IndentLevel::from_node(it.syntax()).indented(); - let mut indented = indent.increase_indent(with_leading_newline); - if !item_list_has_whitespace(&it) { - // In this case we want to make sure there's a newline between the closing - // function brace and the closing module brace (so it doesn't end in `}}`). - indented = ast::make::add_trailing_newlines(1, indented); - } - (indented, it.syntax().text_range().start() + TextUnit::from_usize(1)) + let indent_once = IndentLevel(1); + let indent = IndentLevel::from_node(it.syntax()); + + let fn_def = ast::make::add_leading_newlines(1, fn_def); + let fn_def = indent_once.increase_indent(fn_def); + let fn_def = ast::make::add_trailing_newlines(1, fn_def); + let fn_def = indent.increase_indent(fn_def); + (fn_def, it.syntax().text_range().start() + TextUnit::from_usize(1)) } }; @@ -140,11 +145,6 @@ impl FunctionBuilder { } } -/// Returns true if the given ItemList contains whitespace. -fn item_list_has_whitespace(it: &ast::ItemList) -> bool { - it.syntax().descendants_with_tokens().find(|it| it.kind() == SyntaxKind::WHITESPACE).is_some() -} - enum GeneratedFunctionTarget { BehindItem(SyntaxNode), InEmptyItemList(ast::ItemList), @@ -803,29 +803,7 @@ fn foo() { ", r" mod bar { - fn my_fn() { - <|>todo!() - } -} - -fn foo() { - bar::my_fn() -} -", - ); - check_assist( - add_function, - r" -mod bar { -} - -fn foo() { - bar::my_fn<|>() -} -", - r" -mod bar { - fn my_fn() { + pub(crate) fn my_fn() { <|>todo!() } } @@ -854,7 +832,7 @@ fn foo() { mod bar { fn something_else() {} - fn my_fn() { + pub(crate) fn my_fn() { <|>todo!() } } @@ -872,8 +850,7 @@ fn foo() { add_function, r" mod bar { - mod baz { - } + mod baz {} } fn foo() { @@ -883,7 +860,7 @@ fn foo() { r" mod bar { mod baz { - fn my_fn() { + pub(crate) fn my_fn() { <|>todo!() } } -- cgit v1.2.3