Auto merge of rust-lang#12635 - Alexendoo:doc-check-attributes, r=Jarcho

Use `check_attributes` in doc lints

Ensures we catch all the places that doc comments could occur, found one that we were currently missing - docs on `extern` items

changelog: none

Author: bors

clippy_lints/src/doc/missing_headers.rs

@@ -11,7 +11,7 @@ use super::{DocHeaders, MISSING_ERRORS_DOC, MISSING_PANICS_DOC, MISSING_SAFETY_D
 pub fn check(
     cx: &LateContext<'_>,
     owner_id: OwnerId,
-    sig: &FnSig<'_>,
+    sig: FnSig<'_>,
     headers: DocHeaders,
     body_id: Option<BodyId>,
     panic_span: Option<Span>,

clippy_lints/src/doc/mod.rs

@@ -3,17 +3,16 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::macros::{is_panic, root_macro_call_first_node};
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::visitors::Visitable;
-use clippy_utils::{is_entrypoint_fn, method_chain_args};
+use clippy_utils::{is_entrypoint_fn, is_trait_impl_item, method_chain_args};
 use pulldown_cmark::Event::{
     Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text,
 };
 use pulldown_cmark::Tag::{BlockQuote, CodeBlock, Heading, Item, Link, Paragraph};
 use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options};
 use rustc_ast::ast::Attribute;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_hir as hir;
 use rustc_hir::intravisit::{self, Visitor};
-use rustc_hir::{AnonConst, Expr};
+use rustc_hir::{AnonConst, Expr, ImplItemKind, ItemKind, Node, TraitItemKind, Unsafety};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::lint::in_external_macro;
@@ -366,15 +365,13 @@ declare_clippy_lint! {
 #[derive(Clone)]
 pub struct Documentation {
     valid_idents: FxHashSet<String>,
-    in_trait_impl: bool,
     check_private_items: bool,
 }
 
 impl Documentation {
     pub fn new(valid_idents: &[String], check_private_items: bool) -> Self {
         Self {
             valid_idents: valid_idents.iter().cloned().collect(),
-            in_trait_impl: false,
             check_private_items,
         }
     }
@@ -394,36 +391,72 @@ impl_lint_pass!(Documentation => [
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Documentation {
-    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
-        let attrs = cx.tcx.hir().attrs(hir::CRATE_HIR_ID);
-        check_attrs(cx, &self.valid_idents, attrs);
-    }
-
-    fn check_variant(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::Variant<'tcx>) {
-        let attrs = cx.tcx.hir().attrs(variant.hir_id);
-        check_attrs(cx, &self.valid_idents, attrs);
-    }
-
-    fn check_field_def(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::FieldDef<'tcx>) {
-        let attrs = cx.tcx.hir().attrs(variant.hir_id);
-        check_attrs(cx, &self.valid_idents, attrs);
-    }
-
-    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
-        let attrs = cx.tcx.hir().attrs(item.hir_id());
+    fn check_attributes(&mut self, cx: &LateContext<'tcx>, attrs: &'tcx [Attribute]) {
         let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
             return;
         };
 
-        match item.kind {
-            hir::ItemKind::Fn(ref sig, _, body_id) => {
-                if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
+        match cx.tcx.hir_node(cx.last_node_with_lint_attrs) {
+            Node::Item(item) => match item.kind {
+                ItemKind::Fn(sig, _, body_id) => {
+                    if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
+                        let body = cx.tcx.hir().body(body_id);
+
+                        let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
+                        missing_headers::check(
+                            cx,
+                            item.owner_id,
+                            sig,
+                            headers,
+                            Some(body_id),
+                            panic_span,
+                            self.check_private_items,
+                        );
+                    }
+                },
+                ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
+                    (false, Unsafety::Unsafe) => span_lint(
+                        cx,
+                        MISSING_SAFETY_DOC,
+                        cx.tcx.def_span(item.owner_id),
+                        "docs for unsafe trait missing `# Safety` section",
+                    ),
+                    (true, Unsafety::Normal) => span_lint(
+                        cx,
+                        UNNECESSARY_SAFETY_DOC,
+                        cx.tcx.def_span(item.owner_id),
+                        "docs for safe trait have unnecessary `# Safety` section",
+                    ),
+                    _ => (),
+                },
+                _ => (),
+            },
+            Node::TraitItem(trait_item) => {
+                if let TraitItemKind::Fn(sig, ..) = trait_item.kind
+                    && !in_external_macro(cx.tcx.sess, trait_item.span)
+                {
+                    missing_headers::check(
+                        cx,
+                        trait_item.owner_id,
+                        sig,
+                        headers,
+                        None,
+                        None,
+                        self.check_private_items,
+                    );
+                }
+            },
+            Node::ImplItem(impl_item) => {
+                if let ImplItemKind::Fn(sig, body_id) = impl_item.kind
+                    && !in_external_macro(cx.tcx.sess, impl_item.span)
+                    && !is_trait_impl_item(cx, impl_item.hir_id())
+                {
                     let body = cx.tcx.hir().body(body_id);
 
-                    let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
+                    let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(impl_item.owner_id), body.value);
                     missing_headers::check(
                         cx,
-                        item.owner_id,
+                        impl_item.owner_id,
                         sig,
                         headers,
                         Some(body_id),
@@ -432,67 +465,7 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
                     );
                 }
             },
-            hir::ItemKind::Impl(impl_) => {
-                self.in_trait_impl = impl_.of_trait.is_some();
-            },
-            hir::ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
-                (false, hir::Unsafety::Unsafe) => span_lint(
-                    cx,
-                    MISSING_SAFETY_DOC,
-                    cx.tcx.def_span(item.owner_id),
-                    "docs for unsafe trait missing `# Safety` section",
-                ),
-                (true, hir::Unsafety::Normal) => span_lint(
-                    cx,
-                    UNNECESSARY_SAFETY_DOC,
-                    cx.tcx.def_span(item.owner_id),
-                    "docs for safe trait have unnecessary `# Safety` section",
-                ),
-                _ => (),
-            },
-            _ => (),
-        }
-    }
-
-    fn check_item_post(&mut self, _cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
-        if let hir::ItemKind::Impl { .. } = item.kind {
-            self.in_trait_impl = false;
-        }
-    }
-
-    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
-        let attrs = cx.tcx.hir().attrs(item.hir_id());
-        let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
-            return;
-        };
-        if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind {
-            if !in_external_macro(cx.tcx.sess, item.span) {
-                missing_headers::check(cx, item.owner_id, sig, headers, None, None, self.check_private_items);
-            }
-        }
-    }
-
-    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
-        let attrs = cx.tcx.hir().attrs(item.hir_id());
-        let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
-            return;
-        };
-        if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
-            return;
-        }
-        if let hir::ImplItemKind::Fn(ref sig, body_id) = item.kind {
-            let body = cx.tcx.hir().body(body_id);
-
-            let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
-            missing_headers::check(
-                cx,
-                item.owner_id,
-                sig,
-                headers,
-                Some(body_id),
-                panic_span,
-                self.check_private_items,
-            );
+            _ => {},
         }
     }
 }

tests/ui/doc/doc-fixable.fixed

@@ -235,3 +235,8 @@ fn parenthesized_word() {}
 /// OSes
 /// UXes
 fn plural_acronym_test() {}
+
+extern {
+    /// `foo()`
+    fn in_extern();
+}

tests/ui/doc/doc-fixable.rs

@@ -235,3 +235,8 @@ fn parenthesized_word() {}
 /// OSes
 /// UXes
 fn plural_acronym_test() {}
+
+extern {
+    /// foo()
+    fn in_extern();
+}

tests/ui/doc/doc-fixable.stderr

@@ -352,5 +352,16 @@ help: try
 LL | /// `ABes`
    |     ~~~~~~
 
-error: aborting due to 32 previous errors
+error: item in documentation is missing backticks
+  --> tests/ui/doc/doc-fixable.rs:240:9
+   |
+LL |     /// foo()
+   |         ^^^^^
+   |
+help: try
+   |
+LL |     /// `foo()`
+   |         ~~~~~~~
+
+error: aborting due to 33 previous errors