missing_safety_doc: guide them out of an HTML block with a header

Author: notriddle

clippy_lints/src/doc/doc_suspicious_footnotes.rs

@@ -1,5 +1,4 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use rustc_ast::attr::AttributeExt as _;
 use rustc_ast::token::CommentKind;
 use rustc_errors::Applicability;
 use rustc_hir::{AttrStyle, Attribute};
@@ -8,7 +7,7 @@ use rustc_resolve::rustdoc::DocFragmentKind;
 
 use std::ops::Range;
 
-use super::{DOC_SUSPICIOUS_FOOTNOTES, Fragments};
+use super::{DOC_SUSPICIOUS_FOOTNOTES, Fragments, find_doc_attr_by_span};
 
 pub fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, fragments: &Fragments<'_>, attrs: &[Attribute]) {
     for i in doc[range.clone()]
@@ -44,14 +43,8 @@ pub fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, fragments: &F
                 "looks like a footnote ref, but has no matching footnote",
                 |diag| {
                     if this_fragment.kind == DocFragmentKind::SugaredDoc {
-                        let (doc_attr, (_, doc_attr_comment_kind), attr_style) = attrs
-                            .iter()
-                            .filter(|attr| attr.span().overlaps(this_fragment.span))
-                            .rev()
-                            .find_map(|attr| {
-                                Some((attr, attr.doc_str_and_comment_kind()?, attr.doc_resolution_scope()?))
-                            })
-                            .unwrap();
+                        let (doc_attr, doc_attr_comment_kind, attr_style) =
+                            find_doc_attr_by_span(attrs, this_fragment.span).unwrap();
                         let (to_add, terminator) = match (doc_attr_comment_kind, attr_style) {
                             (CommentKind::Line, AttrStyle::Outer) => ("\n///\n/// ", ""),
                             (CommentKind::Line, AttrStyle::Inner) => ("\n//!\n//! ", ""),

clippy_lints/src/doc/lazy_continuation.rs

@@ -7,14 +7,6 @@ use std::ops::Range;
 
 use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS, Fragments};
 
-fn map_container_to_text(c: &super::Container) -> &'static str {
-    match c {
-        super::Container::Blockquote => "> ",
-        // numbered list can have up to nine digits, plus the dot, plus four spaces on either side
-        super::Container::List(indent) => &"                  "[0..*indent],
-    }
-}
-
 pub(super) fn check(
     cx: &LateContext<'_>,
     doc: &str,
@@ -41,7 +33,7 @@ pub(super) fn check(
                 let mut doc_start_range = &doc[cooked_range];
                 let mut suggested = String::new();
                 for c in containers {
-                    let text = map_container_to_text(c);
+                    let text = c.map_to_text();
                     if doc_start_range.starts_with(text) {
                         doc_start_range = &doc_start_range[text.len()..];
                         span = span.with_lo(

clippy_lints/src/doc/missing_headers.rs

@@ -1,5 +1,5 @@
 use super::{DocHeaders, MISSING_ERRORS_DOC, MISSING_PANICS_DOC, MISSING_SAFETY_DOC, UNNECESSARY_SAFETY_DOC};
-use clippy_utils::diagnostics::{span_lint, span_lint_and_note};
+use clippy_utils::diagnostics::span_lint;
 use clippy_utils::macros::{is_panic, root_macro_call_first_node};
 use clippy_utils::ty::{get_type_diagnostic_name, implements_trait_with_env, is_type_diagnostic_item};
 use clippy_utils::visitors::for_each_expr;
@@ -33,37 +33,38 @@ pub fn check(
     }
 
     let span = cx.tcx.def_span(owner_id);
-    match (headers.safety, sig.header.safety()) {
-        (false, Safety::Unsafe) => span_lint(
+    match (headers.safety.is_missing(), sig.header.safety()) {
+        (true, Safety::Unsafe) => headers.safety.lint(
             cx,
             MISSING_SAFETY_DOC,
             span,
             "unsafe function's docs are missing a `# Safety` section",
         ),
-        (true, Safety::Safe) => span_lint(
+        (false, Safety::Safe) => span_lint(
             cx,
             UNNECESSARY_SAFETY_DOC,
             span,
             "safe function's docs have unnecessary `# Safety` section",
         ),
         _ => (),
     }
-    if !headers.panics
+    if headers.panics.is_missing()
         && let Some(body_id) = body_id
         && let Some(panic_span) = find_panic(cx, body_id)
     {
-        span_lint_and_note(
+        headers.panics.lint_and_then(
             cx,
             MISSING_PANICS_DOC,
             span,
             "docs for function which may panic missing `# Panics` section",
-            Some(panic_span),
-            "first possible panic found here",
+            |diag| {
+                diag.span_note(panic_span, "first possible panic found here");
+            },
         );
     }
-    if !headers.errors {
+    if headers.errors.is_missing() {
         if is_type_diagnostic_item(cx, return_ty(cx, owner_id), sym::Result) {
-            span_lint(
+            headers.errors.lint(
                 cx,
                 MISSING_ERRORS_DOC,
                 span,
@@ -85,7 +86,7 @@ pub fn check(
             && let ty::Coroutine(_, subs) = ret_ty.kind()
             && is_type_diagnostic_item(cx, subs.as_coroutine().return_ty(), sym::Result)
         {
-            span_lint(
+            headers.errors.lint(
                 cx,
                 MISSING_ERRORS_DOC,
                 span,

clippy_lints/src/doc/mod.rs

@@ -10,10 +10,12 @@ use pulldown_cmark::Event::{
 };
 use pulldown_cmark::Tag::{BlockQuote, CodeBlock, FootnoteDefinition, Heading, Item, Link, Paragraph};
 use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options, TagEnd};
+use rustc_ast::attr::AttributeExt;
+use rustc_ast::token::CommentKind;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::Applicability;
-use rustc_hir::{Attribute, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
-use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
+use rustc_errors::{Applicability, Diag, DiagMessage, MultiSpan};
+use rustc_hir::{AttrStyle, Attribute, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
+use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, Lint, LintContext};
 use rustc_resolve::rustdoc::{
     DocFragment, add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range,
     span_of_fragments,
@@ -740,14 +742,14 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
                             );
                         }
                     },
-                    ItemKind::Trait(_, _, unsafety, ..) => match (headers.safety, unsafety) {
+                    ItemKind::Trait(_, _, unsafety, ..) => match (headers.safety.is_missing(), unsafety) {
                         (false, Safety::Unsafe) => span_lint(
                             cx,
                             MISSING_SAFETY_DOC,
                             cx.tcx.def_span(item.owner_id),
                             "docs for unsafe trait missing `# Safety` section",
                         ),
-                        (true, Safety::Safe) => span_lint(
+                        (true, Safety::Safe) => headers.safety.lint(
                             cx,
                             UNNECESSARY_SAFETY_DOC,
                             cx.tcx.def_span(item.owner_id),
@@ -800,11 +802,101 @@ impl Fragments<'_> {
     }
 }
 
-#[derive(Copy, Clone, Default)]
+#[derive(Clone, Default)]
+enum DocHeaderInfo {
+    #[default]
+    None,
+    Found,
+    SuspiciousHtml(Option<Span>, &'static str, Vec<Container>),
+}
+
+impl DocHeaderInfo {
+    fn suspicious_html(
+        cx: &LateContext<'_>,
+        fragments: Fragments<'_>,
+        idx: usize,
+        attrs: &[Attribute],
+        containers: &[Container],
+    ) -> DocHeaderInfo {
+        use rustc_ast::token::CommentKind;
+        use rustc_hir::AttrStyle;
+        let span = fragments.span(cx, idx..idx);
+        let indent = span
+            .and_then(|span| fragments.fragments.iter().find(|fragment| fragment.span.overlaps(span)))
+            .map(|fragment| fragment.indent)
+            .unwrap_or(0);
+        DocHeaderInfo::SuspiciousHtml(
+            span,
+            span.and_then(|span| find_doc_attr_by_span(attrs, span)).map_or(
+                "",
+                |(_doc_attr, doc_attr_comment_kind, attr_style)| match (doc_attr_comment_kind, attr_style) {
+                    (CommentKind::Block, _) => &"        "[..indent],
+                    (CommentKind::Line, AttrStyle::Outer) => &"///        "[..indent + 3],
+                    (CommentKind::Line, AttrStyle::Inner) => &"//!        "[..indent + 3],
+                },
+            ),
+            containers.to_vec(),
+        )
+    }
+    fn is_missing(&self) -> bool {
+        matches!(self, DocHeaderInfo::None | DocHeaderInfo::SuspiciousHtml(..))
+    }
+    fn lint(&self, cx: &LateContext<'_>, lint: &'static Lint, sp: impl Into<MultiSpan>, msg: impl Into<DiagMessage>) {
+        self.lint_and_then(cx, lint, sp, msg, |_| {})
+    }
+    fn lint_and_then(
+        &self,
+        cx: &LateContext<'_>,
+        lint: &'static Lint,
+        sp: impl Into<MultiSpan>,
+        msg: impl Into<DiagMessage>,
+        f: impl FnOnce(&mut Diag<'_, ()>),
+    ) {
+        if let DocHeaderInfo::SuspiciousHtml(html_span, comment_prefix, containers) = self {
+            span_lint_and_then(cx, lint, sp, msg, |diag| {
+                f(diag);
+                diag.note("markdown syntax is not recognized within a block of raw HTML code");
+                if let Some(html_span) = html_span {
+                    diag.span_suggestion(
+                        *html_span,
+                        "to recognize this text as a header, add a blank line",
+                        format!(
+                            "\n{comment_prefix}{containers}",
+                            containers = containers
+                                .iter()
+                                .map(Container::map_to_text)
+                                .collect::<Vec<&str>>()
+                                .join("")
+                        ),
+                        Applicability::Unspecified,
+                    );
+                } else {
+                    diag.help(
+                        "to recognize a markdown header nested within an HTML element,\
+                            add a blank line between the `#` and the HTML",
+                    );
+                }
+            });
+        } else {
+            span_lint(cx, lint, sp, msg);
+        }
+    }
+}
+
+fn find_doc_attr_by_span(attrs: &[Attribute], span: Span) -> Option<(&Attribute, CommentKind, AttrStyle)> {
+    let (doc_attr, (_, doc_attr_comment_kind), attr_style) = attrs
+        .iter()
+        .filter(|attr| attr.span().overlaps(span))
+        .rev()
+        .find_map(|attr| Some((attr, attr.doc_str_and_comment_kind()?, attr.doc_resolution_scope()?)))?;
+    Some((doc_attr, doc_attr_comment_kind, attr_style))
+}
+
+#[derive(Clone, Default)]
 struct DocHeaders {
-    safety: bool,
-    errors: bool,
-    panics: bool,
+    safety: DocHeaderInfo,
+    errors: DocHeaderInfo,
+    panics: DocHeaderInfo,
     first_paragraph_len: usize,
 }
 
@@ -900,11 +992,22 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
 
 const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];
 
+#[derive(Clone)]
 enum Container {
     Blockquote,
     List(usize),
 }
 
+impl Container {
+    fn map_to_text(&self) -> &'static str {
+        match self {
+            Container::Blockquote => "> ",
+            // numbered list can have up to nine digits, plus the dot, plus four spaces on either side
+            Container::List(indent) => &"                  "[0..*indent],
+        }
+    }
+}
+
 /// Scan the documentation for code links that are back-to-back with code spans.
 ///
 /// This is done separately from the rest of the docs, because that makes it easier to produce
@@ -1011,6 +1114,51 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 } else if tag.starts_with("</blockquote") || tag.starts_with("</q") {
                     blockquote_level -= 1;
                 }
+                if headers.safety.is_missing() &&
+                    let Some(idx) = tag.find("# Safety").filter(|idx| tag[idx + 8..].trim().bytes().all(|c| c == b'#'))
+                        .or_else(|| tag.find("Safety\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-'))))
+                        .or_else(|| tag.find("# SAFETY").filter(|idx| tag[idx + 8..].trim().bytes().all(|c| c == b'#')))
+                        .or_else(|| tag.find("SAFETY\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-'))))
+                        .or_else(|| tag.find("# Implementation safety").filter(|idx| tag[idx + 23..].trim().bytes().all(|c| c == b'#')))
+                        .or_else(|| tag.find("Implementation safety\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-'))))
+                        .or_else(|| tag.find("# Implementation Safety").filter(|idx| tag[idx + 23..].trim().bytes().all(|c| c == b'#')))
+                        .or_else(|| tag.find("Implementation Safety\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-')))) &&
+                    tag[..idx].trim().bytes().all(|c| c == b'#')
+                {
+                    headers.safety = DocHeaderInfo::suspicious_html(
+                        cx,
+                        fragments,
+                        range.start,
+                        attrs,
+                        &containers,
+                    );
+                }
+                if headers.errors.is_missing() &&
+                    let Some(idx) = tag.find("# Errors").filter(|idx| tag[idx + 8..].trim().bytes().all(|c| c == b'#'))
+                        .or_else(|| tag.find("Errors\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-')))) &&
+                    tag[..idx].trim().bytes().all(|c| c == b'#')
+                {
+                    headers.errors = DocHeaderInfo::suspicious_html(
+                        cx,
+                        fragments,
+                        range.start,
+                        attrs,
+                        &containers,
+                    );
+                }
+                if headers.panics.is_missing() &&
+                    let Some(idx) = tag.find("# Panics").filter(|idx| tag[idx + 8..].trim().bytes().all(|c| c == b'#'))
+                        .or_else(|| tag.find("Panics\n").filter(|_| matches!(events.peek(), Some((Html(ln) | InlineHtml(ln), _)) if ln.trim().bytes().all(|c| c == b'=') || ln.trim().bytes().all(|c| c == b'-')))) &&
+                    tag[..idx].trim().bytes().all(|c| c == b'#')
+                {
+                    headers.panics = DocHeaderInfo::suspicious_html(
+                        cx,
+                        fragments,
+                        range.start,
+                        attrs,
+                        &containers,
+                    );
+                }
             },
             Start(BlockQuote(_)) => {
                 blockquote_level += 1;
@@ -1197,12 +1345,28 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                     continue;
                 }
                 let trimmed_text = text.trim();
-                headers.safety |= in_heading && trimmed_text == "Safety";
-                headers.safety |= in_heading && trimmed_text == "SAFETY";
-                headers.safety |= in_heading && trimmed_text == "Implementation safety";
-                headers.safety |= in_heading && trimmed_text == "Implementation Safety";
-                headers.errors |= in_heading && trimmed_text == "Errors";
-                headers.panics |= in_heading && trimmed_text == "Panics";
+                if in_heading {
+                    if headers.safety.is_missing()
+                        && (
+                            trimmed_text == "Safety"
+                            || trimmed_text == "SAFETY"
+                            || trimmed_text == "Implementation safety"
+                            || trimmed_text == "Implementation Safety"
+                        )
+                    {
+                        headers.safety = DocHeaderInfo::Found;
+                    }
+                    if headers.errors.is_missing()
+                        && trimmed_text == "Errors"
+                    {
+                        headers.errors = DocHeaderInfo::Found;
+                    }
+                    if headers.panics.is_missing()
+                        && trimmed_text == "Panics"
+                    {
+                        headers.panics = DocHeaderInfo::Found;
+                    }
+                }
                 if in_code {
                     if is_rust && !no_test {
                         let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());