Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow #[expect] and #[allow] within function bodies for missing_panics_doc #14407

Merged
merged 4 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions clippy_lints/src/doc/missing_headers.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
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<'_>,
owner_id: OwnerId,
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) {
Expand Down Expand Up @@ -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",
);
}
Expand Down Expand Up @@ -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
}
112 changes: 19 additions & 93 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
);
}
Expand All @@ -697,32 +700,20 @@ 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) => {
if let ImplItemKind::Fn(sig, body_id) = impl_item.kind
&& !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,
);
}
Expand Down Expand Up @@ -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 {
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/missing_panics_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading