Merge #893

893: Completion presentation r=matklad a=matklad

Just moves completion code around a bit, to keep logic for producing completions and logic for rendering them into completion items separate.

Co-authored-by: Aleksey Kladov <[email protected]>

13 files changed, 294 insertions, 330 deletions

diff --git a/crates/ra_ide_api/src/completion.rs b/crates/ra_ide_api/src/completion.rs
index d92e01bfb..fbfd7e3e7 100644
--- a/crates/ra_ide_api/src/completion.rs
+++ b/crates/ra_ide_api/src/completion.rs
@@ -1,5 +1,6 @@
 mod completion_item;
 mod completion_context;
+mod presentation;
 
 mod complete_dot;
 mod complete_struct_literal;
@@ -20,7 +21,10 @@ use crate::{
         completion_item::{Completions, CompletionKind},
         completion_context::CompletionContext,
     },
+
 };
+#[cfg(test)]
+use crate::completion::completion_item::{do_completion, check_completion};
 
 pub use crate::completion::completion_item::{CompletionItem, CompletionItemKind, InsertTextFormat};
 
diff --git a/crates/ra_ide_api/src/completion/complete_dot.rs b/crates/ra_ide_api/src/completion/complete_dot.rs
index 20fa323ce..d5ad2e79f 100644
--- a/crates/ra_ide_api/src/completion/complete_dot.rs
+++ b/crates/ra_ide_api/src/completion/complete_dot.rs
@@ -1,7 +1,6 @@
-use hir::{Ty, AdtDef, Docs};
+use hir::{Ty, AdtDef};
 
-use crate::completion::{CompletionContext, Completions, CompletionItem, CompletionItemKind};
-use crate::completion::completion_item::CompletionKind;
+use crate::completion::{CompletionContext, Completions};
 
 /// Complete dot accesses, i.e. fields or methods (currently only fields).
 pub(super) fn complete_dot(acc: &mut Completions, ctx: &CompletionContext) {
@@ -29,15 +28,7 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: Ty)
                 match def_id {
                     AdtDef::Struct(s) => {
                         for field in s.fields(ctx.db) {
-                            CompletionItem::new(
-                                CompletionKind::Reference,
-                                ctx.source_range(),
-                                field.name(ctx.db).to_string(),
-                            )
-                            .kind(CompletionItemKind::Field)
-                            .detail(field.ty(ctx.db).subst(substs).to_string())
-                            .set_documentation(field.docs(ctx.db))
-                            .add_to(acc);
+                            acc.add_field(ctx, field, substs);
                         }
                     }
 
@@ -47,14 +38,7 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: Ty)
             }
             Ty::Tuple(fields) => {
                 for (i, ty) in fields.iter().enumerate() {
-                    CompletionItem::new(
-                        CompletionKind::Reference,
-                        ctx.source_range(),
-                        i.to_string(),
-                    )
-                    .kind(CompletionItemKind::Field)
-                    .detail(ty.to_string())
-                    .add_to(acc);
+                    acc.add_pos_field(ctx, i, ty);
                 }
             }
             _ => {}
@@ -66,14 +50,7 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: Ty
     receiver.iterate_methods(ctx.db, |_ty, func| {
         let sig = func.signature(ctx.db);
         if sig.has_self_param() {
-            CompletionItem::new(
-                CompletionKind::Reference,
-                ctx.source_range(),
-                sig.name().to_string(),
-            )
-            .from_function(ctx, func)
-            .kind(CompletionItemKind::Method)
-            .add_to(acc);
+            acc.add_function(ctx, func);
         }
         None::<()>
     });
