Auto merge of rust-lang#73819 - euclio:rustdoc-summaries, r=jyn514,Gu…

…illaumeGomez

rustdoc: do not use plain summary for trait impls

Fixes rust-lang#38386.
Fixes rust-lang#48332.
Fixes rust-lang#49430.
Fixes rust-lang#62741.
Fixes rust-lang#73474.

Unfortunately this is not quite ready to go because the newly-working links trigger a bunch of linkcheck failures. The failures are tough to fix because the links are resolved relative to the implementor, which could be anywhere in the module hierarchy.

(In the current docs, these links end up rendering as uninterpreted markdown syntax, so I don't think these failures are any worse than the status quo. It might be acceptable to just add them to the linkchecker whitelist.)

Ideally this could be fixed with intra-doc links ~~but it isn't working for me: I am currently investigating if it's possible to solve it this way.~~ Opened rust-lang#73829.

EDIT: This is now ready!

library/core/src/fmt/mod.rs

@@ -168,8 +168,6 @@ pub trait Write {
     /// This method should generally not be invoked manually, but rather through
     /// the [`write!`] macro itself.
     ///
-    /// [`write!`]: ../../std/macro.write.html
-    ///
     /// # Examples
     ///
     /// ```

library/core/src/iter/traits/double_ended.rs

@@ -148,7 +148,7 @@ pub trait DoubleEndedIterator: Iterator {
     /// This is the reverse version of [`try_fold()`]: it takes elements
     /// starting from the back of the iterator.
     ///
-    /// [`try_fold()`]: trait.Iterator.html#method.try_fold
+    /// [`try_fold()`]: Iterator::try_fold
     ///
     /// # Examples
     ///
@@ -213,7 +213,7 @@ pub trait DoubleEndedIterator: Iterator {
     /// Folding is useful whenever you have a collection of something, and want
     /// to produce a single value from it.
     ///
-    /// [`fold()`]: trait.Iterator.html#method.fold
+    /// [`fold()`]: Iterator::fold
     ///
     /// # Examples
     ///

library/core/src/slice/mod.rs

@@ -3319,7 +3319,7 @@ pub unsafe trait SliceIndex<T: ?Sized>: private_slice_index::Sealed {
     /// Calling this method with an out-of-bounds index or a dangling `slice` pointer
     /// is *[undefined behavior]* even if the resulting reference is not used.
     ///
-    /// [undefined behavior]: ../../reference/behavior-considered-undefined.html
+    /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
     #[unstable(feature = "slice_index_methods", issue = "none")]
     unsafe fn get_unchecked(self, slice: *const T) -> *const Self::Output;
 
@@ -3328,7 +3328,7 @@ pub unsafe trait SliceIndex<T: ?Sized>: private_slice_index::Sealed {
     /// Calling this method with an out-of-bounds index or a dangling `slice` pointer
     /// is *[undefined behavior]* even if the resulting reference is not used.
     ///
-    /// [undefined behavior]: ../../reference/behavior-considered-undefined.html
+    /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
     #[unstable(feature = "slice_index_methods", issue = "none")]
     unsafe fn get_unchecked_mut(self, slice: *mut T) -> *mut Self::Output;
 

library/std/src/os/linux/fs.rs

@@ -196,7 +196,7 @@ pub trait MetadataExt {
     fn st_atime(&self) -> i64;
     /// Returns the last access time of the file, in nanoseconds since [`st_atime`].
     ///
-    /// [`st_atime`]: #tymethod.st_atime
+    /// [`st_atime`]: Self::st_atime
     ///
     /// # Examples
     ///
@@ -232,7 +232,7 @@ pub trait MetadataExt {
     fn st_mtime(&self) -> i64;
     /// Returns the last modification time of the file, in nanoseconds since [`st_mtime`].
     ///
-    /// [`st_mtime`]: #tymethod.st_mtime
+    /// [`st_mtime`]: Self::st_mtime
     ///
     /// # Examples
     ///
@@ -268,7 +268,7 @@ pub trait MetadataExt {
     fn st_ctime(&self) -> i64;
     /// Returns the last status change time of the file, in nanoseconds since [`st_ctime`].
     ///
-    /// [`st_ctime`]: #tymethod.st_ctime
+    /// [`st_ctime`]: Self::st_ctime
     ///
     /// # Examples
     ///

library/std/src/os/redox/fs.rs

@@ -200,7 +200,7 @@ pub trait MetadataExt {
     fn st_atime(&self) -> i64;
     /// Returns the last access time of the file, in nanoseconds since [`st_atime`].
     ///
-    /// [`st_atime`]: #tymethod.st_atime
+    /// [`st_atime`]: Self::st_atime
     ///
     /// # Examples
     ///
@@ -236,7 +236,7 @@ pub trait MetadataExt {
     fn st_mtime(&self) -> i64;
     /// Returns the last modification time of the file, in nanoseconds since [`st_mtime`].
     ///
-    /// [`st_mtime`]: #tymethod.st_mtime
+    /// [`st_mtime`]: Self::st_mtime
     ///
     /// # Examples
     ///
@@ -272,7 +272,7 @@ pub trait MetadataExt {
     fn st_ctime(&self) -> i64;
     /// Returns the last status change time of the file, in nanoseconds since [`st_ctime`].
     ///
-    /// [`st_ctime`]: #tymethod.st_ctime
+    /// [`st_ctime`]: Self::st_ctime
     ///
     /// # Examples
     ///

src/librustdoc/formats/cache.rs

@@ -16,7 +16,7 @@ use crate::formats::item_type::ItemType;
 use crate::formats::Impl;
 use crate::html::render::cache::{extern_location, get_index_search_type, ExternalLocation};
 use crate::html::render::IndexItem;
-use crate::html::render::{plain_summary_line, shorten};
+use crate::html::render::{plain_text_summary, shorten};
 
 thread_local!(crate static CACHE_KEY: RefCell<Arc<Cache>> = Default::default());
 
@@ -313,7 +313,7 @@ impl DocFolder for Cache {
                             ty: item.type_(),
                             name: s.to_string(),
                             path: path.join("::"),
-                            desc: shorten(plain_summary_line(item.doc_value())),
+                            desc: shorten(plain_text_summary(item.doc_value())),
                             parent,
                             parent_idx: None,
                             search_type: get_index_search_type(&item),

src/librustdoc/html/markdown.rs

@@ -954,44 +954,33 @@ impl MarkdownSummaryLine<'_> {
     }
 }
 
-pub fn plain_summary_line(md: &str) -> String {
-    struct ParserWrapper<'a> {
-        inner: Parser<'a>,
-        is_in: isize,
-        is_first: bool,
+/// Renders the first paragraph of the provided markdown as plain text.
+///
+/// - Headings, links, and formatting are stripped.
+/// - Inline code is rendered as-is, surrounded by backticks.
+/// - HTML and code blocks are ignored.
+pub fn plain_text_summary(md: &str) -> String {
+    if md.is_empty() {
+        return String::new();
     }
 
-    impl<'a> Iterator for ParserWrapper<'a> {
-        type Item = String;
-
-        fn next(&mut self) -> Option<String> {
-            let next_event = self.inner.next()?;
-            let (ret, is_in) = match next_event {
-                Event::Start(Tag::Paragraph) => (None, 1),
-                Event::Start(Tag::Heading(_)) => (None, 1),
-                Event::Code(code) => (Some(format!("`{}`", code)), 0),
-                Event::Text(ref s) if self.is_in > 0 => (Some(s.as_ref().to_owned()), 0),
-                Event::End(Tag::Paragraph | Tag::Heading(_)) => (None, -1),
-                _ => (None, 0),
-            };
-            if is_in > 0 || (is_in < 0 && self.is_in > 0) {
-                self.is_in += is_in;
-            }
-            if ret.is_some() {
-                self.is_first = false;
-                ret
-            } else {
-                Some(String::new())
+    let mut s = String::with_capacity(md.len() * 3 / 2);
+
+    for event in Parser::new_ext(md, Options::ENABLE_STRIKETHROUGH) {
+        match &event {
+            Event::Text(text) => s.push_str(text),
+            Event::Code(code) => {
+                s.push('`');
+                s.push_str(code);
+                s.push('`');
             }
+            Event::HardBreak | Event::SoftBreak => s.push(' '),
+            Event::Start(Tag::CodeBlock(..)) => break,
+            Event::End(Tag::Paragraph) => break,
+            _ => (),
         }
     }
-    let mut s = String::with_capacity(md.len() * 3 / 2);
-    let p = ParserWrapper {
-        inner: Parser::new_ext(md, Options::ENABLE_STRIKETHROUGH),
-        is_in: 0,
-        is_first: true,
-    };
-    p.filter(|t| !t.is_empty()).for_each(|i| s.push_str(&i));
+
     s
 }
 

src/librustdoc/html/markdown/tests.rs

@@ -1,4 +1,4 @@
-use super::plain_summary_line;
+use super::plain_text_summary;
 use super::{ErrorCodes, IdMap, Ignore, LangString, Markdown, MarkdownHtml};
 use rustc_span::edition::{Edition, DEFAULT_EDITION};
 use std::cell::RefCell;
@@ -205,18 +205,25 @@ fn test_header_ids_multiple_blocks() {
 }
 
 #[test]
-fn test_plain_summary_line() {
+fn test_plain_text_summary() {
     fn t(input: &str, expect: &str) {
-        let output = plain_summary_line(input);
+        let output = plain_text_summary(input);
         assert_eq!(output, expect, "original: {}", input);
     }
 
     t("hello [Rust](https://www.rust-lang.org) :)", "hello Rust :)");
+    t("**bold**", "bold");
+    t("Multi-line\nsummary", "Multi-line summary");
+    t("Hard-break  \nsummary", "Hard-break summary");
+    t("hello [Rust] :)\n\n[Rust]: https://www.rust-lang.org", "hello Rust :)");
     t("hello [Rust](https://www.rust-lang.org \"Rust\") :)", "hello Rust :)");
     t("code `let x = i32;` ...", "code `let x = i32;` ...");
     t("type `Type<'static>` ...", "type `Type<'static>` ...");
     t("# top header", "top header");
     t("## header", "header");
+    t("first paragraph\n\nsecond paragraph", "first paragraph");
+    t("```\nfn main() {}\n```", "");
+    t("<div>hello</div>", "");
 }
 
 #[test]

src/librustdoc/html/render/cache.rs

@@ -9,7 +9,7 @@ use crate::clean::types::GetDefId;
 use crate::clean::{self, AttributesExt};
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
-use crate::html::render::{plain_summary_line, shorten};
+use crate::html::render::{plain_text_summary, shorten};
 use crate::html::render::{Generic, IndexItem, IndexItemFunctionType, RenderType, TypeWithKind};
 
 /// Indicates where an external crate can be found.
@@ -78,7 +78,7 @@ pub fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
                 ty: item.type_(),
                 name: item.name.clone().unwrap(),
                 path: fqp[..fqp.len() - 1].join("::"),
-                desc: shorten(plain_summary_line(item.doc_value())),
+                desc: shorten(plain_text_summary(item.doc_value())),
                 parent: Some(did),
                 parent_idx: None,
                 search_type: get_index_search_type(&item),
@@ -127,7 +127,7 @@ pub fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
     let crate_doc = krate
         .module
         .as_ref()
-        .map(|module| shorten(plain_summary_line(module.doc_value())))
+        .map(|module| shorten(plain_text_summary(module.doc_value())))
         .unwrap_or(String::new());
 
     #[derive(Serialize)]

src/librustdoc/html/render/mod.rs

@@ -1508,6 +1508,7 @@ impl Context {
         }
     }
 
+    /// Construct a map of items shown in the sidebar to a plain-text summary of their docs.
     fn build_sidebar_items(&self, m: &clean::Module) -> BTreeMap<String, Vec<NameDoc>> {
         // BTreeMap instead of HashMap to get a sorted output
         let mut map: BTreeMap<_, Vec<_>> = BTreeMap::new();
@@ -1524,7 +1525,7 @@ impl Context {
             let short = short.to_string();
             map.entry(short)
                 .or_default()
-                .push((myname, Some(plain_summary_line(item.doc_value()))));
+                .push((myname, Some(plain_text_summary(item.doc_value()))));
         }
 
         if self.shared.sort_modules_alphabetically {
@@ -1730,22 +1731,15 @@ fn full_path(cx: &Context, item: &clean::Item) -> String {
     s
 }
 
+/// Renders the first paragraph of the given markdown as plain text, making it suitable for
+/// contexts like alt-text or the search index.
+///
+/// If no markdown is supplied, the empty string is returned.
+///
+/// See [`markdown::plain_text_summary`] for further details.
 #[inline]
-crate fn plain_summary_line(s: Option<&str>) -> String {
-    let s = s.unwrap_or("");
-    // This essentially gets the first paragraph of text in one line.
-    let mut line = s
-        .lines()
-        .skip_while(|line| line.chars().all(|c| c.is_whitespace()))
-        .take_while(|line| line.chars().any(|c| !c.is_whitespace()))
-        .fold(String::new(), |mut acc, line| {
-            acc.push_str(line);
-            acc.push(' ');
-            acc
-        });
-    // remove final whitespace
-    line.pop();
-    markdown::plain_summary_line(&line[..])
+crate fn plain_text_summary(s: Option<&str>) -> String {
+    s.map(markdown::plain_text_summary).unwrap_or_default()
 }
 
 crate fn shorten(s: String) -> String {
@@ -1802,25 +1796,35 @@ fn render_markdown(
     )
 }
 
+/// Writes a documentation block containing only the first paragraph of the documentation. If the
+/// docs are longer, a "Read more" link is appended to the end.
 fn document_short(
     w: &mut Buffer,
-    cx: &Context,
     item: &clean::Item,
     link: AssocItemLink<'_>,
     prefix: &str,
     is_hidden: bool,
 ) {
     if let Some(s) = item.doc_value() {
-        let markdown = if s.contains('\n') {
-            format!(
-                "{} [Read more]({})",
-                &plain_summary_line(Some(s)),
-                naive_assoc_href(item, link)
-            )
-        } else {
-            plain_summary_line(Some(s))
-        };
-        render_markdown(w, cx, &markdown, item.links(), prefix, is_hidden);
+        let mut summary_html = MarkdownSummaryLine(s, &item.links()).into_string();
+
+        if s.contains('\n') {
+            let link = format!(r#" <a href="{}">Read more</a>"#, naive_assoc_href(item, link));
+
+            if let Some(idx) = summary_html.rfind("</p>") {
+                summary_html.insert_str(idx, &link);
+            } else {
+                summary_html.push_str(&link);
+            }
+        }
+
+        write!(
+            w,
+            "<div class='docblock{}'>{}{}</div>",
+            if is_hidden { " hidden" } else { "" },
+            prefix,
+            summary_html,
+        );
     } else if !prefix.is_empty() {
         write!(
             w,
@@ -3677,7 +3681,7 @@ fn render_impl(
                         } else if show_def_docs {
                             // In case the item isn't documented,
                             // provide short documentation from the trait.
-                            document_short(w, cx, it, link, "", is_hidden);
+                            document_short(w, it, link, "", is_hidden);
                         }
                     }
                 } else {
@@ -3689,7 +3693,7 @@ fn render_impl(
             } else {
                 document_stability(w, cx, item, is_hidden);
                 if show_def_docs {
-                    document_short(w, cx, item, link, "", is_hidden);
+                    document_short(w, item, link, "", is_hidden);
                 }
             }
         }

src/test/rustdoc/plain-text-summaries.rs

@@ -0,0 +1,26 @@
+#![crate_type = "lib"]
+#![crate_name = "summaries"]
+
+//! This summary has a [link] and `code`.
+//!
+//! This is the second paragraph.
+//!
+//! [link]: https://example.com
+
+// @has search-index.js 'This summary has a link and `code`.'
+// @!has - 'second paragraph'
+
+/// This `code` should be in backticks.
+///
+/// This text should not be rendered.
+pub struct Sidebar;
+
+// @has summaries/sidebar-items.js 'This `code` should be in backticks.'
+// @!has - 'text should not be rendered'
+
+/// ```text
+/// this block should not be rendered
+/// ```
+pub struct Sidebar2;
+
+// @!has summaries/sidebar-items.js 'block should not be rendered'