diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index ec16b4bbf8..0799f98b13 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -303,6 +303,13 @@ impl ModuleDef { Some(segments.into_iter().join("::")) } + pub fn canonical_module_path( + &self, + db: &dyn HirDatabase, + ) -> Option> { + self.module(db).map(|it| it.path_to_root(db).into_iter().rev()) + } + pub fn name(self, db: &dyn HirDatabase) -> Option { match self { ModuleDef::Adt(it) => Some(it.name(db)), diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index 8c411c6906..f8887e8fcd 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -1,21 +1,16 @@ //! Extracts, resolves and rewrites links and intra-doc links in markdown documentation. -use std::{ - convert::{TryFrom, TryInto}, - iter::once, -}; - mod intra_doc_links; -use itertools::Itertools; +use std::convert::{TryFrom, TryInto}; + use pulldown_cmark::{BrokenLink, CowStr, Event, InlineStr, LinkType, Options, Parser, Tag}; use pulldown_cmark_to_cmark::{cmark_with_options, Options as CmarkOptions}; use stdx::format_to; use url::Url; use hir::{ - db::{DefDatabase, HirDatabase}, - Adt, AsAssocItem, AssocItem, AssocItemContainer, Crate, Field, HasAttrs, ItemInNs, ModuleDef, + db::HirDatabase, Adt, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasAttrs, ModuleDef, }; use ide_db::{ defs::{Definition, NameClass, NameRefClass}, @@ -60,9 +55,13 @@ pub(crate) fn rewrite_links(db: &RootDatabase, markdown: &str, definition: Defin } }); let mut out = String::new(); - let mut options = CmarkOptions::default(); - options.code_block_backticks = 3; - cmark_with_options(doc, &mut out, None, options).ok(); + cmark_with_options( + doc, + &mut out, + None, + CmarkOptions { code_block_backticks: 3, ..Default::default() }, + ) + .ok(); out } @@ -94,9 +93,13 @@ pub(crate) fn remove_links(markdown: &str) -> String { }); let mut out = String::new(); - let mut options = CmarkOptions::default(); - options.code_block_backticks = 3; - cmark_with_options(doc, &mut out, None, options).ok(); + cmark_with_options( + doc, + &mut out, + None, + CmarkOptions { code_block_backticks: 3, ..Default::default() }, + ) + .ok(); out } @@ -105,10 +108,10 @@ pub(crate) fn external_docs( db: &RootDatabase, position: &FilePosition, ) -> Option { - let sema = Semantics::new(db); + let sema = &Semantics::new(db); let file = sema.parse(position.file_id).syntax().clone(); let token = pick_best_token(file.token_at_offset(position.offset), |kind| match kind { - IDENT | INT_NUMBER => 3, + IDENT | INT_NUMBER | T![self] => 3, T!['('] | T![')'] => 2, kind if kind.is_trivia() => 0, _ => 1, @@ -118,13 +121,13 @@ pub(crate) fn external_docs( let node = token.parent()?; let definition = match_ast! { match node { - ast::NameRef(name_ref) => match NameRefClass::classify(&sema, &name_ref)? { + ast::NameRef(name_ref) => match NameRefClass::classify(sema, &name_ref)? { NameRefClass::Definition(def) => def, NameRefClass::FieldShorthand { local_ref: _, field_ref } => { Definition::Field(field_ref) } }, - ast::Name(name) => match NameClass::classify(&sema, &name)? { + ast::Name(name) => match NameClass::classify(sema, &name)? { NameClass::Definition(it) | NameClass::ConstReference(it) => it, NameClass::PatFieldShorthand { local_def: _, field_ref } => Definition::Field(field_ref), }, @@ -135,7 +138,8 @@ pub(crate) fn external_docs( get_doc_link(db, definition) } -/// Extracts all links from a given markdown text. +/// Extracts all links from a given markdown text returning the definition text range, link-text +/// and the namespace if known. pub(crate) fn extract_definitions_from_docs( docs: &hir::Documentation, ) -> Vec<(TextRange, String, Option)> { @@ -145,18 +149,16 @@ pub(crate) fn extract_definitions_from_docs( Some(&mut broken_link_clone_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); + .filter_map(|(event, range)| match event { + Event::Start(Tag::Link(_, target, _)) => { + let (link, ns) = parse_intra_doc_link(&target); Some(( TextRange::new(range.start.try_into().ok()?, range.end.try_into().ok()?), link.to_string(), ns, )) - } else { - None } + _ => None, }) .collect() } @@ -232,83 +234,59 @@ fn broken_link_clone_cb<'a, 'b>(link: BrokenLink<'a>) -> Option<(CowStr<'b>, Cow // This should cease to be a problem if RFC2988 (Stable Rustdoc URLs) is implemented // https://github.com/rust-lang/rfcs/pull/2988 fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { - // Get the outermost definition for the module def. This is used to resolve the public path to the type, - // then we can join the method, field, etc onto it if required. - let target_def: ModuleDef = match definition { - Definition::ModuleDef(def) => match def { - ModuleDef::Function(f) => f - .as_assoc_item(db) - .and_then(|assoc| match assoc.container(db) { - AssocItemContainer::Trait(t) => Some(t.into()), - AssocItemContainer::Impl(impl_) => { - impl_.self_ty(db).as_adt().map(|adt| adt.into()) - } - }) - .unwrap_or_else(|| def), - def => def, - }, - Definition::Field(f) => f.parent_def(db).into(), - // FIXME: Handle macros - _ => return None, + let (target, frag) = match definition { + Definition::ModuleDef(def) => { + if let Some(assoc_item) = def.as_assoc_item(db) { + let def = match assoc_item.container(db) { + AssocItemContainer::Trait(t) => t.into(), + AssocItemContainer::Impl(i) => i.self_ty(db).as_adt()?.into(), + }; + let frag = get_assoc_item_fragment(db, assoc_item)?; + (def, Some(frag)) + } else { + (def, None) + } + } + Definition::Field(field) => { + let def = match field.parent_def(db) { + hir::VariantDef::Struct(it) => it.into(), + hir::VariantDef::Union(it) => it.into(), + hir::VariantDef::Variant(it) => it.into(), + }; + (def, Some(format!("structfield.{}", field.name(db)))) + } + Definition::Macro(_) => todo!(), + Definition::SelfType(_) => todo!(), + Definition::Local(_) | Definition::GenericParam(_) | Definition::Label(_) => return None, }; - let ns = ItemInNs::from(target_def); - let krate = match definition { // Definition::module gives back the parent module, we don't want that as it fails for root modules Definition::ModuleDef(ModuleDef::Module(module)) => module.krate(), _ => definition.module(db)?.krate(), }; - // FIXME: using import map doesn't make sense here. What we want here is - // canonical path. What import map returns is the shortest path suitable for - // import. See this test: - cov_mark::hit!(test_reexport_order); - let import_map = db.import_map(krate.into()); let mut base = krate.display_name(db)?.to_string(); - let is_root_module = matches!( + let is_non_root_module = !matches!( definition, Definition::ModuleDef(ModuleDef::Module(module)) if krate.root_module(db) == module ); - if !is_root_module { - base = once(base) - .chain(import_map.path_of(ns)?.segments.iter().map(|name| name.to_string())) - .join("/"); + if is_non_root_module { + target + .canonical_module_path(db)? + .flat_map(|it| it.name(db)) + .for_each(|name| format_to!(base, "/{}", name)); } base += "/"; - let filename = get_symbol_filename(db, &target_def); - let fragment = match definition { - Definition::ModuleDef(def) => match def { - ModuleDef::Function(f) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Function(f))) - } - ModuleDef::Const(c) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::Const(c))) - } - ModuleDef::TypeAlias(ty) => { - get_symbol_fragment(db, &FieldOrAssocItem::AssocItem(AssocItem::TypeAlias(ty))) - } - _ => None, - }, - Definition::Field(field) => get_symbol_fragment(db, &FieldOrAssocItem::Field(field)), - _ => None, - }; - - get_doc_base_url(db, &krate)? + let mut url = get_doc_base_url(db, &krate)? .join(&base) - .ok() - .and_then(|mut url| { - if !matches!(definition, Definition::ModuleDef(ModuleDef::Module(..))) { - url.path_segments_mut().ok()?.pop(); - }; - Some(url) - }) - .and_then(|url| url.join(filename.as_deref()?).ok()) - .and_then( - |url| if let Some(fragment) = fragment { url.join(&fragment).ok() } else { Some(url) }, - ) - .map(|url| url.into()) + .ok()? + .join(&get_symbol_filename(db, &target)?) + .ok()?; + url.set_fragment(frag.as_deref()); + + Some(url.into()) } fn rewrite_intra_doc_link( @@ -322,10 +300,7 @@ fn rewrite_intra_doc_link( let krate = resolved.module(db)?.krate(); let mut mod_path = String::new(); resolved - .module(db)? - .path_to_root(db) - .into_iter() - .rev() + .canonical_module_path(db)? .flat_map(|it| it.name(db)) .for_each(|name| format_to!(mod_path, "{}/", name)); let mut new_url = get_doc_base_url(db, &krate)? @@ -339,11 +314,7 @@ fn rewrite_intra_doc_link( AssocItemContainer::Impl(i) => i.self_ty(db).as_adt()?.into(), }; new_url = new_url.join(&get_symbol_filename(db, &resolved)?).ok()?; - let frag = match assoc_item { - AssocItem::Function(f) => format!("method.{}", f.name(db)), - AssocItem::Const(c) => format!("associatedconstant.{}", c.name(db)?), - AssocItem::TypeAlias(ta) => format!("associatedtype.{}", ta.name(db)), - }; + let frag = get_assoc_item_fragment(db, assoc_item)?; new_url.set_fragment(Some(&frag)); } else { new_url = new_url.join(&get_symbol_filename(db, &resolved)?).ok()?; @@ -360,16 +331,19 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option( /// Get the root URL for the documentation of a crate. /// -/// ``` +/// ```ignore /// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next /// ^^^^^^^^^^^^^^^^^^^^^^^^^^ /// ``` @@ -430,7 +404,7 @@ fn get_doc_base_url(db: &RootDatabase, krate: &Crate) -> Option { /// Get the filename and extension generated for a symbol by rustdoc. /// -/// ``` +/// ```ignore /// https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next /// ^^^^^^^^^^^^^^^^^^^ /// ``` @@ -441,7 +415,10 @@ fn get_symbol_filename(db: &dyn HirDatabase, definition: &ModuleDef) -> Option format!("enum.{}.html", e.name(db)), Adt::Union(u) => format!("union.{}.html", u.name(db)), }, - ModuleDef::Module(_) => "index.html".to_string(), + ModuleDef::Module(m) => match m.name(db) { + Some(name) => format!("{}/index.html", name), + None => String::from("index.html"), + }, ModuleDef::Trait(t) => format!("trait.{}.html", t.name(db)), ModuleDef::TypeAlias(t) => format!("type.{}.html", t.name(db)), ModuleDef::BuiltinType(t) => format!("primitive.{}.html", t.name()), @@ -454,38 +431,28 @@ fn get_symbol_filename(db: &dyn HirDatabase, definition: &ModuleDef) -> Option Option { - Some(match field_or_assoc { - FieldOrAssocItem::Field(field) => format!("#structfield.{}", field.name(db)), - FieldOrAssocItem::AssocItem(assoc) => match assoc { - AssocItem::Function(function) => { - let is_trait_method = function - .as_assoc_item(db) - .and_then(|assoc| assoc.containing_trait(db)) - .is_some(); - // This distinction may get more complicated when specialization is available. - // Rustdoc makes this decision based on whether a method 'has defaultness'. - // Currently this is only the case for provided trait methods. - if is_trait_method && !function.has_body(db) { - format!("#tymethod.{}", function.name(db)) - } else { - format!("#method.{}", function.name(db)) - } +fn get_assoc_item_fragment(db: &dyn HirDatabase, assoc_item: hir::AssocItem) -> Option { + Some(match assoc_item { + AssocItem::Function(function) => { + let is_trait_method = + function.as_assoc_item(db).and_then(|assoc| assoc.containing_trait(db)).is_some(); + // This distinction may get more complicated when specialization is available. + // Rustdoc makes this decision based on whether a method 'has defaultness'. + // Currently this is only the case for provided trait methods. + if is_trait_method && !function.has_body(db) { + format!("tymethod.{}", function.name(db)) + } else { + format!("method.{}", function.name(db)) } - AssocItem::Const(constant) => format!("#associatedconstant.{}", constant.name(db)?), - AssocItem::TypeAlias(ty) => format!("#associatedtype.{}", ty.name(db)), - }, + } + AssocItem::Const(constant) => format!("associatedconstant.{}", constant.name(db)?), + AssocItem::TypeAlias(ty) => format!("associatedtype.{}", ty.name(db)), }) } @@ -493,6 +460,7 @@ fn get_symbol_fragment(db: &dyn HirDatabase, field_or_assoc: &FieldOrAssocItem) mod tests { use expect_test::{expect, Expect}; use ide_db::base_db::FileRange; + use itertools::Itertools; use crate::{display::TryToNav, fixture}; @@ -527,7 +495,7 @@ pub struct Fo$0o; r#" pub fn fo$0o() {} "#, - expect![[r##"https://docs.rs/test/*/test/fn.foo.html#method.foo"##]], + expect![[r##"https://docs.rs/test/*/test/fn.foo.html"##]], ); } @@ -599,13 +567,6 @@ pub mod foo { #[test] fn test_reexport_order() { - cov_mark::check!(test_reexport_order); - // FIXME: This should return - // - // https://docs.rs/test/*/test/wrapper/modulestruct.Item.html - // - // That is, we should point inside the module, rather than at the - // re-export. check_external_docs( r#" pub mod wrapper { @@ -620,7 +581,7 @@ fn foo() { let bar: wrapper::It$0em; } "#, - expect![[r#"https://docs.rs/test/*/test/wrapper/struct.Item.html"#]], + expect![[r#"https://docs.rs/test/*/test/wrapper/module/struct.Item.html"#]], ) } @@ -657,7 +618,7 @@ pub trait Foo { /// [buzz]: Foo::buzz pub struct Bar$0; "#, - expect![[r###"[Foo](https://docs.rs/test/*/test/trait.Foo.html#method.buzz)"###]], + expect![[r###"[Foo](https://docs.rs/test/*/test/trait.Foo.html#tymethod.buzz)"###]], ) } diff --git a/crates/ide/src/doc_links/intra_doc_links.rs b/crates/ide/src/doc_links/intra_doc_links.rs index c6a090a060..d94999dddd 100644 --- a/crates/ide/src/doc_links/intra_doc_links.rs +++ b/crates/ide/src/doc_links/intra_doc_links.rs @@ -1,3 +1,5 @@ +//! Helper tools for intra doc links. + const TYPES: ([&str; 9], [&str; 0]) = (["type", "struct", "enum", "mod", "trait", "union", "module", "prim", "primitive"], []); const VALUES: ([&str; 8], [&str; 1]) = diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 93b1cf1c90..e9ba825c03 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -2103,7 +2103,7 @@ pub struct B$0ar --- - [Foo](https://docs.rs/test/*/test/trait.Foo.html#method.buzz) + [Foo](https://docs.rs/test/*/test/trait.Foo.html#tymethod.buzz) "##]], ); } diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 0fd8ad970d..2cf801751c 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -400,10 +400,7 @@ fn module_def_doctest(db: &RootDatabase, def: hir::ModuleDef) -> Option