-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Deny broken intra-doc links in linkchecker #77971
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
This caught quite a lot of broken links:
|
The ones in the reference seem to be intentional, they're ~EBNF and marking literals as code blocks. I'll ignore them in the linkchecker. |
e3147d1
to
110fd0a
Compare
Fix broken link in reference docs Found in rust-lang/rust#77971.
110fd0a
to
dd161d9
Compare
This is now blocked on a cargo update, rust-embedded/book#271 merged, and an |
271: Fix broken link to std::process::Command r=therealprof a=jyn514 Intra-doc links do not yet work in markdown files; see rust-lang/rust#77974. Part of rust-lang/rust#77971. Co-authored-by: Joshua Nelson <jyn514@gmail.com>
@@ -50,6 +53,44 @@ const LINKCHECK_EXCEPTIONS: &[(&str, &[&str])] = &[ | |||
("alloc/collections/btree_set/struct.BTreeSet.html", &["#insert-and-complex-keys"]), | |||
]; | |||
|
|||
#[rustfmt::skip] | |||
const INTRA_DOC_LINK_EXCEPTIONS: &[(&str, &[&str])] = &[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want to run this check on html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there's no way for the linkchecker to tell if a link will be resolved by rustdoc or not. It has to look at the HTML to see whether the markdown was transformed into a link or left as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds really risky. :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what you mean by risky. What would be a better way? This inherently wants to look at the generated HTML, not the original, because you could have concat!(
and #[doc = ...]
and all sorts of other things that linkchecker
doesn't and shouldn't know about.
Since rustdoc isn't warning about these links, check for them manually.
- [rust-embedded](rust-embedded/book@79ab777...ca8169e) - [cargo](rust-lang/cargo@12db56c...79b397d)
dd161d9
to
b221819
Compare
Ok, this should be ready to go now. I updated two submodules that had broken links (the commit message has a |
This looks good to me. Let's wait for @Mark-Simulacrum's review too. |
I guess this seems fine, r=me. It does feel like the sort of thing that would be better to fix in rustdoc, but I'm sure you're aware of that :) |
Oh, one note I wanted to make -- it look like this is now fixing links, could you update PR description? |
Updated!
Yeah ... I'd like to have #77200 and #77199 but the rest of the rustdoc team disagrees with me (#77276 (comment), various conversations on discord). In the meantime this should catch most of the broken links. This is still a good change even if those issues were fixed though IMO - it protects against bugs in rustdoc (and hopefully makes them easier to find). |
📌 Commit b221819 has been approved by |
…rk-simulacrum Deny broken intra-doc links in linkchecker Since rustdoc isn't warning about these links, check for them manually. This also fixes the broken links that popped up from the lint.
…rk-simulacrum Deny broken intra-doc links in linkchecker Since rustdoc isn't warning about these links, check for them manually. This also fixes the broken links that popped up from the lint.
Rollup of 10 pull requests Successful merges: - rust-lang#75209 (Suggest imports of unresolved macros) - rust-lang#77547 (stabilize union with 'ManuallyDrop' fields and 'impl Drop for Union') - rust-lang#77827 (Don't link to nightly primitives on stable channel) - rust-lang#77855 (resolve: further improvements to "try using the enum's variant" diagnostic) - rust-lang#77900 (Use fdatasync for File::sync_data on more OSes) - rust-lang#77925 (Suggest minimal subset features in `incomplete_features` lint) - rust-lang#77971 (Deny broken intra-doc links in linkchecker) - rust-lang#77991 (Bump backtrace-rs) - rust-lang#77992 (instrument-coverage: try our best to not ICE) - rust-lang#78013 (Fix sidebar scroll on mobile devices) Failed merges: r? `@ghost`
linkchecker: Remove unneeded FIXME about intra-doc links It was added by rust-lang#77971 but the adder [proposed](rust-lang#77971 (comment)) that the added code is a good fallback to have in case rustdoc gets buggy, and I agree. So remove the FIXME. This PR is part of rust-lang#44366 which is E-help-wanted. r? `@jyn514` since you added the FIXME `@rustbot` label T-dev-tools
Since rustdoc isn't warning about these links, check for them manually.
This also fixes the broken links that popped up from the lint.