8 files changed, 335 insertions, 297 deletions

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)> {
-                Some((node.name()?.syntax().text_range().start(), node.syntax().text_range()))
+            fn range_and_position_of<T: NameOwner>(
+                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 {
-            let token = file.token_at_offset(r_range.start()).next()?;
+        for (relative_range, token) in references
+            .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 = sema.to_def(&fn_)?;
+                let def = ast::Fn::cast(node).and_then(|fn_| 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 resolved = 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(_) => 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 (link, ns) = parse_intra_doc_link(link);
+    let resolved = resolve_doc_path_for_def(db, def, link, ns)?;
     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(field_or_assoc_item) = items.iter().find_map(|assoc_item| {
+        if let Some(assoc_item) = t.items(db).into_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();
-    let new_title = strip_prefixes_suffixes(title);
-    Some((new_target, new_title.to_string()))
+    Some((new_url.into_string(), strip_prefixes_suffixes(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>) {
-    let path = strip_prefixes_suffixes(s);
-    let ns = ns_from_intra_spec(s);
-    (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]) =
+const TYPES: ([&str; 9], [&str; 0]) =
+    (["type", "struct", "enum", "mod", "trait", "union", "module", "prim", "primitive"], []);
+const 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`
-/// * `panic!` -> `Namespace::Macros`
-/// * `fn@from_intra_spec` -> `Namespace::Values`
-fn ns_from_intra_spec(s: &str) -> Option<hir::Namespace> {
+/// * `struct MyStruct` -> ("MyStruct", `Namespace::Types`)
+/// * `panic!` -> ("panic", `Namespace::Macros`)
+/// * `fn@from_intra_spec` -> ("from_intra_spec", `Namespace::Values`)
+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))| {
-        prefixes
-            .clone()
-            .map(|prefix| {
-                s.starts_with(*prefix)
-                    && s.chars()
-                        .nth(prefix.len() + 1)
-                        .map(|c| c == '@' || c == ' ')
-                        .unwrap_or(false)
-            })
-            .any(|cond| cond)
-            || suffixes
-                .clone()
-                .map(|suffix| {
-                    s.starts_with(*suffix)
-                        && s.chars()
-                            .nth(suffix.len() + 1)
-                            .map(|c| c == '@' || c == ' ')
-                            .unwrap_or(false)
-                })
-                .any(|cond| cond)
+    .cloned()
+    .find_map(|(ns, (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..], ns))
+        } else {
+            suffixes.find_map(|&suffix| s.strip_suffix(suffix).zip(Some(ns)))
+        }
+    })
+    .map_or((s, None), |(s, ns)| (s, Some(ns)))
+}
+
+fn strip_prefixes_suffixes(s: &str) -> &str {
+    [
+        (TYPES.0.iter(), TYPES.1.iter()),
+        (VALUES.0.iter(), VALUES.1.iter()),
+        (MACROS.0.iter(), MACROS.1.iter()),
+    ]
+    .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)
-    .next()
+    .unwrap_or(s)
 }
 
 /// 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::{
-    ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, TextRange, TextSize,
-    TokenAtOffset, T,
-};
+use syntax::{ast, match_ast, AstNode, AstToken, SyntaxKind::*, SyntaxToken, 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::Lifetime(lifetime) => NameClass::classify_lifetime(&sema, &lifetime)
-                .map_or_else(|| NameRefClass::classify_lifetime(&sema, &lifetime).map(|d| d.referenced(sema.db)), |d| d.defined(sema.db)),
-            _ => None,
+            ast::NameRef(name_ref) => {
+                NameRefClass::classify(&sema, &name_ref).map(|d| d.referenced(db))
+            },
+            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!(
-        def,
+    if matches!(
+        def, // is target a lifetime?
         Definition::GenericParam(hir::GenericParam::LifetimeParam(_)) | Definition::Label(_)
-    );
-    match (ident_kind, def) {
-        (IdentifierKind::ToSelf, _)
-        | (IdentifierKind::Underscore, _)
-        | (IdentifierKind::Ident, _)
-            if def_is_lbl_or_lt =>
-        {
-            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);
+    ) {
+        match ident_kind {
+            IdentifierKind::Ident | IdentifierKind::ToSelf | IdentifierKind::Underscore => {
+                cov_mark::hit!(rename_not_a_lifetime_ident_ref);
+                bail!("Invalid name `{}`: not a lifetime identifier", new_name);
+            }
+            IdentifierKind::Lifetime => cov_mark::hit!(rename_lifetime),
         }
-        (IdentifierKind::ToSelf, _) => bail!("Invalid name `{}`: not an identifier", new_name),
-        (IdentifierKind::Ident, _) | (IdentifierKind::Underscore, _) => {
-            cov_mark::hit!(rename_ident)
+    } else {
+        match (ident_kind, def) {
+            (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
-    // 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() {
+    if let Some(_) = fn_def.self_param(sema.db) {
         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) {
-        InFile { file_id, value: Either::Right(self_param) } => (file_id, self_param),
-        _ => {
-            never!(true, "rename_self_to_param invoked on a non-self local");
-            bail!("rename_self_to_param invoked on a non-self local");
-        }
-    };
+    let InFile { file_id, value: self_param } =
+        self_param.source(sema.db).ok_or_else(|| format_err!("cannot find function source"))?;
 
     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::{
-    ast::{self, AstNode, AttrsOwner},
-    match_ast, SyntaxNode,
-};
+use syntax::ast::{self, AstNode, AttrsOwner};
 
 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)| {
-                                validate_intra_doc_link(sema.db, &def, &link, ns).zip(Some(range))
+                            .filter_map(|(range, link, ns)| {
+                                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(