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
This commit is contained in:
commit
b96fecfee9
4 changed files with 153 additions and 111 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue