Skip to content
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

docs: also check the inline stmt during redundant link check #120702

Merged
merged 1 commit into from
Feb 8, 2024
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
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Difference between issue-120444-1 and issue-120444-2:

  • In issue-120444-1, it is declared as mod webavfs;
  • In issue-120444-2, it is declared as pub mod webavfs

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

Loading