Merge #10561

10561: internal: Don't turn local names into strings in CompletionContext r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <[email protected]>

Author: bors[bot]

crates/hir_expand/src/name.rs

@@ -67,12 +67,30 @@ impl Name {
         Name::new_text("[missing name]".into())
     }
 
+    /// Returns the tuple index this name represents if it is a tuple field.
     pub fn as_tuple_index(&self) -> Option<usize> {
         match self.0 {
             Repr::TupleField(idx) => Some(idx),
             _ => None,
         }
     }
+
+    /// Returns the text this name represents if it isn't a tuple field.
+    pub fn as_text(&self) -> Option<SmolStr> {
+        match &self.0 {
+            Repr::Text(it) => Some(it.clone()),
+            _ => None,
+        }
+    }
+
+    /// Returns the textual representation of this name as a [`SmolStr`].
+    /// Prefer using this over [`ToString::to_string`] if possible as this conversion is cheaper.
+    pub fn to_smol_str(&self) -> SmolStr {
+        match &self.0 {
+            Repr::Text(it) => it.clone(),
+            Repr::TupleField(it) => SmolStr::new(&it.to_string()),
+        }
+    }
 }
 
 pub trait AsName {

crates/ide_completion/src/context.rs

@@ -102,7 +102,9 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) function_def: Option<ast::Fn>,
     /// The parent impl of the cursor position if it exists.
     pub(super) impl_def: Option<ast::Impl>,
+    /// The NameLike under the cursor in the original file if it exists.
     pub(super) name_syntax: Option<ast::NameLike>,
+    pub(super) incomplete_let: bool,
 
     pub(super) completion_location: Option<ImmediateLocation>,
     pub(super) prev_sibling: Option<ImmediatePrevSibling>,
@@ -112,9 +114,7 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) lifetime_ctx: Option<LifetimeContext>,
     pub(super) pattern_ctx: Option<PatternContext>,
     pub(super) path_context: Option<PathCompletionContext>,
-    pub(super) locals: Vec<(String, Local)>,
-
-    pub(super) incomplete_let: bool,
+    pub(super) locals: Vec<(Name, Local)>,
 
     no_completion_required: bool,
 }
