From da95d5cd88d675423870218c77adb2fdf0bcc207 Mon Sep 17 00:00:00 2001 From: Qai Juang <237468078+qaijuang@users.noreply.github.com> Date: Thu, 30 Apr 2026 11:33:54 -0400 Subject: [PATCH] rustdoc: check redundant explicit links against generated URLs --- .../passes/lint/redundant_explicit_links.rs | 132 ++++++++++++------ tests/rustdoc-ui/lints/no-redundancy.rs | 6 + .../lints/redundant_explicit_links.fixed | 7 + .../lints/redundant_explicit_links.rs | 7 + .../lints/redundant_explicit_links.stderr | 18 ++- 5 files changed, 128 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index cd7b7caac69ad..de2414ccb262d 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -12,9 +12,11 @@ use rustc_resolve::rustdoc::{prepare_to_doc_link_resolution, source_span_for_mar use rustc_span::def_id::DefId; use rustc_span::{Span, Symbol}; -use crate::clean::Item; use crate::clean::utils::{find_nearest_parent_module, inherits_doc_hidden}; +use crate::clean::{Item, inline}; use crate::core::DocContext; +use crate::formats::item_type::ItemType; +use crate::html::format::href_relative_parts; use crate::html::markdown::main_body_opts; #[derive(Debug)] @@ -71,12 +73,13 @@ fn check_redundant_explicit_link_for_did( return; }; - check_redundant_explicit_link(cx, item, hir_id, doc, resolutions); + check_redundant_explicit_link(cx, item, module_id, hir_id, doc, resolutions); } fn check_redundant_explicit_link<'md>( cx: &DocContext<'_>, item: &Item, + module_id: DefId, hir_id: HirId, doc: &'md str, resolutions: &DocLinkResMap, @@ -114,41 +117,41 @@ fn check_redundant_explicit_link<'md>( continue; } - if dest_url.ends_with(resolvable_link) || resolvable_link.ends_with(&*dest_url) { - let check_result = match link_type { - LinkType::Inline | LinkType::ReferenceUnknown => { - check_inline_or_reference_unknown_redundancy( - cx, - item, - hir_id, - doc, - resolutions, - link_range, - dest_url.to_string(), - link_data, - if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, - ) - } - LinkType::Reference => check_reference_redundancy( + let check_result = match link_type { + LinkType::Inline | LinkType::ReferenceUnknown => { + check_inline_or_reference_unknown_redundancy( cx, item, + module_id, hir_id, doc, resolutions, link_range, - &dest_url, + dest_url.to_string(), link_data, - ), - _ => Ok(()), - }; - if let Err(lint) = check_result { - cx.tcx.emit_node_span_lint( - crate::lint::REDUNDANT_EXPLICIT_LINKS, - hir_id, - item.attr_span(cx.tcx), - lint, - ); + if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, + ) } + LinkType::Reference => check_reference_redundancy( + cx, + item, + module_id, + hir_id, + doc, + resolutions, + link_range, + &dest_url, + link_data, + ), + _ => Ok(()), + }; + if let Err(lint) = check_result { + cx.tcx.emit_node_span_lint( + crate::lint::REDUNDANT_EXPLICIT_LINKS, + hir_id, + item.attr_span(cx.tcx), + lint, + ); } } } @@ -179,6 +182,7 @@ impl<'a> Diagnostic<'a, ()> for RedundantExplicitLinksWithoutSuggestion { fn check_inline_or_reference_unknown_redundancy( cx: &DocContext<'_>, item: &Item, + module_id: DefId, hir_id: HirId, doc: &str, resolutions: &DocLinkResMap, @@ -226,13 +230,8 @@ fn check_inline_or_reference_unknown_redundancy( else { return Ok(()); }; - let (Some(dest_res), Some(display_res)) = - (find_resolution(resolutions, &dest), find_resolution(resolutions, resolvable_link)) - else { - return Ok(()); - }; - if dest_res == display_res { + if explicit_link_is_redundant(cx, module_id, resolutions, &dest, resolvable_link) { let attr_span = item.attr_span(cx.tcx); let link_span = match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) @@ -302,6 +301,7 @@ fn check_inline_or_reference_unknown_redundancy( fn check_reference_redundancy( cx: &DocContext<'_>, item: &Item, + module_id: DefId, hir_id: HirId, doc: &str, resolutions: &DocLinkResMap, @@ -347,13 +347,8 @@ fn check_reference_redundancy( else { return Ok(()); }; - let (Some(dest_res), Some(display_res)) = - (find_resolution(resolutions, dest), find_resolution(resolutions, resolvable_link)) - else { - return Ok(()); - }; - if dest_res == display_res { + if explicit_link_is_redundant(cx, module_id, resolutions, dest, resolvable_link) { let attr_span = item.attr_span(cx.tcx); let link_span = match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) @@ -437,6 +432,61 @@ fn check_reference_redundancy( Ok(()) } +fn explicit_link_is_redundant( + cx: &DocContext<'_>, + module_id: DefId, + resolutions: &DocLinkResMap, + dest: &str, + resolvable_link: &str, +) -> bool { + let Some(display_res) = find_resolution(resolutions, resolvable_link) else { + return false; + }; + + if (dest.ends_with(resolvable_link) || resolvable_link.ends_with(dest)) + && find_resolution(resolutions, dest).is_some_and(|dest_res| dest_res == display_res) + { + return true; + } + + if dest.contains('#') || !dest.ends_with(".html") { + return false; + } + + local_href_for_res(cx, module_id, display_res).is_some_and(|href| href == dest) +} + +fn local_href_for_res(cx: &DocContext<'_>, module_id: DefId, res: Res) -> Option { + let mut did = res.opt_def_id()?; + if matches!(cx.tcx.def_kind(did), DefKind::Ctor(..)) { + did = cx.tcx.parent(did); + } + + if matches!( + cx.tcx.def_kind(did), + DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst { .. } | DefKind::Variant + ) || !did.is_local() + { + return None; + } + + let item_type = ItemType::from_def_id(did, cx.tcx); + let fqp = inline::get_item_path(cx.tcx, did, item_type); + let module_fqp = if item_type == ItemType::Module { &fqp[..] } else { &fqp[..fqp.len() - 1] }; + let current_fqp = inline::get_item_path(cx.tcx, module_id, ItemType::Module); + + let mut url_parts = href_relative_parts(module_fqp, ¤t_fqp); + match item_type { + ItemType::Module => url_parts.push("index.html"), + _ => url_parts.push_fmt(format_args!( + "{}.{last}.html", + item_type.as_str(), + last = fqp.last()? + )), + } + Some(url_parts.finish()) +} + fn find_resolution(resolutions: &DocLinkResMap, path: &str) -> Option> { [Namespace::TypeNS, Namespace::ValueNS, Namespace::MacroNS] .into_iter() diff --git a/tests/rustdoc-ui/lints/no-redundancy.rs b/tests/rustdoc-ui/lints/no-redundancy.rs index 6609ce6a4f8d4..d51ada4d13527 100644 --- a/tests/rustdoc-ui/lints/no-redundancy.rs +++ b/tests/rustdoc-ui/lints/no-redundancy.rs @@ -5,3 +5,9 @@ /// [Vec][std::vec::Vec#examples] should not warn, because it's not actually redundant! /// [This is just an `Option`][std::option::Option] has different display content to actual link! pub fn func() {} + +// Regression guard for https://github.com/rust-lang/rust/issues/155458. +/// [NoRedundancyTarget](struct.NoRedundancyTarget.html#fragment) should not warn. +pub struct NoRedundancySource; + +pub struct NoRedundancyTarget; diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed index c40c5691e6082..ad0c6d217ffef 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed @@ -156,3 +156,10 @@ pub fn should_warn_reference() {} /// [`Vec`]: Vec /// [`Vec`]: std::vec::Vec pub fn should_not_warn_reference() {} + +// Regression test for https://github.com/rust-lang/rust/issues/155458. +/// [Issue155458B] +//~^ ERROR redundant explicit link target +pub struct Issue155458A; + +pub struct Issue155458B; diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index dc64a5613fb2b..207eeb7ece2db 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -156,3 +156,10 @@ pub fn should_warn_reference() {} /// [`Vec`]: Vec /// [`Vec`]: std::vec::Vec pub fn should_not_warn_reference() {} + +// Regression test for https://github.com/rust-lang/rust/issues/155458. +/// [Issue155458B](struct.Issue155458B.html) +//~^ ERROR redundant explicit link target +pub struct Issue155458A; + +pub struct Issue155458B; diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr index f90c41af9f1ac..32c7ffe1cabec 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr @@ -1063,5 +1063,21 @@ LL - /// [`dummy_target`][dummy_target] TEXT LL + /// [`dummy_target`] TEXT | -error: aborting due to 60 previous errors +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:161:20 + | +LL | /// [Issue155458B](struct.Issue155458B.html) + | ------------ ^^^^^^^^^^^^^^^^^^^^^^^^ 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 +help: remove explicit link target + | +LL - /// [Issue155458B](struct.Issue155458B.html) +LL + /// [Issue155458B] + | + +error: aborting due to 61 previous errors