Rollup merge of #132155 - GuillaumeGomez:impl-block-doc, r=rustdoc

Always display first line of impl blocks even when collapsed

Fixes https://github.com/rust-lang/rust/issues/130612.

It the line is too long, only the beginning will be visible:

![Screenshot from 2024-10-25 17-21-41](https://github.com/user-attachments/assets/dd2d912c-ad55-4410-8195-1d66a0a99ad4)

Otherwise, it looks like this:

![image](https://github.com/user-attachments/assets/1f40b9e0-2143-4b9d-a4b0-338a0cd740df)

Can be tested [here](https://rustdoc.crud.net/imperio/impl-block-doc/foo/struct.ImplDoc.html).

r? `@notriddle`

src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version

@@ -1 +1 @@
-0.18.1
+0.18.2

src/librustdoc/html/markdown.rs

@@ -32,6 +32,8 @@ use std::iter::Peekable;
 use std::ops::{ControlFlow, Range};
 use std::path::PathBuf;
 use std::str::{self, CharIndices};
+use std::sync::atomic::AtomicUsize;
+use std::sync::{Arc, Weak};
 
 use pulldown_cmark::{
     BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html,
@@ -1301,8 +1303,20 @@ impl LangString {
     }
 }
 
-impl Markdown<'_> {
+impl<'a> Markdown<'a> {
     pub fn into_string(self) -> String {
+        // This is actually common enough to special-case
+        if self.content.is_empty() {
+            return String::new();
+        }
+
+        let mut s = String::with_capacity(self.content.len() * 3 / 2);
+        html::push_html(&mut s, self.into_iter());
+
+        s
+    }
+
+    fn into_iter(self) -> CodeBlocks<'a, 'a, impl Iterator<Item = Event<'a>>> {
         let Markdown {
             content: md,
             links,
@@ -1313,32 +1327,72 @@ impl Markdown<'_> {
             heading_offset,
         } = self;
 
-        // This is actually common enough to special-case
-        if md.is_empty() {
-            return String::new();
-        }
-        let mut replacer = |broken_link: BrokenLink<'_>| {
+        let replacer = move |broken_link: BrokenLink<'_>| {
             links
                 .iter()
                 .find(|link| *link.original_text == *broken_link.reference)
                 .map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
         };
 
-        let p = Parser::new_with_broken_link_callback(md, main_body_opts(), Some(&mut replacer));
+        let p = Parser::new_with_broken_link_callback(md, main_body_opts(), Some(replacer));
         let p = p.into_offset_iter();
 
-        let mut s = String::with_capacity(md.len() * 3 / 2);
-
         ids.handle_footnotes(|ids, existing_footnotes| {
             let p = HeadingLinks::new(p, None, ids, heading_offset);
             let p = footnotes::Footnotes::new(p, existing_footnotes);
             let p = LinkReplacer::new(p.map(|(ev, _)| ev), links);
             let p = TableWrapper::new(p);
-            let p = CodeBlocks::new(p, codes, edition, playground);
-            html::push_html(&mut s, p);
-        });
+            CodeBlocks::new(p, codes, edition, playground)
+        })
+    }
 
-        s
+    /// Convert markdown to (summary, remaining) HTML.
+    ///
+    /// - The summary is the first top-level Markdown element (usually a paragraph, but potentially
+    ///   any block).
+    /// - The remaining docs contain everything after the summary.
+    pub(crate) fn split_summary_and_content(self) -> (Option<String>, Option<String>) {
+        if self.content.is_empty() {
+            return (None, None);
+        }
+        let mut p = self.into_iter();
+
+        let mut event_level = 0;
+        let mut summary_events = Vec::new();
+        let mut get_next_tag = false;
+
+        let mut end_of_summary = false;
+        while let Some(event) = p.next() {
+            match event {
+                Event::Start(_) => event_level += 1,
+                Event::End(kind) => {
+                    event_level -= 1;
+                    if event_level == 0 {
+                        // We're back at the "top" so it means we're done with the summary.
+                        end_of_summary = true;
+                        // We surround tables with `<div>` HTML tags so this is a special case.
+                        get_next_tag = kind == TagEnd::Table;
+                    }
+                }
+                _ => {}
+            }
+            summary_events.push(event);
+            if end_of_summary {
+                if get_next_tag && let Some(event) = p.next() {
+                    summary_events.push(event);
+                }
+                break;
+            }
+        }
+        let mut summary = String::new();
+        html::push_html(&mut summary, summary_events.into_iter());
+        if summary.is_empty() {
+            return (None, None);
+        }
+        let mut content = String::new();
+        html::push_html(&mut content, p);
+
+        if content.is_empty() { (Some(summary), None) } else { (Some(summary), Some(content)) }
     }
 }
 
