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

Author: GuillaumeGomez

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()
+    }
 }

src/librustdoc/html/render/print_item.rs

@@ -646,6 +646,27 @@ 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.
+///
+/// This marker appears once in all trait impl lists to divide negative impls from positive impls.
+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();
@@ -1072,7 +1093,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>")?;
@@ -1088,7 +1111,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 +2327,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!("{}", print_impl(i.inner_impl(), false, cx)))
+        let impl_ = i.inner_impl();
+        ImplString {
+            is_negative: impl_.is_negative_trait_impl(),
+            rendered: format!("{}", print_impl(impl_, false, cx)),
+        }
     }
 }
 
@@ -2318,7 +2350,12 @@ impl PartialOrd for ImplString {
 
 impl Ord for ImplString {
     fn cmp(&self, other: &Self) -> Ordering {
-        compare_names(&self.0, &other.0)
+        // We sort negative impls first.
+        match (self.is_negative, other.is_negative) {
+            (false, true) => Ordering::Greater,
+            (true, false) => Ordering::Less,
+            _ => compare_names(&self.rendered, &other.rendered),
+        }
     }
 }
 

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::{OsStr, OsString};
 use std::fs::File;
 use std::io::{self, Write as _};
@@ -47,6 +48,7 @@ use crate::formats::item_type::ItemType;
 use crate::html::format::{print_impl, print_path};
 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};
@@ -667,7 +669,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);
@@ -720,10 +722,12 @@ impl TraitAliasPart {
                     {
                         None
                     } else {
+                        let impl_ = imp.inner_impl();
                         Some(Implementor {
-                            text: print_impl(imp.inner_impl(), false, cx).to_string(),
+                            text: print_impl(impl_, 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(),
                         })
                     }
                 })
@@ -742,19 +746,28 @@ impl TraitAliasPart {
             }
             path.push(format!("{remote_item_type}.{}.js", remote_path[remote_path.len() - 1]));
 
-            let part = OrderedJson::array_sorted(
-                implementors.map(|implementor| OrderedJson::serialize(implementor).unwrap()),
+            let mut implementors = implementors.collect::<Vec<_>>();
+            implementors.sort_unstable();
+
+            let part = OrderedJson::array_unsorted(
+                implementors
+                    .iter()
+                    .map(OrderedJson::serialize)
+                    .collect::<Result<Vec<_>, _>>()
+                    .unwrap(),
             );
             path_parts.push(path, OrderedJson::array_unsorted([crate_name_json, &part]));
         }
         Ok(path_parts)
     }
 }
 
+#[derive(Eq)]
 struct Implementor {
     text: String,
     synthetic: bool,
     types: Vec<String>,
+    is_negative: bool,
 }
 
 impl Serialize for Implementor {
@@ -764,6 +777,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)?;
@@ -772,6 +786,29 @@ impl Serialize for Implementor {
     }
 }
 
+impl PartialEq for Implementor {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == Ordering::Equal
+    }
+}
+
+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 {
+        // We sort negative impls first.
+        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>
 ///

src/librustdoc/html/render/write_shared/tests.rs

@@ -68,7 +68,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 {
@@ -80,7 +80,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 {
@@ -92,7 +92,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 {

src/librustdoc/html/static/css/rustdoc.css

@@ -2976,6 +2976,9 @@ in src-script.js and main.js
 {
 	margin-bottom: 0.75em;
 }
+.negative-marker {
+	display: none;
+}
 
 .variants > .docblock,
 .implementors-toggle > .docblock,

src/librustdoc/html/static/js/main.js

@@ -800,21 +800,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|null, HTMLElement|null]}
+         */
+        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;
@@ -827,7 +840,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 + "-";
@@ -884,8 +897,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;
             }
         }

src/librustdoc/html/static/js/rustdoc.d.ts

@@ -520,7 +520,7 @@ declare namespace rustdoc {
      * Provided by generated `trait.impl` files.
      */
     type Implementors = {
-        [key: string]: Array<[string, number, Array<string>]>
+        [key: string]: Array<[string, 0|1, number, Array<string>]>
     }
 
     type TypeImpls = {