Put negative implementors first and apply same ordering logic to foreign implementors

GuillaumeGomez · GuillaumeGomez · commit cbb9e9b21961 · 2025-06-11T23:26:13.000+02:00
diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs
@@ -76,4 +76,8 @@ impl Impl {
         };
         true
     }
+
+    pub(crate) fn is_negative_trait_impl(&self) -> bool {
+        self.inner_impl().is_negative_trait_impl()
+    }
 }
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
@@ -643,6 +643,25 @@ fn item_function(cx: &Context<'_>, it: &clean::Item, f: &clean::Function) -> imp
     })
 }
 
+/// Struct used to handle insertion of "negative impl" marker in the generated DOM.
+struct NegativeMarker {
+    inserted_negative_marker: bool,
+}
+
+impl NegativeMarker {
+    fn new() -> Self {
+        Self { inserted_negative_marker: false }
+    }
+
+    fn insert_if_needed(&mut self, w: &mut fmt::Formatter<'_>, implementor: &Impl) -> fmt::Result {
+        if !self.inserted_negative_marker && !implementor.is_negative_trait_impl() {
+            write!(w, "<div class=\"negative-marker\"></div>")?;
+            self.inserted_negative_marker = true;
+        }
+        Ok(())
+    }
+}
+
 fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt::Display {
     fmt::from_fn(|w| {
         let tcx = cx.tcx();
@@ -1069,7 +1088,9 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt:
                     "<div id=\"implementors-list\">",
                 )
             )?;
+            let mut negative_marker = NegativeMarker::new();
             for implementor in concrete {
+                negative_marker.insert_if_needed(w, implementor)?;
                 write!(w, "{}", render_implementor(cx, implementor, it, &implementor_dups, &[]))?;
             }
             w.write_str("</div>")?;
@@ -1085,7 +1106,9 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt:
                         "<div id=\"synthetic-implementors-list\">",
                     )
                 )?;
