Skip to content

Commit da582a7

Browse files
committed
Add warn level lint redundant_explicit_links
- Currently it will panic due to the resolution's caching issue
1 parent b1d232a commit da582a7

File tree

4 files changed

+130
-11
lines changed

4 files changed

+130
-11
lines changed

src/doc/rustdoc/src/lints.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,3 +412,25 @@ help: if you meant to use a literal backtick, escape it
412412
413413
warning: 1 warning emitted
414414
```
415+
416+
## `redundant_explicit_links`
417+
418+
This lint is **warned by default**. It detects explicit links that are same
419+
as computed automatic links.
420+
This usually means the explicit links is removeable. For example:
421+
422+
```rust
423+
#![warn(rustdoc::redundant_explicit_links)]
424+
425+
pub fn dummy_target() {} // note: unnecessary - warns by default.
426+
427+
/// [`dummy_target`](dummy_target)
428+
/// [dummy_target](dummy_target)
429+
pub fn c() {}
430+
```
431+
432+
Which will give:
433+
434+
```text
435+
436+
```

src/librustdoc/lint.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,17 @@ declare_rustdoc_lint! {
185185
"detects unescaped backticks in doc comments"
186186
}
187187

188+
declare_rustdoc_lint! {
189+
/// This lint is **warned by default**. It detects explicit links that are same
190+
/// as computed automatic links. This usually means the explicit links is removeable.
191+
/// This is a `rustdoc` only lint, see the documentation in the [rustdoc book].
192+
///
193+
/// [rustdoc book]: ../../../rustdoc/lints.html#redundant_explicit_links
194+
REDUNDANT_EXPLICIT_LINKS,
195+
Warn,
196+
"detects redundant explicit links in doc comments"
197+
}
198+
188199
pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
189200
vec![
190201
BROKEN_INTRA_DOC_LINKS,
@@ -197,6 +208,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
197208
BARE_URLS,
198209
MISSING_CRATE_LEVEL_DOCS,
199210
UNESCAPED_BACKTICKS,
211+
REDUNDANT_EXPLICIT_LINKS,
200212
]
201213
});
202214

src/librustdoc/passes/collect_intra_doc_links.rs

Lines changed: 90 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,15 @@ impl LinkCollector<'_, '_> {
994994
_ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(),
995995
};
996996
for md_link in preprocessed_markdown_links(&doc) {
997-
let link = self.resolve_link(item, item_id, module_id, &doc, &md_link);
997+
let PreprocessedMarkdownLink(_pp_link, ori_link) = &md_link;
998+
let diag_info = DiagnosticInfo {
999+
item,
1000+
dox: &doc,
1001+
ori_link: &ori_link.link,
1002+
link_range: ori_link.range.clone(),
1003+
};
1004+
1005+
let link = self.resolve_link(item, item_id, module_id, &md_link, &diag_info);
9981006
if let Some(link) = link {
9991007
self.cx.cache.intra_doc_links.entry(item.item_id).or_default().insert(link);
10001008
}
@@ -1010,19 +1018,11 @@ impl LinkCollector<'_, '_> {
10101018
item: &Item,
10111019
item_id: DefId,
10121020
module_id: DefId,
1013-
dox: &str,
1014-
link: &PreprocessedMarkdownLink,
1021+
PreprocessedMarkdownLink(pp_link, ori_link): &PreprocessedMarkdownLink,
1022+
diag_info: &DiagnosticInfo<'_>,
10151023
) -> Option<ItemLink> {
1016-
let PreprocessedMarkdownLink(pp_link, ori_link) = link;
10171024
trace!("considering link '{}'", ori_link.link);
10181025

1019-
let diag_info = DiagnosticInfo {
1020-
item,
1021-
dox,
1022-
ori_link: &ori_link.link,
1023-
link_range: ori_link.range.clone(),
1024-
};
1025-
10261026
let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } =
10271027
pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?;
10281028
let disambiguator = *disambiguator;
@@ -1042,6 +1042,17 @@ impl LinkCollector<'_, '_> {
10421042
matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut),
10431043
)?;
10441044

1045+
self.check_redundant_explicit_link(
1046+
&res,
1047+
path_str,
1048+
item_id,
1049+
module_id,
1050+
disambiguator,
1051+
&ori_link,
1052+
extra_fragment,
1053+
&diag_info,
1054+
);
1055+
10451056
// Check for a primitive which might conflict with a module
10461057
// Report the ambiguity and require that the user specify which one they meant.
10471058
// FIXME: could there ever be a primitive not in the type namespace?
@@ -1372,6 +1383,74 @@ impl LinkCollector<'_, '_> {
13721383
}
13731384
}
13741385
}
1386+
1387+
fn check_redundant_explicit_link(
1388+
&mut self,
1389+
ex_res: &Res,
1390+
ex: &Box<str>,
1391+
item_id: DefId,
1392+
module_id: DefId,
1393+
dis: Option<Disambiguator>,
1394+
ori_link: &MarkdownLink,
1395+
extra_fragment: &Option<String>,
1396+
diag_info: &DiagnosticInfo<'_>,
1397+
) {
1398+
// Check if explicit resolution's path is same as resolution of original link's display text path, e.g.
1399+
// [target](target)
1400+
// [`target`](target)
1401+
// [target](path::to::target)
1402+
// [`target`](path::to::target)
1403+
// [path::to::target](path::to::target)
1404+
// [`path::to::target`](path::to::target)
1405+
//
1406+
// To avoid disambiguator from panicking, we check if display text path is possible to be disambiguated
1407+
// into explicit path.
1408+
if ori_link.kind != LinkType::Inline {
1409+
return;
1410+
}
1411+
1412+
let di_text = &ori_link.display_text;
1413+
let di_len = di_text.len();
1414+
let ex_len = ex.len();
1415+
1416+
let intra_doc_links = std::mem::take(&mut self.cx.cache.intra_doc_links);
1417+
1418+
if ex_len >= di_len && &ex[(ex_len - di_len)..] == di_text {
1419+
let Some((di_res, _)) = self.resolve_with_disambiguator_cached(
1420+
ResolutionInfo {
1421+
item_id,
1422+
module_id,
1423+
dis,
1424+
path_str: di_text.clone().into_boxed_str(),
1425+
extra_fragment: extra_fragment.clone(),
1426+
},
1427+
diag_info.clone(), // this struct should really be Copy, but Range is not :(
1428+
// For reference-style links we want to report only one error so unsuccessful
1429+
// resolutions are cached, for other links we want to report an error every
1430+
// time so they are not cached.
1431+
matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut),
1432+
) else {
1433+
return;
1434+
};
1435+
1436+
if &di_res == ex_res {
1437+
use crate::lint::REDUNDANT_EXPLICIT_LINKS;
1438+
1439+
report_diagnostic(
1440+
self.cx.tcx,
1441+
REDUNDANT_EXPLICIT_LINKS,
1442+
"redundant explicit rustdoc link",
1443+
&diag_info,
1444+
|diag, sp, _link_range| {
1445+
if let Some(sp) = sp {
1446+
diag.note("Explicit link does not affect the original link")
1447+
.span_suggestion(sp, "Remove explicit link instead", format!("[{}]", ori_link.link), Applicability::MachineApplicable);
1448+
}
1449+
}
1450+
);
1451+
}
1452+
}
1453+
}
13751454
}
13761455

13771456
/// Get the section of a link between the backticks,
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![deny(rustdoc::redundant_explicit_links)]
2+
3+
pub fn dummy_target() {}
4+
5+
/// [`Vec`](std::vec::Vec)
6+
pub fn c() {}

0 commit comments

Comments
 (0)