4 files changed, 184 insertions, 143 deletions

diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs
index 31c7d4e80..cbeb7054f 100644
--- a/crates/ra_assists/src/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/add_missing_impl_members.rs
@@ -1,13 +1,14 @@
+use hir::{db::HirDatabase, HasSource};
+use ra_syntax::{
+    ast::{self, AstNode, NameOwner},
+    SmolStr,
+};
+
 use crate::{
     ast_editor::{AstBuilder, AstEditor},
     Assist, AssistCtx, AssistId,
 };
 
-use hir::{db::HirDatabase, HasSource};
-use ra_db::FilePosition;
-use ra_syntax::ast::{self, AstNode, NameOwner};
-use ra_syntax::SmolStr;
-
 #[derive(PartialEq)]
 enum AddMissingImplMembersMode {
     DefaultMethodsOnly,
@@ -43,8 +44,7 @@ fn add_missing_impl_members_inner(
 
     let trait_def = {
         let file_id = ctx.frange.file_id;
-        let position = FilePosition { file_id, offset: impl_node.syntax().text_range().start() };
-        let analyzer = hir::SourceAnalyzer::new(ctx.db, position.file_id, impl_node.syntax(), None);
+        let analyzer = hir::SourceAnalyzer::new(ctx.db, file_id, impl_node.syntax(), None);
 
         resolve_target_trait_def(ctx.db, &analyzer, &impl_node)?
     };
diff --git a/crates/ra_assists/src/ast_editor.rs b/crates/ra_assists/src/ast_editor.rs
index 95b871b30..6815638dc 100644
--- a/crates/ra_assists/src/ast_editor.rs
+++ b/crates/ra_assists/src/ast_editor.rs
@@ -1,6 +1,8 @@
 use std::{iter, ops::RangeInclusive};
 
 use arrayvec::ArrayVec;
+use itertools::Itertools;
+
 use hir::Name;
 use ra_fmt::leading_indent;
 use ra_syntax::{
@@ -17,7 +19,10 @@ pub struct AstEditor<N: AstNode> {
 }
 
 impl<N: AstNode> AstEditor<N> {
-    pub fn new(node: N) -> AstEditor<N> {
+    pub fn new(node: N) -> AstEditor<N>
+    where
+        N: Clone,
+    {
         AstEditor { original_ast: node.clone(), ast: node }
     }
 
@@ -88,15 +93,15 @@ impl<N: AstNode> AstEditor<N> {
     }
 }
 
-impl AstEditor<ast::NamedFieldList> {
-    pub fn append_field(&mut self, field: &ast::NamedField) {
+impl AstEditor<ast::RecordFieldList> {
+    pub fn append_field(&mut self, field: &ast::RecordField) {
         self.insert_field(InsertPosition::Last, field)
     }
 
     pub fn insert_field(
         &mut self,
-        position: InsertPosition<&'_ ast::NamedField>,
-        field: &ast::NamedField,
+        position: InsertPosition<&'_ ast::RecordField>,
+        field: &ast::RecordField,
     ) {
         let is_multiline = self.ast().syntax().text().contains_char('\n');
         let ws;
@@ -168,8 +173,7 @@ impl AstEditor<ast::NamedFieldList> {
 
 impl AstEditor<ast::ItemList> {
     pub fn append_items(&mut self, items: impl Iterator<Item = ast::ImplItem>) {
-        let n_existing_items = self.ast().impl_items().count();
-        if n_existing_items == 0 {
+        if !self.ast().syntax().text().contains_char('\n') {
             self.do_make_multiline();
         }
         items.for_each(|it| self.append_item(it));
@@ -241,16 +245,16 @@ pub struct AstBuilder<N: AstNode> {
     _phantom: std::marker::PhantomData<N>,
 }
 
-impl AstBuilder<ast::NamedField> {
-    pub fn from_name(name: &Name) -> ast::NamedField {
+impl AstBuilder<ast::RecordField> {
+    pub fn from_name(name: &Name) -> ast::RecordField {
         ast_node_from_file_text(&format!("fn f() {{ S {{ {}: (), }} }}", name))
     }
 
-    fn from_text(text: &str) -> ast::NamedField {
+    fn from_text(text: &str) -> ast::RecordField {
         ast_node_from_file_text(&format!("fn f() {{ S {{ {}, }} }}", text))
     }
 
-    pub fn from_pieces(name: &ast::NameRef, expr: Option<&ast::Expr>) -> ast::NamedField {
+    pub fn from_pieces(name: &ast::NameRef, expr: Option<&ast::Expr>) -> ast::RecordField {
         match expr {
             Some(expr) => Self::from_text(&format!("{}: {}", name.syntax(), expr.syntax())),
             None => Self::from_text(&name.syntax().to_string()),
@@ -288,9 +292,97 @@ impl AstBuilder<ast::NameRef> {
     }
 }
 
+impl AstBuilder<ast::Path> {
+    fn from_text(text: &str) -> ast::Path {
+        ast_node_from_file_text(text)
+    }
+
+    pub fn from_pieces(enum_name: ast::Name, var_name: ast::Name) -> ast::Path {
+        Self::from_text(&format!("{}::{}", enum_name.syntax(), var_name.syntax()))
+    }
+}
+
+impl AstBuilder<ast::BindPat> {
+    fn from_text(text: &str) -> ast::BindPat {
+        ast_node_from_file_text(&format!("fn f({}: ())", text))
+    }
+
+    pub fn from_name(name: &ast::Name) -> ast::BindPat {
+        Self::from_text(name.text())
+    }
+}
+
+impl AstBuilder<ast::PlaceholderPat> {
+    fn from_text(text: &str) -> ast::PlaceholderPat {
+        ast_node_from_file_text(&format!("fn f({}: ())", text))
+    }
+
+    pub fn placeholder() -> ast::PlaceholderPat {
+        Self::from_text("_")
+    }
+}
+
+impl AstBuilder<ast::TupleStructPat> {
+    fn from_text(text: &str) -> ast::TupleStructPat {
+        ast_node_from_file_text(&format!("fn f({}: ())", text))
+    }
+
+    pub fn from_pieces(
+        path: &ast::Path,
+        pats: impl Iterator<Item = ast::Pat>,
+    ) -> ast::TupleStructPat {
+        let pats_str = pats.map(|p| p.syntax().to_string()).collect::<Vec<_>>().join(", ");
+        Self::from_text(&format!("{}({})", path.syntax(), pats_str))
+    }
+}
+
+impl AstBuilder<ast::RecordPat> {
+    fn from_text(text: &str) -> ast::RecordPat {
+        ast_node_from_file_text(&format!("fn f({}: ())", text))
+    }
+
+    pub fn from_pieces(path: &ast::Path, pats: impl Iterator<Item = ast::Pat>) -> ast::RecordPat {
+        let pats_str = pats.map(|p| p.syntax().to_string()).collect::<Vec<_>>().join(", ");
+        Self::from_text(&format!("{}{{ {} }}", path.syntax(), pats_str))
+    }
+}
+
+impl AstBuilder<ast::PathPat> {
+    fn from_text(text: &str) -> ast::PathPat {
+        ast_node_from_file_text(&format!("fn f({}: ())", text))
+    }
+
+    pub fn from_path(path: &ast::Path) -> ast::PathPat {
+        let path_str = path.syntax().text().to_string();
+        Self::from_text(path_str.as_str())
+    }
+}
+
+impl AstBuilder<ast::MatchArm> {
+    fn from_text(text: &str) -> ast::MatchArm {
+        ast_node_from_file_text(&format!("fn f() {{ match () {{{}}} }}", text))
+    }
+
+    pub fn from_pieces(pats: impl Iterator<Item = ast::Pat>, expr: &ast::Expr) -> ast::MatchArm {
+        let pats_str = pats.map(|p| p.syntax().to_string()).join(" | ");
+        Self::from_text(&format!("{} => {}", pats_str, expr.syntax()))
+    }
+}
+
+impl AstBuilder<ast::MatchArmList> {
+    fn from_text(text: &str) -> ast::MatchArmList {
+        ast_node_from_file_text(&format!("fn f() {{ match () {{{}}} }}", text))
+    }
+
+    pub fn from_arms(arms: impl Iterator<Item = ast::MatchArm>) -> ast::MatchArmList {
+        let arms_str = arms.map(|arm| format!("\n    {}", arm.syntax())).join(",");
+        Self::from_text(&format!("{},\n", arms_str))
+    }
+}
+
 fn ast_node_from_file_text<N: AstNode>(text: &str) -> N {
     let parse = SourceFile::parse(text);
-    let res = parse.tree().syntax().descendants().find_map(N::cast).unwrap().to_owned();
+    let res = parse.tree().syntax().descendants().find_map(N::cast).unwrap();
     res
 }
 
diff --git a/crates/ra_assists/src/change_visibility.rs b/crates/ra_assists/src/change_visibility.rs
index d28cdd07b..60c74debc 100644
--- a/crates/ra_assists/src/change_visibility.rs
+++ b/crates/ra_assists/src/change_visibility.rs
@@ -38,7 +38,7 @@ fn add_vis(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
         (vis_offset(&parent), keyword.text_range())
     } else {
         let ident = ctx.token_at_offset().find(|leaf| leaf.kind() == IDENT)?;
-        let field = ident.parent().ancestors().find_map(ast::NamedFieldDef::cast)?;
+        let field = ident.parent().ancestors().find_map(ast::RecordFieldDef::cast)?;
         if field.name()?.syntax().text_range() != ident.text_range() && field.visibility().is_some()
         {
             return None;
diff --git a/crates/ra_assists/src/fill_match_arms.rs b/crates/ra_assists/src/fill_match_arms.rs
index 85ff5c052..cc3879562 100644
--- a/crates/ra_assists/src/fill_match_arms.rs
+++ b/crates/ra_assists/src/fill_match_arms.rs
@@ -1,97 +1,91 @@
-use itertools::Itertools;
-use std::fmt::Write;
+use std::iter;
 
-use hir::{db::HirDatabase, AdtDef, FieldSource, HasSource};
-use ra_syntax::ast::{self, AstNode};
+use hir::{db::HirDatabase, AdtDef, HasSource};
+use ra_syntax::ast::{self, AstNode, NameOwner};
 
-use crate::{Assist, AssistCtx, AssistId};
-
-fn is_trivial_arm(arm: &ast::MatchArm) -> bool {
-    fn single_pattern(arm: &ast::MatchArm) -> Option<ast::Pat> {
-        let (pat,) = arm.pats().collect_tuple()?;
-        Some(pat)
-    }
-    match single_pattern(arm) {
-        Some(ast::Pat::PlaceholderPat(..)) => true,
-        _ => false,
-    }
-}
+use crate::{ast_editor::AstBuilder, Assist, AssistCtx, AssistId};
 
 pub(crate) fn fill_match_arms(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let match_expr = ctx.node_at_offset::<ast::MatchExpr>()?;
+    let match_arm_list = match_expr.match_arm_list()?;
 
     // We already have some match arms, so we don't provide any assists.
     // Unless if there is only one trivial match arm possibly created
     // by match postfix complete. Trivial match arm is the catch all arm.
-    if let Some(arm_list) = match_expr.match_arm_list() {
-        let mut arm_iter = arm_list.arms();
-        let first = arm_iter.next();
-
-        match &first {
-            // If there arm list is empty or there is only one trivial arm, then proceed.
-            Some(arm) if is_trivial_arm(arm) => {
-                if arm_iter.next() != None {
-                    return None;
-                }
-            }
-            None => {}
-
-            _ => {
-                return None;
-            }
+    let mut existing_arms = match_arm_list.arms();
+    if let Some(arm) = existing_arms.next() {
+        if !is_trivial(&arm) || existing_arms.next().is_some() {
+            return None;
         }
     };
 
     let expr = match_expr.expr()?;
-    let analyzer = hir::SourceAnalyzer::new(ctx.db, ctx.frange.file_id, expr.syntax(), None);
-    let match_expr_ty = analyzer.type_of(ctx.db, &expr)?;
-    let enum_def = analyzer.autoderef(ctx.db, match_expr_ty).find_map(|ty| match ty.as_adt() {
-        Some((AdtDef::Enum(e), _)) => Some(e),
-        _ => None,
-    })?;
-    let enum_name = enum_def.name(ctx.db)?;
-    let db = ctx.db;
+    let enum_def = {
+        let file_id = ctx.frange.file_id;
+        let analyzer = hir::SourceAnalyzer::new(ctx.db, file_id, expr.syntax(), None);
+        resolve_enum_def(ctx.db, &analyzer, &expr)?
+    };
+    let variant_list = enum_def.variant_list()?;
 
     ctx.add_action(AssistId("fill_match_arms"), "fill match arms", |edit| {
-        let mut buf = format!("match {} {{\n", expr.syntax().text().to_string());
-        let variants = enum_def.variants(db);
-        for variant in variants {
-            let name = match variant.name(db) {
-                Some(it) => it,
-                None => continue,
-            };
-            write!(&mut buf, "    {}::{}", enum_name, name.to_string()).unwrap();
-
-            let pat = variant
-                .fields(db)
-                .into_iter()
-                .map(|field| {
-                    let name = field.name(db).to_string();
-                    let src = field.source(db);
-                    match src.ast {
-                        FieldSource::Named(_) => name,
-                        FieldSource::Pos(_) => "_".to_string(),
-                    }
-                })
-                .collect::<Vec<_>>();
+        let variants = variant_list.variants();
+        let arms = variants.into_iter().filter_map(build_pat).map(|pat| {
+            AstBuilder::<ast::MatchArm>::from_pieces(
+                iter::once(pat),
+                &AstBuilder::<ast::Expr>::unit(),
+            )
+        });
+        let new_arm_list = AstBuilder::<ast::MatchArmList>::from_arms(arms);
 
-            match pat.first().map(|s| s.as_str()) {
-                Some("_") => write!(&mut buf, "({})", pat.join(", ")).unwrap(),
-                Some(_) => write!(&mut buf, "{{{}}}", pat.join(", ")).unwrap(),
-                None => (),
-            };
-
-            buf.push_str(" => (),\n");
-        }
-        buf.push_str("}");
         edit.target(match_expr.syntax().text_range());
         edit.set_cursor(expr.syntax().text_range().start());
-        edit.replace_node_and_indent(match_expr.syntax(), buf);
+        edit.replace_node_and_indent(match_arm_list.syntax(), new_arm_list.syntax().text());
     });
 
     ctx.build()
 }
 
+fn is_trivial(arm: &ast::MatchArm) -> bool {
+    arm.pats().any(|pat| match pat {
+        ast::Pat::PlaceholderPat(..) => true,
+        _ => false,
+    })
+}
+
+fn resolve_enum_def(
+    db: &impl HirDatabase,
+    analyzer: &hir::SourceAnalyzer,
+    expr: &ast::Expr,
+) -> Option<ast::EnumDef> {
+    let expr_ty = analyzer.type_of(db, &expr)?;
+
+    analyzer.autoderef(db, expr_ty).find_map(|ty| match ty.as_adt() {
+        Some((AdtDef::Enum(e), _)) => Some(e.source(db).ast),
+        _ => None,
+    })
+}
+
+fn build_pat(var: ast::EnumVariant) -> Option<ast::Pat> {
+    let path = &AstBuilder::<ast::Path>::from_pieces(var.parent_enum().name()?, var.name()?);
+
+    let pat: ast::Pat = match var.kind() {
+        ast::StructKind::Tuple(field_list) => {
+            let pats = iter::repeat(AstBuilder::<ast::PlaceholderPat>::placeholder().into())
+                .take(field_list.fields().count());
+            AstBuilder::<ast::TupleStructPat>::from_pieces(path, pats).into()
+        }
+        ast::StructKind::Named(field_list) => {
+            let pats = field_list
+                .fields()
+                .map(|f| AstBuilder::<ast::BindPat>::from_name(&f.name().unwrap()).into());
+            AstBuilder::<ast::RecordPat>::from_pieces(path, pats).into()
+        }
+        ast::StructKind::Unit => AstBuilder::<ast::PathPat>::from_path(path).into(),
+    };
+
+    Some(pat)
+}
+
 #[cfg(test)]
 mod tests {
     use crate::helpers::{check_assist, check_assist_target};
@@ -108,7 +102,7 @@ mod tests {
                 Bs,
                 Cs(String),
                 Ds(String, String),
-                Es{x: usize, y: usize}
+                Es{ x: usize, y: usize }
             }
 
             fn main() {
@@ -122,7 +116,7 @@ mod tests {
                 Bs,
                 Cs(String),
                 Ds(String, String),
-                Es{x: usize, y: usize}
+                Es{ x: usize, y: usize }
             }
 
             fn main() {
@@ -132,7 +126,7 @@ mod tests {
                     A::Bs => (),
                     A::Cs(_) => (),
                     A::Ds(_, _) => (),
-                    A::Es{x, y} => (),
+                    A::Es{ x, y } => (),
                 }
             }
             "#,
@@ -170,7 +164,7 @@ mod tests {
             fill_match_arms,
             r#"
             enum A {
-                Es{x: usize, y: usize}
+                Es{ x: usize, y: usize }
             }
 
             fn foo(a: &mut A) {
@@ -180,57 +174,12 @@ mod tests {
             "#,
             r#"
             enum A {
-                Es{x: usize, y: usize}
+                Es{ x: usize, y: usize }
             }
 
             fn foo(a: &mut A) {
                 match <|>a {
-                    A::Es{x, y} => (),
-                }
-            }
-            "#,
-        );
-
-        check_assist(
-            fill_match_arms,
-            r#"
-            enum E { X, Y}
-
-            fn main() {
-                match &E::X<|>
-            }
-            "#,
-            r#"
-            enum E { X, Y}
-
-            fn main() {
-                match <|>&E::X {
-                    E::X => (),
-                    E::Y => (),
-                }
-            }
-            "#,
-        );
-    }
-
-    #[test]
-    fn fill_match_arms_no_body() {
-        check_assist(
-            fill_match_arms,
-            r#"
-            enum E { X, Y}
-
-            fn main() {
-                match E::X<|>
-            }
-            "#,
-            r#"
-            enum E { X, Y}
-
-            fn main() {
-                match <|>E::X {
-                    E::X => (),
-                    E::Y => (),
+                    A::Es{ x, y } => (),
                 }
             }
             "#,
@@ -242,7 +191,7 @@ mod tests {
         check_assist_target(
             fill_match_arms,
             r#"
-            enum E { X, Y}
+            enum E { X, Y }
 
             fn main() {
                 match E::X<|> {}