Merge #1184

1184: Start structured editing API r=matklad a=matklad

I think I finally understand how to provide nice, mutable structured editing API on top of red-green trees.

The problem I am trying to solve is that any modification to a particular `SyntaxNode` returns an independent new file. So, if you are editing a struct literal, and add a field, you get back a SourceFile, and you have to find the struct literal inside it yourself! This happens because our trees are immutable, but have parent pointers. 

The main idea here is to introduce `AstEditor<T>` type, which abstracts away that API. So, you create an `AstEditor` for node you want to edit and call various `&mut` taking methods on it. Internally, `AstEditor` stores both the original node and the current node. All edits are applied to the current node, which is replaced by the corresponding node in the new file. In the end, `AstEditor` computes a text edit between old and new nodes.

Note that this also should sole a problem when you create an anchor pointing to a subnode and mutate the parent node, invalidating anchor. Because mutation needs `&mut`, all anchors must be killed before modification. 

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

13 files changed, 569 insertions, 196 deletions

diff --git a/Cargo.lock b/Cargo.lock
index e5729f968..3be73d66d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -903,8 +903,10 @@ version = "0.1.0"
 name = "ra_assists"
 version = "0.1.0"
 dependencies = [
+ "arrayvec 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "join_to_string 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ra_db 0.1.0",
  "ra_fmt 0.1.0",
  "ra_hir 0.1.0",
diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml
index d4056a349..29d9ceb59 100644
--- a/crates/ra_assists/Cargo.toml
+++ b/crates/ra_assists/Cargo.toml
@@ -5,8 +5,10 @@ version = "0.1.0"
 authors = ["rust-analyzer developers"]
 
 [dependencies]
+lazy_static = "1.3.0"
 join_to_string = "0.1.3"
 itertools = "0.8.0"
+arrayvec = "0.4.10"
 
 ra_syntax = { path = "../ra_syntax" }
 ra_text_edit = { path = "../ra_text_edit" }
diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs
index c82447b84..17c2af899 100644
--- a/crates/ra_assists/src/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/add_missing_impl_members.rs
@@ -1,14 +1,9 @@
-use std::fmt::Write;
-
-use crate::{Assist, AssistId, AssistCtx};
+use crate::{Assist, AssistId, AssistCtx, ast_editor::{AstEditor, AstBuilder}};
 
 use hir::db::HirDatabase;
-use ra_syntax::{SmolStr, SyntaxKind, TextRange, TextUnit, TreeArc};
-use ra_syntax::ast::{self, AstNode, AstToken, FnDef, ImplItem, ImplItemKind, NameOwner};
+use ra_syntax::{SmolStr, TreeArc};
+use ra_syntax::ast::{self, AstNode, FnDef, ImplItem, ImplItemKind, NameOwner};
 use ra_db::FilePosition;
-use ra_fmt::{leading_indent, reindent};
-
-use itertools::Itertools;
 
 enum AddMissingImplMembersMode {
     DefaultMethodsOnly,
@@ -76,48 +71,35 @@ fn add_missing_impl_members_inner(
     }
 
     ctx.add_action(AssistId(assist_id), label, |edit| {
-        let (parent_indent, indent) = {
-            // FIXME: Find a way to get the indent already used in the file.
-            // Now, we copy the indent of first item or indent with 4 spaces relative to impl block
-            const DEFAULT_INDENT: &str = "    ";
-            let first_item = impl_item_list.impl_items().next();
-            let first_item_indent =
-                first_item.and_then(|i| leading_indent(i.syntax())).map(ToOwned::to_owned);
-            let impl_block_indent = leading_indent(impl_node.syntax()).unwrap_or_default();
-
-            (
-                impl_block_indent.to_owned(),
-                first_item_indent.unwrap_or_else(|| impl_block_indent.to_owned() + DEFAULT_INDENT),
-            )
-        };
-
-        let changed_range = {
-            let children = impl_item_list.syntax().children_with_tokens();
-            let last_whitespace =
-                children.filter_map(|it| ast::Whitespace::cast(it.as_token()?)).last();
-
-            last_whitespace.map(|w| w.syntax().range()).unwrap_or_else(|| {
-                let in_brackets = impl_item_list.syntax().range().end() - TextUnit::of_str("}");
-                TextRange::from_to(in_brackets, in_brackets)
-            })
-        };
-
-        let func_bodies = format!("\n{}", missing_fns.into_iter().map(build_func_body).join("\n"));
-        let trailing_whitespace = format!("\n{}", parent_indent);
-        let func_bodies = reindent(&func_bodies, &indent) + &trailing_whitespace;
-
-        let replaced_text_range = TextUnit::of_str(&func_bodies);
-
-        edit.replace(changed_range, func_bodies);
-        // FIXME: place the cursor on the first unimplemented?
-        edit.set_cursor(
-            changed_range.start() + replaced_text_range - TextUnit::of_str(&trailing_whitespace),
-        );
+        let n_existing_items = impl_item_list.impl_items().count();
+        let fns = missing_fns.into_iter().map(add_body_and_strip_docstring).collect::<Vec<_>>();
+
+        let mut ast_editor = AstEditor::new(impl_item_list);
+        if n_existing_items == 0 {
+            ast_editor.make_multiline();
+        }
+        ast_editor.append_functions(fns.iter().map(|it| &**it));
+        let first_new_item = ast_editor.ast().impl_items().nth(n_existing_items).unwrap();
+        let cursor_poisition = first_new_item.syntax().range().start();
+        ast_editor.into_text_edit(edit.text_edit_builder());
+
+        edit.set_cursor(cursor_poisition);
     });
 
     ctx.build()
 }
 
+fn add_body_and_strip_docstring(fn_def: &ast::FnDef) -> TreeArc<ast::FnDef> {
+    let mut ast_editor = AstEditor::new(fn_def);
+    if fn_def.body().is_none() {
+        ast_editor.set_body(&AstBuilder::<ast::Block>::single_expr(
+            &AstBuilder::<ast::Expr>::unimplemented(),
+        ));
+    }
+    ast_editor.strip_attrs_and_docs();
+    ast_editor.ast().to_owned()
+}
+
 /// Given an `ast::ImplBlock`, resolves the target trait (the one being
 /// implemented) to a `ast::TraitDef`.
 fn resolve_target_trait_def(
@@ -134,22 +116,6 @@ fn resolve_target_trait_def(
     }
 }
 
-fn build_func_body(def: &ast::FnDef) -> String {
-    let mut buf = String::new();
-
-    for child in def.syntax().children_with_tokens() {
-        match (child.prev_sibling_or_token().map(|c| c.kind()), child.kind()) {
-            (_, SyntaxKind::SEMI) => buf.push_str(" {\n    unimplemented!()\n}"),
-            (_, SyntaxKind::ATTR) | (_, SyntaxKind::COMMENT) => {}
-            (Some(SyntaxKind::ATTR), SyntaxKind::WHITESPACE)
-            | (Some(SyntaxKind::COMMENT), SyntaxKind::WHITESPACE) => {}
-            _ => write!(buf, "{}", child).unwrap(),
-        };
-    }
-
-    buf.trim_end().to_string()
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -170,7 +136,7 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    <|>
+<|>
 }",
             "
 trait Foo {
@@ -183,12 +149,9 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    fn foo(&self) {
-        unimplemented!()
-    }
-    fn baz(&self) {
-        unimplemented!()
-    }<|>
+    <|>fn foo(&self) { unimplemented!() }
+    fn baz(&self) { unimplemented!() }
+
 }",
         );
     }
