Skip to content

Commit 52290d1

Browse files
committed
rustdoc: check redundant explicit links against generated URLs
1 parent f53b654 commit 52290d1

5 files changed

Lines changed: 129 additions & 37 deletions

File tree

src/librustdoc/passes/lint/redundant_explicit_links.rs

Lines changed: 92 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ use rustc_resolve::rustdoc::{prepare_to_doc_link_resolution, source_span_for_mar
1212
use rustc_span::def_id::DefId;
1313
use rustc_span::{Span, Symbol};
1414

15-
use crate::clean::Item;
1615
use crate::clean::utils::{find_nearest_parent_module, inherits_doc_hidden};
16+
use crate::clean::{Item, inline};
1717
use crate::core::DocContext;
18+
use crate::formats::item_type::ItemType;
19+
use crate::html::format::href_relative_parts;
1820
use crate::html::markdown::main_body_opts;
1921

2022
#[derive(Debug)]
@@ -71,12 +73,13 @@ fn check_redundant_explicit_link_for_did(
7173
return;
7274
};
7375

74-
check_redundant_explicit_link(cx, item, hir_id, doc, resolutions);
76+
check_redundant_explicit_link(cx, item, module_id, hir_id, doc, resolutions);
7577
}
7678

7779
fn check_redundant_explicit_link<'md>(
7880
cx: &DocContext<'_>,
7981
item: &Item,
82+
module_id: DefId,
8083
hir_id: HirId,
8184
doc: &'md str,
8285
resolutions: &DocLinkResMap,
@@ -114,35 +117,35 @@ fn check_redundant_explicit_link<'md>(
114117
continue;
115118
}
116119

