Auto merge of rust-lang#115689 - Alexendoo:clippy-doc-comments, r=notriddle,Manishearth,flip1995

Reuse rustdoc's doc comment handling in Clippy

Moves `source_span_for_markdown_range` and `span_of_attrs` (renamed to `span_of_fragments`) to `rustc_resolve::rustdoc` so it can be used in Clippy

Fixes rust-lang/rust-clippy#10277
Fixes rust-lang/rust-clippy#5593
Fixes rust-lang/rust-clippy#10263
Fixes rust-lang/rust-clippy#2581

Author: bors

Cargo.lock

@@ -558,7 +558,6 @@ dependencies = [
  "declare_clippy_lint",
  "if_chain",
  "itertools",
- "pulldown-cmark",
  "quine-mc_cluskey",
  "regex",
  "regex-syntax 0.7.2",

compiler/rustc_resolve/src/rustdoc.rs

@@ -2,9 +2,11 @@ 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;
+use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
 use rustc_span::symbol::{kw, sym, Symbol};
-use rustc_span::Span;
+use rustc_span::{InnerSpan, Span, DUMMY_SP};
+use std::ops::Range;
 use std::{cmp, mem};
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -462,3 +464,88 @@ fn collect_link_data<'input, 'callback>(
 
     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 {
+        return None;
+    }
+    let end = fragments.last().expect("no doc strings provided").span;
+    Some(start.to(end))
+}
+
+/// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code.
+///
+/// This method will return `None` if we cannot construct a span from the source map or if the
+/// fragments are not all sugared doc comments. It's difficult to calculate the correct span in
+/// that case due to escaping and other source features.
+pub fn source_span_for_markdown_range(
+    tcx: TyCtxt<'_>,
+    markdown: &str,
+    md_range: &Range<usize>,
+    fragments: &[DocFragment],
+) -> Option<Span> {
+    let is_all_sugared_doc = fragments.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc);
+
+    if !is_all_sugared_doc {
+        return None;
+    }
+
+    let snippet = tcx.sess.source_map().span_to_snippet(span_of_fragments(fragments)?).ok()?;
+
+    let starting_line = markdown[..md_range.start].matches('\n').count();
+    let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count();
+
+    // We use `split_terminator('\n')` instead of `lines()` when counting bytes so that we treat
+    // CRLF and LF line endings the same way.
+    let mut src_lines = snippet.split_terminator('\n');
+    let md_lines = markdown.split_terminator('\n');
+
+    // The number of bytes from the source span to the markdown span that are not part
+    // of the markdown, like comment markers.
+    let mut start_bytes = 0;
+    let mut end_bytes = 0;
+
+    'outer: for (line_no, md_line) in md_lines.enumerate() {
+        loop {
+            let source_line = src_lines.next()?;
+            match source_line.find(md_line) {
+                Some(offset) => {
+                    if line_no == starting_line {
+                        start_bytes += offset;
+
+                        if starting_line == ending_line {
+                            break 'outer;
+                        }
+                    } else if line_no == ending_line {
+                        end_bytes += offset;
+                        break 'outer;
+                    } else if line_no < starting_line {
+                        start_bytes += source_line.len() - md_line.len();
+                    } else {
+                        end_bytes += source_line.len() - md_line.len();
+                    }
+                    break;
+                }
+                None => {
+                    // Since this is a source line that doesn't include a markdown line,
+                    // we have to count the newline that we split from earlier.
+                    if line_no <= starting_line {
+                        start_bytes += source_line.len() + 1;
+                    } else {
+                        end_bytes += source_line.len() + 1;
+                    }
+                }
+            }
+        }
+    }
+
+    Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new(
+        md_range.start + start_bytes,
+        md_range.end + start_bytes + end_bytes,
+    )))
+}

src/librustdoc/clean/types.rs

@@ -25,7 +25,9 @@ use rustc_index::IndexVec;
 use rustc_metadata::rendered_const;
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::{self, TyCtxt, Visibility};
-use rustc_resolve::rustdoc::{add_doc_fragment, attrs_to_doc_fragments, inner_docs, DocFragment};
+use rustc_resolve::rustdoc::{
+    add_doc_fragment, attrs_to_doc_fragments, inner_docs, span_of_fragments, DocFragment,
+};
 use rustc_session::Session;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
@@ -397,7 +399,7 @@ impl Item {
     }
 
     pub(crate) fn attr_span(&self, tcx: TyCtxt<'_>) -> rustc_span::Span {
-        crate::passes::span_of_attrs(&self.attrs)
+        span_of_fragments(&self.attrs.doc_strings)
             .unwrap_or_else(|| self.span(tcx).map_or(rustc_span::DUMMY_SP, |span| span.inner()))
     }
 

