From e4c28bf61a0631b6bf4bd9e53da53611399c1129 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 13 Sep 2020 20:15:01 -0400 Subject: [PATCH 1/3] Upgrade to pulldown-cmark 0.8.0 Thanks to marcusklaas' hard work in https://github.com/raphlinus/pulldown-cmark/pull/469, this fixes a lot of rustdoc bugs! - Get rid of unnecessary `RefCell` - Fix duplicate warnings for broken implicit reference link - Remove unnecessary copy of links --- Cargo.lock | 17 ++++++++-- src/librustdoc/Cargo.toml | 2 +- src/librustdoc/html/markdown.rs | 34 ++++++++++--------- .../rustdoc-ui/intra-link-double-anchor.rs | 7 ++++ .../intra-link-double-anchor.stderr | 10 ++++++ 5 files changed, 50 insertions(+), 20 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-link-double-anchor.rs create mode 100644 src/test/rustdoc-ui/intra-link-double-anchor.stderr diff --git a/Cargo.lock b/Cargo.lock index b448baf425baa..acf7980a0354c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -534,7 +534,7 @@ dependencies = [ "if_chain", "itertools 0.9.0", "lazy_static", - "pulldown-cmark", + "pulldown-cmark 0.7.2", "quine-mc_cluskey", "quote", "regex-syntax", @@ -1844,7 +1844,7 @@ dependencies = [ "log", "memchr", "open", - "pulldown-cmark", + "pulldown-cmark 0.7.2", "regex", "serde", "serde_derive", @@ -2502,6 +2502,17 @@ dependencies = [ "unicase", ] +[[package]] +name = "pulldown-cmark" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ffade02495f22453cd593159ea2f59827aae7f53fa8323f756799b670881dcf8" +dependencies = [ + "bitflags", + "memchr", + "unicase", +] + [[package]] name = "punycode" version = "0.4.1" @@ -4112,7 +4123,7 @@ dependencies = [ "expect-test", "itertools 0.9.0", "minifier", - "pulldown-cmark", + "pulldown-cmark 0.8.0", "rustc-rayon", "serde", "serde_json", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 90d2a18ea5819..a40a44fe27da3 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -8,7 +8,7 @@ edition = "2018" path = "lib.rs" [dependencies] -pulldown-cmark = { version = "0.7", default-features = false } +pulldown-cmark = { version = "0.8", default-features = false } minifier = "0.0.33" rayon = { version = "0.3.0", package = "rustc-rayon" } serde = { version = "1.0", features = ["derive"] } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index a8c60e4a76df4..178c9b0fad38a 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -27,7 +27,6 @@ use rustc_session::lint; use rustc_span::edition::Edition; use rustc_span::Span; use std::borrow::Cow; -use std::cell::RefCell; use std::collections::VecDeque; use std::default::Default; use std::fmt::Write; @@ -39,7 +38,7 @@ use crate::doctest; use crate::html::highlight; use crate::html::toc::TocBuilder; -use pulldown_cmark::{html, CodeBlockKind, CowStr, Event, Options, Parser, Tag}; +use pulldown_cmark::{html, BrokenLink, CodeBlockKind, CowStr, Event, Options, Parser, Tag}; #[cfg(test)] mod tests; @@ -931,15 +930,17 @@ impl Markdown<'_> { if md.is_empty() { return String::new(); } - let replacer = |_: &str, s: &str| { - if let Some(link) = links.iter().find(|link| &*link.original_text == s) { - Some((link.href.clone(), link.new_text.clone())) + let mut replacer = |broken_link: BrokenLink<'_>| { + if let Some(link) = + links.iter().find(|link| &*link.original_text == broken_link.reference) + { + Some((CowStr::Borrowed(&link.href), CowStr::Borrowed(&link.new_text))) } else { None } }; - let p = Parser::new_with_broken_link_callback(md, opts(), Some(&replacer)); + let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut replacer)); let mut s = String::with_capacity(md.len() * 3 / 2); @@ -1009,9 +1010,11 @@ impl MarkdownSummaryLine<'_> { return String::new(); } - let replacer = |_: &str, s: &str| { - if let Some(link) = links.iter().find(|link| &*link.original_text == s) { - Some((link.href.clone(), link.new_text.clone())) + let mut replacer = |broken_link: BrokenLink<'_>| { + if let Some(link) = + links.iter().find(|link| &*link.original_text == broken_link.reference) + { + Some((CowStr::Borrowed(&link.href), CowStr::Borrowed(&link.new_text))) } else { None } @@ -1020,7 +1023,7 @@ impl MarkdownSummaryLine<'_> { let p = Parser::new_with_broken_link_callback( md, Options::ENABLE_STRIKETHROUGH, - Some(&replacer), + Some(&mut replacer), ); let mut s = String::new(); @@ -1067,7 +1070,7 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { } let mut links = vec![]; - let shortcut_links = RefCell::new(vec![]); + let mut shortcut_links = vec![]; { let locate = |s: &str| unsafe { @@ -1084,11 +1087,11 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { } }; - let push = |_: &str, s: &str| { - shortcut_links.borrow_mut().push((s.to_owned(), locate(s))); + let mut push = |link: BrokenLink<'_>| { + shortcut_links.push((link.reference.to_owned(), Some(link.span))); None }; - let p = Parser::new_with_broken_link_callback(md, opts(), Some(&push)); + let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push)); // There's no need to thread an IdMap through to here because // the IDs generated aren't going to be emitted anywhere. @@ -1106,8 +1109,7 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { } } - let mut shortcut_links = shortcut_links.into_inner(); - links.extend(shortcut_links.drain(..)); + links.append(&mut shortcut_links); links } diff --git a/src/test/rustdoc-ui/intra-link-double-anchor.rs b/src/test/rustdoc-ui/intra-link-double-anchor.rs new file mode 100644 index 0000000000000..a01211c4f32b1 --- /dev/null +++ b/src/test/rustdoc-ui/intra-link-double-anchor.rs @@ -0,0 +1,7 @@ +// check-pass + +// regression test for #73264 +// should only give one error +/// docs [label][with#anchor#error] +//~^ WARNING multiple anchors +pub struct S; diff --git a/src/test/rustdoc-ui/intra-link-double-anchor.stderr b/src/test/rustdoc-ui/intra-link-double-anchor.stderr new file mode 100644 index 0000000000000..55636d5c2f4b5 --- /dev/null +++ b/src/test/rustdoc-ui/intra-link-double-anchor.stderr @@ -0,0 +1,10 @@ +warning: `with#anchor#error` contains multiple anchors + --> $DIR/intra-link-double-anchor.rs:5:10 + | +LL | /// docs [label][with#anchor#error] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ contains invalid anchor + | + = note: `#[warn(broken_intra_doc_links)]` on by default + +warning: 1 warning emitted + From f7983cae7008b8f86d927ddcd83a0b4785307b24 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 13 Sep 2020 20:56:33 -0400 Subject: [PATCH 2/3] Don't use `link.span` yet This shows the span of the _whole_ link, including the brackets. But rustdoc only wants to warn about the link text. --- src/librustdoc/html/markdown.rs | 4 +++- src/test/rustdoc-ui/intra-link-double-anchor.stderr | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 178c9b0fad38a..a25d49c34955d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1088,7 +1088,9 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { }; let mut push = |link: BrokenLink<'_>| { - shortcut_links.push((link.reference.to_owned(), Some(link.span))); + // FIXME: use `link.span` instead of `locate` + // (doing it now includes the `[]` as well as the text) + shortcut_links.push((link.reference.to_owned(), locate(link.reference))); None }; let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push)); diff --git a/src/test/rustdoc-ui/intra-link-double-anchor.stderr b/src/test/rustdoc-ui/intra-link-double-anchor.stderr index 55636d5c2f4b5..3282ec8b79379 100644 --- a/src/test/rustdoc-ui/intra-link-double-anchor.stderr +++ b/src/test/rustdoc-ui/intra-link-double-anchor.stderr @@ -1,8 +1,8 @@ warning: `with#anchor#error` contains multiple anchors - --> $DIR/intra-link-double-anchor.rs:5:10 + --> $DIR/intra-link-double-anchor.rs:5:18 | LL | /// docs [label][with#anchor#error] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ contains invalid anchor + | ^^^^^^^^^^^^^^^^^ contains invalid anchor | = note: `#[warn(broken_intra_doc_links)]` on by default From 6f2e1c65933702186cc681a3772862afa77dd632 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 13 Sep 2020 21:13:04 -0400 Subject: [PATCH 3/3] Use `.as_str()` instead of `CowStr::Borrowed` --- src/librustdoc/html/markdown.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index a25d49c34955d..6c0f1c02ac6da 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -934,7 +934,7 @@ impl Markdown<'_> { if let Some(link) = links.iter().find(|link| &*link.original_text == broken_link.reference) { - Some((CowStr::Borrowed(&link.href), CowStr::Borrowed(&link.new_text))) + Some((link.href.as_str().into(), link.new_text.as_str().into())) } else { None } @@ -1014,7 +1014,7 @@ impl MarkdownSummaryLine<'_> { if let Some(link) = links.iter().find(|link| &*link.original_text == broken_link.reference) { - Some((CowStr::Borrowed(&link.href), CowStr::Borrowed(&link.new_text))) + Some((link.href.as_str().into(), link.new_text.as_str().into())) } else { None }