@@ -208,7 +171,7 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    <|>
+<|>
 }",
             "
 trait Foo {
@@ -221,9 +184,8 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    fn foo(&self) {
-        unimplemented!()
-    }<|>
+    <|>fn foo(&self) { unimplemented!() }
+
 }",
         );
     }
@@ -240,9 +202,7 @@ impl Foo for S { <|> }",
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    fn foo(&self) {
-        unimplemented!()
-    }<|>
+    <|>fn foo(&self) { unimplemented!() }
 }",
         );
     }
@@ -259,9 +219,7 @@ impl Foo for S {}<|>",
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    fn foo(&self) {
-        unimplemented!()
-    }<|>
+    <|>fn foo(&self) { unimplemented!() }
 }",
         )
     }
@@ -292,35 +250,6 @@ impl Foo for S { <|> }",
     }
 
     #[test]
-    fn test_indented_impl_block() {
-        check_assist(
-            add_missing_impl_members,
-            "
-trait Foo {
-    fn valid(some: u32) -> bool;
-}
-struct S;
-
-mod my_mod {
-    impl crate::Foo for S { <|> }
-}",
-            "
-trait Foo {
-    fn valid(some: u32) -> bool;
-}
-struct S;
-
-mod my_mod {
-    impl crate::Foo for S {
-        fn valid(some: u32) -> bool {
-            unimplemented!()
-        }<|>
-    }
-}",
-        )
-    }
-
-    #[test]
     fn test_with_docstring_and_attrs() {
         check_assist(
             add_missing_impl_members,
@@ -342,9 +271,7 @@ trait Foo {
 }
 struct S;
 impl Foo for S {
-    fn foo(&self) {
-        unimplemented!()
-    }<|>
+    <|>fn foo(&self) { unimplemented!() }
 }"#,
         )
     }
@@ -367,7 +294,7 @@ trait Foo {
 }
 struct S;
 impl Foo for S {
-    fn valid(some: u32) -> bool { false }<|>
+    <|>fn valid(some: u32) -> bool { false }
 }",
         )
     }
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index e80e35738..f46de61eb 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -161,6 +161,10 @@ impl AssistBuilder {
         self.target = Some(target)
     }
 
