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

Deny broken intra-doc links in linkchecker #77971

Merged
merged 2 commits into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1744,6 +1744,10 @@ dependencies = [
[[package]]
name = "linkchecker"
version = "0.1.0"
dependencies = [
"once_cell",
"regex",
]

[[package]]
name = "linked-hash-map"
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0660.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ llvm_asm!("nop" "nop");
Considering that this would be a long explanation, we instead recommend you
take a look at the [`llvm_asm`] chapter of the Unstable book:

[llvm_asm]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
[`llvm_asm`]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0661.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ llvm_asm!("nop" : "r"(a));
Considering that this would be a long explanation, we instead recommend you
take a look at the [`llvm_asm`] chapter of the Unstable book:

[llvm_asm]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
[`llvm_asm`]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0662.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ llvm_asm!("xor %eax, %eax"
Considering that this would be a long explanation, we instead recommend you
take a look at the [`llvm_asm`] chapter of the Unstable book:

[llvm_asm]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
[`llvm_asm`]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0663.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ llvm_asm!("xor %eax, %eax"
Considering that this would be a long explanation, we instead recommend you
take a look at the [`llvm_asm`] chapter of the Unstable book:

[llvm_asm]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
[`llvm_asm`]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0664.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ llvm_asm!("mov $$0x200, %eax"
Considering that this would be a long explanation, we instead recommend you
take a look at the [`llvm_asm`] chapter of the Unstable book:

[llvm_asm]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
[`llvm_asm`]: https://doc.rust-lang.org/stable/unstable-book/library-features/llvm-asm.html
12 changes: 3 additions & 9 deletions library/core/src/alloc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,11 @@ impl fmt::Display for AllocError {
pub unsafe trait AllocRef {
/// Attempts to allocate a block of memory.
///
/// On success, returns a [`NonNull<[u8]>`] meeting the size and alignment guarantees of `layout`.
/// On success, returns a [`NonNull<[u8]>`][NonNull] meeting the size and alignment guarantees of `layout`.
///
/// The returned block may have a larger size than specified by `layout.size()`, and may or may
/// not have its contents initialized.
///
/// [`NonNull<[u8]>`]: NonNull
///
/// # Errors
///
/// Returning `Err` indicates that either memory is exhausted or `layout` does not meet
Expand Down Expand Up @@ -146,7 +144,7 @@ pub unsafe trait AllocRef {

/// Attempts to extend the memory block.
///
/// Returns a new [`NonNull<[u8]>`] containing a pointer and the actual size of the allocated
/// Returns a new [`NonNull<[u8]>`][NonNull] containing a pointer and the actual size of the allocated
/// memory. The pointer is suitable for holding data described by `new_layout`. To accomplish
/// this, the allocator may extend the allocation referenced by `ptr` to fit the new layout.
///
Expand All @@ -158,8 +156,6 @@ pub unsafe trait AllocRef {
/// If this method returns `Err`, then ownership of the memory block has not been transferred to
/// this allocator, and the contents of the memory block are unaltered.
///
/// [`NonNull<[u8]>`]: NonNull
///
/// # Safety
///
/// * `ptr` must denote a block of memory [*currently allocated*] via this allocator.
Expand Down Expand Up @@ -271,7 +267,7 @@ pub unsafe trait AllocRef {

/// Attempts to shrink the memory block.
///
/// Returns a new [`NonNull<[u8]>`] containing a pointer and the actual size of the allocated
/// Returns a new [`NonNull<[u8]>`][NonNull] containing a pointer and the actual size of the allocated
/// memory. The pointer is suitable for holding data described by `new_layout`. To accomplish
/// this, the allocator may shrink the allocation referenced by `ptr` to fit the new layout.
///
Expand All @@ -283,8 +279,6 @@ pub unsafe trait AllocRef {
/// If this method returns `Err`, then ownership of the memory block has not been transferred to
/// this allocator, and the contents of the memory block are unaltered.
///
/// [`NonNull<[u8]>`]: NonNull
///
/// # Safety
///
/// * `ptr` must denote a block of memory [*currently allocated*] via this allocator.
Expand Down
1 change: 1 addition & 0 deletions library/core/src/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ pub const fn identity<T>(x: T) -> T {
/// want to accept all references that can be converted to [`&str`] as an argument.
/// Since both [`String`] and [`&str`] implement `AsRef<str>` we can accept both as input argument.
///
/// [`&str`]: primitive@str
/// [`Option<T>`]: Option
/// [`Result<T, E>`]: Result
/// [`Borrow`]: crate::borrow::Borrow
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/iter/traits/double_ended.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ pub trait DoubleEndedIterator: Iterator {
/// assert_eq!(iter.advance_back_by(0), Ok(()));
/// assert_eq!(iter.advance_back_by(100), Err(1)); // only `&3` was skipped
/// ```
///
/// [`Ok(())`]: Ok
/// [`Err(k)`]: Err
#[inline]
#[unstable(feature = "iter_advance_by", reason = "recently added", issue = "77404")]
fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
Expand Down
6 changes: 3 additions & 3 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,12 @@ pub trait Iterator {
/// This method will eagerly skip `n` elements by calling [`next`] up to `n`
/// times until [`None`] is encountered.
///
/// `advance_by(n)` will return [`Ok(())`] if the iterator successfully advances by
/// `n` elements, or [`Err(k)`] if [`None`] is encountered, where `k` is the number
/// `advance_by(n)` will return [`Ok(())`][Ok] if the iterator successfully advances by
/// `n` elements, or [`Err(k)`][Err] if [`None`] is encountered, where `k` is the number
/// of elements the iterator is advanced by before running out of elements (i.e. the
/// length of the iterator). Note that `k` is always less than `n`.
///
/// Calling `advance_by(0)` does not consume any elements and always returns [`Ok(())`].
/// Calling `advance_by(0)` does not consume any elements and always returns [`Ok(())`][Ok].
///
/// [`next`]: Iterator::next
///
Expand Down
1 change: 1 addition & 0 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ impl<T> Option<T> {
/// assert_eq!(Some(4).filter(is_even), Some(4));
/// ```
///
/// [`Some(t)`]: Some
#[inline]
#[stable(feature = "option_filter", since = "1.27.0")]
pub fn filter<P: FnOnce(&T) -> bool>(self, predicate: P) -> Self {
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,8 @@ impl CStr {
/// [`U+FFFD REPLACEMENT CHARACTER`][U+FFFD] and return a
/// [`Cow`]`::`[`Owned`]`(`[`String`]`)` with the result.
///
/// [`str`]: prim@str
/// [`str`]: primitive@str
/// [`&str`]: primitive@str
/// [`Borrowed`]: Cow::Borrowed
/// [`Owned`]: Cow::Owned
/// [U+FFFD]: crate::char::REPLACEMENT_CHARACTER
Expand Down
2 changes: 1 addition & 1 deletion src/doc/embedded-book
2 changes: 2 additions & 0 deletions src/doc/unstable-book/src/library-features/default-free-fn.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Adds a free `default()` function to the `std::default` module. This function
just forwards to [`Default::default()`], but may remove repetition of the word
"default" from the call site.

[`Default::default()`]: https://doc.rust-lang.org/nightly/std/default/trait.Default.html#tymethod.default

Here is an example:

```rust
Expand Down
2 changes: 1 addition & 1 deletion src/tools/cargo
4 changes: 4 additions & 0 deletions src/tools/linkchecker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@ edition = "2018"
[[bin]]
name = "linkchecker"
path = "main.rs"

[dependencies]
regex = "1"
once_cell = "1"
62 changes: 62 additions & 0 deletions src/tools/linkchecker/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ use std::fs;
use std::path::{Component, Path, PathBuf};
use std::rc::Rc;

use once_cell::sync::Lazy;
use regex::Regex;

use crate::Redirect::*;

// Add linkcheck exceptions here
Expand Down Expand Up @@ -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])] = &[
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

That sounds really risky. :-/

Copy link
Member Author

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.

// This will never have links that are not in other pages.
// To avoid repeating the exceptions twice, an empty list means all broken links are allowed.
("reference/print.html", &[]),
// All the reference 'links' are actually ENBF highlighted as code
("reference/comments.html", &[
"/</code> <code>!",
"*</code> <code>!",
]),
("reference/identifiers.html", &[
"a</code>-<code>z</code> <code>A</code>-<code>Z",
"a</code>-<code>z</code> <code>A</code>-<code>Z</code> <code>0</code>-<code>9</code> <code>_",
"a</code>-<code>z</code> <code>A</code>-<code>Z</code>] [<code>a</code>-<code>z</code> <code>A</code>-<code>Z</code> <code>0</code>-<code>9</code> <code>_",
]),
("reference/tokens.html", &[
"0</code>-<code>1",
"0</code>-<code>7",
"0</code>-<code>9",
"0</code>-<code>9",
"0</code>-<code>9</code> <code>a</code>-<code>f</code> <code>A</code>-<code>F",
]),
("reference/notation.html", &[
"b</code> <code>B",
"a</code>-<code>z",
]),
// This is being used in the sense of 'inclusive range', not a markdown link
("core/ops/struct.RangeInclusive.html", &["begin</code>, <code>end"]),
("std/ops/struct.RangeInclusive.html", &["begin</code>, <code>end"]),
("core/slice/trait.SliceIndex.html", &["begin</code>, <code>end"]),
("alloc/slice/trait.SliceIndex.html", &["begin</code>, <code>end"]),
("std/slice/trait.SliceIndex.html", &["begin</code>, <code>end"]),

];

static BROKEN_INTRA_DOC_LINK: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"\[<code>(.*)</code>\]"#).unwrap());

macro_rules! t {
($e:expr) => {
match $e {
Expand Down Expand Up @@ -138,6 +179,14 @@ fn walk(cache: &mut Cache, root: &Path, dir: &Path, errors: &mut bool) {
}
}

fn is_intra_doc_exception(file: &Path, link: &str) -> bool {
if let Some(entry) = INTRA_DOC_LINK_EXCEPTIONS.iter().find(|&(f, _)| file.ends_with(f)) {
entry.1.is_empty() || entry.1.contains(&link)
} else {
false
}
}

fn is_exception(file: &Path, link: &str) -> bool {
if let Some(entry) = LINKCHECK_EXCEPTIONS.iter().find(|&(f, _)| file.ends_with(f)) {
entry.1.contains(&link)
Expand Down Expand Up @@ -292,6 +341,19 @@ fn check(cache: &mut Cache, root: &Path, file: &Path, errors: &mut bool) -> Opti
}
}
});

// Search for intra-doc links that rustdoc didn't warn about
// FIXME(#77199, 77200) Rustdoc should just warn about these directly.
// NOTE: only looks at one line at a time; in practice this should find most links
for (i, line) in contents.lines().enumerate() {
for broken_link in BROKEN_INTRA_DOC_LINK.captures_iter(line) {
if !is_intra_doc_exception(file, &broken_link[1]) {
*errors = true;
print!("{}:{}: broken intra-doc link - ", pretty_file.display(), i + 1);
println!("{}", &broken_link[0]);
}
}
}
Some(pretty_file)
}

Expand Down