Merge #1970

1970: Add <> when completing generics r=matklad a=matklad

@flodiebold wdyt? Is it correct that we always need to add `<>` in generic types? 

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

5 files changed, 222 insertions, 96 deletions

diff --git a/crates/ra_hir/src/code_model/docs.rs b/crates/ra_hir/src/code_model/docs.rs
index 9675e397f..8533b4f5e 100644
--- a/crates/ra_hir/src/code_model/docs.rs
+++ b/crates/ra_hir/src/code_model/docs.rs
@@ -6,21 +6,19 @@ use ra_syntax::ast;
 
 use crate::{
     db::{AstDatabase, DefDatabase, HirDatabase},
-    Const, Enum, EnumVariant, FieldSource, Function, HasSource, MacroDef, Module, Static, Struct,
-    StructField, Trait, TypeAlias, Union,
+    Adt, Const, Enum, EnumVariant, FieldSource, Function, HasSource, MacroDef, Module, Static,
+    Struct, StructField, Trait, TypeAlias, Union,
 };
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum DocDef {
     Module(Module),
     StructField(StructField),
-    Struct(Struct),
-    Enum(Enum),
+    Adt(Adt),
     EnumVariant(EnumVariant),
     Static(Static),
     Const(Const),
     Function(Function),
-    Union(Union),
     Trait(Trait),
     TypeAlias(TypeAlias),
     MacroDef(MacroDef),
@@ -29,13 +27,11 @@ pub enum DocDef {
 impl_froms!(
     DocDef: Module,
     StructField,
-    Struct,
-    Enum,
+    Adt(Struct, Enum, Union),
     EnumVariant,
     Static,
     Const,
     Function,
-    Union,
     Trait,
     TypeAlias,
     MacroDef
@@ -79,13 +75,15 @@ pub(crate) fn documentation_query(
             FieldSource::Named(named) => docs_from_ast(&named),
             FieldSource::Pos(..) => None,
         },
-        DocDef::Struct(it) => docs_from_ast(&it.source(db).ast),
-        DocDef::Enum(it) => docs_from_ast(&it.source(db).ast),
+        DocDef::Adt(it) => match it {
+            Adt::Struct(it) => docs_from_ast(&it.source(db).ast),
+            Adt::Enum(it) => docs_from_ast(&it.source(db).ast),
+            Adt::Union(it) => docs_from_ast(&it.source(db).ast),
+        },
         DocDef::EnumVariant(it) => docs_from_ast(&it.source(db).ast),
         DocDef::Static(it) => docs_from_ast(&it.source(db).ast),
         DocDef::Const(it) => docs_from_ast(&it.source(db).ast),
         DocDef::Function(it) => docs_from_ast(&it.source(db).ast),
-        DocDef::Union(it) => docs_from_ast(&it.source(db).ast),
         DocDef::Trait(it) => docs_from_ast(&it.source(db).ast),
         DocDef::TypeAlias(it) => docs_from_ast(&it.source(db).ast),
         DocDef::MacroDef(it) => docs_from_ast(&it.source(db).ast),
diff --git a/crates/ra_ide_api/src/completion/complete_scope.rs b/crates/ra_ide_api/src/completion/complete_scope.rs
index 38a6c3d37..84826cdcc 100644
--- a/crates/ra_ide_api/src/completion/complete_scope.rs
+++ b/crates/ra_ide_api/src/completion/complete_scope.rs
@@ -290,22 +290,24 @@ mod tests {
                 }
                 "
             ),
-            @r###"[
-    CompletionItem {
-        label: "T",
-        source_range: [54; 54),
-        delete: [54; 54),
-        insert: "T",
-        kind: TypeParam,
-    },
-    CompletionItem {
-        label: "X",
-        source_range: [54; 54),
-        delete: [54; 54),
-        insert: "X",
-        kind: Struct,
-    },
-]"###
+            @r###"
+        [
+            CompletionItem {
+                label: "T",
+                source_range: [54; 54),
+                delete: [54; 54),
+                insert: "T",
+                kind: TypeParam,
+            },
+            CompletionItem {
+                label: "X",
+                source_range: [54; 54),
+                delete: [54; 54),
+                insert: "X<$0>",
+                kind: Struct,
+            },
+        ]
+        "###
         );
     }
 
