Merge #6688

6688: Place cursor correctly when completing assoc fns with self r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

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: Sub>() { 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<T> Sub for Wrap<T> {
 }
 "#,
             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)
             "#]],
         );
     }

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)

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<String>),
     Anonymous(usize),
@@ -24,7 +25,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 +59,7 @@ impl Builder {
         name: String,
         params: Params,
     ) -> Builder {
-        if !self.should_add_parems(ctx) {
+        if !self.should_add_parens(ctx) {
             return self;
         }
 

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},
@@ -22,7 +23,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 +36,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<ImportToAdd>) -> 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,27 +68,39 @@ impl<'a> FunctionRender<'a> {
     }
 
     fn params(&self) -> Params {
-        let params_ty = self.fn_.params(self.ctx.db());
-        let params = self
-            .ast_node
-            .param_list()
+        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())
+        };
+        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)
     }
 
     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
@@ -172,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);

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<Param> {
+    pub fn assoc_fn_params(self, db: &dyn HirDatabase) -> Vec<Param> {
         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<Vec<Param>> {
+        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
@@ -799,6 +806,7 @@ impl From<Mutability> for Access {
     }
 }
 
+#[derive(Debug)]
 pub struct Param {
     ty: Type,
 }

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();