Skip to content

Commit b96fecf

Browse files
authored
Allow #[expect] and #[allow] within function bodies for missing_panics_doc (#14407)
Implements #11436 (comment) > [...] 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
2 parents bb0d09b + f894a81 commit b96fecf

File tree

4 files changed

+153
-111
lines changed

4 files changed

+153
-111
lines changed

clippy_lints/src/doc/missing_headers.rs

+46-5
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
use super::{DocHeaders, MISSING_ERRORS_DOC, MISSING_PANICS_DOC, MISSING_SAFETY_DOC, UNNECESSARY_SAFETY_DOC};
22
use clippy_utils::diagnostics::{span_lint, span_lint_and_note};
3-
use clippy_utils::ty::{implements_trait_with_env, is_type_diagnostic_item};
4-
use clippy_utils::{is_doc_hidden, return_ty};
3+
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
4+
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait_with_env, is_type_diagnostic_item};
5+
use clippy_utils::visitors::for_each_expr;
6+
use clippy_utils::{fulfill_or_allowed, is_doc_hidden, method_chain_args, return_ty};
57
use rustc_hir::{BodyId, FnSig, OwnerId, Safety};
68
use rustc_lint::LateContext;
79
use rustc_middle::ty;
810
use rustc_span::{Span, sym};
11+
use std::ops::ControlFlow;
912

1013
pub fn check(
1114
cx: &LateContext<'_>,
1215
owner_id: OwnerId,
1316
sig: FnSig<'_>,
1417
headers: DocHeaders,
1518
body_id: Option<BodyId>,
16-
panic_info: Option<(Span, bool)>,
1719
check_private_items: bool,
1820
) {
1921
if !check_private_items && !cx.effective_visibilities.is_exported(owner_id.def_id) {
@@ -46,13 +48,16 @@ pub fn check(
4648
),
4749
_ => (),
4850
}
49-
if !headers.panics && panic_info.is_some_and(|el| !el.1) {
51+
if !headers.panics
52+
&& let Some(body_id) = body_id
53+
&& let Some(panic_span) = find_panic(cx, body_id)
54+
{
5055
span_lint_and_note(
5156
cx,
5257
MISSING_PANICS_DOC,
5358
span,
5459
"docs for function which may panic missing `# Panics` section",
55-
panic_info.map(|el| el.0),
60+
Some(panic_span),
5661
"first possible panic found here",
5762
);
5863
}
@@ -89,3 +94,39 @@ pub fn check(
8994
}
9095
}
9196
}
97+
98+
fn find_panic(cx: &LateContext<'_>, body_id: BodyId) -> Option<Span> {
99+
let mut panic_span = None;
100+
let typeck = cx.tcx.typeck_body(body_id);
101+
for_each_expr(cx, cx.tcx.hir_body(body_id), |expr| {
102+
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
103+
&& (is_panic(cx, macro_call.def_id)
104+
|| matches!(
105+
cx.tcx.get_diagnostic_name(macro_call.def_id),
106+
Some(sym::assert_macro | sym::assert_eq_macro | sym::assert_ne_macro)
107+
))
108+
&& !cx.tcx.hir_is_inside_const_context(expr.hir_id)
109+
&& !fulfill_or_allowed(cx, MISSING_PANICS_DOC, [expr.hir_id])
110+
&& panic_span.is_none()
111+
{
112+
panic_span = Some(macro_call.span);
113+
}
114+
115+
// check for `unwrap` and `expect` for both `Option` and `Result`
116+
if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or_else(|| method_chain_args(expr, &["expect"]))
117+
&& let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs()
118+
&& matches!(
119+
get_type_diagnostic_name(cx, receiver_ty),
120+
Some(sym::Option | sym::Result)
121+
)
122+
&& !fulfill_or_allowed(cx, MISSING_PANICS_DOC, [expr.hir_id])
123+
&& panic_span.is_none()
124+
{
125+
panic_span = Some(expr.span);
126+
}
127+
128+
// Visit all nodes to fulfill any `#[expect]`s after the first linted panic
129+
ControlFlow::<!>::Continue(())
130+
});
131+
panic_span
132+
}

clippy_lints/src/doc/mod.rs

