From 9b118230cd523859a54ef950a64850454a1eb657 Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Sat, 25 Apr 2020 16:58:28 +0200 Subject: [PATCH] Use new HirDisplay variant in add_function assist --- .../ra_assists/src/handlers/add_function.rs | 139 +++++++++++++----- 1 file changed, 101 insertions(+), 38 deletions(-) diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index 6c7456579fe4..d92e87c7d483 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -93,18 +93,25 @@ impl FunctionBuilder { path: &ast::Path, target_module: Option>, ) -> Option { - let needs_pub = target_module.is_some(); let mut file = AssistFile::default(); - let target = if let Some(target_module) = target_module { - let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_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 { 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, file, needs_pub }) + let (type_params, params) = fn_args(ctx, target.syntax(), &call)?; + + Some(Self { + target, + fn_name, + type_params, + params, + file, + needs_pub: target_module.is_some(), + }) } fn render(self) -> Option { @@ -150,6 +157,15 @@ enum GeneratedFunctionTarget { InEmptyItemList(ast::ItemList), } +impl GeneratedFunctionTarget { + fn syntax(&self) -> &SyntaxNode { + match self { + GeneratedFunctionTarget::BehindItem(it) => it, + GeneratedFunctionTarget::InEmptyItemList(it) => it.syntax(), + } + } +} + fn fn_name(call: &ast::Path) -> Option { let name = call.segment()?.syntax().to_string(); Some(ast::make::name(&name)) @@ -158,6 +174,7 @@ fn fn_name(call: &ast::Path) -> Option { /// Computes the type variables and arguments required for the generated function fn fn_args( ctx: &AssistCtx, + target: &SyntaxNode, call: &ast::CallExpr, ) -> Option<(Option, ast::ParamList)> { let mut arg_names = Vec::new(); @@ -168,7 +185,7 @@ fn fn_args( None => String::from("arg"), }; arg_names.push(arg_name); - arg_types.push(match fn_arg_type(ctx, &arg) { + arg_types.push(match fn_arg_type(ctx, target, &arg) { Some(ty) => ty, None => String::from("()"), }); @@ -224,12 +241,18 @@ fn fn_arg_name(fn_arg: &ast::Expr) -> Option { } } -fn fn_arg_type(ctx: &AssistCtx, fn_arg: &ast::Expr) -> Option { +fn fn_arg_type(ctx: &AssistCtx, target: &SyntaxNode, fn_arg: &ast::Expr) -> Option { let ty = ctx.sema.type_of_expr(fn_arg)?; if ty.is_unknown() { return None; } - Some(ty.display(ctx.sema.db).to_string()) + + let target_module = ctx.sema.scope(target).module()?; + if let Ok(rendered) = ty.render_in_module(ctx.db, target_module.into()) { + Some(rendered) + } else { + None + } } /// Returns the position inside the current mod or file @@ -258,11 +281,11 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option, + 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 { + let assist_item = match &module.value { hir::ModuleSource::SourceFile(it) => { if let Some(last_item) = it.items().last() { GeneratedFunctionTarget::BehindItem(last_item.syntax().clone()) @@ -606,8 +629,33 @@ fn bar(foo: impl Foo) { } #[test] - #[ignore] - // FIXME print paths properly to make this test pass + fn borrowed_arg() { + check_assist( + add_function, + r" +struct Baz; +fn baz() -> Baz { todo!() } + +fn foo() { + bar<|>(&baz()) +} +", + r" +struct Baz; +fn baz() -> Baz { todo!() } + +fn foo() { + bar(&baz()) +} + +fn bar(baz: &Baz) { + <|>todo!() +} +", + ) + } + + #[test] fn add_function_with_qualified_path_arg() { check_assist( add_function, @@ -616,10 +664,8 @@ mod Baz { pub struct Bof; pub fn baz() -> Bof { Bof } } -mod Foo { - fn foo() { - <|>bar(super::Baz::baz()) - } +fn foo() { + <|>bar(Baz::baz()) } ", r" @@ -627,14 +673,12 @@ mod Baz { pub struct Bof; pub fn baz() -> Bof { Bof } } -mod Foo { - fn foo() { - bar(super::Baz::baz()) - } +fn foo() { + bar(Baz::baz()) +} - fn bar(baz: super::Baz::Bof) { - <|>todo!() - } +fn bar(baz: Baz::Bof) { + <|>todo!() } ", ) @@ -815,6 +859,40 @@ fn foo() { ) } + #[test] + #[ignore] + // Ignored until local imports are supported. + // See https://github.com/rust-analyzer/rust-analyzer/issues/1165 + fn qualified_path_uses_correct_scope() { + check_assist( + add_function, + " +mod foo { + pub struct Foo; +} +fn bar() { + use foo::Foo; + let foo = Foo; + baz<|>(foo) +} +", + " +mod foo { + pub struct Foo; +} +fn bar() { + use foo::Foo; + let foo = Foo; + baz(foo) +} + +fn baz(foo: foo::Foo) { + <|>todo!() +} +", + ) + } + #[test] fn add_function_in_module_containing_other_items() { check_assist( @@ -926,21 +1004,6 @@ fn bar(baz: ()) {} ) } - #[test] - fn add_function_not_applicable_if_function_path_not_singleton() { - // In the future this assist could be extended to generate functions - // if the path is in the same crate (or even the same workspace). - // For the beginning, I think this is fine. - check_assist_not_applicable( - add_function, - r" -fn foo() { - other_crate::bar<|>(); -} - ", - ) - } - #[test] #[ignore] fn create_method_with_no_args() {