25 files changed, 716 insertions, 609 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 5da6a0340..eb7865c84 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -852,6 +852,7 @@ impl Function {
            })
            .collect()
    }
    pub fn method_params(self, db: &dyn HirDatabase) -> Option<Vec<Param>> {
        if self.self_param(db).is_none() {
            return None;
@@ -909,7 +910,7 @@ impl From<hir_ty::Mutability> for Access {
    }
 }
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct Param {
    func: Function,
    /// The index in parameter list, including self parameter.
@@ -922,13 +923,25 @@ impl Param {
        &self.ty
    }
+    pub fn as_local(&self, db: &dyn HirDatabase) -> Local {
+        let parent = DefWithBodyId::FunctionId(self.func.into());
+        let body = db.body(parent);
+        Local { parent, pat_id: body.params[self.idx] }
+    }
    pub fn pattern_source(&self, db: &dyn HirDatabase) -> Option<ast::Pat> {
-        let params = self.func.source(db)?.value.param_list()?;
+        self.source(db).and_then(|p| p.value.pat())
+    }
+    pub fn source(&self, db: &dyn HirDatabase) -> Option<InFile<ast::Param>> {
+        let InFile { file_id, value } = self.func.source(db)?;
+        let params = value.param_list()?;
        if params.self_param().is_some() {
-            params.params().nth(self.idx.checked_sub(1)?)?.pat()
+            params.params().nth(self.idx.checked_sub(1)?)
        } else {
-            params.params().nth(self.idx)?.pat()
+            params.params().nth(self.idx)
        }
+        .map(|value| InFile { file_id, value })
    }
 }
@@ -960,6 +973,14 @@ impl SelfParam {
            Access::Owned => "self",
        }
    }
+    pub fn source(&self, db: &dyn HirDatabase) -> Option<InFile<ast::SelfParam>> {
+        let InFile { file_id, value } = Function::from(self.func).source(db)?;
+        value
+            .param_list()
+            .and_then(|params| params.self_param())
+            .map(|value| InFile { file_id, value })
+    }
 }
 impl HasVisibility for Function {
@@ -1335,6 +1356,13 @@ impl Local {
        }
    }
