new lint: should_panic_without_expect

This commit is contained in:
y21 2023-07-21 11:23:31 +02:00
parent d9e6aac8bb
commit e52bd6f850
6 changed files with 112 additions and 1 deletions

View File

@ -5329,6 +5329,7 @@ Released 2018-09-13
[`short_circuit_statement`]: https://rust-lang.github.io/rust-clippy/master/index.html#short_circuit_statement
[`should_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_assert_eq
[`should_implement_trait`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait
[`should_panic_without_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_panic_without_expect
[`significant_drop_in_scrutinee`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_in_scrutinee
[`significant_drop_tightening`]: https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening
[`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names

View File

@ -6,7 +6,11 @@
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments};
use if_chain::if_chain;
use rustc_ast::{AttrKind, AttrStyle, Attribute, LitKind, MetaItemKind, MetaItemLit, NestedMetaItem};
use rustc_ast::token::{Token, TokenKind};
use rustc_ast::tokenstream::TokenTree;
use rustc_ast::{
AttrArgs, AttrArgsEq, AttrKind, AttrStyle, Attribute, LitKind, MetaItemKind, MetaItemLit, NestedMetaItem,
};
use rustc_errors::Applicability;
use rustc_hir::{
Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
@ -339,6 +343,41 @@
"ensures that all `allow` and `expect` attributes have a reason"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `#[should_panic]` attributes without specifying the expected panic message.
///
/// ### Why is this bad?
/// The expected panic message should be specified to ensure that the test is actually
/// panicking with the expected message, and not another unrelated panic.
///
/// ### Example
/// ```rust
/// fn random() -> i32 { 0 }
///
/// #[should_panic]
/// #[test]
/// fn my_test() {
/// let _ = 1 / random();
/// }
/// ```
///
/// Use instead:
/// ```rust
/// fn random() -> i32 { 0 }
///
/// #[should_panic = "attempt to divide by zero"]
/// #[test]
/// fn my_test() {
/// let _ = 1 / random();
/// }
/// ```
#[clippy::version = "1.73.0"]
pub SHOULD_PANIC_WITHOUT_EXPECT,
pedantic,
"ensures that all `should_panic` attributes specify its expected panic message"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `any` and `all` combinators in `cfg` with only one condition.
@ -395,6 +434,7 @@
DEPRECATED_SEMVER,
USELESS_ATTRIBUTE,
BLANKET_CLIPPY_RESTRICTION_LINTS,
SHOULD_PANIC_WITHOUT_EXPECT,
]);
impl<'tcx> LateLintPass<'tcx> for Attributes {
@ -442,6 +482,9 @@ fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) {
}
}
}
if attr.has_name(sym::should_panic) {
check_should_panic_reason(cx, attr);
}
}
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
@ -550,6 +593,35 @@ fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<Symbol> {
None
}
fn check_should_panic_reason(cx: &LateContext<'_>, attr: &Attribute) {
if let AttrKind::Normal(normal_attr) = &attr.kind {
if let AttrArgs::Eq(_, AttrArgsEq::Hir(_)) = &normal_attr.item.args {
// `#[should_panic = ".."]` found, good
return;
}
if let AttrArgs::Delimited(args) = &normal_attr.item.args
&& let mut tt_iter = args.tokens.trees()
&& let Some(TokenTree::Token(Token { kind: TokenKind::Ident(sym::expected, _), .. }, _)) = tt_iter.next()
&& let Some(TokenTree::Token(Token { kind: TokenKind::Eq, .. }, _)) = tt_iter.next()
&& let Some(TokenTree::Token(Token { kind: TokenKind::Literal(_), .. }, _)) = tt_iter.next()
{
// `#[should_panic(expected = "..")]` found, good
return;
}
span_lint_and_sugg(
cx,
SHOULD_PANIC_WITHOUT_EXPECT,
attr.span,
"#[should_panic] attribute without a reason",
"consider specifying the expected panic",
r#"#[should_panic(expected = /* panic message */)]"#.into(),
Applicability::HasPlaceholders,
);
}
}
fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem]) {
for lint in items {
if let Some(lint_name) = extract_clippy_lint(lint) {

View File

@ -58,6 +58,7 @@
crate::attrs::MAYBE_MISUSED_CFG_INFO,
crate::attrs::MISMATCHED_TARGET_OS_INFO,
crate::attrs::NON_MINIMAL_CFG_INFO,
crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO,
crate::attrs::USELESS_ATTRIBUTE_INFO,
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,

View File

@ -166,3 +166,5 @@
pub const ORD_CMP: [&str; 4] = ["core", "cmp", "Ord", "cmp"];
#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so
pub const BOOL_THEN: [&str; 4] = ["core", "bool", "<impl bool>", "then"];
#[allow(clippy::invalid_paths, reason = "internal lints do not always know about ::test")]
pub const TEST_DESC_AND_FN: [&str; 3] = ["test", "types", "TestDescAndFn"];

View File

@ -0,0 +1,21 @@
//@no-rustfix
#![deny(clippy::should_panic_without_expect)]
#[test]
#[should_panic]
fn no_message() {}
#[test]
#[should_panic]
#[cfg(not(test))]
fn no_message_cfg_false() {}
#[test]
#[should_panic = "message"]
fn metastr() {}
#[test]
#[should_panic(expected = "message")]
fn metalist() {}
fn main() {}

View File

@ -0,0 +1,14 @@
error: #[should_panic] attribute without a reason
--> $DIR/should_panic_without_expect.rs:5:1
|
LL | #[should_panic]
| ^^^^^^^^^^^^^^^ help: consider specifying the expected panic: `#[should_panic(expected = /* panic message */)]`
|
note: the lint level is defined here
--> $DIR/should_panic_without_expect.rs:2:9
|
LL | #![deny(clippy::should_panic_without_expect)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error