+    pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder {
+        &mut self.edit
+    }
+
     fn build(self) -> AssistAction {
         AssistAction {
             edit: self.edit.finish(),
diff --git a/crates/ra_assists/src/ast_editor.rs b/crates/ra_assists/src/ast_editor.rs
new file mode 100644
index 000000000..6854294ae
--- /dev/null
+++ b/crates/ra_assists/src/ast_editor.rs
@@ -0,0 +1,333 @@
+use std::{iter, ops::RangeInclusive};
+
+use arrayvec::ArrayVec;
+use ra_text_edit::TextEditBuilder;
+use ra_syntax::{AstNode, TreeArc, ast, SyntaxKind::*, SyntaxElement, SourceFile, InsertPosition, Direction};
+use ra_fmt::leading_indent;
+
+pub struct AstEditor<N: AstNode> {
+    original_ast: TreeArc<N>,
+    ast: TreeArc<N>,
+}
+
+impl<N: AstNode> AstEditor<N> {
+    pub fn new(node: &N) -> AstEditor<N> {
+        AstEditor { original_ast: node.to_owned(), ast: node.to_owned() }
+    }
+
+    pub fn into_text_edit(self, builder: &mut TextEditBuilder) {
+        // FIXME: compute a more fine-grained diff here.
+        // If *you* know a nice algorithm to compute diff between two syntax
+        // tree, tell me about it!
+        builder.replace(self.original_ast.syntax().range(), self.ast().syntax().text().to_string());
+    }
+
+    pub fn ast(&self) -> &N {
+        &*self.ast
+    }
+
+    #[must_use]
+    fn insert_children<'a>(
+        &self,
+        position: InsertPosition<SyntaxElement<'_>>,
+        to_insert: impl Iterator<Item = SyntaxElement<'a>>,
+    ) -> TreeArc<N> {
+        let new_syntax = self.ast().syntax().insert_children(position, to_insert);
+        N::cast(&new_syntax).unwrap().to_owned()
+    }
+
+    #[must_use]
+    fn replace_children<'a>(
+        &self,
+        to_delete: RangeInclusive<SyntaxElement<'_>>,
+        to_insert: impl Iterator<Item = SyntaxElement<'a>>,
+    ) -> TreeArc<N> {
+        let new_syntax = self.ast().syntax().replace_children(to_delete, to_insert);
+        N::cast(&new_syntax).unwrap().to_owned()
+    }
+
+    fn do_make_multiline(&mut self) {
+        let l_curly =
+            match self.ast().syntax().children_with_tokens().find(|it| it.kind() == L_CURLY) {
+                Some(it) => it,
+                None => return,
+            };
+        let sibling = match l_curly.next_sibling_or_token() {
+            Some(it) => it,
+            None => return,
+        };
+        let existing_ws = match sibling.as_token() {
+            None => None,
+            Some(tok) if tok.kind() != WHITESPACE => None,
+            Some(ws) => {
+                if ws.text().contains('\n') {
+                    return;
+                }
+                Some(ws)
+            }
+        };
+
+        let indent = leading_indent(self.ast().syntax()).unwrap_or("");
+        let ws = tokens::WsBuilder::new(&format!("\n{}", indent));
+        let to_insert = iter::once(ws.ws().into());
+        self.ast = match existing_ws {
+            None => self.insert_children(InsertPosition::After(l_curly), to_insert),
+            Some(ws) => self.replace_children(RangeInclusive::new(ws.into(), ws.into()), to_insert),
+        };
+    }
+}
+
+impl AstEditor<ast::NamedFieldList> {
+    pub fn append_field(&mut self, field: &ast::NamedField) {
+        self.insert_field(InsertPosition::Last, field)
+    }
+
+    pub fn make_multiline(&mut self) {
+        self.do_make_multiline()
+    }
+
+    pub fn insert_field(
+        &mut self,
+        position: InsertPosition<&'_ ast::NamedField>,
+        field: &ast::NamedField,
+    ) {
+        let is_multiline = self.ast().syntax().text().contains('\n');
+        let ws;
+        let space = if is_multiline {
+            ws = tokens::WsBuilder::new(&format!(
+                "\n{}    ",
+                leading_indent(self.ast().syntax()).unwrap_or("")
+            ));
+            ws.ws()
+        } else {
+            tokens::single_space()
+        };
+
+        let mut to_insert: ArrayVec<[SyntaxElement; 4]> = ArrayVec::new();
+        to_insert.push(space.into());
+        to_insert.push(field.syntax().into());
+        to_insert.push(tokens::comma().into());
+
+        macro_rules! after_l_curly {
+            () => {{
+                let anchor = match self.l_curly() {
+                    Some(it) => it,
+                    None => return,
+                };
+                InsertPosition::After(anchor)
+            }};
+        }
+
+        macro_rules! after_field {
+            ($anchor:expr) => {
+                if let Some(comma) = $anchor
+                    .syntax()
+                    .siblings_with_tokens(Direction::Next)
+                    .find(|it| it.kind() == COMMA)
+                {
+                    InsertPosition::After(comma)
+                } else {
+                    to_insert.insert(0, tokens::comma().into());
+                    InsertPosition::After($anchor.syntax().into())
+                }
+            };
+        };
+
+        let position = match position {
+            InsertPosition::First => after_l_curly!(),
+            InsertPosition::Last => {
+                if !is_multiline {
+                    // don't insert comma before curly
+                    to_insert.pop();
+                }
+                match self.ast().fields().last() {
+                    Some(it) => after_field!(it),
+                    None => after_l_curly!(),
+                }
+            }
+            InsertPosition::Before(anchor) => InsertPosition::Before(anchor.syntax().into()),
+            InsertPosition::After(anchor) => after_field!(anchor),
+        };
+
+        self.ast = self.insert_children(position, to_insert.iter().cloned());
+    }
+
+    fn l_curly(&self) -> Option<SyntaxElement> {
+        self.ast().syntax().children_with_tokens().find(|it| it.kind() == L_CURLY)
+    }
+}
+
+impl AstEditor<ast::ItemList> {
+    pub fn make_multiline(&mut self) {
+        self.do_make_multiline()
+    }
+
+    pub fn append_functions<'a>(&mut self, fns: impl Iterator<Item = &'a ast::FnDef>) {
+        fns.for_each(|it| self.append_function(it))
+    }
+
+    pub fn append_function(&mut self, fn_def: &ast::FnDef) {
+        let (indent, position) = match self.ast().impl_items().last() {
+            Some(it) => (
+                leading_indent(it.syntax()).unwrap_or("").to_string(),
+                InsertPosition::After(it.syntax().into()),
+            ),
+            None => match self.l_curly() {
+                Some(it) => (
+                    "    ".to_string() + leading_indent(self.ast().syntax()).unwrap_or(""),
+                    InsertPosition::After(it),
+                ),
+                None => return,
+            },
+        };
+        let ws = tokens::WsBuilder::new(&format!("\n{}", indent));
+        let to_insert: ArrayVec<[SyntaxElement; 2]> =
+            [ws.ws().into(), fn_def.syntax().into()].into();
+        self.ast = self.insert_children(position, to_insert.into_iter());
+    }
+
+    fn l_curly(&self) -> Option<SyntaxElement> {
+        self.ast().syntax().children_with_tokens().find(|it| it.kind() == L_CURLY)
+    }
+}
+
+impl AstEditor<ast::FnDef> {
+    pub fn set_body(&mut self, body: &ast::Block) {
+        let mut to_insert: ArrayVec<[SyntaxElement; 2]> = ArrayVec::new();
+        let old_body_or_semi: SyntaxElement = if let Some(old_body) = self.ast().body() {
+            old_body.syntax().into()
+        } else if let Some(semi) = self.ast().semicolon_token() {
+            to_insert.push(tokens::single_space().into());
+            semi.into()
+        } else {
+            to_insert.push(tokens::single_space().into());
+            to_insert.push(body.syntax().into());
+            self.ast = self.insert_children(InsertPosition::Last, to_insert.into_iter());
+            return;
+        };
+        to_insert.push(body.syntax().into());
+        let replace_range = RangeInclusive::new(old_body_or_semi, old_body_or_semi);
+        self.ast = self.replace_children(replace_range, to_insert.into_iter())
+    }
+
+    pub fn strip_attrs_and_docs(&mut self) {
+        loop {
+            if let Some(start) = self
+                .ast()
+                .syntax()
+                .children_with_tokens()
+                .find(|it| it.kind() == ATTR || it.kind() == COMMENT)
+            {
+                let end = match start.next_sibling_or_token() {
+                    Some(el) if el.kind() == WHITESPACE => el,
+                    Some(_) | None => start,
+                };
+                self.ast = self.replace_children(RangeInclusive::new(start, end), iter::empty());
+            } else {
+                break;
+            }
+        }
+    }
+}
+
+pub struct AstBuilder<N: AstNode> {
+    _phantom: std::marker::PhantomData<N>,
+}
+
+impl AstBuilder<ast::NamedField> {
+    fn from_text(text: &str) -> TreeArc<ast::NamedField> {
+        ast_node_from_file_text(&format!("fn f() {{ S {{ {}, }} }}", text))
+    }
+
+    pub fn from_pieces(name: &ast::NameRef, expr: Option<&ast::Expr>) -> TreeArc<ast::NamedField> {
+        match expr {
+            Some(expr) => Self::from_text(&format!("{}: {}", name.syntax(), expr.syntax())),
+            None => Self::from_text(&name.syntax().to_string()),
+        }
+    }
+}
+
+impl AstBuilder<ast::Block> {
+    fn from_text(text: &str) -> TreeArc<ast::Block> {
+        ast_node_from_file_text(&format!("fn f() {}", text))
+    }
+
+    pub fn single_expr(e: &ast::Expr) -> TreeArc<ast::Block> {
+        Self::from_text(&format!("{{ {} }}", e.syntax()))
+    }
+}
+
+impl AstBuilder<ast::Expr> {
+    fn from_text(text: &str) -> TreeArc<ast::Expr> {
+        ast_node_from_file_text(&format!("fn f() {{ {}; }}", text))
+    }
+
+    pub fn unit() -> TreeArc<ast::Expr> {
+        Self::from_text("()")
+    }
+
+    pub fn unimplemented() -> TreeArc<ast::Expr> {
+        Self::from_text("unimplemented!()")
+    }
+}
+
+impl AstBuilder<ast::NameRef> {
+    pub fn new(text: &str) -> TreeArc<ast::NameRef> {
+        ast_node_from_file_text(&format!("fn f() {{ {}; }}", text))
+    }
+}
+
+fn ast_node_from_file_text<N: AstNode>(text: &str) -> TreeArc<N> {
+    let file = SourceFile::parse(text);
+    let res = file.syntax().descendants().find_map(N::cast).unwrap().to_owned();
+    res
+}
+
+mod tokens {
+    use lazy_static::lazy_static;
+    use ra_syntax::{AstNode, SourceFile, TreeArc, SyntaxToken, SyntaxKind::*};
+
+    lazy_static! {
+        static ref SOURCE_FILE: TreeArc<SourceFile> = SourceFile::parse(",\n; ;");
+    }
+
+    pub(crate) fn comma() -> SyntaxToken<'static> {
+        SOURCE_FILE
+            .syntax()
+            .descendants_with_tokens()
+            .filter_map(|it| it.as_token())
+            .find(|it| it.kind() == COMMA)
+            .unwrap()
+    }
+
+    pub(crate) fn single_space() -> SyntaxToken<'static> {
+        SOURCE_FILE
+            .syntax()
+            .descendants_with_tokens()
+            .filter_map(|it| it.as_token())
+            .find(|it| it.kind() == WHITESPACE && it.text().as_str() == " ")
+            .unwrap()
+    }
+
+    #[allow(unused)]
+    pub(crate) fn single_newline() -> SyntaxToken<'static> {
+        SOURCE_FILE
+            .syntax()
+            .descendants_with_tokens()
+            .filter_map(|it| it.as_token())
+            .find(|it| it.kind() == WHITESPACE && it.text().as_str() == "\n")
+            .unwrap()
+    }
+
+    pub(crate) struct WsBuilder(TreeArc<SourceFile>);
+
+    impl WsBuilder {
+        pub(crate) fn new(text: &str) -> WsBuilder {
+            WsBuilder(SourceFile::parse(text))
+        }
+        pub(crate) fn ws(&self) -> SyntaxToken<'_> {
+            self.0.syntax().first_child_or_token().unwrap().as_token().unwrap()
+        }
+    }
+
+}
diff --git a/crates/ra_assists/src/fill_struct_fields.rs b/crates/ra_assists/src/fill_struct_fields.rs
index 663b4f669..54b70e17d 100644
--- a/crates/ra_assists/src/fill_struct_fields.rs
+++ b/crates/ra_assists/src/fill_struct_fields.rs
@@ -1,94 +1,55 @@
-use std::fmt::Write;
-
 use hir::{AdtDef, db::HirDatabase};
 
 use ra_syntax::ast::{self, AstNode};
 
