From 318b4f30ba25e3e4c34a3ecd6f5cfdd211c8933f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:23:35 +0200 Subject: [PATCH 1/5] 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 c1b2ee7d8ae92..029404a797014 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1784,13 +1784,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 b7cd99dca3104f20590b83ba4a2c42a7df550110 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:24:18 +0200 Subject: [PATCH 2/5] 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 846cb34cdb6fe9437e5e55930150e7b2129cf2ba Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 25 Aug 2024 22:02:57 +0200 Subject: [PATCH 3/5] 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 029404a797014..26df355eab1a4 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1786,24 +1786,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, @@ -1822,12 +1822,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 101cc839f0988..660508a918696 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -223,10 +223,10 @@ fn sidebar_trait<'a>( } let mut blocks: Vec> = [ - ("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 78971f3b6a167ff96c9980aaa8b4ca3f3e4634a6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:23 +0200 Subject: [PATCH 4/5] Add missing sidebar associated items --- src/librustdoc/html/render/print_item.rs | 32 +++++++++--------- src/librustdoc/html/render/sidebar.rs | 42 ++++++++++++++++-------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index d8b111471b85e..e6e96ea0e97a3 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -842,55 +842,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 660508a918696..8922cc19f0f18 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -319,6 +319,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(); @@ -326,22 +327,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(); } @@ -378,6 +371,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), ]; blocks.append(&mut deref_methods); @@ -628,3 +626,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 4a80840e272de06884115eeb970f283b8f8c459b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:35 +0200 Subject: [PATCH 5/5] 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; +}