@@ -1882,7 +1936,7 @@ pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<Rust
 #[derive(Clone, Default, Debug)]
 pub struct IdMap {
     map: FxHashMap<String, usize>,
-    existing_footnotes: usize,
+    existing_footnotes: Arc<AtomicUsize>,
 }
 
 fn is_default_id(id: &str) -> bool {
@@ -1942,7 +1996,7 @@ fn is_default_id(id: &str) -> bool {
 
 impl IdMap {
     pub fn new() -> Self {
-        IdMap { map: FxHashMap::default(), existing_footnotes: 0 }
+        IdMap { map: FxHashMap::default(), existing_footnotes: Arc::new(AtomicUsize::new(0)) }
     }
 
     pub(crate) fn derive<S: AsRef<str> + ToString>(&mut self, candidate: S) -> String {
@@ -1970,15 +2024,17 @@ impl IdMap {
 
     /// Method to handle `existing_footnotes` increment automatically (to prevent forgetting
     /// about it).
-    pub(crate) fn handle_footnotes<F: FnOnce(&mut Self, &mut usize)>(&mut self, closure: F) {
-        let mut existing_footnotes = self.existing_footnotes;
+    pub(crate) fn handle_footnotes<'a, T, F: FnOnce(&'a mut Self, Weak<AtomicUsize>) -> T>(
+        &'a mut self,
+        closure: F,
+    ) -> T {
+        let existing_footnotes = Arc::downgrade(&self.existing_footnotes);
 
-        closure(self, &mut existing_footnotes);
-        self.existing_footnotes = existing_footnotes;
+        closure(self, existing_footnotes)
     }
 
     pub(crate) fn clear(&mut self) {
         self.map.clear();
-        self.existing_footnotes = 0;
+        self.existing_footnotes = Arc::new(AtomicUsize::new(0));
     }
 }

src/librustdoc/html/markdown/footnotes.rs

@@ -1,5 +1,8 @@
 //! Markdown footnote handling.
+
 use std::fmt::Write as _;
+use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::{Arc, Weak};
 
 use pulldown_cmark::{CowStr, Event, Tag, TagEnd, html};
 use rustc_data_structures::fx::FxIndexMap;
@@ -8,10 +11,11 @@ use super::SpannedEvent;
 
 /// Moves all footnote definitions to the end and add back links to the
 /// references.
-pub(super) struct Footnotes<'a, 'b, I> {
+pub(super) struct Footnotes<'a, I> {
     inner: I,
     footnotes: FxIndexMap<String, FootnoteDef<'a>>,
-    existing_footnotes: &'b mut usize,
+    existing_footnotes: Arc<AtomicUsize>,
+    start_id: usize,
 }
 
 /// The definition of a single footnote.
@@ -21,13 +25,16 @@ struct FootnoteDef<'a> {
     id: usize,
 }
 
-impl<'a, 'b, I: Iterator<Item = SpannedEvent<'a>>> Footnotes<'a, 'b, I> {
-    pub(super) fn new(iter: I, existing_footnotes: &'b mut usize) -> Self {
-        Footnotes { inner: iter, footnotes: FxIndexMap::default(), existing_footnotes }
+impl<'a, I: Iterator<Item = SpannedEvent<'a>>> Footnotes<'a, I> {
+    pub(super) fn new(iter: I, existing_footnotes: Weak<AtomicUsize>) -> Self {
+        let existing_footnotes =
+            existing_footnotes.upgrade().expect("`existing_footnotes` was dropped");
+        let start_id = existing_footnotes.load(Ordering::Relaxed);
+        Footnotes { inner: iter, footnotes: FxIndexMap::default(), existing_footnotes, start_id }
     }
 
     fn get_entry(&mut self, key: &str) -> (&mut Vec<Event<'a>>, usize) {
-        let new_id = self.footnotes.len() + 1 + *self.existing_footnotes;
+        let new_id = self.footnotes.len() + 1 + self.start_id;
         let key = key.to_owned();
         let FootnoteDef { content, id } =
             self.footnotes.entry(key).or_insert(FootnoteDef { content: Vec::new(), id: new_id });
@@ -44,7 +51,7 @@ impl<'a, 'b, I: Iterator<Item = SpannedEvent<'a>>> Footnotes<'a, 'b, I> {
             id,
             // Although the ID count is for the whole page, the footnote reference
             // are local to the item so we make this ID "local" when displayed.
-            id - *self.existing_footnotes
+            id - self.start_id
         );
         Event::Html(reference.into())
     }
@@ -64,7 +71,7 @@ impl<'a, 'b, I: Iterator<Item = SpannedEvent<'a>>> Footnotes<'a, 'b, I> {
     }
 }
 
-impl<'a, I: Iterator<Item = SpannedEvent<'a>>> Iterator for Footnotes<'a, '_, I> {
+impl<'a, I: Iterator<Item = SpannedEvent<'a>>> Iterator for Footnotes<'a, I> {
     type Item = SpannedEvent<'a>;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -87,7 +94,7 @@ impl<'a, I: Iterator<Item = SpannedEvent<'a>>> Iterator for Footnotes<'a, '_, I>
                         // After all the markdown is emmited, emit an <hr> then all the footnotes
                         // in a list.
                         let defs: Vec<_> = self.footnotes.drain(..).map(|(_, x)| x).collect();
-                        *self.existing_footnotes += defs.len();
+                        self.existing_footnotes.fetch_add(defs.len(), Ordering::Relaxed);
                         let defs_html = render_footnotes_defs(defs);
                         return Some((Event::Html(defs_html.into()), 0..0));
                     } else {

src/librustdoc/html/render/mod.rs

@@ -1904,7 +1904,6 @@ fn render_impl(
         }
     }
 
-    let trait_is_none = trait_.is_none();
     // If we've implemented a trait, then also emit documentation for all
     // default items which weren't overridden in the implementation block.
     // We don't emit documentation for default items if they appear in the
@@ -1936,6 +1935,23 @@ fn render_impl(
                 if rendering_params.toggle_open_by_default { " open" } else { "" }
             );
         }