@@ -81,8 +58,7 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: Ty
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::*;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{check_completion, CompletionKind};
 
     fn check_ref_completion(name: &str, code: &str) {
         check_completion(name, code, CompletionKind::Reference);
diff --git a/crates/ra_ide_api/src/completion/complete_fn_param.rs b/crates/ra_ide_api/src/completion/complete_fn_param.rs
index 4d6416284..ffdc744b2 100644
--- a/crates/ra_ide_api/src/completion/complete_fn_param.rs
+++ b/crates/ra_ide_api/src/completion/complete_fn_param.rs
@@ -54,8 +54,7 @@ pub(super) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext)
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::*;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{check_completion, CompletionKind};
 
     fn check_magic_completion(name: &str, code: &str) {
         check_completion(name, code, CompletionKind::Magic);
diff --git a/crates/ra_ide_api/src/completion/complete_keyword.rs b/crates/ra_ide_api/src/completion/complete_keyword.rs
index b667381a7..841c0c554 100644
--- a/crates/ra_ide_api/src/completion/complete_keyword.rs
+++ b/crates/ra_ide_api/src/completion/complete_keyword.rs
@@ -109,8 +109,7 @@ fn complete_return(
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::CompletionKind;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{check_completion, CompletionKind};
 
     fn check_keyword_completion(name: &str, code: &str) {
         check_completion(name, code, CompletionKind::Keyword);
diff --git a/crates/ra_ide_api/src/completion/complete_path.rs b/crates/ra_ide_api/src/completion/complete_path.rs
index d337fe970..629a7ee77 100644
--- a/crates/ra_ide_api/src/completion/complete_path.rs
+++ b/crates/ra_ide_api/src/completion/complete_path.rs
@@ -1,9 +1,8 @@
-use join_to_string::join;
-use hir::{Docs, Resolution};
-use ra_syntax::{AstNode, ast::NameOwner};
+use hir::Resolution;
+use ra_syntax::AstNode;
 use test_utils::tested_by;
 
-use crate::completion::{CompletionItem, CompletionItemKind, Completions, CompletionKind, CompletionContext};
+use crate::completion::{Completions, CompletionContext};
 
 pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) {
     let path = match &ctx.path_prefix {
@@ -28,79 +27,28 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) {
                         }
                     }
                 }
-
-                CompletionItem::new(
-                    CompletionKind::Reference,
-                    ctx.source_range(),
-                    name.to_string(),
-                )
-                .from_resolution(ctx, &res.def.map(hir::Resolution::Def))
-                .add_to(acc);
+                acc.add_resolution(ctx, name.to_string(), &res.def.map(hir::Resolution::Def));
             }
         }
         hir::ModuleDef::Enum(e) => {
-            e.variants(ctx.db).into_iter().for_each(|variant| {
-                if let Some(name) = variant.name(ctx.db) {
-                    let detail_types =
-                        variant.fields(ctx.db).into_iter().map(|field| field.ty(ctx.db));
-                    let detail =
-                        join(detail_types).separator(", ").surround_with("(", ")").to_string();
-
-                    CompletionItem::new(
-                        CompletionKind::Reference,
-                        ctx.source_range(),
-                        name.to_string(),
-                    )
-                    .kind(CompletionItemKind::EnumVariant)
-                    .set_documentation(variant.docs(ctx.db))
-                    .set_detail(Some(detail))
-                    .add_to(acc)
-                }
-            });
+            for variant in e.variants(ctx.db) {
+                acc.add_enum_variant(ctx, variant);
+            }
         }
         hir::ModuleDef::Struct(s) => {
             let ty = s.ty(ctx.db);
-            ty.iterate_impl_items(ctx.db, |item| match item {
-                hir::ImplItem::Method(func) => {
-                    let sig = func.signature(ctx.db);
-                    if !sig.has_self_param() {
-                        CompletionItem::new(
-                            CompletionKind::Reference,
-                            ctx.source_range(),
-                            sig.name().to_string(),
-                        )
-                        .from_function(ctx, func)
-                        .kind(CompletionItemKind::Method)
-                        .add_to(acc);
-                    }
-                    None::<()>
-                }
-                hir::ImplItem::Const(ct) => {
-                    let source = ct.source(ctx.db);
-                    if let Some(name) = source.1.name() {
-                        CompletionItem::new(
-                            CompletionKind::Reference,
-                            ctx.source_range(),
-                            name.text().to_string(),
-                        )
-                        .from_const(ctx, ct)
-                        .add_to(acc);
-                    }
-                    None::<()>
-                }
-                hir::ImplItem::Type(ty) => {
-                    let source = ty.source(ctx.db);
-                    if let Some(name) = source.1.name() {
-                        CompletionItem::new(
-                            CompletionKind::Reference,
-                            ctx.source_range(),
-                            name.text().to_string(),
-                        )
-                        .from_type(ctx, ty)
-                        .add_to(acc);
+            ty.iterate_impl_items(ctx.db, |item| {
+                match item {
+                    hir::ImplItem::Method(func) => {
+                        let sig = func.signature(ctx.db);
+                        if !sig.has_self_param() {
+                            acc.add_function(ctx, func);
+                        }
                     }
-                    None::<()>
+                    hir::ImplItem::Const(ct) => acc.add_const(ctx, ct),
+                    hir::ImplItem::Type(ty) => acc.add_type(ctx, ty),
                 }
+                None::<()>
             });
         }
         _ => return,
@@ -109,13 +57,10 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) {
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::{
-        CompletionKind,
-        completion_item::{check_completion, do_completion},
-};
-
     use test_utils::covers;
 
+    use crate::completion::{CompletionKind, check_completion, do_completion};
+
     fn check_reference_completion(code: &str, expected_completions: &str) {
         check_completion(code, expected_completions, CompletionKind::Reference);
     }
diff --git a/crates/ra_ide_api/src/completion/complete_postfix.rs b/crates/ra_ide_api/src/completion/complete_postfix.rs
index ce3d6ed3c..4dfa5f176 100644
--- a/crates/ra_ide_api/src/completion/complete_postfix.rs
+++ b/crates/ra_ide_api/src/completion/complete_postfix.rs
@@ -56,8 +56,7 @@ pub(super) fn complete_postfix(acc: &mut Completions, ctx: &CompletionContext) {
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::completion_item::CompletionKind;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{CompletionKind, check_completion};
 
     fn check_snippet_completion(test_name: &str, code: &str) {
         check_completion(test_name, code, CompletionKind::Postfix);
diff --git a/crates/ra_ide_api/src/completion/complete_scope.rs b/crates/ra_ide_api/src/completion/complete_scope.rs
index eeaf26d93..6146b7bb6 100644
--- a/crates/ra_ide_api/src/completion/complete_scope.rs
+++ b/crates/ra_ide_api/src/completion/complete_scope.rs
@@ -1,4 +1,4 @@
-use crate::completion::{CompletionItem, Completions, CompletionKind, CompletionContext};
+use crate::completion::{Completions, CompletionContext};
 
 pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) {
     if !ctx.is_trivial_path {
@@ -6,17 +6,12 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) {
     }
     let names = ctx.resolver.all_names(ctx.db);
 
-    names.into_iter().for_each(|(name, res)| {
-        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.to_string())
-            .from_resolution(ctx, &res)
-            .add_to(acc)
-    });
+    names.into_iter().for_each(|(name, res)| acc.add_resolution(ctx, name.to_string(), &res));
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::CompletionKind;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{CompletionKind, check_completion};
 
     fn check_reference_completion(name: &str, code: &str) {
         check_completion(name, code, CompletionKind::Reference);
diff --git a/crates/ra_ide_api/src/completion/complete_snippet.rs b/crates/ra_ide_api/src/completion/complete_snippet.rs
index aa3a16fa2..e1df9e625 100644
--- a/crates/ra_ide_api/src/completion/complete_snippet.rs
+++ b/crates/ra_ide_api/src/completion/complete_snippet.rs
@@ -36,8 +36,7 @@ fn ${1:feature}() {
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::CompletionKind;
-    use crate::completion::completion_item::check_completion;
+    use crate::completion::{CompletionKind, check_completion};
 
     fn check_snippet_completion(name: &str, code: &str) {
         check_completion(name, code, CompletionKind::Snippet);
diff --git a/crates/ra_ide_api/src/completion/complete_struct_literal.rs b/crates/ra_ide_api/src/completion/complete_struct_literal.rs
index f8dd2baad..afb092f59 100644
--- a/crates/ra_ide_api/src/completion/complete_struct_literal.rs
+++ b/crates/ra_ide_api/src/completion/complete_struct_literal.rs
@@ -1,7 +1,6 @@
-use hir::{Ty, AdtDef, Docs};
+use hir::{Ty, AdtDef};
 
-use crate::completion::{CompletionContext, Completions, CompletionItem, CompletionItemKind};
-use crate::completion::completion_item::CompletionKind;
+use crate::completion::{CompletionContext, Completions};
 
 /// Complete fields in fields literals.
 pub(super) fn complete_struct_literal(acc: &mut Completions, ctx: &CompletionContext) {
@@ -23,15 +22,7 @@ pub(super) fn complete_struct_literal(acc: &mut Completions, ctx: &CompletionCon
     match adt {
         AdtDef::Struct(s) => {
             for field in s.fields(ctx.db) {
-                CompletionItem::new(
-                    CompletionKind::Reference,
-                    ctx.source_range(),
-                    field.name(ctx.db).to_string(),
-                )
-                .kind(CompletionItemKind::Field)
-                .detail(field.ty(ctx.db).subst(substs).to_string())
-                .set_documentation(field.docs(ctx.db))
-                .add_to(acc);
+                acc.add_field(ctx, field, substs);
             }
         }
 
@@ -43,10 +34,10 @@ pub(super) fn complete_struct_literal(acc: &mut Completions, ctx: &CompletionCon
 #[cfg(test)]
 mod tests {
     use insta::assert_debug_snapshot_matches;
-    use crate::completion::{CompletionItem, CompletionKind};
+    use crate::completion::{CompletionItem, CompletionKind, do_completion};
 
     fn complete(code: &str) -> Vec<CompletionItem> {
-        crate::completion::completion_item::do_completion(code, CompletionKind::Reference)
+        do_completion(code, CompletionKind::Reference)
     }
 
     #[test]
diff --git a/crates/ra_ide_api/src/completion/completion_item.rs b/crates/ra_ide_api/src/completion/completion_item.rs
index 95bdd59b4..f515fcc14 100644
--- a/crates/ra_ide_api/src/completion/completion_item.rs
+++ b/crates/ra_ide_api/src/completion/completion_item.rs
@@ -1,16 +1,8 @@
 use std::fmt;
 
-use hir::{Docs, Documentation, PerNs, Resolution};
+use hir::Documentation;
 use ra_syntax::TextRange;
-use ra_text_edit::{ TextEditBuilder, TextEdit};
-use test_utils::tested_by;
-
-use crate::completion::{
-    completion_context::CompletionContext,
-    function_label,
-    const_label,
-    type_label
-};
+use ra_text_edit::{TextEditBuilder, TextEdit};
 
 /// `CompletionItem` describes a single completion variant in the editor pop-up.
 /// It is basically a POD with various properties. To construct a
@@ -255,91 +247,6 @@ impl Builder {
         self.documentation = docs.map(Into::into);
         self
     }
-    pub(super) fn from_resolution(
-        mut self,
-        ctx: &CompletionContext,
-        resolution: &PerNs<Resolution>,
-    ) -> Builder {
-        use hir::ModuleDef::*;
-
-        let def = resolution.as_ref().take_types().or_else(|| resolution.as_ref().take_values());
-        let def = match def {
-            None => return self,
-            Some(it) => it,
-        };
-        let (kind, docs) = match def {
-            Resolution::Def(Module(it)) => (CompletionItemKind::Module, it.docs(ctx.db)),
-            Resolution::Def(Function(func)) => return self.from_function(ctx, *func),
-            Resolution::Def(Struct(it)) => (CompletionItemKind::Struct, it.docs(ctx.db)),
-            Resolution::Def(Enum(it)) => (CompletionItemKind::Enum, it.docs(ctx.db)),
-            Resolution::Def(EnumVariant(it)) => (CompletionItemKind::EnumVariant, it.docs(ctx.db)),
-            Resolution::Def(Const(it)) => (CompletionItemKind::Const, it.docs(ctx.db)),
-            Resolution::Def(Static(it)) => (CompletionItemKind::Static, it.docs(ctx.db)),
-            Resolution::Def(Trait(it)) => (CompletionItemKind::Trait, it.docs(ctx.db)),
-            Resolution::Def(Type(it)) => (CompletionItemKind::TypeAlias, it.docs(ctx.db)),
-            Resolution::GenericParam(..) => (CompletionItemKind::TypeParam, None),
-            Resolution::LocalBinding(..) => (CompletionItemKind::Binding, None),
-            Resolution::SelfType(..) => (
-                CompletionItemKind::TypeParam, // (does this need its own kind?)
-                None,
-            ),
-        };
-        self.kind = Some(kind);
-        self.documentation = docs;
-
-        self
-    }
-
-    pub(super) fn from_function(
-        mut self,
-        ctx: &CompletionContext,
-        function: hir::Function,
-    ) -> Builder {
-        // If not an import, add parenthesis automatically.
-        if ctx.use_item_syntax.is_none() && !ctx.is_call {
-            tested_by!(inserts_parens_for_function_calls);
-            let sig = function.signature(ctx.db);
-            if sig.params().is_empty() || sig.has_self_param() && sig.params().len() == 1 {
-                self.insert_text = Some(format!("{}()$0", self.label));
-            } else {
-                self.insert_text = Some(format!("{}($0)", self.label));
-            }
-            self.insert_text_format = InsertTextFormat::Snippet;
-        }
-
-        if let Some(docs) = function.docs(ctx.db) {
-            self.documentation = Some(docs);
-        }
-
-        if let Some(label) = function_item_label(ctx, function) {
-            self.detail = Some(label);
-        }
-
-        self.kind = Some(CompletionItemKind::Function);
-        self
-    }
-
-    pub(super) fn from_const(mut self, ctx: &CompletionContext, ct: hir::Const) -> Builder {
-        if let Some(docs) = ct.docs(ctx.db) {
-            self.documentation = Some(docs);
-        }
-
-        self.detail = Some(const_item_label(ctx, ct));
-        self.kind = Some(CompletionItemKind::Const);
-
-        self
-    }
-
-    pub(super) fn from_type(mut self, ctx: &CompletionContext, ty: hir::Type) -> Builder {
-        if let Some(docs) = ty.docs(ctx.db) {
-            self.documentation = Some(docs);
-        }
-
-        self.detail = Some(type_item_label(ctx, ty));
-        self.kind = Some(CompletionItemKind::TypeAlias);
-
-        self
-    }
 }
 
 impl<'a> Into<CompletionItem> for Builder {
@@ -373,21 +280,6 @@ impl Into<Vec<CompletionItem>> for Completions {
     }
 }
 
-fn function_item_label(ctx: &CompletionContext, function: hir::Function) -> Option<String> {
-    let node = function.source(ctx.db).1;
-    function_label(&node)
-}
-
-fn const_item_label(ctx: &CompletionContext, ct: hir::Const) -> String {
-    let node = ct.source(ctx.db).1;
-    const_label(&node)
-}
-
-fn type_item_label(ctx: &CompletionContext, ty: hir::Type) -> String {
-    let node = ty.source(ctx.db).1;
-    type_label(&node)
-}
-
 #[cfg(test)]
 pub(crate) fn do_completion(code: &str, kind: CompletionKind) -> Vec<CompletionItem> {
     use crate::mock_analysis::{single_file_with_position, analysis_and_position};
@@ -411,83 +303,3 @@ pub(crate) fn check_completion(test_name: &str, code: &str, kind: CompletionKind
     let kind_completions = do_completion(code, kind);
     assert_debug_snapshot_matches!(test_name, kind_completions);
 }
-
-#[cfg(test)]
-mod tests {
-    use test_utils::covers;
-
-    use super::*;
-
-    fn check_reference_completion(code: &str, expected_completions: &str) {
-        check_completion(code, expected_completions, CompletionKind::Reference);
-    }
-
-    #[test]
-    fn inserts_parens_for_function_calls() {
-        covers!(inserts_parens_for_function_calls);
-        check_reference_completion(
-            "inserts_parens_for_function_calls1",
-            r"
-            fn no_args() {}
-            fn main() { no_<|> }
-            ",
-        );
-        check_reference_completion(
-            "inserts_parens_for_function_calls2",
-            r"
-            fn with_args(x: i32, y: String) {}
-            fn main() { with_<|> }
-            ",
-        );
-        check_reference_completion(
-            "inserts_parens_for_function_calls3",
-            r"
-            struct S {}
-            impl S {
-                fn foo(&self) {}
-            }
-            fn bar(s: &S) {
-                s.f<|>
-            }
-            ",
-        )
-    }
-
-    #[test]
-    fn dont_render_function_parens_in_use_item() {
-        check_reference_completion(
-            "dont_render_function_parens_in_use_item",
-            "
-            //- /lib.rs
-            mod m { pub fn foo() {} }
-            use crate::m::f<|>;
-            ",
-        )
-    }
-
-    #[test]
-    fn dont_render_function_parens_if_already_call() {
-        check_reference_completion(
-            "dont_render_function_parens_if_already_call",
-            "
-            //- /lib.rs
-            fn frobnicate() {}
-            fn main() {
-                frob<|>();
-            }
-            ",
-        );
-        check_reference_completion(
-            "dont_render_function_parens_if_already_call_assoc_fn",
-            "
-            //- /lib.rs
-            struct Foo {}
-            impl Foo { fn new() -> Foo {} }
-            fn main() {
-                Foo::ne<|>();
-            }
-            ",
-        )
-    }
-
-}
diff --git a/crates/ra_ide_api/src/completion/presentation.rs b/crates/ra_ide_api/src/completion/presentation.rs
new file mode 100644
index 000000000..6454436c9
--- /dev/null
+++ b/crates/ra_ide_api/src/completion/presentation.rs
@@ -0,0 +1,245 @@
+//! This modules takes care of rendering various defenitions as completion items.
+use join_to_string::join;
+use test_utils::tested_by;
+use hir::{Docs, PerNs, Resolution};
+use ra_syntax::ast::NameOwner;
+
+use crate::completion::{
+    Completions, CompletionKind, CompletionItemKind, CompletionContext, CompletionItem,
+    function_label, const_label, type_label,
+};
+
+impl Completions {
+    pub(crate) fn add_field(
+        &mut self,
+        ctx: &CompletionContext,
+        field: hir::StructField,
+        substs: &hir::Substs,
+    ) {
+        CompletionItem::new(
+            CompletionKind::Reference,
+            ctx.source_range(),
+            field.name(ctx.db).to_string(),
+        )
+        .kind(CompletionItemKind::Field)
+        .detail(field.ty(ctx.db).subst(substs).to_string())
+        .set_documentation(field.docs(ctx.db))
+        .add_to(self);
+    }
+
+    pub(crate) fn add_pos_field(&mut self, ctx: &CompletionContext, field: usize, ty: &hir::Ty) {
+        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), field.to_string())
+            .kind(CompletionItemKind::Field)
+            .detail(ty.to_string())
+            .add_to(self);
+    }
+
+    pub(crate) fn add_resolution(
+        &mut self,
+        ctx: &CompletionContext,
+        local_name: String,
+        resolution: &PerNs<Resolution>,
+    ) {
+        use hir::ModuleDef::*;
+
+        let def = resolution.as_ref().take_types().or_else(|| resolution.as_ref().take_values());
+        let def = match def {
+            None => {
+                self.add(CompletionItem::new(
+                    CompletionKind::Reference,
+                    ctx.source_range(),
+                    local_name,
+                ));
+                return;
+            }
+            Some(it) => it,
+        };
+        let (kind, docs) = match def {
+            Resolution::Def(Module(it)) => (CompletionItemKind::Module, it.docs(ctx.db)),
+            Resolution::Def(Function(func)) => {
+                return self.add_function_with_name(ctx, Some(local_name), *func);
+            }
+            Resolution::Def(Struct(it)) => (CompletionItemKind::Struct, it.docs(ctx.db)),
+            Resolution::Def(Enum(it)) => (CompletionItemKind::Enum, it.docs(ctx.db)),
+            Resolution::Def(EnumVariant(it)) => (CompletionItemKind::EnumVariant, it.docs(ctx.db)),
+            Resolution::Def(Const(it)) => (CompletionItemKind::Const, it.docs(ctx.db)),
+            Resolution::Def(Static(it)) => (CompletionItemKind::Static, it.docs(ctx.db)),
+            Resolution::Def(Trait(it)) => (CompletionItemKind::Trait, it.docs(ctx.db)),
+            Resolution::Def(Type(it)) => (CompletionItemKind::TypeAlias, it.docs(ctx.db)),
+            Resolution::GenericParam(..) => (CompletionItemKind::TypeParam, None),
+            Resolution::LocalBinding(..) => (CompletionItemKind::Binding, None),
+            Resolution::SelfType(..) => (
+                CompletionItemKind::TypeParam, // (does this need its own kind?)
+                None,
+            ),
+        };
+        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), local_name)
+            .kind(kind)
+            .set_documentation(docs)
+            .add_to(self)
+    }
+
+    pub(crate) fn add_function(&mut self, ctx: &CompletionContext, func: hir::Function) {
+        self.add_function_with_name(ctx, None, func)
+    }
+
+    fn add_function_with_name(
+        &mut self,
+        ctx: &CompletionContext,
+        name: Option<String>,
+        func: hir::Function,
+    ) {
+        let sig = func.signature(ctx.db);
+        let name = name.unwrap_or_else(|| sig.name().to_string());
+        let (_, ast_node) = func.source(ctx.db);
+        let detail = function_label(&ast_node);
+
+        let mut builder = CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name)
+            .kind(if sig.has_self_param() {
+                CompletionItemKind::Method
+            } else {
+                CompletionItemKind::Function
+            })
+            .set_documentation(func.docs(ctx.db))
+            .set_detail(detail);
+        // If not an import, add parenthesis automatically.
+        if ctx.use_item_syntax.is_none() && !ctx.is_call {
+            tested_by!(inserts_parens_for_function_calls);
+            let snippet =
+                if sig.params().is_empty() || sig.has_self_param() && sig.params().len() == 1 {
+                    format!("{}()$0", sig.name())
+                } else {
+                    format!("{}($0)", sig.name())
+                };
+            builder = builder.insert_snippet(snippet);
+        }
+        self.add(builder)
+    }
+
+    pub(crate) fn add_const(&mut self, ctx: &CompletionContext, constant: hir::Const) {
+        let (_file_id, ast_node) = constant.source(ctx.db);
+        let name = match ast_node.name() {
+            Some(name) => name,
+            _ => return,
+        };
+        let (_, ast_node) = constant.source(ctx.db);
+        let detail = const_label(&ast_node);
+
+        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.text().to_string())
+            .kind(CompletionItemKind::Const)
+            .set_documentation(constant.docs(ctx.db))
+            .detail(detail)
+            .add_to(self);
+    }
+
+    pub(crate) fn add_type(&mut self, ctx: &CompletionContext, type_alias: hir::Type) {
+        let (_file_id, type_def) = type_alias.source(ctx.db);
+        let name = match type_def.name() {
+            Some(name) => name,
+            _ => return,
+        };
+        let (_, ast_node) = type_alias.source(ctx.db);
+        let detail = type_label(&ast_node);
+
+        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.text().to_string())
+            .kind(CompletionItemKind::TypeAlias)
+            .set_documentation(type_alias.docs(ctx.db))
+            .detail(detail)
+            .add_to(self);
+    }
+
+    pub(crate) fn add_enum_variant(&mut self, ctx: &CompletionContext, variant: hir::EnumVariant) {
+        let name = match variant.name(ctx.db) {
+            Some(it) => it,
+            None => return,
+        };
+        let detail_types = variant.fields(ctx.db).into_iter().map(|field| field.ty(ctx.db));
+        let detail = join(detail_types).separator(", ").surround_with("(", ")").to_string();
+
+        CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.to_string())
+            .kind(CompletionItemKind::EnumVariant)
+            .set_documentation(variant.docs(ctx.db))
+            .detail(detail)
+            .add_to(self);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use test_utils::covers;
+
+    use crate::completion::{CompletionKind, check_completion};
+
+    fn check_reference_completion(code: &str, expected_completions: &str) {
+        check_completion(code, expected_completions, CompletionKind::Reference);
+    }
+
+    #[test]
+    fn inserts_parens_for_function_calls() {
+        covers!(inserts_parens_for_function_calls);
+        check_reference_completion(
+            "inserts_parens_for_function_calls1",
+            r"
+            fn no_args() {}
+            fn main() { no_<|> }
+            ",
+        );
+        check_reference_completion(
+            "inserts_parens_for_function_calls2",
+            r"
+            fn with_args(x: i32, y: String) {}
+            fn main() { with_<|> }
+            ",
+        );
+        check_reference_completion(
+            "inserts_parens_for_function_calls3",
+            r"
+            struct S {}
+            impl S {
+                fn foo(&self) {}
+            }
+            fn bar(s: &S) {
+                s.f<|>
+            }
+            ",
+        )
+    }
+
+    #[test]
+    fn dont_render_function_parens_in_use_item() {
+        check_reference_completion(
+            "dont_render_function_parens_in_use_item",
+            "
+            //- /lib.rs
+            mod m { pub fn foo() {} }
+            use crate::m::f<|>;
+            ",
+        )
+    }
+
+    #[test]
+    fn dont_render_function_parens_if_already_call() {
+        check_reference_completion(
+            "dont_render_function_parens_if_already_call",
+            "
+            //- /lib.rs
+            fn frobnicate() {}
+            fn main() {
+                frob<|>();
+            }
+            ",
+        );
+        check_reference_completion(
+            "dont_render_function_parens_if_already_call_assoc_fn",
+            "
+            //- /lib.rs
+            struct Foo {}
+            impl Foo { fn new() -> Foo {} }
+            fn main() {
+                Foo::ne<|>();
+            }
+            ",
+        )
+    }
+
+}
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_render_function_parens_if_already_call_assoc_fn.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_render_function_parens_if_already_call_assoc_fn.snap
index 39fff2cb7..19375ea95 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_render_function_parens_if_already_call_assoc_fn.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_render_function_parens_if_already_call_assoc_fn.snap
@@ -1,6 +1,6 @@
 ---
-created: "2019-02-18T09:22:24.268227065Z"
-creator: [email protected].2
+created: "2019-02-24T16:33:48.008220694Z"
+creator: [email protected].3
 source: crates/ra_ide_api/src/completion/completion_item.rs
 expression: kind_completions
 ---
@@ -10,7 +10,7 @@ expression: kind_completions
         source_range: [67; 69),
         delete: [67; 69),
         insert: "new",
-        kind: Method,
+        kind: Function,
         detail: "fn new() -> Foo"
     }
 ]
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__struct_associated_method.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__struct_associated_method.snap
index 163b325c3..7c69eebeb 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__struct_associated_method.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__struct_associated_method.snap
@@ -1,6 +1,6 @@
 ---
-created: "2019-02-18T09:22:24.093082839Z"
-creator: [email protected].2
+created: "2019-02-24T16:33:47.990111169Z"
+creator: [email protected].3
 source: crates/ra_ide_api/src/completion/completion_item.rs
 expression: kind_completions
 ---
@@ -10,7 +10,7 @@ expression: kind_completions
         source_range: [100; 100),
         delete: [100; 100),
         insert: "m()$0",
-        kind: Method,
+        kind: Function,
         detail: "fn m()",
         documentation: Documentation(
             "An associated method"