src/librustdoc/doctest.rs

@@ -10,6 +10,7 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::TyCtxt;
 use rustc_parse::maybe_new_parser_from_source_str;
 use rustc_parse::parser::attr::InnerAttrPolicy;
+use rustc_resolve::rustdoc::span_of_fragments;
 use rustc_session::config::{self, CrateType, ErrorOutputType};
 use rustc_session::parse::ParseSess;
 use rustc_session::{lint, EarlyErrorHandler, Session};
@@ -33,7 +34,6 @@ use crate::clean::{types::AttributesExt, Attributes};
 use crate::config::Options as RustdocOptions;
 use crate::html::markdown::{self, ErrorCodes, Ignore, LangString};
 use crate::lint::init_lints;
-use crate::passes::span_of_attrs;
 
 /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`).
 #[derive(Clone, Default)]
@@ -1241,7 +1241,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
                 Some(&crate::html::markdown::ExtraInfo::new(
                     self.tcx,
                     def_id.to_def_id(),
-                    span_of_attrs(&attrs).unwrap_or(sp),
+                    span_of_fragments(&attrs.doc_strings).unwrap_or(sp),
                 )),
             );
         }

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -16,7 +16,9 @@ use rustc_hir::Mutability;
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_middle::{bug, span_bug, ty};
 use rustc_resolve::rustdoc::{has_primitive_or_keyword_docs, prepare_to_doc_link_resolution};
-use rustc_resolve::rustdoc::{strip_generics_from_path, MalformedGenerics};
+use rustc_resolve::rustdoc::{
+    source_span_for_markdown_range, strip_generics_from_path, MalformedGenerics,
+};
 use rustc_session::lint::Lint;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{sym, Ident, Symbol};
@@ -1211,11 +1213,11 @@ impl LinkCollector<'_, '_> {
         ori_link: &MarkdownLinkRange,
         item: &Item,
     ) {
-        let span = super::source_span_for_markdown_range(
+        let span = source_span_for_markdown_range(
             self.cx.tcx,
             dox,
             ori_link.inner_range(),
-            &item.attrs,
+            &item.attrs.doc_strings,
         )
         .unwrap_or_else(|| item.attr_span(self.cx.tcx));
         rustc_session::parse::feature_err(
@@ -1702,26 +1704,27 @@ fn report_diagnostic(
         let (span, link_range) = match link_range {
             MarkdownLinkRange::Destination(md_range) => {
                 let mut md_range = md_range.clone();
-                let sp = super::source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs)
-                    .map(|mut sp| {
-                        while dox.as_bytes().get(md_range.start) == Some(&b' ')
-                            || dox.as_bytes().get(md_range.start) == Some(&b'`')
-                        {
-                            md_range.start += 1;
-                            sp = sp.with_lo(sp.lo() + BytePos(1));
-                        }
-                        while dox.as_bytes().get(md_range.end - 1) == Some(&b' ')
-                            || dox.as_bytes().get(md_range.end - 1) == Some(&b'`')
-                        {
-                            md_range.end -= 1;
-                            sp = sp.with_hi(sp.hi() - BytePos(1));
-                        }
-                        sp
-                    });
+                let sp =
+                    source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings)
+                        .map(|mut sp| {
+                            while dox.as_bytes().get(md_range.start) == Some(&b' ')
+                                || dox.as_bytes().get(md_range.start) == Some(&b'`')
+                            {
+                                md_range.start += 1;
+                                sp = sp.with_lo(sp.lo() + BytePos(1));
+                            }
+                            while dox.as_bytes().get(md_range.end - 1) == Some(&b' ')
+                                || dox.as_bytes().get(md_range.end - 1) == Some(&b'`')
+                            {
+                                md_range.end -= 1;
+                                sp = sp.with_hi(sp.hi() - BytePos(1));
+                            }
+                            sp
+                        });
                 (sp, MarkdownLinkRange::Destination(md_range))
             }
             MarkdownLinkRange::WholeLink(md_range) => (
-                super::source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs),
+                source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings),
                 link_range.clone(),
             ),
         };

src/librustdoc/passes/lint/bare_urls.rs

@@ -4,11 +4,11 @@
 use crate::clean::*;
 use crate::core::DocContext;
 use crate::html::markdown::main_body_opts;
-use crate::passes::source_span_for_markdown_range;
 use core::ops::Range;
 use pulldown_cmark::{Event, Parser, Tag};
 use regex::Regex;
 use rustc_errors::Applicability;
+use rustc_resolve::rustdoc::source_span_for_markdown_range;
 use std::mem;
 use std::sync::LazyLock;
 
