From a4594032cf15ad3b0532f67d7b3b5ca517edd90e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:23:35 +0200 Subject: [PATCH 1/6] Sort impl associated items by kinds and then by appearance --- src/librustdoc/html/render/mod.rs | 53 ++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index dc34c7844bfdd..9e9934992768d 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1794,13 +1794,64 @@ fn render_impl( let mut default_impl_items = Buffer::empty_from(w); let impl_ = i.inner_impl(); + // Impl items are grouped by kinds: + // + // 1. Types + // 2. Constants + // 3. Functions + // + // This order is because you can have associated types in associated constants, and both in + // associcated functions. So with this order, when reading from top to bottom, you should always + // see all items definitions before they're actually used. + let mut assoc_consts = Vec::new(); + let mut methods = Vec::new(); + if !impl_.is_negative_trait_impl() { for trait_item in &impl_.items { + match *trait_item.kind { + clean::MethodItem(..) | clean::TyMethodItem(_) => methods.push(trait_item), + clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { + assoc_consts.push(trait_item) + } + clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => { + // We render it directly since they're supposed to come first. + doc_impl_item( + &mut default_impl_items, + &mut impl_items, + cx, + trait_item, + if trait_.is_some() { &i.impl_item } else { parent }, + link, + render_mode, + false, + trait_, + rendering_params, + ); + } + _ => {} + } + } + + for assoc_const in assoc_consts { doc_impl_item( &mut default_impl_items, &mut impl_items, cx, - trait_item, + assoc_const, + if trait_.is_some() { &i.impl_item } else { parent }, + link, + render_mode, + false, + trait_, + rendering_params, + ); + } + for method in methods { + doc_impl_item( + &mut default_impl_items, + &mut impl_items, + cx, + method, if trait_.is_some() { &i.impl_item } else { parent }, link, render_mode, From f96aff95d591a35058ae82580f7e31615b2f3d3d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:24:18 +0200 Subject: [PATCH 2/6] Add regression test for impl associated items sorting --- tests/rustdoc/impl-associated-items-order.rs | 42 ++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/rustdoc/impl-associated-items-order.rs diff --git a/tests/rustdoc/impl-associated-items-order.rs b/tests/rustdoc/impl-associated-items-order.rs new file mode 100644 index 0000000000000..f8744be04ba81 --- /dev/null +++ b/tests/rustdoc/impl-associated-items-order.rs @@ -0,0 +1,42 @@ +// This test ensures that impl associated items always follow this order: +// +// 1. Types +// 2. Consts +// 3. Functions + +#![feature(inherent_associated_types)] +#![allow(incomplete_features)] +#![crate_name = "foo"] + +//@ has 'foo/struct.Bar.html' +pub struct Bar; + +impl Bar { + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ + // 'pub fn foo()' + pub fn foo() {} + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + // 'pub const X: u8 = 12u8' + pub const X: u8 = 12; + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'pub type Y = u8' + pub type Y = u8; +} + +pub trait Foo { + const W: u32; + fn yeay(); + type Z; +} + +impl Foo for Bar { + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + // 'const W: u32 = 12u32' + const W: u32 = 12; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'type Z = u8' + type Z = u8; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ + // 'fn yeay()' + fn yeay() {} +} From 188498300129552c6ea38860a50debada7265fe3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 25 Aug 2024 22:02:57 +0200 Subject: [PATCH 3/6] Make impl associated constants sorted first --- src/librustdoc/html/render/mod.rs | 22 ++++++++++---------- src/librustdoc/html/render/sidebar.rs | 4 ++-- tests/rustdoc/impl-associated-items-order.rs | 14 ++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 9e9934992768d..095bfe158ff19 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1796,24 +1796,24 @@ fn render_impl( // Impl items are grouped by kinds: // - // 1. Types - // 2. Constants + // 1. Constants + // 2. Types // 3. Functions // - // This order is because you can have associated types in associated constants, and both in - // associcated functions. So with this order, when reading from top to bottom, you should always - // see all items definitions before they're actually used. - let mut assoc_consts = Vec::new(); + // This order is because you can have associated constants used in associated types (like array + // length), and both in associcated functions. So with this order, when reading from top to + // bottom, you should see items definitions before they're actually used most of the time. + let mut assoc_types = Vec::new(); let mut methods = Vec::new(); if !impl_.is_negative_trait_impl() { for trait_item in &impl_.items { match *trait_item.kind { clean::MethodItem(..) | clean::TyMethodItem(_) => methods.push(trait_item), - clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { - assoc_consts.push(trait_item) - } clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => { + assoc_types.push(trait_item) + } + clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { // We render it directly since they're supposed to come first. doc_impl_item( &mut default_impl_items, @@ -1832,12 +1832,12 @@ fn render_impl( } } - for assoc_const in assoc_consts { + for assoc_type in assoc_types { doc_impl_item( &mut default_impl_items, &mut impl_items, cx, - assoc_const, + assoc_type, if trait_.is_some() { &i.impl_item } else { parent }, link, render_mode, diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index c75c28ef0a3b4..6d0345042707e 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -302,10 +302,10 @@ fn sidebar_trait<'a>( blocks.extend( [ - ("required-associated-types", "Required Associated Types", req_assoc), - ("provided-associated-types", "Provided Associated Types", prov_assoc), ("required-associated-consts", "Required Associated Constants", req_assoc_const), ("provided-associated-consts", "Provided Associated Constants", prov_assoc_const), + ("required-associated-types", "Required Associated Types", req_assoc), + ("provided-associated-types", "Provided Associated Types", prov_assoc), ("required-methods", "Required Methods", req_method), ("provided-methods", "Provided Methods", prov_method), ("foreign-impls", "Implementations on Foreign Types", foreign_impls), diff --git a/tests/rustdoc/impl-associated-items-order.rs b/tests/rustdoc/impl-associated-items-order.rs index f8744be04ba81..759e0f0b40095 100644 --- a/tests/rustdoc/impl-associated-items-order.rs +++ b/tests/rustdoc/impl-associated-items-order.rs @@ -1,7 +1,7 @@ // This test ensures that impl associated items always follow this order: // -// 1. Types -// 2. Consts +// 1. Consts +// 2. Types // 3. Functions #![feature(inherent_associated_types)] @@ -15,10 +15,10 @@ impl Bar { //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ // 'pub fn foo()' pub fn foo() {} - //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ // 'pub const X: u8 = 12u8' pub const X: u8 = 12; - //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ // 'pub type Y = u8' pub type Y = u8; } @@ -31,11 +31,11 @@ pub trait Foo { impl Foo for Bar { //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ - // 'const W: u32 = 12u32' - const W: u32 = 12; - //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ // 'type Z = u8' type Z = u8; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'const W: u32 = 12u32' + const W: u32 = 12; //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ // 'fn yeay()' fn yeay() {} From d059f37724881d385546b1e7fd27eff4dad65645 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:23 +0200 Subject: [PATCH 4/6] Add missing sidebar associated items --- src/librustdoc/html/render/print_item.rs | 32 ++++++------ src/librustdoc/html/render/sidebar.rs | 62 +++++++++++++++--------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index a0a72d59d123f..cda5409a460ee 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -843,55 +843,55 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean: } } - if !required_types.is_empty() { + if !required_consts.is_empty() { write_section_heading( w, - "Required Associated Types", - "required-associated-types", + "Required Associated Constants", + "required-associated-consts", None, "
", ); - for t in required_types { + for t in required_consts { trait_item(w, cx, t, it); } w.write_str("
"); } - if !provided_types.is_empty() { + if !provided_consts.is_empty() { write_section_heading( w, - "Provided Associated Types", - "provided-associated-types", + "Provided Associated Constants", + "provided-associated-consts", None, "
", ); - for t in provided_types { + for t in provided_consts { trait_item(w, cx, t, it); } w.write_str("
"); } - if !required_consts.is_empty() { + if !required_types.is_empty() { write_section_heading( w, - "Required Associated Constants", - "required-associated-consts", + "Required Associated Types", + "required-associated-types", None, "
", ); - for t in required_consts { + for t in required_types { trait_item(w, cx, t, it); } w.write_str("
"); } - if !provided_consts.is_empty() { + if !provided_types.is_empty() { write_section_heading( w, - "Provided Associated Constants", - "provided-associated-consts", + "Provided Associated Types", + "provided-associated-types", None, "
", ); - for t in provided_consts { + for t in provided_types { trait_item(w, cx, t, it); } w.write_str("
"); diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index 6d0345042707e..b9e46851465d4 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -394,6 +394,7 @@ fn sidebar_assoc_items<'a>( let cache = cx.cache(); let mut assoc_consts = Vec::new(); + let mut assoc_types = Vec::new(); let mut methods = Vec::new(); if let Some(v) = cache.impls.get(&did) { let mut used_links = FxHashSet::default(); @@ -401,22 +402,14 @@ fn sidebar_assoc_items<'a>( { let used_links_bor = &mut used_links; - assoc_consts.extend( - v.iter() - .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_associated_constants(i.inner_impl(), used_links_bor)), - ); + for impl_ in v.iter().map(|i| i.inner_impl()).filter(|i| i.trait_.is_none()) { + assoc_consts.extend(get_associated_constants(impl_, used_links_bor)); + assoc_types.extend(get_associated_types(impl_, used_links_bor)); + methods.extend(get_methods(impl_, false, used_links_bor, false, cx.tcx())); + } // We want links' order to be reproducible so we don't use unstable sort. assoc_consts.sort(); - - #[rustfmt::skip] // rustfmt makes the pipeline less readable - methods.extend( - v.iter() - .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_methods(i.inner_impl(), false, used_links_bor, false, cx.tcx())), - ); - - // We want links' order to be reproducible so we don't use unstable sort. + assoc_types.sort(); methods.sort(); } @@ -443,15 +436,24 @@ fn sidebar_assoc_items<'a>( let (blanket_impl, concrete): (Vec<&Impl>, Vec<&Impl>) = concrete.into_iter().partition::, _>(|i| i.inner_impl().kind.is_blanket()); - sidebar_render_assoc_items( - cx, - &mut id_map, - concrete, - synthetic, - blanket_impl, - &mut blocks, - ); + sidebar_render_assoc_items(cx, &mut id_map, concrete, synthetic, blanket_impl, &mut blocks); } + + blocks.extend([ + LinkBlock::new( + Link::new("implementations", "Associated Constants"), + "associatedconstant", + assoc_consts, + ), + LinkBlock::new( + Link::new("implementations", "Associated Types"), + "associatedtype", + assoc_types, + ), + LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), + ]); + blocks.append(&mut deref_methods); + blocks.extend([concrete, synthetic, blanket]); links.append(&mut blocks); } } @@ -715,3 +717,19 @@ fn get_associated_constants<'a>( }) .collect::>() } + +fn get_associated_types<'a>( + i: &'a clean::Impl, + used_links: &mut FxHashSet, +) -> Vec> { + i.items + .iter() + .filter_map(|item| match item.name { + Some(ref name) if !name.is_empty() && item.is_associated_type() => Some(Link::new( + get_next_url(used_links, format!("{typ}.{name}", typ = ItemType::AssocType)), + name.as_str(), + )), + _ => None, + }) + .collect::>() +} From 238944c5d7035da09810d84179eeb81a9dac37f0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:35 +0200 Subject: [PATCH 5/6] Add regression test for sidebar associated items --- .../rustdoc/impl-associated-items-sidebar.rs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/rustdoc/impl-associated-items-sidebar.rs diff --git a/tests/rustdoc/impl-associated-items-sidebar.rs b/tests/rustdoc/impl-associated-items-sidebar.rs new file mode 100644 index 0000000000000..d393a577e5009 --- /dev/null +++ b/tests/rustdoc/impl-associated-items-sidebar.rs @@ -0,0 +1,42 @@ +// This test ensures that impl/trait associated items are listed in the sidebar. + +// ignore-tidy-linelength + +#![feature(inherent_associated_types)] +#![feature(associated_type_defaults)] +#![allow(incomplete_features)] +#![crate_name = "foo"] + +//@ has 'foo/struct.Bar.html' +pub struct Bar; + +impl Bar { + //@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedconstant"]/li/a[@href="#associatedconstant.X"]' 'X' + pub const X: u8 = 12; + //@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedtype"]/li/a[@href="#associatedtype.Y"]' 'Y' + pub type Y = u8; +} + +//@ has 'foo/trait.Foo.html' +pub trait Foo { + //@ has - '//*[@class="sidebar-elems"]//h3[5]' 'Required Methods' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][5]/li/a[@href="#tymethod.yeay"]' 'yeay' + fn yeay(); + //@ has - '//*[@class="sidebar-elems"]//h3[6]' 'Provided Methods' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][6]/li/a[@href="#method.boo"]' 'boo' + fn boo() {} + //@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Required Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][1]/li/a[@href="#associatedconstant.W"]' 'W' + const W: u32; + //@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Provided Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][2]/li/a[@href="#associatedconstant.U"]' 'U' + const U: u32 = 0; + //@ has - '//*[@class="sidebar-elems"]//h3[3]' 'Required Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][3]/li/a[@href="#associatedtype.Z"]' 'Z' + type Z; + //@ has - '//*[@class="sidebar-elems"]//h3[4]' 'Provided Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][4]/li/a[@href="#associatedtype.T"]' 'T' + type T = u32; +} From 8f9c4b36fe2a2308f24d66e1d75ec54e3f3fcdb7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 5 Sep 2024 12:33:05 +0200 Subject: [PATCH 6/6] Update to new rustdoc internal API --- src/librustdoc/html/render/sidebar.rs | 29 ++++++++++++--------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index b9e46851465d4..06cc57b2a5536 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -419,6 +419,11 @@ fn sidebar_assoc_items<'a>( "associatedconstant", assoc_consts, ), + LinkBlock::new( + Link::new("implementations", "Associated Types"), + "associatedtype", + assoc_types, + ), LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), ]; @@ -436,24 +441,16 @@ fn sidebar_assoc_items<'a>( let (blanket_impl, concrete): (Vec<&Impl>, Vec<&Impl>) = concrete.into_iter().partition::, _>(|i| i.inner_impl().kind.is_blanket()); - sidebar_render_assoc_items(cx, &mut id_map, concrete, synthetic, blanket_impl, &mut blocks); + sidebar_render_assoc_items( + cx, + &mut id_map, + concrete, + synthetic, + blanket_impl, + &mut blocks, + ); } - blocks.extend([ - LinkBlock::new( - Link::new("implementations", "Associated Constants"), - "associatedconstant", - assoc_consts, - ), - LinkBlock::new( - Link::new("implementations", "Associated Types"), - "associatedtype", - assoc_types, - ), - LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), - ]); - blocks.append(&mut deref_methods); - blocks.extend([concrete, synthetic, blanket]); links.append(&mut blocks); } }