Auto merge of rust-lang#113167 - ChAoSUnItY:redundant_explicit_link, r=GuillaumeGomez

rustdoc: Add lint `redundant_explicit_links`

Closes rust-lang#87799.
- Lint warns by default
- Reworks link parser to cache original link's display text

r? `@jyn514`

Author: bors

compiler/rustc_borrowck/src/consumers.rs

@@ -30,7 +30,7 @@ pub use super::{
 /// will be retrieved.
 #[derive(Debug, Copy, Clone)]
 pub enum ConsumerOptions {
-    /// Retrieve the [`Body`] along with the [`BorrowSet`](super::borrow_set::BorrowSet)
+    /// Retrieve the [`Body`] along with the [`BorrowSet`]
     /// and [`RegionInferenceContext`]. If you would like the body only, use
     /// [`TyCtxt::mir_promoted`].
     ///

compiler/rustc_errors/src/lib.rs

@@ -452,11 +452,11 @@ struct HandlerInner {
     /// have been converted.
     check_unstable_expect_diagnostics: bool,
 
-    /// Expected [`Diagnostic`][diagnostic::Diagnostic]s store a [`LintExpectationId`] as part of
+    /// Expected [`Diagnostic`][struct@diagnostic::Diagnostic]s store a [`LintExpectationId`] as part of
     /// the lint level. [`LintExpectationId`]s created early during the compilation
     /// (before `HirId`s have been defined) are not stable and can therefore not be
     /// stored on disk. This buffer stores these diagnostics until the ID has been
-    /// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`][diagnostic::Diagnostic]s are the
+    /// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`][struct@diagnostic::Diagnostic]s are the
     /// submitted for storage and added to the list of fulfilled expectations.
     unstable_expect_diagnostics: Vec<Diagnostic>,
 

compiler/rustc_resolve/src/rustdoc.rs

@@ -1,4 +1,4 @@
-use pulldown_cmark::{BrokenLink, Event, LinkType, Options, Parser, Tag};
+use pulldown_cmark::{BrokenLink, CowStr, Event, LinkType, Options, Parser, Tag};
 use rustc_ast as ast;
 use rustc_ast::util::comments::beautify_doc_string;
 use rustc_data_structures::fx::FxHashMap;
@@ -392,16 +392,73 @@ pub(crate) fn attrs_to_preprocessed_links(attrs: &[ast::Attribute]) -> Vec<Box<s
     let (doc_fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true);
     let doc = prepare_to_doc_link_resolution(&doc_fragments).into_values().next().unwrap();
 
-    Parser::new_with_broken_link_callback(
+    parse_links(&doc)
+}
+
+/// Similiar version of `markdown_links` from rustdoc.
+/// This will collect destination links and display text if exists.
+fn parse_links<'md>(doc: &'md str) -> Vec<Box<str>> {
+    let mut broken_link_callback = |link: BrokenLink<'md>| Some((link.reference, "".into()));
+    let mut event_iter = Parser::new_with_broken_link_callback(
         &doc,
         main_body_opts(),
-        Some(&mut |link: BrokenLink<'_>| Some((link.reference, "".into()))),
+        Some(&mut broken_link_callback),
     )
-    .filter_map(|event| match event {
-        Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => {
-            Some(preprocess_link(&dest))
+    .into_iter();
+    let mut links = Vec::new();
+
+    while let Some(event) = event_iter.next() {
+        match event {
+            Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => {
+                if matches!(
+                    link_type,
+                    LinkType::Inline
+                        | LinkType::ReferenceUnknown
+                        | LinkType::Reference
+                        | LinkType::Shortcut
+                        | LinkType::ShortcutUnknown
+                ) {
+                    if let Some(display_text) = collect_link_data(&mut event_iter) {
+                        links.push(display_text);
+                    }
+                }
+
+                links.push(preprocess_link(&dest));
+            }
+            _ => {}
+        }
+    }
+
+    links
+}
+
+/// Collects additional data of link.
+fn collect_link_data<'input, 'callback>(
+    event_iter: &mut Parser<'input, 'callback>,
+) -> Option<Box<str>> {
+    let mut display_text: Option<String> = None;
+    let mut append_text = |text: CowStr<'_>| {
+        if let Some(display_text) = &mut display_text {
+            display_text.push_str(&text);
+        } else {
+            display_text = Some(text.to_string());
+        }
+    };
+
+    while let Some(event) = event_iter.next() {
+        match event {
+            Event::Text(text) => {
+                append_text(text);
+            }
+            Event::Code(code) => {
+                append_text(code);
+            }
+            Event::End(_) => {
+                break;
+            }
+            _ => {}
         }
-        _ => None,
-    })
-    .collect()
+    }
+
+    display_text.map(String::into_boxed_str)
 }

library/alloc/src/lib.rs

@@ -89,6 +89,7 @@
 #![allow(explicit_outlives_requirements)]
 #![warn(multiple_supertrait_upcastable)]
 #![cfg_attr(not(bootstrap), allow(internal_features))]
+#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))]
 //
 // Library features:
 // tidy-alphabetical-start

library/alloc/src/sync.rs

@@ -153,7 +153,7 @@ macro_rules! acquire {
 ///
 /// ## `Deref` behavior
 ///
-/// `Arc<T>` automatically dereferences to `T` (via the [`Deref`][deref] trait),
+/// `Arc<T>` automatically dereferences to `T` (via the [`Deref`] trait),
 /// so you can call `T`'s methods on a value of type `Arc<T>`. To avoid name
 /// clashes with `T`'s methods, the methods of `Arc<T>` itself are associated
 /// functions, called using [fully qualified syntax]:
@@ -187,7 +187,6 @@ macro_rules! acquire {
 /// [mutex]: ../../std/sync/struct.Mutex.html
 /// [rwlock]: ../../std/sync/struct.RwLock.html
 /// [atomic]: core::sync::atomic
-/// [deref]: core::ops::Deref
 /// [downgrade]: Arc::downgrade
 /// [upgrade]: Weak::upgrade
 /// [RefCell\<T>]: core::cell::RefCell
@@ -1495,7 +1494,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
     /// alignment as `T`. This is trivially true if `U` is `T`.
     /// Note that if `U` is not `T` but has the same size and alignment, this is
     /// basically like transmuting references of different types. See
-    /// [`mem::transmute`][transmute] for more information on what
+    /// [`mem::transmute`] for more information on what
     /// restrictions apply in this case.
     ///
     /// The raw pointer must point to a block of memory allocated by `alloc`
@@ -1507,7 +1506,6 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
     /// even if the returned `Arc<T>` is never accessed.
     ///
     /// [into_raw]: Arc::into_raw
-    /// [transmute]: core::mem::transmute
     ///
     /// # Examples
     ///

library/core/src/lib.rs

@@ -97,6 +97,8 @@
 #![allow(incomplete_features)]
 #![warn(multiple_supertrait_upcastable)]
 #![cfg_attr(not(bootstrap), allow(internal_features))]
+// Do not check link redundancy on bootstraping phase
+#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))]
 //
 // Library features:
 // tidy-alphabetical-start

library/std/src/lib.rs

@@ -223,6 +223,7 @@
 #![cfg_attr(not(bootstrap), allow(internal_features))]
 #![deny(rustc::existing_doc_keyword)]
 #![deny(fuzzy_provenance_casts)]
+#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))]
 // Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind`
 #![deny(ffi_unwind_calls)]
 // std may use features in a platform-specific way

src/doc/rustdoc/src/lints.md

@@ -412,3 +412,37 @@ help: if you meant to use a literal backtick, escape it
 
 warning: 1 warning emitted
 ```
+
+## `redundant_explicit_links`
+
+This lint is **warned by default**. It detects explicit links that are same
+as computed automatic links.
+This usually means the explicit links is removeable. For example:
+
+```rust
+#![warn(rustdoc::redundant_explicit_links)] // note: unnecessary - warns by default.
+
+/// add takes 2 [`usize`](usize) and performs addition
+/// on them, then returns result.
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+```
+
+Which will give:
+
+```text
+error: redundant explicit rustdoc link
+  --> src/lib.rs:3:27
+   |
+3  | /// add takes 2 [`usize`](usize) and performs addition
+   |                           ^^^^^
+   |
+   = note: Explicit link does not affect the original link
+note: the lint level is defined here
+  --> src/lib.rs:1:9
+   |
+1  | #![deny(rustdoc::redundant_explicit_links)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: Remove explicit link instead
+```

src/librustdoc/html/markdown.rs

@@ -50,7 +50,7 @@ use crate::html::render::small_url_encode;
 use crate::html::toc::TocBuilder;
 
 use pulldown_cmark::{
-    html, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag,
+    html, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, OffsetIter, Options, Parser, Tag,
 };
 
 #[cfg(test)]
@@ -1240,6 +1240,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin
 pub(crate) struct MarkdownLink {
     pub kind: LinkType,
     pub link: String,
+    pub display_text: Option<String>,
     pub range: MarkdownLinkRange,
 }
 
@@ -1263,8 +1264,8 @@ impl MarkdownLinkRange {
     }
 }
 
-pub(crate) fn markdown_links<R>(
-    md: &str,
+pub(crate) fn markdown_links<'md, R>(
+    md: &'md str,
     preprocess_link: impl Fn(MarkdownLink) -> Option<R>,
 ) -> Vec<R> {
     if md.is_empty() {
@@ -1375,32 +1376,90 @@ pub(crate) fn markdown_links<R>(
         MarkdownLinkRange::Destination(range.clone())
     };
 
-    Parser::new_with_broken_link_callback(
+    let mut broken_link_callback = |link: BrokenLink<'md>| Some((link.reference, "".into()));
+    let mut event_iter = Parser::new_with_broken_link_callback(
         md,
         main_body_opts(),
-        Some(&mut |link: BrokenLink<'_>| Some((link.reference, "".into()))),
+        Some(&mut broken_link_callback),
     )
-    .into_offset_iter()
-    .filter_map(|(event, span)| match event {
-        Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => {
-            let range = match link_type {
-                // Link is pulled from the link itself.
-                LinkType::ReferenceUnknown | LinkType::ShortcutUnknown => {
-                    span_for_offset_backward(span, b'[', b']')
-                }
-                LinkType::CollapsedUnknown => span_for_offset_forward(span, b'[', b']'),
-                LinkType::Inline => span_for_offset_backward(span, b'(', b')'),
-                // Link is pulled from elsewhere in the document.
-                LinkType::Reference | LinkType::Collapsed | LinkType::Shortcut => {
-                    span_for_link(&dest, span)
+    .into_offset_iter();
+    let mut links = Vec::new();
+
+    while let Some((event, span)) = event_iter.next() {
+        match event {
+            Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => {
+                let range = match link_type {
+                    // Link is pulled from the link itself.
+                    LinkType::ReferenceUnknown | LinkType::ShortcutUnknown => {
+                        span_for_offset_backward(span, b'[', b']')
+                    }
+                    LinkType::CollapsedUnknown => span_for_offset_forward(span, b'[', b']'),
+                    LinkType::Inline => span_for_offset_backward(span, b'(', b')'),
+                    // Link is pulled from elsewhere in the document.
+                    LinkType::Reference | LinkType::Collapsed | LinkType::Shortcut => {
+                        span_for_link(&dest, span)
+                    }
+                    LinkType::Autolink | LinkType::Email => unreachable!(),
+                };
+
+                let display_text = if matches!(
+                    link_type,
+                    LinkType::Inline
+                        | LinkType::ReferenceUnknown
+                        | LinkType::Reference
+                        | LinkType::Shortcut
+                        | LinkType::ShortcutUnknown
+                ) {
+                    collect_link_data(&mut event_iter)
+                } else {
+                    None
+                };
+
+                if let Some(link) = preprocess_link(MarkdownLink {
+                    kind: link_type,
+                    link: dest.into_string(),
+                    display_text,
+                    range,
+                }) {
+                    links.push(link);
                 }
-                LinkType::Autolink | LinkType::Email => unreachable!(),
-            };
-            preprocess_link(MarkdownLink { kind: link_type, range, link: dest.into_string() })
+            }
+            _ => {}
         }
-        _ => None,
-    })
-    .collect()
+    }
+
+    links
+}
+
+/// Collects additional data of link.
+fn collect_link_data<'input, 'callback>(
+    event_iter: &mut OffsetIter<'input, 'callback>,
+) -> Option<String> {
+    let mut display_text: Option<String> = None;
+    let mut append_text = |text: CowStr<'_>| {
+        if let Some(display_text) = &mut display_text {
+            display_text.push_str(&text);
+        } else {
+            display_text = Some(text.to_string());
+        }
+    };
+
+    while let Some((event, _span)) = event_iter.next() {
+        match event {
+            Event::Text(text) => {
+                append_text(text);
+            }
+            Event::Code(code) => {
+                append_text(code);
+            }
+            Event::End(_) => {
+                break;
+            }
+            _ => {}
+        }
+    }
+
+    display_text
 }
 
 #[derive(Debug)]

src/librustdoc/lint.rs

@@ -185,6 +185,17 @@ declare_rustdoc_lint! {
    "detects unescaped backticks in doc comments"
 }
 
+declare_rustdoc_lint! {
+    /// This lint is **warned by default**. It detects explicit links that are same
+    /// as computed automatic links. This usually means the explicit links is removeable.
+    /// This is a `rustdoc` only lint, see the documentation in the [rustdoc book].
+    ///
+    /// [rustdoc book]: ../../../rustdoc/lints.html#redundant_explicit_links
+    REDUNDANT_EXPLICIT_LINKS,
+    Warn,
+    "detects redundant explicit links in doc comments"
+}
+
 pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
     vec![
         BROKEN_INTRA_DOC_LINKS,
@@ -197,6 +208,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
         BARE_URLS,
         MISSING_CRATE_LEVEL_DOCS,
         UNESCAPED_BACKTICKS,
+        REDUNDANT_EXPLICIT_LINKS,
     ]
 });