+19-93
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@
33
use clippy_config::Conf;
44
use clippy_utils::attrs::is_doc_hidden;
55
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then};
6-
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
76
use clippy_utils::source::snippet_opt;
8-
use clippy_utils::ty::is_type_diagnostic_item;
9-
use clippy_utils::visitors::Visitable;
10-
use clippy_utils::{is_entrypoint_fn, is_trait_impl_item, method_chain_args};
7+
use clippy_utils::{is_entrypoint_fn, is_trait_impl_item};
118
use pulldown_cmark::Event::{
129
Code, DisplayMath, End, FootnoteReference, HardBreak, Html, InlineHtml, InlineMath, Rule, SoftBreak, Start,
1310
TaskListMarker, Text,
@@ -16,18 +13,15 @@ use pulldown_cmark::Tag::{BlockQuote, CodeBlock, FootnoteDefinition, Heading, It
1613
use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options, TagEnd};
1714
use rustc_data_structures::fx::FxHashSet;
1815
use rustc_errors::Applicability;
19-
use rustc_hir::intravisit::{self, Visitor};
20-
use rustc_hir::{AnonConst, Attribute, Expr, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
16+
use rustc_hir::{Attribute, ImplItemKind, ItemKind, Node, Safety, TraitItemKind};
2117
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
22-
use rustc_middle::hir::nested_filter;
23-
use rustc_middle::ty;
2418
use rustc_resolve::rustdoc::{
2519
DocFragment, add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range,
2620
span_of_fragments,
2721
};
2822
use rustc_session::impl_lint_pass;
23+
use rustc_span::Span;
2924
use rustc_span::edition::Edition;
30-
use rustc_span::{Span, sym};
3125
use std::ops::Range;
3226
use url::Url;
3327

@@ -194,6 +188,19 @@ declare_clippy_lint! {
194188
/// }
195189
/// }
196190
/// ```
191+
///
192+
/// Individual panics within a function can be ignored with `#[expect]` or
193+
/// `#[allow]`:
194+
///
195+
/// ```no_run
196+
/// # use std::num::NonZeroUsize;
197+
/// pub fn will_not_panic(x: usize) {
198+
/// #[expect(clippy::missing_panics_doc, reason = "infallible")]
199+
/// let y = NonZeroUsize::new(1).unwrap();
200+
///
201+
/// // If any panics are added in the future the lint will still catch them
202+
/// }
203+
/// ```
197204
#[clippy::version = "1.51.0"]
198205
pub MISSING_PANICS_DOC,
199206
pedantic,
@@ -657,20 +664,16 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
657664
self.check_private_items,
658665
);
659666
match item.kind {
660-
ItemKind::Fn { sig, body: body_id, .. } => {
667+
ItemKind::Fn { sig, body, .. } => {
661668
if !(is_entrypoint_fn(cx, item.owner_id.to_def_id())
662669
|| item.span.in_external_macro(cx.tcx.sess.source_map()))
663670
{
664-
let body = cx.tcx.hir_body(body_id);
665-
666-
let panic_info = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
667671
missing_headers::check(
668672
cx,
669673
item.owner_id,
670674
sig,
671675
headers,
672-
Some(body_id),
673-
panic_info,
676+
Some(body),
674677
self.check_private_items,
675678
);
676679
}
@@ -697,32 +700,20 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
697700
if let TraitItemKind::Fn(sig, ..) = trait_item.kind
698701
&& !trait_item.span.in_external_macro(cx.tcx.sess.source_map())
699702
{
700-
missing_headers::check(
701-
cx,
702-
trait_item.owner_id,
703-
sig,
704-
headers,
705-
None,
706-
None,
707-
self.check_private_items,
708-
);
703+
missing_headers::check(cx, trait_item.owner_id, sig, headers, None, self.check_private_items);
709704
}
710705
},
711706
Node::ImplItem(impl_item) => {
712707
if let ImplItemKind::Fn(sig, body_id) = impl_item.kind
713708
&& !impl_item.span.in_external_macro(cx.tcx.sess.source_map())
714709
&& !is_trait_impl_item(cx, impl_item.hir_id())
715710
{
716-
let body = cx.tcx.hir_body(body_id);
717-
718-
let panic_span = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(impl_item.owner_id), body.value);
719711
missing_headers::check(
720712
cx,
721713
impl_item.owner_id,
722714
sig,
723715
headers,
724716
Some(body_id),
725-
panic_span,
726717
self.check_private_items,
727718
);
728719
}
@@ -1168,71 +1159,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
11681159
headers
11691160
}
11701161

1171-
struct FindPanicUnwrap<'a, 'tcx> {
1172-
cx: &'a LateContext<'tcx>,
1173-
is_const: bool,
1174-
panic_span: Option<Span>,
1175-
typeck_results: &'tcx ty::TypeckResults<'tcx>,
1176-
}
1177-
1178-
impl<'a, 'tcx> FindPanicUnwrap<'a, 'tcx> {
1179-
pub fn find_span(
1180-
cx: &'a LateContext<'tcx>,
1181-
typeck_results: &'tcx ty::TypeckResults<'tcx>,
1182-
body: impl Visitable<'tcx>,
1183-
) -> Option<(Span, bool)> {
1184-
let mut vis = Self {
1185-
cx,
1186-
is_const: false,
1187-
panic_span: None,
1188-
typeck_results,
1189-
};
1190-
body.visit(&mut vis);
1191-
vis.panic_span.map(|el| (el, vis.is_const))
1192-
}
1193-
}
1194-
1195-
impl<'tcx> Visitor<'tcx> for FindPanicUnwrap<'_, 'tcx> {
1196-
type NestedFilter = nested_filter::OnlyBodies;
1197-
1198-
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
1199-
if self.panic_span.is_some() {
1200-
return;
1201-
}
1202-
1203-
if let Some(macro_call) = root_macro_call_first_node(self.cx, expr)
1204-
&& (is_panic(self.cx, macro_call.def_id)
1205-
|| matches!(
1206-
self.cx.tcx.item_name(macro_call.def_id).as_str(),
1207-
"assert" | "assert_eq" | "assert_ne"
1208-
))
1209-
{
1210-
self.is_const = self.cx.tcx.hir_is_inside_const_context(expr.hir_id);
1211-
self.panic_span = Some(macro_call.span);
1212-
}
1213-
1214-
// check for `unwrap` and `expect` for both `Option` and `Result`
1215-
if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or(method_chain_args(expr, &["expect"])) {
1216-
let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs();
1217-
if is_type_diagnostic_item(self.cx, receiver_ty, sym::Option)
1218-
|| is_type_diagnostic_item(self.cx, receiver_ty, sym::Result)
1219-
{
1220-
self.panic_span = Some(expr.span);
1221-
}
1222-
}
1223-
1224-
// and check sub-expressions
1225-
intravisit::walk_expr(self, expr);
1226-
}
1227-
1228-
// Panics in const blocks will cause compilation to fail.
1229-
fn visit_anon_const(&mut self, _: &'tcx AnonConst) {}
1230-
1231-
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
1232-
self.cx.tcx
1233-
}
1234-
}
1235-
12361162
#[expect(clippy::range_plus_one)] // inclusive ranges aren't the same type
12371163
fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> {
12381164
if range.end < range.start {

tests/ui/missing_panics_doc.rs

+39
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,45 @@ pub fn debug_assertions() {
151151
debug_assert_ne!(1, 2);
152152
}
153153

154+
pub fn partially_const<const N: usize>(n: usize) {
155+
//~^ missing_panics_doc
156+
157+
const {
158+
assert!(N > 5);
159+
}
160+
161+
assert!(N > n);
162+
}
163+
164+
pub fn expect_allow(i: Option<isize>) {
165+
#[expect(clippy::missing_panics_doc)]
166+
i.unwrap();
167+
168+
#[allow(clippy::missing_panics_doc)]
169+
i.unwrap();
170+
}
171+
172+
pub fn expect_allow_with_error(i: Option<isize>) {
173+
//~^ missing_panics_doc
174+
175+
#[expect(clippy::missing_panics_doc)]
176+
i.unwrap();
177+
178+
#[allow(clippy::missing_panics_doc)]
179+
i.unwrap();
180+
181+
i.unwrap();
182+
}
183+
184+
pub fn expect_after_error(x: Option<u32>, y: Option<u32>) {
185+
//~^ missing_panics_doc
186+
187+
let x = x.unwrap();
188+
189+
#[expect(clippy::missing_panics_doc)]
190+
let y = y.unwrap();
191+
}
192+
154193
// all function must be triggered the lint.
155194
// `pub` is required, because the lint does not consider unreachable items
156195
pub mod issue10240 {

0 commit comments

Comments
 (0)