From fe6b0b8711d30b67ee4ae5f74e6b5b06926e815e Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 22 Nov 2021 01:27:44 +0000 Subject: [PATCH 1/2] Fill in "Implementors" section in navbar Fixes #91118 --- src/librustdoc/formats/mod.rs | 4 + src/librustdoc/html/render/context.rs | 6 +- src/librustdoc/html/render/mod.rs | 148 +++++++++++++---------- src/librustdoc/html/render/print_item.rs | 5 +- 4 files changed, 95 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs index 4f0c5a9edee71..64724a2125413 100644 --- a/src/librustdoc/formats/mod.rs +++ b/src/librustdoc/formats/mod.rs @@ -40,4 +40,8 @@ impl Impl { crate fn trait_did(&self) -> Option { self.inner_impl().trait_.as_ref().map(|t| t.def_id()) } + + crate fn is_local(&self, cache: &cache::Cache) -> bool { + self.inner_impl().for_.def_id(cache).map_or(true, |d| cache.paths.contains_key(&d)) + } } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 069862efde640..9c2c3c9dcbe02 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -59,6 +59,8 @@ crate struct Context<'tcx> { pub(super) deref_id_map: RefCell>, /// The map used to ensure all generated 'id=' attributes are unique. pub(super) id_map: RefCell, + /// Converts the rustc id of an item to the html id so we can link to it + pub(super) item_to_html_id_map: RefCell>, /// Shared mutable state. /// /// Issue for improving the situation: [#82381][] @@ -73,7 +75,7 @@ crate struct Context<'tcx> { // `Context` is cloned a lot, so we don't want the size to grow unexpectedly. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Context<'_>, 144); +rustc_data_structures::static_assert_size!(Context<'_>, 184); /// Shared mutable state used in [`Context`] and elsewhere. crate struct SharedContext<'tcx> { @@ -517,6 +519,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { render_redirect_pages: false, id_map: RefCell::new(id_map), deref_id_map: RefCell::new(FxHashMap::default()), + item_to_html_id_map: RefCell::new(FxHashMap::default()), shared: Rc::new(scx), include_sources, }; @@ -541,6 +544,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { dst: self.dst.clone(), render_redirect_pages: self.render_redirect_pages, deref_id_map: RefCell::new(FxHashMap::default()), + item_to_html_id_map: RefCell::new(FxHashMap::default()), id_map: RefCell::new(IdMap::new()), shared: Rc::clone(&self.shared), include_sources: self.include_sources, diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index ffd09663f8240..c83efd3dc8ec0 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1613,6 +1613,7 @@ fn render_impl( ); write!(w, "") } + render_impl_summary( w, cx, @@ -1691,7 +1692,7 @@ pub(crate) fn render_impl_summary( // in documentation pages for trait with automatic implementations like "Send" and "Sync". aliases: &[String], ) { - let id = cx.derive_id(match i.inner_impl().trait_ { + let html_id = cx.derive_id(match i.inner_impl().trait_ { Some(ref t) => { if is_on_foreign_type { get_id_for_impl_on_foreign_type(&i.inner_impl().for_, t, cx) @@ -1701,14 +1702,17 @@ pub(crate) fn render_impl_summary( } None => "impl".to_string(), }); + + cx.item_to_html_id_map.borrow_mut().insert(i.impl_item.def_id, html_id.clone()); + let aliases = if aliases.is_empty() { String::new() } else { format!(" data-aliases=\"{}\"", aliases.join(",")) }; - write!(w, "
", id, aliases); + write!(w, "
", html_id, aliases); render_rightside(w, cx, &i.impl_item, containing_item); - write!(w, "", id); + write!(w, "", html_id); write!(w, "

