diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs index 2e8b5d9f59489..80c110ec07f56 100644 --- 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 index 515424cbef19b..1d2cf3c9dff95 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -643,6 +643,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, "
")?; + 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 +1090,9 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt: "
", ) )?; + 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("
")?; @@ -1085,7 +1108,9 @@ fn item_trait(cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) -> impl fmt: "
", ) )?; + 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!("{}", 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 +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), + } } } diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 606a911390870..5cc4a43d6ae88 100644 --- 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}; @@ -695,7 +697,7 @@ impl TraitAliasPart { fn blank() -> SortedTemplate<::FileFormat> { SortedTemplate::from_before_after( r"(function() { - var implementors = Object.fromEntries([", + const implementors = Object.fromEntries([", r"]); if (window.register_implementors) { window.register_implementors(implementors); @@ -748,10 +750,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(), }) } }) @@ -770,8 +774,15 @@ 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::>(); + implementors.sort_unstable(); + + let part = OrderedJson::array_unsorted( + implementors + .iter() + .map(OrderedJson::serialize) + .collect::, _>>() + .unwrap(), ); path_parts.push(path, OrderedJson::array_unsorted([crate_name_json, &part])); } @@ -779,10 +790,12 @@ impl TraitAliasPart { } } +#[derive(Eq)] struct Implementor { text: String, synthetic: bool, types: Vec, + is_negative: bool, } impl Serialize for Implementor { @@ -792,6 +805,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)?; @@ -800,6 +814,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 { + 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. /// /// diff --git a/src/librustdoc/html/render/write_shared/tests.rs b/src/librustdoc/html/render/write_shared/tests.rs index 6f185e85345bc..882b201433b0b 100644 --- a/src/librustdoc/html/render/write_shared/tests.rs +++ b/src/librustdoc/html/render/write_shared/tests.rs @@ -101,7 +101,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 { @@ -113,7 +113,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 { @@ -125,7 +125,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/noscript.css b/src/librustdoc/html/static/css/noscript.css index a3c6bf9816169..7b8cb39e71078 100644 --- a/src/librustdoc/html/static/css/noscript.css +++ b/src/librustdoc/html/static/css/noscript.css @@ -29,6 +29,10 @@ nav.sub { display: none; } +#synthetic-implementors-list, #implementors-list { + display: block; +} + /* Begin: styles for themes Keep the default light and dark themes synchronized with the ones in rustdoc.css */ diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 7be83b65fbfaf..d875f9beebdad 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1125,6 +1125,12 @@ div.where { margin-left: calc(var(--docblock-indent) + var(--impl-items-indent)); } +#synthetic-implementors-list, #implementors-list { + /* To prevent layout shift when loading the page with extra implementors being loaded + from JS, we hide the list until it's complete. */ + display: none; +} + .item-info code { font-size: 0.875rem; } @@ -2776,6 +2782,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 index 7b1a61a3ffa45..4efcd6e21282b 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -674,21 +674,34 @@ function preLoadCss(cssUrl) { // 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 syntheticImplementors = 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 (syntheticImplementors[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(syntheticImplementors[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 + "-"; @@ -723,7 +736,7 @@ function preLoadCss(cssUrl) { struct_loop: for (const struct of structs) { - const list = struct[SYNTHETIC_IDX] ? synthetic_implementors : implementors; + const list = struct[SYNTHETIC_IDX] ? syntheticImplementors : implementors; // The types list is only used for synthetic impls. // If this changes, `main.js` and `write_shared.rs` both need changed. @@ -758,11 +771,25 @@ 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; } } + if (implementors[0]) { + implementors[0].style.display = "block"; + } + if (syntheticImplementors[0]) { + syntheticImplementors[0].style.display = "block"; + } }; if (window.pending_implementors) { window.register_implementors(window.pending_implementors); diff --git a/src/librustdoc/html/static/js/rustdoc.d.ts b/src/librustdoc/html/static/js/rustdoc.d.ts index 6af16441de88b..6b7a86db19b48 100644 --- a/src/librustdoc/html/static/js/rustdoc.d.ts +++ b/src/librustdoc/html/static/js/rustdoc.d.ts @@ -465,7 +465,7 @@ declare namespace rustdoc { * Provied by generated `trait.impl` files. */ type Implementors = { - [key: string]: Array<[string, number, Array]> + [key: string]: Array<[string, 0|1, number, Array]> } type TypeImpls = { diff --git a/tests/rustdoc-gui/implementors.goml b/tests/rustdoc-gui/implementors.goml index b39b95c1a9bff..d4542c6f78172 100644 --- a/tests/rustdoc-gui/implementors.goml +++ b/tests/rustdoc-gui/implementors.goml @@ -1,19 +1,46 @@ // The goal of this test is to check that the external trait implementors, generated with JS, // have the same display than the "local" ones. go-to: "file://" + |DOC_PATH| + "/implementors/trait.Whatever.html" -assert: "#implementors-list" -// There are supposed to be two implementors listed. -assert-count: ("#implementors-list .impl", 2) +wait-for-css: ("#implementors-list", {"display": "block"}) +// There are supposed to be four implementors listed. +assert-count: ("#implementors-list .impl", 4) +// There are supposed to be two non-negative implementors. +assert-count: ("#implementors-list .negative-marker ~ *", 2) // Now we check that both implementors have an anchor, an ID and a similar DOM. -assert: ("#implementors-list .impl:nth-child(1) > a.anchor") -assert-attribute: ("#implementors-list .impl:nth-child(1)", {"id": "impl-Whatever-for-Struct"}) -assert-attribute: ("#implementors-list .impl:nth-child(1) > a.anchor", {"href": "#impl-Whatever-for-Struct"}) -assert: "#implementors-list .impl:nth-child(1) > .code-header" +define-function: ( + "check-dom", + [id], + block { + assert-attribute: (|id| + " > a.anchor", {"href": |id|}) + assert: |id| + " > .code-header" + }, +) -assert: ("#implementors-list .impl:nth-child(2) > a.anchor") -assert-attribute: ("#implementors-list .impl:nth-child(2)", {"id": "impl-Whatever-1"}) -assert-attribute: ("#implementors-list .impl:nth-child(2) > a.anchor", {"href": "#impl-Whatever-1"}) -assert: "#implementors-list .impl:nth-child(2) > .code-header" +call-function: ("check-dom", {"id": "#impl-Whatever-for-Struct2"}) +call-function: ("check-dom", {"id": "#impl-Whatever-2"}) +call-function: ("check-dom", {"id": "#impl-Whatever-for-Struct"}) +call-function: ("check-dom", {"id": "#impl-Whatever-3"}) + +// Ensure that negative impl are sorted first. +assert-property: ( + "#implementors-list > *:nth-child(1) > h3", + {"textContent": "impl !Whatever for Struct2"}, +) +assert-property: ( + "#implementors-list > *:nth-child(2) > h3", + {"textContent": "impl !Whatever for StructToImplOnReexport"}, +) +// Third one is the negative marker. +assert-attribute: ("#implementors-list > *:nth-child(3)", {"class": "negative-marker"}) +// This one is a `` so the selector is a bit different. +assert-property: ( + "#implementors-list > *:nth-child(4) section > h3", + {"textContent": "impl Whatever for Struct"}, +) +assert-property: ( + "#implementors-list > *:nth-child(5) > h3", + {"textContent": "impl Whatever for Foo"}, +) go-to: "file://" + |DOC_PATH| + "/test_docs/struct.HasEmptyTraits.html" compare-elements-position-near-false: ( @@ -39,3 +66,8 @@ assert-count: ("#implementors-list .impl", 1) go-to: "file://" + |DOC_PATH| + "/http/trait.HttpTrait.html" assert-count: ("#implementors-list .impl", 1) assert-attribute: ("#implementors-list .impl a.trait", {"href": "../http/trait.HttpTrait.html"}) + +// Now we check that if JS is disabled, the implementors list will be visible. +javascript: false +reload: +assert-css: ("#implementors-list", {"display": "block"}) diff --git a/tests/rustdoc-gui/src/lib2/implementors/lib.rs b/tests/rustdoc-gui/src/lib2/implementors/lib.rs index 2842ac50dc1e8..f081820039275 100644 --- a/tests/rustdoc-gui/src/lib2/implementors/lib.rs +++ b/tests/rustdoc-gui/src/lib2/implementors/lib.rs @@ -1,3 +1,5 @@ +#![feature(negative_impls)] + pub trait Whatever { type Foo; @@ -5,11 +7,14 @@ pub trait Whatever { } pub struct Struct; +pub struct Struct2; impl Whatever for Struct { type Foo = u8; } +impl !Whatever for Struct2 {} + impl http::HttpTrait for Struct {} mod traits { diff --git a/tests/rustdoc-gui/src/lib2/lib.rs b/tests/rustdoc-gui/src/lib2/lib.rs index 8db754f91ce61..213c79ab4bc06 100644 --- a/tests/rustdoc-gui/src/lib2/lib.rs +++ b/tests/rustdoc-gui/src/lib2/lib.rs @@ -2,6 +2,7 @@ #![feature(doc_cfg)] #![feature(doc_auto_cfg)] +#![feature(negative_impls)] pub mod another_folder; pub mod another_mod; @@ -61,6 +62,8 @@ impl implementors::Whatever for Foo { type Foo = u32; } +impl !implementors::Whatever for StructToImplOnReexport {} + #[doc(inline)] pub use implementors::TraitToReexport;