about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAndy Russell <arussell123@gmail.com>2020-06-27 16:44:42 -0400
committerAndy Russell <arussell123@gmail.com>2020-08-30 12:02:18 -0400
commite0822ecdbc43a6128136661bb73fb6f3c3db2b4a (patch)
treeb5ae198d329bf406efc08260053b081b0324b8a1
parentdb534b3ac286cf45688c3bbae6aa6e77439e52d2 (diff)
downloadrust-e0822ecdbc43a6128136661bb73fb6f3c3db2b4a.tar.gz
rust-e0822ecdbc43a6128136661bb73fb6f3c3db2b4a.zip
rustdoc: do not use plain summary for trait impls
Fixes #38386.
Fixes #48332.
Fixes #49430.
Fixes #62741.
Fixes #73474.
-rw-r--r--src/librustdoc/formats/cache.rs4
-rw-r--r--src/librustdoc/html/markdown.rs55
-rw-r--r--src/librustdoc/html/markdown/tests.rs13
-rw-r--r--src/librustdoc/html/render/cache.rs6
-rw-r--r--src/librustdoc/html/render/mod.rs62
-rw-r--r--src/test/rustdoc/plain-text-summaries.rs26
-rw-r--r--src/test/rustdoc/trait-impl.rs46
7 files changed, 142 insertions, 70 deletions
diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
index 99b31473f87..b99321e8484 100644
--- a/src/librustdoc/formats/cache.rs
+++ b/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),
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index 56499f736e1..894d868dcc6 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -955,44 +955,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
 }
 
diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs
index 783977d285d..f071f3d5b4e 100644
--- a/src/librustdoc/html/markdown/tests.rs
+++ b/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;
@@ -210,18 +210,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]
diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs
index ccc07645620..cf785d362cd 100644
--- a/src/librustdoc/html/render/cache.rs
+++ b/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)]
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 15afe9257d1..90206d27781 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -1506,6 +1506,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();
@@ -1522,7 +1523,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 {
@@ -1728,22 +1729,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 {
@@ -1800,25 +1794,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,
@@ -3689,7 +3693,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 {
@@ -3701,7 +3705,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);
                 }
             }
         }
diff --git a/src/test/rustdoc/plain-text-summaries.rs b/src/test/rustdoc/plain-text-summaries.rs
new file mode 100644
index 00000000000..c995ccbf0af
--- /dev/null
+++ b/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'
diff --git a/src/test/rustdoc/trait-impl.rs b/src/test/rustdoc/trait-impl.rs
new file mode 100644
index 00000000000..3bcaa3bb673
--- /dev/null
+++ b/src/test/rustdoc/trait-impl.rs
@@ -0,0 +1,46 @@
+pub trait Trait {
+    /// Some long docs here.
+    ///
+    /// These docs are long enough that a link will be added to the end.
+    fn a();
+
+    /// These docs contain a [reference link].
+    ///
+    /// [reference link]: https://example.com
+    fn b();
+
+    /// ```
+    /// This code block should not be in the output, but a Read more link should be generated
+    /// ```
+    fn c();
+
+    /// Escaped formatting a\*b\*c\* works
+    fn d();
+}
+
+pub struct Struct;
+
+impl Trait for Struct {
+    // @has trait_impl/struct.Struct.html '//*[@id="method.a"]/../div/p' 'Some long docs'
+    // @!has - '//*[@id="method.a"]/../div/p' 'link will be added'
+    // @has - '//*[@id="method.a"]/../div/p/a' 'Read more'
+    // @has - '//*[@id="method.a"]/../div/p/a/@href' 'trait.Trait.html'
+    fn a() {}
+
+    // @has trait_impl/struct.Struct.html '//*[@id="method.b"]/../div/p' 'These docs contain'
+    // @has - '//*[@id="method.b"]/../div/p/a' 'reference link'
+    // @has - '//*[@id="method.b"]/../div/p/a/@href' 'https://example.com'
+    // @has - '//*[@id="method.b"]/../div/p/a' 'Read more'
+    // @has - '//*[@id="method.b"]/../div/p/a/@href' 'trait.Trait.html'
+    fn b() {}
+
+    // @!has trait_impl/struct.Struct.html '//*[@id="method.c"]/../div/p' 'code block'
+    // @has - '//*[@id="method.c"]/../div/p/a' 'Read more'
+    // @has - '//*[@id="method.c"]/../div/p/a/@href' 'trait.Trait.html'
+    fn c() {}
+
+    // @has trait_impl/struct.Struct.html '//*[@id="method.d"]/../div/p' \
+    //   'Escaped formatting a*b*c* works'
+    // @!has trait_impl/struct.Struct.html '//*[@id="method.d"]/../div/p/em'
+    fn d() {}
+}