-use crate::{AssistCtx, Assist, AssistId};
+use crate::{AssistCtx, Assist, AssistId, ast_editor::{AstEditor, AstBuilder}};
 
 pub(crate) fn fill_struct_fields(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let struct_lit = ctx.node_at_offset::<ast::StructLit>()?;
-    let mut fsf = FillStructFields {
-        ctx: &mut ctx,
-        named_field_list: struct_lit.named_field_list()?,
-        struct_fields: vec![],
-        struct_lit,
-    };
-    fsf.evaluate_struct_def_fields()?;
-    if fsf.struct_lit_and_def_have_the_same_number_of_fields() {
-        return None;
-    }
-    fsf.remove_already_included_fields()?;
-    fsf.add_action()?;
-    ctx.build()
-}
+    let named_field_list = struct_lit.named_field_list()?;
 
-struct FillStructFields<'a, 'b: 'a, DB> {
-    ctx: &'a mut AssistCtx<'b, DB>,
-    named_field_list: &'a ast::NamedFieldList,
-    struct_fields: Vec<(String, String)>,
-    struct_lit: &'a ast::StructLit,
-}
-
-impl<DB> FillStructFields<'_, '_, DB>
-where
-    DB: HirDatabase,
-{
-    fn add_action(&mut self) -> Option<()> {
-        let named_field_list = self.named_field_list;
-        let struct_fields_string = self.struct_fields_string()?;
-        let struct_lit = self.struct_lit;
-        self.ctx.add_action(AssistId("fill_struct_fields"), "fill struct fields", |edit| {
-            edit.target(struct_lit.syntax().range());
-            edit.set_cursor(struct_lit.syntax().range().start());
-            edit.replace_node_and_indent(named_field_list.syntax(), struct_fields_string);
-        });
-        Some(())
-    }
-
-    fn struct_lit_and_def_have_the_same_number_of_fields(&self) -> bool {
-        self.named_field_list.fields().count() == self.struct_fields.len()
-    }
-
-    fn evaluate_struct_def_fields(&mut self) -> Option<()> {
-        let analyzer = hir::SourceAnalyzer::new(
-            self.ctx.db,
-            self.ctx.frange.file_id,
-            self.struct_lit.syntax(),
-            None,
-        );
-        let struct_lit_ty = analyzer.type_of(self.ctx.db, self.struct_lit.into())?;
+    // Collect all fields from struct definition
+    let mut fields = {
+        let analyzer =
+            hir::SourceAnalyzer::new(ctx.db, ctx.frange.file_id, struct_lit.syntax(), None);
+        let struct_lit_ty = analyzer.type_of(ctx.db, struct_lit.into())?;
         let struct_def = match struct_lit_ty.as_adt() {
             Some((AdtDef::Struct(s), _)) => s,
             _ => return None,
         };
-        self.struct_fields = struct_def
-            .fields(self.ctx.db)
-            .into_iter()
-            .map(|f| (f.name(self.ctx.db).to_string(), "()".into()))
-            .collect();
-        Some(())
-    }
+        struct_def.fields(ctx.db)
+    };
 
-    fn remove_already_included_fields(&mut self) -> Option<()> {
-        for ast_field in self.named_field_list.fields() {
-            let expr = ast_field.expr()?.syntax().text().to_string();
-            let name_from_ast = ast_field.name_ref()?.text().to_string();
-            if let Some(idx) = self.struct_fields.iter().position(|(n, _)| n == &name_from_ast) {
-                self.struct_fields[idx] = (name_from_ast, expr);
-            }
-        }
-        Some(())
+    // Filter out existing fields
+    for ast_field in named_field_list.fields() {
+        let name_from_ast = ast_field.name_ref()?.text().to_string();
+        fields.retain(|field| field.name(ctx.db).to_string() != name_from_ast);
+    }
+    if fields.is_empty() {
+        return None;
     }
 
-    fn struct_fields_string(&mut self) -> Option<String> {
-        let mut buf = String::from("{\n");
-        for (name, expr) in &self.struct_fields {
-            write!(&mut buf, "    {}: {},\n", name, expr).unwrap();
+    let db = ctx.db;
+    ctx.add_action(AssistId("fill_struct_fields"), "fill struct fields", |edit| {
+        let mut ast_editor = AstEditor::new(named_field_list);
+        if named_field_list.fields().count() == 0 && fields.len() > 2 {
+            ast_editor.make_multiline();
+        };
+
+        for field in fields {
+            let field = AstBuilder::<ast::NamedField>::from_pieces(
+                &AstBuilder::<ast::NameRef>::new(&field.name(db).to_string()),
+                Some(&AstBuilder::<ast::Expr>::unit()),
+            );
+            ast_editor.append_field(&field);
         }
-        buf.push_str("}");
-        Some(buf)
-    }
+
+        edit.target(struct_lit.syntax().range());
+        edit.set_cursor(struct_lit.syntax().range().start());
+
+        ast_editor.into_text_edit(edit.text_edit_builder());
+    });
+    ctx.build()
 }
 
 #[cfg(test)]
@@ -225,14 +186,41 @@ mod tests {
 
             fn main() {
                 let s = <|>S {
+                    c: (1, 2),
+                    e: "foo",
                     a: (),
                     b: (),
-                    c: (1, 2),
                     d: (),
-                    e: "foo",
                 }
             }
             "#,
         );
     }
+
+    #[test]
+    fn fill_struct_short() {
+        check_assist(
+            fill_struct_fields,
+            r#"
+            struct S {
+                foo: u32,
+                bar: String,
+            }
+
+            fn main() {
+                let s = S {<|> };
+            }
+            "#,
+            r#"
+            struct S {
+                foo: u32,
+                bar: String,
+            }
+
+            fn main() {
+                let s = <|>S { foo: (), bar: () };
+            }
+            "#,
+        );
+    }
 }
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index ded401b63..60b4d5c63 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -7,6 +7,7 @@
 
 mod assist_ctx;
 mod marks;
