Rollup merge of rust-lang#157233 - nnethercote:rustdoc-impl-order, r=GuillaumeGomez

rustdoc: Fix trait impl ordering

For types, rustdoc produces different impl orderings in the sidebar vs. the main section.

E.g. for `std::cell::UnsafeCell` the "Trait Implementations" sidebar order is this:
```
!Freeze
!RefUnwindSafe
!Sync
CoerceUnsized<UnsafeCell<U>>
Debug
Default
DispatchFromDyn<UnsafeCell<U>>
From<T>
```
The primary sort is on polarity, and the secondary sort is alphabetical. Makes sense.

The main section order is this:
```
impl<T> Debug for UnsafeCell<T>
impl<T> Default for UnsafeCell<T>
impl<T> From<T> for UnsafeCell<T>
impl<T, U> CoerceUnsized<UnsafeCell<U>> for UnsafeCell<T>
impl<T, U> DispatchFromDyn<UnsafeCell<U>> for UnsafeCell<T>
impl<T> !Freeze for UnsafeCell<T>
impl<T> !RefUnwindSafe for UnsafeCell<T>
impl<T> !Sync for UnsafeCell<T>
```
This strange ordering occurs because the entries are sorted on the generated HTML. Impls with methods (the first three) come first because they start with a `<details>` tag while the others start with a `<section>` tag. After that, the order depends on a section id of the form `impl-{trait}-for-{type}` that doesn't include the polarity, which is why the secondary ordering is alphabetical but ignores the `!`.

The sidebar ordering is the better of the two. This commit changes the main section to use the same ordering as the sidebar. This involves creating a new function `impl_trait_key` shared between the two parts.

r? @GuillaumeGomez

Author: JonathanBrouwer

src/librustdoc/html/render/mod.rs