@@ -319,22 +321,24 @@ mod tests {
                 }
                 "
             ),
-            @r###"[
-    CompletionItem {
-        label: "Self",
-        source_range: [48; 48),
-        delete: [48; 48),
-        insert: "Self",
-        kind: TypeParam,
-    },
-    CompletionItem {
-        label: "X",
-        source_range: [48; 48),
-        delete: [48; 48),
-        insert: "X",
-        kind: Enum,
-    },
-]"###
+            @r###"
+        [
+            CompletionItem {
+                label: "Self",
+                source_range: [48; 48),
+                delete: [48; 48),
+                insert: "Self",
+                kind: TypeParam,
+            },
+            CompletionItem {
+                label: "X",
+                source_range: [48; 48),
+                delete: [48; 48),
+                insert: "X",
+                kind: Enum,
+            },
+        ]
+        "###
         );
     }
 
@@ -442,23 +446,25 @@ mod tests {
                 fn x() -> <|>
                 "
             ),
-            @r###"[
-    CompletionItem {
-        label: "Foo",
-        source_range: [55; 55),
-        delete: [55; 55),
-        insert: "Foo",
-        kind: Struct,
-    },
-    CompletionItem {
-        label: "x",
-        source_range: [55; 55),
-        delete: [55; 55),
-        insert: "x()$0",
-        kind: Function,
-        detail: "fn x()",
-    },
-]"###
+            @r###"
+        [
+            CompletionItem {
+                label: "Foo",
+                source_range: [55; 55),
+                delete: [55; 55),
+                insert: "Foo",
+                kind: Struct,
+            },
+            CompletionItem {
+                label: "x",
+                source_range: [55; 55),
+                delete: [55; 55),
+                insert: "x()$0",
+                kind: Function,
+                detail: "fn x()",
+            },
+        ]
+        "###
         );
     }
 
@@ -538,30 +544,32 @@ mod tests {
                 }
                 "
             ),
-            @r#"[
-    CompletionItem {
-        label: "Option",
-        source_range: [18; 18),
-        delete: [18; 18),
-        insert: "Option",
-        kind: Struct,
-    },
-    CompletionItem {
-        label: "foo",
-        source_range: [18; 18),
-        delete: [18; 18),
-        insert: "foo()$0",
-        kind: Function,
-        detail: "fn foo()",
-    },
-    CompletionItem {
-        label: "std",
-        source_range: [18; 18),
-        delete: [18; 18),
-        insert: "std",
-        kind: Module,
-    },
-]"#
+            @r###"
+        [
+            CompletionItem {
+                label: "Option",
+                source_range: [18; 18),
+                delete: [18; 18),
+                insert: "Option",
+                kind: Struct,
+            },
+            CompletionItem {
+                label: "foo",
+                source_range: [18; 18),
+                delete: [18; 18),
+                insert: "foo()$0",
+                kind: Function,
+                detail: "fn foo()",
+            },
+            CompletionItem {
+                label: "std",
+                source_range: [18; 18),
+                delete: [18; 18),
+                insert: "std",
+                kind: Module,
+            },
+        ]
+        "###
         );
     }
 
diff --git a/crates/ra_ide_api/src/completion/completion_context.rs b/crates/ra_ide_api/src/completion/completion_context.rs
index e9ad06965..73f3f3960 100644
--- a/crates/ra_ide_api/src/completion/completion_context.rs
+++ b/crates/ra_ide_api/src/completion/completion_context.rs
@@ -40,6 +40,8 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) dot_receiver: Option<ast::Expr>,
     /// If this is a call (method or function) in particular, i.e. the () are already there.
     pub(super) is_call: bool,
