Merge branch 'master' of github.com:rust-analyzer/rust-analyzer into modname_spacing

47 files changed, 4197 insertions, 1686 deletions

diff --git a/crates/ra_assists/src/assist_config.rs b/crates/ra_assists/src/assist_config.rs
new file mode 100644
index 000000000..c0a0226fb
--- /dev/null
+++ b/crates/ra_assists/src/assist_config.rs
@@ -0,0 +1,27 @@
+//! Settings for tweaking assists.
+//!
+//! The fun thing here is `SnippetCap` -- this type can only be created in this
+//! module, and we use to statically check that we only produce snippet
+//! assists if we are allowed to.
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct AssistConfig {
+    pub snippet_cap: Option<SnippetCap>,
+}
+
+impl AssistConfig {
+    pub fn allow_snippets(&mut self, yes: bool) {
+        self.snippet_cap = if yes { Some(SnippetCap { _private: () }) } else { None }
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct SnippetCap {
+    _private: (),
+}
+
+impl Default for AssistConfig {
+    fn default() -> Self {
+        AssistConfig { snippet_cap: Some(SnippetCap { _private: () }) }
+    }
+}
diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs
new file mode 100644
index 000000000..f3af70a3e
--- /dev/null
+++ b/crates/ra_assists/src/assist_context.rs
@@ -0,0 +1,257 @@
+//! See `AssistContext`
+
+use algo::find_covering_element;
+use hir::Semantics;
+use ra_db::{FileId, FileRange};
+use ra_fmt::{leading_indent, reindent};
+use ra_ide_db::{
+    source_change::{SingleFileChange, SourceChange},
+    RootDatabase,
+};
+use ra_syntax::{
+    algo::{self, find_node_at_offset, SyntaxRewriter},
+    AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize,
+    TokenAtOffset,
+};
+use ra_text_edit::TextEditBuilder;
+
+use crate::{
+    assist_config::{AssistConfig, SnippetCap},
+    Assist, AssistId, GroupLabel, ResolvedAssist,
+};
+
+/// `AssistContext` allows to apply an assist or check if it could be applied.
+///
+/// Assists use a somewhat over-engineered approach, given the current needs.
+/// The assists workflow consists of two phases. In the first phase, a user asks
+/// for the list of available assists. In the second phase, the user picks a
+/// particular assist and it gets applied.
+///
+/// There are two peculiarities here:
+///
+/// * first, we ideally avoid computing more things then necessary to answer "is
+///   assist applicable" in the first phase.
+/// * second, when we are applying assist, we don't have a guarantee that there
+///   weren't any changes between the point when user asked for assists and when
+///   they applied a particular assist. So, when applying assist, we need to do
+///   all the checks from scratch.
+///
+/// To avoid repeating the same code twice for both "check" and "apply"
+/// functions, we use an approach reminiscent of that of Django's function based
+/// views dealing with forms. Each assist receives a runtime parameter,
+/// `resolve`. It first check if an edit is applicable (potentially computing
+/// info required to compute the actual edit). If it is applicable, and
+/// `resolve` is `true`, it then computes the actual edit.
+///
+/// So, to implement the original assists workflow, we can first apply each edit
+/// with `resolve = false`, and then applying the selected edit again, with
+/// `resolve = true` this time.
+///
+/// Note, however, that we don't actually use such two-phase logic at the
+/// moment, because the LSP API is pretty awkward in this place, and it's much
+/// easier to just compute the edit eagerly :-)
+pub(crate) struct AssistContext<'a> {
+    pub(crate) config: &'a AssistConfig,
+    pub(crate) sema: Semantics<'a, RootDatabase>,
+    pub(crate) db: &'a RootDatabase,
+    pub(crate) frange: FileRange,
+    source_file: SourceFile,
+}
+
+impl<'a> AssistContext<'a> {
+    pub(crate) fn new(
+        sema: Semantics<'a, RootDatabase>,
+        config: &'a AssistConfig,
+        frange: FileRange,
+    ) -> AssistContext<'a> {
+        let source_file = sema.parse(frange.file_id);
+        let db = sema.db;
+        AssistContext { config, sema, db, frange, source_file }
+    }
+
+    // NB, this ignores active selection.
+    pub(crate) fn offset(&self) -> TextSize {
+        self.frange.range.start()
+    }
+
+    pub(crate) fn token_at_offset(&self) -> TokenAtOffset<SyntaxToken> {
+        self.source_file.syntax().token_at_offset(self.offset())
+    }
+    pub(crate) fn find_token_at_offset(&self, kind: SyntaxKind) -> Option<SyntaxToken> {
+        self.token_at_offset().find(|it| it.kind() == kind)
+    }
+    pub(crate) fn find_node_at_offset<N: AstNode>(&self) -> Option<N> {
+        find_node_at_offset(self.source_file.syntax(), self.offset())
+    }
+    pub(crate) fn find_node_at_offset_with_descend<N: AstNode>(&self) -> Option<N> {
+        self.sema.find_node_at_offset_with_descend(self.source_file.syntax(), self.offset())
+    }
+    pub(crate) fn covering_element(&self) -> SyntaxElement {
+        find_covering_element(self.source_file.syntax(), self.frange.range)
+    }
+    // FIXME: remove
+    pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement {
+        find_covering_element(self.source_file.syntax(), range)
+    }
+}
+
+pub(crate) struct Assists {
+    resolve: bool,
+    file: FileId,
+    buf: Vec<(Assist, Option<SourceChange>)>,
+}
+
+impl Assists {
+    pub(crate) fn new_resolved(ctx: &AssistContext) -> Assists {
+        Assists { resolve: true, file: ctx.frange.file_id, buf: Vec::new() }
+    }
+    pub(crate) fn new_unresolved(ctx: &AssistContext) -> Assists {
+        Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new() }
+    }
+
+    pub(crate) fn finish_unresolved(self) -> Vec<Assist> {
+        assert!(!self.resolve);
+        self.finish()
+            .into_iter()
+            .map(|(label, edit)| {
+                assert!(edit.is_none());
+                label
+            })
+            .collect()
+    }
+
+    pub(crate) fn finish_resolved(self) -> Vec<ResolvedAssist> {
+        assert!(self.resolve);
+        self.finish()
+            .into_iter()
+            .map(|(label, edit)| ResolvedAssist { assist: label, source_change: edit.unwrap() })
+            .collect()
+    }
+
+    pub(crate) fn add(
+        &mut self,
+        id: AssistId,
+        label: impl Into<String>,
+        target: TextRange,
+        f: impl FnOnce(&mut AssistBuilder),
+    ) -> Option<()> {
+        let label = Assist::new(id, label.into(), None, target);
+        self.add_impl(label, f)
+    }
+    pub(crate) fn add_group(
+        &mut self,
+        group: &GroupLabel,
+        id: AssistId,
+        label: impl Into<String>,
+        target: TextRange,
+        f: impl FnOnce(&mut AssistBuilder),
+    ) -> Option<()> {
+        let label = Assist::new(id, label.into(), Some(group.clone()), target);
+        self.add_impl(label, f)
+    }
+    fn add_impl(&mut self, label: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> {
+        let change_label = label.label.clone();
+        let source_change = if self.resolve {
+            let mut builder = AssistBuilder::new(self.file);
+            f(&mut builder);
+            Some(builder.finish(change_label))
+        } else {
+            None
+        };
+
+        self.buf.push((label, source_change));
+        Some(())
+    }
+
+    fn finish(mut self) -> Vec<(Assist, Option<SourceChange>)> {
+        self.buf.sort_by_key(|(label, _edit)| label.target.len());
+        self.buf
+    }
+}
+
+pub(crate) struct AssistBuilder {
+    edit: TextEditBuilder,
+    file: FileId,
+    is_snippet: bool,
+}
+
+impl AssistBuilder {
+    pub(crate) fn new(file: FileId) -> AssistBuilder {
+        AssistBuilder { edit: TextEditBuilder::default(), file, is_snippet: false }
+    }
+
+    /// Remove specified `range` of text.
+    pub(crate) fn delete(&mut self, range: TextRange) {
+        self.edit.delete(range)
+    }
+    /// Append specified `text` at the given `offset`
+    pub(crate) fn insert(&mut self, offset: TextSize, text: impl Into<String>) {
+        self.edit.insert(offset, text.into())
+    }
+    /// Append specified `snippet` at the given `offset`
+    pub(crate) fn insert_snippet(
+        &mut self,
+        _cap: SnippetCap,
+        offset: TextSize,
+        snippet: impl Into<String>,
+    ) {
+        self.is_snippet = true;
+        self.insert(offset, snippet);
+    }
+    /// Replaces specified `range` of text with a given string.
+    pub(crate) fn replace(&mut self, range: TextRange, replace_with: impl Into<String>) {
+        self.edit.replace(range, replace_with.into())
+    }
+    /// Replaces specified `range` of text with a given `snippet`.
+    pub(crate) fn replace_snippet(
+        &mut self,
+        _cap: SnippetCap,
+        range: TextRange,
+        snippet: impl Into<String>,
+    ) {
+        self.is_snippet = true;
+        self.replace(range, snippet);
+    }
+    pub(crate) fn replace_ast<N: AstNode>(&mut self, old: N, new: N) {
+        algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit)
+    }
+    /// Replaces specified `node` of text with a given string, reindenting the
+    /// string to maintain `node`'s existing indent.
+    // FIXME: remove in favor of ra_syntax::edit::IndentLevel::increase_indent
+    pub(crate) fn replace_node_and_indent(
+        &mut self,
+        node: &SyntaxNode,
+        replace_with: impl Into<String>,
+    ) {
+        let mut replace_with = replace_with.into();
+        if let Some(indent) = leading_indent(node) {
+            replace_with = reindent(&replace_with, &indent)
+        }
+        self.replace(node.text_range(), replace_with)
+    }
+    pub(crate) fn rewrite(&mut self, rewriter: SyntaxRewriter) {
+        let node = rewriter.rewrite_root().unwrap();
+        let new = rewriter.rewrite(&node);
+        algo::diff(&node, &new).into_text_edit(&mut self.edit)
+    }
+
+    // FIXME: better API
+    pub(crate) fn set_file(&mut self, assist_file: FileId) {
+        self.file = assist_file;
+    }
+
+    // FIXME: kill this API
+    /// Get access to the raw `TextEditBuilder`.
+    pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder {
+        &mut self.edit
+    }
+
+    fn finish(self, change_label: String) -> SourceChange {
+        let edit = self.edit.finish();
+        let mut res = SingleFileChange { label: change_label, edit }.into_source_change(self.file);
+        if self.is_snippet {
+            res.is_snippet = true;
+        }
+        res
+    }
+}
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
deleted file mode 100644
index 2fe7c3de3..000000000
--- a/crates/ra_assists/src/assist_ctx.rs
+++ /dev/null
@@ -1,257 +0,0 @@
-//! This module defines `AssistCtx` -- the API surface that is exposed to assists.
-use hir::Semantics;
-use ra_db::FileRange;
-use ra_fmt::{leading_indent, reindent};
-use ra_ide_db::RootDatabase;
-use ra_syntax::{
-    algo::{self, find_covering_element, find_node_at_offset},
-    AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize,
-    TokenAtOffset,
-};
-use ra_text_edit::TextEditBuilder;
-
-use crate::{AssistAction, AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
-use algo::SyntaxRewriter;
-
-#[derive(Clone, Debug)]
-pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
-
-#[derive(Clone, Debug)]
-pub(crate) struct AssistInfo {
-    pub(crate) label: AssistLabel,
-    pub(crate) group_label: Option<GroupLabel>,
-    pub(crate) action: Option<AssistAction>,
-}
-
-impl AssistInfo {
-    fn new(label: AssistLabel) -> AssistInfo {
-        AssistInfo { label, group_label: None, action: None }
-    }
-
-    fn resolved(self, action: AssistAction) -> AssistInfo {
-        AssistInfo { action: Some(action), ..self }
-    }
-
-    fn with_group(self, group_label: GroupLabel) -> AssistInfo {
-        AssistInfo { group_label: Some(group_label), ..self }
-    }
-
-    pub(crate) fn into_resolved(self) -> Option<ResolvedAssist> {
-        let label = self.label;
-        let group_label = self.group_label;
-        self.action.map(|action| ResolvedAssist { label, group_label, action })
-    }
-}
-
-pub(crate) type AssistHandler = fn(AssistCtx) -> Option<Assist>;
-
-/// `AssistCtx` allows to apply an assist or check if it could be applied.
-///
-/// Assists use a somewhat over-engineered approach, given the current needs. The
-/// assists workflow consists of two phases. In the first phase, a user asks for
-/// the list of available assists. In the second phase, the user picks a
-/// particular assist and it gets applied.
-///
-/// There are two peculiarities here:
-///
-/// * first, we ideally avoid computing more things then necessary to answer
-///   "is assist applicable" in the first phase.
-/// * second, when we are applying assist, we don't have a guarantee that there
-///   weren't any changes between the point when user asked for assists and when
-///   they applied a particular assist. So, when applying assist, we need to do
-///   all the checks from scratch.
-///
-/// To avoid repeating the same code twice for both "check" and "apply"
-/// functions, we use an approach reminiscent of that of Django's function based
-/// views dealing with forms. Each assist receives a runtime parameter,
-/// `should_compute_edit`. It first check if an edit is applicable (potentially
-/// computing info required to compute the actual edit). If it is applicable,
-/// and `should_compute_edit` is `true`, it then computes the actual edit.
-///
-/// So, to implement the original assists workflow, we can first apply each edit
-/// with `should_compute_edit = false`, and then applying the selected edit
-/// again, with `should_compute_edit = true` this time.
-///
-/// Note, however, that we don't actually use such two-phase logic at the
-/// moment, because the LSP API is pretty awkward in this place, and it's much
-/// easier to just compute the edit eagerly :-)
-#[derive(Clone)]
-pub(crate) struct AssistCtx<'a> {
-    pub(crate) sema: &'a Semantics<'a, RootDatabase>,
-    pub(crate) db: &'a RootDatabase,
-    pub(crate) frange: FileRange,
-    source_file: SourceFile,
-    should_compute_edit: bool,
-}
-
-impl<'a> AssistCtx<'a> {
-    pub fn new(
-        sema: &'a Semantics<'a, RootDatabase>,
-        frange: FileRange,
-        should_compute_edit: bool,
-    ) -> AssistCtx<'a> {
-        let source_file = sema.parse(frange.file_id);
-        AssistCtx { sema, db: sema.db, frange, source_file, should_compute_edit }
-    }
-
-    pub(crate) fn add_assist(
-        self,
-        id: AssistId,
-        label: impl Into<String>,
-        f: impl FnOnce(&mut ActionBuilder),
-    ) -> Option<Assist> {
-        let label = AssistLabel::new(label.into(), id);
-
-        let mut info = AssistInfo::new(label);
-        if self.should_compute_edit {
-            let action = {
-                let mut edit = ActionBuilder::default();
-                f(&mut edit);
-                edit.build()
-            };
-            info = info.resolved(action)
-        };
-
-        Some(Assist(vec![info]))
-    }
-
-    pub(crate) fn add_assist_group(self, group_name: impl Into<String>) -> AssistGroup<'a> {
-        AssistGroup { ctx: self, group_name: group_name.into(), assists: Vec::new() }
-    }
-
-    pub(crate) fn token_at_offset(&self) -> TokenAtOffset<SyntaxToken> {
-        self.source_file.syntax().token_at_offset(self.frange.range.start())
-    }
-
-    pub(crate) fn find_token_at_offset(&self, kind: SyntaxKind) -> Option<SyntaxToken> {
-        self.token_at_offset().find(|it| it.kind() == kind)
-    }
-
-    pub(crate) fn find_node_at_offset<N: AstNode>(&self) -> Option<N> {
-        find_node_at_offset(self.source_file.syntax(), self.frange.range.start())
-    }
-    pub(crate) fn covering_element(&self) -> SyntaxElement {
-        find_covering_element(self.source_file.syntax(), self.frange.range)
-    }
-    pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement {
-        find_covering_element(self.source_file.syntax(), range)
-    }
-}
-
-pub(crate) struct AssistGroup<'a> {
-    ctx: AssistCtx<'a>,
-    group_name: String,
-    assists: Vec<AssistInfo>,
-}
-
-impl<'a> AssistGroup<'a> {
-    pub(crate) fn add_assist(
-        &mut self,
-        id: AssistId,
-        label: impl Into<String>,
-        f: impl FnOnce(&mut ActionBuilder),
-    ) {
-        let label = AssistLabel::new(label.into(), id);
-
-        let mut info = AssistInfo::new(label).with_group(GroupLabel(self.group_name.clone()));
-        if self.ctx.should_compute_edit {
-            let action = {
-                let mut edit = ActionBuilder::default();
-                f(&mut edit);
-                edit.build()
-            };
-            info = info.resolved(action)
-        };
-
-        self.assists.push(info)
-    }
-
-    pub(crate) fn finish(self) -> Option<Assist> {
-        if self.assists.is_empty() {
-            None
-        } else {
-            Some(Assist(self.assists))
-        }
-    }
-}
-
-#[derive(Default)]
-pub(crate) struct ActionBuilder {
-    edit: TextEditBuilder,
-    cursor_position: Option<TextSize>,
-    target: Option<TextRange>,
-    file: AssistFile,
-}
-
-impl ActionBuilder {
-    /// Replaces specified `range` of text with a given string.
-    pub(crate) fn replace(&mut self, range: TextRange, replace_with: impl Into<String>) {
-        self.edit.replace(range, replace_with.into())
-    }
-
-    /// Replaces specified `node` of text with a given string, reindenting the
-    /// string to maintain `node`'s existing indent.
-    // FIXME: remove in favor of ra_syntax::edit::IndentLevel::increase_indent
-    pub(crate) fn replace_node_and_indent(
-        &mut self,
-        node: &SyntaxNode,
-        replace_with: impl Into<String>,
-    ) {
-        let mut replace_with = replace_with.into();
-        if let Some(indent) = leading_indent(node) {
-            replace_with = reindent(&replace_with, &indent)
-        }
-        self.replace(node.text_range(), replace_with)
-    }
-
-    /// Remove specified `range` of text.
-    #[allow(unused)]
-    pub(crate) fn delete(&mut self, range: TextRange) {
-        self.edit.delete(range)
-    }
-
-    /// Append specified `text` at the given `offset`
-    pub(crate) fn insert(&mut self, offset: TextSize, text: impl Into<String>) {
-        self.edit.insert(offset, text.into())
-    }
-
-    /// Specify desired position of the cursor after the assist is applied.
-    pub(crate) fn set_cursor(&mut self, offset: TextSize) {
-        self.cursor_position = Some(offset)
-    }
-
-    /// Specify that the assist should be active withing the `target` range.
-    ///
-    /// Target ranges are used to sort assists: the smaller the target range,
-    /// the more specific assist is, and so it should be sorted first.
-    pub(crate) fn target(&mut self, target: TextRange) {
-        self.target = Some(target)
-    }
-
-    /// Get access to the raw `TextEditBuilder`.
-    pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder {
-        &mut self.edit
-    }
-
-    pub(crate) fn replace_ast<N: AstNode>(&mut self, old: N, new: N) {
-        algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit)
-    }
-    pub(crate) fn rewrite(&mut self, rewriter: SyntaxRewriter) {
-        let node = rewriter.rewrite_root().unwrap();
-        let new = rewriter.rewrite(&node);
-        algo::diff(&node, &new).into_text_edit(&mut self.edit)
-    }
-
-    pub(crate) fn set_file(&mut self, assist_file: AssistFile) {
-        self.file = assist_file
-    }
-
-    fn build(self) -> AssistAction {
-        AssistAction {
-            edit: self.edit.finish(),
-            cursor_position: self.cursor_position,
-            target: self.target,
-            file: self.file,
-        }
-    }
-}
diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs
index 9ac65ab39..3079a02a2 100644
--- a/crates/ra_assists/src/ast_transform.rs
+++ b/crates/ra_assists/src/ast_transform.rs
@@ -1,7 +1,7 @@
 //! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined.
 use rustc_hash::FxHashMap;
 
