Skip to content

rustdoc: improve refdef handling in the unresolved link lint #136363

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

Merged
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
18 changes: 16 additions & 2 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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!(
Expand All @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is run on a compiler's happy path, when rustdoc and its lints are not involved.
Could you do a perf run before merging this?

if !refids.contains(label) {
links.push(preprocess_link(&refdef.dest));
}
}

links
}

Expand Down
4 changes: 0 additions & 4 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`"]
Expand Down Expand Up @@ -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`"]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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")]
Expand Down
7 changes: 2 additions & 5 deletions library/core/src/task/wake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
70 changes: 67 additions & 3 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 => {
Expand All @@ -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!(),
};
Expand All @@ -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
}

Expand Down
43 changes: 34 additions & 9 deletions tests/rustdoc-ui/intra-doc/weird-syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines -126 to -127
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment was wrong.

struct@Clone is URL-ish enough to match CommonMark's refdef grammar. The reason it was rejected before was because the //~ERROR link hot comment got fed to the markdown parser, and, as a result, caused the refdef to not get parsed there.

By using a below-the-line comment, I can work around that and demonstrate the change correctly.

/// 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;
66 changes: 45 additions & 21 deletions tests/rustdoc-ui/intra-doc/weird-syntax.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 `\]`
Expand All @@ -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

Loading