Don't use an untyped String for ActiveParam tracking

5 files changed, 44 insertions, 24 deletions

diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs
index 00493a6b5..8e0940184 100644
--- a/crates/ide/src/syntax_highlighting/inject.rs
+++ b/crates/ide/src/syntax_highlighting/inject.rs
@@ -23,7 +23,7 @@ pub(super) fn ra_fixture(
     expanded: SyntaxToken,
 ) -> Option<()> {
     let active_parameter = ActiveParameter::at_token(&sema, expanded)?;
-    if !active_parameter.name.starts_with("ra_fixture") {
+    if !active_parameter.ident().map_or(false, |name| name.text().starts_with("ra_fixture")) {
         return None;
     }
     let value = literal.value()?;
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 6d57da06a..a60b8b09d 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -4,8 +4,11 @@ use hir::{Local, ScopeDef, Semantics, SemanticsScope, Type};
 use ide_db::base_db::{FilePosition, SourceDatabase};
 use ide_db::{call_info::ActiveParameter, RootDatabase};
 use syntax::{
-    algo::find_node_at_offset, ast, match_ast, AstNode, NodeOrToken, SyntaxKind::*, SyntaxNode,
-    SyntaxToken, TextRange, TextSize,
+    algo::find_node_at_offset,
+    ast::{self, NameOrNameRef, NameOwner},
+    match_ast, AstNode, NodeOrToken,
+    SyntaxKind::*,
+    SyntaxNode, SyntaxToken, TextRange, TextSize,
 };
 
 use text_edit::Indel;
@@ -35,7 +38,7 @@ pub(crate) struct CompletionContext<'a> {
     /// The token before the cursor, in the macro-expanded file.
     pub(super) token: SyntaxToken,
     pub(super) krate: Option<hir::Crate>,
-    pub(super) expected_name: Option<String>,
+    pub(super) expected_name: Option<NameOrNameRef>,
     pub(super) expected_type: Option<Type>,
     pub(super) name_ref_syntax: Option<ast::NameRef>,
     pub(super) function_syntax: Option<ast::Fn>,
@@ -292,13 +295,13 @@ impl<'a> CompletionContext<'a> {
         file_with_fake_ident: SyntaxNode,
         offset: TextSize,
     ) {
-        let expected = {
+        let (expected_type, expected_name) = {
             let mut node = match self.token.parent() {
                 Some(it) => it,
                 None => return,
             };
             loop {
-                let ret = match_ast! {
+                break match_ast! {
                     match node {
                         ast::LetStmt(it) => {
                             cov_mark::hit!(expected_type_let_with_leading_char);
@@ -306,7 +309,7 @@ impl<'a> CompletionContext<'a> {
                             let ty = it.pat()
                                 .and_then(|pat| self.sema.type_of_pat(&pat));
                             let name = if let Some(ast::Pat::IdentPat(ident)) = it.pat() {
-                                Some(ident.syntax().text().to_string())
+                                ident.name().map(NameOrNameRef::Name)
                             } else {
                                 None
                             };
@@ -319,7 +322,10 @@ impl<'a> CompletionContext<'a> {
                             ActiveParameter::at_token(
                                 &self.sema,
                                 self.token.clone(),
-                            ).map(|ap| (Some(ap.ty), Some(ap.name)))
+                            ).map(|ap| {
+                                let name = ap.ident().map(NameOrNameRef::Name);
+                                (Some(ap.ty), name)
+                            })
                             .unwrap_or((None, None))
                         },
                         ast::RecordExprFieldList(_it) => {
@@ -327,10 +333,10 @@ impl<'a> CompletionContext<'a> {
                             self.token.prev_sibling_or_token()
                                 .and_then(|se| se.into_node())
                                 .and_then(|node| ast::RecordExprField::cast(node))
-                                .and_then(|rf| self.sema.resolve_record_field(&rf))
-                                .map(|f|(
+                                .and_then(|rf| self.sema.resolve_record_field(&rf).zip(Some(rf)))
+                                .map(|(f, rf)|(
                                     Some(f.0.signature_ty(self.db)),
-                                    Some(f.0.name(self.db).to_string()),
+                                    rf.field_name().map(NameOrNameRef::NameRef),
                                 ))
                                 .unwrap_or((None, None))
                         },
@@ -340,7 +346,7 @@ impl<'a> CompletionContext<'a> {
                                 .resolve_record_field(&it)
                                 .map(|f|(
                                     Some(f.0.signature_ty(self.db)),
-                                    Some(f.0.name(self.db).to_string()),
+                                    it.field_name().map(NameOrNameRef::NameRef),
                                 ))
                                 .unwrap_or((None, None))
                         },
@@ -378,12 +384,10 @@ impl<'a> CompletionContext<'a> {
                         },
                     }
                 };
-
-                break ret;
             }
         };
-        self.expected_type = expected.0;
-        self.expected_name = expected.1;
+        self.expected_type = expected_type;
+        self.expected_name = expected_name;
         self.attribute_under_caret = find_node_at_offset(&file_with_fake_ident, offset);
 
         // First, let's try to complete a reference to some declaration.
@@ -631,7 +635,9 @@ mod tests {
             .map(|t| t.display_test(&db).to_string())
             .unwrap_or("?".to_owned());
 
-        let name = completion_context.expected_name.unwrap_or("?".to_owned());
+        let name = completion_context
+            .expected_name
+            .map_or_else(|| "?".to_owned(), |name| name.to_string());
 
         expect.assert_eq(&format!("ty: {}, name: {}", ty, name));
     }
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs
index 12921e12b..2b6e9ebd1 100644
--- a/crates/ide_completion/src/render.rs
+++ b/crates/ide_completion/src/render.rs
@@ -243,7 +243,7 @@ impl<'a> Render<'a> {
 
             item.set_relevance(CompletionRelevance {
                 exact_type_match: compute_exact_type_match(self.ctx.completion, &ty),
-                exact_name_match: compute_exact_name_match(self.ctx.completion, local_name.clone()),
+                exact_name_match: compute_exact_name_match(self.ctx.completion, &local_name),
                 is_local: true,
                 ..CompletionRelevance::default()
             });
@@ -319,8 +319,7 @@ fn compute_exact_type_match(ctx: &CompletionContext, completion_ty: &hir::Type)
 
 fn compute_exact_name_match(ctx: &CompletionContext, completion_name: impl Into<String>) -> bool {
     let completion_name = completion_name.into();
-
-    Some(&completion_name) == ctx.expected_name.as_ref()
+    ctx.expected_name.as_ref().map_or(false, |name| name.text() == completion_name)
 }
 
 fn compute_ref_match(ctx: &CompletionContext, completion_ty: &hir::Type) -> Option<Mutability> {
diff --git a/crates/ide_db/src/call_info.rs b/crates/ide_db/src/call_info.rs
index 7e26c3ccf..e583a52f4 100644
--- a/crates/ide_db/src/call_info.rs
+++ b/crates/ide_db/src/call_info.rs
@@ -4,7 +4,7 @@ use either::Either;
 use hir::{HasAttrs, HirDisplay, Semantics, Type};
 use stdx::format_to;
 use syntax::{
-    ast::{self, ArgListOwner},
+    ast::{self, ArgListOwner, NameOwner},
     match_ast, AstNode, SyntaxNode, SyntaxToken, TextRange, TextSize,
 };
 
@@ -142,7 +142,7 @@ fn call_info_impl(
 #[derive(Debug)]
 pub struct ActiveParameter {
     pub ty: Type,
-    pub name: String,
+    pub pat: Either<ast::SelfParam, ast::Pat>,
 }
 
 impl ActiveParameter {
@@ -165,8 +165,14 @@ impl ActiveParameter {
             return None;
         }
         let (pat, ty) = params.swap_remove(idx);
-        let name = pat?.to_string();
-        Some(ActiveParameter { ty, name })
+        pat.map(|pat| ActiveParameter { ty, pat })
+    }
+
+    pub fn ident(&self) -> Option<ast::Name> {
+        self.pat.as_ref().right().and_then(|param| match param {
+            ast::Pat::IdentPat(ident) => ident.name(),
+            _ => None,
+        })
     }
 }
 
diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs
index 01f580a40..42a7b9c2a 100644
--- a/crates/syntax/src/ast/node_ext.rs
+++ b/crates/syntax/src/ast/node_ext.rs
@@ -380,6 +380,15 @@ impl fmt::Display for NameOrNameRef {
     }
 }
 
+impl NameOrNameRef {
+    pub fn text(&self) -> &str {
+        match self {
+            NameOrNameRef::Name(name) => name.text(),
+            NameOrNameRef::NameRef(name_ref) => name_ref.text(),
+        }
+    }
+}
+
 impl ast::RecordPatField {
     pub fn for_field_name_ref(field_name: &ast::NameRef) -> Option<ast::RecordPatField> {
         let candidate = field_name.syntax().parent().and_then(ast::RecordPatField::cast)?;