From da582a71d2c19a2ded0c45f464cb64c9544c26eb Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Thu, 29 Jun 2023 13:24:09 +0800 Subject: [PATCH] Add warn level lint `redundant_explicit_links` - Currently it will panic due to the resolution's caching issue --- src/doc/rustdoc/src/lints.md | 22 ++++ src/librustdoc/lint.rs | 12 +++ .../passes/collect_intra_doc_links.rs | 101 ++++++++++++++++-- .../lints/redundant_explicit_links.rs | 6 ++ 4 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 tests/rustdoc-ui/lints/redundant_explicit_links.rs diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index fd57b079644..fe06b303e72 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -412,3 +412,25 @@ help: if you meant to use a literal backtick, escape it warning: 1 warning emitted ``` + +## `redundant_explicit_links` + +This lint is **warned by default**. It detects explicit links that are same +as computed automatic links. +This usually means the explicit links is removeable. For example: + +```rust +#![warn(rustdoc::redundant_explicit_links)] + +pub fn dummy_target() {} // note: unnecessary - warns by default. + +/// [`dummy_target`](dummy_target) +/// [dummy_target](dummy_target) +pub fn c() {} +``` + +Which will give: + +```text + +``` diff --git a/src/librustdoc/lint.rs b/src/librustdoc/lint.rs index 749c1ff51bf..d45040e348a 100644 --- a/src/librustdoc/lint.rs +++ b/src/librustdoc/lint.rs @@ -185,6 +185,17 @@ macro_rules! declare_rustdoc_lint { "detects unescaped backticks in doc comments" } +declare_rustdoc_lint! { + /// This lint is **warned by default**. It detects explicit links that are same + /// as computed automatic links. This usually means the explicit links is removeable. + /// This is a `rustdoc` only lint, see the documentation in the [rustdoc book]. + /// + /// [rustdoc book]: ../../../rustdoc/lints.html#redundant_explicit_links + REDUNDANT_EXPLICIT_LINKS, + Warn, + "detects redundant explicit links in doc comments" +} + pub(crate) static RUSTDOC_LINTS: Lazy> = Lazy::new(|| { vec![ BROKEN_INTRA_DOC_LINKS, @@ -197,6 +208,7 @@ macro_rules! declare_rustdoc_lint { BARE_URLS, MISSING_CRATE_LEVEL_DOCS, UNESCAPED_BACKTICKS, + REDUNDANT_EXPLICIT_LINKS, ] }); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 26ff64f06a3..78b86cdfa82 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -994,7 +994,15 @@ fn resolve_links(&mut self, item: &Item) { _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), }; for md_link in preprocessed_markdown_links(&doc) { - let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); + let PreprocessedMarkdownLink(_pp_link, ori_link) = &md_link; + let diag_info = DiagnosticInfo { + item, + dox: &doc, + ori_link: &ori_link.link, + link_range: ori_link.range.clone(), + }; + + let link = self.resolve_link(item, item_id, module_id, &md_link, &diag_info); if let Some(link) = link { self.cx.cache.intra_doc_links.entry(item.item_id).or_default().insert(link); } @@ -1010,19 +1018,11 @@ fn resolve_link( item: &Item, item_id: DefId, module_id: DefId, - dox: &str, - link: &PreprocessedMarkdownLink, + PreprocessedMarkdownLink(pp_link, ori_link): &PreprocessedMarkdownLink, + diag_info: &DiagnosticInfo<'_>, ) -> Option { - let PreprocessedMarkdownLink(pp_link, ori_link) = link; trace!("considering link '{}'", ori_link.link); - let diag_info = DiagnosticInfo { - item, - dox, - ori_link: &ori_link.link, - link_range: ori_link.range.clone(), - }; - let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } = pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?; let disambiguator = *disambiguator; @@ -1042,6 +1042,17 @@ fn resolve_link( matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), )?; + self.check_redundant_explicit_link( + &res, + path_str, + item_id, + module_id, + disambiguator, + &ori_link, + extra_fragment, + &diag_info, + ); + // Check for a primitive which might conflict with a module // Report the ambiguity and require that the user specify which one they meant. // FIXME: could there ever be a primitive not in the type namespace? @@ -1372,6 +1383,74 @@ fn resolve_with_disambiguator( } } } + + fn check_redundant_explicit_link( + &mut self, + ex_res: &Res, + ex: &Box, + item_id: DefId, + module_id: DefId, + dis: Option, + ori_link: &MarkdownLink, + extra_fragment: &Option, + diag_info: &DiagnosticInfo<'_>, + ) { + // Check if explicit resolution's path is same as resolution of original link's display text path, e.g. + // [target](target) + // [`target`](target) + // [target](path::to::target) + // [`target`](path::to::target) + // [path::to::target](path::to::target) + // [`path::to::target`](path::to::target) + // + // To avoid disambiguator from panicking, we check if display text path is possible to be disambiguated + // into explicit path. + if ori_link.kind != LinkType::Inline { + return; + } + + let di_text = &ori_link.display_text; + let di_len = di_text.len(); + let ex_len = ex.len(); + + let intra_doc_links = std::mem::take(&mut self.cx.cache.intra_doc_links); + + if ex_len >= di_len && &ex[(ex_len - di_len)..] == di_text { + let Some((di_res, _)) = self.resolve_with_disambiguator_cached( + ResolutionInfo { + item_id, + module_id, + dis, + path_str: di_text.clone().into_boxed_str(), + extra_fragment: extra_fragment.clone(), + }, + diag_info.clone(), // this struct should really be Copy, but Range is not :( + // For reference-style links we want to report only one error so unsuccessful + // resolutions are cached, for other links we want to report an error every + // time so they are not cached. + matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), + ) else { + return; + }; + + if &di_res == ex_res { + use crate::lint::REDUNDANT_EXPLICIT_LINKS; + + report_diagnostic( + self.cx.tcx, + REDUNDANT_EXPLICIT_LINKS, + "redundant explicit rustdoc link", + &diag_info, + |diag, sp, _link_range| { + if let Some(sp) = sp { + diag.note("Explicit link does not affect the original link") + .span_suggestion(sp, "Remove explicit link instead", format!("[{}]", ori_link.link), Applicability::MachineApplicable); + } + } + ); + } + } + } } /// Get the section of a link between the backticks, diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs new file mode 100644 index 00000000000..e794388476b --- /dev/null +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -0,0 +1,6 @@ +#![deny(rustdoc::redundant_explicit_links)] + +pub fn dummy_target() {} + +/// [`Vec`](std::vec::Vec) +pub fn c() {}