rename lint and rewrite as late lint pass

CHANGELOG.md

@@ -5238,7 +5238,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_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_panic_without_reason
+[`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

@@ -1,10 +1,10 @@
 //! checks for attributes
 
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::is_from_proc_macro;
 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 clippy_utils::{is_from_proc_macro, match_def_path, paths};
 use if_chain::if_chain;
 use rustc_ast::token::{Token, TokenKind};
 use rustc_ast::tokenstream::TokenTree;
@@ -13,7 +13,7 @@ use rustc_ast::{
 };
 use rustc_errors::Applicability;
 use rustc_hir::{
-    Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind,
+    Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, QPath, StmtKind, TraitFn, TraitItem, TraitItemKind,
 };
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext};
 use rustc_middle::lint::in_external_macro;
@@ -373,9 +373,9 @@ declare_clippy_lint! {
     /// }
     /// ```
     #[clippy::version = "1.73.0"]
-    pub SHOULD_PANIC_WITHOUT_REASON,
+    pub SHOULD_PANIC_WITHOUT_EXPECT,
     pedantic,
-    "ensures that all `should_panic` attributes have a reason"
+    "ensures that all `should_panic` attributes specify its expected panic message"
 }
 
 declare_clippy_lint! {
@@ -434,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 {
@@ -557,6 +558,42 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
                     }
                 }
             },
+            ItemKind::Const(_, body_id) => {
+                // Looking for #[should_panic] attributes without a specified message means
+                // looking for the expanded #[test] const item.
+                //
+                // #[test] attributes expand to a const item that initializes `test::TestDescAndFn`.
+                //
+                // `#[test] #[should_panic] fn f() {}`
+                // becomes roughly:
+                // ```
+                // const f: test::TestDescAndFn = test::TestDescAndFn {
+                //   ...,
+                //   testfn: test::StaticTestFn(|| test::assert_result(f()))
+                // }
+                // ```
+                // The #[should_panic] attribute, along with its span, is on the `f`.
+
+                let body = cx.tcx.hir().body(body_id);
+
+                if let ExprKind::Struct(QPath::Resolved(_, path), fields, _) = body.value.kind
+                    && let Some(did) = path.res.opt_def_id()
+                    && match_def_path(cx, did, &paths::TEST_DESC_AND_FN)
+                    && let Some(testfn_field) = fields.iter().find(|field| field.ident.name == sym!(testfn))
+                    // test::StaticTestFn(|| ..)
+                    && let ExprKind::Call(_, [closure]) = testfn_field.expr.kind
+                    && let ExprKind::Closure(closure) = closure.kind
+                    // test::assert_result(f())
+                    //                     ^ get this `DefId` and its should_panic attr
+                    && let ExprKind::Call(_, [fn_call_closure]) = cx.tcx.hir().body(closure.body).value.kind
+                    && let ExprKind::Call(callee, _) = fn_call_closure.kind
+                    && let ExprKind::Path(QPath::Resolved(_, path)) = callee.kind
+                    && let Some(test_function_did) = path.res.opt_def_id()
+                    && let Some(attribute) = cx.tcx.get_attr(test_function_did, sym::should_panic)
+                {
+                    check_should_panic_reason(cx, attribute);
+                }
+            },
             _ => {},
         }
     }
@@ -589,6 +626,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) {
@@ -743,7 +809,6 @@ impl_lint_pass!(EarlyAttributes => [
     EMPTY_LINE_AFTER_DOC_COMMENTS,
     NON_MINIMAL_CFG,
     MAYBE_MISUSED_CFG,
-    SHOULD_PANIC_WITHOUT_REASON,
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
@@ -756,7 +821,6 @@ impl EarlyLintPass for EarlyAttributes {
         check_mismatched_target_os(cx, attr);
         check_minimal_cfg_condition(cx, attr);
         check_misused_cfg(cx, attr);
-        check_should_panic_reason(cx, attr);
     }
 
     extract_msrv_attr!(EarlyContext);
@@ -808,39 +872,6 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
     }
 }
 
-fn check_should_panic_reason<'cx>(cx: &EarlyContext<'cx>, attr: &'cx Attribute) {
-    if attr.has_name(sym::should_panic)
-        && let AttrKind::Normal(normal_attr) = &attr.kind
-    {
-        if let AttrArgs::Eq(_, AttrArgsEq::Ast(arg)) = &normal_attr.item.args
-            && let rustc_ast::ExprKind::Lit(_) = &arg.kind
-        {
-            // `#[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_REASON,
-            attr.span,
-            "#[should_panic] attribute without a reason",
-            "consider specifying the expected panic",
-            r#"#[should_panic(expected = /* panic message */)]"#.into(),
-            Applicability::HasPlaceholders
-        );
-    }
-}
-
 fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
     if_chain! {
         if msrv.meets(msrvs::TOOL_ATTRIBUTES);

clippy_lints/src/declared_lints.rs

@@ -58,7 +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_REASON_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,

clippy_utils/src/paths.rs

@@ -165,3 +165,5 @@ pub const DEBUG_STRUCT: [&str; 4] = ["core", "fmt", "builders", "DebugStruct"];
 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"];

tests/ui/should_panic_without_expect.rs

@@ -0,0 +1,20 @@
+#![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:4: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:1:9
+   |
+LL | #![deny(clippy::should_panic_without_expect)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+