Properly resolve intra doc links in hover and goto_definition

4 files changed, 103 insertions, 39 deletions

diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs
index 99276168f..69442278b 100644
--- a/crates/ide/src/doc_links.rs
+++ b/crates/ide/src/doc_links.rs
@@ -26,12 +26,7 @@ pub(crate) type DocumentationLink = String;
 
 /// Rewrite documentation links in markdown to point to an online host (e.g. docs.rs)
 pub(crate) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: &Definition) -> String {
-    let mut cb = |link: BrokenLink| {
-        Some((
-            /*url*/ link.reference.to_owned().into(),
-            /*title*/ link.reference.to_owned().into(),
-        ))
-    };
+    let mut cb = broken_link_clone_cb;
     let doc = Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(&mut cb));
 
     let doc = map_links(doc, |target, title: &str| {
@@ -124,24 +119,24 @@ pub(crate) fn external_docs(
 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
+    extract_definitions_from_markdown_(markdown, &mut broken_link_clone_cb).collect()
+}
+
+fn extract_definitions_from_markdown_<'a>(
+    markdown: &'a str,
+    cb: &'a mut dyn FnMut(BrokenLink<'_>) -> Option<(CowStr<'a>, CowStr<'a>)>,
+) -> impl Iterator<Item = (Range<usize>, String, Option<hir::Namespace>)> + 'a {
+    Parser::new_with_broken_link_callback(markdown, Options::empty(), Some(cb))
+        .into_offset_iter()
+        .filter_map(|(event, range)| {
+            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);
+                Some((range, link.to_string(), ns))
+            } else {
+                None
+            }
+        })
 }
 
 /// Extracts a link from a comment at the given position returning the spanning range, link and
@@ -149,20 +144,24 @@ pub(crate) fn extract_definitions_from_markdown(
 pub(crate) fn extract_positioned_link_from_comment(
     position: TextSize,
     comment: &ast::Comment,
+    docs: hir::Documentation,
 ) -> Option<(TextRange, String, Option<hir::Namespace>)> {
-    let doc_comment = comment.doc_comment()?;
+    let doc_comment = comment.doc_comment()?.to_string() + "\n" + docs.as_str();
     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 len = comment.syntax().text_range().len().into();
+    let mut cb = broken_link_clone_cb;
+    // because pulldown_cmarks lifetimes are wrong we gotta dance around a few temporaries here
+    let res = extract_definitions_from_markdown_(&doc_comment, &mut cb)
+        .take_while(|&(Range { end, .. }, ..)| end < len)
+        .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))
+        });
+    res
 }
 
 /// Turns a syntax node into it's [`Definition`] if it can hold docs.
@@ -220,6 +219,15 @@ pub(crate) fn resolve_doc_path_for_def(
     }
 }
 
+fn broken_link_clone_cb<'a, 'b>(link: BrokenLink<'a>) -> Option<(CowStr<'b>, CowStr<'b>)> {
+    // 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(),
+    ))
+}
+
 // FIXME:
 // BUG: For Option::Some
 // Returns https://doc.rust-lang.org/nightly/core/prelude/v1/enum.Option.html#variant.Some
diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index c6556c487..4e4d1b200 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -31,7 +31,8 @@ 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 (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment)?;
+        let docs = doc_owner_to_def(&sema, &parent)?.docs(db)?;
+        let (_, link, ns) = extract_positioned_link_from_comment(position.offset, &comment, docs)?;
         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]));
@@ -1158,4 +1159,25 @@ fn fn_macro() {}
             "#,
         )
     }
+
+    #[test]
+    fn goto_intra_doc_links() {
+        check(
+            r#"
+
+pub mod theitem {
+    /// This is the item. Cool!
+    pub struct TheItem;
+             //^^^^^^^
+}
+
+/// Gives you a [`TheItem$0`].
+///
+/// [`TheItem`]: theitem::TheItem
+pub fn gimme() -> theitem::TheItem {
+    theitem::TheItem
+}
+"#,
+        );
+    }
 }
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 02a1a5b37..5a497e92d 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -115,10 +115,11 @@ pub(crate) fn hover(
 
             _ => ast::Comment::cast(token.clone())
                 .and_then(|comment| {
+                    let def = doc_owner_to_def(&sema, &node)?;
+                    let docs = def.docs(db)?;
                     let (idl_range, link, ns) =
-                        extract_positioned_link_from_comment(position.offset, &comment)?;
+                        extract_positioned_link_from_comment(position.offset, &comment, docs)?;
                     range = Some(idl_range);
-                    let def = doc_owner_to_def(&sema, &node)?;
                     resolve_doc_path_for_def(db, def, &link, ns)
                 })
                 .map(Definition::ModuleDef),
@@ -3812,23 +3813,33 @@ fn main() {
     fn hover_intra_doc_links() {
         check(
             r#"
-/// This is the [`foo`](foo$0) function.
-fn foo() {}
+
+pub mod theitem {
+    /// This is the item. Cool!
+    pub struct TheItem;
+}
+
+/// Gives you a [`TheItem$0`].
+///
+/// [`TheItem`]: theitem::TheItem
+pub fn gimme() -> theitem::TheItem {
+    theitem::TheItem
+}
 "#,
             expect![[r#"
-                *[`foo`](foo)*
+                *[`TheItem`]*
 
                 ```rust
-                test
+                test::theitem
                 ```
 
                 ```rust
-                fn foo()
+                pub struct TheItem
                 ```
 
                 ---
 
-                This is the [`foo`](https://docs.rs/test/*/test/fn.foo.html) function.
+                This is the item. Cool!
             "#]],
         );
     }
diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs
index de0dc2a40..378bac7b2 100644
--- a/crates/ide_db/src/defs.rs
+++ b/crates/ide_db/src/defs.rs
@@ -79,6 +79,29 @@ impl Definition {
         };
         Some(name)
     }
+
+    pub fn docs(&self, db: &RootDatabase) -> Option<hir::Documentation> {
+        match self {
+            Definition::Macro(it) => it.docs(db),
+            Definition::Field(it) => it.docs(db),
+            Definition::ModuleDef(def) => match def {
+                hir::ModuleDef::Module(it) => it.docs(db),
+                hir::ModuleDef::Function(it) => it.docs(db),
+                hir::ModuleDef::Adt(def) => match def {
+                    hir::Adt::Struct(it) => it.docs(db),
+                    hir::Adt::Union(it) => it.docs(db),
+                    hir::Adt::Enum(it) => it.docs(db),
+                },
+                hir::ModuleDef::Variant(it) => it.docs(db),
+                hir::ModuleDef::Const(it) => it.docs(db),
+                hir::ModuleDef::Static(it) => it.docs(db),
+                hir::ModuleDef::Trait(it) => it.docs(db),
+                hir::ModuleDef::TypeAlias(it) => it.docs(db),
+                hir::ModuleDef::BuiltinType(_) => None,
+            },
+            _ => None,
+        }
+    }
 }
 
 #[derive(Debug)]