@@ -79,6 +79,7 @@ use crate::html::format::{
 use crate::html::markdown::{
     HeadingOffset, IdMap, Markdown, MarkdownItemInfo, MarkdownSummaryLine, short_markdown_summary,
 };
+use crate::html::render::print_item::compare_names;
 use crate::html::render::search_index::get_function_type_for_search;
 use crate::html::static_files::SCRAPE_EXAMPLES_HELP_MD;
 use crate::html::{highlight, sources};
@@ -941,6 +942,16 @@ fn short_item_info(
     extra_info
 }
 
+// Prints the polarity and path of an impl's trait, if it has one, e.g. `Send`, `!Sync`.
+fn impl_trait_key(cx: &Context<'_>, i: &Impl) -> Option<String> {
+    let trait_ = i.inner_impl().trait_.as_ref()?;
+    let prefix = match i.inner_impl().polarity {
+        ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "",
+        ty::ImplPolarity::Negative => "!",
+    };
+    Some(format!("{prefix}{:#}", print_path(trait_, cx)))
+}
+
 // Render the list of items inside one of the sections "Trait Implementations",
 // "Auto Trait Implementations," "Blanket Trait Implementations" (on struct/enum pages).
 fn render_impls(
@@ -950,7 +961,9 @@ fn render_impls(
     containing_item: &clean::Item,
     toggle_open_by_default: bool,
 ) -> fmt::Result {
-    let mut rendered_impls = impls
+    // Render each impl alongside its `impl_trait_key`, which is used as the primary sorting key
+    // to match the impl order in the sidebar.
+    let mut keyed_rendered_impls = impls
         .iter()
         .map(|i| {
             let did = i.trait_did().unwrap();
@@ -971,11 +984,16 @@ fn render_impls(
                     toggle_open_by_default,
                 },
             );
-            imp.to_string()
+            (impl_trait_key(cx, i).unwrap(), imp.to_string())
         })
         .collect::<Vec<_>>();
-    rendered_impls.sort();
-    w.write_str(&rendered_impls.join(""))
+
+    // Sort and then remove the `impl_trait_key`s, which are no longer needed after sorting.
+    keyed_rendered_impls
+        .sort_by(|(k1, h1), (k2, h2)| compare_names(k1, k2).then_with(|| h1.cmp(h2)));
+    let joined: String = keyed_rendered_impls.into_iter().map(|a| a.1).collect();
+
+    w.write_str(&joined)
 }
 
 /// Build a (possibly empty) `href` attribute (a key-value pair) for the given associated item.

src/librustdoc/html/render/sidebar.rs

@@ -6,10 +6,10 @@ use askama::Template;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::{DefIdMap, DefIdSet};
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::TyCtxt;
 use tracing::debug;
 
-use super::{Context, ItemSection, item_ty_to_section};
+use super::{Context, ItemSection, impl_trait_key, item_ty_to_section};
 use crate::clean;
 use crate::formats::Impl;
 use crate::formats::item_type::ItemType;
@@ -707,15 +707,9 @@ fn sidebar_render_assoc_items(
 
         let mut ret = impls
             .iter()
-            .filter_map(|it| {
-                let trait_ = it.inner_impl().trait_.as_ref()?;
-                let encoded = id_map.derive(super::get_id_for_impl(cx.tcx(), it.impl_item.item_id));
-
-                let prefix = match it.inner_impl().polarity {
-                    ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "",
-                    ty::ImplPolarity::Negative => "!",
-                };
-                let generated = Link::new(encoded, format!("{prefix}{:#}", print_path(trait_, cx)));
+            .filter_map(|i| {
+                let encoded = id_map.derive(super::get_id_for_impl(cx.tcx(), i.impl_item.item_id));
+                let generated = Link::new(encoded, impl_trait_key(cx, i)?);
                 if links.insert(generated.clone()) { Some(generated) } else { None }
             })
             .collect::<Vec<Link<'static>>>();

tests/rustdoc-gui/search-tab.goml

@@ -81,7 +81,7 @@ set-window-size: (851, 600)
 // Check the size and count in tabs
 assert-text: ("#search-tabs > button:nth-child(1) > .count", " (25) ")
 assert-text: ("#search-tabs > button:nth-child(2) > .count", " (7)  ")
-assert-text: ("#search-tabs > button:nth-child(3) > .count", " (0)  ")
+assert-text: ("#search-tabs > button:nth-child(3) > .count", " (1)  ")
 store-property: ("#search-tabs > button:nth-child(1)", {"offsetWidth": buttonWidth})
 assert-property: ("#search-tabs > button:nth-child(2)", {"offsetWidth": |buttonWidth|})
 assert-property: ("#search-tabs > button:nth-child(3)", {"offsetWidth": |buttonWidth|})

tests/rustdoc-gui/sidebar-foreign-impl-sort.goml

@@ -1,4 +1,4 @@
-// Checks sidebar resizing close the Settings popover
+// Checks sidebar foreign impl ordering
 go-to: "file://" + |DOC_PATH| + "/test_docs/SidebarSort/trait.Sort.html#foreign-impls"
 
 // Check that the sidebar contains the expected foreign implementations

tests/rustdoc-gui/src/test_docs/lib.rs

@@ -10,6 +10,7 @@
 #![feature(associated_type_defaults)]
 #![feature(macro_attr)]
 #![feature(macro_derive)]
+#![feature(negative_impls)]
 
 /*!
 Enable the feature <span class="stab portability"><code>some-feature</code></span> to enjoy
@@ -89,6 +90,19 @@ impl AsRef<str> for Foo {
     }
 }
 
+unsafe impl Send for Foo {}
+impl !Sync for Foo {}
+
+impl From<u8> for Foo {
+    fn from(value: u8) -> Self { todo!(); }
+}
+impl From<u16> for Foo {
+    fn from(value: u16) -> Self { todo!(); }
+}
+impl From<u32> for Foo {
+    fn from(value: u32) -> Self { todo!(); }
+}
+
 /// <div id="doc-warning-0" class="warning">I have warnings!</div>
 pub struct WarningStruct;
 

tests/rustdoc-gui/trait-impl-sort.goml

@@ -0,0 +1,19 @@
+// Check that trait impls in the sidebar and the main section have the same
+// ordering.
+
+go-to: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html"
+
+// .sidebar .trait-implementation li:nth-child(3) a
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(1) a", "!Sync")
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(2) a", "AsRef<str>")
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(3) a", "From<u8>")
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(4) a", "From<u16>")
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(5) a", "From<u32>")
+assert-text: (".sidebar-elems .trait-implementation li:nth-child(6) a", "Send")
+
+assert-text: ("#trait-implementations-list section:nth-child(1) .code-header", "impl !Sync for Foo")
+assert-text: ("#trait-implementations-list details:nth-child(2) .code-header", "impl AsRef<str> for Foo")
+assert-text: ("#trait-implementations-list details:nth-child(3) .code-header", "impl From<u8> for Foo")
+assert-text: ("#trait-implementations-list details:nth-child(4) .code-header", "impl From<u16> for Foo")
+assert-text: ("#trait-implementations-list details:nth-child(5) .code-header", "impl From<u32> for Foo")
+assert-text: ("#trait-implementations-list section:nth-child(6) .code-header", "impl Send for Foo")