-use hir::{PathResolution, SemanticsScope};
+use hir::{HirDisplay, PathResolution, SemanticsScope};
 use ra_ide_db::RootDatabase;
 use ra_syntax::{
     algo::SyntaxRewriter,
@@ -51,7 +51,27 @@ impl<'a> SubstituteTypeParams<'a> {
             .into_iter()
             // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky
             .skip(1)
-            .zip(substs.into_iter())
+            // The actual list of trait type parameters may be longer than the one
+            // used in the `impl` block due to trailing default type parametrs.
+            // For that case we extend the `substs` with an empty iterator so we
+            // can still hit those trailing values and check if they actually have
+            // a default type. If they do, go for that type from `hir` to `ast` so
+            // the resulting change can be applied correctly.
+            .zip(substs.into_iter().map(Some).chain(std::iter::repeat(None)))
+            .filter_map(|(k, v)| match v {
+                Some(v) => Some((k, v)),
+                None => {
+                    let default = k.default(source_scope.db)?;
+                    Some((
+                        k,
+                        ast::make::type_ref(
+                            &default
+                                .display_source_code(source_scope.db, source_scope.module()?.into())
+                                .ok()?,
+                        ),
+                    ))
+                }
+            })
             .collect();
         return SubstituteTypeParams {
             source_scope,
diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs
deleted file mode 100644
index c0f9bc1fb..000000000
--- a/crates/ra_assists/src/doc_tests.rs
+++ /dev/null
@@ -1,35 +0,0 @@
-//! Each assist definition has a special comment, which specifies docs and
-//! example.
-//!
-//! We collect all the example and write the as tests in this module.
-
-mod generated;
-
-use ra_db::FileRange;
-use test_utils::{assert_eq_text, extract_range_or_offset};
-
-use crate::resolved_assists;
-
-fn check(assist_id: &str, before: &str, after: &str) {
-    let (selection, before) = extract_range_or_offset(before);
-    let (db, file_id) = crate::helpers::with_single_file(&before);
-    let frange = FileRange { file_id, range: selection.into() };
-
-    let assist = resolved_assists(&db, frange)
-        .into_iter()
-        .find(|assist| assist.label.id.0 == assist_id)
-        .unwrap_or_else(|| {
-            panic!(
-                "\n\nAssist is not applicable: {}\nAvailable assists: {}",
-                assist_id,
-                resolved_assists(&db, frange)
-                    .into_iter()
-                    .map(|assist| assist.label.id.0)
-                    .collect::<Vec<_>>()
-                    .join(", ")
-            )
-        });
-
-    let actual = assist.action.edit.apply(&before);
-    assert_eq_text!(after, &actual);
-}
diff --git a/crates/ra_assists/src/handlers/add_custom_impl.rs b/crates/ra_assists/src/handlers/add_custom_impl.rs
index 4ea26a550..fa70c8496 100644
--- a/crates/ra_assists/src/handlers/add_custom_impl.rs
+++ b/crates/ra_assists/src/handlers/add_custom_impl.rs
@@ -6,7 +6,10 @@ use ra_syntax::{
 };
 use stdx::SepBy;
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{
+    assist_context::{AssistContext, Assists},
+    AssistId,
+};
 
 // Assist: add_custom_impl
 //
@@ -22,10 +25,10 @@ use crate::{Assist, AssistCtx, AssistId};
 // struct S;
 //
 // impl Debug for S {
-//
+//     $0
 // }
 // ```
-pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let input = ctx.find_node_at_offset::<ast::AttrInput>()?;
     let attr = input.syntax().parent().and_then(ast::Attr::cast)?;
 
@@ -46,11 +49,10 @@ pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option<Assist> {
     let start_offset = annotated.syntax().parent()?.text_range().end();
 
     let label =
-        format!("Add custom impl '{}' for '{}'", trait_token.text().as_str(), annotated_name);
-
-    ctx.add_assist(AssistId("add_custom_impl"), label, |edit| {
-        edit.target(attr.syntax().text_range());
+        format!("Add custom impl `{}` for `{}`", trait_token.text().as_str(), annotated_name);
 
+    let target = attr.syntax().text_range();
+    acc.add(AssistId("add_custom_impl"), label, target, |builder| {
         let new_attr_input = input
             .syntax()
             .descendants_with_tokens()
@@ -61,20 +63,11 @@ pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option<Assist> {
         let has_more_derives = !new_attr_input.is_empty();
         let new_attr_input = new_attr_input.iter().sep_by(", ").surround_with("(", ")").to_string();
 
-        let mut buf = String::new();
-        buf.push_str("\n\nimpl ");
-        buf.push_str(trait_token.text().as_str());
-        buf.push_str(" for ");
-        buf.push_str(annotated_name.as_str());
-        buf.push_str(" {\n");
-
-        let cursor_delta = if has_more_derives {
-            let delta = input.syntax().text_range().len() - TextSize::of(&new_attr_input);
-            edit.replace(input.syntax().text_range(), new_attr_input);
-            delta
+        if has_more_derives {
+            builder.replace(input.syntax().text_range(), new_attr_input);
         } else {
             let attr_range = attr.syntax().text_range();
-            edit.delete(attr_range);
+            builder.delete(attr_range);
 
             let line_break_range = attr
                 .syntax()
@@ -82,20 +75,30 @@ pub(crate) fn add_custom_impl(ctx: AssistCtx) -> Option<Assist> {
                 .filter(|t| t.kind() == WHITESPACE)
                 .map(|t| t.text_range())
                 .unwrap_or_else(|| TextRange::new(TextSize::from(0), TextSize::from(0)));
-            edit.delete(line_break_range);
-
-            attr_range.len() + line_break_range.len()
-        };
-
-        edit.set_cursor(start_offset + TextSize::of(&buf) - cursor_delta);
-        buf.push_str("\n}");
-        edit.insert(start_offset, buf);
+            builder.delete(line_break_range);
+        }
+
+        match ctx.config.snippet_cap {
+            Some(cap) => {
+                builder.insert_snippet(
+                    cap,
+                    start_offset,
+                    format!("\n\nimpl {} for {} {{\n    $0\n}}", trait_token, annotated_name),
+                );
+            }
+            None => {
+                builder.insert(
+                    start_offset,
+                    format!("\n\nimpl {} for {} {{\n\n}}", trait_token, annotated_name),
+                );
+            }
+        }
     })
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_not_applicable};
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
     use super::*;
 
@@ -115,7 +118,7 @@ struct Foo {
 }
 
 impl Debug for Foo {
-<|>
+    $0
 }
             ",
         )
@@ -137,7 +140,7 @@ pub struct Foo {
 }
 
 impl Debug for Foo {
-<|>
+    $0
 }
             ",
         )
@@ -156,7 +159,7 @@ struct Foo {}
 struct Foo {}
 
 impl Debug for Foo {
-<|>
+    $0
 }
             ",
         )
diff --git a/crates/ra_assists/src/handlers/add_derive.rs b/crates/ra_assists/src/handlers/add_derive.rs
index 6254eb7c4..b123b8498 100644
--- a/crates/ra_assists/src/handlers/add_derive.rs
+++ b/crates/ra_assists/src/handlers/add_derive.rs
@@ -4,7 +4,7 @@ use ra_syntax::{
     TextSize,
 };
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{AssistContext, AssistId, Assists};
 
 // Assist: add_derive
 //
@@ -18,31 +18,37 @@ use crate::{Assist, AssistCtx, AssistId};
 // ```
 // ->
 // ```
-// #[derive()]
+// #[derive($0)]
 // struct Point {
 //     x: u32,
 //     y: u32,
 // }
 // ```
-pub(crate) fn add_derive(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_derive(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let cap = ctx.config.snippet_cap?;
     let nominal = ctx.find_node_at_offset::<ast::NominalDef>()?;
     let node_start = derive_insertion_offset(&nominal)?;
-    ctx.add_assist(AssistId("add_derive"), "Add `#[derive]`", |edit| {
+    let target = nominal.syntax().text_range();
+    acc.add(AssistId("add_derive"), "Add `#[derive]`", target, |builder| {
         let derive_attr = nominal
             .attrs()
             .filter_map(|x| x.as_simple_call())
             .filter(|(name, _arg)| name == "derive")
             .map(|(_name, arg)| arg)
             .next();
-        let offset = match derive_attr {
+        match derive_attr {
             None => {
-                edit.insert(node_start, "#[derive()]\n");
-                node_start + TextSize::of("#[derive(")
+                builder.insert_snippet(cap, node_start, "#[derive($0)]\n");
+            }
+            Some(tt) => {
+                // Just move the cursor.
+                builder.insert_snippet(
+                    cap,
+                    tt.syntax().text_range().end() - TextSize::of(')'),
+                    "$0",
+                )
             }
-            Some(tt) => tt.syntax().text_range().end() - TextSize::of(')'),
         };
-        edit.target(nominal.syntax().text_range());
-        edit.set_cursor(offset)
     })
 }
 
@@ -57,20 +63,21 @@ fn derive_insertion_offset(nominal: &ast::NominalDef) -> Option<TextSize> {
 
 #[cfg(test)]
 mod tests {
+    use crate::tests::{check_assist, check_assist_target};
+
     use super::*;
-    use crate::helpers::{check_assist, check_assist_target};
 
     #[test]
     fn add_derive_new() {
         check_assist(
             add_derive,
             "struct Foo { a: i32, <|>}",
-            "#[derive(<|>)]\nstruct Foo { a: i32, }",
+            "#[derive($0)]\nstruct Foo { a: i32, }",
         );
         check_assist(
             add_derive,
             "struct Foo { <|> a: i32, }",
-            "#[derive(<|>)]\nstruct Foo {  a: i32, }",
+            "#[derive($0)]\nstruct Foo {  a: i32, }",
         );
     }
 
@@ -79,7 +86,7 @@ mod tests {
         check_assist(
             add_derive,
             "#[derive(Clone)]\nstruct Foo { a: i32<|>, }",
-            "#[derive(Clone<|>)]\nstruct Foo { a: i32, }",
+            "#[derive(Clone$0)]\nstruct Foo { a: i32, }",
         );
     }
 
@@ -95,7 +102,7 @@ struct Foo { a: i32<|>, }
             "
 /// `Foo` is a pretty important struct.
 /// It does stuff.
-#[derive(<|>)]
+#[derive($0)]
 struct Foo { a: i32, }
             ",
         );
diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs
index bc313782b..ab20c6649 100644
--- a/crates/ra_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ra_assists/src/handlers/add_explicit_type.rs
@@ -4,7 +4,7 @@ use ra_syntax::{
     TextRange,
 };
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{AssistContext, AssistId, Assists};
 
 // Assist: add_explicit_type
 //
@@ -21,12 +21,12 @@ use crate::{Assist, AssistCtx, AssistId};
 //     let x: i32 = 92;
 // }
 // ```
-pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let stmt = ctx.find_node_at_offset::<LetStmt>()?;
+    let module = ctx.sema.scope(stmt.syntax()).module()?;
     let expr = stmt.initializer()?;
-    let pat = stmt.pat()?;
     // Must be a binding
-    let pat = match pat {
+    let pat = match stmt.pat()? {
         ast::Pat::BindPat(bind_pat) => bind_pat,
         _ => return None,
     };
@@ -45,7 +45,7 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
     // Assist not applicable if the type has already been specified
     // and it has no placeholders
     let ascribed_ty = stmt.ascribed_type();
-    if let Some(ref ty) = ascribed_ty {
+    if let Some(ty) = &ascribed_ty {
         if ty.syntax().descendants().find_map(ast::PlaceholderType::cast).is_none() {
             return None;
         }
@@ -57,17 +57,17 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
         return None;
     }
 
-    let db = ctx.db;
-    let new_type_string = ty.display_truncated(db, None).to_string();
-    ctx.add_assist(
+    let inferred_type = ty.display_source_code(ctx.db, module.into()).ok()?;
+    acc.add(
         AssistId("add_explicit_type"),
-        format!("Insert explicit type '{}'", new_type_string),
-        |edit| {
-            edit.target(pat_range);
-            if let Some(ascribed_ty) = ascribed_ty {
-                edit.replace(ascribed_ty.syntax().text_range(), new_type_string);
-            } else {
-                edit.insert(name_range.end(), format!(": {}", new_type_string));
+        format!("Insert explicit type `{}`", inferred_type),
+        pat_range,
+        |builder| match ascribed_ty {
+            Some(ascribed_ty) => {
+                builder.replace(ascribed_ty.syntax().text_range(), inferred_type);
+            }
+            None => {
+                builder.insert(name_range.end(), format!(": {}", inferred_type));
             }
         },
     )
@@ -77,7 +77,7 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
 mod tests {
     use super::*;
 
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
+    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
     #[test]
     fn add_explicit_type_target() {
@@ -86,11 +86,7 @@ mod tests {
 
     #[test]
     fn add_explicit_type_works_for_simple_expr() {
-        check_assist(
-            add_explicit_type,
-            "fn f() { let a<|> = 1; }",
-            "fn f() { let a<|>: i32 = 1; }",
-        );
+        check_assist(add_explicit_type, "fn f() { let a<|> = 1; }", "fn f() { let a: i32 = 1; }");
     }
 
     #[test]
@@ -98,7 +94,7 @@ mod tests {
         check_assist(
             add_explicit_type,
             "fn f() { let a<|>: _ = 1; }",
-            "fn f() { let a<|>: i32 = 1; }",
+            "fn f() { let a: i32 = 1; }",
         );
     }
 
@@ -122,7 +118,7 @@ mod tests {
             }
 
             fn f() {
-                let a<|>: Option<i32> = Option::Some(1);
+                let a: Option<i32> = Option::Some(1);
             }"#,
         );
     }
@@ -132,7 +128,7 @@ mod tests {
         check_assist(
             add_explicit_type,
             r"macro_rules! v { () => {0u64} } fn f() { let a<|> = v!(); }",
-            r"macro_rules! v { () => {0u64} } fn f() { let a<|>: u64 = v!(); }",
+            r"macro_rules! v { () => {0u64} } fn f() { let a: u64 = v!(); }",
         );
     }
 
@@ -140,8 +136,8 @@ mod tests {
     fn add_explicit_type_works_for_macro_call_recursive() {
         check_assist(
             add_explicit_type,
-            "macro_rules! u { () => {0u64} } macro_rules! v { () => {u!()} } fn f() { let a<|> = v!(); }",
-            "macro_rules! u { () => {0u64} } macro_rules! v { () => {u!()} } fn f() { let a<|>: u64 = v!(); }",
+            r#"macro_rules! u { () => {0u64} } macro_rules! v { () => {u!()} } fn f() { let a<|> = v!(); }"#,
+            r#"macro_rules! u { () => {0u64} } macro_rules! v { () => {u!()} } fn f() { let a: u64 = v!(); }"#,
         );
     }
 
@@ -208,7 +204,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let test<|>: Test<i32> = Test { t: 23, k: 33 };
+    let test: Test<i32> = Test { t: 23, k: 33 };
 }"#,
         );
     }
diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
index 49deb6701..6a675e812 100644
--- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
+++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
@@ -1,12 +1,8 @@
 use ra_ide_db::RootDatabase;
-use ra_syntax::{
-    ast::{self, AstNode, NameOwner},
-    TextSize,
-};
-use stdx::format_to;
+use ra_syntax::ast::{self, AstNode, NameOwner};
+use test_utils::mark;
 
-use crate::{utils::FamousDefs, Assist, AssistCtx, AssistId};
-use test_utils::tested_by;
+use crate::{utils::FamousDefs, AssistContext, AssistId, Assists};
 
 // Assist add_from_impl_for_enum
 //
@@ -25,7 +21,7 @@ use test_utils::tested_by;
 //     }
 // }
 // ```
-pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_from_impl_for_enum(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let variant = ctx.find_node_at_offset::<ast::EnumVariant>()?;
     let variant_name = variant.name()?;
     let enum_name = variant.parent_enum().name()?;
@@ -38,23 +34,23 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option<Assist> {
     }
     let field_type = field_list.fields().next()?.type_ref()?;
     let path = match field_type {
-        ast::TypeRef::PathType(p) => p,
+        ast::TypeRef::PathType(it) => it,
         _ => return None,
     };
 
-    if existing_from_impl(ctx.sema, &variant).is_some() {
-        tested_by!(test_add_from_impl_already_exists);
+    if existing_from_impl(&ctx.sema, &variant).is_some() {
+        mark::hit!(test_add_from_impl_already_exists);
         return None;
     }
 
-    ctx.add_assist(
+    let target = variant.syntax().text_range();
+    acc.add(
         AssistId("add_from_impl_for_enum"),
         "Add From impl for this enum variant",
+        target,
         |edit| {
             let start_offset = variant.parent_enum().syntax().text_range().end();
-            let mut buf = String::new();
-            format_to!(
-                buf,
+            let buf = format!(
                 r#"
 
 impl From<{0}> for {1} {{
@@ -67,7 +63,6 @@ impl From<{0}> for {1} {{
                 variant_name
             );
             edit.insert(start_offset, buf);
-            edit.set_cursor(start_offset + TextSize::of("\n\n"));
         },
     )
 }
@@ -95,10 +90,11 @@ fn existing_from_impl(
 
 #[cfg(test)]
 mod tests {
-    use super::*;
+    use test_utils::mark;
+
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
-    use crate::helpers::{check_assist, check_assist_not_applicable};
-    use test_utils::covers;
+    use super::*;
 
     #[test]
     fn test_add_from_impl_for_enum() {
@@ -107,7 +103,7 @@ mod tests {
             "enum A { <|>One(u32) }",
             r#"enum A { One(u32) }
 
-<|>impl From<u32> for A {
+impl From<u32> for A {
     fn from(v: u32) -> Self {
         A::One(v)
     }
@@ -119,10 +115,10 @@ mod tests {
     fn test_add_from_impl_for_enum_complicated_path() {
         check_assist(
             add_from_impl_for_enum,
-            "enum A { <|>One(foo::bar::baz::Boo) }",
+            r#"enum A { <|>One(foo::bar::baz::Boo) }"#,
             r#"enum A { One(foo::bar::baz::Boo) }
 
-<|>impl From<foo::bar::baz::Boo> for A {
+impl From<foo::bar::baz::Boo> for A {
     fn from(v: foo::bar::baz::Boo) -> Self {
         A::One(v)
     }
@@ -153,7 +149,7 @@ mod tests {
 
     #[test]
     fn test_add_from_impl_already_exists() {
-        covers!(test_add_from_impl_already_exists);
+        mark::check!(test_add_from_impl_already_exists);
         check_not_applicable(
             r#"
 enum A { <|>One(u32), }
@@ -184,7 +180,7 @@ pub trait From<T> {
 }"#,
             r#"enum A { One(u32), Two(String), }
 
-<|>impl From<u32> for A {
+impl From<u32> for A {
     fn from(v: u32) -> Self {
         A::One(v)
     }
diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs
index 6c7456579..24f931a85 100644
--- a/crates/ra_assists/src/handlers/add_function.rs
+++ b/crates/ra_assists/src/handlers/add_function.rs
@@ -1,13 +1,21 @@
+use hir::HirDisplay;
+use ra_db::FileId;
 use ra_syntax::{
-    ast::{self, AstNode},
+    ast::{
+        self,
+        edit::{AstNodeEdit, IndentLevel},
+        make, ArgListOwner, AstNode, ModuleItemOwner,
+    },
     SyntaxKind, SyntaxNode, TextSize,
 };
-
-use crate::{Assist, AssistCtx, AssistFile, AssistId};
-use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner};
-use hir::HirDisplay;
 use rustc_hash::{FxHashMap, FxHashSet};
 
+use crate::{
+    assist_config::SnippetCap,
+    utils::{render_snippet, Cursor},
+    AssistContext, AssistId, Assists,
+};
+
 // Assist: add_function
 //
 // Adds a stub function with a signature matching the function under the cursor.
@@ -29,11 +37,11 @@ use rustc_hash::{FxHashMap, FxHashSet};
 // }
 //
 // fn bar(arg: &str, baz: Baz) {
-//     todo!()
+//     ${0:todo!()}
 // }
 //
 // ```
-pub(crate) fn add_function(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let path_expr: ast::PathExpr = ctx.find_node_at_offset()?;
     let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?;
     let path = path_expr.path()?;
@@ -43,36 +51,49 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option<Assist> {
         return None;
     }
 
-    let target_module = if let Some(qualifier) = path.qualifier() {
-        if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) =
-            ctx.sema.resolve_path(&qualifier)
-        {
-            Some(module.definition_source(ctx.sema.db))
-        } else {
-            return None;
-        }
-    } else {
-        None
+    let target_module = match path.qualifier() {
+        Some(qualifier) => match ctx.sema.resolve_path(&qualifier) {
+            Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) => Some(module),
+            _ => return None,
+        },
+        None => None,
     };
 
     let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?;
 
-    ctx.add_assist(AssistId("add_function"), "Add function", |edit| {
-        edit.target(call.syntax().text_range());
-
-        if let Some(function_template) = function_builder.render() {
-            edit.set_file(function_template.file);
-            edit.set_cursor(function_template.cursor_offset);
-            edit.insert(function_template.insert_offset, function_template.fn_def.to_string());
+    let target = call.syntax().text_range();
+    acc.add(AssistId("add_function"), "Add function", target, |builder| {
+        let function_template = function_builder.render();
+        builder.set_file(function_template.file);
+        let new_fn = function_template.to_string(ctx.config.snippet_cap);
+        match ctx.config.snippet_cap {
+            Some(cap) => builder.insert_snippet(cap, function_template.insert_offset, new_fn),
+            None => builder.insert(function_template.insert_offset, new_fn),
         }
     })
 }
 
 struct FunctionTemplate {
     insert_offset: TextSize,
-    cursor_offset: TextSize,
-    fn_def: ast::SourceFile,
-    file: AssistFile,
+    placeholder_expr: ast::MacroCall,
+    leading_ws: String,
+    fn_def: ast::FnDef,
+    trailing_ws: String,
+    file: FileId,
+}
+
+impl FunctionTemplate {
+    fn to_string(&self, cap: Option<SnippetCap>) -> String {
+        let f = match cap {
+            Some(cap) => render_snippet(
+                cap,
+                self.fn_def.syntax(),
+                Cursor::Replace(self.placeholder_expr.syntax()),
+            ),
+            None => self.fn_def.to_string(),
+        };
+        format!("{}{}{}", self.leading_ws, f, self.trailing_ws)
+    }
 }
 
 struct FunctionBuilder {
@@ -80,68 +101,73 @@ struct FunctionBuilder {
     fn_name: ast::Name,
     type_params: Option<ast::TypeParamList>,
     params: ast::ParamList,
-    file: AssistFile,
+    file: FileId,
     needs_pub: bool,
 }
 
 impl FunctionBuilder {
-    /// Prepares a generated function that matches `call` in `generate_in`
-    /// (or as close to `call` as possible, if `generate_in` is `None`)
+    /// Prepares a generated function that matches `call`.
+    /// The function is generated in `target_module` or next to `call`
     fn from_call(
-        ctx: &AssistCtx,
+        ctx: &AssistContext,
         call: &ast::CallExpr,
         path: &ast::Path,
-        target_module: Option<hir::InFile<hir::ModuleSource>>,
+        target_module: Option<hir::Module>,
     ) -> Option<Self> {
-        let needs_pub = target_module.is_some();
-        let mut file = AssistFile::default();
-        let target = if let Some(target_module) = target_module {
-            let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?;
-            file = in_file;
-            target
-        } else {
-            next_space_for_fn_after_call_site(&call)?
+        let mut file = ctx.frange.file_id;
+        let target = match &target_module {
+            Some(target_module) => {
+                let module_source = target_module.definition_source(ctx.db);
+                let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, &module_source)?;
+                file = in_file;
+                target
+            }
+            None => next_space_for_fn_after_call_site(&call)?,
         };
+        let needs_pub = target_module.is_some();
+        let target_module = target_module.or_else(|| ctx.sema.scope(target.syntax()).module())?;
         let fn_name = fn_name(&path)?;
-        let (type_params, params) = fn_args(ctx, &call)?;
+        let (type_params, params) = fn_args(ctx, target_module, &call)?;
+
         Some(Self { target, fn_name, type_params, params, file, needs_pub })
     }
 
-    fn render(self) -> Option<FunctionTemplate> {
-        let placeholder_expr = ast::make::expr_todo();
-        let fn_body = ast::make::block_expr(vec![], Some(placeholder_expr));
-        let mut fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body);
-        if self.needs_pub {
-            fn_def = ast::make::add_pub_crate_modifier(fn_def);
-        }
+    fn render(self) -> FunctionTemplate {
+        let placeholder_expr = make::expr_todo();
+        let fn_body = make::block_expr(vec![], Some(placeholder_expr));
+        let visibility = if self.needs_pub { Some(make::visibility_pub_crate()) } else { None };
+        let mut fn_def =
+            make::fn_def(visibility, self.fn_name, self.type_params, self.params, fn_body);
+        let leading_ws;
+        let trailing_ws;
 
-        let (fn_def, insert_offset) = match self.target {
+        let insert_offset = match self.target {
             GeneratedFunctionTarget::BehindItem(it) => {
-                let with_leading_blank_line = ast::make::add_leading_newlines(2, fn_def);
-                let indented = IndentLevel::from_node(&it).increase_indent(with_leading_blank_line);
-                (indented, it.text_range().end())
+                let indent = IndentLevel::from_node(&it);
+                leading_ws = format!("\n\n{}", indent);
+                fn_def = fn_def.indent(indent);
+                trailing_ws = String::new();
+                it.text_range().end()
             }
             GeneratedFunctionTarget::InEmptyItemList(it) => {
-                let indent_once = IndentLevel(1);
                 let indent = IndentLevel::from_node(it.syntax());
-
-                let fn_def = ast::make::add_leading_newlines(1, fn_def);
-                let fn_def = indent_once.increase_indent(fn_def);
-                let fn_def = ast::make::add_trailing_newlines(1, fn_def);
-                let fn_def = indent.increase_indent(fn_def);
-                (fn_def, it.syntax().text_range().start() + TextSize::of('{'))
+                leading_ws = format!("\n{}", indent + 1);
+                fn_def = fn_def.indent(indent + 1);
+                trailing_ws = format!("\n{}", indent);
+                it.syntax().text_range().start() + TextSize::of('{')
             }
         };
 
-        let cursor_offset_from_fn_start = fn_def
-            .syntax()
-            .descendants()
-            .find_map(ast::MacroCall::cast)?
-            .syntax()
-            .text_range()
-            .start();
-        let cursor_offset = insert_offset + cursor_offset_from_fn_start;
-        Some(FunctionTemplate { insert_offset, cursor_offset, fn_def, file: self.file })
+        let placeholder_expr =
+            fn_def.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
+        FunctionTemplate {
+            insert_offset,
+            placeholder_expr,
+            leading_ws,
+            fn_def,
+            trailing_ws,
+            file: self.file,
+        }
     }
 }
 
@@ -150,32 +176,41 @@ enum GeneratedFunctionTarget {
     InEmptyItemList(ast::ItemList),
 }
 
+impl GeneratedFunctionTarget {
+    fn syntax(&self) -> &SyntaxNode {
+        match self {
+            GeneratedFunctionTarget::BehindItem(it) => it,
+            GeneratedFunctionTarget::InEmptyItemList(it) => it.syntax(),
+        }
+    }
+}
+
 fn fn_name(call: &ast::Path) -> Option<ast::Name> {
     let name = call.segment()?.syntax().to_string();
-    Some(ast::make::name(&name))
+    Some(make::name(&name))
 }
 
 /// Computes the type variables and arguments required for the generated function
 fn fn_args(
-    ctx: &AssistCtx,
+    ctx: &AssistContext,
+    target_module: hir::Module,
     call: &ast::CallExpr,
 ) -> Option<(Option<ast::TypeParamList>, ast::ParamList)> {
     let mut arg_names = Vec::new();
     let mut arg_types = Vec::new();
     for arg in call.arg_list()?.args() {
-        let arg_name = match fn_arg_name(&arg) {
+        arg_names.push(match fn_arg_name(&arg) {
             Some(name) => name,
             None => String::from("arg"),
-        };
-        arg_names.push(arg_name);
-        arg_types.push(match fn_arg_type(ctx, &arg) {
+        });
+        arg_types.push(match fn_arg_type(ctx, target_module, &arg) {
             Some(ty) => ty,
             None => String::from("()"),
         });
     }
     deduplicate_arg_names(&mut arg_names);
-    let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| ast::make::param(name, ty));
-    Some((None, ast::make::param_list(params)))
+    let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| make::param(name, ty));
+    Some((None, make::param_list(params)))
 }
 
 /// Makes duplicate argument names unique by appending incrementing numbers.
@@ -224,12 +259,21 @@ fn fn_arg_name(fn_arg: &ast::Expr) -> Option<String> {
     }
 }
 
-fn fn_arg_type(ctx: &AssistCtx, fn_arg: &ast::Expr) -> Option<String> {
+fn fn_arg_type(
+    ctx: &AssistContext,
+    target_module: hir::Module,
+    fn_arg: &ast::Expr,
+) -> Option<String> {
     let ty = ctx.sema.type_of_expr(fn_arg)?;
     if ty.is_unknown() {
         return None;
     }
-    Some(ty.display(ctx.sema.db).to_string())
+
+    if let Ok(rendered) = ty.display_source_code(ctx.db, target_module.into()) {
+        Some(rendered)
+    } else {
+        None
+    }
 }
 
 /// Returns the position inside the current mod or file
@@ -258,11 +302,10 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option<GeneratedFu
 
 fn next_space_for_fn_in_module(
     db: &dyn hir::db::AstDatabase,
-    module: hir::InFile<hir::ModuleSource>,
-) -> Option<(AssistFile, GeneratedFunctionTarget)> {
-    let file = module.file_id.original_file(db);
-    let assist_file = AssistFile::TargetFile(file);
-    let assist_item = match module.value {
+    module_source: &hir::InFile<hir::ModuleSource>,
+) -> Option<(FileId, GeneratedFunctionTarget)> {
+    let file = module_source.file_id.original_file(db);
+    let assist_item = match &module_source.value {
         hir::ModuleSource::SourceFile(it) => {
             if let Some(last_item) = it.items().last() {
                 GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())
@@ -278,12 +321,12 @@ fn next_space_for_fn_in_module(
             }
         }
     };
-    Some((assist_file, assist_item))
+    Some((file, assist_item))
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_not_applicable};
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
     use super::*;
 
@@ -302,7 +345,7 @@ fn foo() {
 }
 
 fn bar() {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -329,7 +372,7 @@ impl Foo {
 }
 
 fn bar() {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -353,7 +396,7 @@ fn foo1() {
 }
 
 fn bar() {
-    <|>todo!()
+    ${0:todo!()}
 }
 
 fn foo2() {}
@@ -379,7 +422,7 @@ mod baz {
     }
 
     fn bar() {
-        <|>todo!()
+        ${0:todo!()}
     }
 }
 ",
@@ -405,7 +448,7 @@ fn foo() {
 }
 
 fn bar(baz: Baz) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         );
@@ -438,7 +481,7 @@ impl Baz {
 }
 
 fn bar(baz: Baz) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -459,7 +502,7 @@ fn foo() {
 }
 
 fn bar(arg: &str) {
-    <|>todo!()
+    ${0:todo!()}
 }
 "#,
         )
@@ -480,7 +523,7 @@ fn foo() {
 }
 
 fn bar(arg: char) {
-    <|>todo!()
+    ${0:todo!()}
 }
 "#,
         )
@@ -501,7 +544,7 @@ fn foo() {
 }
 
 fn bar(arg: i32) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -522,7 +565,7 @@ fn foo() {
 }
 
 fn bar(arg: u8) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -547,7 +590,7 @@ fn foo() {
 }
 
 fn bar(x: u8) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -570,7 +613,7 @@ fn foo() {
 }
 
 fn bar(worble: ()) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -599,15 +642,40 @@ fn baz() {
 }
 
 fn bar(foo: impl Foo) {
-    <|>todo!()
+    ${0:todo!()}
+}
+",
+        )
+    }
+
+    #[test]
+    fn borrowed_arg() {
+        check_assist(
+            add_function,
+            r"
+struct Baz;
+fn baz() -> Baz { todo!() }
+
+fn foo() {
+    bar<|>(&baz())
+}
+",
+            r"
+struct Baz;
+fn baz() -> Baz { todo!() }
+
+fn foo() {
+    bar(&baz())
+}
+
+fn bar(baz: &Baz) {
+    ${0:todo!()}
 }
 ",
         )
     }
 
     #[test]
-    #[ignore]
-    // FIXME print paths properly to make this test pass
     fn add_function_with_qualified_path_arg() {
         check_assist(
             add_function,
@@ -616,10 +684,8 @@ mod Baz {
     pub struct Bof;
     pub fn baz() -> Bof { Bof }
 }
-mod Foo {
-    fn foo() {
-        <|>bar(super::Baz::baz())
-    }
+fn foo() {
+    <|>bar(Baz::baz())
 }
 ",
             r"
@@ -627,14 +693,12 @@ mod Baz {
     pub struct Bof;
     pub fn baz() -> Bof { Bof }
 }
-mod Foo {
-    fn foo() {
-        bar(super::Baz::baz())
-    }
+fn foo() {
+    bar(Baz::baz())
+}
 
-    fn bar(baz: super::Baz::Bof) {
-        <|>todo!()
-    }
+fn bar(baz: Baz::Bof) {
+    ${0:todo!()}
 }
 ",
         )
@@ -657,7 +721,7 @@ fn foo<T>(t: T) {
 }
 
 fn bar<T>(t: T) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -688,7 +752,7 @@ fn foo() {
 }
 
 fn bar(arg: fn() -> Baz) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -713,7 +777,7 @@ fn foo() {
 }
 
 fn bar(closure: impl Fn(i64) -> i64) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -734,7 +798,7 @@ fn foo() {
 }
 
 fn bar(baz: ()) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -759,7 +823,7 @@ fn foo() {
 }
 
 fn bar(baz_1: Baz, baz_2: Baz) {
-    <|>todo!()
+    ${0:todo!()}
 }
 ",
         )
@@ -784,7 +848,7 @@ fn foo() {
 }
 
 fn bar(baz_1: Baz, baz_2: Baz, arg_1: &str, arg_2: &str) {
-    <|>todo!()
+    ${0:todo!()}
 }
 "#,
         )
@@ -804,7 +868,7 @@ fn foo() {
             r"
 mod bar {
     pub(crate) fn my_fn() {
-        <|>todo!()
+        ${0:todo!()}
     }
 }
 
@@ -816,6 +880,40 @@ fn foo() {
     }
 
     #[test]
+    #[ignore]
+    // Ignored until local imports are supported.
+    // See https://github.com/rust-analyzer/rust-analyzer/issues/1165
+    fn qualified_path_uses_correct_scope() {
+        check_assist(
+            add_function,
+            "
+mod foo {
+    pub struct Foo;
+}
+fn bar() {
+    use foo::Foo;
+    let foo = Foo;
+    baz<|>(foo)
+}
+",
+            "
+mod foo {
+    pub struct Foo;
+}
+fn bar() {
+    use foo::Foo;
+    let foo = Foo;
+    baz(foo)
+}
+
+fn baz(foo: foo::Foo) {
+    ${0:todo!()}
+}
+",
+        )
+    }
+
+    #[test]
     fn add_function_in_module_containing_other_items() {
         check_assist(
             add_function,
@@ -833,7 +931,7 @@ mod bar {
     fn something_else() {}
 
     pub(crate) fn my_fn() {
-        <|>todo!()
+        ${0:todo!()}
     }
 }
 
@@ -861,7 +959,7 @@ fn foo() {
 mod bar {
     mod baz {
         pub(crate) fn my_fn() {
-            <|>todo!()
+            ${0:todo!()}
         }
     }
 }
@@ -890,7 +988,7 @@ fn main() {
 
 
 pub(crate) fn bar() {
-    <|>todo!()
+    ${0:todo!()}
 }",
         )
     }
@@ -927,21 +1025,6 @@ fn bar(baz: ()) {}
     }
 
     #[test]
-    fn add_function_not_applicable_if_function_path_not_singleton() {
-        // In the future this assist could be extended to generate functions
-        // if the path is in the same crate (or even the same workspace).
-        // For the beginning, I think this is fine.
-        check_assist_not_applicable(
-            add_function,
-            r"
-fn foo() {
-    other_crate::bar<|>();
-}
-        ",
-        )
-    }
-
-    #[test]
     #[ignore]
     fn create_method_with_no_args() {
         check_assist(
diff --git a/crates/ra_assists/src/handlers/add_impl.rs b/crates/ra_assists/src/handlers/add_impl.rs
index d26f8b93d..eceba7d0a 100644
--- a/crates/ra_assists/src/handlers/add_impl.rs
+++ b/crates/ra_assists/src/handlers/add_impl.rs
@@ -1,10 +1,7 @@
-use ra_syntax::{
-    ast::{self, AstNode, NameOwner, TypeParamsOwner},
-    TextSize,
-};
+use ra_syntax::ast::{self, AstNode, NameOwner, TypeParamsOwner};
 use stdx::{format_to, SepBy};
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{AssistContext, AssistId, Assists};
 
 // Assist: add_impl
 //
@@ -12,24 +9,24 @@ use crate::{Assist, AssistCtx, AssistId};
 //
 // ```
 // struct Ctx<T: Clone> {
-//      data: T,<|>
+//     data: T,<|>
 // }
 // ```
 // ->
 // ```
 // struct Ctx<T: Clone> {
-//      data: T,
+//     data: T,
 // }
 //
 // impl<T: Clone> Ctx<T> {
-//
+//     $0
 // }
 // ```
-pub(crate) fn add_impl(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let nominal = ctx.find_node_at_offset::<ast::NominalDef>()?;
     let name = nominal.name()?;
-    ctx.add_assist(AssistId("add_impl"), format!("Implement {}", name.text().as_str()), |edit| {
-        edit.target(nominal.syntax().text_range());
+    let target = nominal.syntax().text_range();
+    acc.add(AssistId("add_impl"), format!("Implement {}", name.text().as_str()), target, |edit| {
         let type_params = nominal.type_param_list();
         let start_offset = nominal.syntax().text_range().end();
         let mut buf = String::new();
@@ -50,30 +47,37 @@ pub(crate) fn add_impl(ctx: AssistCtx) -> Option<Assist> {
             let generic_params = lifetime_params.chain(type_params).sep_by(", ");
             format_to!(buf, "<{}>", generic_params)
         }
-        buf.push_str(" {\n");
-        edit.set_cursor(start_offset + TextSize::of(&buf));
-        buf.push_str("\n}");
-        edit.insert(start_offset, buf);
+        match ctx.config.snippet_cap {
+            Some(cap) => {
+                buf.push_str(" {\n    $0\n}");
+                edit.insert_snippet(cap, start_offset, buf);
+            }
+            None => {
+                buf.push_str(" {\n}");
+                edit.insert(start_offset, buf);
+            }
+        }
     })
 }
 
 #[cfg(test)]
 mod tests {
+    use crate::tests::{check_assist, check_assist_target};
+
     use super::*;
-    use crate::helpers::{check_assist, check_assist_target};
 
     #[test]
     fn test_add_impl() {
-        check_assist(add_impl, "struct Foo {<|>}\n", "struct Foo {}\n\nimpl Foo {\n<|>\n}\n");
+        check_assist(add_impl, "struct Foo {<|>}\n", "struct Foo {}\n\nimpl Foo {\n    $0\n}\n");
         check_assist(
             add_impl,
             "struct Foo<T: Clone> {<|>}",
-            "struct Foo<T: Clone> {}\n\nimpl<T: Clone> Foo<T> {\n<|>\n}",
+            "struct Foo<T: Clone> {}\n\nimpl<T: Clone> Foo<T> {\n    $0\n}",
         );
         check_assist(
             add_impl,
             "struct Foo<'a, T: Foo<'a>> {<|>}",
-            "struct Foo<'a, T: Foo<'a>> {}\n\nimpl<'a, T: Foo<'a>> Foo<'a, T> {\n<|>\n}",
+            "struct Foo<'a, T: Foo<'a>> {}\n\nimpl<'a, T: Foo<'a>> Foo<'a, T> {\n    $0\n}",
         );
     }
 
diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index 2d6d44980..abacd4065 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -1,13 +1,18 @@
 use hir::HasSource;
 use ra_syntax::{
-    ast::{self, edit, make, AstNode, NameOwner},
+    ast::{
+        self,
+        edit::{self, AstNodeEdit, IndentLevel},
+        make, AstNode, NameOwner,
+    },
     SmolStr,
 };
 
 use crate::{
+    assist_context::{AssistContext, Assists},
     ast_transform::{self, AstTransform, QualifyPaths, SubstituteTypeParams},
-    utils::{get_missing_impl_items, resolve_target_trait},
-    Assist, AssistCtx, AssistId,
+    utils::{get_missing_assoc_items, render_snippet, resolve_target_trait, Cursor},
+    AssistId,
 };
 
 #[derive(PartialEq)]
@@ -40,12 +45,15 @@ enum AddMissingImplMembersMode {
 // }
 //
 // impl Trait<u32> for () {
-//     fn foo(&self) -> u32 { todo!() }
+//     fn foo(&self) -> u32 {
+//         ${0:todo!()}
+//     }
 //
 // }
 // ```
-pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     add_missing_impl_members_inner(
+        acc,
         ctx,
         AddMissingImplMembersMode::NoDefaultMethods,
         "add_impl_missing_members",
@@ -81,12 +89,13 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option<Assist> {
 // impl Trait for () {
 //     Type X = ();
 //     fn foo(&self) {}
-//     fn bar(&self) {}
+//     $0fn bar(&self) {}
 //
 // }
 // ```
-pub(crate) fn add_missing_default_members(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_missing_default_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     add_missing_impl_members_inner(
+        acc,
         ctx,
         AddMissingImplMembersMode::DefaultMethodsOnly,
         "add_impl_default_members",
@@ -95,36 +104,37 @@ pub(crate) fn add_missing_default_members(ctx: AssistCtx) -> Option<Assist> {
 }
 
 fn add_missing_impl_members_inner(
-    ctx: AssistCtx,
+    acc: &mut Assists,
+    ctx: &AssistContext,
     mode: AddMissingImplMembersMode,
     assist_id: &'static str,
     label: &'static str,
-) -> Option<Assist> {
+) -> Option<()> {
     let _p = ra_prof::profile("add_missing_impl_members_inner");
-    let impl_node = ctx.find_node_at_offset::<ast::ImplDef>()?;
-    let impl_item_list = impl_node.item_list()?;
+    let impl_def = ctx.find_node_at_offset::<ast::ImplDef>()?;
+    let impl_item_list = impl_def.item_list()?;
 
-    let trait_ = resolve_target_trait(&ctx.sema, &impl_node)?;
+    let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?;
 
-    let def_name = |item: &ast::ImplItem| -> Option<SmolStr> {
+    let def_name = |item: &ast::AssocItem| -> Option<SmolStr> {
         match item {
-            ast::ImplItem::FnDef(def) => def.name(),
-            ast::ImplItem::TypeAliasDef(def) => def.name(),
-            ast::ImplItem::ConstDef(def) => def.name(),
+            ast::AssocItem::FnDef(def) => def.name(),
+            ast::AssocItem::TypeAliasDef(def) => def.name(),
+            ast::AssocItem::ConstDef(def) => def.name(),
         }
         .map(|it| it.text().clone())
     };
 
-    let missing_items = get_missing_impl_items(&ctx.sema, &impl_node)
+    let missing_items = get_missing_assoc_items(&ctx.sema, &impl_def)
         .iter()
         .map(|i| match i {
-            hir::AssocItem::Function(i) => ast::ImplItem::FnDef(i.source(ctx.db).value),
-            hir::AssocItem::TypeAlias(i) => ast::ImplItem::TypeAliasDef(i.source(ctx.db).value),
-            hir::AssocItem::Const(i) => ast::ImplItem::ConstDef(i.source(ctx.db).value),
+            hir::AssocItem::Function(i) => ast::AssocItem::FnDef(i.source(ctx.db).value),
+            hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAliasDef(i.source(ctx.db).value),
+            hir::AssocItem::Const(i) => ast::AssocItem::ConstDef(i.source(ctx.db).value),
         })
         .filter(|t| def_name(&t).is_some())
         .filter(|t| match t {
-            ast::ImplItem::FnDef(def) => match mode {
+            ast::AssocItem::FnDef(def) => match mode {
                 AddMissingImplMembersMode::DefaultMethodsOnly => def.body().is_some(),
                 AddMissingImplMembersMode::NoDefaultMethods => def.body().is_none(),
             },
@@ -136,44 +146,59 @@ fn add_missing_impl_members_inner(
         return None;
     }
 
-    let sema = ctx.sema;
-
-    ctx.add_assist(AssistId(assist_id), label, |edit| {
-        let n_existing_items = impl_item_list.impl_items().count();
-        let source_scope = sema.scope_for_def(trait_);
-        let target_scope = sema.scope(impl_item_list.syntax());
+    let target = impl_def.syntax().text_range();
+    acc.add(AssistId(assist_id), label, target, |builder| {
+        let n_existing_items = impl_item_list.assoc_items().count();
+        let source_scope = ctx.sema.scope_for_def(trait_);
+        let target_scope = ctx.sema.scope(impl_item_list.syntax());
         let ast_transform = QualifyPaths::new(&target_scope, &source_scope)
-            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_node));
+            .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def));
         let items = missing_items
             .into_iter()
             .map(|it| ast_transform::apply(&*ast_transform, it))
             .map(|it| match it {
-                ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)),
+                ast::AssocItem::FnDef(def) => ast::AssocItem::FnDef(add_body(def)),
                 _ => it,
             })
             .map(|it| edit::remove_attrs_and_docs(&it));
         let new_impl_item_list = impl_item_list.append_items(items);
-        let cursor_position = {
-            let first_new_item = new_impl_item_list.impl_items().nth(n_existing_items).unwrap();
-            first_new_item.syntax().text_range().start()
+        let first_new_item = new_impl_item_list.assoc_items().nth(n_existing_items).unwrap();
+
+        let original_range = impl_item_list.syntax().text_range();
+        match ctx.config.snippet_cap {
+            None => builder.replace(original_range, new_impl_item_list.to_string()),
+            Some(cap) => {
+                let mut cursor = Cursor::Before(first_new_item.syntax());
+                let placeholder;
+                if let ast::AssocItem::FnDef(func) = &first_new_item {
+                    if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) {
+                        if m.syntax().text() == "todo!()" {
+                            placeholder = m;
+                            cursor = Cursor::Replace(placeholder.syntax());
+                        }
+                    }
+                }
+                builder.replace_snippet(
+                    cap,
+                    original_range,
+                    render_snippet(cap, new_impl_item_list.syntax(), cursor),
+                )
+            }
         };
-
-        edit.replace_ast(impl_item_list, new_impl_item_list);
-        edit.set_cursor(cursor_position);
     })
 }
 
 fn add_body(fn_def: ast::FnDef) -> ast::FnDef {
-    if fn_def.body().is_none() {
-        fn_def.with_body(make::block_from_expr(make::expr_todo()))
-    } else {
-        fn_def
+    if fn_def.body().is_some() {
+        return fn_def;
     }
+    let body = make::block_expr(None, Some(make::expr_todo())).indent(IndentLevel(1));
+    fn_def.with_body(body)
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_not_applicable};
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
     use super::*;
 
@@ -181,7 +206,7 @@ mod tests {
     fn test_add_missing_impl_members() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo {
     type Output;
 
@@ -197,8 +222,8 @@ struct S;
 impl Foo for S {
     fn bar(&self) {}
 <|>
-}",
-            "
+}"#,
+            r#"
 trait Foo {
     type Output;
 
@@ -213,12 +238,16 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    <|>type Output;
+    $0type Output;
     const CONST: usize = 42;
-    fn foo(&self) { todo!() }
-    fn baz(&self) { todo!() }
+    fn foo(&self) {
+        todo!()
+    }
+    fn baz(&self) {
+        todo!()
+    }
 
-}",
+}"#,
         );
     }
 
@@ -226,7 +255,7 @@ impl Foo for S {
     fn test_copied_overriden_members() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo {
     fn foo(&self);
     fn bar(&self) -> bool { true }
@@ -238,8 +267,8 @@ struct S;
 impl Foo for S {
     fn bar(&self) {}
 <|>
-}",
-            "
+}"#,
+            r#"
 trait Foo {
     fn foo(&self);
     fn bar(&self) -> bool { true }
@@ -250,9 +279,11 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
-    <|>fn foo(&self) { todo!() }
+    fn foo(&self) {
+        ${0:todo!()}
+    }
 
-}",
+}"#,
         );
     }
 
@@ -260,16 +291,18 @@ impl Foo for S {
     fn test_empty_impl_def() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo { fn foo(&self); }
 struct S;
-impl Foo for S { <|> }",
-            "
+impl Foo for S { <|> }"#,
+            r#"
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    <|>fn foo(&self) { todo!() }
-}",
+    fn foo(&self) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -277,16 +310,18 @@ impl Foo for S {
     fn fill_in_type_params_1() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
-impl Foo<u32> for S { <|> }",
-            "
+impl Foo<u32> for S { <|> }"#,
+            r#"
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
 impl Foo<u32> for S {
-    <|>fn foo(&self, t: u32) -> &u32 { todo!() }
-}",
+    fn foo(&self, t: u32) -> &u32 {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -294,16 +329,18 @@ impl Foo<u32> for S {
     fn fill_in_type_params_2() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
-impl<U> Foo<U> for S { <|> }",
-            "
+impl<U> Foo<U> for S { <|> }"#,
+            r#"
 trait Foo<T> { fn foo(&self, t: T) -> &T; }
 struct S;
 impl<U> Foo<U> for S {
-    <|>fn foo(&self, t: U) -> &U { todo!() }
-}",
+    fn foo(&self, t: U) -> &U {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -311,16 +348,18 @@ impl<U> Foo<U> for S {
     fn test_cursor_after_empty_impl_def() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo { fn foo(&self); }
 struct S;
-impl Foo for S {}<|>",
-            "
+impl Foo for S {}<|>"#,
+            r#"
 trait Foo { fn foo(&self); }
 struct S;
 impl Foo for S {
-    <|>fn foo(&self) { todo!() }
-}",
+    fn foo(&self) {
+        ${0:todo!()}
+    }
+}"#,
         )
     }
 
@@ -328,22 +367,24 @@ impl Foo for S {
     fn test_qualify_path_1() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub struct Bar;
     trait Foo { fn foo(&self, bar: Bar); }
 }
 struct S;
-impl foo::Foo for S { <|> }",
-            "
+impl foo::Foo for S { <|> }"#,
+            r#"
 mod foo {
     pub struct Bar;
     trait Foo { fn foo(&self, bar: Bar); }
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar) { todo!() }
-}",
+    fn foo(&self, bar: foo::Bar) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -351,22 +392,24 @@ impl foo::Foo for S {
     fn test_qualify_path_generic() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub struct Bar<T>;
     trait Foo { fn foo(&self, bar: Bar<u32>); }
 }
 struct S;
-impl foo::Foo for S { <|> }",
-            "
+impl foo::Foo for S { <|> }"#,
+            r#"
 mod foo {
     pub struct Bar<T>;
     trait Foo { fn foo(&self, bar: Bar<u32>); }
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>) { todo!() }
-}",
+    fn foo(&self, bar: foo::Bar<u32>) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -374,22 +417,24 @@ impl foo::Foo for S {
     fn test_qualify_path_and_substitute_param() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub struct Bar<T>;
     trait Foo<T> { fn foo(&self, bar: Bar<T>); }
 }
 struct S;
-impl foo::Foo<u32> for S { <|> }",
-            "
+impl foo::Foo<u32> for S { <|> }"#,
+            r#"
 mod foo {
     pub struct Bar<T>;
     trait Foo<T> { fn foo(&self, bar: Bar<T>); }
 }
 struct S;
 impl foo::Foo<u32> for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>) { todo!() }
-}",
+    fn foo(&self, bar: foo::Bar<u32>) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -398,15 +443,15 @@ impl foo::Foo<u32> for S {
         // when substituting params, the substituted param should not be qualified!
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     trait Foo<T> { fn foo(&self, bar: T); }
     pub struct Param;
 }
 struct Param;
 struct S;
-impl foo::Foo<Param> for S { <|> }",
-            "
+impl foo::Foo<Param> for S { <|> }"#,
+            r#"
 mod foo {
     trait Foo<T> { fn foo(&self, bar: T); }
     pub struct Param;
@@ -414,8 +459,10 @@ mod foo {
 struct Param;
 struct S;
 impl foo::Foo<Param> for S {
-    <|>fn foo(&self, bar: Param) { todo!() }
-}",
+    fn foo(&self, bar: Param) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -423,15 +470,15 @@ impl foo::Foo<Param> for S {
     fn test_qualify_path_associated_item() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub struct Bar<T>;
     impl Bar<T> { type Assoc = u32; }
     trait Foo { fn foo(&self, bar: Bar<u32>::Assoc); }
 }
 struct S;
-impl foo::Foo for S { <|> }",
-            "
+impl foo::Foo for S { <|> }"#,
+            r#"
 mod foo {
     pub struct Bar<T>;
     impl Bar<T> { type Assoc = u32; }
@@ -439,8 +486,10 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<u32>::Assoc) { todo!() }
-}",
+    fn foo(&self, bar: foo::Bar<u32>::Assoc) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -448,15 +497,15 @@ impl foo::Foo for S {
     fn test_qualify_path_nested() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub struct Bar<T>;
     pub struct Baz;
     trait Foo { fn foo(&self, bar: Bar<Baz>); }
 }
 struct S;
-impl foo::Foo for S { <|> }",
-            "
+impl foo::Foo for S { <|> }"#,
+            r#"
 mod foo {
     pub struct Bar<T>;
     pub struct Baz;
@@ -464,8 +513,10 @@ mod foo {
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: foo::Bar<foo::Baz>) { todo!() }
-}",
+    fn foo(&self, bar: foo::Bar<foo::Baz>) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -473,22 +524,24 @@ impl foo::Foo for S {
     fn test_qualify_path_fn_trait_notation() {
         check_assist(
             add_missing_impl_members,
-            "
+            r#"
 mod foo {
     pub trait Fn<Args> { type Output; }
     trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
 }
 struct S;
-impl foo::Foo for S { <|> }",
-            "
+impl foo::Foo for S { <|> }"#,
+            r#"
 mod foo {
     pub trait Fn<Args> { type Output; }
     trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
 }
 struct S;
 impl foo::Foo for S {
-    <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { todo!() }
-}",
+    fn foo(&self, bar: dyn Fn(u32) -> i32) {
+        ${0:todo!()}
+    }
+}"#,
         );
     }
 
@@ -496,10 +549,10 @@ impl foo::Foo for S {
     fn test_empty_trait() {
         check_assist_not_applicable(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo;
 struct S;
-impl Foo for S { <|> }",
+impl Foo for S { <|> }"#,
         )
     }
 
@@ -507,13 +560,13 @@ impl Foo for S { <|> }",
     fn test_ignore_unnamed_trait_members_and_default_methods() {
         check_assist_not_applicable(
             add_missing_impl_members,
-            "
+            r#"
 trait Foo {
     fn (arg: u32);
     fn valid(some: u32) -> bool { false }
 }
 struct S;
-impl Foo for S { <|> }",
+impl Foo for S { <|> }"#,
         )
     }
 
@@ -543,8 +596,10 @@ trait Foo {
 }
 struct S;
 impl Foo for S {
-    <|>type Output;
-    fn foo(&self) { todo!() }
+    $0type Output;
+    fn foo(&self) {
+        todo!()
+    }
 }"#,
         )
     }
@@ -553,7 +608,7 @@ impl Foo for S {
     fn test_default_methods() {
         check_assist(
             add_missing_default_members,
-            "
+            r#"
 trait Foo {
     type Output;
 
@@ -563,8 +618,8 @@ trait Foo {
     fn foo(some: u32) -> bool;
 }
 struct S;
-impl Foo for S { <|> }",
-            "
+impl Foo for S { <|> }"#,
+            r#"
 trait Foo {
     type Output;
 
@@ -575,8 +630,58 @@ trait Foo {
 }
 struct S;
 impl Foo for S {
-    <|>fn valid(some: u32) -> bool { false }
-}",
+    $0fn valid(some: u32) -> bool { false }
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_generic_single_default_parameter() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S { <|> }"#,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S {
+    fn bar(&self, other: &Self) {
+        ${0:todo!()}
+    }
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_generic_default_parameter_is_second() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> { <|> }"#,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> {
+    fn bar(&self, this: &T, that: &Self) {
+        ${0:todo!()}
+    }
+}"#,
         )
     }
 }
diff --git a/crates/ra_assists/src/handlers/add_new.rs b/crates/ra_assists/src/handlers/add_new.rs
index 0f9174a29..837aa8377 100644
--- a/crates/ra_assists/src/handlers/add_new.rs
+++ b/crates/ra_assists/src/handlers/add_new.rs
@@ -3,11 +3,11 @@ use ra_syntax::{
     ast::{
         self, AstNode, NameOwner, StructKind, TypeAscriptionOwner, TypeParamsOwner, VisibilityOwner,
     },
-    TextSize, T,
+    T,
 };
 use stdx::{format_to, SepBy};
 
-use crate::{Assist, AssistCtx, AssistId};
+use crate::{AssistContext, AssistId, Assists};
 
 // Assist: add_new
 //
@@ -25,11 +25,11 @@ use crate::{Assist, AssistCtx, AssistId};
 // }
 //
 // impl<T: Clone> Ctx<T> {
-//     fn new(data: T) -> Self { Self { data } }
+//     fn $0new(data: T) -> Self { Self { data } }
 // }
 //
 // ```
-pub(crate) fn add_new(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn add_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let strukt = ctx.find_node_at_offset::<ast::StructDef>()?;
 
     // We want to only apply this to non-union structs with named fields
@@ -41,33 +41,27 @@ pub(crate) fn add_new(ctx: AssistCtx) -> Option<Assist> {
     // Return early if we've found an existing new fn
     let impl_def = find_struct_impl(&ctx, &strukt)?;
 
-    ctx.add_assist(AssistId("add_new"), "Add default constructor", |edit| {
-        edit.target(strukt.syntax().text_range());
-
+    let target = strukt.syntax().text_range();
+    acc.add(AssistId("add_new"), "Add default constructor", target, |builder| {
         let mut buf = String::with_capacity(512);
 
         if impl_def.is_some() {
             buf.push('\n');
         }
 
-        let vis = strukt.visibility().map(|v| format!("{} ", v));
-        let vis = vis.as_deref().unwrap_or("");
+        let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v));
 
         let params = field_list
             .fields()
             .filter_map(|f| {
-                Some(format!(
-                    "{}: {}",
-                    f.name()?.syntax().text(),
-                    f.ascribed_type()?.syntax().text()
-                ))
+                Some(format!("{}: {}", f.name()?.syntax(), f.ascribed_type()?.syntax()))
             })
             .sep_by(", ");
         let fields = field_list.fields().filter_map(|f| f.name()).sep_by(", ");
 
         format_to!(buf, "    {}fn new({}) -> Self {{ Self {{ {} }} }}", vis, params, fields);
 
-        let (start_offset, end_offset) = impl_def
+        let start_offset = impl_def
             .and_then(|impl_def| {
                 buf.push('\n');
                 let start = impl_def
@@ -77,17 +71,20 @@ pub(crate) fn add_new(ctx: AssistCtx) -> Option<Assist> {
                     .text_range()
                     .end();
 
-                Some((start, TextSize::of("\n")))
+                Some(start)
             })
             .unwrap_or_else(|| {
                 buf = generate_impl_text(&strukt, &buf);
-                let start = strukt.syntax().text_range().end();
-
-                (start, TextSize::of("\n}\n"))
+                strukt.syntax().text_range().end()
             });
 
-        edit.set_cursor(start_offset + TextSize::of(&buf) - end_offset);
-        edit.insert(start_offset, buf);
+        match ctx.config.snippet_cap {
+            None => builder.insert(start_offset, buf),
+            Some(cap) => {
+                buf = buf.replace("fn new", "fn $0new");
+                builder.insert_snippet(cap, start_offset, buf);
+            }
+        }
     })
 }
 
@@ -124,7 +121,7 @@ fn generate_impl_text(strukt: &ast::StructDef, code: &str) -> String {
 //
 // FIXME: change the new fn checking to a more semantic approach when that's more
 // viable (e.g. we process proc macros, etc)
-fn find_struct_impl(ctx: &AssistCtx, strukt: &ast::StructDef) -> Option<Option<ast::ImplDef>> {
+fn find_struct_impl(ctx: &AssistContext, strukt: &ast::StructDef) -> Option<Option<ast::ImplDef>> {
     let db = ctx.db;
     let module = strukt.syntax().ancestors().find(|node| {
         ast::Module::can_cast(node.kind()) || ast::SourceFile::can_cast(node.kind())
@@ -162,8 +159,8 @@ fn find_struct_impl(ctx: &AssistCtx, strukt: &ast::StructDef) -> Option<Option<a
 
 fn has_new_fn(imp: &ast::ImplDef) -> bool {
     if let Some(il) = imp.item_list() {
-        for item in il.impl_items() {
-            if let ast::ImplItem::FnDef(f) = item {
+        for item in il.assoc_items() {
+            if let ast::AssocItem::FnDef(f) = item {
                 if let Some(name) = f.name() {
                     if name.text().eq_ignore_ascii_case("new") {
                         return true;
@@ -178,7 +175,7 @@ fn has_new_fn(imp: &ast::ImplDef) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
+    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
     use super::*;
 
@@ -192,7 +189,7 @@ mod tests {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -202,7 +199,7 @@ impl Foo {
 "struct Foo<T: Clone> {}
 
 impl<T: Clone> Foo<T> {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -212,7 +209,7 @@ impl<T: Clone> Foo<T> {
 "struct Foo<'a, T: Foo<'a>> {}
 
 impl<'a, T: Foo<'a>> Foo<'a, T> {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -222,7 +219,7 @@ impl<'a, T: Foo<'a>> Foo<'a, T> {
 "struct Foo { baz: String }
 
 impl Foo {
-    fn new(baz: String) -> Self { Self { baz } }<|>
+    fn $0new(baz: String) -> Self { Self { baz } }
 }
 ",
         );
@@ -232,7 +229,7 @@ impl Foo {
 "struct Foo { baz: String, qux: Vec<i32> }
 
 impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
+    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
 }
 ",
         );
@@ -244,7 +241,7 @@ impl Foo {
 "struct Foo { pub baz: String, pub qux: Vec<i32> }
 
 impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
+    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
 }
 ",
         );
@@ -259,7 +256,7 @@ impl Foo {}
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -274,7 +271,7 @@ impl Foo {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 
     fn qux(&self) {}
 }
@@ -295,7 +292,7 @@ impl Foo {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 
     fn qux(&self) {}
     fn baz() -> i32 {
@@ -312,7 +309,7 @@ impl Foo {
 "pub struct Foo {}
 
 impl Foo {
-    pub fn new() -> Self { Self {  } }<|>
+    pub fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -322,7 +319,7 @@ impl Foo {
 "pub(crate) struct Foo {}
 
 impl Foo {
-    pub(crate) fn new() -> Self { Self {  } }<|>
+    pub(crate) fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -415,7 +412,7 @@ pub struct Source<T> {
 }
 
 impl<T> Source<T> {
-    pub fn new(file_id: HirFileId, ast: T) -> Self { Self { file_id, ast } }<|>
+    pub fn $0new(file_id: HirFileId, ast: T) -> Self { Self { file_id, ast } }
 
     pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
         Source { file_id: self.file_id, ast: f(self.ast) }
diff --git a/crates/ra_assists/src/handlers/add_turbo_fish.rs b/crates/ra_assists/src/handlers/add_turbo_fish.rs
new file mode 100644
index 000000000..26acf81f2
--- /dev/null
+++ b/crates/ra_assists/src/handlers/add_turbo_fish.rs
@@ -0,0 +1,134 @@
+use ra_ide_db::defs::{classify_name_ref, Definition, NameRefClass};
+use ra_syntax::{ast, AstNode, SyntaxKind, T};
+use test_utils::mark;
+
+use crate::{
+    assist_context::{AssistContext, Assists},
+    AssistId,
+};
+
+// Assist: add_turbo_fish
+//
+// Adds `::<_>` to a call of a generic method or function.
+//
+// ```
+// fn make<T>() -> T { todo!() }
+// fn main() {
+//     let x = make<|>();
+// }
+// ```
+// ->
+// ```
+// fn make<T>() -> T { todo!() }
+// fn main() {
+//     let x = make::<${0:_}>();
+// }
+// ```
+pub(crate) fn add_turbo_fish(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let ident = ctx.find_token_at_offset(SyntaxKind::IDENT)?;
+    let next_token = ident.next_token()?;
+    if next_token.kind() == T![::] {
+        mark::hit!(add_turbo_fish_one_fish_is_enough);
+        return None;
+    }
+    let name_ref = ast::NameRef::cast(ident.parent())?;
+    let def = match classify_name_ref(&ctx.sema, &name_ref)? {
+        NameRefClass::Definition(def) => def,
+        NameRefClass::FieldShorthand { .. } => return None,
+    };
+    let fun = match def {
+        Definition::ModuleDef(hir::ModuleDef::Function(it)) => it,
+        _ => return None,
+    };
+    let generics = hir::GenericDef::Function(fun).params(ctx.sema.db);
+    if generics.is_empty() {
+        mark::hit!(add_turbo_fish_non_generic);
+        return None;
+    }
+    acc.add(AssistId("add_turbo_fish"), "Add `::<>`", ident.text_range(), |builder| {
+        match ctx.config.snippet_cap {
+            Some(cap) => builder.insert_snippet(cap, ident.text_range().end(), "::<${0:_}>"),
+            None => builder.insert(ident.text_range().end(), "::<_>"),
+        }
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    use super::*;
+    use test_utils::mark;
+
+    #[test]
+    fn add_turbo_fish_function() {
+        check_assist(
+            add_turbo_fish,
+            r#"
+fn make<T>() -> T {}
+fn main() {
+    make<|>();
+}
+"#,
+            r#"
+fn make<T>() -> T {}
+fn main() {
+    make::<${0:_}>();
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn add_turbo_fish_method() {
+        check_assist(
+            add_turbo_fish,
+            r#"
+struct S;
+impl S {
+    fn make<T>(&self) -> T {}
+}
+fn main() {
+    S.make<|>();
+}
+"#,
+            r#"
+struct S;
+impl S {
+    fn make<T>(&self) -> T {}
+}
+fn main() {
+    S.make::<${0:_}>();
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn add_turbo_fish_one_fish_is_enough() {
+        mark::check!(add_turbo_fish_one_fish_is_enough);
+        check_assist_not_applicable(
+            add_turbo_fish,
+            r#"
+fn make<T>() -> T {}
+fn main() {
+    make<|>::<()>();
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn add_turbo_fish_non_generic() {
+        mark::check!(add_turbo_fish_non_generic);
+        check_assist_not_applicable(
+            add_turbo_fish,
+            r#"
+fn make() -> () {}
+fn main() {
+    make<|>();
+}
+"#,
+        );
+    }
+}
diff --git a/crates/ra_assists/src/handlers/apply_demorgan.rs b/crates/ra_assists/src/handlers/apply_demorgan.rs
index 260b9e073..233e8fb8e 100644
--- a/crates/ra_assists/src/handlers/apply_demorgan.rs
+++ b/crates/ra_assists/src/handlers/apply_demorgan.rs
@@ -1,6 +1,6 @@
 use ra_syntax::ast::{self, AstNode};
 
-use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId};
+use crate::{utils::invert_boolean_expression, AssistContext, AssistId, Assists};
 
 // Assist: apply_demorgan
 //
@@ -21,7 +21,7 @@ use crate::{utils::invert_boolean_expression, Assist, AssistCtx, AssistId};
 //     if !(x == 4 && y) {}
 // }
 // ```
-pub(crate) fn apply_demorgan(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let expr = ctx.find_node_at_offset::<ast::BinExpr>()?;
     let op = expr.op_kind()?;
     let op_range = expr.op_token()?.text_range();
@@ -39,8 +39,7 @@ pub(crate) fn apply_demorgan(ctx: AssistCtx) -> Option<Assist> {
     let rhs_range = rhs.syntax().text_range();
     let not_rhs = invert_boolean_expression(rhs);
 
-    ctx.add_assist(AssistId("apply_demorgan"), "Apply De Morgan's law", |edit| {
-        edit.target(op_range);
+    acc.add(AssistId("apply_demorgan"), "Apply De Morgan's law", op_range, |edit| {
         edit.replace(op_range, opposite_op);
         edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text()));
         edit.replace(rhs_range, format!("{})", not_rhs.syntax().text()));
@@ -60,26 +59,26 @@ fn opposite_logic_op(kind: ast::BinOp) -> Option<&'static str> {
 mod tests {
     use super::*;
 
-    use crate::helpers::{check_assist, check_assist_not_applicable};
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
     #[test]
     fn demorgan_turns_and_into_or() {
-        check_assist(apply_demorgan, "fn f() { !x &&<|> !x }", "fn f() { !(x ||<|> x) }")
+        check_assist(apply_demorgan, "fn f() { !x &&<|> !x }", "fn f() { !(x || x) }")
     }
 
     #[test]
     fn demorgan_turns_or_into_and() {
-        check_assist(apply_demorgan, "fn f() { !x ||<|> !x }", "fn f() { !(x &&<|> x) }")
+        check_assist(apply_demorgan, "fn f() { !x ||<|> !x }", "fn f() { !(x && x) }")
     }
 
     #[test]
     fn demorgan_removes_inequality() {
-        check_assist(apply_demorgan, "fn f() { x != x ||<|> !x }", "fn f() { !(x == x &&<|> x) }")
+        check_assist(apply_demorgan, "fn f() { x != x ||<|> !x }", "fn f() { !(x == x && x) }")
     }
 
     #[test]
     fn demorgan_general_case() {
-        check_assist(apply_demorgan, "fn f() { x ||<|> x }", "fn f() { !(!x &&<|> !x) }")
+        check_assist(apply_demorgan, "fn f() { x ||<|> x }", "fn f() { !(!x && !x) }")
     }
 
     #[test]
diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs
index 99682e023..edf96d50e 100644
--- a/crates/ra_assists/src/handlers/auto_import.rs
+++ b/crates/ra_assists/src/handlers/auto_import.rs
@@ -1,5 +1,6 @@
 use std::collections::BTreeSet;
 
+use either::Either;
 use hir::{
     AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait,
     Type,
@@ -12,12 +13,7 @@ use ra_syntax::{
 };
 use rustc_hash::FxHashSet;
 
-use crate::{
-    assist_ctx::{Assist, AssistCtx},
-    utils::insert_use_statement,
-    AssistId,
-};
-use either::Either;
+use crate::{utils::insert_use_statement, AssistContext, AssistId, Assists, GroupLabel};
 
 // Assist: auto_import
 //
@@ -38,25 +34,32 @@ use either::Either;
 // }
 // # pub mod std { pub mod collections { pub struct HashMap { } } }
 // ```
-pub(crate) fn auto_import(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let auto_import_assets = AutoImportAssets::new(&ctx)?;
     let proposed_imports = auto_import_assets.search_for_imports(ctx.db);
     if proposed_imports.is_empty() {
         return None;
     }
 
-    let mut group = ctx.add_assist_group(auto_import_assets.get_import_group_message());
+    let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range;
+    let group = auto_import_assets.get_import_group_message();
     for import in proposed_imports {
-        group.add_assist(AssistId("auto_import"), format!("Import `{}`", &import), |edit| {
-            edit.target(auto_import_assets.syntax_under_caret.text_range());
-            insert_use_statement(
-                &auto_import_assets.syntax_under_caret,
-                &import,
-                edit.text_edit_builder(),
-            );
-        });
-    }
-    group.finish()
+        acc.add_group(
+            &group,
+            AssistId("auto_import"),
+            format!("Import `{}`", &import),
+            range,
+            |builder| {
+                insert_use_statement(
+                    &auto_import_assets.syntax_under_caret,
+                    &import,
+                    ctx,
+                    builder.text_edit_builder(),
+                );
+            },
+        );
+    }
+    Some(())
 }
 
 #[derive(Debug)]
@@ -67,15 +70,15 @@ struct AutoImportAssets {
 }
 
 impl AutoImportAssets {
-    fn new(ctx: &AssistCtx) -> Option<Self> {
-        if let Some(path_under_caret) = ctx.find_node_at_offset::<ast::Path>() {
+    fn new(ctx: &AssistContext) -> Option<Self> {
+        if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::<ast::Path>() {
             Self::for_regular_path(path_under_caret, &ctx)
         } else {
-            Self::for_method_call(ctx.find_node_at_offset()?, &ctx)
+            Self::for_method_call(ctx.find_node_at_offset_with_descend()?, &ctx)
         }
     }
 
-    fn for_method_call(method_call: ast::MethodCallExpr, ctx: &AssistCtx) -> Option<Self> {
+    fn for_method_call(method_call: ast::MethodCallExpr, ctx: &AssistContext) -> Option<Self> {
         let syntax_under_caret = method_call.syntax().to_owned();
         let module_with_name_to_import = ctx.sema.scope(&syntax_under_caret).module()?;
         Some(Self {
@@ -85,7 +88,7 @@ impl AutoImportAssets {
         })
     }
 
-    fn for_regular_path(path_under_caret: ast::Path, ctx: &AssistCtx) -> Option<Self> {
+    fn for_regular_path(path_under_caret: ast::Path, ctx: &AssistContext) -> Option<Self> {
         let syntax_under_caret = path_under_caret.syntax().to_owned();
         if syntax_under_caret.ancestors().find_map(ast::UseItem::cast).is_some() {
             return None;
@@ -108,8 +111,8 @@ impl AutoImportAssets {
         }
     }
 
-    fn get_import_group_message(&self) -> String {
-        match &self.import_candidate {
+    fn get_import_group_message(&self) -> GroupLabel {
+        let name = match &self.import_candidate {
             ImportCandidate::UnqualifiedName(name) => format!("Import {}", name),
             ImportCandidate::QualifierStart(qualifier_start) => {
                 format!("Import {}", qualifier_start)
@@ -120,7 +123,8 @@ impl AutoImportAssets {
             ImportCandidate::TraitMethod(_, trait_method_name) => {
                 format!("Import a trait for method {}", trait_method_name)
             }
-        }
+        };
+        GroupLabel(name)
     }
 
     fn search_for_imports(&self, db: &RootDatabase) -> BTreeSet<ModPath> {
@@ -280,7 +284,7 @@ impl ImportCandidate {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
+    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
     #[test]
     fn applicable_when_found_an_import() {
@@ -294,7 +298,7 @@ mod tests {
             }
             ",
             r"
-            <|>use PubMod::PubStruct;
+            use PubMod::PubStruct;
 
             PubStruct
 
@@ -306,6 +310,35 @@ mod tests {
     }
 
     #[test]
+    fn applicable_when_found_an_import_in_macros() {
+        check_assist(
+            auto_import,
+            r"
+            macro_rules! foo {
+                ($i:ident) => { fn foo(a: $i) {} }
+            }
+            foo!(Pub<|>Struct);
+
+            pub mod PubMod {
+                pub struct PubStruct;
+            }
+            ",
+            r"
+            use PubMod::PubStruct;
+
+            macro_rules! foo {
+                ($i:ident) => { fn foo(a: $i) {} }
+            }
+            foo!(PubStruct);
+
+            pub mod PubMod {
+                pub struct PubStruct;
+            }
+            ",
+        );
+    }
+
+    #[test]
     fn auto_imports_are_merged() {
         check_assist(
             auto_import,
@@ -327,7 +360,7 @@ mod tests {
             use PubMod::{PubStruct2, PubStruct1};
 
             struct Test {
-                test: Pub<|>Struct2<u8>,
+                test: PubStruct2<u8>,
             }
 
             pub mod PubMod {
@@ -358,9 +391,9 @@ mod tests {
             }
             ",
             r"
-            use PubMod1::PubStruct;
+            use PubMod3::PubStruct;
 
-            PubSt<|>ruct
+            PubStruct
 
             pub mod PubMod1 {
                 pub struct PubStruct;
@@ -441,7 +474,7 @@ mod tests {
             r"
             use PubMod::test_function;
 
-            test_function<|>
+            test_function
 
             pub mod PubMod {
                 pub fn test_function() {};
@@ -468,7 +501,7 @@ mod tests {
             r"use crate_with_macro::foo;
 
 fn main() {
-    foo<|>
+    foo
 }
 ",
         );
@@ -554,7 +587,7 @@ fn main() {
             }
 
             fn main() {
-                TestStruct::test_function<|>
+                TestStruct::test_function
             }
             ",
         );
@@ -587,7 +620,7 @@ fn main() {
             }
 
             fn main() {
-                TestStruct::TEST_CONST<|>
+                TestStruct::TEST_CONST
             }
             ",
         );
@@ -626,7 +659,7 @@ fn main() {
             }
 
             fn main() {
-                test_mod::TestStruct::test_function<|>
+                test_mod::TestStruct::test_function
             }
             ",
         );
@@ -697,7 +730,7 @@ fn main() {
             }
 
             fn main() {
-                test_mod::TestStruct::TEST_CONST<|>
+                test_mod::TestStruct::TEST_CONST
             }
             ",
         );
@@ -770,7 +803,7 @@ fn main() {
 
             fn main() {
                 let test_struct = test_mod::TestStruct {};
-                test_struct.test_meth<|>od()
+                test_struct.test_method()
             }
             ",
         );
diff --git a/crates/ra_assists/src/handlers/change_return_type_to_result.rs b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
new file mode 100644
index 000000000..c6baa0a57
--- /dev/null
+++ b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
@@ -0,0 +1,961 @@
+use ra_syntax::{
+    ast::{self, BlockExpr, Expr, LoopBodyOwner},
+    AstNode, SyntaxNode,
+};
+
+use crate::{AssistContext, AssistId, Assists};
+
+// Assist: change_return_type_to_result
+//
+// Change the function's return type to Result.
+//
+// ```
+// fn foo() -> i32<|> { 42i32 }
+// ```
+// ->
+// ```
+// fn foo() -> Result<i32, ${0:_}> { Ok(42i32) }
+// ```
+pub(crate) fn change_return_type_to_result(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let ret_type = ctx.find_node_at_offset::<ast::RetType>()?;
+    // FIXME: extend to lambdas as well
+    let fn_def = ret_type.syntax().parent().and_then(ast::FnDef::cast)?;
+
+    let type_ref = &ret_type.type_ref()?;
+    if type_ref.syntax().text().to_string().starts_with("Result<") {
+        return None;
+    }
+
+    let block_expr = &fn_def.body()?;
+
+    acc.add(
+        AssistId("change_return_type_to_result"),
+        "Change return type to Result",
+        type_ref.syntax().text_range(),
+        |builder| {
+            let mut tail_return_expr_collector = TailReturnCollector::new();
+            tail_return_expr_collector.collect_jump_exprs(block_expr, false);
+            tail_return_expr_collector.collect_tail_exprs(block_expr);
+
+            for ret_expr_arg in tail_return_expr_collector.exprs_to_wrap {
+                builder.replace_node_and_indent(&ret_expr_arg, format!("Ok({})", ret_expr_arg));
+            }
+
+            match ctx.config.snippet_cap {
+                Some(cap) => {
+                    let snippet = format!("Result<{}, ${{0:_}}>", type_ref);
+                    builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet)
+                }
+                None => builder
+                    .replace(type_ref.syntax().text_range(), format!("Result<{}, _>", type_ref)),
+            }
+        },
+    )
+}
+
+struct TailReturnCollector {
+    exprs_to_wrap: Vec<SyntaxNode>,
+}
+
+impl TailReturnCollector {
+    fn new() -> Self {
+        Self { exprs_to_wrap: vec![] }
+    }
+    /// Collect all`return` expression
+    fn collect_jump_exprs(&mut self, block_expr: &BlockExpr, collect_break: bool) {
+        let statements = block_expr.statements();
+        for stmt in statements {
+            let expr = match &stmt {
+                ast::Stmt::ExprStmt(stmt) => stmt.expr(),
+                ast::Stmt::LetStmt(stmt) => stmt.initializer(),
+            };
+            if let Some(expr) = &expr {
+                self.handle_exprs(expr, collect_break);
+            }
+        }
+
+        // Browse tail expressions for each block
+        if let Some(expr) = block_expr.expr() {
+            if let Some(last_exprs) = get_tail_expr_from_block(&expr) {
+                for last_expr in last_exprs {
+                    let last_expr = match last_expr {
+                        NodeType::Node(expr) | NodeType::Leaf(expr) => expr,
+                    };
+
+                    if let Some(last_expr) = Expr::cast(last_expr.clone()) {
+                        self.handle_exprs(&last_expr, collect_break);
+                    } else if let Some(expr_stmt) = ast::Stmt::cast(last_expr) {
+                        let expr_stmt = match &expr_stmt {
+                            ast::Stmt::ExprStmt(stmt) => stmt.expr(),
+                            ast::Stmt::LetStmt(stmt) => stmt.initializer(),
+                        };
+                        if let Some(expr) = &expr_stmt {
+                            self.handle_exprs(expr, collect_break);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    fn handle_exprs(&mut self, expr: &Expr, collect_break: bool) {
+        match expr {
+            Expr::BlockExpr(block_expr) => {
+                self.collect_jump_exprs(&block_expr, collect_break);
+            }
+            Expr::ReturnExpr(ret_expr) => {
+                if let Some(ret_expr_arg) = &ret_expr.expr() {
+                    self.exprs_to_wrap.push(ret_expr_arg.syntax().clone());
+                }
+            }
+            Expr::BreakExpr(break_expr) if collect_break => {
+                if let Some(break_expr_arg) = &break_expr.expr() {
+                    self.exprs_to_wrap.push(break_expr_arg.syntax().clone());
+                }
+            }
+            Expr::IfExpr(if_expr) => {
+                for block in if_expr.blocks() {
+                    self.collect_jump_exprs(&block, collect_break);
+                }
+            }
+            Expr::LoopExpr(loop_expr) => {
+                if let Some(block_expr) = loop_expr.loop_body() {
+                    self.collect_jump_exprs(&block_expr, collect_break);
+                }
+            }
+            Expr::ForExpr(for_expr) => {
+                if let Some(block_expr) = for_expr.loop_body() {
+                    self.collect_jump_exprs(&block_expr, collect_break);
+                }
+            }
+            Expr::WhileExpr(while_expr) => {
+                if let Some(block_expr) = while_expr.loop_body() {
+                    self.collect_jump_exprs(&block_expr, collect_break);
+                }
+            }
+            Expr::MatchExpr(match_expr) => {
+                if let Some(arm_list) = match_expr.match_arm_list() {
+                    arm_list.arms().filter_map(|match_arm| match_arm.expr()).for_each(|expr| {
+                        self.handle_exprs(&expr, collect_break);
+                    });
+                }
+            }
+            _ => {}
+        }
+    }
+
+    fn collect_tail_exprs(&mut self, block: &BlockExpr) {
+        if let Some(expr) = block.expr() {
+            self.handle_exprs(&expr, true);
+            self.fetch_tail_exprs(&expr);
+        }
+    }
+
+    fn fetch_tail_exprs(&mut self, expr: &Expr) {
+        if let Some(exprs) = get_tail_expr_from_block(expr) {
+            for node_type in &exprs {
+                match node_type {
+                    NodeType::Leaf(expr) => {
+                        self.exprs_to_wrap.push(expr.clone());
+                    }
+                    NodeType::Node(expr) => match &Expr::cast(expr.clone()) {
+                        Some(last_expr) => {
+                            self.fetch_tail_exprs(last_expr);
+                        }
+                        None => {
+                            self.exprs_to_wrap.push(expr.clone());
+                        }
+                    },
+                }
+            }
+        }
+    }
+}
+
+#[derive(Debug)]
+enum NodeType {
+    Leaf(SyntaxNode),
+    Node(SyntaxNode),
+}
+
+/// Get a tail expression inside a block
+fn get_tail_expr_from_block(expr: &Expr) -> Option<Vec<NodeType>> {
+    match expr {
+        Expr::IfExpr(if_expr) => {
+            let mut nodes = vec![];
+            for block in if_expr.blocks() {
+                if let Some(block_expr) = block.expr() {
+                    if let Some(tail_exprs) = get_tail_expr_from_block(&block_expr) {
+                        nodes.extend(tail_exprs);
+                    }
+                } else if let Some(last_expr) = block.syntax().last_child() {
+                    nodes.push(NodeType::Node(last_expr));
+                } else {
+                    nodes.push(NodeType::Node(block.syntax().clone()));
+                }
+            }
+            Some(nodes)
+        }
+        Expr::LoopExpr(loop_expr) => {
+            loop_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)])
+        }
+        Expr::ForExpr(for_expr) => {
+            for_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)])
+        }
+        Expr::WhileExpr(while_expr) => {
+            while_expr.syntax().last_child().map(|lc| vec![NodeType::Node(lc)])
+        }
+        Expr::BlockExpr(block_expr) => {
+            block_expr.expr().map(|lc| vec![NodeType::Node(lc.syntax().clone())])
+        }
+        Expr::MatchExpr(match_expr) => {
+            let arm_list = match_expr.match_arm_list()?;
+            let arms: Vec<NodeType> = arm_list
+                .arms()
+                .filter_map(|match_arm| match_arm.expr())
+                .map(|expr| match expr {
+                    Expr::ReturnExpr(ret_expr) => NodeType::Node(ret_expr.syntax().clone()),
+                    Expr::BreakExpr(break_expr) => NodeType::Node(break_expr.syntax().clone()),
+                    _ => match expr.syntax().last_child() {
+                        Some(last_expr) => NodeType::Node(last_expr),
+                        None => NodeType::Node(expr.syntax().clone()),
+                    },
+                })
+                .collect();
+
+            Some(arms)
+        }
+        Expr::BreakExpr(expr) => expr.expr().map(|e| vec![NodeType::Leaf(e.syntax().clone())]),
+        Expr::ReturnExpr(ret_expr) => Some(vec![NodeType::Node(ret_expr.syntax().clone())]),
+        Expr::CallExpr(call_expr) => Some(vec![NodeType::Leaf(call_expr.syntax().clone())]),
+        Expr::Literal(lit_expr) => Some(vec![NodeType::Leaf(lit_expr.syntax().clone())]),
+        Expr::TupleExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::ArrayExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::ParenExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::PathExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::Label(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::RecordLit(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::IndexExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::MethodCallExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::AwaitExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::CastExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::RefExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::PrefixExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::RangeExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::BinExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::MacroCall(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        Expr::BoxExpr(expr) => Some(vec![NodeType::Leaf(expr.syntax().clone())]),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    use super::*;
+
+    #[test]
+    fn change_return_type_to_result_simple() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i3<|>2 {
+                let test = "test";
+                return 42i32;
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                return Ok(42i32);
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_return_type() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let test = "test";
+                return 42i32;
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                return Ok(42i32);
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_return_type_bad_cursor() {
+        check_assist_not_applicable(
+            change_return_type_to_result,
+            r#"fn foo() -> i32 {
+                let test = "test";<|>
+                return 42i32;
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_cursor() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> <|>i32 {
+                let test = "test";
+                return 42i32;
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                return Ok(42i32);
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -><|> i32 {
+                let test = "test";
+                42i32
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                Ok(42i32)
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_only() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                42i32
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                Ok(42i32)
+            }"#,
+        );
+    }
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_block_like() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                if true {
+                    42i32
+                } else {
+                    24i32
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                if true {
+                    Ok(42i32)
+                } else {
+                    Ok(24i32)
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_nested_if() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                if true {
+                    if false {
+                        1
+                    } else {
+                        2
+                    }
+                } else {
+                    24i32
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                if true {
+                    if false {
+                        Ok(1)
+                    } else {
+                        Ok(2)
+                    }
+                } else {
+                    Ok(24i32)
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_await() {
+        check_assist(
+            change_return_type_to_result,
+            r#"async fn foo() -> i<|>32 {
+                if true {
+                    if false {
+                        1.await
+                    } else {
+                        2.await
+                    }
+                } else {
+                    24i32.await
+                }
+            }"#,
+            r#"async fn foo() -> Result<i32, ${0:_}> {
+                if true {
+                    if false {
+                        Ok(1.await)
+                    } else {
+                        Ok(2.await)
+                    }
+                } else {
+                    Ok(24i32.await)
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_array() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> [i32;<|> 3] {
+                [1, 2, 3]
+            }"#,
+            r#"fn foo() -> Result<[i32; 3], ${0:_}> {
+                Ok([1, 2, 3])
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_cast() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -<|>> i32 {
+                if true {
+                    if false {
+                        1 as i32
+                    } else {
+                        2 as i32
+                    }
+                } else {
+                    24 as i32
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                if true {
+                    if false {
+                        Ok(1 as i32)
+                    } else {
+                        Ok(2 as i32)
+                    }
+                } else {
+                    Ok(24 as i32)
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_block_like_match() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = 5;
+                match my_var {
+                    5 => 42i32,
+                    _ => 24i32,
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = 5;
+                match my_var {
+                    5 => Ok(42i32),
+                    _ => Ok(24i32),
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_loop_with_tail() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = 5;
+                loop {
+                    println!("test");
+                    5
+                }
+
+                my_var
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = 5;
+                loop {
+                    println!("test");
+                    5
+                }
+
+                Ok(my_var)
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_loop_in_let_stmt() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = let x = loop {
+                    break 1;
+                };
+
+                my_var
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = let x = loop {
+                    break 1;
+                };
+
+                Ok(my_var)
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_block_like_match_return_expr() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = 5;
+                let res = match my_var {
+                    5 => 42i32,
+                    _ => return 24i32,
+                };
+
+                res
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = 5;
+                let res = match my_var {
+                    5 => 42i32,
+                    _ => return Ok(24i32),
+                };
+
+                Ok(res)
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = 5;
+                let res = if my_var == 5 {
+                    42i32
+                } else {
+                    return 24i32;
+                };
+
+                res
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = 5;
+                let res = if my_var == 5 {
+                    42i32
+                } else {
+                    return Ok(24i32);
+                };
+
+                Ok(res)
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_block_like_match_deeper() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let my_var = 5;
+                match my_var {
+                    5 => {
+                        if true {
+                            42i32
+                        } else {
+                            25i32
+                        }
+                    },
+                    _ => {
+                        let test = "test";
+                        if test == "test" {
+                            return bar();
+                        }
+                        53i32
+                    },
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let my_var = 5;
+                match my_var {
+                    5 => {
+                        if true {
+                            Ok(42i32)
+                        } else {
+                            Ok(25i32)
+                        }
+                    },
+                    _ => {
+                        let test = "test";
+                        if test == "test" {
+                            return Ok(bar());
+                        }
+                        Ok(53i32)
+                    },
+                }
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_tail_block_like_early_return() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i<|>32 {
+                let test = "test";
+                if test == "test" {
+                    return 24i32;
+                }
+                53i32
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                if test == "test" {
+                    return Ok(24i32);
+                }
+                Ok(53i32)
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_closure() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -><|> u32 {
+                let true_closure = || {
+                    return true;
+                };
+                if the_field < 5 {
+                    let mut i = 0;
+
+
+                    if true_closure() {
+                        return 99;
+                    } else {
+                        return 0;
+                    }
+                }
+
+                the_field
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                let true_closure = || {
+                    return true;
+                };
+                if the_field < 5 {
+                    let mut i = 0;
+
+
+                    if true_closure() {
+                        return Ok(99);
+                    } else {
+                        return Ok(0);
+                    }
+                }
+
+                Ok(the_field)
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -> u32<|> {
+                let true_closure = || {
+                    return true;
+                };
+                if the_field < 5 {
+                    let mut i = 0;
+
+
+                    if true_closure() {
+                        return 99;
+                    } else {
+                        return 0;
+                    }
+                }
+                let t = None;
+
+                t.unwrap_or_else(|| the_field)
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                let true_closure = || {
+                    return true;
+                };
+                if the_field < 5 {
+                    let mut i = 0;
+
+
+                    if true_closure() {
+                        return Ok(99);
+                    } else {
+                        return Ok(0);
+                    }
+                }
+                let t = None;
+
+                Ok(t.unwrap_or_else(|| the_field))
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_with_weird_forms() {
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let test = "test";
+                if test == "test" {
+                    return 24i32;
+                }
+                let mut i = 0;
+                loop {
+                    if i == 1 {
+                        break 55;
+                    }
+                    i += 1;
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                if test == "test" {
+                    return Ok(24i32);
+                }
+                let mut i = 0;
+                loop {
+                    if i == 1 {
+                        break Ok(55);
+                    }
+                    i += 1;
+                }
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i32<|> {
+                let test = "test";
+                if test == "test" {
+                    return 24i32;
+                }
+                let mut i = 0;
+                loop {
+                    loop {
+                        if i == 1 {
+                            break 55;
+                        }
+                        i += 1;
+                    }
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                if test == "test" {
+                    return Ok(24i32);
+                }
+                let mut i = 0;
+                loop {
+                    loop {
+                        if i == 1 {
+                            break Ok(55);
+                        }
+                        i += 1;
+                    }
+                }
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo() -> i3<|>2 {
+                let test = "test";
+                let other = 5;
+                if test == "test" {
+                    let res = match other {
+                        5 => 43,
+                        _ => return 56,
+                    };
+                }
+                let mut i = 0;
+                loop {
+                    loop {
+                        if i == 1 {
+                            break 55;
+                        }
+                        i += 1;
+                    }
+                }
+            }"#,
+            r#"fn foo() -> Result<i32, ${0:_}> {
+                let test = "test";
+                let other = 5;
+                if test == "test" {
+                    let res = match other {
+                        5 => 43,
+                        _ => return Ok(56),
+                    };
+                }
+                let mut i = 0;
+                loop {
+                    loop {
+                        if i == 1 {
+                            break Ok(55);
+                        }
+                        i += 1;
+                    }
+                }
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -> u32<|> {
+                if the_field < 5 {
+                    let mut i = 0;
+                    loop {
+                        if i > 5 {
+                            return 55u32;
+                        }
+                        i += 3;
+                    }
+
+                    match i {
+                        5 => return 99,
+                        _ => return 0,
+                    };
+                }
+
+                the_field
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                if the_field < 5 {
+                    let mut i = 0;
+                    loop {
+                        if i > 5 {
+                            return Ok(55u32);
+                        }
+                        i += 3;
+                    }
+
+                    match i {
+                        5 => return Ok(99),
+                        _ => return Ok(0),
+                    };
+                }
+
+                Ok(the_field)
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -> u3<|>2 {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    match i {
+                        5 => return 99,
+                        _ => return 0,
+                    }
+                }
+
+                the_field
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    match i {
+                        5 => return Ok(99),
+                        _ => return Ok(0),
+                    }
+                }
+
+                Ok(the_field)
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -> u32<|> {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    if i == 5 {
+                        return 99
+                    } else {
+                        return 0
+                    }
+                }
+
+                the_field
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    if i == 5 {
+                        return Ok(99)
+                    } else {
+                        return Ok(0)
+                    }
+                }
+
+                Ok(the_field)
+            }"#,
+        );
+
+        check_assist(
+            change_return_type_to_result,
+            r#"fn foo(the_field: u32) -> <|>u32 {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    if i == 5 {
+                        return 99;
+                    } else {
+                        return 0;
+                    }
+                }
+
+                the_field
+            }"#,
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
+                if the_field < 5 {
+                    let mut i = 0;
+
+                    if i == 5 {
+                        return Ok(99);
+                    } else {
+                        return Ok(0);
+                    }
+                }
+
+                Ok(the_field)
+            }"#,
+        );
+    }
+}
diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs
index 44f6a1dae..c21d75be0 100644
--- a/crates/ra_assists/src/handlers/change_visibility.rs
+++ b/crates/ra_assists/src/handlers/change_visibility.rs
@@ -7,9 +7,9 @@ use ra_syntax::{
     },
     SyntaxNode, TextSize, T,
 };
+use test_utils::mark;
 
-use crate::{Assist, AssistCtx, AssistId};
-use test_utils::tested_by;
+use crate::{AssistContext, AssistId, Assists};
 
 // Assist: change_visibility
 //
@@ -22,14 +22,14 @@ use test_utils::tested_by;
 // ```
 // pub(crate) fn frobnicate() {}
 // ```
-pub(crate) fn change_visibility(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     if let Some(vis) = ctx.find_node_at_offset::<ast::Visibility>() {
-        return change_vis(ctx, vis);
+        return change_vis(acc, vis);
     }
-    add_vis(ctx)
+    add_vis(acc, ctx)
 }
 
-fn add_vis(ctx: AssistCtx) -> Option<Assist> {
+fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let item_keyword = ctx.token_at_offset().find(|leaf| match leaf.kind() {
         T![const] | T![fn] | T![mod] | T![struct] | T![enum] | T![trait] => true,
         _ => false,
@@ -47,23 +47,27 @@ fn add_vis(ctx: AssistCtx) -> Option<Assist> {
             return None;
         }
         (vis_offset(&parent), keyword.text_range())
-    } else {
-        let field_name: ast::Name = ctx.find_node_at_offset()?;
+    } else if let Some(field_name) = ctx.find_node_at_offset::<ast::Name>() {
         let field = field_name.syntax().ancestors().find_map(ast::RecordFieldDef::cast)?;
         if field.name()? != field_name {
-            tested_by!(change_visibility_field_false_positive);
+            mark::hit!(change_visibility_field_false_positive);
             return None;
         }
         if field.visibility().is_some() {
             return None;
         }
         (vis_offset(field.syntax()), field_name.syntax().text_range())
+    } else if let Some(field) = ctx.find_node_at_offset::<ast::TupleFieldDef>() {
+        if field.visibility().is_some() {
+            return None;
+        }
+        (vis_offset(field.syntax()), field.syntax().text_range())
+    } else {
+        return None;
     };
 
-    ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub(crate)", |edit| {
-        edit.target(target);
+    acc.add(AssistId("change_visibility"), "Change visibility to pub(crate)", target, |edit| {
         edit.insert(offset, "pub(crate) ");
-        edit.set_cursor(offset);
     })
 }
 
@@ -78,49 +82,49 @@ fn vis_offset(node: &SyntaxNode) -> TextSize {
         .unwrap_or_else(|| node.text_range().start())
 }
 
-fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option<Assist> {
+fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> {
     if vis.syntax().text() == "pub" {
-        return ctx.add_assist(
+        let target = vis.syntax().text_range();
+        return acc.add(
             AssistId("change_visibility"),
             "Change Visibility to pub(crate)",
+            target,
             |edit| {
-                edit.target(vis.syntax().text_range());
                 edit.replace(vis.syntax().text_range(), "pub(crate)");
-                edit.set_cursor(vis.syntax().text_range().start())
             },
         );
     }
     if vis.syntax().text() == "pub(crate)" {
-        return ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub", |edit| {
-            edit.target(vis.syntax().text_range());
-            edit.replace(vis.syntax().text_range(), "pub");
-            edit.set_cursor(vis.syntax().text_range().start());
-        });
+        let target = vis.syntax().text_range();
+        return acc.add(
+            AssistId("change_visibility"),
+            "Change visibility to pub",
+            target,
+            |edit| {
+                edit.replace(vis.syntax().text_range(), "pub");
+            },
+        );
     }
     None
 }
 
 #[cfg(test)]
 mod tests {
-    use test_utils::covers;
+    use test_utils::mark;
 
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
+    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
     use super::*;
 
     #[test]
     fn change_visibility_adds_pub_crate_to_items() {
-        check_assist(change_visibility, "<|>fn foo() {}", "<|>pub(crate) fn foo() {}");
-        check_assist(change_visibility, "f<|>n foo() {}", "<|>pub(crate) fn foo() {}");
-        check_assist(change_visibility, "<|>struct Foo {}", "<|>pub(crate) struct Foo {}");
-        check_assist(change_visibility, "<|>mod foo {}", "<|>pub(crate) mod foo {}");
-        check_assist(change_visibility, "<|>trait Foo {}", "<|>pub(crate) trait Foo {}");
-        check_assist(change_visibility, "m<|>od {}", "<|>pub(crate) mod {}");
-        check_assist(
-            change_visibility,
-            "unsafe f<|>n foo() {}",
-            "<|>pub(crate) unsafe fn foo() {}",
-        );
+        check_assist(change_visibility, "<|>fn foo() {}", "pub(crate) fn foo() {}");
+        check_assist(change_visibility, "f<|>n foo() {}", "pub(crate) fn foo() {}");
+        check_assist(change_visibility, "<|>struct Foo {}", "pub(crate) struct Foo {}");
+        check_assist(change_visibility, "<|>mod foo {}", "pub(crate) mod foo {}");
+        check_assist(change_visibility, "<|>trait Foo {}", "pub(crate) trait Foo {}");
+        check_assist(change_visibility, "m<|>od {}", "pub(crate) mod {}");
+        check_assist(change_visibility, "unsafe f<|>n foo() {}", "pub(crate) unsafe fn foo() {}");
     }
 
     #[test]
@@ -128,13 +132,14 @@ mod tests {
         check_assist(
             change_visibility,
             r"struct S { <|>field: u32 }",
-            r"struct S { <|>pub(crate) field: u32 }",
-        )
+            r"struct S { pub(crate) field: u32 }",
+        );
+        check_assist(change_visibility, r"struct S ( <|>u32 )", r"struct S ( pub(crate) u32 )");
     }
 
     #[test]
     fn change_visibility_field_false_positive() {
-        covers!(change_visibility_field_false_positive);
+        mark::check!(change_visibility_field_false_positive);
         check_assist_not_applicable(
             change_visibility,
             r"struct S { field: [(); { let <|>x = ();}] }",
@@ -143,17 +148,17 @@ mod tests {
 
     #[test]
     fn change_visibility_pub_to_pub_crate() {
-        check_assist(change_visibility, "<|>pub fn foo() {}", "<|>pub(crate) fn foo() {}")
+        check_assist(change_visibility, "<|>pub fn foo() {}", "pub(crate) fn foo() {}")
     }
 
     #[test]
     fn change_visibility_pub_crate_to_pub() {
-        check_assist(change_visibility, "<|>pub(crate) fn foo() {}", "<|>pub fn foo() {}")
+        check_assist(change_visibility, "<|>pub(crate) fn foo() {}", "pub fn foo() {}")
     }
 
     #[test]
     fn change_visibility_const() {
-        check_assist(change_visibility, "<|>const FOO = 3u8;", "<|>pub(crate) const FOO = 3u8;");
+        check_assist(change_visibility, "<|>const FOO = 3u8;", "pub(crate) const FOO = 3u8;");
     }
 
     #[test]
@@ -174,12 +179,21 @@ mod tests {
             // comments
 
             #[derive(Debug)]
-            <|>pub(crate) struct Foo;
+            pub(crate) struct Foo;
             ",
         )
     }
 
     #[test]
+    fn not_applicable_for_enum_variants() {
+        check_assist_not_applicable(
+            change_visibility,
+            r"mod foo { pub enum Foo {Foo1} }
+              fn main() { foo::Foo::Foo1<|> } ",
+        );
+    }
+
+    #[test]
     fn change_visibility_target() {
         check_assist_target(change_visibility, "<|>fn foo() {}", "fn");
         check_assist_target(change_visibility, "pub(crate)<|> fn foo() {}", "pub(crate)");
diff --git a/crates/ra_assists/src/handlers/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs
index ea6c56f8c..4cc75a7ce 100644
--- a/crates/ra_assists/src/handlers/early_return.rs
+++ b/crates/ra_assists/src/handlers/early_return.rs
@@ -2,14 +2,18 @@ use std::{iter::once, ops::RangeInclusive};
 
 use ra_syntax::{
     algo::replace_children,
-    ast::{self, edit::IndentLevel, make, Block, Pat::TupleStructPat},
+    ast::{
+        self,
+        edit::{AstNodeEdit, IndentLevel},
+        make,
+    },
     AstNode,
     SyntaxKind::{FN_DEF, LOOP_EXPR, L_CURLY, R_CURLY, WHILE_EXPR, WHITESPACE},
     SyntaxNode,
 };
 
 use crate::{
-    assist_ctx::{Assist, AssistCtx},
+    assist_context::{AssistContext, Assists},
     utils::invert_boolean_expression,
     AssistId,
 };
@@ -36,7 +40,7 @@ use crate::{
 //     bar();
 // }
 // ```
-pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
+pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let if_expr: ast::IfExpr = ctx.find_node_at_offset()?;
     if if_expr.else_branch().is_some() {
         return None;
@@ -47,7 +51,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
     // Check if there is an IfLet that we can handle.
     let if_let_pat = match cond.pat() {
         None => None, // No IfLet, supported.
-        Some(TupleStructPat(pat)) if pat.args().count() == 1 => {
+        Some(ast::Pat::TupleStructPat(pat)) if pat.args().count() == 1 => {
             let path = pat.path()?;
             match path.qualifier() {
                 None => {
@@ -61,9 +65,9 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
     };
 
     let cond_expr = cond.expr()?;
-    let then_block = if_expr.then_branch()?.block()?;
+    let then_block = if_expr.then_branch()?;
 
-    let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::Block::cast)?;
+    let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::BlockExpr::cast)?;
 
     if parent_block.expr()? != if_expr.clone().into() {
         return None;
@@ -80,7 +84,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
         return None;
     }
 
-    let parent_container = parent_block.syntax().parent()?.parent()?;
+    let parent_container = parent_block.syntax().parent()?;
 
     let early_expression: ast::Expr = match parent_container.kind() {
         WHILE_EXPR | LOOP_EXPR => make::expr_continue(),
@@ -93,9 +97,9 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
     }
 
     then_block.syntax().last_child_or_token().filter(|t| t.kind() == R_CURLY)?;
-    let cursor_position = ctx.frange.range.start();
 
-    ctx.add_assist(AssistId("convert_to_guarded_return"), "Convert to guarded return", |edit| {
+    let target = if_expr.syntax().text_range();
+    acc.add(AssistId("convert_to_guarded_return"), "Convert to guarded return", target, |edit| {
         let if_indent_level = IndentLevel::from_node(&if_expr.syntax());
         let new_block = match if_let_pat {
             None => {
@@ -104,8 +108,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
                     let then_branch =
                         make::block_expr(once(make::expr_stmt(early_expression).into()), None);
                     let cond = invert_boolean_expression(cond_expr);
-                    let e = make::expr_if(make::condition(cond, None), then_branch);
-                    if_indent_level.increase_indent(e)
+                    make::expr_if(make::condition(cond, None), then_branch).indent(if_indent_level)
                 };
                 replace(new_expr.syntax(), &then_block, &parent_block, &if_expr)
             }
@@ -139,21 +142,19 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
                     make::bind_pat(make::name(&bound_ident.syntax().to_string())).into(),
                     Some(match_expr),
                 );
-                let let_stmt = if_indent_level.increase_indent(let_stmt);
+                let let_stmt = let_stmt.indent(if_indent_level);
                 replace(let_stmt.syntax(), &then_block, &parent_block, &if_expr)
             }
         };
-        edit.target(if_expr.syntax().text_range());
-        edit.replace_ast(parent_block, ast::Block::cast(new_block).unwrap());
-        edit.set_cursor(cursor_position);
+        edit.replace_ast(parent_block, ast::BlockExpr::cast(new_block).unwrap());
 
         fn replace(
             new_expr: &SyntaxNode,
-            then_block: &Block,
-            parent_block: &Block,
+            then_block: &ast::BlockExpr,
+            parent_block: &ast::BlockExpr,
             if_expr: &ast::IfExpr,
         ) -> SyntaxNode {
-            let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone());
+            let then_block_items = then_block.dedent(IndentLevel::from(1));
             let end_of_then = then_block_items.syntax().last_child_or_token().unwrap();
             let end_of_then =
                 if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) {
@@ -182,7 +183,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option<Assist> {
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_not_applicable};
+    use crate::tests::{check_assist, check_assist_not_applicable};
 
     use super::*;
 
@@ -204,7 +205,7 @@ mod tests {
             r#"
             fn main() {
                 bar();
-                if<|> !true {
+                if !true {
                     return;
                 }
                 foo();
@@ -234,7 +235,7 @@ mod tests {
             r#"
             fn main(n: Option<String>) {
                 bar();
-                le<|>t n = match n {
+                let n = match n {
                     Some(it) => it,
                     _ => return,
                 };
@@ -260,7 +261,7 @@ mod tests {
             "#,
             r#"
             fn main() {
-                le<|>t x = match Err(92) {
+                let x = match Err(92) {