Rollup merge of #148021 - GuillaumeGomez:simplify-mod-render, r=notriddle

[rustdoc] Simplify module rendering and HTML tags handling

Extracted code from #145458.

This PR simplifies the rendering of modules, in particular the HTML tags handling. Instead of having all items in a `vec`, we make a map with their types as key, allowing to then iterate over the types, which allows us to open and close the HTML tag at every turn without the need to check if a tag was opened or not, or to check it's still the same kind of type.

For a better review experience: enable "Hide whitespace", the diff will be much smaller.

r? `@notriddle`

Author: Zalathar

src/librustdoc/html/render/print_item.rs

@@ -5,7 +5,7 @@ use std::iter;
 use askama::Template;
 use rustc_abi::VariantIdx;
 use rustc_ast::join_path_syms;
-use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
+use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::DefId;
@@ -307,8 +307,12 @@ fn item_module(cx: &Context<'_>, item: &clean::Item, items: &[clean::Item]) -> i
     fmt::from_fn(|w| {
         write!(w, "{}", document(cx, item, None, HeadingOffset::H2))?;
 
-        let mut not_stripped_items =
-            items.iter().filter(|i| !i.is_stripped()).enumerate().collect::<Vec<_>>();
+        let mut not_stripped_items: FxIndexMap<ItemType, Vec<(usize, &clean::Item)>> =
+            FxIndexMap::default();
+
+        for (index, item) in items.iter().filter(|i| !i.is_stripped()).enumerate() {
+            not_stripped_items.entry(item.type_()).or_default().push((index, item));
+        }
 
         // the order of item types in the listing
         fn reorder(ty: ItemType) -> u8 {
@@ -331,11 +335,6 @@ fn item_module(cx: &Context<'_>, item: &clean::Item, items: &[clean::Item]) -> i
         }
 
         fn cmp(i1: &clean::Item, i2: &clean::Item, tcx: TyCtxt<'_>) -> Ordering {
-            let rty1 = reorder(i1.type_());
-            let rty2 = reorder(i2.type_());
-            if rty1 != rty2 {
-                return rty1.cmp(&rty2);
-            }
             let is_stable1 =
                 i1.stability(tcx).as_ref().map(|s| s.level.is_stable()).unwrap_or(true);
             let is_stable2 =
@@ -357,7 +356,9 @@ fn item_module(cx: &Context<'_>, item: &clean::Item, items: &[clean::Item]) -> i
 
         match cx.shared.module_sorting {
             ModuleSorting::Alphabetical => {
-                not_stripped_items.sort_by(|(_, i1), (_, i2)| cmp(i1, i2, tcx));
+                for items in not_stripped_items.values_mut() {
+                    items.sort_by(|(_, i1), (_, i2)| cmp(i1, i2, tcx));
+                }
             }
             ModuleSorting::DeclarationOrder => {}
         }
@@ -380,155 +381,152 @@ fn item_module(cx: &Context<'_>, item: &clean::Item, items: &[clean::Item]) -> i
         // can be identical even if the elements are different (mostly in imports).
         // So in case this is an import, we keep everything by adding a "unique id"
         // (which is the position in the vector).
-        not_stripped_items.dedup_by_key(|(idx, i)| {
-            (
-                i.item_id,
-                if i.name.is_some() { Some(full_path(cx, i)) } else { None },
-                i.type_(),
-                if i.is_import() { *idx } else { 0 },
-            )
-        });
+        for items in not_stripped_items.values_mut() {
+            items.dedup_by_key(|(idx, i)| {
+                (
+                    i.item_id,
+                    if i.name.is_some() { Some(full_path(cx, i)) } else { None },
+                    i.type_(),
+                    if i.is_import() { *idx } else { 0 },
+                )
+            });
+        }
 
         debug!("{not_stripped_items:?}");
-        let mut last_section = None;
 
-        for (_, myitem) in &not_stripped_items {
-            let my_section = item_ty_to_section(myitem.type_());
-            if Some(my_section) != last_section {
-                if last_section.is_some() {
-                    w.write_str(ITEM_TABLE_CLOSE)?;
-                }
-                last_section = Some(my_section);
-                let section_id = my_section.id();
-                let tag =
-                    if section_id == "reexports" { REEXPORTS_TABLE_OPEN } else { ITEM_TABLE_OPEN };
-                write!(
-                    w,
-                    "{}",
-                    write_section_heading(my_section.name(), &cx.derive_id(section_id), None, tag)
-                )?;
-            }
-
-            match myitem.kind {
-                clean::ExternCrateItem { ref src } => {
-                    use crate::html::format::print_anchor;
+        let mut types = not_stripped_items.keys().copied().collect::<Vec<_>>();
+        types.sort_unstable_by(|a, b| reorder(*a).cmp(&reorder(*b)));
 
-                    match *src {
-                        Some(src) => {
-                            write!(
-                                w,
-                                "<dt><code>{}extern crate {} as {};",
-                                visibility_print_with_space(myitem, cx),
-                                print_anchor(myitem.item_id.expect_def_id(), src, cx),
-                                EscapeBodyTextWithWbr(myitem.name.unwrap().as_str())
-                            )?;
-                        }
-                        None => {
-                            write!(
-                                w,
-                                "<dt><code>{}extern crate {};",
-                                visibility_print_with_space(myitem, cx),
-                                print_anchor(
-                                    myitem.item_id.expect_def_id(),
-                                    myitem.name.unwrap(),
-                                    cx
-                                )
-                            )?;
-                        }
-                    }
-                    w.write_str("</code></dt>")?;
-                }
+        for type_ in types {
+            let my_section = item_ty_to_section(type_);
+            let tag = if my_section == super::ItemSection::Reexports {
+                REEXPORTS_TABLE_OPEN
+            } else {
+                ITEM_TABLE_OPEN
+            };
+            write!(
+                w,
+                "{}",
+                write_section_heading(my_section.name(), &cx.derive_id(my_section.id()), None, tag)
+            )?;
 
-                clean::ImportItem(ref import) => {
-                    let stab_tags = import.source.did.map_or_else(String::new, |import_def_id| {
-                        print_extra_info_tags(tcx, myitem, item, Some(import_def_id)).to_string()
-                    });
+            for (_, myitem) in &not_stripped_items[&type_] {
+                match myitem.kind {
+                    clean::ExternCrateItem { ref src } => {
+                        use crate::html::format::print_anchor;
 
-                    let id = match import.kind {
-                        clean::ImportKind::Simple(s) => {
-                            format!(" id=\"{}\"", cx.derive_id(format!("reexport.{s}")))
+                        match *src {
+                            Some(src) => {
+                                write!(
+                                    w,
+                                    "<dt><code>{}extern crate {} as {};",
+                                    visibility_print_with_space(myitem, cx),
+                                    print_anchor(myitem.item_id.expect_def_id(), src, cx),
+                                    EscapeBodyTextWithWbr(myitem.name.unwrap().as_str())
+                                )?;
+                            }
+                            None => {
+                                write!(
+                                    w,
+                                    "<dt><code>{}extern crate {};",
+                                    visibility_print_with_space(myitem, cx),
+                                    print_anchor(
+                                        myitem.item_id.expect_def_id(),
+                                        myitem.name.unwrap(),
+                                        cx
+                                    )
+                                )?;
+                            }
                         }
-                        clean::ImportKind::Glob => String::new(),
-                    };
-                    write!(
-                        w,
-                        "<dt{id}>\
-                            <code>"
-                    )?;
-                    render_attributes_in_code(w, myitem, "", cx);
-                    write!(
-                        w,
-                        "{vis}{imp}</code>{stab_tags}\
-                        </dt>",
-                        vis = visibility_print_with_space(myitem, cx),
-                        imp = import.print(cx)
-                    )?;
-                }
-
-                _ => {
-                    if myitem.name.is_none() {
-                        continue;
                     }
-
-                    let unsafety_flag = match myitem.kind {
-                        clean::FunctionItem(_) | clean::ForeignFunctionItem(..)
-                            if myitem.fn_header(tcx).unwrap().safety
-                                == hir::HeaderSafety::Normal(hir::Safety::Unsafe) =>
-                        {
-                            "<sup title=\"unsafe function\">⚠</sup>"
-                        }
-                        clean::ForeignStaticItem(_, hir::Safety::Unsafe) => {
-                            "<sup title=\"unsafe static\">⚠</sup>"
-                        }
-                        _ => "",
-                    };
-
-                    let visibility_and_hidden = match myitem.visibility(tcx) {
-                        Some(ty::Visibility::Restricted(_)) => {
-                            if myitem.is_doc_hidden() {
-                                // Don't separate with a space when there are two of them
-                                "<span title=\"Restricted Visibility\">&nbsp;🔒</span><span title=\"Hidden item\">👻</span> "
-                            } else {
-                                "<span title=\"Restricted Visibility\">&nbsp;🔒</span> "
+                    clean::ImportItem(ref import) => {
+                        let stab_tags =
+                            import.source.did.map_or_else(String::new, |import_def_id| {
+                                print_extra_info_tags(tcx, myitem, item, Some(import_def_id))
+                                    .to_string()
+                            });
+                        let id = match import.kind {
+                            clean::ImportKind::Simple(s) => {
+                                format!(" id=\"{}\"", cx.derive_id(format!("reexport.{s}")))
                             }
+                            clean::ImportKind::Glob => String::new(),
+                        };
+                        write!(
+                            w,
+                            "<dt{id}>\
+                                <code>"
+                        )?;
+                        render_attributes_in_code(w, myitem, "", cx);
+                        write!(
+                            w,
+                            "{vis}{imp}</code>{stab_tags}\
+                            </dt>",
+                            vis = visibility_print_with_space(myitem, cx),
+                            imp = import.print(cx)
+                        )?;
+                    }
+                    _ => {
+                        if myitem.name.is_none() {
+                            continue;
                         }
-                        _ if myitem.is_doc_hidden() => {
-                            "<span title=\"Hidden item\">&nbsp;👻</span> "
-                        }
-                        _ => "",
-                    };
 
-                    let docs =
-                        MarkdownSummaryLine(&myitem.doc_value(), &myitem.links(cx)).into_string();
-                    let (docs_before, docs_after) =
-                        if docs.is_empty() { ("", "") } else { ("<dd>", "</dd>") };
-                    write!(
-                        w,
-                        "<dt>\
-                            <a class=\"{class}\" href=\"{href}\" title=\"{title1} {title2}\">\
-                            {name}\
-                            </a>\
-                            {visibility_and_hidden}\
-                            {unsafety_flag}\
-                            {stab_tags}\
-                        </dt>\
-                        {docs_before}{docs}{docs_after}",
-                        name = EscapeBodyTextWithWbr(myitem.name.unwrap().as_str()),
-                        visibility_and_hidden = visibility_and_hidden,
-                        stab_tags = print_extra_info_tags(tcx, myitem, item, None),
-                        class = myitem.type_(),
-                        unsafety_flag = unsafety_flag,
-                        href = print_item_path(myitem.type_(), myitem.name.unwrap().as_str()),
-                        title1 = myitem.type_(),
-                        title2 = full_path(cx, myitem),
-                    )?;
+                        let unsafety_flag = match myitem.kind {
+                            clean::FunctionItem(_) | clean::ForeignFunctionItem(..)
+                                if myitem.fn_header(tcx).unwrap().safety
+                                    == hir::HeaderSafety::Normal(hir::Safety::Unsafe) =>
+                            {
+                                "<sup title=\"unsafe function\">⚠</sup>"
+                            }
+                            clean::ForeignStaticItem(_, hir::Safety::Unsafe) => {
+                                "<sup title=\"unsafe static\">⚠</sup>"
+                            }
+                            _ => "",
+                        };
+                        let visibility_and_hidden = match myitem.visibility(tcx) {
+                            Some(ty::Visibility::Restricted(_)) => {
+                                if myitem.is_doc_hidden() {
+                                    // Don't separate with a space when there are two of them
+                                    "<span title=\"Restricted Visibility\">&nbsp;🔒</span><span title=\"Hidden item\">👻</span> "
+                                } else {
+                                    "<span title=\"Restricted Visibility\">&nbsp;🔒</span> "
+                                }
+                            }
+                            _ if myitem.is_doc_hidden() => {
+                                "<span title=\"Hidden item\">&nbsp;👻</span> "
+                            }
+                            _ => "",
+                        };
+
+                        let docs = MarkdownSummaryLine(&myitem.doc_value(), &myitem.links(cx))
+                            .into_string();
+                        let (docs_before, docs_after) =
+                            if docs.is_empty() { ("", "") } else { ("<dd>", "</dd>") };
+                        write!(
+                            w,
+                            "<dt>\
+                                <a class=\"{class}\" href=\"{href}\" title=\"{title1} {title2}\">\
+                                {name}\
+                                </a>\
+                                {visibility_and_hidden}\
+                                {unsafety_flag}\
+                                {stab_tags}\
+                            </dt>\
+                            {docs_before}{docs}{docs_after}",
+                            name = EscapeBodyTextWithWbr(myitem.name.unwrap().as_str()),
+                            visibility_and_hidden = visibility_and_hidden,
+                            stab_tags = print_extra_info_tags(tcx, myitem, item, None),
+                            class = type_,
+                            unsafety_flag = unsafety_flag,
+                            href = print_item_path(type_, myitem.name.unwrap().as_str()),
+                            title1 = myitem.type_(),
+                            title2 = full_path(cx, myitem),
+                        )?;
+                    }
                 }
             }
-        }
-
-        if last_section.is_some() {
             w.write_str(ITEM_TABLE_CLOSE)?;
         }
+
         Ok(())
     })
 }