@@ -148,7 +148,7 @@ impl<'a> CompletionContext<'a> {
         let mut locals = vec![];
         scope.process_all_names(&mut |name, scope| {
             if let ScopeDef::Local(local) = scope {
-                locals.push((name.to_string(), local));
+                locals.push((name, local));
             }
         });
         let mut ctx = CompletionContext {
@@ -492,14 +492,6 @@ impl<'a> CompletionContext<'a> {
         false
     }
 
-    fn fill_impl_def(&mut self) {
-        self.impl_def = self
-            .sema
-            .token_ancestors_with_macros(self.token.clone())
-            .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
-            .find_map(ast::Impl::cast);
-    }
-
     fn expected_type_and_name(&self) -> (Option<Type>, Option<NameOrNameRef>) {
         let mut node = match self.token.parent() {
             Some(it) => it,
@@ -654,6 +646,16 @@ impl<'a> CompletionContext<'a> {
         self.prev_sibling = determine_prev_sibling(&name_like);
         self.name_syntax =
             find_node_at_offset(original_file, name_like.syntax().text_range().start());
+        self.impl_def = self
+            .sema
+            .token_ancestors_with_macros(self.token.clone())
+            .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
+            .find_map(ast::Impl::cast);
+        self.function_def = self
+            .sema
+            .token_ancestors_with_macros(self.token.clone())
+            .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
+            .find_map(ast::Fn::cast);
         match name_like {
             ast::NameLike::Lifetime(lifetime) => {
                 self.classify_lifetime(original_file, lifetime, offset);
@@ -691,8 +693,6 @@ impl<'a> CompletionContext<'a> {
     }
 
     fn classify_name(&mut self, name: ast::Name) {
-        self.fill_impl_def();
-
         if let Some(bind_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) {
             let is_name_in_field_pat = bind_pat
                 .syntax()
@@ -740,14 +740,6 @@ impl<'a> CompletionContext<'a> {
     }
 
     fn classify_name_ref(&mut self, original_file: &SyntaxNode, name_ref: ast::NameRef) {
-        self.fill_impl_def();
-
-        self.function_def = self
-            .sema
-            .token_ancestors_with_macros(self.token.clone())
-            .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
-            .find_map(ast::Fn::cast);
-
         let parent = match name_ref.syntax().parent() {
             Some(it) => it,
             None => return,

crates/ide_completion/src/render/builder_ext.rs

@@ -1,12 +1,14 @@
 //! Extensions for `Builder` structure required for item rendering.
 
+use either::Either;
 use itertools::Itertools;
+use syntax::ast::{self, HasName};
 
 use crate::{context::CallKind, item::Builder, patterns::ImmediateLocation, CompletionContext};
 
 #[derive(Debug)]
 pub(super) enum Params {
-    Named(Vec<String>),
+    Named(Vec<(Either<ast::SelfParam, ast::Param>, hir::Param)>),
     Anonymous(usize),
 }
 
@@ -76,10 +78,46 @@ impl Builder {
             self.trigger_call_info();
             let snippet = match (ctx.config.add_call_argument_snippets, params) {
                 (true, Params::Named(params)) => {
-                    let function_params_snippet =
-                        params.iter().enumerate().format_with(", ", |(index, param_name), f| {
-                            f(&format_args!("${{{}:{}}}", index + 1, param_name))
-                        });
+                    let function_params_snippet = params.iter().enumerate().format_with(
+                        ", ",
+                        |(index, (param_source, param)), f| {
+                            let name;
+                            let text;
+                            let (ref_, name) = match param_source {
+                                Either::Left(self_param) => (
+                                    match self_param.kind() {
+                                        ast::SelfParamKind::Owned => "",
+                                        ast::SelfParamKind::Ref => "&",
+                                        ast::SelfParamKind::MutRef => "&mut ",
+                                    },
+                                    "self",
+                                ),
+                                Either::Right(it) => {
+                                    let n = (|| {
+                                        let mut pat = it.pat()?;
+                                        loop {
+                                            match pat {
+                                                ast::Pat::IdentPat(pat) => break pat.name(),
+                                                ast::Pat::RefPat(it) => pat = it.pat()?,
+                                                _ => return None,
+                                            }
+                                        }
+                                    })();
+                                    match n {
+                                        Some(n) => {
+                                            name = n;
+                                            text = name.text();
+                                            let text = text.as_str().trim_start_matches('_');
+                                            let ref_ = ref_of_param(ctx, text, param.ty());
+                                            (ref_, text)
+                                        }
+                                        None => ("", "_"),
+                                    }
+                                }
+                            };
+                            f(&format_args!("${{{}:{}{}}}", index + 1, ref_, name))
+                        },
+                    );
                     format!("{}({})$0", name, function_params_snippet)
                 }
                 _ => {
@@ -93,3 +131,13 @@ impl Builder {
         self.lookup_by(name).label(label).insert_snippet(cap, snippet)
     }
 }
+fn ref_of_param(ctx: &CompletionContext, arg: &str, ty: &hir::Type) -> &'static str {
+    if let Some(derefed_ty) = ty.remove_ref() {
+        for (name, local) in ctx.locals.iter() {
+            if name.as_text().as_deref() == Some(arg) && local.ty(ctx.db) == derefed_ty {
+                return if ty.is_mutable_reference() { "&mut " } else { "&" };
+            }
+        }
+    }
+    ""
+}

crates/ide_completion/src/render/function.rs

@@ -1,5 +1,6 @@
 //! Renderer for function calls.
 
+use either::Either;
 use hir::{AsAssocItem, HasSource, HirDisplay};
 use ide_db::SymbolKind;
 use itertools::Itertools;
@@ -160,47 +161,25 @@ impl<'a> FunctionRender<'a> {
         format!("-> {}", ret_ty.display(self.ctx.db()))
     }
 
-    fn add_arg(&self, arg: &str, ty: &hir::Type) -> String {
-        if let Some(derefed_ty) = ty.remove_ref() {
-            for (name, local) in self.ctx.completion.locals.iter() {
-                if name == arg && local.ty(self.ctx.db()) == derefed_ty {
-                    let mutability = if ty.is_mutable_reference() { "&mut " } else { "&" };
-                    return format!("{}{}", mutability, arg);
-                }
-            }
-        }
-        arg.to_string()
-    }
-
     fn params(&self) -> Params {
         let ast_params = match self.ast_node.param_list() {
             Some(it) => it,
             None => return Params::Named(Vec::new()),
         };
+        let params = ast_params.params().map(Either::Right);
 
-        let mut params_pats = Vec::new();
-        let params_ty = if self.ctx.completion.has_dot_receiver() || self.receiver.is_some() {
-            self.func.method_params(self.ctx.db()).unwrap_or_default()
+        let params = if self.ctx.completion.has_dot_receiver() || self.receiver.is_some() {
+            params.zip(self.func.method_params(self.ctx.db()).unwrap_or_default()).collect()
         } else {
-            if let Some(s) = ast_params.self_param() {
-                cov_mark::hit!(parens_for_method_call_as_assoc_fn);
-                params_pats.push(Some(s.to_string()));
-            }
-            self.func.assoc_fn_params(self.ctx.db())
+            ast_params
+                .self_param()
+                .map(Either::Left)
+                .into_iter()
+                .chain(params)
+                .zip(self.func.assoc_fn_params(self.ctx.db()))
+                .collect()
         };
-        params_pats
-            .extend(ast_params.params().into_iter().map(|it| it.pat().map(|it| it.to_string())));
-
-        let params = params_pats
-            .into_iter()
-            .zip(params_ty)
-            .flat_map(|(pat, param_ty)| {
-                let pat = pat?;
-                let name = pat;
-                let arg = name.trim_start_matches("mut ").trim_start_matches('_');
-                Some(self.add_arg(arg, param_ty.ty()))
-            })
-            .collect();
+
         Params::Named(params)
     }
 
@@ -310,7 +289,6 @@ impl S {
 
     #[test]
     fn parens_for_method_call_as_assoc_fn() {
-        cov_mark::check!(parens_for_method_call_as_assoc_fn);
         check_edit(
             "foo",
             r#"