Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint on unknown intra-doc link disambiguators #83543

Merged
merged 4 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,7 @@ crate fn plain_text_summary(md: &str) -> String {
s
}

#[derive(Debug)]
crate struct MarkdownLink {
pub kind: LinkType,
pub link: String,
Expand Down
64 changes: 53 additions & 11 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>();
let (link, extra_fragment) = if parts.len() > 2 {
// A valid link can't have multiple #'s
Expand All @@ -973,10 +974,15 @@ 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, 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;
}
};

if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*&;".contains(ch))) {
Expand Down Expand Up @@ -1488,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<usize> {
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());
Comment on lines +1507 to +1513
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will break on things like

[`a`b`c`]

but that seems fine, that's an awful abuse of rustdoc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually works correctly! But I imagine there are some edge cases that don't work correctly.

error: unknown disambiguator `foo`
  --> src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs:12:7
   |
12 | //! [`foo@`b`c`]
   |       ^^^

(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 {
Expand All @@ -1514,8 +1541,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<Option<(Self, &str)>, (String, Range<usize>)> {
use Disambiguator::{Kind, Namespace as NS, Primitive};

let find_suffix = || {
Expand All @@ -1528,11 +1559,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('@') {
Expand All @@ -1551,11 +1582,11 @@ impl Disambiguator {
"value" => NS(Namespace::ValueNS),
"macro" => NS(Namespace::MacroNS),
"prim" | "primitive" => Primitive,
_ => return find_suffix(),
_ => return Err((format!("unknown disambiguator `{}`", prefix), 0..idx)),
};
Ok((d, &rest[1..]))
Ok(Some((d, &rest[1..])))
} else {
find_suffix()
Ok(find_suffix())
}
}

Expand Down Expand Up @@ -1979,6 +2010,17 @@ fn anchor_failure(
});
}

/// Report an error in the link disambiguator.
fn disambiguator_error(
cx: &DocContext<'_>,
item: &Item,
dox: &str,
link_range: Range<usize>,
msg: &str,
) {
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.
fn ambiguity_error(
cx: &DocContext<'_>,
Expand Down
13 changes: 13 additions & 0 deletions src/test/rustdoc-ui/intra-doc/unknown-disambiguator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +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 ``
//! And with weird backticks: [``foo@hello``] [foo`@`hello].
//~^ ERROR unknown disambiguator `foo`
//~| ERROR unknown disambiguator `foo`

fn main() {}
45 changes: 45 additions & 0 deletions src/test/rustdoc-ui/intra-doc/unknown-disambiguator.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error: unknown disambiguator `foo`
--> $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:1:9
|
LL | #![deny(warnings)]
| ^^^^^^^^
= note: `#[deny(rustdoc::broken_intra_doc_links)]` implied by `#[deny(warnings)]`

error: unknown disambiguator `bar`
--> $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:6:31
|
LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()).
| ^
Comment on lines +14 to +36
Copy link
Member Author

@camelid camelid Mar 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is caused by my change, but any idea why it's reporting diagnostics for line 3, then line 9, and then back to line 6? It's kind of confusing that they're out of order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, sorry. Probably it's something to do with how pulldown-cmark returns the links.


error: unknown disambiguator ``
--> $DIR/unknown-disambiguator.rs:6:57
|
LL | //! And to [no disambiguator](@nectarine) and [another](@apricot!()).
| ^

error: aborting due to 6 previous errors