Auto merge of rust-lang#136363 - notriddle:notriddle/unresolved-link-unused-refdef, r=GuillaumeGomez

rustdoc: improve refdef handling in the unresolved link lint

This commit takes advantage of a feature in pulldown-cmark that makes the list of link definitions available to the consuming application. It produces unresolved link warnings for refdefs that aren't used, and can now produce exact spans for the dest even when it has escapes.

Closes rust-lang#133150 since this lint would have caught the mistake in that issue, and, along with rust-lang/rust-clippy#13707, most mistakes in this class should produce a warning from one of them.

Author: bors

compiler/rustc_resolve/src/rustdoc.rs

@@ -7,7 +7,7 @@ use pulldown_cmark::{
 use rustc_ast as ast;
 use rustc_ast::attr::AttributeExt;
 use rustc_ast::util::comments::beautify_doc_string;
-use rustc_data_structures::fx::FxIndexMap;
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
 use rustc_span::{DUMMY_SP, InnerSpan, Span, Symbol, kw, sym};
@@ -422,9 +422,11 @@ fn parse_links<'md>(doc: &'md str) -> Vec<Box<str>> {
     );
     let mut links = Vec::new();
 
+    let mut refids = FxHashSet::default();
+
     while let Some(event) = event_iter.next() {
         match event {
-            Event::Start(Tag::Link { link_type, dest_url, title: _, id: _ })
+            Event::Start(Tag::Link { link_type, dest_url, title: _, id })
                 if may_be_doc_link(link_type) =>
             {
                 if matches!(
@@ -439,13 +441,25 @@ fn parse_links<'md>(doc: &'md str) -> Vec<Box<str>> {
                         links.push(display_text);
                     }
                 }
+                if matches!(
+                    link_type,
+                    LinkType::Reference | LinkType::Shortcut | LinkType::Collapsed
+                ) {
+                    refids.insert(id);
+                }
 
                 links.push(preprocess_link(&dest_url));
             }
             _ => {}
         }
     }
 
+    for (label, refdef) in event_iter.reference_definitions().iter() {
+        if !refids.contains(label) {
+            links.push(preprocess_link(&refdef.dest));
+        }
+    }
+
     links
 }
 

library/alloc/src/boxed.rs