+pub mod ast_editor;
 
 use itertools::Itertools;
 
@@ -36,7 +37,7 @@ pub struct AssistAction {
     pub target: Option<TextRange>,
 }
 
-/// Return all the assists applicable at the given position.
+/// Return all the assists eapplicable at the given position.
 ///
 /// Assists are returned in the "unresolved" state, that is only labels are
 /// returned, without actual edits.
diff --git a/crates/ra_ide_api/src/completion/complete_postfix.rs b/crates/ra_ide_api/src/completion/complete_postfix.rs
index 4dfa5f176..278b1b797 100644
--- a/crates/ra_ide_api/src/completion/complete_postfix.rs
+++ b/crates/ra_ide_api/src/completion/complete_postfix.rs
@@ -40,7 +40,7 @@ pub(super) fn complete_postfix(acc: &mut Completions, ctx: &CompletionContext) {
             ctx,
             "match",
             "match expr {}",
-            &format!("match {} {{\n${{1:_}} => {{$0\\}},\n}}", receiver_text),
+            &format!("match {} {{\n    ${{1:_}} => {{$0\\}},\n}}", receiver_text),
         )
         .add_to(acc);
         postfix_snippet(
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__postfix_completion_works_for_trivial_path_expression.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__postfix_completion_works_for_trivial_path_expression.snap
index 2b5435f0c..3bbc9e3c4 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__postfix_completion_works_for_trivial_path_expression.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__postfix_completion_works_for_trivial_path_expression.snap
@@ -1,6 +1,6 @@
 ---
-created: "2019-02-18T09:22:24.127119709Z"
-creator: insta@0.6.2
+created: "2019-04-22T07:37:13.981826301Z"
+creator: insta@0.7.4
 source: crates/ra_ide_api/src/completion/completion_item.rs
 expression: kind_completions
 ---
@@ -23,7 +23,7 @@ expression: kind_completions
         label: "match",
         source_range: [76; 76),
         delete: [72; 76),
-        insert: "match bar {\n${1:_} => {$0\\},\n}",
+        insert: "match bar {\n    ${1:_} => {$0\\},\n}",
         detail: "match expr {}"
     },
     CompletionItem {
diff --git a/crates/ra_syntax/src/ast/extensions.rs b/crates/ra_syntax/src/ast/extensions.rs
index 5c4c0ffc1..9cbd2c6b8 100644
--- a/crates/ra_syntax/src/ast/extensions.rs
+++ b/crates/ra_syntax/src/ast/extensions.rs
@@ -210,6 +210,15 @@ impl ast::EnumVariant {
     }
 }
 
