From 6f51f728a114078a0c3a029fc66cfb8c4daf9a28 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 1 Dec 2020 13:53:12 +0300 Subject: [PATCH 1/3] Type-safer API for dealing with parameter lists with optional self --- crates/completion/src/completions/trait_impl.rs | 2 +- crates/completion/src/render/function.rs | 16 ++++++++++------ crates/hir/src/code_model.rs | 11 +++++++++-- crates/ide/src/references/rename.rs | 2 +- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/crates/completion/src/completions/trait_impl.rs b/crates/completion/src/completions/trait_impl.rs index a14be9c73b..e2fe44aff9 100644 --- a/crates/completion/src/completions/trait_impl.rs +++ b/crates/completion/src/completions/trait_impl.rs @@ -139,7 +139,7 @@ fn add_function_impl( ) { let fn_name = func.name(ctx.db).to_string(); - let label = if func.params(ctx.db).is_empty() { + let label = if func.assoc_fn_params(ctx.db).is_empty() { format!("fn {}()", fn_name) } else { format!("fn {}(..)", fn_name) diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index 542383d7e7..09d9f85bc0 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -22,7 +22,7 @@ pub(crate) fn render_fn<'a>( struct FunctionRender<'a> { ctx: RenderContext<'a>, name: String, - fn_: hir::Function, + func: hir::Function, ast_node: Fn, } @@ -35,15 +35,15 @@ impl<'a> FunctionRender<'a> { let name = local_name.unwrap_or_else(|| fn_.name(ctx.db()).to_string()); let ast_node = fn_.source(ctx.db()).value; - FunctionRender { ctx, name, fn_, ast_node } + FunctionRender { ctx, name, func: fn_, ast_node } } fn render(self, import_to_add: Option) -> CompletionItem { let params = self.params(); CompletionItem::new(CompletionKind::Reference, self.ctx.source_range(), self.name.clone()) .kind(self.kind()) - .set_documentation(self.ctx.docs(self.fn_)) - .set_deprecated(self.ctx.is_deprecated(self.fn_)) + .set_documentation(self.ctx.docs(self.func)) + .set_deprecated(self.ctx.is_deprecated(self.func)) .detail(self.detail()) .add_call_parens(self.ctx.completion, self.name, params) .add_import(import_to_add) @@ -67,7 +67,11 @@ impl<'a> FunctionRender<'a> { } fn params(&self) -> Params { - let params_ty = self.fn_.params(self.ctx.db()); + let params_ty = if self.ctx.completion.dot_receiver.is_some() { + self.func.method_params(self.ctx.db()).unwrap_or_default() + } else { + self.func.assoc_fn_params(self.ctx.db()) + }; let params = self .ast_node .param_list() @@ -87,7 +91,7 @@ impl<'a> FunctionRender<'a> { } fn kind(&self) -> CompletionItemKind { - if self.fn_.self_param(self.ctx.db()).is_some() { + if self.func.self_param(self.ctx.db()).is_some() { CompletionItemKind::Method } else { CompletionItemKind::Function diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index f06b5cd9f0..ba121104ba 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -744,14 +744,13 @@ impl Function { Some(SelfParam { func: self.id }) } - pub fn params(self, db: &dyn HirDatabase) -> Vec { + pub fn assoc_fn_params(self, db: &dyn HirDatabase) -> Vec { let resolver = self.id.resolver(db.upcast()); let ctx = hir_ty::TyLoweringContext::new(db, &resolver); let environment = TraitEnvironment::lower(db, &resolver); db.function_data(self.id) .params .iter() - .skip(if self.self_param(db).is_some() { 1 } else { 0 }) .map(|type_ref| { let ty = Type { krate: self.id.lookup(db.upcast()).container.module(db.upcast()).krate, @@ -764,6 +763,14 @@ impl Function { }) .collect() } + pub fn method_params(self, db: &dyn HirDatabase) -> Option> { + if self.self_param(db).is_none() { + return None; + } + let mut res = self.assoc_fn_params(db); + res.remove(0); + Some(res) + } pub fn is_unsafe(self, db: &dyn HirDatabase) -> bool { db.function_data(self.id).is_unsafe diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 7314576967..64fe8bd654 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -241,7 +241,7 @@ fn rename_to_self( return Err(RenameError("Method already has a self parameter".to_string())); } - let params = fn_def.params(sema.db); + let params = fn_def.assoc_fn_params(sema.db); let first_param = params.first().ok_or_else(|| RenameError("Method has no parameters".into()))?; let first_param_ty = first_param.ty(); From 02955661a0e8b39fd0b887b245f0e1284ea8f504 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 30 Nov 2020 13:45:32 +0300 Subject: [PATCH 2/3] Fix typo --- crates/completion/src/render/builder_ext.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/completion/src/render/builder_ext.rs b/crates/completion/src/render/builder_ext.rs index 37b0d0459a..79599de4b7 100644 --- a/crates/completion/src/render/builder_ext.rs +++ b/crates/completion/src/render/builder_ext.rs @@ -24,7 +24,7 @@ impl Params { } impl Builder { - pub(super) fn should_add_parems(&self, ctx: &CompletionContext) -> bool { + fn should_add_parens(&self, ctx: &CompletionContext) -> bool { if !ctx.config.add_call_parenthesis { return false; } @@ -58,7 +58,7 @@ impl Builder { name: String, params: Params, ) -> Builder { - if !self.should_add_parems(ctx) { + if !self.should_add_parens(ctx) { return self; } From 9d94ffad44f2b250e498f162cd498aed62877c8e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 1 Dec 2020 14:23:00 +0300 Subject: [PATCH 3/3] Place cursor correctly when completing assoc fns with self --- .../src/completions/qualified_path.rs | 40 +++++++------- crates/completion/src/render/builder_ext.rs | 1 + crates/completion/src/render/function.rs | 53 +++++++++++++++---- crates/hir/src/code_model.rs | 1 + 4 files changed, 64 insertions(+), 31 deletions(-) diff --git a/crates/completion/src/completions/qualified_path.rs b/crates/completion/src/completions/qualified_path.rs index d9387054dc..bc23bea3fa 100644 --- a/crates/completion/src/completions/qualified_path.rs +++ b/crates/completion/src/completions/qualified_path.rs @@ -353,10 +353,10 @@ impl S { fn foo() { let _ = S::<|> } "#, expect![[r#" - ct C const C: i32 = 42; - ta T type T = i32; - fn a() fn a() - me b() fn b(&self) + ct C const C: i32 = 42; + ta T type T = i32; + fn a() fn a() + me b(…) fn b(&self) "#]], ); } @@ -503,14 +503,14 @@ trait Sub: Super { fn foo() { T::<|> } "#, expect![[r#" - ct C2 const C2: (); - ct CONST const CONST: u8; - ta SubTy type SubTy; - ta Ty type Ty; - fn func() fn func() - me method() fn method(&self) - fn subfunc() fn subfunc() - me submethod() fn submethod(&self) + ct C2 const C2: (); + ct CONST const CONST: u8; + ta SubTy type SubTy; + ta Ty type Ty; + fn func() fn func() + me method(…) fn method(&self) + fn subfunc() fn subfunc() + me submethod(…) fn submethod(&self) "#]], ); } @@ -543,14 +543,14 @@ impl Sub for Wrap { } "#, expect![[r#" - ct C2 const C2: () = (); - ct CONST const CONST: u8 = 0; - ta SubTy type SubTy; - ta Ty type Ty; - fn func() fn func() - me method() fn method(&self) - fn subfunc() fn subfunc() - me submethod() fn submethod(&self) + ct C2 const C2: () = (); + ct CONST const CONST: u8 = 0; + ta SubTy type SubTy; + ta Ty type Ty; + fn func() fn func() + me method(…) fn method(&self) + fn subfunc() fn subfunc() + me submethod(…) fn submethod(&self) "#]], ); } diff --git a/crates/completion/src/render/builder_ext.rs b/crates/completion/src/render/builder_ext.rs index 79599de4b7..ce8718bd54 100644 --- a/crates/completion/src/render/builder_ext.rs +++ b/crates/completion/src/render/builder_ext.rs @@ -5,6 +5,7 @@ use test_utils::mark; use crate::{item::Builder, CompletionContext}; +#[derive(Debug)] pub(super) enum Params { Named(Vec), Anonymous(usize), diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index 09d9f85bc0..00e3eb203e 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -2,6 +2,7 @@ use hir::{HasSource, Type}; use syntax::{ast::Fn, display::function_declaration}; +use test_utils::mark; use crate::{ item::{CompletionItem, CompletionItemKind, CompletionKind, ImportToAdd}, @@ -67,24 +68,32 @@ impl<'a> FunctionRender<'a> { } fn params(&self) -> Params { + let ast_params = match self.ast_node.param_list() { + Some(it) => it, + None => return Params::Named(Vec::new()), + }; + + let mut params_pats = Vec::new(); let params_ty = if self.ctx.completion.dot_receiver.is_some() { self.func.method_params(self.ctx.db()).unwrap_or_default() } else { + if let Some(s) = ast_params.self_param() { + mark::hit!(parens_for_method_call_as_assoc_fn); + params_pats.push(Some(s.to_string())); + } self.func.assoc_fn_params(self.ctx.db()) }; - let params = self - .ast_node - .param_list() + params_pats + .extend(ast_params.params().into_iter().map(|it| it.pat().map(|it| it.to_string()))); + + let params = params_pats .into_iter() - .flat_map(|it| it.params()) .zip(params_ty) - .flat_map(|(it, param_ty)| { - if let Some(pat) = it.pat() { - let name = pat.to_string(); - let arg = name.trim_start_matches("mut ").trim_start_matches('_'); - return Some(self.add_arg(arg, param_ty.ty())); - } - None + .flat_map(|(pat, param_ty)| { + let pat = pat?; + let name = pat.to_string(); + let arg = name.trim_start_matches("mut ").trim_start_matches('_'); + Some(self.add_arg(arg, param_ty.ty())) }) .collect(); Params::Named(params) @@ -176,6 +185,28 @@ fn bar(s: &S) { ); } + #[test] + fn parens_for_method_call_as_assoc_fn() { + mark::check!(parens_for_method_call_as_assoc_fn); + check_edit( + "foo", + r#" +struct S; +impl S { + fn foo(&self) {} +} +fn main() { S::f<|> } +"#, + r#" +struct S; +impl S { + fn foo(&self) {} +} +fn main() { S::foo(${1:&self})$0 } +"#, + ); + } + #[test] fn suppress_arg_snippets() { mark::check!(suppress_arg_snippets); diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index ba121104ba..4500050f1b 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -806,6 +806,7 @@ impl From for Access { } } +#[derive(Debug)] pub struct Param { ty: Type, }