fix: Improve self param completion applicability

6 files changed, 124 insertions, 18 deletions

diff --git a/crates/ide-completion/src/completions/fn_param.rs b/crates/ide-completion/src/completions/fn_param.rs
index 94e2f489f57..cc6c17450b5 100644
--- a/crates/ide-completion/src/completions/fn_param.rs
+++ b/crates/ide-completion/src/completions/fn_param.rs
@@ -5,11 +5,11 @@ use ide_db::FxHashMap;
 use syntax::{
     algo,
     ast::{self, HasModuleItem},
-    match_ast, AstNode, Direction, SyntaxKind, TextRange,
+    match_ast, AstNode, Direction, SyntaxKind, TextRange, TextSize,
 };
 
 use crate::{
-    context::{ParamKind, PatternContext},
+    context::{ParamContext, ParamKind, PatternContext},
     CompletionContext, CompletionItem, CompletionItemKind, Completions,
 };
 
@@ -24,7 +24,7 @@ pub(crate) fn complete_fn_param(
     ctx: &CompletionContext,
     pattern_ctx: &PatternContext,
 ) -> Option<()> {
-    let ((param_list, _, param_kind), impl_) = match pattern_ctx {
+    let (ParamContext { param_list, kind, .. }, impl_) = match pattern_ctx {
         PatternContext { param_ctx: Some(kind), impl_, .. } => (kind, impl_),
         _ => return None,
     };
@@ -43,7 +43,7 @@ pub(crate) fn complete_fn_param(
         item.add_to(acc)
     };
 
-    match param_kind {
+    match kind {
         ParamKind::Function(function) => {
             fill_fn_params(ctx, function, param_list, impl_, add_new_item_to_acc);
         }
@@ -105,7 +105,7 @@ fn fill_fn_params(
     }
     remove_duplicated(&mut file_params, param_list.params());
     let self_completion_items = ["self", "&self", "mut self", "&mut self"];
-    if should_add_self_completions(param_list, impl_) {
+    if should_add_self_completions(ctx.token.text_range().start(), param_list, impl_) {
         self_completion_items.into_iter().for_each(|self_item| add_new_item_to_acc(self_item));
     }
 
@@ -156,10 +156,18 @@ fn remove_duplicated(
     })
 }
 
-fn should_add_self_completions(param_list: &ast::ParamList, impl_: &Option<ast::Impl>) -> bool {
-    let no_params = param_list.params().next().is_none() && param_list.self_param().is_none();
-
-    impl_.is_some() && no_params
+fn should_add_self_completions(
+    cursor: TextSize,
+    param_list: &ast::ParamList,
+    impl_: &Option<ast::Impl>,
+) -> bool {
+    if impl_.is_none() || param_list.self_param().is_some() {
+        return false;
+    }
+    match param_list.params().next() {
+        Some(first) => first.pat().map_or(false, |pat| pat.syntax().text_range().contains(cursor)),
+        None => true,
+    }
 }
 
 fn comma_wrapper(ctx: &CompletionContext) -> Option<(impl Fn(&str) -> String, TextRange)> {
diff --git a/crates/ide-completion/src/completions/item_list/trait_impl.rs b/crates/ide-completion/src/completions/item_list/trait_impl.rs
index 67eb1754607..49b8537029d 100644
--- a/crates/ide-completion/src/completions/item_list/trait_impl.rs
+++ b/crates/ide-completion/src/completions/item_list/trait_impl.rs
@@ -438,6 +438,10 @@ impl Test for T {
             expect![[r#"
                 sp Self
                 st T
+                bn &mut self
+                bn &self
+                bn mut self
+                bn self
             "#]],
         );
 
diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
index c93fd0d7bb0..e80381c36a4 100644
--- a/crates/ide-completion/src/context.rs
+++ b/crates/ide-completion/src/context.rs
@@ -198,7 +198,7 @@ pub(super) enum Qualified {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(super) struct PatternContext {
     pub(super) refutability: PatternRefutability,
-    pub(super) param_ctx: Option<(ast::ParamList, ast::Param, ParamKind)>,
+    pub(super) param_ctx: Option<ParamContext>,
     pub(super) has_type_ascription: bool,
     pub(super) parent_pat: Option<ast::Pat>,
     pub(super) ref_token: Option<SyntaxToken>,
@@ -208,6 +208,13 @@ pub(super) struct PatternContext {
     pub(super) impl_: Option<ast::Impl>,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(super) struct ParamContext {
+    pub(super) param_list: ast::ParamList,
+    pub(super) param: ast::Param,
+    pub(super) kind: ParamKind,
+}
+
 /// The state of the lifetime we are completing.
 #[derive(Debug)]
 pub(super) struct LifetimeContext {
diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs
index 7fc77aae91f..fabbbe867ee 100644
--- a/crates/ide-completion/src/context/analysis.rs
+++ b/crates/ide-completion/src/context/analysis.rs
@@ -13,8 +13,9 @@ use syntax::{
 use crate::context::{
     AttrCtx, CompletionAnalysis, CompletionContext, DotAccess, DotAccessKind, ExprCtx,
     ItemListKind, LifetimeContext, LifetimeKind, NameContext, NameKind, NameRefContext,
-    NameRefKind, ParamKind, PathCompletionCtx, PathKind, PatternContext, PatternRefutability,
-    Qualified, QualifierCtx, TypeAscriptionTarget, TypeLocation, COMPLETION_MARKER,
+    NameRefKind, ParamContext, ParamKind, PathCompletionCtx, PathKind, PatternContext,
+    PatternRefutability, Qualified, QualifierCtx, TypeAscriptionTarget, TypeLocation,
+    COMPLETION_MARKER,
 };
 
 impl<'a> CompletionContext<'a> {
@@ -990,7 +991,7 @@ fn pattern_context_for(
     original_file: &SyntaxNode,
     pat: ast::Pat,
 ) -> PatternContext {
-    let mut is_param = None;
+    let mut param_ctx = None;
     let (refutability, has_type_ascription) =
     pat
         .syntax()
@@ -1003,7 +1004,7 @@ fn pattern_context_for(
                     ast::LetStmt(let_) => return (PatternRefutability::Irrefutable, let_.ty().is_some()),
                     ast::Param(param) => {
                         let has_type_ascription = param.ty().is_some();
-                        is_param = (|| {
+                        param_ctx = (|| {
                             let fake_param_list = param.syntax().parent().and_then(ast::ParamList::cast)?;
                             let param_list = find_node_in_file_compensated(sema, original_file, &fake_param_list)?;
                             let param_list_owner = param_list.syntax().parent()?;
@@ -1014,7 +1015,9 @@ fn pattern_context_for(
                                     _ => return None,
                                 }
                             };
-                            Some((param_list, param, kind))
+                            Some(ParamContext {
+                                param_list, param, kind
+                            })
                         })();
                         return (PatternRefutability::Irrefutable, has_type_ascription)
                     },
@@ -1033,7 +1036,7 @@ fn pattern_context_for(
 
     PatternContext {
         refutability,
-        param_ctx: is_param,
+        param_ctx,
         has_type_ascription,
         parent_pat: pat.syntax().parent().and_then(ast::Pat::cast),
         mut_token,
diff --git a/crates/ide-completion/src/render/pattern.rs b/crates/ide-completion/src/render/pattern.rs
index d6779961c07..03db08a911e 100644
--- a/crates/ide-completion/src/render/pattern.rs
+++ b/crates/ide-completion/src/render/pattern.rs
@@ -6,7 +6,7 @@ use itertools::Itertools;
 use syntax::SmolStr;
 
 use crate::{
-    context::{ParamKind, PatternContext},
+    context::{ParamContext, ParamKind, PatternContext},
     render::{
         variant::{format_literal_label, visible_fields},
         RenderContext,
@@ -102,7 +102,7 @@ fn render_pat(
     let needs_ascription = matches!(
         pattern_ctx,
         PatternContext {
-            param_ctx: Some((.., ParamKind::Function(_))),
+            param_ctx: Some(ParamContext { kind: ParamKind::Function(_), .. }),
             has_type_ascription: false,
             ..
         }
diff --git a/crates/ide-completion/src/tests/pattern.rs b/crates/ide-completion/src/tests/pattern.rs
index 18088680660..877b5f21643 100644
--- a/crates/ide-completion/src/tests/pattern.rs
+++ b/crates/ide-completion/src/tests/pattern.rs
@@ -630,3 +630,87 @@ fn f(v: u32) {
         "#]],
     );
 }
+
+#[test]
+fn in_method_param() {
+    check_empty(
+        r#"
+struct Ty(u8);
+
+impl Ty {
+    fn foo($0)
+}
+"#,
+        expect![[r#"
+            sp Self
+            st Ty
+            bn &mut self
+            bn &self
+            bn Self(…)   Self($1): Self$0
+            bn Ty(…)     Ty($1): Ty$0
+            bn mut self
+            bn self
+            kw mut
+            kw ref
+        "#]],
+    );
+    check_empty(
+        r#"
+struct Ty(u8);
+
+impl Ty {
+    fn foo(s$0)
+}
+"#,
+        expect![[r#"
+            sp Self
+            st Ty
+            bn &mut self
+            bn &self
+            bn Self(…)   Self($1): Self$0
+            bn Ty(…)     Ty($1): Ty$0
+            bn mut self
+            bn self
+            kw mut
+            kw ref
+        "#]],
+    );
+    check_empty(
+        r#"
+struct Ty(u8);
+
+impl Ty {
+    fn foo(s$0, foo: u8)
+}
+"#,
+        expect![[r#"
+            sp Self
+            st Ty
+            bn &mut self
+            bn &self
+            bn Self(…)   Self($1): Self$0
+            bn Ty(…)     Ty($1): Ty$0
+            bn mut self
+            bn self
+            kw mut
+            kw ref
+        "#]],
+    );
+    check_empty(
+        r#"
+struct Ty(u8);
+
+impl Ty {
+    fn foo(foo: u8, b$0)
+}
+"#,
+        expect![[r#"
+            sp Self
+            st Ty
+            bn Self(…) Self($1): Self$0
+            bn Ty(…)   Ty($1): Ty$0
+            kw mut
+            kw ref
+        "#]],
+    );
+}