+impl ast::FnDef {
+    pub fn semicolon_token(&self) -> Option<SyntaxToken<'_>> {
+        self.syntax()
+            .last_child_or_token()
+            .and_then(|it| it.as_token())
+            .filter(|it| it.kind() == SEMI)
+    }
+}
+
 impl ast::LetStmt {
     pub fn has_semi(&self) -> bool {
         match self.syntax().last_child_or_token() {
diff --git a/crates/ra_syntax/src/lib.rs b/crates/ra_syntax/src/lib.rs
index a6ce14f06..9cb66b76b 100644
--- a/crates/ra_syntax/src/lib.rs
+++ b/crates/ra_syntax/src/lib.rs
@@ -38,7 +38,7 @@ pub use crate::{
     ast::AstNode,
     syntax_error::{SyntaxError, SyntaxErrorKind, Location},
     syntax_text::SyntaxText,
-    syntax_node::{Direction,  SyntaxNode, WalkEvent, TreeArc, SyntaxTreeBuilder, SyntaxElement, SyntaxToken},
+    syntax_node::{Direction,  SyntaxNode, WalkEvent, TreeArc, SyntaxTreeBuilder, SyntaxElement, SyntaxToken, InsertPosition},
     ptr::{SyntaxNodePtr, AstPtr},
     parsing::{tokenize, classify_literal, Token},
 };
diff --git a/crates/ra_syntax/src/ptr.rs b/crates/ra_syntax/src/ptr.rs
index 15a8b94cd..b0816b135 100644
--- a/crates/ra_syntax/src/ptr.rs
+++ b/crates/ra_syntax/src/ptr.rs
@@ -10,7 +10,7 @@ use crate::{
 /// specific node across reparses of the same file.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct SyntaxNodePtr {
-    range: TextRange,
+    pub(crate) range: TextRange,
     kind: SyntaxKind,
 }
 
diff --git a/crates/ra_syntax/src/syntax_node.rs b/crates/ra_syntax/src/syntax_node.rs
index dc2352c76..92c15234e 100644
--- a/crates/ra_syntax/src/syntax_node.rs
+++ b/crates/ra_syntax/src/syntax_node.rs
@@ -7,6 +7,7 @@
 //! modules just wraps its API.
 
 use std::{
+    ops::RangeInclusive,
     fmt::{self, Write},
     any::Any,
     borrow::Borrow,
@@ -17,13 +18,21 @@ use ra_parser::ParseError;
 use rowan::{TransparentNewType, GreenNodeBuilder};
 
 use crate::{
-    SmolStr, SyntaxKind, TextUnit, TextRange, SyntaxText, SourceFile, AstNode,
+    SmolStr, SyntaxKind, TextUnit, TextRange, SyntaxText, SourceFile, AstNode, SyntaxNodePtr,
     syntax_error::{SyntaxError, SyntaxErrorKind},
 };
 
 pub use rowan::WalkEvent;
 pub(crate) use rowan::{GreenNode, GreenToken};
 
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+pub enum InsertPosition<T> {
+    First,
+    Last,
+    Before(T),
+    After(T),
+}
+
 /// Marker trait for CST and AST nodes
 pub trait SyntaxNodeWrapper: TransparentNewType<Repr = rowan::SyntaxNode> {}
 impl<T: TransparentNewType<Repr = rowan::SyntaxNode>> SyntaxNodeWrapper for T {}
@@ -309,6 +318,97 @@ impl SyntaxNode {
     pub(crate) fn replace_with(&self, replacement: GreenNode) -> GreenNode {
         self.0.replace_with(replacement)
     }
+
+    /// Adds specified children (tokens or nodes) to the current node at the
+    /// specific position.
+    ///
+    /// This is a type-unsafe low-level editing API, if you need to use it,
+    /// prefer to create a type-safe abstraction on top of it instead.
+    pub fn insert_children<'a>(
+        &self,
+        position: InsertPosition<SyntaxElement<'_>>,
+        to_insert: impl Iterator<Item = SyntaxElement<'a>>,
+    ) -> TreeArc<SyntaxNode> {
+        let mut delta = TextUnit::default();
+        let to_insert = to_insert.map(|element| {
+            delta += element.text_len();
+            to_green_element(element)
+        });
+
+        let old_children = self.0.green().children();
+
+        let new_children = match position {
+            InsertPosition::First => {
+                to_insert.chain(old_children.iter().cloned()).collect::<Box<[_]>>()
+            }
+            InsertPosition::Last => {
+                old_children.iter().cloned().chain(to_insert).collect::<Box<[_]>>()
+            }
+            InsertPosition::Before(anchor) | InsertPosition::After(anchor) => {
+                let take_anchor = if let InsertPosition::After(_) = position { 1 } else { 0 };
+                let split_at = self.position_of_child(anchor) + take_anchor;
+                let (before, after) = old_children.split_at(split_at);
+                before
+                    .iter()
+                    .cloned()
+                    .chain(to_insert)
+                    .chain(after.iter().cloned())
+                    .collect::<Box<[_]>>()
+            }
+        };
+
+        self.with_children(new_children)
+    }
+
+    /// Replaces all nodes in `to_delete` with nodes from `to_insert`
+    ///
+    /// This is a type-unsafe low-level editing API, if you need to use it,
+    /// prefer to create a type-safe abstraction on top of it instead.
+    pub fn replace_children<'a>(
+        &self,
+        to_delete: RangeInclusive<SyntaxElement<'_>>,
+        to_insert: impl Iterator<Item = SyntaxElement<'a>>,
+    ) -> TreeArc<SyntaxNode> {
+        let start = self.position_of_child(*to_delete.start());
+        let end = self.position_of_child(*to_delete.end());
+        let old_children = self.0.green().children();
+
+        let new_children = old_children[..start]
+            .iter()
+            .cloned()
+            .chain(to_insert.map(to_green_element))
+            .chain(old_children[end + 1..].iter().cloned())
+            .collect::<Box<[_]>>();
+        self.with_children(new_children)
+    }
+
+    fn with_children(&self, new_children: Box<[rowan::GreenElement]>) -> TreeArc<SyntaxNode> {
+        let len = new_children.iter().map(|it| it.text_len()).sum::<TextUnit>();
+        let new_node = GreenNode::new(rowan::SyntaxKind(self.kind() as u16), new_children);
+        let new_file_node = self.replace_with(new_node);
+        let file = SourceFile::new(new_file_node, Vec::new());
+
+        // FIXME: use a more elegant way to re-fetch the node (#1185), make
+        // `range` private afterwards
+        let mut ptr = SyntaxNodePtr::new(self);
+        ptr.range = TextRange::offset_len(ptr.range().start(), len);
+        return ptr.to_node(&file).to_owned();
+    }
+
+    fn position_of_child(&self, child: SyntaxElement) -> usize {
+        self.children_with_tokens()
+            .position(|it| it == child)
+            .expect("elemetn is not a child of current element")
+    }
+}
+
+fn to_green_element(element: SyntaxElement) -> rowan::GreenElement {
+    match element {
+        SyntaxElement::Node(node) => node.0.green().clone().into(),
+        SyntaxElement::Token(tok) => {
+            GreenToken::new(rowan::SyntaxKind(tok.kind() as u16), tok.text().clone()).into()
+        }
+    }
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
@@ -451,6 +551,13 @@ impl<'a> SyntaxElement<'a> {
         }
         .ancestors()
     }
+
+    fn text_len(&self) -> TextUnit {
+        match self {
+            SyntaxElement::Node(node) => node.0.green().text_len(),
+            SyntaxElement::Token(token) => TextUnit::of_str(token.0.text()),
+        }
+    }
 }
 
 impl<'a> From<rowan::SyntaxElement<'a>> for SyntaxElement<'a> {