Skip to content

Commit

Permalink
Rollup merge of #120702 - bvanjoi:fix-120444, r=notriddle
Browse files Browse the repository at this point in the history
docs: also check the inline stmt during redundant link check

Fixes #120444

This issue was brought about by querying `root::webdavfs::A`, a key that doesn't exist in `doc_link_resolutions`. To avoid a panic, I've altered the gating mechanism to allow this lint pass to be skipped.

I'm not certain if this is the best solution. An alternative approach might be to leverage other info from the name resolutions instead of `doc_link_resolutions`. After all, all we need is to get the resolution from a combination of `(module, name)`. However, I believe they would yield the same outcome, both skipping this lint.
  • Loading branch information
matthiaskrgr authored Feb 7, 2024
2 parents 55d078e + 0a50dba commit e7e77b4
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 e7e77b4

Please sign in to comment.