Merge #3169

3169: Show record field names in Enum completion r=flodiebold a=adamrk

Adresses #2947.
Previously the details shown when autocompleting an Enum variant would look like the variant was a tuple even if it was a record:
![2020-02-16-15:59:32_crop](https://user-images.githubusercontent.com/16367467/74607233-64f21980-50d7-11ea-99db-e973e29c71d7.png)

This change will show the names of the fields for a record and use curly braces instead of parentheses:
![2020-02-16-15:33:00_crop](https://user-images.githubusercontent.com/16367467/74607251-8ce17d00-50d7-11ea-9d4d-38d198a4aec0.png)

This required exposing the type `adt::StructKind` from `ra_hir` and adding a function 
```
kind(self, db: &impl HirDatabase) -> StructKind
```
in the `impl` of `EnumVariant`. 

There was also a previously existing function `is_unit(self, db: &impl HirDatabase) -> bool` for `EnumVariant` which I removed because it seemed redundant after adding `kind`.

Co-authored-by: adamrk <[email protected]>

Author: bors[bot]

crates/ra_hir/src/code_model.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use either::Either;
 use hir_def::{
+    adt::StructKind,
     adt::VariantData,
     builtin_type::BuiltinType,
     docs::Documentation,
@@ -424,6 +425,10 @@ impl EnumVariant {
             .collect()
     }
 
+    pub fn kind(self, db: &impl HirDatabase) -> StructKind {
+        self.variant_data(db).kind()
+    }
+
     pub(crate) fn variant_data(self, db: &impl DefDatabase) -> Arc<VariantData> {
         db.enum_data(self.parent.id).variants[self.id].variant_data.clone()
     }

crates/ra_hir/src/lib.rs

@@ -50,6 +50,7 @@ pub use crate::{
 };
 
 pub use hir_def::{
+    adt::StructKind,
     body::scope::ExprScopes,
     builtin_type::BuiltinType,
     docs::Documentation,

crates/ra_hir_def/src/adt.rs

@@ -140,10 +140,11 @@ impl VariantData {
         self.fields().iter().find_map(|(id, data)| if &data.name == name { Some(id) } else { None })
     }
 
-    pub fn is_unit(&self) -> bool {
+    pub fn kind(&self) -> StructKind {
         match self {
-            VariantData::Unit => true,
-            _ => false,
+            VariantData::Record(_) => StructKind::Record,
+            VariantData::Tuple(_) => StructKind::Tuple,
+            VariantData::Unit => StructKind::Unit,
         }
     }
 }
@@ -173,7 +174,7 @@ impl HasChildSource for VariantId {
     }
 }
 
-enum StructKind {
+pub enum StructKind {
     Tuple,
     Record,
     Unit,

crates/ra_hir_ty/src/lower.rs

@@ -9,6 +9,7 @@ use std::iter;
 use std::sync::Arc;
 
 use hir_def::{
+    adt::StructKind,
     builtin_type::BuiltinType,
     generics::{TypeParamProvenance, WherePredicate, WherePredicateTarget},
     path::{GenericArg, Path, PathSegment, PathSegments},
@@ -805,8 +806,8 @@ fn fn_sig_for_struct_constructor(db: &impl HirDatabase, def: StructId) -> PolyFn
 /// Build the type of a tuple struct constructor.
 fn type_for_struct_constructor(db: &impl HirDatabase, def: StructId) -> Binders<Ty> {
     let struct_data = db.struct_data(def.into());
-    if struct_data.variant_data.is_unit() {
-        return type_for_adt(db, def.into()); // Unit struct
+    if let StructKind::Unit = struct_data.variant_data.kind() {
+        return type_for_adt(db, def.into());
     }
     let generics = generics(db, def.into());
     let substs = Substs::bound_vars(&generics);
@@ -830,8 +831,8 @@ fn fn_sig_for_enum_variant_constructor(db: &impl HirDatabase, def: EnumVariantId
 fn type_for_enum_variant_constructor(db: &impl HirDatabase, def: EnumVariantId) -> Binders<Ty> {
     let enum_data = db.enum_data(def.parent);
     let var_data = &enum_data.variants[def.local_id].variant_data;
-    if var_data.is_unit() {
-        return type_for_adt(db, def.parent.into()); // Unit variant
+    if let StructKind::Unit = var_data.kind() {
+        return type_for_adt(db, def.parent.into());
     }
     let generics = generics(db, def.parent.into());
     let substs = Substs::bound_vars(&generics);

crates/ra_ide/src/completion/presentation.rs

@@ -1,6 +1,6 @@
 //! This modules takes care of rendering various definitions as completion items.
 
-use hir::{db::HirDatabase, Docs, HasAttrs, HasSource, HirDisplay, ScopeDef, Type};
+use hir::{db::HirDatabase, Docs, HasAttrs, HasSource, HirDisplay, ScopeDef, StructKind, Type};
 use join_to_string::join;
 use ra_syntax::ast::NameOwner;
 use test_utils::tested_by;
@@ -268,11 +268,22 @@ impl Completions {
     pub(crate) fn add_enum_variant(&mut self, ctx: &CompletionContext, variant: hir::EnumVariant) {
         let is_deprecated = is_deprecated(variant, ctx.db);
         let 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.map(|t| t.display(ctx.db).to_string()))
-            .separator(", ")
-            .surround_with("(", ")")
-            .to_string();
+        let detail_types =
+            variant.fields(ctx.db).into_iter().map(|field| (field.name(ctx.db), field.ty(ctx.db)));
+        let detail = match variant.kind(ctx.db) {
+            StructKind::Tuple | StructKind::Unit => {
+                join(detail_types.map(|(_, t)| t.display(ctx.db).to_string()))
+                    .separator(", ")
+                    .surround_with("(", ")")
+                    .to_string()
+            }
+            StructKind::Record => {
+                join(detail_types.map(|(n, t)| format!("{}: {}", n, t.display(ctx.db).to_string())))
+                    .separator(", ")
+                    .surround_with("{ ", " }")
+                    .to_string()
+            }
+        };
         CompletionItem::new(CompletionKind::Reference, ctx.source_range(), name.to_string())
             .kind(CompletionItemKind::EnumVariant)
             .set_documentation(variant.docs(ctx.db))
@@ -297,6 +308,84 @@ mod tests {
         do_completion(code, CompletionKind::Reference)
     }
 
+    #[test]
+    fn enum_detail_includes_names_for_record() {
+        assert_debug_snapshot!(
+        do_reference_completion(
+            r#"
+                enum Foo {
+                    Foo {x: i32, y: i32}
+                }
+
+                fn main() { Foo::Fo<|> }
+                "#,
+        ),
+        @r###"
+        [
+            CompletionItem {
+                label: "Foo",
+                source_range: [121; 123),
+                delete: [121; 123),
+                insert: "Foo",
+                kind: EnumVariant,
+                detail: "{ x: i32, y: i32 }",
+            },
+        ]"###
+        );
+    }
+
+    #[test]
+    fn enum_detail_doesnt_include_names_for_tuple() {
+        assert_debug_snapshot!(
+        do_reference_completion(
+            r#"
+                enum Foo {
+                    Foo (i32, i32)
+                }
+
+                fn main() { Foo::Fo<|> }
+                "#,
+        ),
+        @r###"
+        [
+            CompletionItem {
+                label: "Foo",
+                source_range: [115; 117),
+                delete: [115; 117),
+                insert: "Foo",
+                kind: EnumVariant,
+                detail: "(i32, i32)",
+            },
+        ]"###
+        );
+    }
+
+    #[test]
+    fn enum_detail_just_parentheses_for_unit() {
+        assert_debug_snapshot!(
+        do_reference_completion(
+            r#"
+                enum Foo {
+                    Foo
+                }
+
+                fn main() { Foo::Fo<|> }
+                "#,
+        ),
+        @r###"
+        [
+            CompletionItem {
+                label: "Foo",
+                source_range: [104; 106),
+                delete: [104; 106),
+                insert: "Foo",
+                kind: EnumVariant,
+                detail: "()",
+            },
+        ]"###
+        );
+    }
+
     #[test]
     fn sets_deprecated_flag_in_completion_items() {
         assert_debug_snapshot!(