+                let mut negative_marker = NegativeMarker::new();
                 for implementor in synthetic {
+                    negative_marker.insert_if_needed(w, implementor)?;
                     write!(
                         w,
                         "{}",
@@ -2302,11 +2325,18 @@ where
 }
 
 #[derive(PartialEq, Eq)]
-struct ImplString(String);
+struct ImplString {
+    rendered: String,
+    is_negative: bool,
+}
 
 impl ImplString {
     fn new(i: &Impl, cx: &Context<'_>) -> ImplString {
-        ImplString(format!("{}", i.inner_impl().print(false, cx)))
+        let impl_ = i.inner_impl();
+        ImplString {
+            is_negative: impl_.is_negative_trait_impl(),
+            rendered: format!("{}", impl_.print(false, cx)),
+        }
     }
 }
 
@@ -2318,7 +2348,11 @@ impl PartialOrd for ImplString {
 
 impl Ord for ImplString {
     fn cmp(&self, other: &Self) -> Ordering {
-        compare_names(&self.0, &other.0)
+        match (self.is_negative, other.is_negative) {
+            (false, true) => Ordering::Greater,
+            (true, false) => Ordering::Less,
+            _ => compare_names(&self.rendered, &other.rendered),
+        }
     }
 }
 
diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs
@@ -14,6 +14,7 @@
 //!    or contains "invocation-specific".
 
 use std::cell::RefCell;
+use std::cmp::Ordering;
 use std::ffi::OsString;
 use std::fs::File;
 use std::io::{self, Write as _};
@@ -46,6 +47,7 @@ use crate::formats::Impl;
 use crate::formats::item_type::ItemType;
 use crate::html::layout;
 use crate::html::render::ordered_json::{EscapedJson, OrderedJson};
+use crate::html::render::print_item::compare_names;
 use crate::html::render::search_index::{SerializedSearchIndex, build_index};
 use crate::html::render::sorted_template::{self, FileFormat, SortedTemplate};
 use crate::html::render::{AssocItemLink, ImplRenderingParameters, StylePath};
@@ -703,7 +705,7 @@ impl TraitAliasPart {
     fn blank() -> SortedTemplate<<Self as CciPart>::FileFormat> {
         SortedTemplate::from_before_after(
             r"(function() {
-    var implementors = Object.fromEntries([",
+    const implementors = Object.fromEntries([",
             r"]);
     if (window.register_implementors) {
         window.register_implementors(implementors);
@@ -741,7 +743,7 @@ impl TraitAliasPart {
                 },
             };
 
-            let implementors = imps
+            let mut implementors = imps
                 .iter()
                 .filter_map(|imp| {
                     // If the trait and implementation are in the same crate, then
@@ -756,10 +758,12 @@ impl TraitAliasPart {
                     {
                         None
                     } else {
+                        let impl_ = imp.inner_impl();
                         Some(Implementor {
-                            text: imp.inner_impl().print(false, cx).to_string(),
+                            text: impl_.print(false, cx).to_string(),
                             synthetic: imp.inner_impl().kind.is_auto(),
                             types: collect_paths_for_type(&imp.inner_impl().for_, cache),
+                            is_negative: impl_.is_negative_trait_impl(),
                         })
                     }
                 })
@@ -778,7 +782,9 @@ impl TraitAliasPart {
             }
             path.push(format!("{remote_item_type}.{}.js", remote_path[remote_path.len() - 1]));
 
-            let part = OrderedJson::array_sorted(
+            implementors.sort_unstable();
+
+            let part = OrderedJson::array_unsorted(
                 implementors
                     .iter()
                     .map(OrderedJson::serialize)
@@ -791,10 +797,12 @@ impl TraitAliasPart {
     }
 }
 
+#[derive(PartialEq, Eq)]
 struct Implementor {
     text: String,
     synthetic: bool,
     types: Vec<String>,
+    is_negative: bool,
 }
 
 impl Serialize for Implementor {
@@ -804,6 +812,7 @@ impl Serialize for Implementor {
     {
         let mut seq = serializer.serialize_seq(None)?;
         seq.serialize_element(&self.text)?;
+        seq.serialize_element(if self.is_negative { &1 } else { &0 })?;
         if self.synthetic {
             seq.serialize_element(&1)?;
             seq.serialize_element(&self.types)?;
@@ -812,6 +821,22 @@ impl Serialize for Implementor {
     }
 }
 
+impl PartialOrd for Implementor {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(Ord::cmp(self, other))
+    }
+}
+
+impl Ord for Implementor {
+    fn cmp(&self, other: &Self) -> Ordering {
+        match (self.is_negative, other.is_negative) {
+            (false, true) => Ordering::Greater,
+            (true, false) => Ordering::Less,
+            _ => compare_names(&self.text, &other.text),
+        }
+    }
+}
+
 /// Collect the list of aliased types and their aliases.
 /// <https://github.com/search?q=repo%3Arust-lang%2Frust+[RUSTDOCIMPL]+type.impl&type=code>
 ///
diff --git a/src/librustdoc/html/render/write_shared/tests.rs b/src/librustdoc/html/render/write_shared/tests.rs
@@ -113,7 +113,7 @@ fn trait_alias_template() {
     assert_eq!(
         but_last_line(&template.to_string()),
         r#"(function() {
-    var implementors = Object.fromEntries([]);
+    const implementors = Object.fromEntries([]);
     if (window.register_implementors) {
         window.register_implementors(implementors);
     } else {
@@ -125,7 +125,7 @@ fn trait_alias_template() {
     assert_eq!(
         but_last_line(&template.to_string()),
         r#"(function() {
-    var implementors = Object.fromEntries([["a"]]);
+    const implementors = Object.fromEntries([["a"]]);
     if (window.register_implementors) {
         window.register_implementors(implementors);
     } else {
@@ -137,7 +137,7 @@ fn trait_alias_template() {
     assert_eq!(
         but_last_line(&template.to_string()),
         r#"(function() {
-    var implementors = Object.fromEntries([["a"],["b"]]);
+    const implementors = Object.fromEntries([["a"],["b"]]);
     if (window.register_implementors) {
         window.register_implementors(implementors);
     } else {
diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css
@@ -2776,6 +2776,9 @@ in src-script.js and main.js
 {
 	margin-bottom: 0.75em;
 }
+.negative-marker {
+	display: none;
+}
 
 .variants > .docblock,
 .implementors-toggle > .docblock,
diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js
@@ -674,21 +674,34 @@ function preLoadCss(cssUrl) {
 
     // <https://github.com/search?q=repo%3Arust-lang%2Frust+[RUSTDOCIMPL]+trait.impl&type=code>
     window.register_implementors = imp => {
-        const implementors = document.getElementById("implementors-list");
-        const synthetic_implementors = document.getElementById("synthetic-implementors-list");
+        /** Takes an ID as input and returns a list of two elements. The first element is the DOM
+         * element with the given ID and the second is the "negative marker", meaning the location
+         * between the negative and non-negative impls.
+         *
+         * @param {string} id: ID of the DOM element.
+         *
+         * @return {HTMLElement}
+         */
+        function implementorsElems(id) {
+            const elem = document.getElementById(id);
+            return [elem, elem ? elem.querySelector(".negative-marker") : null];
+        }
+        const implementors = implementorsElems("implementors-list");
+        const synthetic_implementors = implementorsElems("synthetic-implementors-list");
         const inlined_types = new Set();
 
         const TEXT_IDX = 0;
-        const SYNTHETIC_IDX = 1;
-        const TYPES_IDX = 2;
+        const IS_NEG_IDX = 1;
+        const SYNTHETIC_IDX = 2;
+        const TYPES_IDX = 3;
 
-        if (synthetic_implementors) {
+        if (synthetic_implementors[0]) {
             // This `inlined_types` variable is used to avoid having the same implementation
             // showing up twice. For example "String" in the "Sync" doc page.
             //
             // By the way, this is only used by and useful for traits implemented automatically
             // (like "Send" and "Sync").
-            onEachLazy(synthetic_implementors.getElementsByClassName("impl"), el => {
+            onEachLazy(synthetic_implementors[0].getElementsByClassName("impl"), el => {
                 const aliases = el.getAttribute("data-aliases");
                 if (!aliases) {
                     return;
@@ -701,7 +714,7 @@ function preLoadCss(cssUrl) {
         }
 
         // @ts-expect-error
-        let currentNbImpls = implementors.getElementsByClassName("impl").length;
+        let currentNbImpls = implementors[0].getElementsByClassName("impl").length;
         // @ts-expect-error
         const traitName = document.querySelector(".main-heading h1 > .trait").textContent;
         const baseIdName = "impl-" + traitName + "-";
@@ -758,8 +771,16 @@ function preLoadCss(cssUrl) {
                 addClass(display, "impl");
                 display.appendChild(anchor);
                 display.appendChild(code);
-                // @ts-expect-error
-                list.appendChild(display);
+
+                // If this is a negative implementor, we put it into the right location (just
+                // before the negative impl marker).
+                if (struct[IS_NEG_IDX]) {
+                    // @ts-expect-error
+                    list[1].before(display);
+                } else {
+                    // @ts-expect-error
+                    list[0].appendChild(display);
+                }
                 currentNbImpls += 1;
             }
         }
@@ -1622,7 +1643,7 @@ function preLoadCss(cssUrl) {
      * @param {boolean} buildNeeded - If this argument is `false`, the help menu element won't be
      *                                built if it doesn't exist.
      *
-     * @return {HTMLElement}
+     * @return {Array<HTMLElement>}
      */
     function getHelpMenu(buildNeeded) {
         // @ts-expect-error

Original file line number	Diff line number	Diff line change
`@@ -76,4 +76,8 @@ impl Impl {`
`76`	`76`	`};`
`77`	`77`	`true`
`78`	`78`	`}`
	`79`	`+`
	`80`	`+ pub(crate) fn is_negative_trait_impl(&self) -> bool {`
	`81`	`+ self.inner_impl().is_negative_trait_impl()`
	`82`	`+ }`
`79`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2776,6 +2776,9 @@ in src-script.js and main.js`
`2776`	`2776`	`{`
`2777`	`2777`	`margin-bottom: 0.75em;`
`2778`	`2778`	`}`
	`2779`	`+.negative-marker {`
	`2780`	`+ display: none;`
	`2781`	`+}`
`2779`	`2782`
`2780`	`2783`	`.variants > .docblock,`
`2781`	`2784`	`.implementors-toggle > .docblock,`