Rollup merge of #141648 - GuillaumeGomez:redundant_explicit_links-expansion, r=lolbinarycat

[rustdoc] Do not emit redundant_explicit_links lint if the doc comment comes from expansion

Fixes https://github.com/rust-lang/rust/issues/141553.

The problem was that we change the context for the attributes in some cases to get better error output, preventing us to detect if the attribute comes from expansion. Most of the changes are about keeping track of the "does this span comes from expansion" information.

r? ```@Manishearth```

12 files changed, 212 insertions, 65 deletions

diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs
index 931c6241bf2..3fe5db8ca54 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 tamper 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());
@@ -505,17 +509,26 @@ fn collect_link_data<'input, F: BrokenLinkCallback<'input>>(
     display_text.map(String::into_boxed_str)
 }
 
-/// Returns a span encompassing all the document fragments.
-pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
-    if fragments.is_empty() {
-        return None;
-    }
-    let start = fragments[0].span;
-    if start == DUMMY_SP {
+/// Returns a tuple containing a span encompassing all the document fragments and a boolean that is
+/// `true` if any of the fragments are from a macro expansion.
+pub fn span_of_fragments_with_expansion(fragments: &[DocFragment]) -> Option<(Span, bool)> {
+    let (first_fragment, last_fragment) = match fragments {
+        [] => return None,
+        [first, .., last] => (first, last),
+        [first] => (first, first),
+    };
+    if first_fragment.span == DUMMY_SP {
         return None;
     }
-    let end = fragments.last().expect("no doc strings provided").span;
-    Some(start.to(end))
+    Some((
+        first_fragment.span.to(last_fragment.span),
+        fragments.iter().any(|frag| frag.from_expansion),
+    ))
+}
+
+/// Returns a span encompassing all the document fragments.
+pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
+    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.
@@ -529,18 +542,22 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
 /// This method will return `Some` only if one of the following is true:
 ///
 /// - The doc is made entirely from sugared doc comments, which cannot contain escapes
-/// - The doc is entirely from a single doc fragment with a string literal exactly equal to `markdown`.
+/// - The doc is entirely from a single doc fragment with a string literal exactly equal to
+///   `markdown`.
 /// - The doc comes from `include_str!`
-/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a single doc fragment.
+/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a
+///   single doc fragment.
+///
+/// This function is defined in the compiler so it can be used by both `rustdoc` and `clippy`.
 ///
-/// This function is defined in the compiler so it can be used by
-/// both `rustdoc` and `clippy`.
+/// It returns a tuple containing a span encompassing all the document fragments and a boolean that
+/// is `true` if any of the *matched* fragments are from a macro expansion.
 pub fn source_span_for_markdown_range(
     tcx: TyCtxt<'_>,
     markdown: &str,
     md_range: &Range<usize>,
     fragments: &[DocFragment],
-) -> Option<Span> {
+) -> Option<(Span, bool)> {
     let map = tcx.sess.source_map();
     source_span_for_markdown_range_inner(map, markdown, md_range, fragments)
 }
@@ -551,7 +568,7 @@ pub fn source_span_for_markdown_range_inner(
     markdown: &str,
     md_range: &Range<usize>,
     fragments: &[DocFragment],
-) -> Option<Span> {
+) -> Option<(Span, bool)> {
     use rustc_span::BytePos;
 
     if let &[fragment] = &fragments
@@ -562,11 +579,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,
         ));
     }
 
@@ -598,19 +618,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;
     }
@@ -664,8 +686,13 @@ pub fn source_span_for_markdown_range_inner(
         }
     }
 
-    Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new(
+    let (span, _) = span_of_fragments_with_expansion(fragments)?;
+    let src_span = span.from_inner(InnerSpan::new(
         md_range.start + start_bytes,
         md_range.end + start_bytes + end_bytes,
-    )))
+    ));
+    Some((
+        src_span,
+        fragments.iter().any(|frag| frag.span.overlaps(src_span) && frag.from_expansion),
+    ))
 }
