Deny broken intra-doc links in linkchecker

jyn514 · jyn514 · commit 65835d1059b9 · 2020-10-15T20:22:16.000-04:00
Since rustdoc isn't warning about these links, check for them manually.
diff --git a/Cargo.lock b/Cargo.lock
@@ -1744,6 +1744,10 @@ dependencies = [
 [[package]]
 name = "linkchecker"
 version = "0.1.0"
+dependencies = [
+ "once_cell",
+ "regex",
+]
 
 [[package]]
 name = "linked-hash-map"
diff --git a/compiler/rustc_error_codes/src/error_codes/E0660.md b/compiler/rustc_error_codes/src/error_codes/E0660.md
@@ -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
diff --git a/compiler/rustc_error_codes/src/error_codes/E0661.md b/compiler/rustc_error_codes/src/error_codes/E0661.md
@@ -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
diff --git a/compiler/rustc_error_codes/src/error_codes/E0662.md b/compiler/rustc_error_codes/src/error_codes/E0662.md
@@ -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
diff --git a/compiler/rustc_error_codes/src/error_codes/E0663.md b/compiler/rustc_error_codes/src/error_codes/E0663.md
@@ -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
diff --git a/compiler/rustc_error_codes/src/error_codes/E0664.md b/compiler/rustc_error_codes/src/error_codes/E0664.md
@@ -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
diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs
@@ -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
@@ -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.
     ///
@@ -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.
@@ -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.
     ///
@@ -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.
diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs
@@ -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
diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs
@@ -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> {
diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs
@@ -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
     ///
diff --git a/library/core/src/option.rs b/library/core/src/option.rs
@@ -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 {
diff --git a/library/std/src/ffi/c_str.rs b/library/std/src/ffi/c_str.rs
@@ -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
diff --git a/src/doc/unstable-book/src/library-features/default-free-fn.md b/src/doc/unstable-book/src/library-features/default-free-fn.md
@@ -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
diff --git a/src/tools/linkchecker/Cargo.toml b/src/tools/linkchecker/Cargo.toml
@@ -7,3 +7,7 @@ edition = "2018"
 [[bin]]
 name = "linkchecker"
 path = "main.rs"
+
+[dependencies]
+regex = "1"
+once_cell = "1"
diff --git a/src/tools/linkchecker/main.rs b/src/tools/linkchecker/main.rs
@@ -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
@@ -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])] = &[
+    // 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 {
@@ -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)
@@ -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)
 }
 