@@ -21,8 +21,9 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
     if !dox.is_empty() {
         let report_diag =
             |cx: &DocContext<'_>, msg: &'static str, url: &str, range: Range<usize>| {
-                let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)
-                    .unwrap_or_else(|| item.attr_span(cx.tcx));
+                let sp =
+                    source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
+                        .unwrap_or_else(|| item.attr_span(cx.tcx));
                 cx.tcx.struct_span_lint_hir(crate::lint::BARE_URLS, hir_id, sp, msg, |lint| {
                     lint.note("bare URLs are not automatically turned into clickable links")
                         .span_suggestion(

src/librustdoc/passes/lint/check_code_block_syntax.rs

@@ -6,6 +6,7 @@ use rustc_errors::{
     Applicability, Diagnostic, Handler, LazyFallbackBundle,
 };
 use rustc_parse::parse_stream_from_source_str;
+use rustc_resolve::rustdoc::source_span_for_markdown_range;
 use rustc_session::parse::ParseSess;
 use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
 use rustc_span::source_map::{FilePathMapping, SourceMap};
@@ -14,7 +15,6 @@ use rustc_span::{FileName, InnerSpan, DUMMY_SP};
 use crate::clean;
 use crate::core::DocContext;
 use crate::html::markdown::{self, RustCodeBlock};
-use crate::passes::source_span_for_markdown_range;
 
 pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) {
     if let Some(dox) = &item.opt_doc_value() {
@@ -77,11 +77,15 @@ fn check_rust_syntax(
     let is_ignore = code_block.lang_string.ignore != markdown::Ignore::None;
 
     // The span and whether it is precise or not.
-    let (sp, precise_span) =
-        match source_span_for_markdown_range(cx.tcx, dox, &code_block.range, &item.attrs) {
-            Some(sp) => (sp, true),
-            None => (item.attr_span(cx.tcx), false),
-        };
+    let (sp, precise_span) = match source_span_for_markdown_range(
+        cx.tcx,
+        dox,
+        &code_block.range,
+        &item.attrs.doc_strings,
+    ) {
+        Some(sp) => (sp, true),
+        None => (item.attr_span(cx.tcx), false),
+    };
 
     let msg = if buffer.has_errors {
         "could not parse code block as Rust code"

src/librustdoc/passes/lint/html_tags.rs

@@ -2,9 +2,9 @@
 use crate::clean::*;
 use crate::core::DocContext;
 use crate::html::markdown::main_body_opts;
-use crate::passes::source_span_for_markdown_range;
 
 use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag};
+use rustc_resolve::rustdoc::source_span_for_markdown_range;
 
 use std::iter::Peekable;
 use std::ops::Range;
@@ -20,7 +20,8 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
     let dox = item.doc_value();
     if !dox.is_empty() {
         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) {
+            let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings)
+            {
                 Some(sp) => sp,
                 None => item.attr_span(tcx),
             };
@@ -60,7 +61,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
                             tcx,
                             &dox,
                             &(generics_start..generics_end),
-                            &item.attrs,
+                            &item.attrs.doc_strings,
                         ) {
                             Some(sp) => sp,
                             None => item.attr_span(tcx),

src/librustdoc/passes/lint/redundant_explicit_links.rs

@@ -6,14 +6,14 @@ use rustc_errors::SuggestionStyle;
 use rustc_hir::def::{DefKind, DocLinkResMap, Namespace, Res};
 use rustc_hir::HirId;
 use rustc_lint_defs::Applicability;
+use rustc_resolve::rustdoc::source_span_for_markdown_range;
 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;
 
 #[derive(Debug)]
 struct LinkData {
@@ -160,16 +160,21 @@ fn check_inline_or_reference_unknown_redundancy(
         (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 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(
             cx.tcx,
             &doc,
             &offset_explicit_range(doc, link_range, open, close),
-            &item.attrs,
+            &item.attrs.doc_strings,
+        )?;
+        let display_span = source_span_for_markdown_range(
+            cx.tcx,
+            &doc,
+            &resolvable_link_range,
+            &item.attrs.doc_strings,
         )?;
-        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")
@@ -201,21 +206,26 @@ fn check_reference_redundancy(
         (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 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(
             cx.tcx,
             &doc,
             &offset_explicit_range(doc, link_range.clone(), b'[', b']'),
-            &item.attrs,
+            &item.attrs.doc_strings,
+        )?;
+        let display_span = source_span_for_markdown_range(
+            cx.tcx,
+            &doc,
+            &resolvable_link_range,
+            &item.attrs.doc_strings,
         )?;
-        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,
             &offset_reference_def_range(doc, dest, link_range),
-            &item.attrs,
+            &item.attrs.doc_strings,
         )?;
 
         cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| {