From 4572e7f903a3d6bd5b401e886c9e7e2ef97998f0 Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 26 Mar 2021 18:14:47 -0700 Subject: [PATCH 1/4] Lint on unknown intra-doc link disambiguators --- .../passes/collect_intra_doc_links.rs | 45 ++++++++++++++----- .../intra-doc/unknown-disambiguator.rs | 10 +++++ .../intra-doc/unknown-disambiguator.stderr | 40 +++++++++++++++++ 3 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs create mode 100644 src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 499931f7e9631..f5c5c9ca4aa9c 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -973,10 +973,13 @@ impl LinkCollector<'_, '_> { }; // Parse and strip the disambiguator from the link, if present. - let (mut path_str, disambiguator) = if let Ok((d, path)) = Disambiguator::from_str(&link) { - (path.trim(), Some(d)) - } else { - (link.trim(), None) + let (mut path_str, disambiguator) = match Disambiguator::from_str(&link) { + Ok(Some((d, path))) => (path.trim(), Some(d)), + Ok(None) => (link.trim(), None), + Err(err_msg) => { + disambiguator_error(self.cx, &item, dox, ori_link.range, &err_msg); + return None; + } }; if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*&;".contains(ch))) { @@ -1514,8 +1517,12 @@ impl Disambiguator { } } - /// Given a link, parse and return `(disambiguator, path_str)` - fn from_str(link: &str) -> Result<(Self, &str), ()> { + /// Given a link, parse and return `(disambiguator, path_str)`. + /// + /// This returns `Ok(Some(...))` if a disambiguator was found, + /// `Ok(None)` if no disambiguator was found, or `Err(...)` + /// if there was a problem with the disambiguator. + fn from_str(link: &str) -> Result, String> { use Disambiguator::{Kind, Namespace as NS, Primitive}; let find_suffix = || { @@ -1528,11 +1535,11 @@ impl Disambiguator { if let Some(link) = link.strip_suffix(suffix) { // Avoid turning `!` or `()` into an empty string if !link.is_empty() { - return Ok((Kind(kind), link)); + return Some((Kind(kind), link)); } } } - Err(()) + None }; if let Some(idx) = link.find('@') { @@ -1551,11 +1558,11 @@ impl Disambiguator { "value" => NS(Namespace::ValueNS), "macro" => NS(Namespace::MacroNS), "prim" | "primitive" => Primitive, - _ => return find_suffix(), + _ => return Err(format!("unknown disambiguator `{}`", prefix)), }; - Ok((d, &rest[1..])) + Ok(Some((d, &rest[1..]))) } else { - find_suffix() + Ok(find_suffix()) } } @@ -1979,6 +1986,22 @@ fn anchor_failure( }); } +/// Report an error in the link disambiguator. +fn disambiguator_error( + cx: &DocContext<'_>, + item: &Item, + dox: &str, + link_range: Range, + msg: &str, +) { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, msg, item, dox, &link_range, |diag, _sp| { + diag.note( + "the disambiguator is the part of the link before the `@` sign, \ + or a suffix such as `()` for functions", + ); + }); +} + /// Report an ambiguity error, where there were multiple possible resolutions. fn ambiguity_error( cx: &DocContext<'_>, diff --git a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs new file mode 100644 index 0000000000000..9222025367dd5 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs @@ -0,0 +1,10 @@ +//! Linking to [foo@banana] and [`bar@banana!()`]. +//~^ ERROR unknown disambiguator `foo` +//~| ERROR unknown disambiguator `bar` +//! And to [no disambiguator](@nectarine) and [another](@apricot!()). +//~^ ERROR unknown disambiguator `` +//~| ERROR unknown disambiguator `` + +#![deny(warnings)] + +fn main() {} diff --git a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr new file mode 100644 index 0000000000000..9e7699eea9a28 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr @@ -0,0 +1,40 @@ +error: unknown disambiguator `foo` + --> $DIR/unknown-disambiguator.rs:1:17 + | +LL | //! Linking to [foo@banana] and [`bar@banana!()`]. + | ^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unknown-disambiguator.rs:8:9 + | +LL | #![deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(rustdoc::broken_intra_doc_links)]` implied by `#[deny(warnings)]` + = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + +error: unknown disambiguator `bar` + --> $DIR/unknown-disambiguator.rs:1:34 + | +LL | //! Linking to [foo@banana] and [`bar@banana!()`]. + | ^^^^^^^^^^^^^^^ + | + = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + +error: unknown disambiguator `` + --> $DIR/unknown-disambiguator.rs:4:31 + | +LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). + | ^^^^^^^^^^ + | + = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + +error: unknown disambiguator `` + --> $DIR/unknown-disambiguator.rs:4:57 + | +LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). + | ^^^^^^^^^^^ + | + = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + +error: aborting due to 4 previous errors + From 56347a173a49ef1232ced3974ccbf8e16c5da78c Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 28 Mar 2021 17:02:26 -0700 Subject: [PATCH 2/4] Point to disambiguator instead of whole link And, now that we do that, we can remove the explanatory note since the error span should make it clear what the disambiguator is. --- src/librustdoc/html/markdown.rs | 1 + .../passes/collect_intra_doc_links.rs | 39 ++++++++++++++----- .../intra-doc/unknown-disambiguator.stderr | 17 +++----- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index b39f9f878921a..c44514ed3cb83 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1162,6 +1162,7 @@ crate fn plain_text_summary(md: &str) -> String { s } +#[derive(Debug)] crate struct MarkdownLink { pub kind: LinkType, pub link: String, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f5c5c9ca4aa9c..417637d238423 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -950,6 +950,7 @@ impl LinkCollector<'_, '_> { } let link = ori_link.link.replace("`", ""); + let no_backticks_range = range_between_backticks(&ori_link); let parts = link.split('#').collect::>(); let (link, extra_fragment) = if parts.len() > 2 { // A valid link can't have multiple #'s @@ -976,8 +977,10 @@ impl LinkCollector<'_, '_> { let (mut path_str, disambiguator) = match Disambiguator::from_str(&link) { Ok(Some((d, path))) => (path.trim(), Some(d)), Ok(None) => (link.trim(), None), - Err(err_msg) => { - disambiguator_error(self.cx, &item, dox, ori_link.range, &err_msg); + Err((err_msg, relative_range)) => { + let disambiguator_range = (no_backticks_range.start + relative_range.start) + ..(no_backticks_range.start + relative_range.end); + disambiguator_error(self.cx, &item, dox, disambiguator_range, &err_msg); return None; } }; @@ -1491,6 +1494,27 @@ impl LinkCollector<'_, '_> { } } +/// Get the section of a link between the backticks, +/// or the whole link if there aren't any backticks. +/// +/// For example: +/// +/// ```text +/// [`Foo`] +/// ^^^ +/// ``` +fn range_between_backticks(ori_link: &MarkdownLink) -> Range { + let after_first_backtick_group = ori_link.link.bytes().position(|b| b != b'`').unwrap_or(0); + let before_second_backtick_group = ori_link + .link + .bytes() + .skip(after_first_backtick_group) + .position(|b| b == b'`') + .unwrap_or(ori_link.link.len()); + (ori_link.range.start + after_first_backtick_group) + ..(ori_link.range.start + before_second_backtick_group) +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] /// Disambiguators for a link. enum Disambiguator { @@ -1522,7 +1546,7 @@ impl Disambiguator { /// This returns `Ok(Some(...))` if a disambiguator was found, /// `Ok(None)` if no disambiguator was found, or `Err(...)` /// if there was a problem with the disambiguator. - fn from_str(link: &str) -> Result, String> { + fn from_str(link: &str) -> Result, (String, Range)> { use Disambiguator::{Kind, Namespace as NS, Primitive}; let find_suffix = || { @@ -1558,7 +1582,7 @@ impl Disambiguator { "value" => NS(Namespace::ValueNS), "macro" => NS(Namespace::MacroNS), "prim" | "primitive" => Primitive, - _ => return Err(format!("unknown disambiguator `{}`", prefix)), + _ => return Err((format!("unknown disambiguator `{}`", prefix), 0..idx)), }; Ok(Some((d, &rest[1..]))) } else { @@ -1994,12 +2018,7 @@ fn disambiguator_error( link_range: Range, msg: &str, ) { - report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, msg, item, dox, &link_range, |diag, _sp| { - diag.note( - "the disambiguator is the part of the link before the `@` sign, \ - or a suffix such as `()` for functions", - ); - }); + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, msg, item, dox, &link_range, |_diag, _sp| {}); } /// Report an ambiguity error, where there were multiple possible resolutions. diff --git a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr index 9e7699eea9a28..2904603dfc312 100644 --- a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr +++ b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr @@ -2,7 +2,7 @@ error: unknown disambiguator `foo` --> $DIR/unknown-disambiguator.rs:1:17 | LL | //! Linking to [foo@banana] and [`bar@banana!()`]. - | ^^^^^^^^^^ + | ^^^ | note: the lint level is defined here --> $DIR/unknown-disambiguator.rs:8:9 @@ -10,31 +10,24 @@ note: the lint level is defined here LL | #![deny(warnings)] | ^^^^^^^^ = note: `#[deny(rustdoc::broken_intra_doc_links)]` implied by `#[deny(warnings)]` - = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions error: unknown disambiguator `bar` - --> $DIR/unknown-disambiguator.rs:1:34 + --> $DIR/unknown-disambiguator.rs:1:35 | LL | //! Linking to [foo@banana] and [`bar@banana!()`]. - | ^^^^^^^^^^^^^^^ - | - = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + | ^^^ error: unknown disambiguator `` --> $DIR/unknown-disambiguator.rs:4:31 | LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). - | ^^^^^^^^^^ - | - = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + | ^ error: unknown disambiguator `` --> $DIR/unknown-disambiguator.rs:4:57 | LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). - | ^^^^^^^^^^^ - | - = note: the disambiguator is the part of the link before the `@` sign, or a suffix such as `()` for functions + | ^ error: aborting due to 4 previous errors From 5497f158af58bf6420222472bbaeeae78081d26f Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 28 Mar 2021 17:16:54 -0700 Subject: [PATCH 3/4] Add test for weird backticks placement --- .../intra-doc/unknown-disambiguator.rs | 7 ++++-- .../intra-doc/unknown-disambiguator.stderr | 24 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs index 9222025367dd5..925fc515a3e65 100644 --- a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs +++ b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs @@ -1,10 +1,13 @@ +#![deny(warnings)] + //! Linking to [foo@banana] and [`bar@banana!()`]. //~^ ERROR unknown disambiguator `foo` //~| ERROR unknown disambiguator `bar` //! And to [no disambiguator](@nectarine) and [another](@apricot!()). //~^ ERROR unknown disambiguator `` //~| ERROR unknown disambiguator `` - -#![deny(warnings)] +//! And with weird backticks: [``foo@hello``] [foo`@`hello]. +//~^ ERROR unknown disambiguator `foo` +//~| ERROR unknown disambiguator `foo` fn main() {} diff --git a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr index 2904603dfc312..195aaca32a27d 100644 --- a/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr +++ b/src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr @@ -1,33 +1,45 @@ error: unknown disambiguator `foo` - --> $DIR/unknown-disambiguator.rs:1:17 + --> $DIR/unknown-disambiguator.rs:3:17 | LL | //! Linking to [foo@banana] and [`bar@banana!()`]. | ^^^ | note: the lint level is defined here - --> $DIR/unknown-disambiguator.rs:8:9 + --> $DIR/unknown-disambiguator.rs:1:9 | LL | #![deny(warnings)] | ^^^^^^^^ = note: `#[deny(rustdoc::broken_intra_doc_links)]` implied by `#[deny(warnings)]` error: unknown disambiguator `bar` - --> $DIR/unknown-disambiguator.rs:1:35 + --> $DIR/unknown-disambiguator.rs:3:35 | LL | //! Linking to [foo@banana] and [`bar@banana!()`]. | ^^^ +error: unknown disambiguator `foo` + --> $DIR/unknown-disambiguator.rs:9:34 + | +LL | //! And with weird backticks: [``foo@hello``] [foo`@`hello]. + | ^^^ + +error: unknown disambiguator `foo` + --> $DIR/unknown-disambiguator.rs:9:48 + | +LL | //! And with weird backticks: [``foo@hello``] [foo`@`hello]. + | ^^^ + error: unknown disambiguator `` - --> $DIR/unknown-disambiguator.rs:4:31 + --> $DIR/unknown-disambiguator.rs:6:31 | LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). | ^ error: unknown disambiguator `` - --> $DIR/unknown-disambiguator.rs:4:57 + --> $DIR/unknown-disambiguator.rs:6:57 | LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()). | ^ -error: aborting due to 4 previous errors +error: aborting due to 6 previous errors From 141df6f60e28c83711db5a2a94d5c4ff5e9aecaa Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 28 Mar 2021 17:22:45 -0700 Subject: [PATCH 4/4] Inline `find_suffix` closure that's only used once --- .../passes/collect_intra_doc_links.rs | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 417637d238423..55978ca551b05 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1549,23 +1549,6 @@ impl Disambiguator { fn from_str(link: &str) -> Result, (String, Range)> { use Disambiguator::{Kind, Namespace as NS, Primitive}; - let find_suffix = || { - let suffixes = [ - ("!()", DefKind::Macro(MacroKind::Bang)), - ("()", DefKind::Fn), - ("!", DefKind::Macro(MacroKind::Bang)), - ]; - for &(suffix, kind) in &suffixes { - if let Some(link) = link.strip_suffix(suffix) { - // Avoid turning `!` or `()` into an empty string - if !link.is_empty() { - return Some((Kind(kind), link)); - } - } - } - None - }; - if let Some(idx) = link.find('@') { let (prefix, rest) = link.split_at(idx); let d = match prefix { @@ -1586,7 +1569,20 @@ impl Disambiguator { }; Ok(Some((d, &rest[1..]))) } else { - Ok(find_suffix()) + let suffixes = [ + ("!()", DefKind::Macro(MacroKind::Bang)), + ("()", DefKind::Fn), + ("!", DefKind::Macro(MacroKind::Bang)), + ]; + for &(suffix, kind) in &suffixes { + if let Some(link) = link.strip_suffix(suffix) { + // Avoid turning `!` or `()` into an empty string + if !link.is_empty() { + return Ok(Some((Kind(kind), link))); + } + } + } + Ok(None) } }