@@ -1053,7 +1053,6 @@ impl<T: ?Sized> Box<T> {
     /// ```
     ///
     /// [memory layout]: self#memory-layout
-    /// [`Layout`]: crate::Layout
     #[stable(feature = "box_raw", since = "1.4.0")]
     #[inline]
     #[must_use = "call `drop(Box::from_raw(ptr))` if you intend to drop the `Box`"]
@@ -1108,7 +1107,6 @@ impl<T: ?Sized> Box<T> {
     /// ```
     ///
     /// [memory layout]: self#memory-layout
-    /// [`Layout`]: crate::Layout
     #[unstable(feature = "box_vec_non_null", reason = "new API", issue = "130364")]
     #[inline]
     #[must_use = "call `drop(Box::from_non_null(ptr))` if you intend to drop the `Box`"]
@@ -1165,7 +1163,6 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
     /// ```
     ///
     /// [memory layout]: self#memory-layout
-    /// [`Layout`]: crate::Layout
     #[unstable(feature = "allocator_api", issue = "32838")]
     #[rustc_const_unstable(feature = "const_box", issue = "92521")]
     #[inline]
@@ -1219,7 +1216,6 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
     /// ```
     ///
     /// [memory layout]: self#memory-layout
-    /// [`Layout`]: crate::Layout
     #[unstable(feature = "allocator_api", issue = "32838")]
     // #[unstable(feature = "box_vec_non_null", reason = "new API", issue = "130364")]
     #[rustc_const_unstable(feature = "const_box", issue = "92521")]

library/core/src/task/wake.rs

@@ -40,17 +40,14 @@ impl RawWaker {
     /// of the `vtable` as the first parameter.
     ///
     /// It is important to consider that the `data` pointer must point to a
-    /// thread safe type such as an `[Arc]<T: Send + Sync>`
+    /// thread safe type such as an `Arc<T: Send + Sync>`
     /// when used to construct a [`Waker`]. This restriction is lifted when
     /// constructing a [`LocalWaker`], which allows using types that do not implement
-    /// <code>[Send] + [Sync]</code> like `[Rc]<T>`.
+    /// <code>[Send] + [Sync]</code> like `Rc<T>`.
     ///
     /// The `vtable` customizes the behavior of a `Waker` which gets created
     /// from a `RawWaker`. For each operation on the `Waker`, the associated
     /// function in the `vtable` of the underlying `RawWaker` will be called.
-    ///
-    /// [`Arc`]: std::sync::Arc
-    /// [`Rc`]: std::rc::Rc
     #[inline]
     #[rustc_promotable]
     #[stable(feature = "futures_api", since = "1.36.0")]

src/librustdoc/html/markdown.rs

@@ -38,7 +38,7 @@ use std::sync::{Arc, Weak};
 use pulldown_cmark::{
     BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html,
 };
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_errors::{Diag, DiagMessage};
 use rustc_hir::def_id::LocalDefId;
 use rustc_middle::ty::TyCtxt;
@@ -1763,6 +1763,46 @@ pub(crate) fn markdown_links<'md, R>(
         }
     };
 
+    let span_for_refdef = |link: &CowStr<'_>, span: Range<usize>| {
+        // We want to underline the link's definition, but `span` will point at the entire refdef.
+        // Skip the label, then try to find the entire URL.
+        let mut square_brace_count = 0;
+        let mut iter = md.as_bytes()[span.start..span.end].iter().copied().enumerate();
+        for (_i, c) in &mut iter {
+            match c {
+                b':' if square_brace_count == 0 => break,
+                b'[' => square_brace_count += 1,
+                b']' => square_brace_count -= 1,
+                _ => {}
+            }
+        }
+        while let Some((i, c)) = iter.next() {
+            if c == b'<' {
+                while let Some((j, c)) = iter.next() {
+                    match c {
+                        b'\\' => {
+                            let _ = iter.next();
+                        }
+                        b'>' => {
+                            return MarkdownLinkRange::Destination(
+                                i + 1 + span.start..j + span.start,
+                            );
+                        }
+                        _ => {}
+                    }
+                }
+            } else if !c.is_ascii_whitespace() {
+                while let Some((j, c)) = iter.next() {
+                    if c.is_ascii_whitespace() {
+                        return MarkdownLinkRange::Destination(i + span.start..j + span.start);
+                    }
+                }
+                return MarkdownLinkRange::Destination(i + span.start..span.end);
+            }
+        }
+        span_for_link(link, span)
+    };
+
     let span_for_offset_backward = |span: Range<usize>, open: u8, close: u8| {
         let mut open_brace = !0;
         let mut close_brace = !0;
@@ -1844,9 +1884,16 @@ pub(crate) fn markdown_links<'md, R>(
     .into_offset_iter();
     let mut links = Vec::new();
 
+    let mut refdefs = FxIndexMap::default();
+    for (label, refdef) in event_iter.reference_definitions().iter() {
+        refdefs.insert(label.to_string(), (false, refdef.dest.to_string(), refdef.span.clone()));
+    }
+
     for (event, span) in event_iter {
         match event {
-            Event::Start(Tag::Link { link_type, dest_url, .. }) if may_be_doc_link(link_type) => {
+            Event::Start(Tag::Link { link_type, dest_url, id, .. })
+                if may_be_doc_link(link_type) =>
+            {
                 let range = match link_type {
                     // Link is pulled from the link itself.
                     LinkType::ReferenceUnknown | LinkType::ShortcutUnknown => {
@@ -1856,7 +1903,12 @@ pub(crate) fn markdown_links<'md, R>(
                     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_url, span)
+                        if let Some((is_used, dest_url, span)) = refdefs.get_mut(&id[..]) {
+                            *is_used = true;
+                            span_for_refdef(&CowStr::from(&dest_url[..]), span.clone())
+                        } else {
+                            span_for_link(&dest_url, span)
+                        }
                     }
                     LinkType::Autolink | LinkType::Email => unreachable!(),
                 };
@@ -1873,6 +1925,18 @@ pub(crate) fn markdown_links<'md, R>(
         }
     }
 
+    for (_label, (is_used, dest_url, span)) in refdefs.into_iter() {
+        if !is_used
+            && let Some(link) = preprocess_link(MarkdownLink {
+                kind: LinkType::Reference,
+                range: span_for_refdef(&CowStr::from(&dest_url[..]), span),
+                link: dest_url,
+            })
+        {
+            links.push(link);
+        }
+    }
+
     links
 }
 

tests/rustdoc-ui/intra-doc/weird-syntax.rs

@@ -117,24 +117,49 @@ pub struct WLinkToCloneWithUnmatchedEscapedCloseParenAndDoubleSpace;
 
 // References
 
-/// The [cln][] link here is going to be unresolved, because `Clone()` gets rejected //~ERROR link
-/// in Markdown for not being URL-shaped enough.
-///
-/// [cln]: Clone() //~ERROR link
+/// The [cln][] link here is going to be unresolved, because `Clone()` gets
+//~^ ERROR link
+/// rejected in Markdown for not being URL-shaped enough.
+/// [cln]: Clone()
+//~^ ERROR link
 pub struct LinkToCloneWithParensInReference;
 
-/// The [cln][] link here is going to be unresolved, because `struct@Clone` gets //~ERROR link
-/// rejected in Markdown for not being URL-shaped enough.
+/// The [cln][] link here is going to produce a good inline suggestion
 ///
-/// [cln]: struct@Clone //~ERROR link
+/// [cln]: struct@Clone
+//~^ ERROR link
 pub struct LinkToCloneWithWrongPrefix;
 
-/// The [cln][] link here will produce a plain text suggestion //~ERROR link
+/// The [cln][] link here will produce a good inline suggestion
 ///
 /// [cln]: Clone\(\)
+//~^ ERROR link
 pub struct LinkToCloneWithEscapedParensInReference;
 
-/// The [cln][] link here will produce a plain text suggestion //~ERROR link
+/// The [cln][] link here will produce a good inline suggestion
 ///
 /// [cln]: struct\@Clone
+//~^ ERROR link
 pub struct LinkToCloneWithEscapedAtsInReference;
+
+
+/// This link reference definition isn't used, but since it is still parsed,
+/// it should still produce a warning.
+///
+/// [cln]: struct\@Clone
+//~^ ERROR link
+pub struct UnusedLinkToCloneReferenceDefinition;
+
+/// <https://github.com/rust-lang/rust/issues/133150>
+///
+/// - [`SDL_PROP_WINDOW_CREATE_COCOA_WINDOW_POINTER`]: the
+//~^ ERROR link
+///   `(__unsafe_unretained)` NSWindow associated with the window, if you want
+///   to wrap an existing window.
+/// - [`SDL_PROP_WINDOW_CREATE_COCOA_VIEW_POINTER`]: the `(__unsafe_unretained)`
+///   NSView associated with the window, defaults to `[window contentView]`
+pub fn a() {}
+#[allow(nonstandard_style)]
+pub struct SDL_PROP_WINDOW_CREATE_COCOA_WINDOW_POINTER;
+#[allow(nonstandard_style)]
+pub struct SDL_PROP_WINDOW_CREATE_COCOA_VIEW_POINTER;

tests/rustdoc-ui/intra-doc/weird-syntax.stderr

@@ -230,7 +230,7 @@ LL | /// [w](Clone  \))
 error: unresolved link to `cln`
   --> $DIR/weird-syntax.rs:120:10
    |
-LL | /// The [cln][] link here is going to be unresolved, because `Clone()` gets rejected
+LL | /// The [cln][] link here is going to be unresolved, because `Clone()` gets
    |          ^^^ no item named `cln` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
@@ -243,37 +243,61 @@ LL | /// [cln]: Clone()
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
 
-error: unresolved link to `cln`
-  --> $DIR/weird-syntax.rs:126:10
+error: incompatible link kind for `Clone`
+  --> $DIR/weird-syntax.rs:129:12
    |
-LL | /// The [cln][] link here is going to be unresolved, because `struct@Clone` gets
-   |          ^^^ no item named `cln` in scope
+LL | /// [cln]: struct@Clone
+   |            ^^^^^^^^^^^^ this link resolved to a trait, which is not a struct
    |
-   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
-
-error: unresolved link to `cln`
-  --> $DIR/weird-syntax.rs:129:6
+help: to link to the trait, prefix with `trait@`
    |
-LL | /// [cln]: struct@Clone
-   |      ^^^ no item named `cln` in scope
+LL - /// [cln]: struct@Clone
+LL + /// [cln]: trait@Clone
    |
-   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
 
 error: unresolved link to `Clone`
-  --> $DIR/weird-syntax.rs:132:9
+  --> $DIR/weird-syntax.rs:135:12
+   |
+LL | /// [cln]: Clone\(\)
+   |            ^^^^^^^^^ this link resolves to the trait `Clone`, which is not a function
+   |
+help: to link to the trait, prefix with `trait@`
    |
-LL | /// The [cln][] link here will produce a plain text suggestion
-   |         ^^^^^ this link resolves to the trait `Clone`, which is not a function
+LL - /// [cln]: Clone\(\)
+LL + /// [cln]: trait@Clone
    |
-   = help: to link to the trait, prefix with `trait@`: trait@Clone
 
 error: incompatible link kind for `Clone`
-  --> $DIR/weird-syntax.rs:137:9
+  --> $DIR/weird-syntax.rs:141:12
    |
-LL | /// The [cln][] link here will produce a plain text suggestion
-   |         ^^^^^ this link resolved to a trait, which is not a struct
+LL | /// [cln]: struct\@Clone
+   |            ^^^^^^^^^^^^^ this link resolved to a trait, which is not a struct
    |
-   = help: to link to the trait, prefix with `trait@`: trait@Clone
+help: to link to the trait, prefix with `trait@`
+   |
+LL - /// [cln]: struct\@Clone
+LL + /// [cln]: trait@struct
+   |
+
+error: incompatible link kind for `Clone`
+  --> $DIR/weird-syntax.rs:149:12
+   |
+LL | /// [cln]: struct\@Clone
+   |            ^^^^^^^^^^^^^ this link resolved to a trait, which is not a struct
+   |
+help: to link to the trait, prefix with `trait@`
+   |
+LL - /// [cln]: struct\@Clone
+LL + /// [cln]: trait@struct
+   |
+
+error: unresolved link to `the`
+  --> $DIR/weird-syntax.rs:155:56
+   |
+LL | /// - [`SDL_PROP_WINDOW_CREATE_COCOA_WINDOW_POINTER`]: the
+   |                                                        ^^^ no item named `the` in scope
+   |
+   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
 
-error: aborting due to 26 previous errors
+error: aborting due to 27 previous errors