Skip to content

get rid of some false negatives in rustdoc::broken_intra_doc_links #132748

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
+290 −26
Open

get rid of some false negatives in rustdoc::broken_intra_doc_links #132748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ca4b042
get rid of some false negatives in rustdoc::broken_intra_doc_links
lolbinarycat Nov 7, 2024
53cb8b0
Update src/librustdoc/passes/collect_intra_doc_links.rs
lolbinarycat Nov 7, 2024
cee9e0a
Update tests/rustdoc-ui/bad-intra-doc.rs
lolbinarycat Nov 7, 2024
a20414f
fix formatting
lolbinarycat Nov 7, 2024
ddd7540
add more error annotations
lolbinarycat Nov 7, 2024
f330a98
go back to ignoring "intra-doc links" with spaces in them
lolbinarycat Nov 8, 2024
e20ff19
fix braketing error
lolbinarycat Nov 8, 2024
6747e21
adjust more unit tests to reflect more aggressive intra-doc linting
lolbinarycat Nov 8, 2024
333f802
rustdoc::broken_intra_doc_links: no backticks = use old behavior
lolbinarycat Apr 18, 2025
5485535
rustdoc: update tests to match new lint behavior
lolbinarycat Apr 18, 2025
b3fda87
rustdoc::broken_intra_doc_links: only be lenient with shortcut links
lolbinarycat Apr 19, 2025
f5df571
tests/rustdoc-json: directives should not use doc comments
lolbinarycat Apr 19, 2025
32a73bf
move bad-intra-doc test into intra-doc dir
lolbinarycat Apr 19, 2025
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
31 changes: 28 additions & 3 deletions src/librustdoc/passes/collect_intra_doc_links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,13 +931,20 @@ fn preprocess_link(
ori_link: &MarkdownLink,
dox: &str,
) -> Option<Result<PreprocessingInfo, PreprocessingError>> {
// certain link kinds cannot have their path be urls,
// so they should not be ignored, no matter how much they look like urls.
// e.g. [https://example.com/] is not a link to example.com.
let can_be_url = ori_link.kind != LinkType::ShortcutUnknown
&& ori_link.kind != LinkType::CollapsedUnknown
&& ori_link.kind != LinkType::ReferenceUnknown;
Comment on lines +937 to +939
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let can_be_url = ori_link.kind != LinkType::ShortcutUnknown
&& ori_link.kind != LinkType::CollapsedUnknown
&& ori_link.kind != LinkType::ReferenceUnknown;
let can_be_url = !matches!(
ori_link.kind,
LinkType::ShortcutUnknown | LinkType::CollapsedUnknown | LinkType::ReferenceUnknown
);

maybe?


// [] is mostly likely not supposed to be a link
if ori_link.link.is_empty() {
return None;
}

// Bail early for real links.
if ori_link.link.contains('/') {
if can_be_url && ori_link.link.contains('/') {
return None;
}

Expand All @@ -962,7 +969,7 @@ fn preprocess_link(
Ok(None) => (None, link, link),
Err((err_msg, relative_range)) => {
// Only report error if we would not have ignored this link. See issue #83859.
if !should_ignore_link_with_disambiguators(link) {
if !(can_be_url && should_ignore_link_with_disambiguators(link)) {
let disambiguator_range = match range_between_backticks(&ori_link.range, dox) {
MarkdownLinkRange::Destination(no_backticks_range) => {
MarkdownLinkRange::Destination(
Expand All @@ -979,7 +986,25 @@ fn preprocess_link(
}
};

if should_ignore_link(path_str) {
// If there's no backticks, be lenient revert to old behavior.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// If there's no backticks, be lenient revert to old behavior.
// If there's no backticks, be lenient and revert to the old behavior.

I think?

// This is to prevent churn by linting on stuff that isn't meant to be a link.
// only shortcut links have simple enough syntax that they
// are likely to be written accidentlly, collapsed and reference links
// need 4 metachars, and reference links will not usually use
// backticks in the reference name.
// therefore, only shortcut syntax gets the lenient behavior.
//
// here's a truth table for how link kinds that cannot be urls are handled:
//
// |-------------------------------------------------------|
// | | is shortcut link | not shortcut link |
// |--------------|--------------------|-------------------|
// | has backtick | never ignore | never ignore |
// | no backtick | ignore if url-like | never ignore |
// |-------------------------------------------------------|
let ignore_urllike =
can_be_url || (ori_link.kind == LinkType::ShortcutUnknown && !ori_link.link.contains('`'));
if ignore_urllike && should_ignore_link(path_str) {
Comment on lines +999 to +1007
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the complete truth table is:

unknown shortcut link unknown reference / collapsed link other link
has backtick never ignore never ignore ignore if url-like
no backtick ignore if url-like never ignore ignore if url-like

Assuming I'm understanding the logic correctly (the difference is to things like [first](second)).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct. Technically I do explicitly define the scope of the existing table with the line "here's a truth table for how link kinds that cannot be urls are handled:"

the line is currently 66 chars long, i should be able to fit in the other case without upsetting tidy (100 char limit), do you want me to?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I should've read it more closely.

return None;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc-json/fns/return_type_alias.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Regression test for <https://github.com/rust-lang/rust/issues/104851>

///@ set foo = "$.index[?(@.name=='Foo')].id"
//@ set foo = "$.index[?(@.name=='Foo')].id"
pub type Foo = i32;

//@ is "$.index[?(@.name=='demo')].inner.function.sig.output.resolved_path.id" $foo
Expand Down
30 changes: 15 additions & 15 deletions tests/rustdoc-ui/disambiguator-endswith-named-suffix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,67 +2,67 @@
//@ normalize-stderr: "nightly|beta|1\.[0-9][0-9]\.[0-9]" -> "$$CHANNEL"

//! [struct@m!()] //~ WARN: unmatched disambiguator `struct` and suffix `!()`
//! [struct@m!{}]
//! [struct@m!{}] //~ WARN: unmatched disambiguator `struct` and suffix `!{}`
//! [struct@m![]]
//! [struct@f()] //~ WARN: unmatched disambiguator `struct` and suffix `()`
//! [struct@m!] //~ WARN: unmatched disambiguator `struct` and suffix `!`
//!
//! [enum@m!()] //~ WARN: unmatched disambiguator `enum` and suffix `!()`
//! [enum@m!{}]
//! [enum@m!{}] //~ WARN: unmatched disambiguator `enum` and suffix `!{}`
//! [enum@m![]]
//! [enum@f()] //~ WARN: unmatched disambiguator `enum` and suffix `()`
//! [enum@m!] //~ WARN: unmatched disambiguator `enum` and suffix `!`
//!
//! [trait@m!()] //~ WARN: unmatched disambiguator `trait` and suffix `!()`
//! [trait@m!{}]
//! [trait@m!{}] //~ WARN: unmatched disambiguator `trait` and suffix `!{}`
//! [trait@m![]]
//! [trait@f()] //~ WARN: unmatched disambiguator `trait` and suffix `()`
//! [trait@m!] //~ WARN: unmatched disambiguator `trait` and suffix `!`
//!
//! [module@m!()] //~ WARN: unmatched disambiguator `module` and suffix `!()`
//! [module@m!{}]
//! [module@m!{}] //~ WARN: unmatched disambiguator `module` and suffix `!{}`
//! [module@m![]]
//! [module@f()] //~ WARN: unmatched disambiguator `module` and suffix `()`
//! [module@m!] //~ WARN: unmatched disambiguator `module` and suffix `!`
//!
//! [mod@m!()] //~ WARN: unmatched disambiguator `mod` and suffix `!()`
//! [mod@m!{}]
//! [mod@m!{}] //~ WARN: unmatched disambiguator `mod` and suffix `!{}`
//! [mod@m![]]
//! [mod@f()] //~ WARN: unmatched disambiguator `mod` and suffix `()`
//! [mod@m!] //~ WARN: unmatched disambiguator `mod` and suffix `!`
//!
//! [const@m!()] //~ WARN: unmatched disambiguator `const` and suffix `!()`
//! [const@m!{}]
//! [const@m!{}] //~ WARN: unmatched disambiguator `const` and suffix `!{}`
//! [const@m![]]
//! [const@f()] //~ WARN: incompatible link kind for `f`
//! [const@m!] //~ WARN: unmatched disambiguator `const` and suffix `!`
//!
//! [constant@m!()] //~ WARN: unmatched disambiguator `constant` and suffix `!()`
//! [constant@m!{}]
//! [constant@m!{}] //~ WARN: unmatched disambiguator `constant` and suffix `!{}`
//! [constant@m![]]
//! [constant@f()] //~ WARN: incompatible link kind for `f`
//! [constant@m!] //~ WARN: unmatched disambiguator `constant` and suffix `!`
//!
//! [static@m!()] //~ WARN: unmatched disambiguator `static` and suffix `!()`
//! [static@m!{}]
//! [static@m!{}] //~ WARN: unmatched disambiguator `static` and suffix `!{}`
//! [static@m![]]
//! [static@f()] //~ WARN: incompatible link kind for `f`
//! [static@m!] //~ WARN: unmatched disambiguator `static` and suffix `!`
//!
//! [function@m!()] //~ WARN: unmatched disambiguator `function` and suffix `!()`
//! [function@m!{}]
//! [function@m!{}] //~ WARN: unmatched disambiguator `function` and suffix `!{}`
//! [function@m![]]
//! [function@f()]
//! [function@m!] //~ WARN: unmatched disambiguator `function` and suffix `!`
//!
//! [fn@m!()] //~ WARN: unmatched disambiguator `fn` and suffix `!()`
//! [fn@m!{}]
//! [fn@m!{}] //~ WARN: unmatched disambiguator `fn` and suffix `!{}`
//! [fn@m![]]
//! [fn@f()]
//! [fn@m!] //~ WARN: unmatched disambiguator `fn` and suffix `!`
//!
//! [method@m!()] //~ WARN: unmatched disambiguator `method` and suffix `!()`
//! [method@m!{}]
//! [method@m!{}] //~ WARN: unmatched disambiguator `method` and suffix `!{}`
//! [method@m![]]
//! [method@f()]
//! [method@m!] //~ WARN: unmatched disambiguator `method` and suffix `!`
Expand All @@ -74,13 +74,13 @@
//! [derive@m!] //~ WARN: incompatible link kind for `m`
//!
//! [type@m!()] //~ WARN: unmatched disambiguator `type` and suffix `!()`
//! [type@m!{}]
//! [type@m!{}] //~ WARN: unmatched disambiguator `type` and suffix `!{}`
//! [type@m![]]
//! [type@f()] //~ WARN: unmatched disambiguator `type` and suffix `()`
//! [type@m!] //~ WARN: unmatched disambiguator `type` and suffix `!`
//!
//! [value@m!()] //~ WARN: unmatched disambiguator `value` and suffix `!()`
//! [value@m!{}]
//! [value@m!{}] //~ WARN: unmatched disambiguator `value` and suffix `!{}`
//! [value@m![]]
//! [value@f()]
//! [value@m!] //~ WARN: unmatched disambiguator `value` and suffix `!`
Expand All @@ -92,13 +92,13 @@
//! [macro@m!]
//!
//! [prim@m!()] //~ WARN: unmatched disambiguator `prim` and suffix `!()`
//! [prim@m!{}]
//! [prim@m!{}] //~ WARN: unmatched disambiguator `prim` and suffix `!{}`
//! [prim@m![]]
//! [prim@f()] //~ WARN: unmatched disambiguator `prim` and suffix `()`
//! [prim@m!] //~ WARN: unmatched disambiguator `prim` and suffix `!`
//!
//! [primitive@m!()] //~ WARN: unmatched disambiguator `primitive` and suffix `!()`
//! [primitive@m!{}]
//! [primitive@m!{}] //~ WARN: unmatched disambiguator `primitive` and suffix `!{}`
//! [primitive@m![]]
//! [primitive@f()] //~ WARN: unmatched disambiguator `primitive` and suffix `()`
//! [primitive@m!] //~ WARN: unmatched disambiguator `primitive` and suffix `!`
Expand Down
Loading
Loading