From e0822ecdbc43a6128136661bb73fb6f3c3db2b4a Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sat, 27 Jun 2020 16:44:42 -0400 Subject: [PATCH 1/2] rustdoc: do not use plain summary for trait impls Fixes #38386. Fixes #48332. Fixes #49430. Fixes #62741. Fixes #73474. --- src/librustdoc/formats/cache.rs | 4 +- src/librustdoc/html/markdown.rs | 55 +++++++++------------ src/librustdoc/html/markdown/tests.rs | 13 +++-- src/librustdoc/html/render/cache.rs | 6 +-- src/librustdoc/html/render/mod.rs | 62 +++++++++++++----------- src/test/rustdoc/plain-text-summaries.rs | 26 ++++++++++ src/test/rustdoc/trait-impl.rs | 46 ++++++++++++++++++ 7 files changed, 142 insertions(+), 70 deletions(-) create mode 100644 src/test/rustdoc/plain-text-summaries.rs create mode 100644 src/test/rustdoc/trait-impl.rs diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 99b31473f87a3..b99321e8484c9 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> = 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 56499f736e163..894d868dcc63e 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 { - 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 783977d285dc4..f071f3d5b4e73 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("
hello
", ""); } #[test] diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index ccc07645620a9..cf785d362cd11 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 15afe9257d187..90206d27781cb 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> { // 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#" Read more"#, naive_assoc_href(item, link)); + + if let Some(idx) = summary_html.rfind("

") { + summary_html.insert_str(idx, &link); + } else { + summary_html.push_str(&link); + } + } + + write!( + w, + "
{}{}
", + 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 0000000000000..c995ccbf0af67 --- /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 0000000000000..3bcaa3bb67313 --- /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() {} +} From 98232ece14bdd68aeac3d761039d9a7c88c30b3f Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sun, 2 Aug 2020 13:58:34 -0400 Subject: [PATCH 2/2] fix broken trait method links --- library/core/src/fmt/mod.rs | 2 -- library/core/src/iter/traits/double_ended.rs | 4 ++-- library/core/src/iter/traits/iterator.rs | 2 +- library/core/src/slice/mod.rs | 4 ++-- library/std/src/os/linux/fs.rs | 6 +++--- library/std/src/os/redox/fs.rs | 6 +++--- 6 files changed, 11 insertions(+), 13 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 52f73c03e02d9..48b7f2739eeb2 100644 --- a/library/core/src/fmt/mod.rs +++ b/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 /// /// ``` diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs index 851a1e49a493b..323fdbb45f36b 100644 --- a/library/core/src/iter/traits/double_ended.rs +++ b/library/core/src/iter/traits/double_ended.rs @@ -149,7 +149,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 /// @@ -214,7 +214,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 /// diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index aca6699b9efbb..a1d11b597a140 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2713,7 +2713,7 @@ pub trait Iterator { /// This is useful when you have an iterator over `&T`, but you need an /// iterator over `T`. /// - /// [`clone`]: crate::clone::Clone::clone + /// [`clone`]: Clone::clone /// /// # Examples /// diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 0d97ddb29af79..4dcb91d63c9c3 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -3219,7 +3219,7 @@ pub unsafe trait SliceIndex: 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; @@ -3228,7 +3228,7 @@ pub unsafe trait SliceIndex: 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; diff --git a/library/std/src/os/linux/fs.rs b/library/std/src/os/linux/fs.rs index cae65f12187e2..14719a9be5e8a 100644 --- a/library/std/src/os/linux/fs.rs +++ b/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 /// diff --git a/library/std/src/os/redox/fs.rs b/library/std/src/os/redox/fs.rs index 94d65651daa3c..0f179c8b837dd 100644 --- a/library/std/src/os/redox/fs.rs +++ b/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 ///