"); if let Some(use_absolute) = use_absolute { @@ -2214,20 +2218,12 @@ fn get_id_for_impl_on_foreign_type( small_url_encode(format!("impl-{:#}-for-{:#}", trait_.print(cx), for_.print(cx))) } -fn extract_for_impl_name(item: &clean::Item, cx: &Context<'_>) -> Option<(String, String)> { - match *item.kind { - clean::ItemKind::ImplItem(ref i) => { - i.trait_.as_ref().map(|trait_| { - // Alternative format produces no URLs, - // so this parameter does nothing. - ( - format!("{:#}", i.for_.print(cx)), - get_id_for_impl_on_foreign_type(&i.for_, trait_, cx), - ) - }) - } - _ => None, - } +fn extract_for_impl_name(item: &Impl, cx: &Context<'_>) -> (String, String) { + let i = item.inner_impl(); + let trait_ = i.trait_.as_ref().unwrap(); + // Alternative format produces no URLs, + // so this parameter does nothing. + (format!("{:#}", i.for_.print(cx)), get_id_for_impl_on_foreign_type(&i.for_, trait_, cx)) } fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { @@ -2236,101 +2232,125 @@ fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean fn print_sidebar_section( out: &mut Buffer, items: &[clean::Item], - before: &str, + title: &str, + id: &str, filter: impl Fn(&clean::Item) -> bool, write: impl Fn(&mut Buffer, &str), - after: &str, ) { - let mut items = items - .iter() - .filter_map(|m| match m.name { - Some(ref name) if filter(m) => Some(name.as_str()), - _ => None, - }) - .collect::>(); + print_sidebar_section_with( + out, + title, + id, + items.iter().filter(|i| filter(i)).filter_map(|i| i.name.map(|n| n.as_str())), + |buf, s| write(buf, &s), + ) + } + + fn print_sidebar_section_with( + out: &mut Buffer, + title: &str, + id: &str, + iter: impl Iterator, + write: impl Fn(&mut Buffer, T), + ) { + let mut items = iter.collect::>(); if !items.is_empty() { items.sort_unstable(); - out.push_str(before); + write!( + out, + "

\ + {}

", + id, title + ); for item in items.into_iter() { - write(out, &item); + write(out, item); } - out.push_str(after); + out.push_str("
"); } } print_sidebar_section( buf, &t.items, - "

\ - Associated Types

", + "Associated Types", + "associated-types", |m| m.is_associated_type(), |out, sym| write!(out, "{0}", sym), - "
", ); print_sidebar_section( buf, &t.items, - "

\ - Associated Constants

", + "Associated Constants", + "associated-const", |m| m.is_associated_const(), |out, sym| write!(out, "{0}", sym), - "
", ); print_sidebar_section( buf, &t.items, - "

\ - Required Methods

", + "Required Methods", + "required-methods", |m| m.is_ty_method(), |out, sym| write!(out, "{0}", sym), - "
", ); print_sidebar_section( buf, &t.items, - "

\ - Provided Methods

", + "Provided Methods", + "provided-methods", |m| m.is_method(), |out, sym| write!(out, "{0}", sym), - "
", ); let cache = cx.cache(); - if let Some(implementors) = cache.implementors.get(&it.def_id.expect_def_id()) { - let mut res = implementors - .iter() - .filter(|i| { - i.inner_impl().for_.def_id(cache).map_or(false, |d| !cache.paths.contains_key(&d)) - }) - .filter_map(|i| extract_for_impl_name(&i.impl_item, cx)) - .collect::>(); - - if !res.is_empty() { - res.sort(); - buf.push_str( - "

\ - Implementations on Foreign Types

\ -
", - ); - for (name, id) in res.into_iter() { - write!(buf, "{}", id, Escape(&name)); - } - buf.push_str("
"); - } - } + let did = it.def_id.expect_def_id(); + + let (local_impl, foreign_impls) = cache + .implementors + .get(&did) + .iter() + .flat_map(|x| *x) + .partition::, _>(|i| i.is_local(cache)); + + print_sidebar_section_with( + buf, + "Implementations on Foreign Types", + "foreign-impls", + foreign_impls.iter().map(|i| extract_for_impl_name(i, cx)), + |buf, (name, id)| write!(buf, "{}", id, Escape(&name)), + ); sidebar_assoc_items(cx, buf, it); - buf.push_str("

Implementors

"); + print_sidebar_section_with( + buf, + "Implementors", + "implementors", + local_impl.iter().filter(|i| i.inner_impl().polarity == ty::ImplPolarity::Positive).map( + |i| { + ( + format!("{:#}", i.inner_impl().for_.print(cx)), + cx.item_to_html_id_map + .borrow() + .get(&i.impl_item.def_id) + .unwrap_or_else(|| panic!("Not in index {:#?}", i)) + .clone(), + ) + }, + ), + |buf, (name, id)| write!(buf, "{}", id, Escape(&name)), + ); + if t.is_auto { buf.push_str( "

Auto Implementors

", ); + // FIXME: List Auto Implementors } buf.push_str("

") diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 049d17a4b477a..78f547936ddf6 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -741,9 +741,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra } } - let (local, foreign) = implementors.iter().partition::, _>(|i| { - i.inner_impl().for_.def_id(cache).map_or(true, |d| cache.paths.contains_key(&d)) - }); + let (local, foreign) = implementors.iter().partition::, _>(|i| i.is_local(cache)); let (mut synthetic, mut concrete): (Vec<&&Impl>, Vec<&&Impl>) = local.iter().partition(|i| i.inner_impl().kind.is_auto()); @@ -784,6 +782,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra "Implementors", "
", ); + for implementor in concrete { render_implementor(cx, implementor, it, w, &implementor_dups, &[]); } From 3ff81dbd184683001c494a0e867ef2e3ce290824 Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Sat, 27 Nov 2021 01:28:46 +0000 Subject: [PATCH 2/2] Add trait implementors sidebar tests --- src/test/rustdoc/sidebar-links-impls.rs | 49 +++++++++++++++++++ .../rustdoc/sidebar-links-to-foreign-impl.rs | 16 ------ 2 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 src/test/rustdoc/sidebar-links-impls.rs delete mode 100644 src/test/rustdoc/sidebar-links-to-foreign-impl.rs diff --git a/src/test/rustdoc/sidebar-links-impls.rs b/src/test/rustdoc/sidebar-links-impls.rs new file mode 100644 index 0000000000000..76fc59af4109b --- /dev/null +++ b/src/test/rustdoc/sidebar-links-impls.rs @@ -0,0 +1,49 @@ +// issue #56018: "Implementations on Foreign Types" sidebar items should link to specific impls + +#![crate_name = "foo"] + +// @has foo/trait.Foo.html +// @has - '//*[@class="sidebar-title"]/a[@href="#foreign-impls"]' 'Implementations on Foreign Types' +// @has - '//h2[@id="foreign-impls"]' 'Implementations on Foreign Types' +// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Foo-for-u32"]' 'u32' +// @has - '//div[@id="impl-Foo-for-u32"]//h3[@class="code-header in-band"]' 'impl Foo for u32' +// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Foo-for-%26%27a%20str"]' "&'a str" +// @has - '//div[@id="impl-Foo-for-%26%27a%20str"]//h3[@class="code-header in-band"]' "impl<'a> Foo for &'a str" +pub trait Foo {} + +impl Foo for u32 {} + +impl<'a> Foo for &'a str {} + +// Issue #91118" "Implementors column on trait page is always empty" + +pub struct B; +pub struct A; +struct C; + +// @has foo/trait.Bar.html +// @has - '//*[@class="sidebar-title"]/a[@href="#implementors"]' 'Implementors' +// @has - '//h2[@id="implementors"]' 'Implementors' +// @!has - '//*[@class="sidebar-title"]/a[@href="#foreign-impls"]' +// @!has - '//h2[@id="foreign-impls"]' + +// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Bar"]' '&A' +// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Bar-1"]' 'A' +// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Bar-2"]' 'B' + +// @has - '//div[@id="impl-Bar"]//h3[@class="code-header in-band"]' 'impl Bar for &A' +// @has - '//div[@id="impl-Bar-1"]//h3[@class="code-header in-band"]' 'impl Bar for A' +// @has - '//div[@id="impl-Bar-2"]//h3[@class="code-header in-band"]' 'impl Bar for B' +pub trait Bar {} +impl Bar for B {} +impl Bar for A {} +impl Bar for &A {} +impl Bar for C {} + +// @has foo/trait.NotImpled.html +// @!has - '//*[@class="sidebar-title"]/a[@href="#implementors"]' +// FIXME: Is this the semantics we want +// @has - '//h2[@id="implementors"]' 'Implementors' +// @!has - '//*[@class="sidebar-title"]/a[@href="#foreign-impls"]' +// @!has - '//h2[@id="foreign-impls"]' +pub trait NotImpled {} diff --git a/src/test/rustdoc/sidebar-links-to-foreign-impl.rs b/src/test/rustdoc/sidebar-links-to-foreign-impl.rs deleted file mode 100644 index d1083c487642d..0000000000000 --- a/src/test/rustdoc/sidebar-links-to-foreign-impl.rs +++ /dev/null @@ -1,16 +0,0 @@ -// issue #56018: "Implementations on Foreign Types" sidebar items should link to specific impls - -#![crate_name = "foo"] - -// @has foo/trait.Foo.html -// @has - '//*[@class="sidebar-title"]/a[@href="#foreign-impls"]' 'Implementations on Foreign Types' -// @has - '//h2[@id="foreign-impls"]' 'Implementations on Foreign Types' -// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Foo-for-u32"]' 'u32' -// @has - '//div[@id="impl-Foo-for-u32"]//h3[@class="code-header in-band"]' 'impl Foo for u32' -// @has - '//*[@class="sidebar-links"]/a[@href="#impl-Foo-for-%26%27a%20str"]' "&'a str" -// @has - '//div[@id="impl-Foo-for-%26%27a%20str"]//h3[@class="code-header in-band"]' "impl<'a> Foo for &'a str" -pub trait Foo {} - -impl Foo for u32 {} - -impl<'a> Foo for &'a str {}