Merge #11693

11693: internal: Remove ide_completion::render::build_ext module r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir/src/lib.rs

@@ -1325,7 +1325,7 @@ impl Function {
     /// Get this function's return type
     pub fn ret_type(self, db: &dyn HirDatabase) -> Type {
         let resolver = self.id.resolver(db.upcast());
-        let krate = self.id.lookup(db.upcast()).container.module(db.upcast()).krate();
+        let krate = self.krate_id(db);
         let ret_type = &db.function_data(self.id).ret_type;
         let ctx = hir_ty::TyLoweringContext::new(db, &resolver);
         let ty = ctx.lower_ty(ret_type);
@@ -1341,7 +1341,7 @@ impl Function {
 
     pub fn assoc_fn_params(self, db: &dyn HirDatabase) -> Vec<Param> {
         let resolver = self.id.resolver(db.upcast());
-        let krate = self.id.lookup(db.upcast()).container.module(db.upcast()).krate();
+        let krate = self.krate_id(db);
         let ctx = hir_ty::TyLoweringContext::new(db, &resolver);
         let environment = db.trait_environment(self.id.into());
         db.function_data(self.id)
@@ -1359,9 +1359,25 @@ impl Function {
         if self.self_param(db).is_none() {
             return None;
         }
-        let mut res = self.assoc_fn_params(db);
+        Some(self.params_without_self(db))
-        res.remove(0);
+    }
-        Some(res)
+
+    pub fn params_without_self(self, db: &dyn HirDatabase) -> Vec<Param> {
+        let resolver = self.id.resolver(db.upcast());
+        let krate = self.krate_id(db);
+        let ctx = hir_ty::TyLoweringContext::new(db, &resolver);
+        let environment = db.trait_environment(self.id.into());
+        let skip = if db.function_data(self.id).has_self_param() { 1 } else { 0 };
+        db.function_data(self.id)
+            .params
+            .iter()
+            .enumerate()
+            .skip(skip)
+            .map(|(idx, (_, type_ref))| {
+                let ty = Type { krate, env: environment.clone(), ty: ctx.lower_ty(type_ref) };
+                Param { func: self, ty, idx }
+            })
+            .collect()
     }
 
     pub fn is_unsafe(self, db: &dyn HirDatabase) -> bool {
@@ -1410,6 +1426,10 @@ impl Function {
 
         result
     }
+
+    fn krate_id(self, db: &dyn HirDatabase) -> CrateId {
+        self.id.lookup(db.upcast()).module(db.upcast()).krate()
+    }
 }
 
 // Note: logically, this belongs to `hir_ty`, but we are not using it there yet.

crates/ide_completion/src/context.rs

@@ -59,7 +59,7 @@ pub(super) enum PathKind {
 #[derive(Debug)]
 pub(crate) struct PathCompletionCtx {
     /// If this is a call with () already there
-    has_call_parens: bool,
+    pub(super) has_call_parens: bool,
     /// Whether this path stars with a `::`.
     pub(super) is_absolute_path: bool,
     /// The qualifier of the current path if it exists.

crates/ide_completion/src/render.rs

@@ -10,8 +10,6 @@ pub(crate) mod type_alias;
 pub(crate) mod struct_literal;
 pub(crate) mod compound;
 
-mod builder_ext;
-
 use hir::{AsAssocItem, HasAttrs, HirDisplay, ScopeDef};
 use ide_db::{helpers::item_name, RootDatabase, SnippetCap, SymbolKind};
 use syntax::{SmolStr, SyntaxKind, TextRange};

crates/ide_completion/src/render/builder_ext.rs

@@ -1,129 +0,0 @@
-//! Extensions for `Builder` structure required for item rendering.
-
-use itertools::Itertools;
-use syntax::SmolStr;
-
-use crate::{context::PathKind, item::Builder, patterns::ImmediateLocation, CompletionContext};
-
-#[derive(Debug)]
-pub(super) enum Params {
-    Named(Option<hir::SelfParam>, Vec<hir::Param>),
-    #[allow(dead_code)]
-    Anonymous(usize),
-}
-
-impl Params {
-    pub(super) fn len(&self) -> usize {
-        match self {
-            Params::Named(selv, params) => params.len() + if selv.is_some() { 1 } else { 0 },
-            Params::Anonymous(len) => *len,
-        }
-    }
-
-    pub(super) fn is_empty(&self) -> bool {
-        self.len() == 0
-    }
-}
-
-impl Builder {
-    fn should_add_parens(&self, ctx: &CompletionContext) -> bool {
-        if !ctx.config.add_call_parenthesis {
-            return false;
-        }
-        if let Some(PathKind::Use) = ctx.path_kind() {
-            cov_mark::hit!(no_parens_in_use_item);
-            return false;
-        }
-        if matches!(ctx.path_kind(), Some(PathKind::Expr | PathKind::Pat) if ctx.path_is_call())
-            | matches!(
-                ctx.completion_location,
-                Some(ImmediateLocation::MethodCall { has_parens: true, .. })
-            )
-        {
-            return false;
-        }
-
-        // Don't add parentheses if the expected type is some function reference.
-        if let Some(ty) = &ctx.expected_type {
-            if ty.is_fn() {
-                cov_mark::hit!(no_call_parens_if_fn_ptr_needed);
-                return false;
-            }
-        }
-
-        // Nothing prevents us from adding parentheses
-        true
-    }
-
-    pub(super) fn add_call_parens(
-        &mut self,
-        ctx: &CompletionContext,
-        name: SmolStr,
-        params: Params,
-    ) -> &mut Builder {
-        if !self.should_add_parens(ctx) {
-            return self;
-        }
-
-        let cap = match ctx.config.snippet_cap {
-            Some(it) => it,
-            None => return self,
-        };
-        // If not an import, add parenthesis automatically.
-        cov_mark::hit!(inserts_parens_for_function_calls);
-
-        let (snippet, label) = if params.is_empty() {
-            (format!("{}()$0", name), format!("{}()", name))
-        } else {
-            self.trigger_call_info();
-            let snippet = match (ctx.config.add_call_argument_snippets, params) {
-                (true, Params::Named(self_param, params)) => {
-                    let offset = if self_param.is_some() { 2 } else { 1 };
-                    let function_params_snippet = params.iter().enumerate().format_with(
-                        ", ",
-                        |(index, param), f| match param.name(ctx.db) {
-                            Some(n) => {
-                                let smol_str = n.to_smol_str();
-                                let text = smol_str.as_str().trim_start_matches('_');
-                                let ref_ = ref_of_param(ctx, text, param.ty());
-                                f(&format_args!("${{{}:{}{}}}", index + offset, ref_, text))
-                            }
-                            None => f(&format_args!("${{{}:_}}", index + offset,)),
-                        },
-                    );
-                    match self_param {
-                        Some(self_param) => {
-                            format!(
-                                "{}(${{1:{}}}{}{})$0",
-                                name,
-                                self_param.display(ctx.db),
-                                if params.is_empty() { "" } else { ", " },
-                                function_params_snippet
-                            )
-                        }
-                        None => {
-                            format!("{}({})$0", name, function_params_snippet)
-                        }
-                    }
-                }
-                _ => {
-                    cov_mark::hit!(suppress_arg_snippets);
-                    format!("{}($0)", name)
-                }
-            };
-
-            (snippet, format!("{}(…)", name))
-        };
-        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,20 +1,19 @@
 //! Renderer for function calls.
 
 use hir::{db::HirDatabase, AsAssocItem, HirDisplay};
-use ide_db::SymbolKind;
+use ide_db::{SnippetCap, SymbolKind};
 use itertools::Itertools;
 use stdx::format_to;
+use syntax::SmolStr;
 
 use crate::{
-    context::CompletionContext,
+    context::{CompletionContext, PathCompletionCtx, PathKind},
-    item::{CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit},
+    item::{Builder, CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit},
-    render::{
+    patterns::ImmediateLocation,
-        builder_ext::Params, compute_exact_name_match, compute_ref_match, compute_type_match,
+    render::{compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext},
-        RenderContext,
-    },
 };
 
-enum FuncType {
+enum FuncKind {
     Function,
     Method(Option<hir::Name>),
 }
@@ -26,7 +25,7 @@ pub(crate) fn render_fn(
     func: hir::Function,
 ) -> CompletionItem {
     let _p = profile::span("render_fn");
-    render(ctx, local_name, func, FuncType::Function, import_to_add)
+    render(ctx, local_name, func, FuncKind::Function, import_to_add)
 }
 
 pub(crate) fn render_method(
@@ -37,23 +36,22 @@ pub(crate) fn render_method(
     func: hir::Function,
 ) -> CompletionItem {
     let _p = profile::span("render_method");
-    render(ctx, local_name, func, FuncType::Method(receiver), import_to_add)
+    render(ctx, local_name, func, FuncKind::Method(receiver), import_to_add)
 }
 
 fn render(
     ctx @ RenderContext { completion, .. }: RenderContext<'_>,
     local_name: Option<hir::Name>,
     func: hir::Function,
-    func_type: FuncType,
+    func_kind: FuncKind,
     import_to_add: Option<ImportEdit>,
 ) -> CompletionItem {
     let db = completion.db;
 
     let name = local_name.unwrap_or_else(|| func.name(db));
-    let params = params(completion, func, &func_type);
 
-    let call = match &func_type {
+    let call = match &func_kind {
-        FuncType::Method(Some(receiver)) => format!("{}.{}", receiver, &name).into(),
+        FuncKind::Method(Some(receiver)) => format!("{}.{}", receiver, &name).into(),
         _ => name.to_smol_str(),
     };
     let mut item = CompletionItem::new(
@@ -82,7 +80,7 @@ fn render(
         // FIXME
         // For now we don't properly calculate the edits for ref match
         // completions on methods, so we've disabled them. See #8058.
-        if matches!(func_type, FuncType::Function) {
+        if matches!(func_kind, FuncKind::Function) {
             item.ref_match(ref_match);
         }
     }
@@ -90,7 +88,15 @@ fn render(
     item.set_documentation(ctx.docs(func))
         .set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(func))
         .detail(detail(db, func))
-        .add_call_parens(completion, call, params);
+        .lookup_by(name.to_smol_str());
+
+    match completion.config.snippet_cap {
+        Some(cap) if should_add_parens(completion) => {
+            let (self_param, params) = params(completion, func, &func_kind);
+            add_call_parens(&mut item, completion, cap, call, self_param, params);
+        }
+        _ => (),
+    }
 
     if import_to_add.is_none() {
         if let Some(actm) = func.as_assoc_item(db) {
@@ -103,11 +109,116 @@ fn render(
     if let Some(import_to_add) = import_to_add {
         item.add_import(import_to_add);
     }
-    item.lookup_by(name.to_smol_str());
-
     item.build()
 }
 
+pub(super) fn add_call_parens<'b>(
+    builder: &'b mut Builder,
+    ctx: &CompletionContext,
+    cap: SnippetCap,
+    name: SmolStr,
+    self_param: Option<hir::SelfParam>,
+    params: Vec<hir::Param>,
+) -> &'b mut Builder {
+    cov_mark::hit!(inserts_parens_for_function_calls);
+
+    let (snippet, label_suffix) = if self_param.is_none() && params.is_empty() {
+        (format!("{}()$0", name), "()")
+    } else {
+        builder.trigger_call_info();
+        let snippet = if ctx.config.add_call_argument_snippets {
+            let offset = if self_param.is_some() { 2 } else { 1 };
+            let function_params_snippet =
+                params.iter().enumerate().format_with(", ", |(index, param), f| {
+                    match param.name(ctx.db) {
+                        Some(n) => {
+                            let smol_str = n.to_smol_str();
+                            let text = smol_str.as_str().trim_start_matches('_');
+                            let ref_ = ref_of_param(ctx, text, param.ty());
+                            f(&format_args!("${{{}:{}{}}}", index + offset, ref_, text))
+                        }
+                        None => f(&format_args!("${{{}:_}}", index + offset,)),
+                    }
+                });
+            match self_param {
+                Some(self_param) => {
+                    format!(
+                        "{}(${{1:{}}}{}{})$0",
+                        name,
+                        self_param.display(ctx.db),
+                        if params.is_empty() { "" } else { ", " },
+                        function_params_snippet
+                    )
+                }
+                None => {
+                    format!("{}({})$0", name, function_params_snippet)
+                }
+            }
+        } else {
+            cov_mark::hit!(suppress_arg_snippets);
+            format!("{}($0)", name)
+        };
+
+        (snippet, "(…)")
+    };
+    builder.label(SmolStr::from_iter([&name, label_suffix])).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) {
+                return if local.ty(ctx.db) == derefed_ty {
+                    if ty.is_mutable_reference() {
+                        "&mut "
+                    } else {
+                        "&"
+                    }
+                } else {
+                    ""
+                };
+            }
+        }
+    }
+    ""
+}
+
+fn should_add_parens(ctx: &CompletionContext) -> bool {
+    if !ctx.config.add_call_parenthesis {
+        return false;
+    }
+
+    match ctx.path_context {
+        Some(PathCompletionCtx { kind: Some(PathKind::Expr), has_call_parens: true, .. }) => {
+            return false
+        }
+        Some(PathCompletionCtx { kind: Some(PathKind::Use), .. }) => {
+            cov_mark::hit!(no_parens_in_use_item);
+            return false;
+        }
+        _ => {}
+    };
+
+    if matches!(
+        ctx.completion_location,
+        Some(ImmediateLocation::MethodCall { has_parens: true, .. })
+    ) {
+        return false;
+    }
+
+    // Don't add parentheses if the expected type is some function reference.
+    if let Some(ty) = &ctx.expected_type {
+        // FIXME: check signature matches?
+        if ty.is_fn() {
+            cov_mark::hit!(no_call_parens_if_fn_ptr_needed);
+            return false;
+        }
+    }
+
+    // Nothing prevents us from adding parentheses
+    true
+}
+
 fn detail(db: &dyn HirDatabase, func: hir::Function) -> String {
     let ret_ty = func.ret_type(db);
     let mut detail = String::new();
@@ -150,21 +261,17 @@ fn params_display(db: &dyn HirDatabase, func: hir::Function) -> String {
     }
 }
 
-fn params(ctx: &CompletionContext<'_>, func: hir::Function, func_type: &FuncType) -> Params {
+fn params(
-    let (params, self_param) =
+    ctx: &CompletionContext<'_>,
-        if ctx.has_dot_receiver() || matches!(func_type, FuncType::Method(Some(_))) {
+    func: hir::Function,
-            (func.method_params(ctx.db).unwrap_or_default(), None)
+    func_kind: &FuncKind,
+) -> (Option<hir::SelfParam>, Vec<hir::Param>) {
+    let self_param = if ctx.has_dot_receiver() || matches!(func_kind, FuncKind::Method(Some(_))) {
+        None
     } else {
-            let self_param = func.self_param(ctx.db);
+        func.self_param(ctx.db)
-
-            let mut assoc_params = func.assoc_fn_params(ctx.db);
-            if self_param.is_some() {
-                assoc_params.remove(0);
-            }
-            (assoc_params, self_param)
     };
-
+    (self_param, func.params_without_self(ctx.db))
-    Params::Named(self_param, params)
 }
 
 #[cfg(test)]