From 9df78ec4a4e41ca94b25f292aba90e266f104f02 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 29 Mar 2021 21:23:45 +0200 Subject: Properly resolve intra doc links in hover and goto_definition --- crates/ide/src/doc_links.rs | 68 ++++++++++++++++++++++----------------- crates/ide/src/goto_definition.rs | 24 +++++++++++++- crates/ide/src/hover.rs | 27 +++++++++++----- crates/ide_db/src/defs.rs | 23 +++++++++++++ 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, String, Option)> { - 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, String, Option)> + '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)> { - 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 { + 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)] -- cgit v1.2.3