+    pub(super) is_path_type: bool,
+    pub(super) has_type_args: bool,
 }
 
 impl<'a> CompletionContext<'a> {
@@ -76,6 +78,8 @@ impl<'a> CompletionContext<'a> {
             is_new_item: false,
             dot_receiver: None,
             is_call: false,
+            is_path_type: false,
+            has_type_args: false,
         };
         ctx.fill(&original_parse, position.offset);
         Some(ctx)
@@ -176,6 +180,9 @@ impl<'a> CompletionContext<'a> {
                 .and_then(|it| it.syntax().parent().and_then(ast::CallExpr::cast))
                 .is_some();
 
+            self.is_path_type = path.syntax().parent().and_then(ast::PathType::cast).is_some();
+            self.has_type_args = segment.type_arg_list().is_some();
+
             if let Some(mut path) = hir::Path::from_ast(path.clone()) {
                 if !path.is_ident() {
                     path.segments.pop().unwrap();
diff --git a/crates/ra_ide_api/src/completion/presentation.rs b/crates/ra_ide_api/src/completion/presentation.rs
index eb480a775..400a266a2 100644
--- a/crates/ra_ide_api/src/completion/presentation.rs
+++ b/crates/ra_ide_api/src/completion/presentation.rs
@@ -1,12 +1,12 @@
 //! This modules takes care of rendering various definitions as completion items.
 
-use hir::{Docs, HasSource, HirDisplay, ScopeDef, Ty, TypeWalk};
+use hir::{db::HirDatabase, Docs, HasSource, HirDisplay, ScopeDef, Ty, TypeWalk};
 use join_to_string::join;
 use ra_syntax::ast::NameOwner;
 use test_utils::tested_by;
 
 use crate::completion::{
-    CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, Completions,
+    db, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, Completions,
 };
 
 use crate::display::{const_label, function_label, macro_label, type_label};
@@ -50,14 +50,8 @@ impl Completions {
             ScopeDef::ModuleDef(Function(func)) => {
                 return self.add_function_with_name(ctx, Some(local_name), *func);
             }
-            ScopeDef::ModuleDef(Adt(hir::Adt::Struct(it))) => {
-                (CompletionItemKind::Struct, it.docs(ctx.db))
-            }
-            ScopeDef::ModuleDef(Adt(hir::Adt::Union(it))) => {
-                (CompletionItemKind::Struct, it.docs(ctx.db))
-            }
-            ScopeDef::ModuleDef(Adt(hir::Adt::Enum(it))) => {
-                (CompletionItemKind::Enum, it.docs(ctx.db))
+            ScopeDef::ModuleDef(Adt(adt)) => {
+                return self.add_adt_with_name(ctx, local_name, *adt);
             }
             ScopeDef::ModuleDef(EnumVariant(it)) => {
                 (CompletionItemKind::EnumVariant, it.docs(ctx.db))
@@ -156,7 +150,8 @@ impl Completions {
             })
             .set_documentation(func.docs(ctx.db))
             .detail(detail);
-        // If not an import, add parenthesis automatically.
+
+        // Add `<>` for generic types
         if ctx.use_item_syntax.is_none()
             && !ctx.is_call
             && ctx.db.feature_flags.get("completion.insertion.add-call-parenthesis")
@@ -170,9 +165,36 @@ impl Completions {
                 };
             builder = builder.insert_snippet(snippet);
         }
+
         self.add(builder)
     }
 
+    fn add_adt_with_name(&mut self, ctx: &CompletionContext, name: String, adt: hir::Adt) {
+        let mut builder =
+            CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.clone());
+
+        let kind = match adt {
+            hir::Adt::Struct(_) => CompletionItemKind::Struct,
+            // FIXME: add CompletionItemKind::Union
+            hir::Adt::Union(_) => CompletionItemKind::Struct,
+            hir::Adt::Enum(_) => CompletionItemKind::Enum,
+        };
+        let docs = adt.docs(ctx.db);
+
+        // If not an import, add parenthesis automatically.
+        if ctx.is_path_type
+            && !ctx.has_type_args
+            && ctx.db.feature_flags.get("completion.insertion.add-call-parenthesis")
+        {
+            if has_non_default_type_params(adt, ctx.db) {
+                tested_by!(inserts_angle_brackets_for_generics);
+                builder = builder.insert_snippet(format!("{}<$0>", name));
+            }
+        }
+
+        builder.kind(kind).set_documentation(docs).add_to(self)
+    }
+
     pub(crate) fn add_const(&mut self, ctx: &CompletionContext, constant: hir::Const) {
         let ast_node = constant.source(ctx.db).ast;
         let name = match ast_node.name() {
@@ -213,7 +235,6 @@ impl Completions {
             .separator(", ")
             .surround_with("(", ")")
             .to_string();
-
         CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.to_string())
             .kind(CompletionItemKind::EnumVariant)
             .set_documentation(variant.docs(ctx.db))
@@ -222,6 +243,11 @@ impl Completions {
     }
 }
 
+fn has_non_default_type_params(adt: hir::Adt, db: &db::RootDatabase) -> bool {
+    let subst = db.generic_defaults(adt.into());
+    subst.iter().any(|ty| ty == &Ty::Unknown)
+}
+
 #[cfg(test)]
 mod tests {
     use crate::completion::{do_completion, CompletionItem, CompletionKind};
@@ -389,4 +415,90 @@ mod tests {
 ]"#
         );
     }
+
+    #[test]
+    fn inserts_angle_brackets_for_generics() {
+        covers!(inserts_angle_brackets_for_generics);
+        assert_debug_snapshot!(
+            do_reference_completion(
+                r"
+                struct Vec<T> {}
+                fn foo(xs: Ve<|>)
+                "
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "Vec",
+                source_range: [61; 63),
+                delete: [61; 63),
+                insert: "Vec<$0>",
+                kind: Struct,
+            },
+            CompletionItem {
+                label: "foo",
+                source_range: [61; 63),
+                delete: [61; 63),
+                insert: "foo($0)",
+                kind: Function,
+                detail: "fn foo(xs: Ve)",
+            },
+        ]
+        "###
+        );
+        assert_debug_snapshot!(
+            do_reference_completion(
+                r"
+                struct Vec<T = i128> {}
+                fn foo(xs: Ve<|>)
+                "
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "Vec",
+                source_range: [68; 70),
+                delete: [68; 70),
+                insert: "Vec",
+                kind: Struct,
+            },
+            CompletionItem {
+                label: "foo",
+                source_range: [68; 70),
+                delete: [68; 70),
+                insert: "foo($0)",
+                kind: Function,
+                detail: "fn foo(xs: Ve)",
+            },
+        ]
+        "###
+        );
+        assert_debug_snapshot!(
+            do_reference_completion(
+                r"
+                struct Vec<T> {}
+                fn foo(xs: Ve<|><i128>)
+                "
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "Vec",
+                source_range: [61; 63),
+                delete: [61; 63),
+                insert: "Vec",
+                kind: Struct,
+            },
+            CompletionItem {
+                label: "foo",
+                source_range: [61; 63),
+                delete: [61; 63),
+                insert: "foo($0)",
+                kind: Function,
+                detail: "fn foo(xs: Ve<i128>)",
+            },
+        ]
+        "###
+        );
+    }
 }
diff --git a/crates/ra_ide_api/src/marks.rs b/crates/ra_ide_api/src/marks.rs
index 3f4ba248b..848ae4dc7 100644
--- a/crates/ra_ide_api/src/marks.rs
+++ b/crates/ra_ide_api/src/marks.rs
@@ -1,6 +1,7 @@
 //! See test_utils/src/marks.rs
 
 test_utils::marks!(
+    inserts_angle_brackets_for_generics
     inserts_parens_for_function_calls
     goto_definition_works_for_macros
     goto_definition_works_for_methods