Skip to content

rustdoc: Add lint redundant_explicit_links #113167

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 26 commits into from
Aug 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b1d232a
rework link parsing loop
ChAoSUnItY Jun 29, 2023
da582a7
Add warn level lint `redundant_explicit_links`
ChAoSUnItY Jun 29, 2023
65e24a5
Fix resolution caching
ChAoSUnItY Jun 29, 2023
1c6b237
add more tests
ChAoSUnItY Jun 29, 2023
f1b23f2
bless test output
ChAoSUnItY Jun 29, 2023
e583318
fix trailing whitespace
ChAoSUnItY Jun 29, 2023
c736989
Refactor lint from rustc to rustdoc
ChAoSUnItY Jun 30, 2023
46df958
Support Reference & ReferenceUnknown link lint
ChAoSUnItY Jun 30, 2023
5ce6cc7
Still resolving rustdoc resolution panicking
ChAoSUnItY Jun 30, 2023
0e2f2cc
Add check-pass tests and fix test behavior
ChAoSUnItY Jun 30, 2023
fe17ae3
add missing deny lint
ChAoSUnItY Jun 30, 2023
78c85f4
fomar files
ChAoSUnItY Jun 30, 2023
ecb2637
narrow down the lint trigger constraint
ChAoSUnItY Jul 2, 2023
f0b2cca
lint links
ChAoSUnItY Jul 3, 2023
713e78c
fix
ChAoSUnItY Jul 3, 2023
2ec3e29
tidy doc link
ChAoSUnItY Jul 3, 2023
c4afb8a
resolve conflicts
ChAoSUnItY Jul 3, 2023
4896fc0
resolve conflicts
ChAoSUnItY Jul 3, 2023
15ece93
relax redundancy constraint
ChAoSUnItY Jul 3, 2023
62113f6
fix `unescaped_backticks` error
ChAoSUnItY Jul 3, 2023
1476b39
Skip lint check when item is not fully public
ChAoSUnItY Jul 20, 2023
25919b0
Add regression test for inline doc
ChAoSUnItY Jul 20, 2023
23c9a4a
resolve conflicts
ChAoSUnItY Jul 21, 2023
8e34c68
Fix private function importing
ChAoSUnItY Aug 18, 2023
e17d2da
Fix format
ChAoSUnItY Aug 18, 2023
297ff8c
Fix redundant explicit link in rustc_borrowck
ChAoSUnItY Aug 18, 2023
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
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
///
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,

Expand Down
75 changes: 66 additions & 9 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand All @@ -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
///
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions src/doc/rustdoc/src/lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
107 changes: 83 additions & 24 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
}

Expand All @@ -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() {
Expand Down Expand Up @@ -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)]
Expand Down
12 changes: 12 additions & 0 deletions src/librustdoc/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
]
});

Expand Down
Loading