+
+        let (before_dox, after_dox) = i
+            .impl_item
+            .opt_doc_value()
+            .map(|dox| {
+                Markdown {
+                    content: &*dox,
+                    links: &i.impl_item.links(cx),
+                    ids: &mut cx.id_map.borrow_mut(),
+                    error_codes: cx.shared.codes,
+                    edition: cx.shared.edition(),
+                    playground: &cx.shared.playground,
+                    heading_offset: HeadingOffset::H4,
+                }
+                .split_summary_and_content()
+            })
+            .unwrap_or((None, None));
         render_impl_summary(
             w,
             cx,
@@ -1944,33 +1960,23 @@ fn render_impl(
             rendering_params.show_def_docs,
             use_absolute,
             aliases,
+            &before_dox,
         );
         if toggled {
             w.write_str("</summary>");
         }
 
-        if let Some(ref dox) = i.impl_item.opt_doc_value() {
-            if trait_is_none && impl_.items.is_empty() {
+        if before_dox.is_some() {
+            if trait_.is_none() && impl_.items.is_empty() {
                 w.write_str(
                     "<div class=\"item-info\">\
                          <div class=\"stab empty-impl\">This impl block contains no items.</div>\
                      </div>",
                 );
             }
-            write!(
-                w,
-                "<div class=\"docblock\">{}</div>",
-                Markdown {
-                    content: dox,
-                    links: &i.impl_item.links(cx),
-                    ids: &mut cx.id_map.borrow_mut(),
-                    error_codes: cx.shared.codes,
-                    edition: cx.shared.edition(),
-                    playground: &cx.shared.playground,
-                    heading_offset: HeadingOffset::H4,
-                }
-                .into_string()
-            );
+            if let Some(after_dox) = after_dox {
+                write!(w, "<div class=\"docblock\">{after_dox}</div>");
+            }
         }
         if !default_impl_items.is_empty() || !impl_items.is_empty() {
             w.write_str("<div class=\"impl-items\">");
@@ -2031,6 +2037,7 @@ pub(crate) fn render_impl_summary(
     // This argument is used to reference same type with different paths to avoid duplication
     // in documentation pages for trait with automatic implementations like "Send" and "Sync".
     aliases: &[String],
+    doc: &Option<String>,
 ) {
     let inner_impl = i.inner_impl();
     let id = cx.derive_id(get_id_for_impl(cx.tcx(), i.impl_item.item_id));
@@ -2082,6 +2089,10 @@ pub(crate) fn render_impl_summary(
         );
     }
 
+    if let Some(doc) = doc {
+        write!(w, "<div class=\"docblock\">{doc}</div>");
+    }
+
     w.write_str("</section>");
 }
 

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

@@ -2210,6 +2210,39 @@ details.toggle[open] > summary::after {
 	content: "Collapse";
 }
 
+details.toggle:not([open]) > summary .docblock {
+	max-height: calc(1.5em + 0.75em);
+	overflow-y: hidden;
+}
+details.toggle:not([open]) > summary .docblock > :first-child {
+	max-width: 100%;
+	overflow: hidden;
+	width: fit-content;
+	white-space: nowrap;
+	position: relative;
+	padding-right: 1em;
+}
+details.toggle:not([open]) > summary .docblock > :first-child::after {
+	content: "…";
+	position: absolute;
+	right: 0;
+	top: 0;
+	bottom: 0;
+	z-index: 1;
+	background-color: var(--main-background-color);
+	font: 1rem/1.5 "Source Serif 4", NanumBarunGothic, serif;
+	/* To make it look a bit better and not have it stuck to the preceding element. */
+	padding-left: 0.2em;
+}
+details.toggle:not([open]) > summary .docblock > div:first-child::after {
+	/* This is to make the "..." always appear at the bottom. */
+	padding-top: calc(1.5em + 0.75em - 1.2rem);
+}
+
+details.toggle > summary .docblock {
+	margin-top: 0.75em;
+}
+
 /* This is needed in docblocks to have the "▶" element to be on the same line. */
 .docblock summary > * {
 	display: inline-block;

tests/rustdoc-gui/docblock-table-overflow.goml

@@ -10,12 +10,8 @@ assert-property: (".top-doc .docblock table", {"scrollWidth": "1572"})
 
 // Checking it works on other doc blocks as well...
 
-// Logically, the ".docblock" and the "<p>" should have the same scroll width.
-compare-elements-property: (
-    "#implementations-list > details .docblock",
-    "#implementations-list > details .docblock > p",
-    ["scrollWidth"],
-)
-assert-property: ("#implementations-list > details .docblock", {"scrollWidth": "835"})
+// Logically, the ".docblock" and the "<p>" should have the same scroll width (if we exclude the margin).
+assert-property: ("#implementations-list > details .docblock", {"scrollWidth": 816})
+assert-property: ("#implementations-list > details .docblock > p", {"scrollWidth": 835})
 // However, since there is overflow in the <table>, its scroll width is bigger.
 assert-property: ("#implementations-list > details .docblock table", {"scrollWidth": "1572"})

tests/rustdoc-gui/impl-block-doc.goml

@@ -0,0 +1,42 @@
+// Checks that the first sentence of an impl block doc is always visible even when the impl
+// block is collapsed.
+go-to: "file://" + |DOC_PATH| + "/test_docs/struct.ImplDoc.html"
+
+set-window-size: (900, 600)
+
+define-function: (
+    "compare-size-and-pos",
+    [nth_impl],
+    block {
+        // First we collapse the impl block.
+        store-value: (impl_path, "#implementations-list details:nth-of-type(" + |nth_impl| + ")")
+        set-property: (|impl_path|, {"open": false})
+        wait-for: |impl_path| + ":not([open])"
+
+        store-value: (impl_path, |impl_path| + " summary")
+        store-size: (|impl_path|, {"height": impl_height})
+        store-position: (|impl_path|, {"y": impl_y})
+
+        store-size: (|impl_path| + " .docblock", {"height": doc_height})
+        store-position: (|impl_path| + " .docblock", {"y": doc_y})
+
+        assert: |impl_y| + |impl_height| >= |doc_y|
+    }
+)
+
+call-function: ("compare-size-and-pos", {"nth_impl": 1})
+// Since the first impl block has a long line, we ensure that it doesn't display all of it.
+assert: (|impl_y| + |impl_height|) <= (|doc_y| + |doc_height|)
+
+call-function: ("compare-size-and-pos", {"nth_impl": 2})
+// The second impl block has a short line.
+assert: (|impl_y| + |impl_height|) >= (|doc_y| + |doc_height|)
+
+// FIXME: Needs `if` condition to make this test check that `padding-top` on the "..." element
+// is as expected for tables.
+call-function: ("compare-size-and-pos", {"nth_impl": 3})
+assert: (|impl_y| + |impl_height|) >= (|doc_y| + |doc_height|)
+call-function: ("compare-size-and-pos", {"nth_impl": 4})
+assert: (|impl_y| + |impl_height|) >= (|doc_y| + |doc_height|)
+call-function: ("compare-size-and-pos", {"nth_impl": 5})
+assert: (|impl_y| + |impl_height|) >= (|doc_y| + |doc_height|)

tests/rustdoc-gui/impl-doc.goml

@@ -3,7 +3,7 @@ go-to: "file://" + |DOC_PATH| + "/test_docs/struct.TypeWithImplDoc.html"
 
 // The text is about 24px tall, so if there's a margin, then their position will be >24px apart
 compare-elements-position-near-false: (
-    "#implementations-list > .implementors-toggle > .docblock > p",
+    "#implementations-list > .implementors-toggle .docblock > p",
     "#implementations-list > .implementors-toggle > .impl-items",
     {"y": 24}
 )

tests/rustdoc-gui/item-info-overflow.goml

@@ -16,7 +16,7 @@ assert-text: (
 go-to: "file://" + |DOC_PATH| + "/lib2/struct.LongItemInfo2.html"
 compare-elements-property: (
     "#impl-SimpleTrait-for-LongItemInfo2 .item-info",
-    "#impl-SimpleTrait-for-LongItemInfo2 + .docblock",
+    "#impl-SimpleTrait-for-LongItemInfo2 .docblock",
     ["scrollWidth"],
 )
 assert-property: (

tests/rustdoc-gui/source-code-page-code-scroll.goml

@@ -2,7 +2,7 @@
 go-to: "file://" + |DOC_PATH| + "/src/test_docs/lib.rs.html"
 set-window-size: (800, 1000)
 // "scrollWidth" should be superior than "clientWidth".
-assert-property: ("body", {"scrollWidth": 1114, "clientWidth": 800})
+assert-property: ("body", {"scrollWidth": 1776, "clientWidth": 800})
 
 // Both properties should be equal (ie, no scroll on the code block).
-assert-property: (".example-wrap .rust", {"scrollWidth": 1000, "clientWidth": 1000})
+assert-property: (".example-wrap .rust", {"scrollWidth": 1662, "clientWidth": 1662})

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

@@ -652,3 +652,42 @@ pub mod long_list {
     //! * [`FromBytes`](#a) indicates that a type may safely be converted from an arbitrary byte
     //!   sequence
 }
+
+pub struct ImplDoc;
+
+/// bla sondfosdnf sdfasd fadsd fdsa f ads fad sf sad f sad fasdfsafsa df dsafasdasd fsa dfadsfasd
+/// fads fadfadd
+///
+/// bla
+impl ImplDoc {
+    pub fn bar() {}
+}
+
+/// bla
+///
+/// bla
+impl ImplDoc {
+    pub fn bar2() {}
+}
+
+// ignore-tidy-linelength
+/// | this::is::a::kinda::very::long::header::number::one | this::is::a::kinda::very::long::header::number::two | this::is::a::kinda::very::long::header::number::three |
+/// |-|-|-|
+/// | bla | bli | blob |
+impl ImplDoc {
+    pub fn bar3() {}
+}
+
+/// # h1
+///
+/// bla
+impl ImplDoc {
+    pub fn bar4() {}
+}
+
+/// * list
+/// * list
+/// * list
+impl ImplDoc {
+    pub fn bar5() {}
+}