+    pub fn as_self_param(self, db: &dyn HirDatabase) -> Option<SelfParam> {
+        match self.parent {
+            DefWithBodyId::FunctionId(func) if self.is_self(db) => Some(SelfParam { func }),
+            _ => None,
+        }
+    }
    // FIXME: why is this an option? It shouldn't be?
    pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
        let body = db.body(self.parent);
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 28b73c3a1..d8fabe49b 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -91,7 +91,6 @@ pub(super) fn collect_defs(
        resolved_imports: Vec::new(),
        unexpanded_macros: Vec::new(),
-        unexpanded_attribute_macros: Vec::new(),
        mod_dirs: FxHashMap::default(),
        cfg_options,
        proc_macros,
@@ -202,15 +201,14 @@ struct ImportDirective {
 #[derive(Clone, Debug, Eq, PartialEq)]
 struct MacroDirective {
    module_id: LocalModuleId,
-    ast_id: AstIdWithPath<ast::MacroCall>,
-    legacy: Option<MacroCallId>,
    depth: usize,
+    kind: MacroDirectiveKind,
 }
 #[derive(Clone, Debug, Eq, PartialEq)]
-struct DeriveDirective {
+enum MacroDirectiveKind {
-    module_id: LocalModuleId,
+    FnLike { ast_id: AstIdWithPath<ast::MacroCall>, legacy: Option<MacroCallId> },
-    ast_id: AstIdWithPath<ast::Item>,
+    Derive { ast_id: AstIdWithPath<ast::Item> },
 }
 struct DefData<'a> {
@@ -228,7 +226,6 @@ struct DefCollector<'a> {
    unresolved_imports: Vec<ImportDirective>,
    resolved_imports: Vec<ImportDirective>,
    unexpanded_macros: Vec<MacroDirective>,
-    unexpanded_attribute_macros: Vec<DeriveDirective>,
    mod_dirs: FxHashMap<LocalModuleId, ModDir>,
    cfg_options: &'a CfgOptions,
    /// List of procedural macros defined by this crate. This is read from the dynamic library
@@ -782,60 +779,58 @@ impl DefCollector<'_> {
    fn resolve_macros(&mut self) -> ReachedFixedPoint {
        let mut macros = std::mem::replace(&mut self.unexpanded_macros, Vec::new());
-        let mut attribute_macros =
-            std::mem::replace(&mut self.unexpanded_attribute_macros, Vec::new());
        let mut resolved = Vec::new();
        let mut res = ReachedFixedPoint::Yes;
        macros.retain(|directive| {
-            if let Some(call_id) = directive.legacy {
+            match &directive.kind {
-                res = ReachedFixedPoint::No;
+                MacroDirectiveKind::FnLike { ast_id, legacy } => {
-                resolved.push((directive.module_id, call_id, directive.depth));
+                    if let Some(call_id) = legacy {
-                return false;
+                        res = ReachedFixedPoint::No;
-            }
+                        resolved.push((directive.module_id, *call_id, directive.depth));
+                        return false;
+                    }
-            match macro_call_as_call_id(
+                    match macro_call_as_call_id(
-                &directive.ast_id,
+                        ast_id,
-                self.db,
-                self.def_map.krate,
-                |path| {
-                    let resolved_res = self.def_map.resolve_path_fp_with_macro(
                        self.db,
-                        ResolveMode::Other,
+                        self.def_map.krate,
-                        directive.module_id,
+                        |path| {
-                        &path,
+                            let resolved_res = self.def_map.resolve_path_fp_with_macro(
-                        BuiltinShadowMode::Module,
+                                self.db,
-                    );
+                                ResolveMode::Other,
-                    resolved_res.resolved_def.take_macros()
+                                directive.module_id,
-                },
+                                &path,
-                &mut |_err| (),
+                                BuiltinShadowMode::Module,
-            ) {
+                            );
-                Ok(Ok(call_id)) => {
+                            resolved_res.resolved_def.take_macros()
-                    resolved.push((directive.module_id, call_id, directive.depth));
+                        },
-                    res = ReachedFixedPoint::No;
+                        &mut |_err| (),
-                    return false;
+                    ) {
+                        Ok(Ok(call_id)) => {
+                            resolved.push((directive.module_id, call_id, directive.depth));
+                            res = ReachedFixedPoint::No;
+                            return false;
+                        }
+                        Err(UnresolvedMacro) | Ok(Err(_)) => {}
+                    }
                }
-                Err(UnresolvedMacro) | Ok(Err(_)) => {}
+                MacroDirectiveKind::Derive { ast_id } => {
-            }
+                    match derive_macro_as_call_id(ast_id, self.db, self.def_map.krate, |path| {
+                        self.resolve_derive_macro(directive.module_id, &path)
-            true
+                    }) {
-        });
+                        Ok(call_id) => {
-        attribute_macros.retain(|directive| {
+                            resolved.push((directive.module_id, call_id, 0));
-            match derive_macro_as_call_id(&directive.ast_id, self.db, self.def_map.krate, |path| {
+                            res = ReachedFixedPoint::No;
-                self.resolve_derive_macro(&directive, &path)
+                            return false;
-            }) {
+                        }
-                Ok(call_id) => {
+                        Err(UnresolvedMacro) => (),
-                    resolved.push((directive.module_id, call_id, 0));
+                    }
-                    res = ReachedFixedPoint::No;
-                    return false;
                }
-                Err(UnresolvedMacro) => (),
            }
            true
        });
        self.unexpanded_macros = macros;
-        self.unexpanded_attribute_macros = attribute_macros;
        for (module_id, macro_call_id, depth) in resolved {
            self.collect_macro_expansion(module_id, macro_call_id, depth);
@@ -844,15 +839,11 @@ impl DefCollector<'_> {
        res
    }
-    fn resolve_derive_macro(
+    fn resolve_derive_macro(&self, module: LocalModuleId, path: &ModPath) -> Option<MacroDefId> {
-        &self,
-        directive: &DeriveDirective,
-        path: &ModPath,
-    ) -> Option<MacroDefId> {
        let resolved_res = self.def_map.resolve_path_fp_with_macro(
            self.db,
            ResolveMode::Other,
-            directive.module_id,
+            module,
            &path,
            BuiltinShadowMode::Module,
        );
@@ -912,33 +903,35 @@ impl DefCollector<'_> {
        // Emit diagnostics for all remaining unexpanded macros.
        for directive in &self.unexpanded_macros {
-            let mut error = None;
+            match &directive.kind {
-            match macro_call_as_call_id(
+                MacroDirectiveKind::FnLike { ast_id, .. } => match macro_call_as_call_id(
-                &directive.ast_id,
+                    ast_id,
-                self.db,
+                    self.db,
-                self.def_map.krate,
+                    self.def_map.krate,
-                |path| {
+                    |path| {
-                    let resolved_res = self.def_map.resolve_path_fp_with_macro(
+                        let resolved_res = self.def_map.resolve_path_fp_with_macro(
-                        self.db,
+                            self.db,
-                        ResolveMode::Other,
+                            ResolveMode::Other,
-                        directive.module_id,
+                            directive.module_id,
-                        &path,
+                            &path,
-                        BuiltinShadowMode::Module,
+                            BuiltinShadowMode::Module,
-                    );
+                        );
-                    resolved_res.resolved_def.take_macros()
+                        resolved_res.resolved_def.take_macros()
-                },
+                    },
-                &mut |e| {
+                    &mut |_| (),
-                    error.get_or_insert(e);
+                ) {
+                    Ok(_) => (),
+                    Err(UnresolvedMacro) => {
+                        self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
+                            directive.module_id,
+                            ast_id.ast_id,
+                        ));
+                    }
                },
-            ) {
+                MacroDirectiveKind::Derive { .. } => {
-                Ok(_) => (),
+                    // FIXME: we might want to diagnose this too
-                Err(UnresolvedMacro) => {
-                    self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
-                        directive.module_id,
-                        directive.ast_id.ast_id,
-                    ));
                }
-            };
+            }
        }
        // Emit diagnostics for all remaining unresolved imports.
@@ -1380,9 +1373,11 @@ impl ModCollector<'_, '_> {
                Some(derive_macros) => {
                    for path in derive_macros {
                        let ast_id = AstIdWithPath::new(self.file_id, ast_id, path);
-                        self.def_collector
+                        self.def_collector.unexpanded_macros.push(MacroDirective {
-                            .unexpanded_attribute_macros
+                            module_id: self.module_id,
-                            .push(DeriveDirective { module_id: self.module_id, ast_id });
+                            depth: self.macro_depth + 1,
+                            kind: MacroDirectiveKind::Derive { ast_id },
+                        });
                    }
                }
                None => {
@@ -1497,9 +1492,8 @@ impl ModCollector<'_, '_> {
        self.def_collector.unexpanded_macros.push(MacroDirective {
            module_id: self.module_id,
-            ast_id,
-            legacy: None,
            depth: self.macro_depth + 1,
+            kind: MacroDirectiveKind::FnLike { ast_id, legacy: None },
        });
    }
@@ -1542,7 +1536,6 @@ mod tests {
            unresolved_imports: Vec::new(),
            resolved_imports: Vec::new(),
            unexpanded_macros: Vec::new(),
-            unexpanded_attribute_macros: Vec::new(),
            mod_dirs: FxHashMap::default(),
            cfg_options: &CfgOptions::default(),
            proc_macros: Default::default(),
diff --git a/crates/ide/src/annotations.rs b/crates/ide/src/annotations.rs
index 72492f826..64bc926f1 100644
--- a/crates/ide/src/annotations.rs
+++ b/crates/ide/src/annotations.rs
@@ -1,5 +1,5 @@
 use either::Either;
-use hir::{HasSource, Semantics};
+use hir::{HasSource, InFile, Semantics};
 use ide_db::{
    base_db::{FileId, FilePosition, FileRange},
    helpers::visit_file_defs,
@@ -80,19 +80,19 @@ pub(crate) fn annotations(
        Either::Left(def) => {
            let node = match def {
                hir::ModuleDef::Const(konst) => {
-                    konst.source(db).and_then(|node| range_and_position_of(&node.value))
+                    konst.source(db).and_then(|node| range_and_position_of(&node, file_id))
                }
                hir::ModuleDef::Trait(trait_) => {
-                    trait_.source(db).and_then(|node| range_and_position_of(&node.value))
+                    trait_.source(db).and_then(|node| range_and_position_of(&node, file_id))
                }
                hir::ModuleDef::Adt(hir::Adt::Struct(strukt)) => {
-                    strukt.source(db).and_then(|node| range_and_position_of(&node.value))
+                    strukt.source(db).and_then(|node| range_and_position_of(&node, file_id))
                }
                hir::ModuleDef::Adt(hir::Adt::Enum(enum_)) => {
-                    enum_.source(db).and_then(|node| range_and_position_of(&node.value))
+                    enum_.source(db).and_then(|node| range_and_position_of(&node, file_id))
                }
                hir::ModuleDef::Adt(hir::Adt::Union(union)) => {
-                    union.source(db).and_then(|node| range_and_position_of(&node.value))
+                    union.source(db).and_then(|node| range_and_position_of(&node, file_id))
                }
                _ => None,
            };
@@ -120,8 +120,19 @@ pub(crate) fn annotations(
                });
            }
-            fn range_and_position_of(node: &dyn NameOwner) -> Option<(TextSize, TextRange)> {
+            fn range_and_position_of<T: NameOwner>(
-                Some((node.name()?.syntax().text_range().start(), node.syntax().text_range()))
+                node: &InFile<T>,
+                file_id: FileId,
+            ) -> Option<(TextSize, TextRange)> {
+                if node.file_id != file_id.into() {
+                    // Node is outside the file we are adding annotations to (e.g. macros).
+                    None
+                } else {
+                    Some((
+                        node.value.name()?.syntax().text_range().start(),
+                        node.value.syntax().text_range(),
+                    ))
+                }
            }
        }
        Either::Right(_) => (),
@@ -967,4 +978,23 @@ struct Foo;
            "#]],
        );
    }
+    #[test]
+    fn test_no_annotations_macro_struct_def() {
+        check(
+            r#"
+//- /lib.rs
+macro_rules! m {
+    () => {
+        struct A {}
+    };
+}
+m!();
+"#,
+            expect![[r#"
+                []
+            "#]],
+        );
+    }
 }
diff --git a/crates/ide/src/call_hierarchy.rs b/crates/ide/src/call_hierarchy.rs
index 96021f677..5cd186565 100644
--- a/crates/ide/src/call_hierarchy.rs
+++ b/crates/ide/src/call_hierarchy.rs
@@ -50,16 +50,16 @@ pub(crate) fn incoming_calls(db: &RootDatabase, position: FilePosition) -> Optio
    for (file_id, references) in refs.references {
        let file = sema.parse(file_id);
        let file = file.syntax();
-        for (r_range, _) in references {
+        for (relative_range, token) in references
-            let token = file.token_at_offset(r_range.start()).next()?;
+            .into_iter()
+            .filter_map(|(range, _)| Some(range).zip(file.token_at_offset(range.start()).next()))
+        {
            let token = sema.descend_into_macros(token);
            // This target is the containing function
            if let Some(nav) = token.ancestors().find_map(|node| {
-                let fn_ = ast::Fn::cast(node)?;
+                let def = ast::Fn::cast(node).and_then(|fn_| sema.to_def(&fn_))?;
-                let def = sema.to_def(&fn_)?;
                def.try_to_nav(sema.db)
            }) {
-                let relative_range = r_range;
                calls.add(&nav, relative_range);
            }
        }
@@ -87,7 +87,6 @@ pub(crate) fn outgoing_calls(db: &RootDatabase, position: FilePosition) -> Optio
            let name_ref = call_node.name_ref()?;
            let func_target = match call_node {
                FnCallNode::CallExpr(expr) => {
-                    //FIXME: Type::as_callable is broken
                    let callable = sema.type_of_expr(&expr.expr()?)?.as_callable(db)?;
                    match callable.kind() {
                        hir::CallableKind::Function(it) => it.try_to_nav(db),
diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs
index c7c1f4fee..0cee741ac 100644
--- a/crates/ide/src/doc_links.rs
+++ b/crates/ide/src/doc_links.rs
@@ -1,4 +1,4 @@
-//! Resolves and rewrites links in markdown documentation.
+//! Extracts, resolves and rewrites links and intra-doc links in markdown documentation.
 use std::{convert::TryFrom, iter::once, ops::Range};
@@ -15,7 +15,10 @@ use ide_db::{
    defs::{Definition, NameClass, NameRefClass},
    RootDatabase,
 };
-use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T};
+use syntax::{
+    ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxNode, SyntaxToken, TextRange, TextSize,
+    TokenAtOffset, T,
+};
 use crate::{FilePosition, Semantics};
@@ -60,29 +63,6 @@ pub(crate) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Defi
    out
 }
-pub(crate) fn extract_definitions_from_markdown(
-    markdown: &str,
-) -> Vec<(String, Option<hir::Namespace>, Range<usize>)> {
-    let mut res = vec![];
-    let mut cb = |link: BrokenLink| {
-        // These allocations are actually unnecessary but the lifetimes on BrokenLinkCallback are wrong
-        // this is fixed in the repo but not on the crates.io release yet
-        Some((
-            /*url*/ link.reference.to_owned().into(),
-            /*title*/ link.reference.to_owned().into(),
-        ))
-    };
-    let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb));
-    for (event, range) in doc.into_offset_iter() {
-        if let Event::Start(Tag::Link(_, target, title)) = event {
-            let link = if target.is_empty() { title } else { target };
-            let (link, ns) = parse_link(&link);
-            res.push((link.to_string(), ns, range));
-        }
-    }
-    res
-}
 /// Remove all links in markdown documentation.
 pub(crate) fn remove_links(markdown: &str) -> String {
    let mut drop_link = false;
@@ -118,6 +98,105 @@ pub(crate) fn remove_links(markdown: &str) -> String {
    out
 }
+pub(crate) fn extract_definitions_from_markdown(
+    markdown: &str,
+) -> Vec<(Range<usize>, String, Option<hir::Namespace>)> {
+    let mut res = vec![];
+    let mut cb = |link: BrokenLink| {
+        // These allocations are actually unnecessary but the lifetimes on BrokenLinkCallback are wrong
+        // this is fixed in the repo but not on the crates.io release yet
+        Some((
+            /*url*/ link.reference.to_owned().into(),
+            /*title*/ link.reference.to_owned().into(),
+        ))
+    };
+    let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb));
+    for (event, range) in doc.into_offset_iter() {
+        if let Event::Start(Tag::Link(_, target, title)) = event {
+            let link = if target.is_empty() { title } else { target };
+            let (link, ns) = parse_intra_doc_link(&link);
+            res.push((range, link.to_string(), ns));
+        }
+    }
+    res
+}
+/// Extracts a link from a comment at the given position returning the spanning range, link and
+/// optionally it's namespace.
+pub(crate) fn extract_positioned_link_from_comment(
+    position: TextSize,
+    comment: &ast::Comment,
+) -> Option<(TextRange, String, Option<hir::Namespace>)> {
+    let doc_comment = comment.doc_comment()?;
+    let comment_start =
+        comment.syntax().text_range().start() + TextSize::from(comment.prefix().len() as u32);
+    let def_links = extract_definitions_from_markdown(doc_comment);
+    let (range, def_link, ns) =
+        def_links.into_iter().find_map(|(Range { start, end }, def_link, ns)| {
+            let range = TextRange::at(
+                comment_start + TextSize::from(start as u32),
+                TextSize::from((end - start) as u32),
+            );
+            range.contains(position).then(|| (range, def_link, ns))
+        })?;
+    Some((range, def_link, ns))
+}
+/// Turns a syntax node into it's [`Definition`] if it can hold docs.
+pub(crate) fn doc_owner_to_def(
+    sema: &Semantics<RootDatabase>,
+    item: &SyntaxNode,
+) -> Option<Definition> {
+    let res: hir::ModuleDef = match_ast! {
+        match item {
+            ast::SourceFile(_it) => sema.scope(item).module()?.into(),
+            ast::Fn(it) => sema.to_def(&it)?.into(),
+            ast::Struct(it) => sema.to_def(&it)?.into(),
+            ast::Enum(it) => sema.to_def(&it)?.into(),
+            ast::Union(it) => sema.to_def(&it)?.into(),
+            ast::Trait(it) => sema.to_def(&it)?.into(),
+            ast::Const(it) => sema.to_def(&it)?.into(),
+            ast::Static(it) => sema.to_def(&it)?.into(),
+            ast::TypeAlias(it) => sema.to_def(&it)?.into(),
+            ast::Variant(it) => sema.to_def(&it)?.into(),
+            ast::Trait(it) => sema.to_def(&it)?.into(),
+            ast::Impl(it) => return sema.to_def(&it).map(Definition::SelfType),
+            ast::MacroRules(it) => return sema.to_def(&it).map(Definition::Macro),
+            ast::TupleField(it) => return sema.to_def(&it).map(Definition::Field),
+            ast::RecordField(it) => return sema.to_def(&it).map(Definition::Field),
+            _ => return None,
+        }
+    };
+    Some(Definition::ModuleDef(res))
+}
+pub(crate) fn resolve_doc_path_for_def(
+    db: &dyn HirDatabase,
+    def: Definition,
+    link: &str,
+    ns: Option<hir::Namespace>,
+) -> Option<hir::ModuleDef> {
+    match def {
+        Definition::ModuleDef(def) => match def {
+            ModuleDef::Module(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Function(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Adt(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Variant(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Const(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Static(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::Trait(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::TypeAlias(it) => it.resolve_doc_path(db, &link, ns),
+            ModuleDef::BuiltinType(_) => None,
+        },
+        Definition::Macro(it) => it.resolve_doc_path(db, &link, ns),
+        Definition::Field(it) => it.resolve_doc_path(db, &link, ns),
+        Definition::SelfType(_)
+        | Definition::Local(_)
+        | Definition::GenericParam(_)
+        | Definition::Label(_) => None,
+    }
+}
 // FIXME:
 // BUG: For Option::Some
 // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some
@@ -197,26 +276,8 @@ fn rewrite_intra_doc_link(
    title: &str,
 ) -> Option<(String, String)> {
    let link = if target.is_empty() { title } else { target };
-    let (link, ns) = parse_link(link);
+    let (link, ns) = parse_intra_doc_link(link);
-    let resolved = match def {
+    let resolved = resolve_doc_path_for_def(db, def, link, ns)?;
-        Definition::ModuleDef(def) => match def {
-            ModuleDef::Module(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Function(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Adt(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Variant(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Const(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Static(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::Trait(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::TypeAlias(it) => it.resolve_doc_path(db, link, ns),
-            ModuleDef::BuiltinType(_) => return None,
-        },
-        Definition::Macro(it) => it.resolve_doc_path(db, link, ns),
-        Definition::Field(it) => it.resolve_doc_path(db, link, ns),
-        Definition::SelfType(_)
-        | Definition::Local(_)
-        | Definition::GenericParam(_)
-        | Definition::Label(_) => return None,
-    }?;
    let krate = resolved.module(db)?.krate();
    let canonical_path = resolved.canonical_path(db)?;
    let mut new_url = get_doc_url(db, &krate)?
@@ -228,24 +289,23 @@ fn rewrite_intra_doc_link(
        .ok()?;
    if let ModuleDef::Trait(t) = resolved {
-        let items = t.items(db);
+        if let Some(assoc_item) = t.items(db).into_iter().find_map(|assoc_item| {
-        if let Some(field_or_assoc_item) = items.iter().find_map(|assoc_item| {
            if let Some(name) = assoc_item.name(db) {
                if *link == format!("{}::{}", canonical_path, name) {
-                    return Some(FieldOrAssocItem::AssocItem(*assoc_item));
+                    return Some(assoc_item);
                }
            }
            None
        }) {
-            if let Some(fragment) = get_symbol_fragment(db, &field_or_assoc_item) {
+            if let Some(fragment) =
+                get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(assoc_item))
+            {
                new_url = new_url.join(&fragment).ok()?;
            }
        };
    }
-    let new_target = new_url.into_string();
+    Some((new_url.into_string(), strip_prefixes_suffixes(title).to_string()))
-    let new_title = strip_prefixes_suffixes(title);
-    Some((new_target, new_title.to_string()))
 }
 /// Try to resolve path to local documentation via path-based links (i.e. `../gateway/struct.Shard.html`).
@@ -322,73 +382,61 @@ fn map_links<'e>(
    })
 }
-fn parse_link(s: &str) -> (&str, Option<hir::Namespace>) {
+const TYPES: ([&str; 9], [&str; 0]) =
-    let path = strip_prefixes_suffixes(s);
+    (["type", "struct", "enum", "mod", "trait", "union", "module", "prim", "primitive"], []);
-    let ns = ns_from_intra_spec(s);
+const VALUES: ([&str; 8], [&str; 1]) =
-    (path, ns)
-}
-/// Strip prefixes, suffixes, and inline code marks from the given string.
-fn strip_prefixes_suffixes(mut s: &str) -> &str {
-    s = s.trim_matches('`');
-    [
-        (TYPES.0.iter(), TYPES.1.iter()),
-        (VALUES.0.iter(), VALUES.1.iter()),
-        (MACROS.0.iter(), MACROS.1.iter()),
-    ]
-    .iter()
-    .for_each(|(prefixes, suffixes)| {
-        prefixes.clone().for_each(|prefix| s = s.trim_start_matches(*prefix));
-        suffixes.clone().for_each(|suffix| s = s.trim_end_matches(*suffix));
-    });
-    s.trim_start_matches('@').trim()
-}
-static TYPES: ([&str; 7], [&str; 0]) =
-    (["type", "struct", "enum", "mod", "trait", "union", "module"], []);
-static VALUES: ([&str; 8], [&str; 1]) =
    (["value", "function", "fn", "method", "const", "static", "mod", "module"], ["()"]);
-static MACROS: ([&str; 1], [&str; 1]) = (["macro"], ["!"]);
+const MACROS: ([&str; 2], [&str; 1]) = (["macro", "derive"], ["!"]);
 /// Extract the specified namespace from an intra-doc-link if one exists.
 ///
 /// # Examples
 ///
-/// * `struct MyStruct` -> `Namespace::Types`
+/// * `struct MyStruct` -> ("MyStruct", `Namespace::Types`)
-/// * `panic!` -> `Namespace::Macros`
+/// * `panic!` -> ("panic", `Namespace::Macros`)
-/// * `fn@from_intra_spec` -> `Namespace::Values`
+/// * `fn@from_intra_spec` -> ("from_intra_spec", `Namespace::Values`)
-fn ns_from_intra_spec(s: &str) -> Option<hir::Namespace> {
+fn parse_intra_doc_link(s: &str) -> (&str, Option<hir::Namespace>) {
+    let s = s.trim_matches('`');
    [
        (hir::Namespace::Types, (TYPES.0.iter(), TYPES.1.iter())),
        (hir::Namespace::Values, (VALUES.0.iter(), VALUES.1.iter())),
        (hir::Namespace::Macros, (MACROS.0.iter(), MACROS.1.iter())),
    ]
    .iter()
-    .filter(|(_ns, (prefixes, suffixes))| {
+    .cloned()
-        prefixes
+    .find_map(|(ns, (mut prefixes, mut suffixes))| {
-            .clone()
+        if let Some(prefix) = prefixes.find(|&&prefix| {
-            .map(|prefix| {
+            s.starts_with(prefix)
-                s.starts_with(*prefix)
+                && s.chars().nth(prefix.len()).map_or(false, |c| c == '@' || c == ' ')
-                    && s.chars()
+        }) {
-                        .nth(prefix.len() + 1)
+            Some((&s[prefix.len() + 1..], ns))
-                        .map(|c| c == '@' || c == ' ')
+        } else {
-                        .unwrap_or(false)
+            suffixes.find_map(|&suffix| s.strip_suffix(suffix).zip(Some(ns)))
-            })
+        }
-            .any(|cond| cond)
+    })
-            || suffixes
+    .map_or((s, None), |(s, ns)| (s, Some(ns)))
-                .clone()
+}
-                .map(|suffix| {
-                    s.starts_with(*suffix)
+fn strip_prefixes_suffixes(s: &str) -> &str {
-                        && s.chars()
+    [
-                            .nth(suffix.len() + 1)
+        (TYPES.0.iter(), TYPES.1.iter()),
-                            .map(|c| c == '@' || c == ' ')
+        (VALUES.0.iter(), VALUES.1.iter()),
-                            .unwrap_or(false)
+        (MACROS.0.iter(), MACROS.1.iter()),
-                })
+    ]
-                .any(|cond| cond)
+    .iter()
+    .cloned()
+    .find_map(|(mut prefixes, mut suffixes)| {
+        if let Some(prefix) = prefixes.find(|&&prefix| {
+            s.starts_with(prefix)
+                && s.chars().nth(prefix.len()).map_or(false, |c| c == '@' || c == ' ')
+        }) {
+            Some(&s[prefix.len() + 1..])
+        } else {
+            suffixes.find_map(|&suffix| s.strip_suffix(suffix))
+        }
    })
-    .map(|(ns, (_, _))| *ns)
+    .unwrap_or(s)
-    .next()
 }
 /// Get the root URL for the documentation of a crate.
diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index 473d48c2f..a2c97061f 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -1,18 +1,14 @@
-use std::ops::Range;
 use either::Either;
-use hir::{HasAttrs, ModuleDef, Semantics};
+use hir::Semantics;
 use ide_db::{
-    defs::{Definition, NameClass, NameRefClass},
+    defs::{NameClass, NameRefClass},
    RootDatabase,
 };
-use syntax::{
+use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TokenAtOffset, T};
-    ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TextRange, TextSize,
-    TokenAtOffset, T,
-};
 use crate::{
-    display::TryToNav, doc_links::extract_definitions_from_markdown, runnables::doc_owner_to_def,
+    display::TryToNav,
+    doc_links::{doc_owner_to_def, extract_positioned_link_from_comment, resolve_doc_path_for_def},
    FilePosition, NavigationTarget, RangeInfo,
 };
@@ -35,7 +31,9 @@ pub(crate) fn goto_definition(
    let token = sema.descend_into_macros(original_token.clone());
    let parent = token.parent()?;
    if let Some(comment) = ast::Comment::cast(token) {
-        let nav = def_for_doc_comment(&sema, position, &comment)?.try_to_nav(db)?;
+        let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment)?;
+        let def = doc_owner_to_def(&sema, &parent)?;
+        let nav = resolve_doc_path_for_def(db, def, &link, ns)?.try_to_nav(db)?;
        return Some(RangeInfo::new(original_token.text_range(), vec![nav]));
    }
@@ -61,54 +59,6 @@ pub(crate) fn goto_definition(
    Some(RangeInfo::new(original_token.text_range(), nav.into_iter().collect()))
 }
-fn def_for_doc_comment(
-    sema: &Semantics<RootDatabase>,
-    position: FilePosition,
-    doc_comment: &ast::Comment,
-) -> Option<hir::ModuleDef> {
-    let parent = doc_comment.syntax().parent()?;
-    let (link, ns) = extract_positioned_link_from_comment(position, doc_comment)?;
-    let def = doc_owner_to_def(sema, parent)?;
-    match def {
-        Definition::ModuleDef(def) => match def {
-            ModuleDef::Module(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Function(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Adt(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Variant(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Const(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Static(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::Trait(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::TypeAlias(it) => it.resolve_doc_path(sema.db, &link, ns),
-            ModuleDef::BuiltinType(_) => return None,
-        },
-        Definition::Macro(it) => it.resolve_doc_path(sema.db, &link, ns),
-        Definition::Field(it) => it.resolve_doc_path(sema.db, &link, ns),
-        Definition::SelfType(_)
-        | Definition::Local(_)
-        | Definition::GenericParam(_)
-        | Definition::Label(_) => return None,
-    }
-}
-fn extract_positioned_link_from_comment(
-    position: FilePosition,
-    comment: &ast::Comment,
-) -> Option<(String, Option<hir::Namespace>)> {
-    let doc_comment = comment.doc_comment()?;
-    let comment_start =
-        comment.syntax().text_range().start() + TextSize::from(comment.prefix().len() as u32);
-    let def_links = extract_definitions_from_markdown(doc_comment);
-    let (def_link, ns, _) = def_links.into_iter().find(|&(_, _, Range { start, end })| {
-        TextRange::at(
-            comment_start + TextSize::from(start as u32),
-            TextSize::from((end - start) as u32),
-        )
-        .contains(position.offset)
-    })?;
-    Some((def_link, ns))
-}
 fn pick_best(tokens: TokenAtOffset<SyntaxToken>) -> Option<SyntaxToken> {
    return tokens.max_by_key(priority);
    fn priority(n: &SyntaxToken) -> usize {
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index a3fb17c0a..c43089476 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -11,11 +11,14 @@ use ide_db::{
 };
 use itertools::Itertools;
 use stdx::format_to;
-use syntax::{ast, match_ast, AstNode, SyntaxKind::*, SyntaxToken, TokenAtOffset, T};
+use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TokenAtOffset, T};
 use crate::{
    display::{macro_label, TryToNav},
-    doc_links::{remove_links, rewrite_links},
+    doc_links::{
+        doc_owner_to_def, extract_positioned_link_from_comment, remove_links,
+        resolve_doc_path_for_def, rewrite_links,
+    },
    markdown_remove::remove_markdown,
    markup::Markup,
    runnables::{runnable_fn, runnable_mod},
@@ -93,20 +96,35 @@ pub(crate) fn hover(
    let mut res = HoverResult::default();
    let node = token.parent()?;
+    let mut range = None;
    let definition = match_ast! {
        match node {
            // we don't use NameClass::referenced_or_defined here as we do not want to resolve
            // field pattern shorthands to their definition
            ast::Name(name) => NameClass::classify(&sema, &name).and_then(|class| match class {
                NameClass::ConstReference(def) => Some(def),
-                def => def.defined(sema.db),
+                def => def.defined(db),
            }),
-            ast::NameRef(name_ref) => NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(sema.db)),
+            ast::NameRef(name_ref) => {
-            ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime)
+                NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(db))
-                .map_or_else(|| NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(sema.db)), |d| d.defined(sema.db)),
+            },
-            _ => None,
+            ast::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime).map_or_else(
+                || NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(db)),
+                |d| d.defined(db),
+            ),
+            _ => ast::Comment::cast(token.clone())
+                .and_then(|comment| {
+                    let (idl_range, link, ns) =
+                        extract_positioned_link_from_comment(position.offset, &comment)?;
+                    range = Some(idl_range);
+                    let def = doc_owner_to_def(&sema, &node)?;
+                    resolve_doc_path_for_def(db, def, &link, ns)
+                })
+                .map(Definition::ModuleDef),
        }
    };
    if let Some(definition) = definition {
        let famous_defs = match &definition {
            Definition::ModuleDef(ModuleDef::BuiltinType(_)) => {
@@ -128,15 +146,16 @@ pub(crate) fn hover(
                res.actions.push(action);
            }
-            let range = sema.original_range(&node).range;
+            let range = range.unwrap_or_else(|| sema.original_range(&node).range);
            return Some(RangeInfo::new(range, res));
        }
    }
    if token.kind() == syntax::SyntaxKind::COMMENT {
-        // don't highlight the entire parent node on comment hover
+        cov_mark::hit!(no_highlight_on_comment_hover);
        return None;
    }
    if let res @ Some(_) = hover_for_keyword(&sema, links_in_hover, markdown, &token) {
        return res;
    }
@@ -3483,6 +3502,7 @@ fn foo$0() {}
    #[test]
    fn hover_comments_dont_highlight_parent() {
+        cov_mark::check!(no_highlight_on_comment_hover);
        check_hover_no_result(
            r#"
 fn no_hover() {
@@ -3755,4 +3775,29 @@ fn main() {
            "#]],
        )
    }
+    #[test]
+    fn hover_intra_doc_links() {
+        check(
+            r#"
+/// This is the [`foo`](foo$0) function.
+fn foo() {}
+"#,
+            expect![[r#"
+                *[`foo`](foo)*
+                ```rust
+                test
+                ```
+                ```rust
+                fn foo()
+                ```
+                ---
+                This is the [`foo`](https://docs.rs/test/*/test/fn.foo.html) function.
+            "#]],
+        );
+    }
 }
diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index 5340b638a..26d6dc9c9 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -1,8 +1,11 @@
-//! FIXME: write short doc here
+//! Renaming functionality
+//!
+//! All reference and file rename requests go through here where the corresponding [`SourceChange`]s
+//! will be calculated.
 use std::fmt::{self, Display};
 use either::Either;
-use hir::{HasSource, InFile, Module, ModuleDef, ModuleSource, Semantics};
+use hir::{AsAssocItem, InFile, Module, ModuleDef, ModuleSource, Semantics};
 use ide_db::{
    base_db::{AnchoredPathBuf, FileId},
    defs::{Definition, NameClass, NameRefClass},
@@ -196,7 +199,7 @@ fn rename_mod(
                file_id,
                TextEdit::replace(name.syntax().text_range(), new_name.to_string()),
            ),
-            _ => unreachable!(),
+            _ => never!("Module source node is missing a name"),
        }
    }
    let def = Definition::ModuleDef(ModuleDef::Module(module));
@@ -216,40 +219,44 @@ fn rename_reference(
 ) -> RenameResult<SourceChange> {
    let ident_kind = check_identifier(new_name)?;
-    let def_is_lbl_or_lt = matches!(
+    if matches!(
-        def,
+        def, // is target a lifetime?
        Definition::GenericParam(hir::GenericParam::LifetimeParam(_)) | Definition::Label(_)
-    );
+    ) {
-    match (ident_kind, def) {
+        match ident_kind {
-        (IdentifierKind::ToSelf, _)
+            IdentifierKind::Ident | IdentifierKind::ToSelf | IdentifierKind::Underscore => {
-        | (IdentifierKind::Underscore, _)
+                cov_mark::hit!(rename_not_a_lifetime_ident_ref);
-        | (IdentifierKind::Ident, _)
+                bail!("Invalid name `{}`: not a lifetime identifier", new_name);
-            if def_is_lbl_or_lt =>
+            }
-        {
+            IdentifierKind::Lifetime => cov_mark::hit!(rename_lifetime),
-            cov_mark::hit!(rename_not_a_lifetime_ident_ref);
-            bail!("Invalid name `{}`: not a lifetime identifier", new_name)
-        }
-        (IdentifierKind::Lifetime, _) if def_is_lbl_or_lt => cov_mark::hit!(rename_lifetime),
-        (IdentifierKind::Lifetime, _) => {
-            cov_mark::hit!(rename_not_an_ident_ref);
-            bail!("Invalid name `{}`: not an identifier", new_name)
-        }
-        (IdentifierKind::ToSelf, Definition::Local(local)) if local.is_self(sema.db) => {
-            // no-op
-            cov_mark::hit!(rename_self_to_self);
-            return Ok(SourceChange::default());
-        }
-        (ident_kind, Definition::Local(local)) if local.is_self(sema.db) => {
-            cov_mark::hit!(rename_self_to_param);
-            return rename_self_to_param(sema, local, new_name, ident_kind);
-        }
-        (IdentifierKind::ToSelf, Definition::Local(local)) => {
-            cov_mark::hit!(rename_to_self);
-            return rename_to_self(sema, local);
        }
-        (IdentifierKind::ToSelf, _) => bail!("Invalid name `{}`: not an identifier", new_name),
+    } else {
-        (IdentifierKind::Ident, _) | (IdentifierKind::Underscore, _) => {
+        match (ident_kind, def) {
-            cov_mark::hit!(rename_ident)
+            (IdentifierKind::Lifetime, _) => {
+                cov_mark::hit!(rename_not_an_ident_ref);
+                bail!("Invalid name `{}`: not an identifier", new_name);
+            }
+            (IdentifierKind::ToSelf, Definition::Local(local)) => {
+                if local.is_self(sema.db) {
+                    // no-op
+                    cov_mark::hit!(rename_self_to_self);
+                    return Ok(SourceChange::default());
+                } else {
+                    cov_mark::hit!(rename_to_self);
+                    return rename_to_self(sema, local);
+                }
+            }
+            (ident_kind, Definition::Local(local)) => {
+                if let Some(self_param) = local.as_self_param(sema.db) {
+                    cov_mark::hit!(rename_self_to_param);
+                    return rename_self_to_param(sema, local, self_param, new_name, ident_kind);
+                } else {
+                    cov_mark::hit!(rename_local);
+                }
+            }
+            (IdentifierKind::ToSelf, _) => bail!("Invalid name `{}`: not an identifier", new_name),
+            (IdentifierKind::Ident, _) => cov_mark::hit!(rename_non_local),
+            (IdentifierKind::Underscore, _) => (),
        }
    }
@@ -275,46 +282,32 @@ fn rename_to_self(sema: &Semantics<RootDatabase>, local: hir::Local) -> RenameRe
    let fn_def = match local.parent(sema.db) {
        hir::DefWithBody::Function(func) => func,
-        _ => bail!("Cannot rename non-param local to self"),
+        _ => bail!("Cannot rename local to self outside of function"),
    };
-    // FIXME: reimplement this on the hir instead
+    if let Some(_) = fn_def.self_param(sema.db) {
-    // as of the time of this writing params in hir don't keep their names
-    let fn_ast = fn_def
-        .source(sema.db)
-        .ok_or_else(|| format_err!("Cannot rename non-param local to self"))?
-        .value;
-    let first_param_range = fn_ast
-        .param_list()
-        .and_then(|p| p.params().next())
-        .ok_or_else(|| format_err!("Method has no parameters"))?
-        .syntax()
-        .text_range();
-    let InFile { file_id, value: local_source } = local.source(sema.db);
-    match local_source {
-        either::Either::Left(pat)
-            if !first_param_range.contains_range(pat.syntax().text_range()) =>
-        {
-            bail!("Only the first parameter can be self");
-        }
-        _ => (),
-    }
-    let impl_block = fn_ast
-        .syntax()
-        .ancestors()
-        .find_map(|node| ast::Impl::cast(node))
-        .and_then(|def| sema.to_def(&def))
-        .ok_or_else(|| format_err!("No impl block found for function"))?;
-    if fn_def.self_param(sema.db).is_some() {
        bail!("Method already has a self parameter");
    }
    let params = fn_def.assoc_fn_params(sema.db);
-    let first_param = params.first().ok_or_else(|| format_err!("Method has no parameters"))?;
+    let first_param = params
+        .first()
+        .ok_or_else(|| format_err!("Cannot rename local to self unless it is a parameter"))?;
+    if first_param.as_local(sema.db) != local {
+        bail!("Only the first parameter may be renamed to self");
+    }
+    let assoc_item = fn_def
+        .as_assoc_item(sema.db)
+        .ok_or_else(|| format_err!("Cannot rename parameter to self for free function"))?;
+    let impl_ = match assoc_item.container(sema.db) {
+        hir::AssocItemContainer::Trait(_) => {
+            bail!("Cannot rename parameter to self for trait functions");
+        }
+        hir::AssocItemContainer::Impl(impl_) => impl_,
+    };
    let first_param_ty = first_param.ty();
-    let impl_ty = impl_block.target_ty(sema.db);
+    let impl_ty = impl_.target_ty(sema.db);
    let (ty, self_param) = if impl_ty.remove_ref().is_some() {
        // if the impl is a ref to the type we can just match the `&T` with self directly
        (first_param_ty.clone(), "self")
@@ -328,6 +321,9 @@ fn rename_to_self(sema: &Semantics<RootDatabase>, local: hir::Local) -> RenameRe
        bail!("Parameter type differs from impl block type");
    }
+    let InFile { file_id, value: param_source } =
+        first_param.source(sema.db).ok_or_else(|| format_err!("No source for parameter found"))?;
    let def = Definition::Local(local);
    let usages = def.usages(sema).all();
    let mut source_change = SourceChange::default();
@@ -336,25 +332,20 @@ fn rename_to_self(sema: &Semantics<RootDatabase>, local: hir::Local) -> RenameRe
    }));
    source_change.insert_source_edit(
        file_id.original_file(sema.db),
-        TextEdit::replace(first_param_range, String::from(self_param)),
+        TextEdit::replace(param_source.syntax().text_range(), String::from(self_param)),
    );
    Ok(source_change)
 }
 fn rename_self_to_param(
    sema: &Semantics<RootDatabase>,
    local: hir::Local,
+    self_param: hir::SelfParam,
    new_name: &str,
    identifier_kind: IdentifierKind,
 ) -> RenameResult<SourceChange> {
-    let (file_id, self_param) = match local.source(sema.db) {
+    let InFile { file_id, value: self_param } =
-        InFile { file_id, value: Either::Right(self_param) } => (file_id, self_param),
+        self_param.source(sema.db).ok_or_else(|| format_err!("cannot find function source"))?;
-        _ => {
-            never!(true, "rename_self_to_param invoked on a non-self local");
-            bail!("rename_self_to_param invoked on a non-self local");
-        }
-    };
    let def = Definition::Local(local);
    let usages = def.usages(sema).all();
@@ -710,7 +701,7 @@ foo!(Foo$0);",
    #[test]
    fn test_rename_for_local() {
-        cov_mark::check!(rename_ident);
+        cov_mark::check!(rename_local);
        check(
            "k",
            r#"
@@ -1251,6 +1242,7 @@ pub mod foo$0;
    #[test]
    fn test_enum_variant_from_module_1() {
+        cov_mark::check!(rename_non_local);
        check(
            "Baz",
            r#"
@@ -1361,7 +1353,7 @@ fn f(foo$0: &mut Foo) -> i32 {
    foo.i
 }
 "#,
-            "error: No impl block found for function",
+            "error: Cannot rename parameter to self for free function",
        );
        check(
            "self",
@@ -1391,7 +1383,7 @@ impl Foo {
    }
 }
 "#,
-            "error: Only the first parameter can be self",
+            "error: Only the first parameter may be renamed to self",
        );
    }
diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs
index bea020b06..5b488e2c5 100644
--- a/crates/ide/src/runnables.rs
+++ b/crates/ide/src/runnables.rs
@@ -7,17 +7,13 @@ use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, Semantics};
 use ide_assists::utils::test_related_attribute;
 use ide_db::{
    base_db::{FilePosition, FileRange},
-    defs::Definition,
    helpers::visit_file_defs,
    search::SearchScope,
    RootDatabase, SymbolKind,
 };
 use itertools::Itertools;
 use rustc_hash::FxHashSet;
-use syntax::{
+use syntax::ast::{self, AstNode, AttrsOwner};
-    ast::{self, AstNode, AttrsOwner},
-    match_ast, SyntaxNode,
-};
 use crate::{
    display::{ToNav, TryToNav},
@@ -271,28 +267,6 @@ pub(crate) fn runnable_mod(sema: &Semantics<RootDatabase>, def: hir::Module) ->
    Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg })
 }
-// FIXME: figure out a proper API here.
-pub(crate) fn doc_owner_to_def(
-    sema: &Semantics<RootDatabase>,
-    item: SyntaxNode,
-) -> Option<Definition> {
-    let res: hir::ModuleDef = match_ast! {
-        match item {
-            ast::SourceFile(_it) => sema.scope(&item).module()?.into(),
-            ast::Fn(it) => sema.to_def(&it)?.into(),
-            ast::Struct(it) => sema.to_def(&it)?.into(),
-            ast::Enum(it) => sema.to_def(&it)?.into(),
-            ast::Union(it) => sema.to_def(&it)?.into(),
-            ast::Trait(it) => sema.to_def(&it)?.into(),
-            ast::Const(it) => sema.to_def(&it)?.into(),
-            ast::Static(it) => sema.to_def(&it)?.into(),
-            ast::TypeAlias(it) => sema.to_def(&it)?.into(),
-            _ => return None,
-        }
-    };
-    Some(Definition::ModuleDef(res))
-}
 fn module_def_doctest(sema: &Semantics<RootDatabase>, def: hir::ModuleDef) -> Option<Runnable> {
    let attrs = match def {
        hir::ModuleDef::Module(it) => it.attrs(sema.db),
diff --git a/crates/ide/src/syntax_highlighting/inject.rs b/crates/ide/src/syntax_highlighting/inject.rs
index 8e0940184..38bf49348 100644
--- a/crates/ide/src/syntax_highlighting/inject.rs
+++ b/crates/ide/src/syntax_highlighting/inject.rs
@@ -190,10 +190,10 @@ pub(super) fn doc_comment(
                    intra_doc_links.extend(
                        extract_definitions_from_markdown(line)
                            .into_iter()
-                            .filter_map(|(link, ns, range)| {
+                            .filter_map(|(range, link, ns)| {
-                                validate_intra_doc_link(sema.db, &def, &link, ns).zip(Some(range))
+                                Some(range).zip(validate_intra_doc_link(sema.db, &def, &link, ns))
                            })
-                            .map(|(def, Range { start, end })| {
+                            .map(|(Range { start, end }, def)| {
                                (
                                    def,
                                    TextRange::at(
diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs
index bba6c08e0..1482d37f8 100644
--- a/crates/ide_assists/src/assist_context.rs
+++ b/crates/ide_assists/src/assist_context.rs
@@ -14,8 +14,8 @@ use ide_db::{
 };
 use syntax::{
    algo::{self, find_node_at_offset, SyntaxRewriter},
-    AstNode, AstToken, SourceFile, SyntaxElement, SyntaxKind, SyntaxToken, TextRange, TextSize,
+    AstNode, AstToken, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxNodePtr,
-    TokenAtOffset,
+    SyntaxToken, TextRange, TextSize, TokenAtOffset,
 };
 use text_edit::{TextEdit, TextEditBuilder};
@@ -180,11 +180,19 @@ pub(crate) struct AssistBuilder {
    edit: TextEditBuilder,
    file_id: FileId,
    source_change: SourceChange,
+    /// Maps the original, immutable `SyntaxNode` to a `clone_for_update` twin.
+    mutated_tree: Option<(SyntaxNode, SyntaxNode)>,
 }
 impl AssistBuilder {
    pub(crate) fn new(file_id: FileId) -> AssistBuilder {
-        AssistBuilder { edit: TextEdit::builder(), file_id, source_change: SourceChange::default() }
+        AssistBuilder {
+            edit: TextEdit::builder(),
+            file_id,
+            source_change: SourceChange::default(),
+            mutated_tree: None,
+        }
    }
    pub(crate) fn edit_file(&mut self, file_id: FileId) {
@@ -193,12 +201,42 @@ impl AssistBuilder {
    }
    fn commit(&mut self) {
+        if let Some((old, new)) = self.mutated_tree.take() {
+            algo::diff(&old, &new).into_text_edit(&mut self.edit)
+        }
        let edit = mem::take(&mut self.edit).finish();
        if !edit.is_empty() {
            self.source_change.insert_source_edit(self.file_id, edit);
        }
    }
+    pub(crate) fn make_ast_mut<N: AstNode>(&mut self, node: N) -> N {
+        N::cast(self.make_mut(node.syntax().clone())).unwrap()
+    }
+    /// Returns a copy of the `node`, suitable for mutation.
+    ///
+    /// Syntax trees in rust-analyzer are typically immutable, and mutating
+    /// operations panic at runtime. However, it is possible to make a copy of
+    /// the tree and mutate the copy freely. Mutation is based on interior
+    /// mutability, and different nodes in the same tree see the same mutations.
+    ///
+    /// The typical pattern for an assist is to find specific nodes in the read
+    /// phase, and then get their mutable couterparts using `make_mut` in the
+    /// mutable state.
+    pub(crate) fn make_mut(&mut self, node: SyntaxNode) -> SyntaxNode {
+        let root = &self
+            .mutated_tree
+            .get_or_insert_with(|| {
+                let immutable = node.ancestors().last().unwrap();
+                let mutable = immutable.clone_for_update();
+                (immutable, mutable)
+            })
+            .1;
+        let ptr = SyntaxNodePtr::new(&&node);
+        ptr.to_node(root)
+    }
    /// Remove specified `range` of text.
    pub(crate) fn delete(&mut self, range: TextRange) {
        self.edit.delete(range)
diff --git a/crates/ide_assists/src/handlers/add_explicit_type.rs b/crates/ide_assists/src/handlers/add_explicit_type.rs
index cb1548cef..62db31952 100644
--- a/crates/ide_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ide_assists/src/handlers/add_explicit_type.rs
@@ -34,26 +34,33 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Optio
    // The binding must have a name
    let name = pat.name()?;
    let name_range = name.syntax().text_range();
-    let stmt_range = let_stmt.syntax().text_range();
-    let eq_range = let_stmt.eq_token()?.text_range();
    // Assist should only be applicable if cursor is between 'let' and '='
-    let let_range = TextRange::new(stmt_range.start(), eq_range.start());
+    let cursor_in_range = {
-    let cursor_in_range = let_range.contains_range(ctx.frange.range);
+        let stmt_range = let_stmt.syntax().text_range();
+        let eq_range = let_stmt.eq_token()?.text_range();
+        let let_range = TextRange::new(stmt_range.start(), eq_range.start());
+        let_range.contains_range(ctx.frange.range)
+    };
    if !cursor_in_range {
+        cov_mark::hit!(add_explicit_type_not_applicable_if_cursor_after_equals);
        return None;
    }
    // Assist not applicable if the type has already been specified
    // and it has no placeholders
    let ascribed_ty = let_stmt.ty();
    if let Some(ty) = &ascribed_ty {
        if ty.syntax().descendants().find_map(ast::InferType::cast).is_none() {
+            cov_mark::hit!(add_explicit_type_not_applicable_if_ty_already_specified);
            return None;
        }
    }
    // Infer type
    let ty = ctx.sema.type_of_expr(&expr)?;
    if ty.contains_unknown() || ty.is_closure() {
+        cov_mark::hit!(add_explicit_type_not_applicable_if_ty_not_inferred);
        return None;
    }
@@ -81,17 +88,25 @@ mod tests {
    #[test]
    fn add_explicit_type_target() {
-        check_assist_target(add_explicit_type, "fn f() { let a$0 = 1; }", "a");
+        check_assist_target(add_explicit_type, r#"fn f() { let a$0 = 1; }"#, "a");
    }
    #[test]
    fn add_explicit_type_works_for_simple_expr() {
-        check_assist(add_explicit_type, "fn f() { let a$0 = 1; }", "fn f() { let a: i32 = 1; }");
+        check_assist(
+            add_explicit_type,
+            r#"fn f() { let a$0 = 1; }"#,
+            r#"fn f() { let a: i32 = 1; }"#,
+        );
    }
    #[test]
    fn add_explicit_type_works_for_underscore() {
-        check_assist(add_explicit_type, "fn f() { let a$0: _ = 1; }", "fn f() { let a: i32 = 1; }");
+        check_assist(
+            add_explicit_type,
+            r#"fn f() { let a$0: _ = 1; }"#,
+            r#"fn f() { let a: i32 = 1; }"#,
+        );
    }
    #[test]
@@ -99,23 +114,19 @@ mod tests {
        check_assist(
            add_explicit_type,
            r#"
-            enum Option<T> {
+enum Option<T> { Some(T), None }
-                Some(T),
-                None
-            }
-            fn f() {
+fn f() {
-                let a$0: Option<_> = Option::Some(1);
+    let a$0: Option<_> = Option::Some(1);
-            }"#,
+}
+"#,
            r#"
-            enum Option<T> {
+enum Option<T> { Some(T), None }
-                Some(T),
-                None
-            }
-            fn f() {
+fn f() {
-                let a: Option<i32> = Option::Some(1);
+    let a: Option<i32> = Option::Some(1);
-            }"#,
+}
+"#,
        );
    }
@@ -139,24 +150,30 @@ mod tests {
    #[test]
    fn add_explicit_type_not_applicable_if_ty_not_inferred() {
-        check_assist_not_applicable(add_explicit_type, "fn f() { let a$0 = None; }");
+        cov_mark::check!(add_explicit_type_not_applicable_if_ty_not_inferred);
+        check_assist_not_applicable(add_explicit_type, r#"fn f() { let a$0 = None; }"#);
    }
    #[test]
    fn add_explicit_type_not_applicable_if_ty_already_specified() {
-        check_assist_not_applicable(add_explicit_type, "fn f() { let a$0: i32 = 1; }");
+        cov_mark::check!(add_explicit_type_not_applicable_if_ty_already_specified);
+        check_assist_not_applicable(add_explicit_type, r#"fn f() { let a$0: i32 = 1; }"#);
    }
    #[test]
    fn add_explicit_type_not_applicable_if_specified_ty_is_tuple() {
-        check_assist_not_applicable(add_explicit_type, "fn f() { let a$0: (i32, i32) = (3, 4); }");
+        check_assist_not_applicable(
+            add_explicit_type,
+            r#"fn f() { let a$0: (i32, i32) = (3, 4); }"#,
+        );
    }
    #[test]
    fn add_explicit_type_not_applicable_if_cursor_after_equals() {
+        cov_mark::check!(add_explicit_type_not_applicable_if_cursor_after_equals);
        check_assist_not_applicable(
            add_explicit_type,
-            "fn f() {let a =$0 match 1 {2 => 3, 3 => 5};}",
+            r#"fn f() {let a =$0 match 1 {2 => 3, 3 => 5};}"#,
        )
    }
@@ -164,7 +181,7 @@ mod tests {
    fn add_explicit_type_not_applicable_if_cursor_before_let() {
        check_assist_not_applicable(
            add_explicit_type,
-            "fn f() $0{let a = match 1 {2 => 3, 3 => 5};}",
+            r#"fn f() $0{let a = match 1 {2 => 3, 3 => 5};}"#,
        )
    }
@@ -176,7 +193,8 @@ mod tests {
 fn main() {
    let multiply_by_two$0 = |i| i * 3;
    let six = multiply_by_two(2);
-}"#,
+}
+"#,
        )
    }
@@ -185,23 +203,19 @@ fn main() {
        check_assist(
            add_explicit_type,
            r#"
-struct Test<K, T = u8> {
+struct Test<K, T = u8> { k: K, t: T }
-    k: K,
-    t: T,
-}
 fn main() {
    let test$0 = Test { t: 23u8, k: 33 };
-}"#,
-            r#"
-struct Test<K, T = u8> {
-    k: K,
-    t: T,
 }
+"#,
+            r#"
+struct Test<K, T = u8> { k: K, t: T }
 fn main() {
    let test: Test<i32> = Test { t: 23u8, k: 33 };
-}"#,
+}
+"#,
        );
    }
 }
diff --git a/crates/ide_assists/src/handlers/add_lifetime_to_type.rs b/crates/ide_assists/src/handlers/add_lifetime_to_type.rs
index 844928754..7030d0f97 100644
--- a/crates/ide_assists/src/handlers/add_lifetime_to_type.rs
+++ b/crates/ide_assists/src/handlers/add_lifetime_to_type.rs
@@ -1,5 +1,4 @@
-use ast::FieldList;
+use syntax::ast::{self, AstNode, GenericParamsOwner, NameOwner};
-use syntax::ast::{self, AstNode, GenericParamsOwner, NameOwner, RefType, Type};
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -65,8 +64,8 @@ pub(crate) fn add_lifetime_to_type(acc: &mut Assists, ctx: &AssistContext) -> Op
    )
 }
-fn fetch_borrowed_types(node: &ast::Adt) -> Option<Vec<RefType>> {
+fn fetch_borrowed_types(node: &ast::Adt) -> Option<Vec<ast::RefType>> {
-    let ref_types: Vec<RefType> = match node {
+    let ref_types: Vec<ast::RefType> = match node {
        ast::Adt::Enum(enum_) => {
            let variant_list = enum_.variant_list()?;
            variant_list
@@ -88,7 +87,7 @@ fn fetch_borrowed_types(node: &ast::Adt) -> Option<Vec<RefType>> {
            record_field_list
                .fields()
                .filter_map(|r_field| {
-                    if let Type::RefType(ref_type) = r_field.ty()? {
+                    if let ast::Type::RefType(ref_type) = r_field.ty()? {
                        if ref_type.lifetime().is_none() {
                            return Some(ref_type);
                        }
@@ -107,12 +106,12 @@ fn fetch_borrowed_types(node: &ast::Adt) -> Option<Vec<RefType>> {
    }
 }
-fn find_ref_types_from_field_list(field_list: &FieldList) -> Option<Vec<RefType>> {
+fn find_ref_types_from_field_list(field_list: &ast::FieldList) -> Option<Vec<ast::RefType>> {
-    let ref_types: Vec<RefType> = match field_list {
+    let ref_types: Vec<ast::RefType> = match field_list {
        ast::FieldList::RecordFieldList(record_list) => record_list
            .fields()
            .filter_map(|f| {
-                if let Type::RefType(ref_type) = f.ty()? {
+                if let ast::Type::RefType(ref_type) = f.ty()? {
                    if ref_type.lifetime().is_none() {
                        return Some(ref_type);
                    }
@@ -124,7 +123,7 @@ fn find_ref_types_from_field_list(field_list: &FieldList) -> Option<Vec<RefType>
        ast::FieldList::TupleFieldList(tuple_field_list) => tuple_field_list
            .fields()
            .filter_map(|f| {
-                if let Type::RefType(ref_type) = f.ty()? {
+                if let ast::Type::RefType(ref_type) = f.ty()? {
                    if ref_type.lifetime().is_none() {
                        return Some(ref_type);
                    }
@@ -152,76 +151,79 @@ mod tests {
    fn add_lifetime_to_struct() {
        check_assist(
            add_lifetime_to_type,
-            "struct Foo { a: &$0i32 }",
+            r#"struct Foo { a: &$0i32 }"#,
-            "struct Foo<'a> { a: &'a i32 }",
+            r#"struct Foo<'a> { a: &'a i32 }"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "struct Foo { a: &$0i32, b: &usize }",
+            r#"struct Foo { a: &$0i32, b: &usize }"#,
-            "struct Foo<'a> { a: &'a i32, b: &'a usize }",
+            r#"struct Foo<'a> { a: &'a i32, b: &'a usize }"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "struct Foo { a: &$0i32, b: usize }",
+            r#"struct Foo { a: &$0i32, b: usize }"#,
-            "struct Foo<'a> { a: &'a i32, b: usize }",
+            r#"struct Foo<'a> { a: &'a i32, b: usize }"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "struct Foo<T> { a: &$0T, b: usize }",
+            r#"struct Foo<T> { a: &$0T, b: usize }"#,
-            "struct Foo<'a, T> { a: &'a T, b: usize }",
+            r#"struct Foo<'a, T> { a: &'a T, b: usize }"#,
        );
-        check_assist_not_applicable(add_lifetime_to_type, "struct Foo<'a> { a: &$0'a i32 }");
+        check_assist_not_applicable(add_lifetime_to_type, r#"struct Foo<'a> { a: &$0'a i32 }"#);
-        check_assist_not_applicable(add_lifetime_to_type, "struct Foo { a: &'a$0 i32 }");
+        check_assist_not_applicable(add_lifetime_to_type, r#"struct Foo { a: &'a$0 i32 }"#);
    }
    #[test]
    fn add_lifetime_to_enum() {
        check_assist(
            add_lifetime_to_type,
-            "enum Foo { Bar { a: i32 }, Other, Tuple(u32, &$0u32)}",
+            r#"enum Foo { Bar { a: i32 }, Other, Tuple(u32, &$0u32)}"#,
-            "enum Foo<'a> { Bar { a: i32 }, Other, Tuple(u32, &'a u32)}",
+            r#"enum Foo<'a> { Bar { a: i32 }, Other, Tuple(u32, &'a u32)}"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "enum Foo { Bar { a: &$0i32 }}",
+            r#"enum Foo { Bar { a: &$0i32 }}"#,
-            "enum Foo<'a> { Bar { a: &'a i32 }}",
+            r#"enum Foo<'a> { Bar { a: &'a i32 }}"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "enum Foo<T> { Bar { a: &$0i32, b: &T }}",
+            r#"enum Foo<T> { Bar { a: &$0i32, b: &T }}"#,
-            "enum Foo<'a, T> { Bar { a: &'a i32, b: &'a T }}",
+            r#"enum Foo<'a, T> { Bar { a: &'a i32, b: &'a T }}"#,
        );
-        check_assist_not_applicable(add_lifetime_to_type, "enum Foo<'a> { Bar { a: &$0'a i32 }}");
+        check_assist_not_applicable(
-        check_assist_not_applicable(add_lifetime_to_type, "enum Foo { Bar, $0Misc }");
+            add_lifetime_to_type,
+            r#"enum Foo<'a> { Bar { a: &$0'a i32 }}"#,
+        );
+        check_assist_not_applicable(add_lifetime_to_type, r#"enum Foo { Bar, $0Misc }"#);
    }
    #[test]
    fn add_lifetime_to_union() {
        check_assist(
            add_lifetime_to_type,
-            "union Foo { a: &$0i32 }",
+            r#"union Foo { a: &$0i32 }"#,
-            "union Foo<'a> { a: &'a i32 }",
+            r#"union Foo<'a> { a: &'a i32 }"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "union Foo { a: &$0i32, b: &usize }",
+            r#"union Foo { a: &$0i32, b: &usize }"#,
-            "union Foo<'a> { a: &'a i32, b: &'a usize }",
+            r#"union Foo<'a> { a: &'a i32, b: &'a usize }"#,
        );
        check_assist(
            add_lifetime_to_type,
-            "union Foo<T> { a: &$0T, b: usize }",
+            r#"union Foo<T> { a: &$0T, b: usize }"#,
-            "union Foo<'a, T> { a: &'a T, b: usize }",
+            r#"union Foo<'a, T> { a: &'a T, b: usize }"#,
        );
-        check_assist_not_applicable(add_lifetime_to_type, "struct Foo<'a> { a: &'a $0i32 }");
+        check_assist_not_applicable(add_lifetime_to_type, r#"struct Foo<'a> { a: &'a $0i32 }"#);
    }
 }
diff --git a/crates/ide_assists/src/handlers/add_missing_impl_members.rs b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
index 63cea754d..0148635f9 100644
--- a/crates/ide_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
@@ -3,9 +3,9 @@ use syntax::ast::{self, AstNode};
 use crate::{
    assist_context::{AssistContext, Assists},
-    utils::add_trait_assoc_items_to_impl,
+    utils::{
-    utils::DefaultMethods,
+        add_trait_assoc_items_to_impl, filter_assoc_items, render_snippet, Cursor, DefaultMethods,
-    utils::{filter_assoc_items, render_snippet, Cursor},
+    },
    AssistId, AssistKind,
 };
diff --git a/crates/ide_assists/src/handlers/convert_comment_block.rs b/crates/ide_assists/src/handlers/convert_comment_block.rs
index 9dc3ee28f..d202a85f9 100644
--- a/crates/ide_assists/src/handlers/convert_comment_block.rs
+++ b/crates/ide_assists/src/handlers/convert_comment_block.rs
@@ -1,13 +1,6 @@
 use itertools::Itertools;
 use syntax::{
-    ast::{
+    ast::{self, edit::IndentLevel, Comment, CommentKind, CommentShape, Whitespace},
-        self,
-        edit::IndentLevel,
-        Comment, CommentKind,
-        CommentPlacement::{Inner, Outer},
-        CommentShape::{self, Block, Line},
-        Whitespace,
-    },
    AstToken, Direction, SyntaxElement, TextRange,
 };
@@ -29,21 +22,18 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 ///   */
 /// ```
 pub(crate) fn convert_comment_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    if let Some(comment) = ctx.find_token_at_offset::<ast::Comment>() {
+    let comment = ctx.find_token_at_offset::<ast::Comment>()?;
-        // Only allow comments which are alone on their line
+    // Only allow comments which are alone on their line
-        if let Some(prev) = comment.syntax().prev_token() {
+    if let Some(prev) = comment.syntax().prev_token() {
-            if Whitespace::cast(prev).filter(|w| w.text().contains('\n')).is_none() {
+        if Whitespace::cast(prev).filter(|w| w.text().contains('\n')).is_none() {
-                return None;
+            return None;
-            }
        }
-        return match comment.kind().shape {
-            ast::CommentShape::Block => block_to_line(acc, comment),
-            ast::CommentShape::Line => line_to_block(acc, comment),
-        };
    }
-    return None;
+    match comment.kind().shape {
+        ast::CommentShape::Block => block_to_line(acc, comment),
+        ast::CommentShape::Line => line_to_block(acc, comment),
+    }
 }
 fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
@@ -55,8 +45,7 @@ fn block_to_line(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
        target,
        |edit| {
            let indentation = IndentLevel::from_token(comment.syntax());
-            let line_prefix =
+            let line_prefix = CommentKind { shape: CommentShape::Line, ..comment.kind() }.prefix();
-                comment_kind_prefix(CommentKind { shape: CommentShape::Line, ..comment.kind() });
            let text = comment.text();
            let text = &text[comment.prefix().len()..(text.len() - "*/".len())].trim();
@@ -105,7 +94,7 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
                comments.into_iter().map(|c| line_comment_text(indentation, c)).join("\n");
            let block_prefix =
-                comment_kind_prefix(CommentKind { shape: CommentShape::Block, ..comment.kind() });
+                CommentKind { shape: CommentShape::Block, ..comment.kind() }.prefix();
            let output =
                format!("{}\n{}\n{}*/", block_prefix, block_comment_body, indentation.to_string());
@@ -182,17 +171,6 @@ fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String {
    }
 }
-fn comment_kind_prefix(ck: ast::CommentKind) -> &'static str {
-    match (ck.shape, ck.doc) {
-        (Line, Some(Inner)) => "//!",
-        (Line, Some(Outer)) => "///",
-        (Line, None) => "//",
-        (Block, Some(Inner)) => "/*!",
-        (Block, Some(Outer)) => "/**",
-        (Block, None) => "/*",
-    }
-}
 #[cfg(test)]
 mod tests {
    use crate::tests::{check_assist, check_assist_not_applicable};
diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs
index cfc472a32..8e0794218 100644
--- a/crates/ide_assists/src/handlers/merge_imports.rs
+++ b/crates/ide_assists/src/handlers/merge_imports.rs
@@ -21,12 +21,6 @@ use crate::{
 // ```
 pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
    let tree: ast::UseTree = ctx.find_node_at_offset()?;
-    let original_parent = tree.syntax().ancestors().last()?;
-    let tree = tree.clone_for_update();
-    let new_parent = tree.syntax().ancestors().last()?;
-    let mut offset = ctx.offset();
    let mut imports = None;
    let mut uses = None;
@@ -53,22 +47,20 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()
        target,
        |builder| {
            if let Some((to_replace, replacement, to_remove)) = imports {
-                if to_remove.syntax().text_range().end() < offset {
+                let to_replace = builder.make_ast_mut(to_replace);
-                    offset -= to_remove.syntax().text_range().len();
+                let to_remove = builder.make_ast_mut(to_remove);
-                }
-                ted::replace(to_replace.syntax().clone(), replacement.syntax().clone());
+                ted::replace(to_replace.syntax(), replacement.syntax());
                to_remove.remove();
            }
            if let Some((to_replace, replacement, to_remove)) = uses {
-                if to_remove.syntax().text_range().end() < offset {
+                let to_replace = builder.make_ast_mut(to_replace);
-                    offset -= to_remove.syntax().text_range().len();
+                let to_remove = builder.make_ast_mut(to_remove);
-                }
-                ted::replace(to_replace.syntax().clone(), replacement.syntax().clone());
+                ted::replace(to_replace.syntax(), replacement.syntax());
                to_remove.remove()
            }
-            builder.replace(original_parent.text_range(), new_parent.to_string())
        },
    )
 }
diff --git a/crates/ide_assists/src/handlers/move_bounds.rs b/crates/ide_assists/src/handlers/move_bounds.rs
index b5dec8014..011a28d44 100644
--- a/crates/ide_assists/src/handlers/move_bounds.rs
+++ b/crates/ide_assists/src/handlers/move_bounds.rs
@@ -21,7 +21,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 // }
 // ```
 pub(crate) fn move_bounds_to_where_clause(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let type_param_list = ctx.find_node_at_offset::<ast::GenericParamList>()?.clone_for_update();
+    let type_param_list = ctx.find_node_at_offset::<ast::GenericParamList>()?;
    let mut type_params = type_param_list.type_params();
    if type_params.all(|p| p.type_bound_list().is_none()) {
@@ -29,7 +29,6 @@ pub(crate) fn move_bounds_to_where_clause(acc: &mut Assists, ctx: &AssistContext
    }
    let parent = type_param_list.syntax().parent()?;
-    let original_parent_range = parent.text_range();
    let target = type_param_list.syntax().text_range();
    acc.add(
@@ -37,6 +36,9 @@ pub(crate) fn move_bounds_to_where_clause(acc: &mut Assists, ctx: &AssistContext
        "Move to where clause",
        target,
        |edit| {
+            let type_param_list = edit.make_ast_mut(type_param_list);
+            let parent = edit.make_mut(parent);
            let where_clause: ast::WhereClause = match_ast! {
                match parent {
                    ast::Fn(it) => it.get_or_create_where_clause(),
@@ -56,8 +58,6 @@ pub(crate) fn move_bounds_to_where_clause(acc: &mut Assists, ctx: &AssistContext
                    tbl.remove()
                }
            }
-            edit.replace(original_parent_range, parent.to_string())
        },
    )
 }
diff --git a/crates/ide_assists/src/handlers/reorder_fields.rs b/crates/ide_assists/src/handlers/reorder_fields.rs
index 794c89323..383ca6c47 100644
--- a/crates/ide_assists/src/handlers/reorder_fields.rs
+++ b/crates/ide_assists/src/handlers/reorder_fields.rs
@@ -1,9 +1,6 @@
-use itertools::Itertools;
 use rustc_hash::FxHashMap;
-use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct};
+use syntax::{algo, ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode};
-use ide_db::RootDatabase;
-use syntax::{algo, ast, match_ast, AstNode, SyntaxKind, SyntaxKind::*, SyntaxNode};
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -23,26 +20,39 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 // ```
 //
 pub(crate) fn reorder_fields(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    reorder::<ast::RecordExpr>(acc, ctx).or_else(|| reorder::<ast::RecordPat>(acc, ctx))
+    let record = ctx
-}
+        .find_node_at_offset::<ast::RecordExpr>()
+        .map(|it| it.syntax().clone())
+        .or_else(|| ctx.find_node_at_offset::<ast::RecordPat>().map(|it| it.syntax().clone()))?;
-fn reorder<R: AstNode>(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let path = record.children().find_map(ast::Path::cast)?;
-    let record = ctx.find_node_at_offset::<R>()?;
-    let path = record.syntax().children().find_map(ast::Path::cast)?;
    let ranks = compute_fields_ranks(&path, &ctx)?;
-    let fields = get_fields(&record.syntax());
+    let fields: Vec<SyntaxNode> = {
-    let sorted_fields = sorted_by_rank(&fields, |node| {
+        let field_kind = match record.kind() {
-        *ranks.get(&get_field_name(node)).unwrap_or(&usize::max_value())
+            RECORD_EXPR => RECORD_EXPR_FIELD,
-    });
+            RECORD_PAT => RECORD_PAT_FIELD,
+            _ => {
+                stdx::never!();
+                return None;
+            }
+        };
+        record.children().flat_map(|n| n.children()).filter(|n| n.kind() == field_kind).collect()
+    };
+    let sorted_fields = {
+        let mut fields = fields.clone();
+        fields.sort_by_key(|node| *ranks.get(&get_field_name(node)).unwrap_or(&usize::max_value()));
+        fields
+    };
    if sorted_fields == fields {
        cov_mark::hit!(reorder_sorted_fields);
        return None;
    }
-    let target = record.syntax().text_range();
+    let target = record.text_range();
    acc.add(
        AssistId("reorder_fields", AssistKind::RefactorRewrite),
        "Reorder record fields",
@@ -57,14 +67,6 @@ fn reorder<R: AstNode>(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
    )
 }
-fn get_fields_kind(node: &SyntaxNode) -> Vec<SyntaxKind> {
-    match node.kind() {
-        RECORD_EXPR => vec![RECORD_EXPR_FIELD],
-        RECORD_PAT => vec![RECORD_PAT_FIELD, IDENT_PAT],
-        _ => vec![],
-    }
-}
 fn get_field_name(node: &SyntaxNode) -> String {
    let res = match_ast! {
        match node {
@@ -76,34 +78,20 @@ fn get_field_name(node: &SyntaxNode) -> String {
    res.unwrap_or_default()
 }
-fn get_fields(record: &SyntaxNode) -> Vec<SyntaxNode> {
+fn compute_fields_ranks(path: &ast::Path, ctx: &AssistContext) -> Option<FxHashMap<String, usize>> {
-    let kinds = get_fields_kind(record);
+    let strukt = match ctx.sema.resolve_path(path) {
-    record.children().flat_map(|n| n.children()).filter(|n| kinds.contains(&n.kind())).collect()
+        Some(hir::PathResolution::Def(hir::ModuleDef::Adt(hir::Adt::Struct(it)))) => it,
-}
+        _ => return None,
+    };
-fn sorted_by_rank(
-    fields: &[SyntaxNode],
-    get_rank: impl Fn(&SyntaxNode) -> usize,
-) -> Vec<SyntaxNode> {
-    fields.iter().cloned().sorted_by_key(get_rank).collect()
-}
-fn struct_definition(path: &ast::Path, sema: &Semantics<RootDatabase>) -> Option<Struct> {
+    let res = strukt
-    match sema.resolve_path(path) {
+        .fields(ctx.db())
-        Some(PathResolution::Def(ModuleDef::Adt(Adt::Struct(s)))) => Some(s),
+        .iter()
-        _ => None,
+        .enumerate()
-    }
+        .map(|(idx, field)| (field.name(ctx.db()).to_string(), idx))
-}
+        .collect();
-fn compute_fields_ranks(path: &ast::Path, ctx: &AssistContext) -> Option<FxHashMap<String, usize>> {
+    Some(res)
-    Some(
-        struct_definition(path, &ctx.sema)?
-            .fields(ctx.db())
-            .iter()
-            .enumerate()
-            .map(|(idx, field)| (field.name(ctx.db()).to_string(), idx))
-            .collect(),
-    )
 }
 #[cfg(test)]
@@ -118,11 +106,7 @@ mod tests {
        check_assist_not_applicable(
            reorder_fields,
            r#"
-struct Foo {
+struct Foo { foo: i32, bar: i32 }
-    foo: i32,
-    bar: i32,
-}
 const test: Foo = $0Foo { foo: 0, bar: 0 };
 "#,
        )
@@ -133,8 +117,8 @@ const test: Foo = $0Foo { foo: 0, bar: 0 };
        check_assist_not_applicable(
            reorder_fields,
            r#"
-struct Foo {};
+struct Foo {}
-const test: Foo = $0Foo {}
+const test: Foo = $0Foo {};
 "#,
        )
    }
@@ -144,12 +128,12 @@ const test: Foo = $0Foo {}
        check_assist(
            reorder_fields,
            r#"
-struct Foo {foo: i32, bar: i32};
+struct Foo { foo: i32, bar: i32 }
-const test: Foo = $0Foo {bar: 0, foo: 1}
+const test: Foo = $0Foo { bar: 0, foo: 1 };
 "#,
            r#"
-struct Foo {foo: i32, bar: i32};
+struct Foo { foo: i32, bar: i32 }
-const test: Foo = Foo {foo: 1, bar: 0}
+const test: Foo = Foo { foo: 1, bar: 0 };
 "#,
        )
    }
@@ -186,10 +170,7 @@ fn f(f: Foo) -> {
        check_assist(
            reorder_fields,
            r#"
-struct Foo {
+struct Foo { foo: String, bar: String }
-    foo: String,
-    bar: String,
-}
 impl Foo {
    fn new() -> Foo {
@@ -203,10 +184,7 @@ impl Foo {
 }
 "#,
            r#"
-struct Foo {
+struct Foo { foo: String, bar: String }
-    foo: String,
-    bar: String,
-}
 impl Foo {
    fn new() -> Foo {
diff --git a/crates/proc_macro_api/src/msg.rs b/crates/proc_macro_api/src/msg.rs
index 970f165ed..f525df152 100644
--- a/crates/proc_macro_api/src/msg.rs
+++ b/crates/proc_macro_api/src/msg.rs
@@ -55,8 +55,8 @@ pub enum ErrorCode {
 }
 pub trait Message: Serialize + DeserializeOwned {
-    fn read(inp: &mut impl BufRead) -> io::Result<Option<Self>> {
+    fn read(inp: &mut impl BufRead, buf: &mut String) -> io::Result<Option<Self>> {
-        Ok(match read_json(inp)? {
+        Ok(match read_json(inp, buf)? {
            None => None,
            Some(text) => {
                let mut deserializer = serde_json::Deserializer::from_str(&text);
@@ -76,14 +76,29 @@ pub trait Message: Serialize + DeserializeOwned {
 impl Message for Request {}
 impl Message for Response {}
-fn read_json(inp: &mut impl BufRead) -> io::Result<Option<String>> {
+fn read_json<'a>(
-    let mut buf = String::new();
+    inp: &mut impl BufRead,
-    inp.read_line(&mut buf)?;
+    mut buf: &'a mut String,
-    buf.pop(); // Remove trailing '\n'
+) -> io::Result<Option<&'a String>> {
-    Ok(match buf.len() {
+    loop {
-        0 => None,
+        buf.clear();
-        _ => Some(buf),
-    })
+        inp.read_line(&mut buf)?;
+        buf.pop(); // Remove trailing '\n'
+        if buf.is_empty() {
+            return Ok(None);
+        }
+        // Some ill behaved macro try to use stdout for debugging
+        // We ignore it here
+        if !buf.starts_with("{") {
+            log::error!("proc-macro tried to print : {}", buf);
+            continue;
+        }
+        return Ok(Some(buf));
+    }
 }
 fn write_json(out: &mut impl Write, msg: &str) -> io::Result<()> {
diff --git a/crates/proc_macro_api/src/process.rs b/crates/proc_macro_api/src/process.rs
index 30bb1b687..99d05aef3 100644
--- a/crates/proc_macro_api/src/process.rs
+++ b/crates/proc_macro_api/src/process.rs
@@ -90,8 +90,10 @@ impl ProcMacroProcessSrv {
 fn client_loop(task_rx: Receiver<Task>, mut process: Process) {
    let (mut stdin, mut stdout) = process.stdio().expect("couldn't access child stdio");
+    let mut buf = String::new();
    for Task { req, result_tx } in task_rx {
-        match send_request(&mut stdin, &mut stdout, req) {
+        match send_request(&mut stdin, &mut stdout, req, &mut buf) {
            Ok(res) => result_tx.send(res).unwrap(),
            Err(err) => {
                log::error!(
@@ -152,7 +154,8 @@ fn send_request(
    mut writer: &mut impl Write,
    mut reader: &mut impl BufRead,
    req: Request,
+    buf: &mut String,
 ) -> io::Result<Option<Response>> {
    req.write(&mut writer)?;
-    Response::read(&mut reader)
+    Response::read(&mut reader, buf)
 }
diff --git a/crates/proc_macro_api/src/rpc.rs b/crates/proc_macro_api/src/rpc.rs
index 9a68e2cc5..8f7270afe 100644
--- a/crates/proc_macro_api/src/rpc.rs
+++ b/crates/proc_macro_api/src/rpc.rs
@@ -77,7 +77,11 @@ struct TokenIdDef(u32);
 #[derive(Serialize, Deserialize)]
 #[serde(remote = "Delimiter")]
 struct DelimiterDef {
-    #[serde(with = "TokenIdDef")]
+    #[serde(
+        with = "TokenIdDef",
+        default = "tt::TokenId::unspecified",
+        skip_serializing_if = "token_id_def::skip_if"
+    )]
    id: TokenId,
    #[serde(with = "DelimiterKindDef")]
    kind: DelimiterKind,
@@ -116,7 +120,11 @@ enum LeafDef {
 #[serde(remote = "Literal")]
 struct LiteralDef {
    text: SmolStr,
-    #[serde(with = "TokenIdDef")]
+    #[serde(
+        with = "TokenIdDef",
+        default = "tt::TokenId::unspecified",
+        skip_serializing_if = "token_id_def::skip_if"
+    )]
    id: TokenId,
 }
@@ -126,7 +134,11 @@ struct PunctDef {
    char: char,
    #[serde(with = "SpacingDef")]
    spacing: Spacing,
-    #[serde(with = "TokenIdDef")]
+    #[serde(
+        with = "TokenIdDef",
+        default = "tt::TokenId::unspecified",
+        skip_serializing_if = "token_id_def::skip_if"
+    )]
    id: TokenId,
 }
@@ -141,10 +153,20 @@ enum SpacingDef {
 #[serde(remote = "Ident")]
 struct IdentDef {
    text: SmolStr,
-    #[serde(with = "TokenIdDef")]
+    #[serde(
+        with = "TokenIdDef",
+        default = "tt::TokenId::unspecified",
+        skip_serializing_if = "token_id_def::skip_if"
+    )]
    id: TokenId,
 }
+mod token_id_def {
+    pub(super) fn skip_if(value: &tt::TokenId) -> bool {
+        *value == tt::TokenId::unspecified()
+    }
+}
 mod opt_delimiter_def {
    use super::{Delimiter, DelimiterDef};
    use serde::{Deserialize, Deserializer, Serialize, Serializer};
diff --git a/crates/proc_macro_srv/src/cli.rs b/crates/proc_macro_srv/src/cli.rs
index d428b9567..bc48f1c43 100644
--- a/crates/proc_macro_srv/src/cli.rs
+++ b/crates/proc_macro_srv/src/cli.rs
@@ -6,8 +6,9 @@ use std::io;
 pub fn run() -> io::Result<()> {
    let mut srv = ProcMacroSrv::default();
+    let mut buf = String::new();
-    while let Some(req) = read_request()? {
+    while let Some(req) = read_request(&mut buf)? {
        let res = match req {
            msg::Request::ListMacro(task) => srv.list_macros(&task).map(msg::Response::ListMacro),
            msg::Request::ExpansionMacro(task) => {
@@ -30,8 +31,8 @@ pub fn run() -> io::Result<()> {
    Ok(())
 }
-fn read_request() -> io::Result<Option<msg::Request>> {
+fn read_request(buf: &mut String) -> io::Result<Option<msg::Request>> {
-    msg::Request::read(&mut io::stdin().lock())
+    msg::Request::read(&mut io::stdin().lock(), buf)
 }
 fn write_response(msg: msg::Response) -> io::Result<()> {
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 5c88c3a9b..cda272fd4 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -131,8 +131,8 @@ config_data! {
        /// Whether to show inlay type hints for method chains.
        inlayHints_chainingHints: bool      = "true",
-        /// Maximum length for inlay hints. Default is unlimited.
+        /// Maximum length for inlay hints. Set to null to have an unlimited length.
-        inlayHints_maxLength: Option<usize> = "null",
+        inlayHints_maxLength: Option<usize> = "25",
        /// Whether to show function parameter name inlay hints at the call
        /// site.
        inlayHints_parameterHints: bool     = "true",
diff --git a/crates/rust-analyzer/tests/rust-analyzer/main.rs b/crates/rust-analyzer/tests/rust-analyzer/main.rs
index 19516de7b..4442cbff6 100644
--- a/crates/rust-analyzer/tests/rust-analyzer/main.rs
+++ b/crates/rust-analyzer/tests/rust-analyzer/main.rs
@@ -712,6 +712,10 @@ pub fn foo(_input: TokenStream) -> TokenStream {
    // We hard code the output here for preventing to use any deps
    let mut res = TokenStream::new();
+    // ill behaved proc-macro will use the stdout
+    // we should ignore it
+    println!("I am bad guy");
    // impl Bar for Foo { fn bar() {} }
    let mut tokens = vec![t!("impl"), t!("Bar"), t!("for"), t!("Foo")];
    let mut fn_stream = TokenStream::new();
diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs
index 090282d28..29d25a58a 100644
--- a/crates/syntax/src/ast/token_ext.rs
+++ b/crates/syntax/src/ast/token_ext.rs
@@ -102,8 +102,9 @@ impl CommentKind {
        kind
    }
-    fn prefix(&self) -> &'static str {
+    pub fn prefix(&self) -> &'static str {
-        let &(prefix, _) = CommentKind::BY_PREFIX.iter().find(|(_, kind)| kind == self).unwrap();
+        let &(prefix, _) =
+            CommentKind::BY_PREFIX.iter().rev().find(|(_, kind)| kind == self).unwrap();
        prefix
    }
 }