diff --git a/compiler/rustc_resolve/src/rustdoc/tests.rs b/compiler/rustc_resolve/src/rustdoc/tests.rs
index 221ac907e7c..6a98ae06630 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 7ff5026150b..9499507b2c0 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<DocFragment> {
         doc: Symbol::intern(s),
         kind: DocFragmentKind::SugaredDoc,
         indent: 0,
+        from_expansion: false,
     }]
 }
 
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 1daaba3b86c..ca6f67eb6df 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 3b3ce3e9220..f70bdf4e4fe 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<usize>| {
-        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 91cddbe5a5b..b08533317ab 100644
--- a/src/librustdoc/passes/lint/check_code_block_syntax.rs
+++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs
@@ -87,7 +87,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 b9739726c95..19cf15d40a3 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<usize>, 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 6bc4374c06b..5757b6a9740 100644
--- a/src/librustdoc/passes/lint/redundant_explicit_links.rs
+++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs
@@ -161,20 +161,36 @@ 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, false) = 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(
+        )?
+        else {
+            // This `span` comes from macro expansion so skipping it.
+            return None;
+        };
+        let (display_span, false) = source_span_for_markdown_range(
             cx.tcx,
             doc,
             resolvable_link_range,
             &item.attrs.doc_strings,
-        )?;
+        )?
+        else {
+            // This `span` comes from macro expansion so skipping it.
+            return None;
+        };
 
         cx.tcx.node_span_lint(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, |lint| {
             lint.primary_message("redundant explicit link target")
@@ -206,21 +222,37 @@ 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, false) = 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(
+        )?
+        else {
+            // This `span` comes from macro expansion so skipping it.
+            return None;
+        };
+        let (display_span, false) = source_span_for_markdown_range(
             cx.tcx,
             doc,
             resolvable_link_range,
             &item.attrs.doc_strings,
-        )?;
-        let def_span = source_span_for_markdown_range(
+        )?
+        else {
+            // This `span` comes from macro expansion so skipping it.
+            return None;
+        };
+        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 88f4c3ac1cd..7f5643f4ba8 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),
diff --git a/src/tools/clippy/clippy_lints/src/doc/mod.rs b/src/tools/clippy/clippy_lints/src/doc/mod.rs
index e0fc2fd9347..d38588bb799 100644
--- a/src/tools/clippy/clippy_lints/src/doc/mod.rs
+++ b/src/tools/clippy/clippy_lints/src/doc/mod.rs
@@ -765,8 +765,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<usize>) -> Option<Span> {
-        source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments)
+    fn span(self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> {
+        source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments).map(|(sp, _)| sp)
     }
 }
 
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 00000000000..2e42a0a5c5d
--- /dev/null
+++ b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.rs
@@ -0,0 +1,40 @@
+// This is a regression test for <https://github.com/rust-lang/rust/issues/141553>.
+// 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;
+    }
+}
+
+macro_rules! mac3 {
+    () => {
+        "Provided by"
+    };
+}
+
+// Should not lint.
+#[doc = mac1!()]
+pub struct Foo;
+
+// Should not lint.
+mac2!{}
+
+#[doc = "provided by a [`BufferProvider`](crate::BufferProvider)."]
+/// bla
+//~^^ ERROR: redundant_explicit_links
+pub struct Bla;
+
+#[doc = mac3!()]
+/// a [`BufferProvider`](crate::BufferProvider).
+//~^ ERROR: redundant_explicit_links
+pub fn f() {}
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 00000000000..a81931fb073
--- /dev/null
+++ b/tests/rustdoc-ui/lints/redundant_explicit_links-expansion.stderr
@@ -0,0 +1,39 @@
+error: redundant explicit link target
+  --> $DIR/redundant_explicit_links-expansion.rs:32: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: redundant explicit link target
+  --> $DIR/redundant_explicit_links-expansion.rs:38:26
+   |
+LL | /// 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
+help: remove explicit link target
+   |
+LL - /// a [`BufferProvider`](crate::BufferProvider).
+LL + /// a [`BufferProvider`].
+   |
+
+error: aborting due to 2 previous errors
+