Auto merge of #11204 - y21:issue10956, r=Alexendoo

new lint: [`should_panic_without_expect`]

Closes #10956

changelog: new lint: [`should_panic_without_expect`]

Author: bors

CHANGELOG.md

@@ -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

clippy_lints/src/attrs.rs

@@ -6,7 +6,11 @@ use clippy_utils::macros::{is_panic, macro_backtrace};
 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 @@ declare_clippy_lint! {
     "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 @@ declare_lint_pass!(Attributes => [
     DEPRECATED_SEMVER,
     USELESS_ATTRIBUTE,
     BLANKET_CLIPPY_RESTRICTION_LINTS,
+    SHOULD_PANIC_WITHOUT_EXPECT,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Attributes {
@@ -442,6 +482,9 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
                 }
             }
         }
+        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) {

clippy_lints/src/declared_lints.rs

@@ -58,6 +58,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     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,

tests/ui/should_panic_without_expect.rs

@@ -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() {}

tests/ui/should_panic_without_expect.stderr

@@ -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
+