From fbbda9e0c3f6b673a739d704e9bba1cf96e1423d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 27 May 2025 15:16:51 +0200 Subject: [PATCH 1/4] Do not emit `redundant_explicit_links` rustdoc lint if the doc comment comes from expansion --- compiler/rustc_resolve/src/rustdoc.rs | 82 +++++++++++-------- .../passes/collect_intra_doc_links.rs | 13 +-- src/librustdoc/passes/lint/bare_urls.rs | 3 +- .../passes/lint/check_code_block_syntax.rs | 2 +- src/librustdoc/passes/lint/html_tags.rs | 4 +- .../passes/lint/redundant_explicit_links.rs | 40 +++++++-- .../passes/lint/unescaped_backticks.rs | 10 ++- 7 files changed, 100 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index fa839d2748d86..d73eb4b1c135e 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -49,6 +49,9 @@ pub struct DocFragment { pub doc: Symbol, pub kind: DocFragmentKind, pub indent: usize, + /// Because we temper with the spans context, this information cannot be correctly retrieved + /// later on. So instead, we compute it and store it here. + pub from_expansion: bool, } #[derive(Clone, Copy, Debug)] @@ -208,17 +211,18 @@ pub fn attrs_to_doc_fragments<'a, A: AttributeExt + Clone + 'a>( for (attr, item_id) in attrs { if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() { let doc = beautify_doc_string(doc_str, comment_kind); - let (span, kind) = if attr.is_doc_comment() { - (attr.span(), DocFragmentKind::SugaredDoc) + let (span, kind, from_expansion) = if attr.is_doc_comment() { + let span = attr.span(); + (span, DocFragmentKind::SugaredDoc, span.from_expansion()) } else { - ( - attr.value_span() - .map(|i| i.with_ctxt(attr.span().ctxt())) - .unwrap_or(attr.span()), - DocFragmentKind::RawDoc, - ) + let attr_span = attr.span(); + let (span, from_expansion) = match attr.value_span() { + Some(sp) => (sp.with_ctxt(attr_span.ctxt()), sp.from_expansion()), + None => (attr_span, attr_span.from_expansion()), + }; + (span, DocFragmentKind::RawDoc, from_expansion) }; - let fragment = DocFragment { span, doc, kind, item_id, indent: 0 }; + let fragment = DocFragment { span, doc, kind, item_id, indent: 0, from_expansion }; doc_fragments.push(fragment); } else if !doc_only { other_attrs.push(attr.clone()); @@ -501,16 +505,21 @@ fn collect_link_data<'input, F: BrokenLinkCallback<'input>>( } /// Returns a span encompassing all the document fragments. -pub fn span_of_fragments(fragments: &[DocFragment]) -> Option { - if fragments.is_empty() { - return None; - } - let start = fragments[0].span; - if start == DUMMY_SP { +pub fn span_of_fragments_with_expansion(fragments: &[DocFragment]) -> Option<(Span, bool)> { + let Some(first_fragment) = fragments.first() else { return None }; + if first_fragment.span == DUMMY_SP { return None; } - let end = fragments.last().expect("no doc strings provided").span; - Some(start.to(end)) + let last_fragment = fragments.last().expect("no doc strings provided"); + Some(( + first_fragment.span.to(last_fragment.span), + first_fragment.from_expansion || last_fragment.from_expansion, + )) +} + +/// Returns a span encompassing all the document fragments. +pub fn span_of_fragments(fragments: &[DocFragment]) -> Option { + span_of_fragments_with_expansion(fragments).map(|(sp, _)| sp) } /// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code. @@ -535,7 +544,7 @@ pub fn source_span_for_markdown_range( markdown: &str, md_range: &Range, fragments: &[DocFragment], -) -> Option { +) -> Option<(Span, bool)> { let map = tcx.sess.source_map(); source_span_for_markdown_range_inner(map, markdown, md_range, fragments) } @@ -546,7 +555,7 @@ pub fn source_span_for_markdown_range_inner( markdown: &str, md_range: &Range, fragments: &[DocFragment], -) -> Option { +) -> Option<(Span, bool)> { use rustc_span::BytePos; if let &[fragment] = &fragments @@ -557,11 +566,14 @@ pub fn source_span_for_markdown_range_inner( && let Ok(md_range_hi) = u32::try_from(md_range.end) { // Single fragment with string that contains same bytes as doc. - return Some(Span::new( - fragment.span.lo() + rustc_span::BytePos(md_range_lo), - fragment.span.lo() + rustc_span::BytePos(md_range_hi), - fragment.span.ctxt(), - fragment.span.parent(), + return Some(( + Span::new( + fragment.span.lo() + rustc_span::BytePos(md_range_lo), + fragment.span.lo() + rustc_span::BytePos(md_range_hi), + fragment.span.ctxt(), + fragment.span.parent(), + ), + fragment.from_expansion, )); } @@ -593,19 +605,21 @@ pub fn source_span_for_markdown_range_inner( { match_data = Some((i, match_start)); } else { - // Heirustic produced ambiguity, return nothing. + // Heuristic produced ambiguity, return nothing. return None; } } } if let Some((i, match_start)) = match_data { - let sp = fragments[i].span; + let fragment = &fragments[i]; + let sp = fragment.span; // we need to calculate the span start, // then use that in our calulations for the span end let lo = sp.lo() + BytePos(match_start as u32); - return Some( + return Some(( sp.with_lo(lo).with_hi(lo + BytePos((md_range.end - md_range.start) as u32)), - ); + fragment.from_expansion, + )); } return None; } @@ -659,8 +673,12 @@ pub fn source_span_for_markdown_range_inner( } } - Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new( - md_range.start + start_bytes, - md_range.end + start_bytes + end_bytes, - ))) + let (span, from_expansion) = span_of_fragments_with_expansion(fragments)?; + Some(( + span.from_inner(InnerSpan::new( + md_range.start + start_bytes, + md_range.end + start_bytes + end_bytes, + )), + from_expansion, + )) } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1daaba3b86c5c..ca6f67eb6dfd6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1387,13 +1387,15 @@ impl LinkCollector<'_, '_> { ori_link: &MarkdownLinkRange, item: &Item, ) { - let span = source_span_for_markdown_range( + let span = match source_span_for_markdown_range( self.cx.tcx, dox, ori_link.inner_range(), &item.attrs.doc_strings, - ) - .unwrap_or_else(|| item.attr_span(self.cx.tcx)); + ) { + Some((sp, _)) => sp, + None => item.attr_span(self.cx.tcx), + }; rustc_session::parse::feature_err( self.cx.tcx.sess, sym::intra_doc_pointers, @@ -1836,7 +1838,7 @@ fn report_diagnostic( let mut md_range = md_range.clone(); let sp = source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings) - .map(|mut sp| { + .map(|(mut sp, _)| { while dox.as_bytes().get(md_range.start) == Some(&b' ') || dox.as_bytes().get(md_range.start) == Some(&b'`') { @@ -1854,7 +1856,8 @@ fn report_diagnostic( (sp, MarkdownLinkRange::Destination(md_range)) } MarkdownLinkRange::WholeLink(md_range) => ( - source_span_for_markdown_range(tcx, dox, md_range, &item.attrs.doc_strings), + source_span_for_markdown_range(tcx, dox, md_range, &item.attrs.doc_strings) + .map(|(sp, _)| sp), link_range.clone(), ), }; diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs index 3b3ce3e92202a..f70bdf4e4fe37 100644 --- a/src/librustdoc/passes/lint/bare_urls.rs +++ b/src/librustdoc/passes/lint/bare_urls.rs @@ -18,7 +18,8 @@ use crate::html::markdown::main_body_opts; pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) { let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range| { - let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings); + let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings) + .map(|(sp, _)| sp); let sp = maybe_sp.unwrap_or_else(|| item.attr_span(cx.tcx)); cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| { lint.primary_message(msg) diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index 9662dd85d678b..de1df8c2ff0e2 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -90,7 +90,7 @@ fn check_rust_syntax( &code_block.range, &item.attrs.doc_strings, ) { - Some(sp) => (sp, true), + Some((sp, _)) => (sp, true), None => (item.attr_span(cx.tcx), false), }; diff --git a/src/librustdoc/passes/lint/html_tags.rs b/src/librustdoc/passes/lint/html_tags.rs index b9739726c9569..19cf15d40a3b4 100644 --- a/src/librustdoc/passes/lint/html_tags.rs +++ b/src/librustdoc/passes/lint/html_tags.rs @@ -16,7 +16,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: & let tcx = cx.tcx; let report_diag = |msg: String, range: &Range, is_open_tag: bool| { let sp = match source_span_for_markdown_range(tcx, dox, range, &item.attrs.doc_strings) { - Some(sp) => sp, + Some((sp, _)) => sp, None => item.attr_span(tcx), }; tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| { @@ -55,7 +55,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: & &(generics_start..generics_end), &item.attrs.doc_strings, ) { - Some(sp) => sp, + Some((sp, _)) => sp, None => item.attr_span(tcx), }; // Sometimes, we only extract part of a path. For example, consider this: diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 6bc4374c06b1d..0482bc1058f0e 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -161,15 +161,26 @@ fn check_inline_or_reference_unknown_redundancy( if dest_res == display_res { let link_span = - source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) - .unwrap_or(item.attr_span(cx.tcx)); - let explicit_span = source_span_for_markdown_range( + match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) + { + Some((sp, from_expansion)) => { + if from_expansion { + return None; + } + sp + } + None => item.attr_span(cx.tcx), + }; + let (explicit_span, from_expansion) = source_span_for_markdown_range( cx.tcx, doc, &offset_explicit_range(doc, link_range, open, close), &item.attrs.doc_strings, )?; - let display_span = source_span_for_markdown_range( + if from_expansion { + return None; + } + let (display_span, _) = source_span_for_markdown_range( cx.tcx, doc, resolvable_link_range, @@ -206,21 +217,32 @@ fn check_reference_redundancy( if dest_res == display_res { let link_span = - source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) - .unwrap_or(item.attr_span(cx.tcx)); - let explicit_span = source_span_for_markdown_range( + match source_span_for_markdown_range(cx.tcx, doc, &link_range, &item.attrs.doc_strings) + { + Some((sp, from_expansion)) => { + if from_expansion { + return None; + } + sp + } + None => item.attr_span(cx.tcx), + }; + let (explicit_span, from_expansion) = source_span_for_markdown_range( cx.tcx, doc, &offset_explicit_range(doc, link_range.clone(), b'[', b']'), &item.attrs.doc_strings, )?; - let display_span = source_span_for_markdown_range( + if from_expansion { + return None; + } + let (display_span, _) = source_span_for_markdown_range( cx.tcx, doc, resolvable_link_range, &item.attrs.doc_strings, )?; - let def_span = source_span_for_markdown_range( + let (def_span, _) = source_span_for_markdown_range( cx.tcx, doc, &offset_reference_def_range(doc, dest, link_range), diff --git a/src/librustdoc/passes/lint/unescaped_backticks.rs b/src/librustdoc/passes/lint/unescaped_backticks.rs index 88f4c3ac1cd79..7f5643f4ba814 100644 --- a/src/librustdoc/passes/lint/unescaped_backticks.rs +++ b/src/librustdoc/passes/lint/unescaped_backticks.rs @@ -42,13 +42,15 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: & // If we can't get a span of the backtick, because it is in a `#[doc = ""]` attribute, // use the span of the entire attribute as a fallback. - let span = source_span_for_markdown_range( + let span = match source_span_for_markdown_range( tcx, dox, &(backtick_index..backtick_index + 1), &item.attrs.doc_strings, - ) - .unwrap_or_else(|| item.attr_span(tcx)); + ) { + Some((sp, _)) => sp, + None => item.attr_span(tcx), + }; tcx.node_span_lint(crate::lint::UNESCAPED_BACKTICKS, hir_id, span, |lint| { lint.primary_message("unescaped backtick"); @@ -419,7 +421,7 @@ fn suggest_insertion( /// Maximum bytes of context to show around the insertion. const CONTEXT_MAX_LEN: usize = 80; - if let Some(span) = source_span_for_markdown_range( + if let Some((span, _)) = source_span_for_markdown_range( cx.tcx, dox, &(insert_index..insert_index), From 2723a5d14ae638a72b01d327cf248d41a3d02a97 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 27 May 2025 15:17:22 +0200 Subject: [PATCH 2/4] Add ui test for `redundant_explicit_links` rustdoc lint for items coming from expansion --- .../redundant_explicit_links-expansion.rs | 28 +++++++++++++++++++ .../redundant_explicit_links-expansion.stderr | 23 +++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 tests/rustdoc-ui/lints/redundant_explicit_links-expansion.rs create mode 100644 tests/rustdoc-ui/lints/redundant_explicit_links-expansion.stderr diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.rs b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.rs new file mode 100644 index 0000000000000..8cb75ba4e18ad --- /dev/null +++ b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.rs @@ -0,0 +1,28 @@ +// This is a regression test for . +// If the link is generated from expansion, we should not emit the lint. + +#![deny(rustdoc::redundant_explicit_links)] + +macro_rules! mac1 { + () => { + "provided by a [`BufferProvider`](crate::BufferProvider)." + }; +} + +macro_rules! mac2 { + () => { + #[doc = mac1!()] + pub struct BufferProvider; + } +} + +// Should not lint. +#[doc = mac1!()] +pub struct Foo; + +// Should not lint. +mac2!{} + +#[doc = "provided by a [`BufferProvider`](crate::BufferProvider)."] +//~^ ERROR: redundant_explicit_links +pub struct Bla; diff --git a/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.stderr b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.stderr new file mode 100644 index 0000000000000..0a38ec2aa6a42 --- /dev/null +++ b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.stderr @@ -0,0 +1,23 @@ +error: redundant explicit link target + --> $DIR/redundant_explicit_links-expansion.rs:26:43 + | +LL | #[doc = "provided by a [`BufferProvider`](crate::BufferProvider)."] + | ---------------- ^^^^^^^^^^^^^^^^^^^^^ 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 +note: the lint level is defined here + --> $DIR/redundant_explicit_links-expansion.rs:4:9 + | +LL | #![deny(rustdoc::redundant_explicit_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: remove explicit link target + | +LL - #[doc = "provided by a [`BufferProvider`](crate::BufferProvider)."] +LL + #[doc = "provided by a [`BufferProvider`]."] + | + +error: aborting due to 1 previous error + From 9ac0d61e2aa83a84882cfa7478677eac91878028 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 27 May 2025 16:16:18 +0200 Subject: [PATCH 3/4] Update clippy source code to changes on `source_span_for_markdown_range` --- src/tools/clippy/clippy_lints/src/doc/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/doc/mod.rs b/src/tools/clippy/clippy_lints/src/doc/mod.rs index c46dd09d60c56..ae3fcccadba68 100644 --- a/src/tools/clippy/clippy_lints/src/doc/mod.rs +++ b/src/tools/clippy/clippy_lints/src/doc/mod.rs @@ -732,8 +732,8 @@ impl Fragments<'_> { /// get the span for the markdown range. Note that this function is not cheap, use it with /// caution. #[must_use] - fn span(&self, cx: &LateContext<'_>, range: Range) -> Option { - source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments) + fn span(self, cx: &LateContext<'_>, range: Range) -> Option { + source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments).map(|(sp, _)| sp) } } From 0ada37b0aca7adf7f2a8bede20b6081e0f6bb878 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 3 Jun 2025 23:00:16 +0200 Subject: [PATCH 4/4] Update tests to work with new DocFragment field and `redundant_explicit_links` new API --- compiler/rustc_resolve/src/rustdoc/tests.rs | 6 ++++-- src/librustdoc/clean/types/tests.rs | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_resolve/src/rustdoc/tests.rs b/compiler/rustc_resolve/src/rustdoc/tests.rs index 221ac907e7c9c..6a98ae0663048 100644 --- a/compiler/rustc_resolve/src/rustdoc/tests.rs +++ b/compiler/rustc_resolve/src/rustdoc/tests.rs @@ -10,7 +10,7 @@ use super::{DocFragment, DocFragmentKind, source_span_for_markdown_range_inner}; fn single_backtick() { let sm = SourceMap::new(FilePathMapping::empty()); sm.new_source_file(PathBuf::from("foo.rs").into(), r#"#[doc = "`"] fn foo() {}"#.to_string()); - let span = source_span_for_markdown_range_inner( + let (span, _) = source_span_for_markdown_range_inner( &sm, "`", &(0..1), @@ -20,6 +20,7 @@ fn single_backtick() { kind: DocFragmentKind::RawDoc, doc: sym::empty, // unused placeholder indent: 0, + from_expansion: false, }], ) .unwrap(); @@ -32,7 +33,7 @@ fn utf8() { // regression test for https://github.com/rust-lang/rust/issues/141665 let sm = SourceMap::new(FilePathMapping::empty()); sm.new_source_file(PathBuf::from("foo.rs").into(), r#"#[doc = "⚠"] fn foo() {}"#.to_string()); - let span = source_span_for_markdown_range_inner( + let (span, _) = source_span_for_markdown_range_inner( &sm, "⚠", &(0..3), @@ -42,6 +43,7 @@ fn utf8() { kind: DocFragmentKind::RawDoc, doc: sym::empty, // unused placeholder indent: 0, + from_expansion: false, }], ) .unwrap(); diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 7ff5026150b16..9499507b2c0f9 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -10,6 +10,7 @@ fn create_doc_fragment(s: &str) -> Vec { doc: Symbol::intern(s), kind: DocFragmentKind::SugaredDoc, indent: 0, + from_expansion: false, }] }