Skip to content

Commit

Permalink
docs: also check the inline stmt during redundant link check
Browse files Browse the repository at this point in the history
  • Loading branch information
bvanjoi committed Feb 7, 2024
1 parent d4f6f9e commit 0a50dba
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
// but there's already an item with the same namespace and same name. Rust gives
// priority to the not-imported one, so we should, too.
items.extend(doc.items.values().flat_map(|(item, renamed, import_id)| {
// First, lower everything other than imports.
// First, lower everything other than glob imports.
if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) {
return Vec::new();
}
Expand Down
49 changes: 32 additions & 17 deletions src/librustdoc/passes/lint/redundant_explicit_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_hir::def::{DefKind, DocLinkResMap, Namespace, Res};
use rustc_hir::HirId;
use rustc_lint_defs::Applicability;
use rustc_resolve::rustdoc::source_span_for_markdown_range;
use rustc_span::def_id::DefId;
use rustc_span::Symbol;

use crate::clean::utils::find_nearest_parent_module;
Expand All @@ -33,17 +34,22 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
return;
}

if item.link_names(&cx.cache).is_empty() {
// If there's no link names in this item,
// then we skip resolution querying to
// avoid from panicking.
return;
if let Some(item_id) = item.def_id() {
check_redundant_explicit_link_for_did(cx, item, item_id, hir_id, &doc);
}
if let Some(item_id) = item.inline_stmt_id {
check_redundant_explicit_link_for_did(cx, item, item_id, hir_id, &doc);
}
}

let Some(item_id) = item.def_id() else {
return;
};
let Some(local_item_id) = item_id.as_local() else {
fn check_redundant_explicit_link_for_did<'md>(
cx: &DocContext<'_>,
item: &Item,
did: DefId,
hir_id: HirId,
doc: &'md str,
) {
let Some(local_item_id) = did.as_local() else {
return;
};

Expand All @@ -53,19 +59,34 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
return;
}
let is_private = !cx.render_options.document_private
&& !cx.cache.effective_visibilities.is_directly_public(cx.tcx, item_id);
&& !cx.cache.effective_visibilities.is_directly_public(cx.tcx, did);
if is_private {
return;
}

check_redundant_explicit_link(cx, item, hir_id, &doc);
let module_id = match cx.tcx.def_kind(did) {
DefKind::Mod if item.inner_docs(cx.tcx) => did,
_ => find_nearest_parent_module(cx.tcx, did).unwrap(),
};

let Some(resolutions) =
cx.tcx.resolutions(()).doc_link_resolutions.get(&module_id.expect_local())
else {
// If there's no resolutions in this module,
// then we skip resolution querying to
// avoid from panicking.
return;
};

check_redundant_explicit_link(cx, item, hir_id, &doc, &resolutions);
}

fn check_redundant_explicit_link<'md>(
cx: &DocContext<'_>,
item: &Item,
hir_id: HirId,
doc: &'md str,
resolutions: &DocLinkResMap,
) -> Option<()> {
let mut broken_line_callback = |link: BrokenLink<'md>| Some((link.reference, "".into()));
let mut offset_iter = Parser::new_with_broken_link_callback(
Expand All @@ -74,12 +95,6 @@ fn check_redundant_explicit_link<'md>(
Some(&mut broken_line_callback),
)
.into_offset_iter();
let item_id = item.def_id()?;
let module_id = match cx.tcx.def_kind(item_id) {
DefKind::Mod if item.inner_docs(cx.tcx) => item_id,
_ => find_nearest_parent_module(cx.tcx, item_id).unwrap(),
};
let resolutions = cx.tcx.doc_link_resolutions(module_id);

while let Some((event, link_range)) = offset_iter.next() {
match event {
Expand Down
17 changes: 17 additions & 0 deletions tests/rustdoc-ui/issues/issue-120444-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: --document-private-items

#![deny(rustdoc::redundant_explicit_links)]

mod webdavfs {
pub struct A;
pub struct B;
}

/// [`Vfs`][crate::Vfs]
pub use webdavfs::A;
//~^^ error: redundant explicit link target

/// [`Vfs`]
pub use webdavfs::B;

pub struct Vfs;
22 changes: 22 additions & 0 deletions tests/rustdoc-ui/issues/issue-120444-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: redundant explicit link target
--> $DIR/issue-120444-1.rs:10:13
|
LL | /// [`Vfs`][crate::Vfs]
| ----- ^^^^^^^^^^ explicit target is redundant
| |
| because label contains path that resolves to same destination
|
= note: when a link's destination is not specified,
the label is used to resolve intra-doc links
note: the lint level is defined here
--> $DIR/issue-120444-1.rs:3:9
|
LL | #![deny(rustdoc::redundant_explicit_links)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: remove explicit link target
|
LL | /// [`Vfs`]
| ~~~~~~~

error: aborting due to 1 previous error

17 changes: 17 additions & 0 deletions tests/rustdoc-ui/issues/issue-120444-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: --document-private-items

#![deny(rustdoc::redundant_explicit_links)]

pub mod webdavfs {
pub struct A;
pub struct B;
}

/// [`Vfs`][crate::Vfs]
pub use webdavfs::A;
//~^^ error: redundant explicit link target

/// [`Vfs`]
pub use webdavfs::B;

pub struct Vfs;
22 changes: 22 additions & 0 deletions tests/rustdoc-ui/issues/issue-120444-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: redundant explicit link target
--> $DIR/issue-120444-2.rs:10:13
|
LL | /// [`Vfs`][crate::Vfs]
| ----- ^^^^^^^^^^ explicit target is redundant
| |
| because label contains path that resolves to same destination
|
= note: when a link's destination is not specified,
the label is used to resolve intra-doc links
note: the lint level is defined here
--> $DIR/issue-120444-2.rs:3:9
|
LL | #![deny(rustdoc::redundant_explicit_links)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: remove explicit link target
|
LL | /// [`Vfs`]
| ~~~~~~~

error: aborting due to 1 previous error

0 comments on commit 0a50dba

Please sign in to comment.