From b1d232a6da800b26f18d79a0340436fa4aec7327 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Thu, 29 Jun 2023 12:07:24 +0800 Subject: [PATCH 01/26] rework link parsing loop --- src/librustdoc/html/markdown.rs | 89 ++++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 0ba2d992392e9..b5ab8a7b12ac6 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -50,7 +50,7 @@ use crate::html::render::small_url_encode; use crate::html::toc::TocBuilder; use pulldown_cmark::{ - html, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, + html, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, OffsetIter, Options, Parser, Tag, }; #[cfg(test)] @@ -1240,6 +1240,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin pub(crate) struct MarkdownLink { pub kind: LinkType, pub link: String, + pub display_text: String, pub range: MarkdownLinkRange, } @@ -1263,8 +1264,8 @@ impl MarkdownLinkRange { } } -pub(crate) fn markdown_links( - md: &str, +pub(crate) fn markdown_links<'md, R>( + md: &'md str, preprocess_link: impl Fn(MarkdownLink) -> Option, ) -> Vec { if md.is_empty() { @@ -1375,32 +1376,72 @@ pub(crate) fn markdown_links( MarkdownLinkRange::Destination(range.clone()) }; - Parser::new_with_broken_link_callback( + let mut broken_link_callback = |link: BrokenLink<'md>| Some((link.reference, "".into())); + let mut event_iter = Parser::new_with_broken_link_callback( md, main_body_opts(), - Some(&mut |link: BrokenLink<'_>| Some((link.reference, "".into()))), + Some(&mut broken_link_callback), ) - .into_offset_iter() - .filter_map(|(event, span)| match event { - Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { - let range = match link_type { - // Link is pulled from the link itself. - LinkType::ReferenceUnknown | LinkType::ShortcutUnknown => { - span_for_offset_backward(span, b'[', b']') - } - LinkType::CollapsedUnknown => span_for_offset_forward(span, b'[', b']'), - LinkType::Inline => span_for_offset_backward(span, b'(', b')'), - // Link is pulled from elsewhere in the document. - LinkType::Reference | LinkType::Collapsed | LinkType::Shortcut => { - span_for_link(&dest, span) + .into_offset_iter(); + let mut links = Vec::new(); + + while let Some((event, span)) = event_iter.next() { + match event { + Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { + let range = match link_type { + // Link is pulled from the link itself. + LinkType::ReferenceUnknown | LinkType::ShortcutUnknown => { + span_for_offset_backward(span, b'[', b']') + } + LinkType::CollapsedUnknown => span_for_offset_forward(span, b'[', b']'), + LinkType::Inline => span_for_offset_backward(span, b'(', b')'), + // Link is pulled from elsewhere in the document. + LinkType::Reference | LinkType::Collapsed | LinkType::Shortcut => { + span_for_link(&dest, span) + } + LinkType::Autolink | LinkType::Email => unreachable!(), + }; + + let display_text = + collect_link_data(&mut event_iter).map_or(String::new(), CowStr::into_string); + + if let Some(link) = preprocess_link(MarkdownLink { + kind: link_type, + display_text, + link: dest.into_string(), + range, + }) { + links.push(link); } - LinkType::Autolink | LinkType::Email => unreachable!(), - }; - preprocess_link(MarkdownLink { kind: link_type, range, link: dest.into_string() }) + } + _ => {} } - _ => None, - }) - .collect() + } + + links +} + +fn collect_link_data<'input, 'callback>( + event_iter: &mut OffsetIter<'input, 'callback>, +) -> Option> { + let mut display_text = None; + + while let Some((event, _span)) = event_iter.next() { + match event { + Event::Text(code) => { + display_text = Some(code); + } + Event::Code(code) => { + display_text = Some(code); + } + Event::End(_) => { + break; + } + _ => {} + } + } + + display_text } #[derive(Debug)] From da582a71d2c19a2ded0c45f464cb64c9544c26eb Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Thu, 29 Jun 2023 13:24:09 +0800 Subject: [PATCH 02/26] 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 fd57b07964481..fe06b303e72fb 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 749c1ff51bfc5..d45040e348a24 100644 --- a/src/librustdoc/lint.rs +++ b/src/librustdoc/lint.rs @@ -185,6 +185,17 @@ 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 @@ pub(crate) static RUSTDOC_LINTS: Lazy> = Lazy::new(|| { 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 26ff64f06a321..78b86cdfa8245 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -994,7 +994,15 @@ impl LinkCollector<'_, '_> { _ => 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 @@ impl LinkCollector<'_, '_> { 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 @@ impl LinkCollector<'_, '_> { 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 @@ impl LinkCollector<'_, '_> { } } } + + 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 0000000000000..e794388476be8 --- /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() {} From 65e24a57bbd510e79d667d5c6e18507895e36447 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 02:01:21 +0800 Subject: [PATCH 03/26] Fix resolution caching --- compiler/rustc_resolve/src/rustdoc.rs | 57 +++++++++-- src/doc/rustdoc/src/lints.md | 26 +++-- src/librustdoc/html/markdown.rs | 1 + .../passes/collect_intra_doc_links.rs | 72 +++++++------- .../lints/redundant_explicit_links.rs | 17 +++- .../lints/redundant_explicit_links.stderr | 97 +++++++++++++++++++ 6 files changed, 217 insertions(+), 53 deletions(-) create mode 100644 tests/rustdoc-ui/lints/redundant_explicit_links.stderr diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index d433391f272e0..083d16d3b047d 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -392,16 +392,57 @@ pub(crate) fn attrs_to_preprocessed_links(attrs: &[ast::Attribute]) -> Vec(doc: &'md str) -> Vec> { + let mut broken_link_callback = |link: BrokenLink<'md>| Some((link.reference, "".into())); + let mut event_iter = Parser::new_with_broken_link_callback( &doc, main_body_opts(), - Some(&mut |link: BrokenLink<'_>| Some((link.reference, "".into()))), + Some(&mut broken_link_callback), ) - .filter_map(|event| match event { - Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { - Some(preprocess_link(&dest)) + .into_iter(); + let mut links = Vec::new(); + + while let Some(event) = event_iter.next() { + match event { + Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { + if let Some(display_text) = collect_link_data(&mut event_iter) { + links.push(display_text); + } + + links.push(preprocess_link(&dest)); + } + _ => {} + } + } + + links +} + +/// Collects additional data of link. +fn collect_link_data<'input, 'callback>( + event_iter: &mut Parser<'input, 'callback>, +) -> Option> { + let mut display_text = None; + + while let Some(event) = event_iter.next() { + match event { + Event::Text(code) => { + display_text = Some(code.to_string().into_boxed_str()); + } + Event::Code(code) => { + display_text = Some(code.to_string().into_boxed_str()); + } + Event::End(_) => { + break; + } + _ => {} } - _ => None, - }) - .collect() + } + + display_text } diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index fe06b303e72fb..f15e6e451e757 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -420,17 +420,29 @@ as computed automatic links. This usually means the explicit links is removeable. For example: ```rust -#![warn(rustdoc::redundant_explicit_links)] +#![warn(rustdoc::redundant_explicit_links)] // note: unnecessary - warns by default. -pub fn dummy_target() {} // note: unnecessary - warns by default. - -/// [`dummy_target`](dummy_target) -/// [dummy_target](dummy_target) -pub fn c() {} +/// add takes 2 [`usize`](usize) and performs addition +/// on them, then returns result. +pub fn add(left: usize, right: usize) -> usize { + left + right +} ``` Which will give: ```text - +error: redundant explicit rustdoc link + --> src/lib.rs:3:27 + | +3 | /// add takes 2 [`usize`](usize) and performs addition + | ^^^^^ + | + = note: Explicit link does not affect the original link +note: the lint level is defined here + --> src/lib.rs:1:9 + | +1 | #![deny(rustdoc::redundant_explicit_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: Remove explicit link instead ``` diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index b5ab8a7b12ac6..2fe052283a99a 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1421,6 +1421,7 @@ pub(crate) fn markdown_links<'md, R>( links } +/// Collects additional data of link. fn collect_link_data<'input, 'callback>( event_iter: &mut OffsetIter<'input, 'callback>, ) -> Option> { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 78b86cdfa8245..2ed0077c3d564 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -994,15 +994,7 @@ impl LinkCollector<'_, '_> { _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), }; for md_link in preprocessed_markdown_links(&doc) { - 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); + let link = self.resolve_link(&doc, item, item_id, module_id, &md_link); if let Some(link) = link { self.cx.cache.intra_doc_links.entry(item.item_id).or_default().insert(link); } @@ -1015,14 +1007,20 @@ impl LinkCollector<'_, '_> { /// FIXME(jynelson): this is way too many arguments fn resolve_link( &mut self, + dox: &String, item: &Item, item_id: DefId, module_id: DefId, PreprocessedMarkdownLink(pp_link, ori_link): &PreprocessedMarkdownLink, - diag_info: &DiagnosticInfo<'_>, ) -> Option { 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; @@ -1045,11 +1043,14 @@ impl LinkCollector<'_, '_> { self.check_redundant_explicit_link( &res, path_str, - item_id, - module_id, - disambiguator, + ResolutionInfo { + item_id, + module_id, + dis: disambiguator, + path_str: ori_link.display_text.clone().into_boxed_str(), + extra_fragment: extra_fragment.clone(), + }, &ori_link, - extra_fragment, &diag_info, ); @@ -1384,15 +1385,13 @@ impl LinkCollector<'_, '_> { } } + /// Check if resolution of inline link's display text and explicit link are same. fn check_redundant_explicit_link( &mut self, - ex_res: &Res, - ex: &Box, - item_id: DefId, - module_id: DefId, - dis: Option, + explicit_res: &Res, + explicit_link: &Box, + display_res_info: ResolutionInfo, 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. @@ -1409,21 +1408,15 @@ impl LinkCollector<'_, '_> { 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); + let display_text = &ori_link.display_text; + let display_len = display_text.len(); + let explicit_len = explicit_link.len(); - 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(), - }, + if explicit_len >= display_len + && &explicit_link[(explicit_len - display_len)..] == display_text + { + let Some((display_res, _)) = self.resolve_with_disambiguator_cached( + display_res_info, 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 @@ -1433,7 +1426,7 @@ impl LinkCollector<'_, '_> { return; }; - if &di_res == ex_res { + if &display_res == explicit_res { use crate::lint::REDUNDANT_EXPLICIT_LINKS; report_diagnostic( @@ -1444,9 +1437,14 @@ impl LinkCollector<'_, '_> { |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); + .span_suggestion_hidden( + sp, + "Remove explicit link instead", + format!(""), + Applicability::MachineApplicable, + ); } - } + }, ); } } diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index e794388476be8..d33396f681097 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -2,5 +2,20 @@ pub fn dummy_target() {} +/// [dummy_target](dummy_target) +/// [`dummy_target`](dummy_target) +/// [Vec](Vec) +/// [`Vec`](Vec) +/// [Vec](std::vec::Vec) /// [`Vec`](std::vec::Vec) -pub fn c() {} +/// [std::vec::Vec](std::vec::Vec) +/// [`std::vec::Vec`](std::vec::Vec) +/// [usize](usize) +/// [`usize`](usize) +/// [std::primitive::usize](usize) +/// [`std::primitive::usize`](usize) +pub fn should_warn() {} + +/// [`Vec`](Vec) +/// [`Vec`](std::vec::Vec) +pub fn should_not_warn() {} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr new file mode 100644 index 0000000000000..4ca427d62ce17 --- /dev/null +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr @@ -0,0 +1,97 @@ +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:5:20 + | +LL | /// [dummy_target](dummy_target) + | ^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link +note: the lint level is defined here + --> $DIR/redundant_explicit_links.rs:1:9 + | +LL | #![deny(rustdoc::redundant_explicit_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:6:22 + | +LL | /// [`dummy_target`](dummy_target) + | ^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:7:11 + | +LL | /// [Vec](Vec) + | ^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:8:13 + | +LL | /// [`Vec`](Vec) + | ^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:9:11 + | +LL | /// [Vec](std::vec::Vec) + | ^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:10:13 + | +LL | /// [`Vec`](std::vec::Vec) + | ^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:11:21 + | +LL | /// [std::vec::Vec](std::vec::Vec) + | ^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:12:23 + | +LL | /// [`std::vec::Vec`](std::vec::Vec) + | ^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:13:13 + | +LL | /// [usize](usize) + | ^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:14:15 + | +LL | /// [`usize`](usize) + | ^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: aborting due to 10 previous errors + From 1c6b237f9e360356f2b0b3712f83e91a53ddbe86 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 02:08:37 +0800 Subject: [PATCH 04/26] add more tests --- src/librustdoc/passes/collect_intra_doc_links.rs | 2 +- tests/rustdoc-ui/lints/redundant_explicit_links.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 2ed0077c3d564..d78fec2cb0ab4 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1421,7 +1421,7 @@ impl LinkCollector<'_, '_> { // 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), + matches!(ori_link.kind, LinkType::Reference), ) else { return; }; diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index d33396f681097..a78b4837b26a0 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -4,16 +4,24 @@ pub fn dummy_target() {} /// [dummy_target](dummy_target) /// [`dummy_target`](dummy_target) +/// /// [Vec](Vec) /// [`Vec`](Vec) /// [Vec](std::vec::Vec) /// [`Vec`](std::vec::Vec) +/// [std::vec::Vec](Vec) +/// [`std::vec::Vec`](Vec) /// [std::vec::Vec](std::vec::Vec) /// [`std::vec::Vec`](std::vec::Vec) +/// /// [usize](usize) /// [`usize`](usize) +/// [usize](std::primitive::usize) +/// [`usize`](std::primitive::usize) /// [std::primitive::usize](usize) /// [`std::primitive::usize`](usize) +/// [std::primitive::usize](std::primitive::usize) +/// [`std::primitive::usize`](std::primitive::usize) pub fn should_warn() {} /// [`Vec`](Vec) From f1b23f29db326292e8f61f63b69725c79a328bac Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 02:10:55 +0800 Subject: [PATCH 05/26] bless test output --- .../lints/redundant_explicit_links.stderr | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr index 4ca427d62ce17..e72105f394f58 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr @@ -22,7 +22,7 @@ LL | /// [`dummy_target`](dummy_target) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:7:11 + --> $DIR/redundant_explicit_links.rs:8:11 | LL | /// [Vec](Vec) | ^^^ @@ -31,7 +31,7 @@ LL | /// [Vec](Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:8:13 + --> $DIR/redundant_explicit_links.rs:9:13 | LL | /// [`Vec`](Vec) | ^^^ @@ -40,7 +40,7 @@ LL | /// [`Vec`](Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:9:11 + --> $DIR/redundant_explicit_links.rs:10:11 | LL | /// [Vec](std::vec::Vec) | ^^^^^^^^^^^^^ @@ -49,7 +49,7 @@ LL | /// [Vec](std::vec::Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:10:13 + --> $DIR/redundant_explicit_links.rs:11:13 | LL | /// [`Vec`](std::vec::Vec) | ^^^^^^^^^^^^^ @@ -58,7 +58,7 @@ LL | /// [`Vec`](std::vec::Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:11:21 + --> $DIR/redundant_explicit_links.rs:14:21 | LL | /// [std::vec::Vec](std::vec::Vec) | ^^^^^^^^^^^^^ @@ -67,7 +67,7 @@ LL | /// [std::vec::Vec](std::vec::Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:12:23 + --> $DIR/redundant_explicit_links.rs:15:23 | LL | /// [`std::vec::Vec`](std::vec::Vec) | ^^^^^^^^^^^^^ @@ -76,7 +76,7 @@ LL | /// [`std::vec::Vec`](std::vec::Vec) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:13:13 + --> $DIR/redundant_explicit_links.rs:17:13 | LL | /// [usize](usize) | ^^^^^ @@ -85,7 +85,7 @@ LL | /// [usize](usize) = help: Remove explicit link instead error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:14:15 + --> $DIR/redundant_explicit_links.rs:18:15 | LL | /// [`usize`](usize) | ^^^^^ @@ -93,5 +93,41 @@ LL | /// [`usize`](usize) = note: Explicit link does not affect the original link = help: Remove explicit link instead -error: aborting due to 10 previous errors +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:19:13 + | +LL | /// [usize](std::primitive::usize) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:20:15 + | +LL | /// [`usize`](std::primitive::usize) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:23:29 + | +LL | /// [std::primitive::usize](std::primitive::usize) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: redundant explicit rustdoc link + --> $DIR/redundant_explicit_links.rs:24:31 + | +LL | /// [`std::primitive::usize`](std::primitive::usize) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: Explicit link does not affect the original link + = help: Remove explicit link instead + +error: aborting due to 14 previous errors From e583318aa864ce0a3d53c339600d51a70d7b6440 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 02:35:44 +0800 Subject: [PATCH 06/26] fix trailing whitespace --- tests/rustdoc-ui/lints/redundant_explicit_links.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index a78b4837b26a0..b3203a2690f0a 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -4,7 +4,7 @@ pub fn dummy_target() {} /// [dummy_target](dummy_target) /// [`dummy_target`](dummy_target) -/// +/// /// [Vec](Vec) /// [`Vec`](Vec) /// [Vec](std::vec::Vec) @@ -13,7 +13,7 @@ pub fn dummy_target() {} /// [`std::vec::Vec`](Vec) /// [std::vec::Vec](std::vec::Vec) /// [`std::vec::Vec`](std::vec::Vec) -/// +/// /// [usize](usize) /// [`usize`](usize) /// [usize](std::primitive::usize) From c7369891ba3265e47346bf44d6f5b5bf4451f3ca Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 18:10:01 +0800 Subject: [PATCH 07/26] Refactor lint from rustc to rustdoc --- .../passes/collect_intra_doc_links.rs | 30 +- src/librustdoc/passes/lint.rs | 2 + .../passes/lint/redundant_explicit_links.rs | 188 +++++++++++ .../lints/redundant_explicit_links.fixed | 54 +++ .../lints/redundant_explicit_links.rs | 29 +- .../lints/redundant_explicit_links.stderr | 318 ++++++++++++++---- 6 files changed, 519 insertions(+), 102 deletions(-) create mode 100644 src/librustdoc/passes/lint/redundant_explicit_links.rs create mode 100644 tests/rustdoc-ui/lints/redundant_explicit_links.fixed diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d78fec2cb0ab4..0432bd40c9032 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1041,7 +1041,6 @@ impl LinkCollector<'_, '_> { )?; self.check_redundant_explicit_link( - &res, path_str, ResolutionInfo { item_id, @@ -1388,7 +1387,6 @@ impl LinkCollector<'_, '_> { /// Check if resolution of inline link's display text and explicit link are same. fn check_redundant_explicit_link( &mut self, - explicit_res: &Res, explicit_link: &Box, display_res_info: ResolutionInfo, ori_link: &MarkdownLink, @@ -1415,38 +1413,14 @@ impl LinkCollector<'_, '_> { if explicit_len >= display_len && &explicit_link[(explicit_len - display_len)..] == display_text { - let Some((display_res, _)) = self.resolve_with_disambiguator_cached( + self.resolve_with_disambiguator_cached( display_res_info, 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), - ) else { - return; - }; - - if &display_res == explicit_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_hidden( - sp, - "Remove explicit link instead", - format!(""), - Applicability::MachineApplicable, - ); - } - }, - ); - } + ); } } } diff --git a/src/librustdoc/passes/lint.rs b/src/librustdoc/passes/lint.rs index e653207b9b6d4..c6d5b7bd346d4 100644 --- a/src/librustdoc/passes/lint.rs +++ b/src/librustdoc/passes/lint.rs @@ -4,6 +4,7 @@ mod bare_urls; mod check_code_block_syntax; mod html_tags; +mod redundant_explicit_links; mod unescaped_backticks; use super::Pass; @@ -29,6 +30,7 @@ impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> { check_code_block_syntax::visit_item(self.cx, item); html_tags::visit_item(self.cx, item); unescaped_backticks::visit_item(self.cx, item); + redundant_explicit_links::visit_item(self.cx, item); self.visit_item_recur(item) } diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs new file mode 100644 index 0000000000000..1bccedde92a20 --- /dev/null +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -0,0 +1,188 @@ +use std::ops::Range; + +use pulldown_cmark::{Parser, BrokenLink, Event, Tag, LinkType, OffsetIter}; +use rustc_ast::NodeId; +use rustc_errors::SuggestionStyle; +use rustc_hir::HirId; +use rustc_hir::def::{Namespace, DefKind, DocLinkResMap, Res}; +use rustc_lint_defs::Applicability; +use rustc_span::Symbol; + +use crate::clean::Item; +use crate::clean::utils::find_nearest_parent_module; +use crate::core::DocContext; +use crate::html::markdown::main_body_opts; +use crate::passes::source_span_for_markdown_range; + +struct LinkData { + resolvable_link: Option, + resolvable_link_range: Option>, + display_link: String, +} + +pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { + let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) else { + // If non-local, no need to check anything. + return; + }; + + let doc = item.doc_value(); + if doc.is_empty() { + return; + } + + check_redundant_explicit_link(cx, item, hir_id, &doc); +} + +fn check_redundant_explicit_link<'md>(cx: &DocContext<'_>, item: &Item, hir_id: HirId, doc: &'md str) { + let mut broken_line_callback = |link: BrokenLink<'md>| Some((link.reference, "".into())); + let mut offset_iter = Parser::new_with_broken_link_callback(&doc, main_body_opts(), Some(&mut broken_line_callback)).into_offset_iter(); + + while let Some((event, link_range)) = offset_iter.next() { + match event { + Event::Start(Tag::Link(link_type, dest, _)) => { + let link_data = collect_link_data(&mut offset_iter); + let dest = dest.to_string(); + + if link_type == LinkType::Inline { + check_inline_link_redundancy(cx, item, hir_id, doc, link_range, dest, link_data); + } + } + _ => {} + } + } +} + +fn check_inline_link_redundancy(cx: &DocContext<'_>, item: &Item, hir_id: HirId, doc: &str, link_range: Range, dest: String, link_data: LinkData) -> Option<()> { + 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); + + let (resolvable_link, resolvable_link_range) = (&link_data.resolvable_link?, &link_data.resolvable_link_range?); + let (dest_res, display_res) = (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); + + if dest_res == display_res { + let link_span = source_span_for_markdown_range( + cx.tcx, + &doc, + &link_range, + &item.attrs, + ).unwrap_or(item.attr_span(cx.tcx)); + let explicit_span = source_span_for_markdown_range( + cx.tcx, + &doc, + &offset_explicit_range(doc, &link_range, b'(', b')'), + &item.attrs + )?; + let display_span = source_span_for_markdown_range( + cx.tcx, + &doc, + &resolvable_link_range, + &item.attrs + )?; + + + cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| { + lint.span_label(explicit_span, "explicit target is redundant") + .span_label(display_span, "because label contains path that resolves to same destination") + .note("when a link's destination is not specified,\nthe label is used to resolve intra-doc links") + .span_suggestion_with_style(link_span, "remove explicit link target", format!("[{}]", link_data.display_link), Applicability::MaybeIncorrect, SuggestionStyle::ShowAlways); + + lint + }); + } + + None +} + +fn find_resolution<'tcx>(resolutions: &'tcx DocLinkResMap, path: &str) -> Option<&'tcx Res> { + for ns in [Namespace::TypeNS, Namespace::ValueNS, Namespace::MacroNS] { + let Some(Some(res)) = resolutions.get(&(Symbol::intern(path), ns)) + else { + continue; + }; + + return Some(res); + } + + None +} + +/// Collects all neccessary data of link. +fn collect_link_data(offset_iter: &mut OffsetIter<'_, '_>) -> LinkData { + let mut resolvable_link = None; + let mut resolvable_link_range = None; + let mut display_link = String::new(); + + while let Some((event, range)) = offset_iter.next() { + match event { + Event::Text(code) => { + let code = code.to_string(); + display_link.push_str(&code); + resolvable_link = Some(code); + resolvable_link_range = Some(range); + } + Event::Code(code) => { + let code = code.to_string(); + display_link.push('`'); + display_link.push_str(&code); + display_link.push('`'); + resolvable_link = Some(code); + resolvable_link_range = Some(range); + } + Event::End(_) => { + break; + } + _ => {} + } + } + + LinkData { + resolvable_link, + resolvable_link_range, + display_link, + } +} + +fn offset_explicit_range(md: &str, link_range: &Range, open: u8, close: u8) -> Range { + let mut open_brace = !0; + let mut close_brace = !0; + for (i, b) in md.as_bytes()[link_range.clone()].iter().copied().enumerate().rev() { + let i = i + link_range.start; + if b == close { + close_brace = i; + break; + } + } + + if close_brace < link_range.start || close_brace >= link_range.end { + return link_range.clone(); + } + + let mut nesting = 1; + + for (i, b) in md.as_bytes()[link_range.start..close_brace].iter().copied().enumerate().rev() { + let i = i + link_range.start; + if b == close { + nesting += 1; + } + if b == open { + nesting -= 1; + } + if nesting == 0 { + open_brace = i; + break; + } + } + + assert!(open_brace != close_brace); + + if open_brace < link_range.start || open_brace >= link_range.end { + return link_range.clone(); + } + // do not actually include braces in the span + (open_brace + 1)..close_brace +} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed new file mode 100644 index 0000000000000..6759ee100491d --- /dev/null +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed @@ -0,0 +1,54 @@ +// run-rustfix + +#![deny(rustdoc::redundant_explicit_links)] + +pub fn dummy_target() {} + +/// [dummy_target] +//~^ ERROR redundant explicit link target +/// [`dummy_target`] +//~^ ERROR redundant explicit link target +/// +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// +/// [dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`] TEXT +//~^ ERROR redundant explicit link target +pub fn should_warn_inline() {} + +/// [`Vec`](Vec) +/// [`Vec`](std::vec::Vec) +pub fn should_not_warn_inline() {} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index b3203a2690f0a..7c30b15d993d2 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -1,29 +1,54 @@ +// run-rustfix + #![deny(rustdoc::redundant_explicit_links)] pub fn dummy_target() {} /// [dummy_target](dummy_target) +//~^ ERROR redundant explicit link target /// [`dummy_target`](dummy_target) +//~^ ERROR redundant explicit link target /// /// [Vec](Vec) +//~^ ERROR redundant explicit link target /// [`Vec`](Vec) +//~^ ERROR redundant explicit link target /// [Vec](std::vec::Vec) +//~^ ERROR redundant explicit link target /// [`Vec`](std::vec::Vec) +//~^ ERROR redundant explicit link target /// [std::vec::Vec](Vec) +//~^ ERROR redundant explicit link target /// [`std::vec::Vec`](Vec) +//~^ ERROR redundant explicit link target /// [std::vec::Vec](std::vec::Vec) +//~^ ERROR redundant explicit link target /// [`std::vec::Vec`](std::vec::Vec) +//~^ ERROR redundant explicit link target /// /// [usize](usize) +//~^ ERROR redundant explicit link target /// [`usize`](usize) +//~^ ERROR redundant explicit link target /// [usize](std::primitive::usize) +//~^ ERROR redundant explicit link target /// [`usize`](std::primitive::usize) +//~^ ERROR redundant explicit link target /// [std::primitive::usize](usize) +//~^ ERROR redundant explicit link target /// [`std::primitive::usize`](usize) +//~^ ERROR redundant explicit link target /// [std::primitive::usize](std::primitive::usize) +//~^ ERROR redundant explicit link target /// [`std::primitive::usize`](std::primitive::usize) -pub fn should_warn() {} +//~^ ERROR redundant explicit link target +/// +/// [dummy_target](dummy_target) TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`](dummy_target) TEXT +//~^ ERROR redundant explicit link target +pub fn should_warn_inline() {} /// [`Vec`](Vec) /// [`Vec`](std::vec::Vec) -pub fn should_not_warn() {} +pub fn should_not_warn_inline() {} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr index e72105f394f58..3f10a2e662d6a 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr @@ -1,133 +1,307 @@ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:5:20 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:7:20 | LL | /// [dummy_target](dummy_target) - | ^^^^^^^^^^^^ + | ------------ ^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination | - = note: Explicit link does not affect the original link + = 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/redundant_explicit_links.rs:1:9 + --> $DIR/redundant_explicit_links.rs:3:9 | LL | #![deny(rustdoc::redundant_explicit_links)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: Remove explicit link instead +help: remove explicit link target + | +LL | /// [dummy_target] + | ~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:6:22 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:9:22 | LL | /// [`dummy_target`](dummy_target) - | ^^^^^^^^^^^^ + | -------------- ^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [`dummy_target`] + | ~~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:8:11 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:12:11 | LL | /// [Vec](Vec) - | ^^^ + | --- ^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [Vec] + | ~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:9:13 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:14:13 | LL | /// [`Vec`](Vec) - | ^^^ + | ----- ^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`Vec`] + | ~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:10:11 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:16:11 | LL | /// [Vec](std::vec::Vec) - | ^^^^^^^^^^^^^ + | --- ^^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [Vec] + | ~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:11:13 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:18:13 | LL | /// [`Vec`](std::vec::Vec) - | ^^^^^^^^^^^^^ + | ----- ^^^^^^^^^^^^^ 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 | /// [`Vec`] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:20:21 + | +LL | /// [std::vec::Vec](Vec) + | ------------- ^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:14:21 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:22:23 + | +LL | /// [`std::vec::Vec`](Vec) + | --------------- ^^^ 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 | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:24:21 | LL | /// [std::vec::Vec](std::vec::Vec) - | ^^^^^^^^^^^^^ + | ------------- ^^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:15:23 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:26:23 | LL | /// [`std::vec::Vec`](std::vec::Vec) - | ^^^^^^^^^^^^^ + | --------------- ^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:17:13 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:29:13 | LL | /// [usize](usize) - | ^^^^^ + | ----- ^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [usize] + | ~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:18:15 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:31:15 | LL | /// [`usize`](usize) - | ^^^^^ + | ------- ^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`usize`] + | ~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:19:13 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:33:13 | LL | /// [usize](std::primitive::usize) - | ^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^^^^^^^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [usize] + | ~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:20:15 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:35:15 | LL | /// [`usize`](std::primitive::usize) - | ^^^^^^^^^^^^^^^^^^^^^ + | ------- ^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`usize`] + | ~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:37:29 + | +LL | /// [std::primitive::usize](usize) + | --------------------- ^^^^^ 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 | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:23:29 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:39:31 + | +LL | /// [`std::primitive::usize`](usize) + | ----------------------- ^^^^^ 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 | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:41:29 | LL | /// [std::primitive::usize](std::primitive::usize) - | ^^^^^^^^^^^^^^^^^^^^^ + | --------------------- ^^^^^^^^^^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ -error: redundant explicit rustdoc link - --> $DIR/redundant_explicit_links.rs:24:31 +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:43:31 | LL | /// [`std::primitive::usize`](std::primitive::usize) - | ^^^^^^^^^^^^^^^^^^^^^ + | ----------------------- ^^^^^^^^^^^^^^^^^^^^^ 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 | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:46:20 + | +LL | /// [dummy_target](dummy_target) TEXT + | ------------ ^^^^^^^^^^^^ 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 | /// [dummy_target] TEXT + | ~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:48:22 + | +LL | /// [`dummy_target`](dummy_target) TEXT + | -------------- ^^^^^^^^^^^^ 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 | - = note: Explicit link does not affect the original link - = help: Remove explicit link instead +LL | /// [`dummy_target`] TEXT + | ~~~~~~~~~~~~~~~~ -error: aborting due to 14 previous errors +error: aborting due to 20 previous errors From 46df95817d2f9700f0c5a69ea8b05b1c83d9ee35 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 30 Jun 2023 22:48:20 +0800 Subject: [PATCH 08/26] Support Reference & ReferenceUnknown link lint --- .../passes/collect_intra_doc_links.rs | 71 +- .../passes/lint/redundant_explicit_links.rs | 211 ++++-- .../lints/redundant_explicit_links.fixed | 104 +++ .../lints/redundant_explicit_links.rs | 104 +++ .../lints/redundant_explicit_links.stderr | 702 +++++++++++++++++- 5 files changed, 1130 insertions(+), 62 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0432bd40c9032..0a0ffcb5b1ece 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1040,7 +1040,7 @@ impl LinkCollector<'_, '_> { matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), )?; - self.check_redundant_explicit_link( + self.resolve_display_text( path_str, ResolutionInfo { item_id, @@ -1384,8 +1384,12 @@ impl LinkCollector<'_, '_> { } } - /// Check if resolution of inline link's display text and explicit link are same. - fn check_redundant_explicit_link( + /// Resolve display text if the provided link has separated parts of links. + /// + /// For example: + /// Inline link `[display_text](dest_link)` and reference link `[display_text][reference_link]` has + /// separated parts of links. + fn resolve_display_text( &mut self, explicit_link: &Box, display_res_info: ResolutionInfo, @@ -1393,33 +1397,80 @@ impl LinkCollector<'_, '_> { diag_info: &DiagnosticInfo<'_>, ) { // Check if explicit resolution's path is same as resolution of original link's display text path, e.g. + // + // LinkType::Inline: + // // [target](target) // [`target`](target) // [target](path::to::target) // [`target`](path::to::target) + // [path::to::target](target) + // [`path::to::target`](target) // [path::to::target](path::to::target) // [`path::to::target`](path::to::target) // + // LinkType::ReferenceUnknown + // + // [target][target] + // [`target`][target] + // [target][path::to::target] + // [`target`][path::to::target] + // [path::to::target][target] + // [`path::to::target`][target] + // [path::to::target][path::to::target] + // [`path::to::target`][path::to::target] + // + // LinkType::Reference + // + // [target][target] + // [`target`][target] + // [target][path::to::target] + // [`target`][path::to::target] + // [path::to::target][target] + // [`path::to::target`][target] + // [path::to::target][path::to::target] + // [`path::to::target`][path::to::target] + // + // [target]: target // or [target]: path::to::target + // [path::to::target]: path::to::target // or [path::to::target]: 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 { + if !matches!( + ori_link.kind, + LinkType::Inline | LinkType::Reference | LinkType::ReferenceUnknown + ) { return; } + // Algorithm to check if display text could possibly be the explicit link: + // + // Consider 2 links which are display text and explicit link, pick the shorter + // one as symbol and longer one as full qualified path, and tries to match symbol + // to the full qualified path's last symbol. + // + // Otherwise, check if 2 links are same, if so, skip the resolve process. + // + // Notice that this algorithm is passive, might possibly miss actual redudant cases. + let explicit_link = &explicit_link.to_string(); let display_text = &ori_link.display_text; let display_len = display_text.len(); let explicit_len = explicit_link.len(); - if explicit_len >= display_len - && &explicit_link[(explicit_len - display_len)..] == display_text + if display_len == explicit_len { + // Whether they are same or not, skip the resolve process. + return; + } + + if (explicit_len >= display_len + && &explicit_link[(explicit_len - display_len)..] == display_text) + || (display_len >= explicit_len + && &display_text[(display_len - explicit_len)..] == explicit_link) { self.resolve_with_disambiguator_cached( display_res_info, 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), + false, ); } } diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 1bccedde92a20..722088eb79aa4 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -1,19 +1,20 @@ use std::ops::Range; -use pulldown_cmark::{Parser, BrokenLink, Event, Tag, LinkType, OffsetIter}; +use pulldown_cmark::{BrokenLink, CowStr, Event, LinkType, OffsetIter, Parser, Tag}; use rustc_ast::NodeId; use rustc_errors::SuggestionStyle; +use rustc_hir::def::{DefKind, DocLinkResMap, Namespace, Res}; use rustc_hir::HirId; -use rustc_hir::def::{Namespace, DefKind, DocLinkResMap, Res}; use rustc_lint_defs::Applicability; use rustc_span::Symbol; -use crate::clean::Item; use crate::clean::utils::find_nearest_parent_module; +use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; use crate::passes::source_span_for_markdown_range; +#[derive(Debug)] struct LinkData { resolvable_link: Option, resolvable_link_range: Option>, @@ -34,60 +35,143 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { check_redundant_explicit_link(cx, item, hir_id, &doc); } -fn check_redundant_explicit_link<'md>(cx: &DocContext<'_>, item: &Item, hir_id: HirId, doc: &'md str) { +fn check_redundant_explicit_link<'md>( + cx: &DocContext<'_>, + item: &Item, + hir_id: HirId, + doc: &'md str, +) -> Option<()> { let mut broken_line_callback = |link: BrokenLink<'md>| Some((link.reference, "".into())); - let mut offset_iter = Parser::new_with_broken_link_callback(&doc, main_body_opts(), Some(&mut broken_line_callback)).into_offset_iter(); + let mut offset_iter = Parser::new_with_broken_link_callback( + &doc, + main_body_opts(), + 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 { - Event::Start(Tag::Link(link_type, dest, _)) => { - let link_data = collect_link_data(&mut offset_iter); - let dest = dest.to_string(); - - if link_type == LinkType::Inline { - check_inline_link_redundancy(cx, item, hir_id, doc, link_range, dest, link_data); + Event::Start(Tag::Link(link_type, dest, _)) => match link_type { + LinkType::Inline | LinkType::ReferenceUnknown => { + check_inline_or_reference_unknown_redundancy( + cx, + item, + hir_id, + doc, + resolutions, + link_range, + dest.to_string(), + collect_link_data(&mut offset_iter), + if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, + ); } - } + LinkType::Reference => { + check_reference_redundancy( + cx, + item, + hir_id, + doc, + resolutions, + link_range, + &dest, + collect_link_data(&mut offset_iter), + ); + } + _ => {} + }, _ => {} } } + + None } -fn check_inline_link_redundancy(cx: &DocContext<'_>, item: &Item, hir_id: HirId, doc: &str, link_range: Range, dest: String, link_data: LinkData) -> Option<()> { - 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); - - let (resolvable_link, resolvable_link_range) = (&link_data.resolvable_link?, &link_data.resolvable_link_range?); - let (dest_res, display_res) = (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); +/// FIXME(ChAoSUnItY): Too many arguments. +fn check_inline_or_reference_unknown_redundancy( + cx: &DocContext<'_>, + item: &Item, + hir_id: HirId, + doc: &str, + resolutions: &DocLinkResMap, + link_range: Range, + dest: String, + link_data: LinkData, + (open, close): (u8, u8), +) -> Option<()> { + let (resolvable_link, resolvable_link_range) = + (&link_data.resolvable_link?, &link_data.resolvable_link_range?); + let (dest_res, display_res) = + (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); if dest_res == display_res { - let link_span = source_span_for_markdown_range( + let link_span = source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs) + .unwrap_or(item.attr_span(cx.tcx)); + let explicit_span = source_span_for_markdown_range( cx.tcx, &doc, - &link_range, + &offset_explicit_range(doc, link_range, open, close), &item.attrs, - ).unwrap_or(item.attr_span(cx.tcx)); + )?; + let display_span = + source_span_for_markdown_range(cx.tcx, &doc, &resolvable_link_range, &item.attrs)?; + + cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| { + lint.span_label(explicit_span, "explicit target is redundant") + .span_label(display_span, "because label contains path that resolves to same destination") + .note("when a link's destination is not specified,\nthe label is used to resolve intra-doc links") + .span_suggestion_with_style(link_span, "remove explicit link target", format!("[{}]", link_data.display_link), Applicability::MaybeIncorrect, SuggestionStyle::ShowAlways); + + lint + }); + } + + None +} + +/// FIXME(ChAoSUnItY): Too many arguments. +fn check_reference_redundancy( + cx: &DocContext<'_>, + item: &Item, + hir_id: HirId, + doc: &str, + resolutions: &DocLinkResMap, + link_range: Range, + dest: &CowStr<'_>, + link_data: LinkData, +) -> Option<()> { + let (resolvable_link, resolvable_link_range) = + (&link_data.resolvable_link?, &link_data.resolvable_link_range?); + let (dest_res, display_res) = + (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); + + if dest_res == display_res { + let link_span = source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs) + .unwrap_or(item.attr_span(cx.tcx)); let explicit_span = source_span_for_markdown_range( cx.tcx, &doc, - &offset_explicit_range(doc, &link_range, b'(', b')'), - &item.attrs + &offset_explicit_range(doc, link_range.clone(), b'[', b']'), + &item.attrs, )?; - let display_span = source_span_for_markdown_range( + let display_span = + source_span_for_markdown_range(cx.tcx, &doc, &resolvable_link_range, &item.attrs)?; + let def_span = source_span_for_markdown_range( cx.tcx, &doc, - &resolvable_link_range, - &item.attrs + &offset_reference_def_range(doc, dest, link_range), + &item.attrs, )?; - cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| { lint.span_label(explicit_span, "explicit target is redundant") .span_label(display_span, "because label contains path that resolves to same destination") + .span_note(def_span, "referenced explicit link target defined here") .note("when a link's destination is not specified,\nthe label is used to resolve intra-doc links") .span_suggestion_with_style(link_span, "remove explicit link target", format!("[{}]", link_data.display_link), Applicability::MaybeIncorrect, SuggestionStyle::ShowAlways); @@ -98,17 +182,10 @@ fn check_inline_link_redundancy(cx: &DocContext<'_>, item: &Item, hir_id: HirId, None } -fn find_resolution<'tcx>(resolutions: &'tcx DocLinkResMap, path: &str) -> Option<&'tcx Res> { - for ns in [Namespace::TypeNS, Namespace::ValueNS, Namespace::MacroNS] { - let Some(Some(res)) = resolutions.get(&(Symbol::intern(path), ns)) - else { - continue; - }; - - return Some(res); - } - - None +fn find_resolution(resolutions: &DocLinkResMap, path: &str) -> Option> { + [Namespace::TypeNS, Namespace::ValueNS, Namespace::MacroNS] + .into_iter() + .find_map(|ns| resolutions.get(&(Symbol::intern(path), ns)).copied().flatten()) } /// Collects all neccessary data of link. @@ -116,7 +193,7 @@ fn collect_link_data(offset_iter: &mut OffsetIter<'_, '_>) -> LinkData { let mut resolvable_link = None; let mut resolvable_link_range = None; let mut display_link = String::new(); - + while let Some((event, range)) = offset_iter.next() { match event { Event::Text(code) => { @@ -140,14 +217,10 @@ fn collect_link_data(offset_iter: &mut OffsetIter<'_, '_>) -> LinkData { } } - LinkData { - resolvable_link, - resolvable_link_range, - display_link, - } + LinkData { resolvable_link, resolvable_link_range, display_link } } -fn offset_explicit_range(md: &str, link_range: &Range, open: u8, close: u8) -> Range { +fn offset_explicit_range(md: &str, link_range: Range, open: u8, close: u8) -> Range { let mut open_brace = !0; let mut close_brace = !0; for (i, b) in md.as_bytes()[link_range.clone()].iter().copied().enumerate().rev() { @@ -159,7 +232,7 @@ fn offset_explicit_range(md: &str, link_range: &Range, open: u8, close: u } if close_brace < link_range.start || close_brace >= link_range.end { - return link_range.clone(); + return link_range; } let mut nesting = 1; @@ -181,8 +254,44 @@ fn offset_explicit_range(md: &str, link_range: &Range, open: u8, close: u assert!(open_brace != close_brace); if open_brace < link_range.start || open_brace >= link_range.end { - return link_range.clone(); + return link_range; } // do not actually include braces in the span (open_brace + 1)..close_brace } + +fn offset_reference_def_range( + md: &str, + dest: &CowStr<'_>, + link_range: Range, +) -> Range { + // For diagnostics, we want to underline the link's definition but `span` will point at + // where the link is used. This is a problem for reference-style links, where the definition + // is separate from the usage. + + match dest { + // `Borrowed` variant means the string (the link's destination) may come directly from + // the markdown text and we can locate the original link destination. + // NOTE: LinkReplacer also provides `Borrowed` but possibly from other sources, + // so `locate()` can fall back to use `span`. + CowStr::Borrowed(s) => { + // FIXME: remove this function once pulldown_cmark can provide spans for link definitions. + unsafe { + let s_start = dest.as_ptr(); + let s_end = s_start.add(s.len()); + let md_start = md.as_ptr(); + let md_end = md_start.add(md.len()); + if md_start <= s_start && s_end <= md_end { + let start = s_start.offset_from(md_start) as usize; + let end = s_end.offset_from(md_start) as usize; + start..end + } else { + link_range + } + } + } + + // For anything else, we can only use the provided range. + CowStr::Boxed(_) | CowStr::Inlined(_) => link_range, + } +} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed index 6759ee100491d..4745da879a358 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed @@ -52,3 +52,107 @@ pub fn should_warn_inline() {} /// [`Vec`](Vec) /// [`Vec`](std::vec::Vec) pub fn should_not_warn_inline() {} + +/// [dummy_target] +//~^ ERROR redundant explicit link target +/// [`dummy_target`] +//~^ ERROR redundant explicit link target +/// +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// +/// [dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`] TEXT +//~^ ERROR redundant explicit link target +pub fn should_warn_reference_unknown() {} + +/// [`Vec`][Vec] +/// [`Vec`][std::vec::Vec] +pub fn should_not_warn_reference_unknown() {} + +/// [dummy_target] +//~^ ERROR redundant explicit link target +/// [`dummy_target`] +//~^ ERROR redundant explicit link target +/// +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`] +//~^ ERROR redundant explicit link target +/// +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [usize] +//~^ ERROR redundant explicit link target +/// [`usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`] +//~^ ERROR redundant explicit link target +/// +/// [dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`] TEXT +//~^ ERROR redundant explicit link target +/// +/// [dummy_target]: dummy_target +/// [Vec]: Vec +/// [std::vec::Vec]: Vec +/// [usize]: usize +/// [std::primitive::usize]: usize +pub fn should_warn_reference() {} + +/// [`Vec`]: Vec +/// [`Vec`]: std::vec::Vec +pub fn should_not_warn_reference() {} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index 7c30b15d993d2..7dd8eba5a5e3f 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -52,3 +52,107 @@ pub fn should_warn_inline() {} /// [`Vec`](Vec) /// [`Vec`](std::vec::Vec) pub fn should_not_warn_inline() {} + +/// [dummy_target][dummy_target] +//~^ ERROR redundant explicit link target +/// [`dummy_target`][dummy_target] +//~^ ERROR redundant explicit link target +/// +/// [Vec][Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`][Vec] +//~^ ERROR redundant explicit link target +/// [Vec][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec][Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`][Vec] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// +/// [usize][usize] +//~^ ERROR redundant explicit link target +/// [`usize`][usize] +//~^ ERROR redundant explicit link target +/// [usize][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`usize`][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize][usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`][usize] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// +/// [dummy_target][dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`][dummy_target] TEXT +//~^ ERROR redundant explicit link target +pub fn should_warn_reference_unknown() {} + +/// [`Vec`][Vec] +/// [`Vec`][std::vec::Vec] +pub fn should_not_warn_reference_unknown() {} + +/// [dummy_target][dummy_target] +//~^ ERROR redundant explicit link target +/// [`dummy_target`][dummy_target] +//~^ ERROR redundant explicit link target +/// +/// [Vec][Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`][Vec] +//~^ ERROR redundant explicit link target +/// [Vec][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`Vec`][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec][Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`][Vec] +//~^ ERROR redundant explicit link target +/// [std::vec::Vec][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// [`std::vec::Vec`][std::vec::Vec] +//~^ ERROR redundant explicit link target +/// +/// [usize][usize] +//~^ ERROR redundant explicit link target +/// [`usize`][usize] +//~^ ERROR redundant explicit link target +/// [usize][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`usize`][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize][usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`][usize] +//~^ ERROR redundant explicit link target +/// [std::primitive::usize][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// [`std::primitive::usize`][std::primitive::usize] +//~^ ERROR redundant explicit link target +/// +/// [dummy_target][dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// [`dummy_target`][dummy_target] TEXT +//~^ ERROR redundant explicit link target +/// +/// [dummy_target]: dummy_target +/// [Vec]: Vec +/// [std::vec::Vec]: Vec +/// [usize]: usize +/// [std::primitive::usize]: usize +pub fn should_warn_reference() {} + +/// [`Vec`]: Vec +/// [`Vec`]: std::vec::Vec +pub fn should_not_warn_reference() {} diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr index 3f10a2e662d6a..34ec9be6646c8 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.stderr +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.stderr @@ -303,5 +303,705 @@ help: remove explicit link target LL | /// [`dummy_target`] TEXT | ~~~~~~~~~~~~~~~~ -error: aborting due to 20 previous errors +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:56:20 + | +LL | /// [dummy_target][dummy_target] + | ------------ ^^^^^^^^^^^^ 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 | /// [dummy_target] + | ~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:58:22 + | +LL | /// [`dummy_target`][dummy_target] + | -------------- ^^^^^^^^^^^^ 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 | /// [`dummy_target`] + | ~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:61:11 + | +LL | /// [Vec][Vec] + | --- ^^^ 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 | /// [Vec] + | ~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:63:13 + | +LL | /// [`Vec`][Vec] + | ----- ^^^ 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 | /// [`Vec`] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:65:11 + | +LL | /// [Vec][std::vec::Vec] + | --- ^^^^^^^^^^^^^ 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 | /// [Vec] + | ~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:67:13 + | +LL | /// [`Vec`][std::vec::Vec] + | ----- ^^^^^^^^^^^^^ 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 | /// [`Vec`] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:69:21 + | +LL | /// [std::vec::Vec][Vec] + | ------------- ^^^ 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 | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:71:23 + | +LL | /// [`std::vec::Vec`][Vec] + | --------------- ^^^ 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 | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:73:21 + | +LL | /// [std::vec::Vec][std::vec::Vec] + | ------------- ^^^^^^^^^^^^^ 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 | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:75:23 + | +LL | /// [`std::vec::Vec`][std::vec::Vec] + | --------------- ^^^^^^^^^^^^^ 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 | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:78:13 + | +LL | /// [usize][usize] + | ----- ^^^^^ 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 | /// [usize] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:80:15 + | +LL | /// [`usize`][usize] + | ------- ^^^^^ 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 | /// [`usize`] + | ~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:82:13 + | +LL | /// [usize][std::primitive::usize] + | ----- ^^^^^^^^^^^^^^^^^^^^^ 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 | /// [usize] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:84:15 + | +LL | /// [`usize`][std::primitive::usize] + | ------- ^^^^^^^^^^^^^^^^^^^^^ 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 | /// [`usize`] + | ~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:86:29 + | +LL | /// [std::primitive::usize][usize] + | --------------------- ^^^^^ 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 | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:88:31 + | +LL | /// [`std::primitive::usize`][usize] + | ----------------------- ^^^^^ 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 | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:90:29 + | +LL | /// [std::primitive::usize][std::primitive::usize] + | --------------------- ^^^^^^^^^^^^^^^^^^^^^ 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 | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:92:31 + | +LL | /// [`std::primitive::usize`][std::primitive::usize] + | ----------------------- ^^^^^^^^^^^^^^^^^^^^^ 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 | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:95:20 + | +LL | /// [dummy_target][dummy_target] TEXT + | ------------ ^^^^^^^^^^^^ 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 | /// [dummy_target] TEXT + | ~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:97:22 + | +LL | /// [`dummy_target`][dummy_target] TEXT + | -------------- ^^^^^^^^^^^^ 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 | /// [`dummy_target`] TEXT + | ~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:105:20 + | +LL | /// [dummy_target][dummy_target] + | ------------ ^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:149:21 + | +LL | /// [dummy_target]: dummy_target + | ^^^^^^^^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [dummy_target] + | ~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:107:22 + | +LL | /// [`dummy_target`][dummy_target] + | -------------- ^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:149:21 + | +LL | /// [dummy_target]: dummy_target + | ^^^^^^^^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`dummy_target`] + | ~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:110:11 + | +LL | /// [Vec][Vec] + | --- ^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:150:12 + | +LL | /// [Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [Vec] + | ~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:112:13 + | +LL | /// [`Vec`][Vec] + | ----- ^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:150:12 + | +LL | /// [Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`Vec`] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:114:11 + | +LL | /// [Vec][std::vec::Vec] + | --- ^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:151:22 + | +LL | /// [std::vec::Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [Vec] + | ~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:116:13 + | +LL | /// [`Vec`][std::vec::Vec] + | ----- ^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:151:22 + | +LL | /// [std::vec::Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`Vec`] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:118:21 + | +LL | /// [std::vec::Vec][Vec] + | ------------- ^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:150:12 + | +LL | /// [Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:120:23 + | +LL | /// [`std::vec::Vec`][Vec] + | --------------- ^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:150:12 + | +LL | /// [Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:122:21 + | +LL | /// [std::vec::Vec][std::vec::Vec] + | ------------- ^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:151:22 + | +LL | /// [std::vec::Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [std::vec::Vec] + | ~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:124:23 + | +LL | /// [`std::vec::Vec`][std::vec::Vec] + | --------------- ^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:151:22 + | +LL | /// [std::vec::Vec]: Vec + | ^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`std::vec::Vec`] + | ~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:127:13 + | +LL | /// [usize][usize] + | ----- ^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:152:14 + | +LL | /// [usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [usize] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:129:15 + | +LL | /// [`usize`][usize] + | ------- ^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:152:14 + | +LL | /// [usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`usize`] + | ~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:131:13 + | +LL | /// [usize][std::primitive::usize] + | ----- ^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:153:30 + | +LL | /// [std::primitive::usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [usize] + | ~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:133:15 + | +LL | /// [`usize`][std::primitive::usize] + | ------- ^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:153:30 + | +LL | /// [std::primitive::usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`usize`] + | ~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:135:29 + | +LL | /// [std::primitive::usize][usize] + | --------------------- ^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:152:14 + | +LL | /// [usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:137:31 + | +LL | /// [`std::primitive::usize`][usize] + | ----------------------- ^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:152:14 + | +LL | /// [usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:139:29 + | +LL | /// [std::primitive::usize][std::primitive::usize] + | --------------------- ^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:153:30 + | +LL | /// [std::primitive::usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [std::primitive::usize] + | ~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:141:31 + | +LL | /// [`std::primitive::usize`][std::primitive::usize] + | ----------------------- ^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:153:30 + | +LL | /// [std::primitive::usize]: usize + | ^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`std::primitive::usize`] + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:144:20 + | +LL | /// [dummy_target][dummy_target] TEXT + | ------------ ^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:149:21 + | +LL | /// [dummy_target]: dummy_target + | ^^^^^^^^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [dummy_target] TEXT + | ~~~~~~~~~~~~~~ + +error: redundant explicit link target + --> $DIR/redundant_explicit_links.rs:146:22 + | +LL | /// [`dummy_target`][dummy_target] TEXT + | -------------- ^^^^^^^^^^^^ explicit target is redundant + | | + | because label contains path that resolves to same destination + | +note: referenced explicit link target defined here + --> $DIR/redundant_explicit_links.rs:149:21 + | +LL | /// [dummy_target]: dummy_target + | ^^^^^^^^^^^^ + = note: when a link's destination is not specified, + the label is used to resolve intra-doc links +help: remove explicit link target + | +LL | /// [`dummy_target`] TEXT + | ~~~~~~~~~~~~~~~~ + +error: aborting due to 60 previous errors From 5ce6cc7df3175519219c091059dd663313438c97 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sat, 1 Jul 2023 00:55:37 +0800 Subject: [PATCH 09/26] Still resolving rustdoc resolution panicking --- compiler/rustc_resolve/src/rustdoc.rs | 6 +- .../passes/collect_intra_doc_links.rs | 69 ++++++----------- .../passes/lint/redundant_explicit_links.rs | 76 +++++++++++++------ .../lints/redundant_explicit_links.fixed | 6 +- .../lints/redundant_explicit_links.rs | 6 +- tests/rustdoc-ui/unescaped_backticks.rs | 1 + tests/rustdoc/description.rs | 2 +- tests/rustdoc/intra-doc/basic.rs | 2 + tests/rustdoc/intra-doc/generic-params.rs | 1 + tests/rustdoc/intra-doc/issue-108459.rs | 1 + 10 files changed, 90 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index 083d16d3b047d..f7275bed59c15 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -410,8 +410,10 @@ fn parse_links<'md>(doc: &'md str) -> Vec> { while let Some(event) = event_iter.next() { match event { Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { - if let Some(display_text) = collect_link_data(&mut event_iter) { - links.push(display_text); + if matches!(link_type, LinkType::Inline | LinkType::ReferenceUnknown | LinkType::Reference) { + if let Some(display_text) = collect_link_data(&mut event_iter) { + links.push(display_text); + } } links.push(preprocess_link(&dest)); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0a0ffcb5b1ece..4c40363ac1d64 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1038,6 +1038,7 @@ impl LinkCollector<'_, '_> { // 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), + false, )?; self.resolve_display_text( @@ -1232,6 +1233,9 @@ impl LinkCollector<'_, '_> { // If errors are cached then they are only reported on first occurrence // which we want in some cases but not in others. cache_errors: bool, + // If this call is intended to be recoverable, then pass true to silence. + // This is only recoverable when path is failed to resolved. + recoverable: bool, ) -> Option<(Res, Option)> { if let Some(res) = self.visited_links.get(&key) { if res.is_some() || cache_errors { @@ -1239,7 +1243,7 @@ impl LinkCollector<'_, '_> { } } - let mut candidates = self.resolve_with_disambiguator(&key, diag.clone()); + let mut candidates = self.resolve_with_disambiguator(&key, diag.clone(), recoverable); // FIXME: it would be nice to check that the feature gate was enabled in the original crate, not just ignore it altogether. // However I'm not sure how to check that across crates. @@ -1290,6 +1294,9 @@ impl LinkCollector<'_, '_> { &mut self, key: &ResolutionInfo, diag: DiagnosticInfo<'_>, + // If this call is intended to be recoverable, then pass true to silence. + // This is only recoverable when path is failed to resolved. + recoverable: bool, ) -> Vec<(Res, Option)> { let disambiguator = key.dis; let path_str = &key.path_str; @@ -1319,7 +1326,9 @@ impl LinkCollector<'_, '_> { } } } - resolution_failure(self, diag, path_str, disambiguator, smallvec![err]); + if !recoverable { + resolution_failure(self, diag, path_str, disambiguator, smallvec![err]); + } return vec![]; } } @@ -1356,13 +1365,15 @@ impl LinkCollector<'_, '_> { .fold(0, |acc, res| if let Ok(res) = res { acc + res.len() } else { acc }); if len == 0 { - resolution_failure( - self, - diag, - path_str, - disambiguator, - candidates.into_iter().filter_map(|res| res.err()).collect(), - ); + if !recoverable { + resolution_failure( + self, + diag, + path_str, + disambiguator, + candidates.into_iter().filter_map(|res| res.err()).collect(), + ); + } return vec![]; } else if len == 1 { candidates.into_iter().filter_map(|res| res.ok()).flatten().collect::>() @@ -1396,43 +1407,8 @@ impl LinkCollector<'_, '_> { ori_link: &MarkdownLink, diag_info: &DiagnosticInfo<'_>, ) { - // Check if explicit resolution's path is same as resolution of original link's display text path, e.g. - // - // LinkType::Inline: - // - // [target](target) - // [`target`](target) - // [target](path::to::target) - // [`target`](path::to::target) - // [path::to::target](target) - // [`path::to::target`](target) - // [path::to::target](path::to::target) - // [`path::to::target`](path::to::target) - // - // LinkType::ReferenceUnknown - // - // [target][target] - // [`target`][target] - // [target][path::to::target] - // [`target`][path::to::target] - // [path::to::target][target] - // [`path::to::target`][target] - // [path::to::target][path::to::target] - // [`path::to::target`][path::to::target] - // - // LinkType::Reference - // - // [target][target] - // [`target`][target] - // [target][path::to::target] - // [`target`][path::to::target] - // [path::to::target][target] - // [`path::to::target`][target] - // [path::to::target][path::to::target] - // [`path::to::target`][path::to::target] - // - // [target]: target // or [target]: path::to::target - // [path::to::target]: path::to::target // or [path::to::target]: target + // Check if explicit resolution's path is same as resolution of original link's display text path, see + // tests/rustdoc-ui/lint/redundant_explicit_links.rs for more cases. // // To avoid disambiguator from panicking, we check if display text path is possible to be disambiguated // into explicit path. @@ -1471,6 +1447,7 @@ impl LinkCollector<'_, '_> { display_res_info, diag_info.clone(), // this struct should really be Copy, but Range is not :( false, + true, ); } } diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 722088eb79aa4..da8a73be60b27 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -32,6 +32,13 @@ 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; + } + check_redundant_explicit_link(cx, item, hir_id, &doc); } @@ -57,33 +64,52 @@ fn check_redundant_explicit_link<'md>( while let Some((event, link_range)) = offset_iter.next() { match event { - Event::Start(Tag::Link(link_type, dest, _)) => match link_type { - LinkType::Inline | LinkType::ReferenceUnknown => { - check_inline_or_reference_unknown_redundancy( - cx, - item, - hir_id, - doc, - resolutions, - link_range, - dest.to_string(), - collect_link_data(&mut offset_iter), - if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, - ); + Event::Start(Tag::Link(link_type, dest, _)) => { + let link_data = collect_link_data(&mut offset_iter); + + let explicit_link = dest.to_string(); + let display_link = link_data.resolvable_link.clone()?; + let explicit_len = explicit_link.len(); + let display_len = display_link.len(); + + if explicit_len == display_len && explicit_link != display_link { + // Skips if they possibly have no relativity. + continue; } - LinkType::Reference => { - check_reference_redundancy( - cx, - item, - hir_id, - doc, - resolutions, - link_range, - &dest, - collect_link_data(&mut offset_iter), - ); + + if (explicit_len >= display_len + && &explicit_link[(explicit_len - display_len)..] == display_link) + || (display_len >= explicit_len + && &display_link[(display_len - explicit_len)..] == explicit_link) { + match link_type { + LinkType::Inline | LinkType::ReferenceUnknown => { + check_inline_or_reference_unknown_redundancy( + cx, + item, + hir_id, + doc, + resolutions, + link_range, + dest.to_string(), + link_data, + if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, + ); + } + LinkType::Reference => { + check_reference_redundancy( + cx, + item, + hir_id, + doc, + resolutions, + link_range, + &dest, + link_data, + ); + } + _ => {} + } } - _ => {} }, _ => {} } diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed index 4745da879a358..900234e31e980 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.fixed +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.fixed @@ -42,7 +42,7 @@ pub fn dummy_target() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`] //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target] TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`] TEXT @@ -91,7 +91,7 @@ pub fn should_not_warn_inline() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`] //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target] TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`] TEXT @@ -140,7 +140,7 @@ pub fn should_not_warn_reference_unknown() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`] //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target] TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`] TEXT diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links.rs b/tests/rustdoc-ui/lints/redundant_explicit_links.rs index 7dd8eba5a5e3f..13feb85e05178 100644 --- a/tests/rustdoc-ui/lints/redundant_explicit_links.rs +++ b/tests/rustdoc-ui/lints/redundant_explicit_links.rs @@ -42,7 +42,7 @@ pub fn dummy_target() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`](std::primitive::usize) //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target](dummy_target) TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`](dummy_target) TEXT @@ -91,7 +91,7 @@ pub fn should_not_warn_inline() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`][std::primitive::usize] //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target][dummy_target] TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`][dummy_target] TEXT @@ -140,7 +140,7 @@ pub fn should_not_warn_reference_unknown() {} //~^ ERROR redundant explicit link target /// [`std::primitive::usize`][std::primitive::usize] //~^ ERROR redundant explicit link target -/// +/// /// [dummy_target][dummy_target] TEXT //~^ ERROR redundant explicit link target /// [`dummy_target`][dummy_target] TEXT diff --git a/tests/rustdoc-ui/unescaped_backticks.rs b/tests/rustdoc-ui/unescaped_backticks.rs index e99cd1f3d58ff..e960e9f59e9cf 100644 --- a/tests/rustdoc-ui/unescaped_backticks.rs +++ b/tests/rustdoc-ui/unescaped_backticks.rs @@ -1,6 +1,7 @@ #![deny(rustdoc::unescaped_backticks)] #![allow(rustdoc::broken_intra_doc_links)] #![allow(rustdoc::invalid_html_tags)] +#![allow(rustdoc::redundant_explicit_links)] /// pub fn empty() {} diff --git a/tests/rustdoc/description.rs b/tests/rustdoc/description.rs index 43cd59ebd0924..918ccd6396830 100644 --- a/tests/rustdoc/description.rs +++ b/tests/rustdoc/description.rs @@ -26,5 +26,5 @@ pub fn foo_fn() {} // @has 'foo/fn.bar_fn.html' '//meta[@name="description"]/@content' \ // 'Description with intra-doc link to foo_fn and [nonexistent_item] and foo_fn.' #[allow(rustdoc::broken_intra_doc_links)] -/// Description with intra-doc link to [foo_fn] and [nonexistent_item] and [foo_fn](self::foo_fn). +/// Description with intra-doc link to [foo_fn] and [nonexistent_item] and [foo_fn]. pub fn bar_fn() {} diff --git a/tests/rustdoc/intra-doc/basic.rs b/tests/rustdoc/intra-doc/basic.rs index 96e21137b2dc2..c88a7887f11ec 100644 --- a/tests/rustdoc/intra-doc/basic.rs +++ b/tests/rustdoc/intra-doc/basic.rs @@ -1,3 +1,5 @@ +#![allow(rustdoc::redundant_explicit_links)] + // @has basic/index.html // @has - '//a/@href' 'struct.ThisType.html' // @has - '//a/@title' 'struct basic::ThisType' diff --git a/tests/rustdoc/intra-doc/generic-params.rs b/tests/rustdoc/intra-doc/generic-params.rs index fbc9fc6a9bc21..359f775f97fd1 100644 --- a/tests/rustdoc/intra-doc/generic-params.rs +++ b/tests/rustdoc/intra-doc/generic-params.rs @@ -1,6 +1,7 @@ // ignore-tidy-linelength #![crate_name = "foo"] +#![allow(rustdoc::redundant_explicit_links)] //! Here's a link to [`Vec`] and one to [`Box>>`]. //! Here's a link to [`Iterator>::Item`]. diff --git a/tests/rustdoc/intra-doc/issue-108459.rs b/tests/rustdoc/intra-doc/issue-108459.rs index eb1c7a05e5409..b8cd478b4df69 100644 --- a/tests/rustdoc/intra-doc/issue-108459.rs +++ b/tests/rustdoc/intra-doc/issue-108459.rs @@ -1,4 +1,5 @@ #![deny(rustdoc::broken_intra_doc_links)] +#![allow(rustdoc::redundant_explicit_links)] pub struct S; pub mod char {} From 0e2f2cccd748028a39603e46afc56d7c20bcc7c8 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sat, 1 Jul 2023 01:20:33 +0800 Subject: [PATCH 10/26] Add check-pass tests and fix test behavior --- tests/rustdoc-ui/lints/no-redundancy.rs | 4 ++++ tests/rustdoc/description.rs | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 tests/rustdoc-ui/lints/no-redundancy.rs diff --git a/tests/rustdoc-ui/lints/no-redundancy.rs b/tests/rustdoc-ui/lints/no-redundancy.rs new file mode 100644 index 0000000000000..17582dff6c802 --- /dev/null +++ b/tests/rustdoc-ui/lints/no-redundancy.rs @@ -0,0 +1,4 @@ +// check-pass + +/// [Vec][std::vec::Vec#examples] should not warn, because it's not actually redundant! +pub fn func() {} diff --git a/tests/rustdoc/description.rs b/tests/rustdoc/description.rs index 918ccd6396830..aabbb4c4c8f63 100644 --- a/tests/rustdoc/description.rs +++ b/tests/rustdoc/description.rs @@ -1,4 +1,5 @@ #![crate_name = "foo"] +#![allow(rustdoc::redundant_explicit_links)] //! # Description test crate //! //! This is the contents of the test crate docstring. @@ -26,5 +27,5 @@ pub fn foo_fn() {} // @has 'foo/fn.bar_fn.html' '//meta[@name="description"]/@content' \ // 'Description with intra-doc link to foo_fn and [nonexistent_item] and foo_fn.' #[allow(rustdoc::broken_intra_doc_links)] -/// Description with intra-doc link to [foo_fn] and [nonexistent_item] and [foo_fn]. +/// Description with intra-doc link to [foo_fn] and [nonexistent_item] and [foo_fn](self::foo_fn). pub fn bar_fn() {} From fe17ae3af6e1ff42b81a5309dc88b36dacfc8577 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sat, 1 Jul 2023 01:23:20 +0800 Subject: [PATCH 11/26] add missing deny lint --- tests/rustdoc-ui/lints/no-redundancy.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/rustdoc-ui/lints/no-redundancy.rs b/tests/rustdoc-ui/lints/no-redundancy.rs index 17582dff6c802..f9c5e17a5f39f 100644 --- a/tests/rustdoc-ui/lints/no-redundancy.rs +++ b/tests/rustdoc-ui/lints/no-redundancy.rs @@ -1,4 +1,6 @@ // check-pass +#![deny(rustdoc::redundant_explicit_links)] + /// [Vec][std::vec::Vec#examples] should not warn, because it's not actually redundant! pub fn func() {} From 78c85f439f04f2aa484a9f1be3133d74bda87c7d Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sat, 1 Jul 2023 01:29:40 +0800 Subject: [PATCH 12/26] fomar files --- compiler/rustc_resolve/src/rustdoc.rs | 9 ++++++++- .../passes/lint/redundant_explicit_links.rs | 13 +++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index f7275bed59c15..cbda0535c8997 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -410,7 +410,14 @@ fn parse_links<'md>(doc: &'md str) -> Vec> { while let Some(event) = event_iter.next() { match event { Event::Start(Tag::Link(link_type, dest, _)) if may_be_doc_link(link_type) => { - if matches!(link_type, LinkType::Inline | LinkType::ReferenceUnknown | LinkType::Reference) { + if matches!( + link_type, + LinkType::Inline + | LinkType::ReferenceUnknown + | LinkType::Reference + | LinkType::Shortcut + | LinkType::ShortcutUnknown + ) { if let Some(display_text) = collect_link_data(&mut event_iter) { links.push(display_text); } diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index da8a73be60b27..a41e278fd8165 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -34,7 +34,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { if item.link_names(&cx.cache).is_empty() { // If there's no link names in this item, - // then we skip resolution querying to + // then we skip resolution querying to // avoid from panicking. return; } @@ -80,7 +80,8 @@ fn check_redundant_explicit_link<'md>( if (explicit_len >= display_len && &explicit_link[(explicit_len - display_len)..] == display_link) || (display_len >= explicit_len - && &display_link[(display_len - explicit_len)..] == explicit_link) { + && &display_link[(display_len - explicit_len)..] == explicit_link) + { match link_type { LinkType::Inline | LinkType::ReferenceUnknown => { check_inline_or_reference_unknown_redundancy( @@ -92,7 +93,11 @@ fn check_redundant_explicit_link<'md>( link_range, dest.to_string(), link_data, - if link_type == LinkType::Inline { (b'(', b')') } else { (b'[', b']') }, + if link_type == LinkType::Inline { + (b'(', b')') + } else { + (b'[', b']') + }, ); } LinkType::Reference => { @@ -110,7 +115,7 @@ fn check_redundant_explicit_link<'md>( _ => {} } } - }, + } _ => {} } } From ecb26376e5d4a03bc5f536b315161d5450bd914e Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sun, 2 Jul 2023 23:32:46 +0800 Subject: [PATCH 13/26] narrow down the lint trigger constraint --- compiler/rustc_resolve/src/rustdoc.rs | 19 ++++++---- src/librustdoc/html/markdown.rs | 35 ++++++++++++++----- .../passes/collect_intra_doc_links.rs | 28 ++++++++------- .../passes/lint/redundant_explicit_links.rs | 10 ++++++ tests/rustdoc-ui/lints/no-redundancy.rs | 1 + 5 files changed, 65 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index cbda0535c8997..ba7417b6ddaf6 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -1,4 +1,4 @@ -use pulldown_cmark::{BrokenLink, Event, LinkType, Options, Parser, Tag}; +use pulldown_cmark::{BrokenLink, CowStr, Event, LinkType, Options, Parser, Tag}; use rustc_ast as ast; use rustc_ast::util::comments::beautify_doc_string; use rustc_data_structures::fx::FxHashMap; @@ -436,15 +436,22 @@ fn parse_links<'md>(doc: &'md str) -> Vec> { fn collect_link_data<'input, 'callback>( event_iter: &mut Parser<'input, 'callback>, ) -> Option> { - let mut display_text = None; + let mut display_text: Option = None; + let mut append_text = |text: CowStr<'_>| { + if let Some(display_text) = &mut display_text { + display_text.push_str(&text); + } else { + display_text = Some(text.to_string()); + } + }; while let Some(event) = event_iter.next() { match event { - Event::Text(code) => { - display_text = Some(code.to_string().into_boxed_str()); + Event::Text(text) => { + append_text(text); } Event::Code(code) => { - display_text = Some(code.to_string().into_boxed_str()); + append_text(code); } Event::End(_) => { break; @@ -453,5 +460,5 @@ fn collect_link_data<'input, 'callback>( } } - display_text + display_text.map(String::into_boxed_str) } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 2fe052283a99a..98cc38a10d44d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1240,7 +1240,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin pub(crate) struct MarkdownLink { pub kind: LinkType, pub link: String, - pub display_text: String, + pub display_text: Option, pub range: MarkdownLinkRange, } @@ -1402,13 +1402,23 @@ pub(crate) fn markdown_links<'md, R>( LinkType::Autolink | LinkType::Email => unreachable!(), }; - let display_text = - collect_link_data(&mut event_iter).map_or(String::new(), CowStr::into_string); + let display_text = if matches!( + link_type, + LinkType::Inline + | LinkType::ReferenceUnknown + | LinkType::Reference + | LinkType::Shortcut + | LinkType::ShortcutUnknown + ) { + collect_link_data(&mut event_iter) + } else { + None + }; if let Some(link) = preprocess_link(MarkdownLink { kind: link_type, - display_text, link: dest.into_string(), + display_text, range, }) { links.push(link); @@ -1424,16 +1434,23 @@ pub(crate) fn markdown_links<'md, R>( /// Collects additional data of link. fn collect_link_data<'input, 'callback>( event_iter: &mut OffsetIter<'input, 'callback>, -) -> Option> { - let mut display_text = None; +) -> Option { + let mut display_text: Option = None; + let mut append_text = |text: CowStr<'_>| { + if let Some(display_text) = &mut display_text { + display_text.push_str(&text); + } else { + display_text = Some(text.to_string()); + } + }; while let Some((event, _span)) = event_iter.next() { match event { - Event::Text(code) => { - display_text = Some(code); + Event::Text(text) => { + append_text(text); } Event::Code(code) => { - display_text = Some(code); + append_text(code); } Event::End(_) => { break; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 4c40363ac1d64..36872266ee12f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1041,18 +1041,20 @@ impl LinkCollector<'_, '_> { false, )?; - self.resolve_display_text( - path_str, - ResolutionInfo { - item_id, - module_id, - dis: disambiguator, - path_str: ori_link.display_text.clone().into_boxed_str(), - extra_fragment: extra_fragment.clone(), - }, - &ori_link, - &diag_info, - ); + if ori_link.display_text.is_some() { + self.resolve_display_text( + path_str, + ResolutionInfo { + item_id, + module_id, + dis: disambiguator, + path_str: ori_link.display_text.clone()?.into_boxed_str(), + extra_fragment: extra_fragment.clone(), + }, + &ori_link, + &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. @@ -1429,7 +1431,7 @@ impl LinkCollector<'_, '_> { // // Notice that this algorithm is passive, might possibly miss actual redudant cases. let explicit_link = &explicit_link.to_string(); - let display_text = &ori_link.display_text; + let display_text = ori_link.display_text.as_ref().unwrap(); let display_len = display_text.len(); let explicit_len = explicit_link.len(); diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index a41e278fd8165..da71e537f6848 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -67,6 +67,16 @@ fn check_redundant_explicit_link<'md>( Event::Start(Tag::Link(link_type, dest, _)) => { let link_data = collect_link_data(&mut offset_iter); + if let Some(resolvable_link) = link_data.resolvable_link.as_ref() { + if &link_data.display_link != resolvable_link { + // Skips if display link does not match to actual + // resolvable link, usually happens if display link + // has several segments, e.g. + // [this is just an `Option`](Option) + continue; + } + } + let explicit_link = dest.to_string(); let display_link = link_data.resolvable_link.clone()?; let explicit_len = explicit_link.len(); diff --git a/tests/rustdoc-ui/lints/no-redundancy.rs b/tests/rustdoc-ui/lints/no-redundancy.rs index f9c5e17a5f39f..e3358728b1b11 100644 --- a/tests/rustdoc-ui/lints/no-redundancy.rs +++ b/tests/rustdoc-ui/lints/no-redundancy.rs @@ -3,4 +3,5 @@ #![deny(rustdoc::redundant_explicit_links)] /// [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() {} From f0b2cca185555ba20d7b95d073eedd03cda99ce9 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Mon, 3 Jul 2023 08:39:44 +0800 Subject: [PATCH 14/26] lint links --- compiler/rustc_errors/src/lib.rs | 6 +++--- library/std/src/primitive_docs.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7d660d2dbaa99..bbbe21a5893de 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -452,11 +452,11 @@ struct HandlerInner { /// have been converted. check_unstable_expect_diagnostics: bool, - /// Expected [`Diagnostic`][diagnostic::Diagnostic]s store a [`LintExpectationId`] as part of + /// Expected [`Diagnostic`][struct@diagnostic::Diagnostic]s store a [`LintExpectationId`] as part of /// the lint level. [`LintExpectationId`]s created early during the compilation /// (before `HirId`s have been defined) are not stable and can therefore not be /// stored on disk. This buffer stores these diagnostics until the ID has been - /// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`][diagnostic::Diagnostic]s are the + /// replaced by a stable [`LintExpectationId`]. The [`Diagnostic`][struct@diagnostic::Diagnostic]s are the /// submitted for storage and added to the list of fulfilled expectations. unstable_expect_diagnostics: Vec, @@ -1384,7 +1384,7 @@ impl HandlerInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; - diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {}); + diagnostic.children.extract_from(already_emitted_sub).for_each(|_| {}); self.emitter.emit_diagnostic(diagnostic); if diagnostic.is_error() { diff --git a/library/std/src/primitive_docs.rs b/library/std/src/primitive_docs.rs index 80289ca08c3fc..7180236fb2224 100644 --- a/library/std/src/primitive_docs.rs +++ b/library/std/src/primitive_docs.rs @@ -624,7 +624,7 @@ mod prim_pointer {} /// array implementations) succeed if the input slice length is the same as the result /// array length. They optimize especially well when the optimizer can easily determine /// the slice length, e.g. `<[u8; 4]>::try_from(&slice[4..8]).unwrap()`. Array implements -/// [TryFrom](crate::convert::TryFrom) returning: +/// [TryFrom] returning: /// /// - `[T; N]` copies from the slice's elements /// - `&[T; N]` references the original slice's elements From 713e78cdd8d24cbefbc6b1ff97b15bf4c30b6567 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Mon, 3 Jul 2023 08:40:34 +0800 Subject: [PATCH 15/26] fix --- compiler/rustc_errors/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index bbbe21a5893de..34518b53759d7 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1384,7 +1384,7 @@ impl HandlerInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; - diagnostic.children.extract_from(already_emitted_sub).for_each(|_| {}); + diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {}); self.emitter.emit_diagnostic(diagnostic); if diagnostic.is_error() { From 2ec3e297ab054054d428d7d28666b95484b57fc0 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Mon, 3 Jul 2023 08:46:33 +0800 Subject: [PATCH 16/26] tidy doc link --- library/core/src/primitive_docs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 80289ca08c3fc..7180236fb2224 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -624,7 +624,7 @@ mod prim_pointer {} /// array implementations) succeed if the input slice length is the same as the result /// array length. They optimize especially well when the optimizer can easily determine /// the slice length, e.g. `<[u8; 4]>::try_from(&slice[4..8]).unwrap()`. Array implements -/// [TryFrom](crate::convert::TryFrom) returning: +/// [TryFrom] returning: /// /// - `[T; N]` copies from the slice's elements /// - `&[T; N]` references the original slice's elements From c4afb8a8684aceabf54030c7e2648494d3c9bbe2 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Mon, 3 Jul 2023 10:30:29 +0800 Subject: [PATCH 17/26] resolve conflicts --- library/core/src/lib.rs | 2 ++ library/core/src/primitive_docs.rs | 2 +- library/std/src/primitive_docs.rs | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 48c3c1f212344..a2729b3743cc2 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -97,6 +97,8 @@ #![allow(incomplete_features)] #![warn(multiple_supertrait_upcastable)] #![cfg_attr(not(bootstrap), allow(internal_features))] +// Do not check link redundancy on bootstraping phase +#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))] // // Library features: // tidy-alphabetical-start diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 7180236fb2224..80289ca08c3fc 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -624,7 +624,7 @@ mod prim_pointer {} /// array implementations) succeed if the input slice length is the same as the result /// array length. They optimize especially well when the optimizer can easily determine /// the slice length, e.g. `<[u8; 4]>::try_from(&slice[4..8]).unwrap()`. Array implements -/// [TryFrom] returning: +/// [TryFrom](crate::convert::TryFrom) returning: /// /// - `[T; N]` copies from the slice's elements /// - `&[T; N]` references the original slice's elements diff --git a/library/std/src/primitive_docs.rs b/library/std/src/primitive_docs.rs index 7180236fb2224..80289ca08c3fc 100644 --- a/library/std/src/primitive_docs.rs +++ b/library/std/src/primitive_docs.rs @@ -624,7 +624,7 @@ mod prim_pointer {} /// array implementations) succeed if the input slice length is the same as the result /// array length. They optimize especially well when the optimizer can easily determine /// the slice length, e.g. `<[u8; 4]>::try_from(&slice[4..8]).unwrap()`. Array implements -/// [TryFrom] returning: +/// [TryFrom](crate::convert::TryFrom) returning: /// /// - `[T; N]` copies from the slice's elements /// - `&[T; N]` references the original slice's elements From 4896fc0f5919d7facee3ce77e089495cc133dc74 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Mon, 3 Jul 2023 23:34:38 +0800 Subject: [PATCH 18/26] resolve conflicts --- library/std/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ac4ce222fbaac..58684ffe500ad 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -223,6 +223,7 @@ #![cfg_attr(not(bootstrap), allow(internal_features))] #![deny(rustc::existing_doc_keyword)] #![deny(fuzzy_provenance_casts)] +#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))] // Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind` #![deny(ffi_unwind_calls)] // std may use features in a platform-specific way From 15ece93e34be62b383c5b67744d8ad68678fb954 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Tue, 4 Jul 2023 01:51:31 +0800 Subject: [PATCH 19/26] relax redundancy constraint --- library/alloc/src/sync.rs | 6 ++---- src/librustdoc/passes/lint/redundant_explicit_links.rs | 7 +------ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index e2a2fe932ab27..476a4fea54f82 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -153,7 +153,7 @@ macro_rules! acquire { /// /// ## `Deref` behavior /// -/// `Arc` automatically dereferences to `T` (via the [`Deref`][deref] trait), +/// `Arc` automatically dereferences to `T` (via the [`Deref`] trait), /// so you can call `T`'s methods on a value of type `Arc`. To avoid name /// clashes with `T`'s methods, the methods of `Arc` itself are associated /// functions, called using [fully qualified syntax]: @@ -187,7 +187,6 @@ macro_rules! acquire { /// [mutex]: ../../std/sync/struct.Mutex.html /// [rwlock]: ../../std/sync/struct.RwLock.html /// [atomic]: core::sync::atomic -/// [deref]: core::ops::Deref /// [downgrade]: Arc::downgrade /// [upgrade]: Weak::upgrade /// [RefCell\]: core::cell::RefCell @@ -1495,7 +1494,7 @@ impl Arc { /// alignment as `T`. This is trivially true if `U` is `T`. /// Note that if `U` is not `T` but has the same size and alignment, this is /// basically like transmuting references of different types. See - /// [`mem::transmute`][transmute] for more information on what + /// [`mem::transmute`] for more information on what /// restrictions apply in this case. /// /// The raw pointer must point to a block of memory allocated by `alloc` @@ -1507,7 +1506,6 @@ impl Arc { /// even if the returned `Arc` is never accessed. /// /// [into_raw]: Arc::into_raw - /// [transmute]: core::mem::transmute /// /// # Examples /// diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index da71e537f6848..94d07ec034d71 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -68,7 +68,7 @@ fn check_redundant_explicit_link<'md>( let link_data = collect_link_data(&mut offset_iter); if let Some(resolvable_link) = link_data.resolvable_link.as_ref() { - if &link_data.display_link != resolvable_link { + if &link_data.display_link.replace("`", "") != resolvable_link { // Skips if display link does not match to actual // resolvable link, usually happens if display link // has several segments, e.g. @@ -82,11 +82,6 @@ fn check_redundant_explicit_link<'md>( let explicit_len = explicit_link.len(); let display_len = display_link.len(); - if explicit_len == display_len && explicit_link != display_link { - // Skips if they possibly have no relativity. - continue; - } - if (explicit_len >= display_len && &explicit_link[(explicit_len - display_len)..] == display_link) || (display_len >= explicit_len From 62113f6657d91ba294abe96852c25c1c4eb4c501 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Tue, 4 Jul 2023 02:40:05 +0800 Subject: [PATCH 20/26] fix `unescaped_backticks` error --- tests/rustdoc-ui/unescaped_backticks.stderr | 128 ++++++++++---------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/tests/rustdoc-ui/unescaped_backticks.stderr b/tests/rustdoc-ui/unescaped_backticks.stderr index bf1f18889c40f..83822f778d0f4 100644 --- a/tests/rustdoc-ui/unescaped_backticks.stderr +++ b/tests/rustdoc-ui/unescaped_backticks.stderr @@ -1,5 +1,5 @@ error: unescaped backtick - --> $DIR/unescaped_backticks.rs:186:70 + --> $DIR/unescaped_backticks.rs:187:70 | LL | /// if you want your MIR to be modified by the full MIR pipeline, or `#![custom_mir(dialect = | ^ @@ -19,7 +19,7 @@ LL | /// if you want your MIR to be modified by the full MIR pipeline, or \`#![c | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:231:13 + --> $DIR/unescaped_backticks.rs:232:13 | LL | //! `#![rustc_expected_cgu_reuse(module="spike", cfg="rpass2", kind="post-lto")] | ^ @@ -34,7 +34,7 @@ LL | //! \`#![rustc_expected_cgu_reuse(module="spike", cfg="rpass2", kin | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:236:13 + --> $DIR/unescaped_backticks.rs:237:13 | LL | /// `cfg=... | ^ @@ -49,7 +49,7 @@ LL | /// \`cfg=... | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:240:42 + --> $DIR/unescaped_backticks.rs:241:42 | LL | /// `cfg=... and not `#[cfg_attr]` | ^ @@ -64,7 +64,7 @@ LL | /// `cfg=... and not `#[cfg_attr]\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:192:91 + --> $DIR/unescaped_backticks.rs:193:91 | LL | /// Constructs a `TyKind::Error` type and registers a `delay_span_bug` with the given `msg to | ^ @@ -79,7 +79,7 @@ LL | /// Constructs a `TyKind::Error` type and registers a `delay_span_bug` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:201:34 + --> $DIR/unescaped_backticks.rs:202:34 | LL | /// in `nt_to_tokenstream` | ^ @@ -94,7 +94,7 @@ LL | /// in `nt_to_tokenstream\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:207:62 + --> $DIR/unescaped_backticks.rs:208:62 | LL | /// that `Option` only takes up 4 bytes, because `newtype_index! reserves | ^ @@ -109,7 +109,7 @@ LL | /// that `Option` only takes up 4 bytes, because \`newtype_inde | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:215:52 + --> $DIR/unescaped_backticks.rs:216:52 | LL | /// also avoids the need to import `OpenOptions`. | ^ @@ -124,7 +124,7 @@ LL | /// also avoids the need to import `OpenOptions\`. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:220:46 + --> $DIR/unescaped_backticks.rs:221:46 | LL | /// `HybridBitSet`. Has no effect if `row` does not exist. | ^ @@ -139,7 +139,7 @@ LL | /// `HybridBitSet`. Has no effect if `row\` does not exist. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:246:12 + --> $DIR/unescaped_backticks.rs:247:12 | LL | /// RWU`s can get very large, so it uses a more compact representation. | ^ @@ -154,7 +154,7 @@ LL | /// RWU\`s can get very large, so it uses a more compact representation | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:253:15 + --> $DIR/unescaped_backticks.rs:254:15 | LL | /// in `U2`. | ^ @@ -169,7 +169,7 @@ LL | /// in `U2\`. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:270:42 + --> $DIR/unescaped_backticks.rs:271:42 | LL | /// because it contains `[type error]`. Yuck! (See issue #29857 for | ^ @@ -184,7 +184,7 @@ LL | /// because it contains `[type error]\`. Yuck! (See issue #29857 for | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:280:53 + --> $DIR/unescaped_backticks.rs:281:53 | LL | /// well as the second instance of `A: AutoTrait`) to suppress | ^ @@ -199,7 +199,7 @@ LL | /// well as the second instance of `A: AutoTrait\`) to suppress | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:290:40 + --> $DIR/unescaped_backticks.rs:291:40 | LL | /// `'a` with `'b` and not `'static`. But it will have to do for | ^ @@ -211,7 +211,7 @@ LL | /// `'a` with `'b` and not `'static\`. But it will have to do for | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:299:54 + --> $DIR/unescaped_backticks.rs:300:54 | LL | /// `None`. Otherwise, it will return `Some(Dispatch)`. | ^ @@ -226,7 +226,7 @@ LL | /// `None`. Otherwise, it will return `Some(Dispatch)\`. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:303:13 + --> $DIR/unescaped_backticks.rs:304:13 | LL | /// or `None` if it isn't. | ^ @@ -238,7 +238,7 @@ LL | /// or `None\` if it isn't. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:307:14 + --> $DIR/unescaped_backticks.rs:308:14 | LL | /// `on_event` should be called. | ^ @@ -253,7 +253,7 @@ LL | /// `on_event\` should be called. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:312:29 + --> $DIR/unescaped_backticks.rs:313:29 | LL | /// [`rebuild_interest_cache`][rebuild] is called after the value of the max | ^ @@ -268,7 +268,7 @@ LL | /// [`rebuild_interest_cache\`][rebuild] is called after the value of the m | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:322:5 + --> $DIR/unescaped_backticks.rs:323:5 | LL | / /// The Subscriber` may be accessed by calling [`WeakDispatch::upgrade`], LL | | @@ -287,7 +287,7 @@ LL | | /// level changes. to this: `None`. Otherwise, it will return `Some(Dispatch)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:322:5 + --> $DIR/unescaped_backticks.rs:323:5 | LL | / /// The Subscriber` may be accessed by calling [`WeakDispatch::upgrade`], LL | | @@ -304,7 +304,7 @@ LL | | /// level changes. to this: or `None\` if it isn't. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:322:5 + --> $DIR/unescaped_backticks.rs:323:5 | LL | / /// The Subscriber` may be accessed by calling [`WeakDispatch::upgrade`], LL | | @@ -323,7 +323,7 @@ LL | | /// level changes. to this: `on_event\` should be called. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:322:5 + --> $DIR/unescaped_backticks.rs:323:5 | LL | / /// The Subscriber` may be accessed by calling [`WeakDispatch::upgrade`], LL | | @@ -342,7 +342,7 @@ LL | | /// level changes. to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max error: unescaped backtick - --> $DIR/unescaped_backticks.rs:348:56 + --> $DIR/unescaped_backticks.rs:349:56 | LL | /// instead and use [`CloneCounterObserver::counter`] to increment. | ^ @@ -354,7 +354,7 @@ LL | /// instead and use [`CloneCounterObserver::counter\`] to increment. | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:11:5 + --> $DIR/unescaped_backticks.rs:12:5 | LL | /// ` | ^ @@ -366,7 +366,7 @@ LL | /// \` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:18:7 + --> $DIR/unescaped_backticks.rs:19:7 | LL | /// \` | ^ @@ -381,7 +381,7 @@ LL | /// \\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:25:6 + --> $DIR/unescaped_backticks.rs:26:6 | LL | /// [`link1] | ^ @@ -396,7 +396,7 @@ LL | /// [\`link1] | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:29:11 + --> $DIR/unescaped_backticks.rs:30:11 | LL | /// [link2`] | ^ @@ -411,7 +411,7 @@ LL | /// [link2\`] | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:33:6 + --> $DIR/unescaped_backticks.rs:34:6 | LL | /// [`link_long](link_long) | ^ @@ -426,7 +426,7 @@ LL | /// [\`link_long](link_long) | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:37:6 + --> $DIR/unescaped_backticks.rs:38:6 | LL | /// [`broken-link] | ^ @@ -441,7 +441,7 @@ LL | /// [\`broken-link] | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:44:8 + --> $DIR/unescaped_backticks.rs:45:8 | LL | /// | ^ @@ -456,7 +456,7 @@ LL | /// | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:54:6 + --> $DIR/unescaped_backticks.rs:55:6 | LL | /// 🦀`🦀 | ^ @@ -475,7 +475,7 @@ LL | /// 🦀\`🦀 | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:58:5 + --> $DIR/unescaped_backticks.rs:59:5 | LL | /// `foo( | ^ @@ -490,7 +490,7 @@ LL | /// \`foo( | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:64:14 + --> $DIR/unescaped_backticks.rs:65:14 | LL | /// `foo `bar` | ^ @@ -505,7 +505,7 @@ LL | /// `foo `bar\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:70:5 + --> $DIR/unescaped_backticks.rs:71:5 | LL | /// `foo( | ^ @@ -520,7 +520,7 @@ LL | /// \`foo( | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:75:83 + --> $DIR/unescaped_backticks.rs:76:83 | LL | /// Addition is commutative, which means that add(a, b)` is the same as `add(b, a)`. | ^ @@ -535,7 +535,7 @@ LL | /// Addition is commutative, which means that add(a, b)` is the same as `ad | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:79:51 + --> $DIR/unescaped_backticks.rs:80:51 | LL | /// or even to add a number `n` to 42 (`add(42, b)`)! | ^ @@ -550,7 +550,7 @@ LL | /// or even to add a number `n` to 42 (`add(42, b)\`)! | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:83:83 + --> $DIR/unescaped_backticks.rs:84:83 | LL | /// Addition is commutative, which means that `add(a, b) is the same as `add(b, a)`. | ^ @@ -565,7 +565,7 @@ LL | /// Addition is commutative, which means that `add(a, b) is the same as `ad | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:87:51 + --> $DIR/unescaped_backticks.rs:88:51 | LL | /// or even to add a number `n` to 42 (`add(42, n)`)! | ^ @@ -580,7 +580,7 @@ LL | /// or even to add a number `n` to 42 (`add(42, n)\`)! | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:91:83 + --> $DIR/unescaped_backticks.rs:92:83 | LL | /// Addition is commutative, which means that `add(a, b)` is the same as add(b, a)`. | ^ @@ -595,7 +595,7 @@ LL | /// Addition is commutative, which means that `add(a, b)` is the same as ad | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:95:50 + --> $DIR/unescaped_backticks.rs:96:50 | LL | /// or even to add a number `n` to 42 (add(42, n)`)! | ^ @@ -610,7 +610,7 @@ LL | /// or even to add a number `n` to 42 (add(42, n)\`)! | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:99:74 + --> $DIR/unescaped_backticks.rs:100:74 | LL | /// Addition is commutative, which means that `add(a, b)` is the same as `add(b, a). | ^ @@ -625,7 +625,7 @@ LL | /// Addition is commutative, which means that `add(a, b)` is the same as \` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:103:51 + --> $DIR/unescaped_backticks.rs:104:51 | LL | /// or even to add a number `n` to 42 (`add(42, n)`)! | ^ @@ -640,7 +640,7 @@ LL | /// or even to add a number `n` to 42 (`add(42, n)\`)! | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:107:1 + --> $DIR/unescaped_backticks.rs:108:1 | LL | #[doc = "`"] | ^^^^^^^^^^^^ @@ -651,7 +651,7 @@ LL | #[doc = "`"] to this: \` error: unescaped backtick - --> $DIR/unescaped_backticks.rs:114:1 + --> $DIR/unescaped_backticks.rs:115:1 | LL | #[doc = concat!("\\", "`")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -664,7 +664,7 @@ LL | #[doc = concat!("\\", "`")] to this: \\` error: unescaped backtick - --> $DIR/unescaped_backticks.rs:118:1 + --> $DIR/unescaped_backticks.rs:119:1 | LL | #[doc = "Addition is commutative, which means that add(a, b)` is the same as `add(b, a)`."] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -677,7 +677,7 @@ LL | #[doc = "Addition is commutative, which means that add(a, b)` is the same a to this: Addition is commutative, which means that add(a, b)` is the same as `add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:122:1 + --> $DIR/unescaped_backticks.rs:123:1 | LL | #[doc = "Addition is commutative, which means that `add(a, b) is the same as `add(b, a)`."] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -690,7 +690,7 @@ LL | #[doc = "Addition is commutative, which means that `add(a, b) is the same a to this: Addition is commutative, which means that `add(a, b) is the same as `add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:126:1 + --> $DIR/unescaped_backticks.rs:127:1 | LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same as add(b, a)`."] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -703,7 +703,7 @@ LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same to this: Addition is commutative, which means that `add(a, b)` is the same as add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:130:1 + --> $DIR/unescaped_backticks.rs:131:1 | LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same as `add(b, a)."] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -716,7 +716,7 @@ LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same to this: Addition is commutative, which means that `add(a, b)` is the same as \`add(b, a). error: unescaped backtick - --> $DIR/unescaped_backticks.rs:135:5 + --> $DIR/unescaped_backticks.rs:136:5 | LL | /// `foo | ^ @@ -731,7 +731,7 @@ LL | /// \`foo | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:139:7 + --> $DIR/unescaped_backticks.rs:140:7 | LL | /// # `(heading | ^ @@ -746,7 +746,7 @@ LL | /// # \`(heading | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:141:17 + --> $DIR/unescaped_backticks.rs:142:17 | LL | /// ## heading2)` | ^ @@ -761,7 +761,7 @@ LL | /// ## heading2)\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:144:11 + --> $DIR/unescaped_backticks.rs:145:11 | LL | /// multi `( | ^ @@ -776,7 +776,7 @@ LL | /// multi \`( | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:150:10 + --> $DIR/unescaped_backticks.rs:151:10 | LL | /// para)`(graph | ^ @@ -795,7 +795,7 @@ LL | /// para)\`(graph | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:153:10 + --> $DIR/unescaped_backticks.rs:154:10 | LL | /// para)`(graph2 | ^ @@ -814,7 +814,7 @@ LL | /// para)\`(graph2 | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:156:12 + --> $DIR/unescaped_backticks.rs:157:12 | LL | /// 1. foo)` | ^ @@ -829,7 +829,7 @@ LL | /// 1. foo)\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:158:8 + --> $DIR/unescaped_backticks.rs:159:8 | LL | /// 2. `(bar | ^ @@ -844,7 +844,7 @@ LL | /// 2. \`(bar | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:160:11 + --> $DIR/unescaped_backticks.rs:161:11 | LL | /// * baz)` | ^ @@ -859,7 +859,7 @@ LL | /// * baz)\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:162:7 + --> $DIR/unescaped_backticks.rs:163:7 | LL | /// * `(quux | ^ @@ -874,7 +874,7 @@ LL | /// * \`(quux | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:165:5 + --> $DIR/unescaped_backticks.rs:166:5 | LL | /// `#![this_is_actually_an_image(and(not), an = "attribute")] | ^ @@ -889,7 +889,7 @@ LL | /// \`#![this_is_actually_an_image(and(not), an = "attribute")] | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:168:62 + --> $DIR/unescaped_backticks.rs:169:62 | LL | /// #![this_is_actually_an_image(and(not), an = "attribute")]` | ^ @@ -904,7 +904,7 @@ LL | /// #![this_is_actually_an_image(and(not), an = "attribute")]\` | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:173:7 + --> $DIR/unescaped_backticks.rs:174:7 | LL | /// | `table( | )head` | | ^ @@ -919,7 +919,7 @@ LL | /// | \`table( | )head` | | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:173:22 + --> $DIR/unescaped_backticks.rs:174:22 | LL | /// | `table( | )head` | | ^ @@ -934,7 +934,7 @@ LL | /// | `table( | )head\` | | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:177:12 + --> $DIR/unescaped_backticks.rs:178:12 | LL | /// | table`( | )`body | | ^ @@ -949,7 +949,7 @@ LL | /// | table\`( | )`body | | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:177:18 + --> $DIR/unescaped_backticks.rs:178:18 | LL | /// | table`( | )`body | | ^ From 1476b39faed28f3c5121611c16617b0080cde53a Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 21 Jul 2023 01:21:58 +0800 Subject: [PATCH 21/26] Skip lint check when item is not fully public --- .../passes/lint/redundant_explicit_links.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 94d07ec034d71..343cc3163fb18 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -13,6 +13,7 @@ use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; use crate::passes::source_span_for_markdown_range; +use crate::visit_ast::inherits_doc_hidden; #[derive(Debug)] struct LinkData { @@ -39,6 +40,24 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { return; } + let Some(item_id) = item.def_id() else { + return; + }; + let Some(local_item_id) = item_id.as_local() else { + return; + }; + + let is_hidden = !cx.render_options.document_hidden + && (item.is_doc_hidden() || inherits_doc_hidden(cx.tcx, local_item_id, None)); + if is_hidden { + return; + } + let is_private = !cx.render_options.document_private + && !cx.cache.effective_visibilities.is_directly_public(cx.tcx, item_id); + if is_private { + return; + } + check_redundant_explicit_link(cx, item, hir_id, &doc); } From 25919b09a963e776b0c35aa965aa47a04828e194 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 21 Jul 2023 03:54:58 +0800 Subject: [PATCH 22/26] Add regression test for inline doc --- tests/rustdoc-ui/lints/inline-doc-link.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/rustdoc-ui/lints/inline-doc-link.rs diff --git a/tests/rustdoc-ui/lints/inline-doc-link.rs b/tests/rustdoc-ui/lints/inline-doc-link.rs new file mode 100644 index 0000000000000..596f89be3d6d8 --- /dev/null +++ b/tests/rustdoc-ui/lints/inline-doc-link.rs @@ -0,0 +1,13 @@ +// Regression test for + +// check-pass +#![deny(rustdoc::redundant_explicit_links)] + +mod m { + pub enum ValueEnum {} +} +mod m2 { + /// [`ValueEnum`] + pub use crate::m::ValueEnum; +} +pub use m2::ValueEnum; From 23c9a4a1cab1bb0442fe77aae305eb2864958427 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 21 Jul 2023 16:06:34 +0800 Subject: [PATCH 23/26] resolve conflicts --- library/alloc/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 41aac02eaa97a..ffe6d6373875c 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -89,6 +89,7 @@ #![allow(explicit_outlives_requirements)] #![warn(multiple_supertrait_upcastable)] #![cfg_attr(not(bootstrap), allow(internal_features))] +#![cfg_attr(not(bootstrap), allow(rustdoc::redundant_explicit_links))] // // Library features: // tidy-alphabetical-start From 8e34c68c636b618f98bd2e11b2cf04b04b964a3c Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 18 Aug 2023 15:47:51 +0800 Subject: [PATCH 24/26] Fix private function importing --- src/librustdoc/passes/lint/redundant_explicit_links.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 343cc3163fb18..d71384045e719 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -13,7 +13,7 @@ use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; use crate::passes::source_span_for_markdown_range; -use crate::visit_ast::inherits_doc_hidden; +use crate::clean::utils::inherits_doc_hidden; #[derive(Debug)] struct LinkData { From e17d2da2fcdd915a631d3ff25b4462397453e0f1 Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Fri, 18 Aug 2023 15:56:40 +0800 Subject: [PATCH 25/26] Fix format --- src/librustdoc/passes/lint/redundant_explicit_links.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index d71384045e719..ef0f8716aa887 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -9,11 +9,11 @@ use rustc_lint_defs::Applicability; use rustc_span::Symbol; use crate::clean::utils::find_nearest_parent_module; +use crate::clean::utils::inherits_doc_hidden; use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; use crate::passes::source_span_for_markdown_range; -use crate::clean::utils::inherits_doc_hidden; #[derive(Debug)] struct LinkData { From 297ff8c97ef7b34e620e8f016361227f0be79a7c Mon Sep 17 00:00:00 2001 From: Kyle Lin Date: Sat, 19 Aug 2023 02:22:13 +0800 Subject: [PATCH 26/26] Fix redundant explicit link in rustc_borrowck --- compiler/rustc_borrowck/src/consumers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/consumers.rs b/compiler/rustc_borrowck/src/consumers.rs index d257145373f72..becfa535a59cc 100644 --- a/compiler/rustc_borrowck/src/consumers.rs +++ b/compiler/rustc_borrowck/src/consumers.rs @@ -30,7 +30,7 @@ pub use super::{ /// will be retrieved. #[derive(Debug, Copy, Clone)] pub enum ConsumerOptions { - /// Retrieve the [`Body`] along with the [`BorrowSet`](super::borrow_set::BorrowSet) + /// Retrieve the [`Body`] along with the [`BorrowSet`] /// and [`RegionInferenceContext`]. If you would like the body only, use /// [`TyCtxt::mir_promoted`]. ///