about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2025-03-31 12:56:49 +0000
committerGitHub <noreply@github.com>2025-03-31 12:56:49 +0000
commitb96fecfee984502c5ea1ca76ef871374efc0da85 (patch)
tree0be57b5bab69022dfa5c654c5f3bef4a0714b168
parentbb0d09b220dc94fa956d59b395c825b217a8070e (diff)
parentf894a81654d6fd67137ba0676bfbd295a48cb471 (diff)
downloadrust-b96fecfee984502c5ea1ca76ef871374efc0da85.tar.gz
rust-b96fecfee984502c5ea1ca76ef871374efc0da85.zip
Allow `#[expect]` and `#[allow]` within function bodies for `missing_panics_doc` (#14407)
Implements
https://github.com/rust-lang/rust-clippy/issues/11436#issuecomment-2719199421

> [...] I'd really like to be able to (reusing some above examples),
>
> ``` rust
> /// Do something
> pub fn string_from_byte_stream() -> String {
>     let bytes = get_valid_utf8();
> #[expect(clippy::missing_panics_doc_ok, reason="caller can't do
anything about this")]
> String::from_utf8(bytes).expect("`get_valid_utf8()` always returns
valid UTF-8")
> }
> ```

Also fixes an issue where if the first panic is in a `const` context
disables the lint for the rest of the function

The first commit is just moving code around

changelog: [`missing_panics_doc`]: `#[allow]` and `#[expect]` can now be
used within the function body to ignore individual panics
-rw-r--r--clippy_lints/src/doc/missing_headers.rs51
-rw-r--r--clippy_lints/src/doc/mod.rs112
-rw-r--r--tests/ui/missing_panics_doc.rs39
-rw-r--r--tests/ui/missing_panics_doc.stderr62
4 files changed, 153 insertions, 111 deletions
diff --git a/clippy_lints/src/doc/missing_headers.rs b/clippy_lints/src/doc/missing_headers.rs
index e75abf28bac..039937e0207 100644
--- a/clippy_lints/src/doc/missing_headers.rs
+++ b/clippy_lints/src/doc/missing_headers.rs
@@ -1,11 +1,14 @@
 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::ty::{implements_trait_with_env, is_type_diagnostic_item};
-use clippy_utils::{is_doc_hidden, return_ty};
+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;
+use clippy_utils::{fulfill_or_allowed, is_doc_hidden, method_chain_args, return_ty};
 use rustc_hir::{BodyId, FnSig, OwnerId, Safety};
 use rustc_lint::LateContext;
 use rustc_middle::ty;
 use rustc_span::{Span, sym};
+use std::ops::ControlFlow;
 
 pub fn check(
     cx: &LateContext<'_>,
@@ -13,7 +16,6 @@ pub fn check(
     sig: FnSig<'_>,
     headers: DocHeaders,
     body_id: Option<BodyId>,
-    panic_info: Option<(Span, bool)>,
     check_private_items: bool,
 ) {
     if !check_private_items && !cx.effective_visibilities.is_exported(owner_id.def_id) {
@@ -46,13 +48,16 @@ pub fn check(
         ),
         _ => (),
     }
-    if !headers.panics && panic_info.is_some_and(|el| !el.1) {
+    if !headers.panics
+        && let Some(body_id) = body_id
+        && let Some(panic_span) = find_panic(cx, body_id)
+    {
         span_lint_and_note(
             cx,
             MISSING_PANICS_DOC,
             span,
             "docs for function which may panic missing `# Panics` section",
-            panic_info.map(|el| el.0),
+            Some(panic_span),
             "first possible panic found here",
         );
     }
@@ -89,3 +94,39 @@ pub fn check(
         }
     }
 }
+
+fn find_panic(cx: &LateContext<'_>, body_id: BodyId) -> Option<Span> {
+    let mut panic_span = None;
+    let typeck = cx.tcx.typeck_body(body_id);
+    for_each_expr(cx, cx.tcx.hir_body(body_id), |expr| {
+        if let Some(macro_call) = root_macro_call_first_node(cx, expr)
+            && (is_panic(cx, macro_call.def_id)
+                || matches!(
+                    cx.tcx.get_diagnostic_name(macro_call.def_id),
+                    Some(sym::assert_macro | sym::assert_eq_macro | sym::assert_ne_macro)
+                ))
+            && !cx.tcx.hir_is_inside_const_context(expr.hir_id)
+            && !fulfill_or_allowed(cx, MISSING_PANICS_DOC, [expr.hir_id])
+            && panic_span.is_none()
+        {
+            panic_span = Some(macro_call.span);
+        }
+
+        // check for `unwrap` and `expect` for both `Option` and `Result`
+        if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or_else(|| method_chain_args(expr, &["expect"]))
+            && let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs()
+            && matches!(
+                get_type_diagnostic_name(cx, receiver_ty),
+                Some(sym::Option | sym::Result)
+            )
+            && !fulfill_or_allowed(cx, MISSING_PANICS_DOC, [expr.hir_id])
+            && panic_span.is_none()
+        {
+            panic_span = Some(expr.span);
+        }
+
+        // Visit all nodes to fulfill any `#[expect]`s after the first linted panic
+        ControlFlow::<!>::Continue(())
+    });
+    panic_span
+}
diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
index d0075d01eea..ab77edf1147 100644
--- a/clippy_lints/src/doc/mod.rs
+++ b/clippy_lints/src/doc/mod.rs
@@ -3,11 +3,8 @@
 use clippy_config::Conf;
 use clippy_utils::attrs::is_doc_hidden;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then};
-use clippy_utils::macros::{is_panic, root_macro_call_first_node};
 use clippy_utils::source::snippet_opt;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::visitors::Visitable;
-use clippy_utils::{is_entrypoint_fn, is_trait_impl_item, method_chain_args};
+use clippy_utils::{is_entrypoint_fn, is_trait_impl_item};
 use pulldown_cmark::Event::{
     Code, DisplayMath, End, FootnoteReference, HardBreak, Html, InlineHtml, InlineMath, Rule, SoftBreak, Start,
     TaskListMarker, Text,
@@ -16,18 +13,15 @@ use pulldown_cmark::Tag::{BlockQuote, CodeBlock, FootnoteDefinition, Heading, It
 use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options, TagEnd};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
-use rustc_hir::intravisit::{self, Visitor};
-use rustc_hir::{AnonConst, Attribute, Expr, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
+use rustc_hir::{Attribute, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
-use rustc_middle::hir::nested_filter;
-use rustc_middle::ty;
 use rustc_resolve::rustdoc::{
     DocFragment, add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range,
     span_of_fragments,
 };
 use rustc_session::impl_lint_pass;
+use rustc_span::Span;
 use rustc_span::edition::Edition;
-use rustc_span::{Span, sym};
 use std::ops::Range;
 use url::Url;
 
@@ -194,6 +188,19 @@ declare_clippy_lint! {
     ///     }
     /// }
     /// ```
+    ///
+    /// Individual panics within a function can be ignored with `#[expect]` or
+    /// `#[allow]`:
+    ///
+    /// ```no_run
+    /// # use std::num::NonZeroUsize;
+    /// pub fn will_not_panic(x: usize) {
+    ///     #[expect(clippy::missing_panics_doc, reason = "infallible")]
+    ///     let y = NonZeroUsize::new(1).unwrap();
+    ///
+    ///     // If any panics are added in the future the lint will still catch them
+    /// }
+    /// ```
     #[clippy::version = "1.51.0"]
     pub MISSING_PANICS_DOC,
     pedantic,
@@ -657,20 +664,16 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
                     self.check_private_items,
                 );
                 match item.kind {
-                    ItemKind::Fn { sig, body: body_id, .. } => {
+                    ItemKind::Fn { sig, body, .. } => {
                         if !(is_entrypoint_fn(cx, item.owner_id.to_def_id())
                             || item.span.in_external_macro(cx.tcx.sess.source_map()))
                         {
-                            let body = cx.tcx.hir_body(body_id);
-
-                            let panic_info = 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_info,
+                                Some(body),
                                 self.check_private_items,
                             );
                         }
@@ -697,15 +700,7 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
                 if let TraitItemKind::Fn(sig, ..) = trait_item.kind
                     && !trait_item.span.in_external_macro(cx.tcx.sess.source_map())
                 {
-                    missing_headers::check(
-                        cx,
-                        trait_item.owner_id,
-                        sig,
-                        headers,
-                        None,
-                        None,
-                        self.check_private_items,
-                    );
+                    missing_headers::check(cx, trait_item.owner_id, sig, headers, None, self.check_private_items);
                 }
             },
             Node::ImplItem(impl_item) => {
@@ -713,16 +708,12 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
                     && !impl_item.span.in_external_macro(cx.tcx.sess.source_map())
                     && !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(impl_item.owner_id), body.value);
                     missing_headers::check(
                         cx,
                         impl_item.owner_id,
                         sig,
                         headers,
                         Some(body_id),
-                        panic_span,
                         self.check_private_items,
                     );
                 }
@@ -1168,71 +1159,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
     headers
 }
 
-struct FindPanicUnwrap<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,
-    is_const: bool,
-    panic_span: Option<Span>,
-    typeck_results: &'tcx ty::TypeckResults<'tcx>,
-}
-
-impl<'a, 'tcx> FindPanicUnwrap<'a, 'tcx> {
-    pub fn find_span(
-        cx: &'a LateContext<'tcx>,
-        typeck_results: &'tcx ty::TypeckResults<'tcx>,
-        body: impl Visitable<'tcx>,
-    ) -> Option<(Span, bool)> {
-        let mut vis = Self {
-            cx,
-            is_const: false,
-            panic_span: None,
-            typeck_results,
-        };
-        body.visit(&mut vis);
-        vis.panic_span.map(|el| (el, vis.is_const))
-    }
-}
-
-impl<'tcx> Visitor<'tcx> for FindPanicUnwrap<'_, 'tcx> {
-    type NestedFilter = nested_filter::OnlyBodies;
-
-    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
-        if self.panic_span.is_some() {
-            return;
-        }
-
-        if let Some(macro_call) = root_macro_call_first_node(self.cx, expr)
-            && (is_panic(self.cx, macro_call.def_id)
-                || matches!(
-                    self.cx.tcx.item_name(macro_call.def_id).as_str(),
-                    "assert" | "assert_eq" | "assert_ne"
-                ))
-        {
-            self.is_const = self.cx.tcx.hir_is_inside_const_context(expr.hir_id);
-            self.panic_span = Some(macro_call.span);
-        }
-
-        // check for `unwrap` and `expect` for both `Option` and `Result`
-        if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or(method_chain_args(expr, &["expect"])) {
-            let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs();
-            if is_type_diagnostic_item(self.cx, receiver_ty, sym::Option)
-                || is_type_diagnostic_item(self.cx, receiver_ty, sym::Result)
-            {
-                self.panic_span = Some(expr.span);
-            }
-        }
-
-        // and check sub-expressions
-        intravisit::walk_expr(self, expr);
-    }
-
-    // Panics in const blocks will cause compilation to fail.
-    fn visit_anon_const(&mut self, _: &'tcx AnonConst) {}
-
-    fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
-        self.cx.tcx
-    }
-}
-
 #[expect(clippy::range_plus_one)] // inclusive ranges aren't the same type
 fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> {
     if range.end < range.start {
diff --git a/tests/ui/missing_panics_doc.rs b/tests/ui/missing_panics_doc.rs
index 95e361c5d55..ffdae8504f7 100644
--- a/tests/ui/missing_panics_doc.rs
+++ b/tests/ui/missing_panics_doc.rs
@@ -151,6 +151,45 @@ pub fn debug_assertions() {
     debug_assert_ne!(1, 2);
 }
 
+pub fn partially_const<const N: usize>(n: usize) {
+    //~^ missing_panics_doc
+
+    const {
+        assert!(N > 5);
+    }
+
+    assert!(N > n);
+}
+
+pub fn expect_allow(i: Option<isize>) {
+    #[expect(clippy::missing_panics_doc)]
+    i.unwrap();
+
+    #[allow(clippy::missing_panics_doc)]
+    i.unwrap();
+}
+
+pub fn expect_allow_with_error(i: Option<isize>) {
+    //~^ missing_panics_doc
+
+    #[expect(clippy::missing_panics_doc)]
+    i.unwrap();
+
+    #[allow(clippy::missing_panics_doc)]
+    i.unwrap();
+
+    i.unwrap();
+}
+
+pub fn expect_after_error(x: Option<u32>, y: Option<u32>) {
+    //~^ missing_panics_doc
+
+    let x = x.unwrap();
+
+    #[expect(clippy::missing_panics_doc)]
+    let y = y.unwrap();
+}
+
 // all function must be triggered the lint.
 // `pub` is required, because the lint does not consider unreachable items
 pub mod issue10240 {
diff --git a/tests/ui/missing_panics_doc.stderr b/tests/ui/missing_panics_doc.stderr
index a83e2fa367d..7f0acf8de9b 100644
--- a/tests/ui/missing_panics_doc.stderr
+++ b/tests/ui/missing_panics_doc.stderr
@@ -73,76 +73,112 @@ LL |     assert_ne!(x, 0);
    |     ^^^^^^^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:157:5
+  --> tests/ui/missing_panics_doc.rs:154:1
+   |
+LL | pub fn partially_const<const N: usize>(n: usize) {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first possible panic found here
+  --> tests/ui/missing_panics_doc.rs:161:5
+   |
+LL |     assert!(N > n);
+   |     ^^^^^^^^^^^^^^
+
+error: docs for function which may panic missing `# Panics` section
+  --> tests/ui/missing_panics_doc.rs:172:1
+   |
+LL | pub fn expect_allow_with_error(i: Option<isize>) {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first possible panic found here
+  --> tests/ui/missing_panics_doc.rs:181:5
+   |
+LL |     i.unwrap();
+   |     ^^^^^^^^^^
+
+error: docs for function which may panic missing `# Panics` section
+  --> tests/ui/missing_panics_doc.rs:184:1
+   |
+LL | pub fn expect_after_error(x: Option<u32>, y: Option<u32>) {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first possible panic found here
+  --> tests/ui/missing_panics_doc.rs:187:13
+   |
+LL |     let x = x.unwrap();
+   |             ^^^^^^^^^^
+
+error: docs for function which may panic missing `# Panics` section
+  --> tests/ui/missing_panics_doc.rs:196:5
    |
 LL |     pub fn option_unwrap<T>(v: &[T]) -> &T {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:160:9
+  --> tests/ui/missing_panics_doc.rs:199:9
    |
 LL |         o.unwrap()
    |         ^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:163:5
+  --> tests/ui/missing_panics_doc.rs:202:5
    |
 LL |     pub fn option_expect<T>(v: &[T]) -> &T {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:166:9
+  --> tests/ui/missing_panics_doc.rs:205:9
    |
 LL |         o.expect("passed an empty thing")
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:169:5
+  --> tests/ui/missing_panics_doc.rs:208:5
    |
 LL |     pub fn result_unwrap<T>(v: &[T]) -> &T {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:172:9
+  --> tests/ui/missing_panics_doc.rs:211:9
    |
 LL |         res.unwrap()
    |         ^^^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:175:5
+  --> tests/ui/missing_panics_doc.rs:214:5
    |
 LL |     pub fn result_expect<T>(v: &[T]) -> &T {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:178:9
+  --> tests/ui/missing_panics_doc.rs:217:9
    |
 LL |         res.expect("passed an empty thing")
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:181:5
+  --> tests/ui/missing_panics_doc.rs:220:5
    |
 LL |     pub fn last_unwrap(v: &[u32]) -> u32 {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:183:10
+  --> tests/ui/missing_panics_doc.rs:222:10
    |
 LL |         *v.last().unwrap()
    |          ^^^^^^^^^^^^^^^^^
 
 error: docs for function which may panic missing `# Panics` section
-  --> tests/ui/missing_panics_doc.rs:186:5
+  --> tests/ui/missing_panics_doc.rs:225:5
    |
 LL |     pub fn last_expect(v: &[u32]) -> u32 {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: first possible panic found here
-  --> tests/ui/missing_panics_doc.rs:188:10
+  --> tests/ui/missing_panics_doc.rs:227:10
    |
 LL |         *v.last().expect("passed an empty thing")
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 12 previous errors
+error: aborting due to 15 previous errors