From 61a97448e51bafab3f94a7c4ccb5d43c6c97ad22 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 31 Jan 2025 11:55:53 -0700 Subject: [PATCH 1/2] 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. --- compiler/rustc_resolve/src/rustdoc.rs | 18 ++++- src/librustdoc/html/markdown.rs | 70 ++++++++++++++++++- tests/rustdoc-ui/intra-doc/weird-syntax.rs | 43 +++++++++--- .../rustdoc-ui/intra-doc/weird-syntax.stderr | 66 +++++++++++------ 4 files changed, 162 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index fecb973501933..52aaab77ebed6 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/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> { ); 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,6 +441,12 @@ fn parse_links<'md>(doc: &'md str) -> Vec> { links.push(display_text); } } + if matches!( + link_type, + LinkType::Reference | LinkType::Shortcut | LinkType::Collapsed + ) { + refids.insert(id); + } links.push(preprocess_link(&dest_url)); } @@ -446,6 +454,12 @@ fn parse_links<'md>(doc: &'md str) -> Vec> { } } + for (label, refdef) in event_iter.reference_definitions().iter() { + if !refids.contains(label) { + links.push(preprocess_link(&refdef.dest)); + } + } + links } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index bd8eda2fed621..d9e49577d3929 100644 --- a/src/librustdoc/html/markdown.rs +++ b/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| { + // 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, 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 } diff --git a/tests/rustdoc-ui/intra-doc/weird-syntax.rs b/tests/rustdoc-ui/intra-doc/weird-syntax.rs index ca18842fb21c5..d2a922b2b6247 100644 --- a/tests/rustdoc-ui/intra-doc/weird-syntax.rs +++ b/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; + +/// +/// +/// - [`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; diff --git a/tests/rustdoc-ui/intra-doc/weird-syntax.stderr b/tests/rustdoc-ui/intra-doc/weird-syntax.stderr index 1381c1b31ebbf..ad813f0f9b623 100644 --- a/tests/rustdoc-ui/intra-doc/weird-syntax.stderr +++ b/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 From 4d551dd754477bb81ddef0829121ca90c8a8b286 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 31 Jan 2025 14:28:44 -0700 Subject: [PATCH 2/2] docs: fix broken intra-doc links that never worked --- library/alloc/src/boxed.rs | 4 ---- library/core/src/task/wake.rs | 7 ++----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 1ea7b731461e7..4b124b5a3b38f 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1053,7 +1053,6 @@ impl Box { /// ``` /// /// [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 Box { /// ``` /// /// [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 Box { /// ``` /// /// [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 Box { /// ``` /// /// [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")] diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index 4c51ca0a5e437..3f57b04753a6b 100644 --- a/library/core/src/task/wake.rs +++ b/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]` + /// thread safe type such as an `Arc` /// when used to construct a [`Waker`]. This restriction is lifted when /// constructing a [`LocalWaker`], which allows using types that do not implement - /// [Send] + [Sync] like `[Rc]`. + /// [Send] + [Sync] like `Rc`. /// /// 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")]