117-
if dest_url.ends_with(resolvable_link) || resolvable_link.ends_with(&*dest_url) {
118-
match link_type {
119-
LinkType::Inline | LinkType::ReferenceUnknown => {
120-
check_inline_or_reference_unknown_redundancy(
121-
cx,
122-
item,
123-
hir_id,
124-
doc,
125-
resolutions,
126-
link_range,
127-
dest_url.to_string(),
128-
link_data,
129-
if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') },
130-
);
131-
}
132-
LinkType::Reference => {
133-
check_reference_redundancy(
134-
cx,
135-
item,
136-
hir_id,
137-
doc,
138-
resolutions,
139-
link_range,
140-
&dest_url,
141-
link_data,
142-
);
143-
}
144-
_ => {}
120+
match link_type {
121+
LinkType::Inline | LinkType::ReferenceUnknown => {
122+
check_inline_or_reference_unknown_redundancy(
123+
cx,
124+
item,
125+
module_id,
126+
hir_id,
127+
doc,
128+
resolutions,
129+
link_range,
130+
dest_url.to_string(),
131+
link_data,
132+
if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') },
133+
);
134+
}
135+
LinkType::Reference => {
136+
check_reference_redundancy(
137+
cx,
138+
item,
139+
module_id,
140+
hir_id,
141+
doc,
142+
resolutions,
143+
link_range,
144+
&dest_url,
145+
link_data,
146+
);
145147
}
148+
_ => {}
146149
}
147150
}
148151
}
@@ -154,6 +157,7 @@ fn check_redundant_explicit_link<'md>(
154157
fn check_inline_or_reference_unknown_redundancy(
155158
cx: &DocContext<'_>,
156159
item: &Item,
160+
module_id: DefId,
157161
hir_id: HirId,
158162
doc: &str,
159163
resolutions: &DocLinkResMap,
@@ -198,10 +202,8 @@ fn check_inline_or_reference_unknown_redundancy(
198202

199203
let (resolvable_link, resolvable_link_range) =
200204
(&link_data.resolvable_link?, &link_data.resolvable_link_range?);
201-
let (dest_res, display_res) =
202-
(find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?);
203205

204-
if dest_res == display_res {
206+
if explicit_link_is_redundant(cx, module_id, resolutions, &dest, resolvable_link) {
205207
let link_span =
206208
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
207209
{
@@ -254,6 +256,7 @@ fn check_inline_or_reference_unknown_redundancy(
254256
fn check_reference_redundancy(
255257
cx: &DocContext<'_>,
256258
item: &Item,
259+
module_id: DefId,
257260
hir_id: HirId,
258261
doc: &str,
259262
resolutions: &DocLinkResMap,
@@ -296,10 +299,8 @@ fn check_reference_redundancy(
296299

297300
let (resolvable_link, resolvable_link_range) =
298301
(&link_data.resolvable_link?, &link_data.resolvable_link_range?);
299-
let (dest_res, display_res) =
300-
(find_resolution(resolutions, dest)?, find_resolution(resolutions, resolvable_link)?);
301302

302-
if dest_res == display_res {
303+
if explicit_link_is_redundant(cx, module_id, resolutions, dest, resolvable_link) {
303304
let link_span =
304305
match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings)
305306
{
@@ -355,6 +356,61 @@ fn check_reference_redundancy(
355356
None
356357
}
357358

359+
fn explicit_link_is_redundant(
360+
cx: &DocContext<'_>,
361+
module_id: DefId,
362+
resolutions: &DocLinkResMap,
363+
dest: &str,
364+
resolvable_link: &str,
365+
) -> bool {
366+
let Some(display_res) = find_resolution(resolutions, resolvable_link) else {
367+
return false;
368+
};
369+
370+
if (dest.ends_with(resolvable_link) || resolvable_link.ends_with(dest))
371+
&& find_resolution(resolutions, dest).is_some_and(|dest_res| dest_res == display_res)
372+
{
373+
return true;
374+
}
375+
376+
if !dest.contains('#') && dest.ends_with(".html") {
377+
return false;
378+
}
379+
380+
local_href_for_res(cx, module_id, display_res).is_some_and(|href| href == dest)
381+
}
382+
383+
fn local_href_for_res(cx: &DocContext<'_>, module_id: DefId, res: Res<NodeId>) -> Option<String> {
384+
let mut did = res.opt_def_id()?;
385+
if matches!(cx.tcx.def_kind(did), DefKind::Ctor(..)) {
386+
did = cx.tcx.parent(did);
387+
}
388+
389+
if matches!(
390+
cx.tcx.def_kind(did),
391+
DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst { .. } | DefKind::Variant
392+
) || !did.is_local()
393+
{
394+
return None;
395+
}
396+
397+
let item_type = ItemType::from_def_id(did, cx.tcx);
398+
let fqp = inline::get_item_path(cx.tcx, did, item_type);
399+
let module_fqp = if item_type == ItemType::Module { &fqp[..] } else { &fqp[..fqp.len() - 1] };
400+
let current_fqp = inline::get_item_path(cx.tcx, module_id, ItemType::Module);
401+
402+
let mut url_parts = href_relative_parts(module_fqp, &current_fqp);
403+
match item_type {
404+
ItemType::Module => url_parts.push("index.html"),
405+
_ => url_parts.push_fmt(format_args!(
406+
"{}.{last}.html",
407+
item_type.as_str(),
408+
last = fqp.last()?
409+
)),
410+
}
411+
Some(url_parts.finish())
412+
}
413+
358414
fn find_resolution(resolutions: &DocLinkResMap, path: &str) -> Option<Res<NodeId>> {
359415
[Namespace::TypeNS, Namespace::ValueNS, Namespace::MacroNS]
360416
.into_iter()

tests/rustdoc-ui/lints/no-redundancy.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,9 @@
55
/// [Vec][std::vec::Vec#examples] should not warn, because it's not actually redundant!
66
/// [This is just an `Option`][std::option::Option] has different display content to actual link!
77
pub fn func() {}
8+
9+
// Regression guard for https://github.com/rust-lang/rust/issues/155458.
10+
/// [NoRedundancyTarget](struct.NoRedundancyTarget.html#fragment) should not warn.
11+
pub struct NoRedundancySource;
12+
13+
pub struct NoRedundancyTarget;

tests/rustdoc-ui/lints/redundant_explicit_links.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,10 @@ pub fn should_warn_reference() {}
156156
/// [`Vec<T>`]: Vec
157157
/// [`Vec<T>`]: std::vec::Vec
158158
pub fn should_not_warn_reference() {}
159+
160+
// Regression test for https://github.com/rust-lang/rust/issues/155458.
161+
/// [Issue155458B]
162+
//~^ ERROR redundant explicit link target
163+
pub struct Issue155458A;
164+
165+
pub struct Issue155458B;

tests/rustdoc-ui/lints/redundant_explicit_links.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,10 @@ pub fn should_warn_reference() {}
156156
/// [`Vec<T>`]: Vec
157157
/// [`Vec<T>`]: std::vec::Vec
158158
pub fn should_not_warn_reference() {}
159+
160+
// Regression test for https://github.com/rust-lang/rust/issues/155458.
161+
/// [Issue155458B](struct.Issue155458B.html)
162+
//~^ ERROR redundant explicit link target
163+
pub struct Issue155458A;
164+
165+
pub struct Issue155458B;

tests/rustdoc-ui/lints/redundant_explicit_links.stderr

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1063,5 +1063,21 @@ LL - /// [`dummy_target`][dummy_target] TEXT
10631063
LL + /// [`dummy_target`] TEXT
10641064
|
10651065

1066-
error: aborting due to 60 previous errors
1066+
error: redundant explicit link target
1067+
--> $DIR/redundant_explicit_links.rs:161:20
1068+
|
1069+
LL | /// [Issue155458B](struct.Issue155458B.html)
1070+
| ------------ ^^^^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant
1071+
| |
1072+
| because label contains path that resolves to same destination
1073+
|
1074+
= note: when a link's destination is not specified,
1075+
the label is used to resolve intra-doc links
1076+
help: remove explicit link target
1077+
|
1078+
LL - /// [Issue155458B](struct.Issue155458B.html)
1079+
LL + /// [Issue155458B]
1080+
|
1081+
1082+
error: aborting due to 61 previous errors
10671083

0 commit comments

Comments
 (0)