Rollup merge of #129471 - GuillaumeGomez:sort-impl-associated-items, r=t-rustdoc-frontend

[rustdoc] Sort impl associated items by kinds and then by appearance

Following [this zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/.22Freeze.22.20order.20of.20items.20in.20.28trait.29.20impls.3F), I implemented it.

This brings the following change: impl associated items will now be grouped by kind and will now be first sorted by kind and then by the order they are declared in the source code (like currently).

The kinds are sorted in the following order:
1. Constants
2. Types
3. Functions

The reason behind this order is that associated constants can be used in associated types (like length in arrays) and both associated types and associated constants can be used in associated functions. So if an associated item from the same impl is used, its definition will always be above where it's being used.

cc ``@camelid``
r? ``@notriddle``

Author: matthiaskrgr

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. Constants
+    // 2. Types
+    // 3. Functions
+    //
+    // 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::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,
+                        &mut impl_items,
+                        cx,
+                        trait_item,
+                        if trait_.is_some() { &i.impl_item } else { parent },
+                        link,
+                        render_mode,
+                        false,
+                        trait_,
+                        rendering_params,
+                    );
+                }
+                _ => {}
+            }
+        }
+
+        for assoc_type in assoc_types {
             doc_impl_item(
                 &mut default_impl_items,
                 &mut impl_items,
                 cx,
-                trait_item,
+                assoc_type,
+                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,

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,
             "<div class=\"methods\">",
         );
-        for t in required_types {
+        for t in required_consts {
             trait_item(w, cx, t, it);
         }
         w.write_str("</div>");
     }
-    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,
             "<div class=\"methods\">",
         );
-        for t in provided_types {
+        for t in provided_consts {
             trait_item(w, cx, t, it);
         }
         w.write_str("</div>");
     }
 
-    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,
             "<div class=\"methods\">",
         );
-        for t in required_consts {
+        for t in required_types {
             trait_item(w, cx, t, it);
         }
         w.write_str("</div>");
     }
-    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,
             "<div class=\"methods\">",
         );
-        for t in provided_consts {
+        for t in provided_types {
             trait_item(w, cx, t, it);
         }
         w.write_str("</div>");

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),
@@ -394,29 +394,22 @@ 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();
         let mut id_map = IdMap::new();
 
         {
             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();
         }
 
@@ -426,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),
         ];
 
@@ -452,6 +450,7 @@ fn sidebar_assoc_items<'a>(
                 &mut blocks,
             );
         }
+
         links.append(&mut blocks);
     }
 }
@@ -715,3 +714,19 @@ fn get_associated_constants<'a>(
         })
         .collect::<Vec<_>>()
 }
+
+fn get_associated_types<'a>(
+    i: &'a clean::Impl,
+    used_links: &mut FxHashSet<String>,
+) -> Vec<Link<'a>> {
+    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::<Vec<_>>()
+}

tests/rustdoc/impl-associated-items-order.rs

@@ -0,0 +1,42 @@
+// This test ensures that impl associated items always follow this order:
+//
+// 1. Consts
+// 2. Types
+// 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[1]/h4' \
+    // 'pub const X: u8 = 12u8'
+    pub const X: u8 = 12;
+    //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/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' \
+    // '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() {}
+}

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;
+}