feat: implement test without panic check

Author: kayagokalp

CHANGELOG.md

@@ -6004,6 +6004,7 @@ Released 2018-09-13
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
 [`test_attr_in_doctest`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_attr_in_doctest
+[`test_without_fail_case`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case
 [`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module
 [`thread_local_initializer_can_be_made_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const
 [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -6,7 +6,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_lints/src/declared_lints.rs

@@ -691,6 +691,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::swap_ptr_to_ref::SWAP_PTR_TO_REF_INFO,
     crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO,
     crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO,
+    crate::test_without_fail_case::TEST_WITHOUT_FAIL_CASE_INFO,
     crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
     crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
     crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,

clippy_lints/src/lib.rs

@@ -346,6 +346,7 @@ mod swap;
 mod swap_ptr_to_ref;
 mod tabs_in_doc_comments;
 mod temporary_assignment;
+mod test_without_fail_case;
 mod tests_outside_test_module;
 mod to_digit_is_some;
 mod to_string_trait_impl;
@@ -949,5 +950,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
     store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
+    store.register_late_pass(|_| Box::new(test_without_fail_case::TestWithoutFailCase));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/test_without_fail_case.rs

@@ -0,0 +1,157 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::{is_panic, root_macro_call_first_node};
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::visitors::Visitable;
+use clippy_utils::{is_in_test_function, method_chain_args};
+use rustc_hir::intravisit::{self, FnKind, Visitor};
+use rustc_hir::{AnonConst, Body, Expr, FnDecl, Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::nested_filter;
+use rustc_middle::ty;
+use rustc_session::declare_lint_pass;
+use rustc_span::{Span, sym};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Triggers when a testing function (marked with the `#[test]` attribute) does not have a way to fail.
+    ///
+    /// ### Why restrict this?
+    /// If a test does not have a way to fail, the developer might be getting false positives from their test suites.
+    /// The idiomatic way of using test functions should be such that they actually can fail in an erroneous state.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// #[test]
+    /// fn my_cool_test() {
+    ///     // [...]
+    /// }
+    ///
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     // [...]
+    /// }
+    ///
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// #[cfg(test)]
+    /// mod tests {
+    ///     #[test]
+    ///     fn my_cool_test() {
+    ///         // [...]
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub TEST_WITHOUT_FAIL_CASE,
+    restriction,
+    "A test function is outside the testing module."
+}
+
+declare_lint_pass!(TestWithoutFailCase => [TEST_WITHOUT_FAIL_CASE]);
+
+/*
+impl<'tcx> LateLintPass<'tcx> for TestWithoutFailCase {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::Item<'_>) {
+        if let rustc_hir::ItemKind::Fn(sig, _, body_id) = item.kind {
+
+        }
+        if is_in_test_function(cx.tcx, item.hir_id()) {
+            let typck = cx.tcx.typeck(item.id.owner_id.def_id);
+            let find_panic_visitor = FindPanicUnwrap::find_span(cx, typck, item.body)
+            span_lint(cx, TEST_WITHOUT_FAIL_CASE, item.span, "test function cannot panic");
+        }
+    }
+}
+    */
+
+impl<'tcx> LateLintPass<'tcx> for TestWithoutFailCase {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        if let ItemKind::Fn(_, _, body_id) = item.kind
+            && is_in_test_function(cx.tcx, item.hir_id())
+        {
+            let body = cx.tcx.hir().body(body_id);
+            let typ = cx.tcx.typeck(item.owner_id);
+            let panic_span = FindPanicUnwrap::find_span(cx, typ, body);
+            if panic_span.is_none() {
+                // No way to panic for this test.
+                #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
+                span_lint_and_then(
+                    cx,
+                    TEST_WITHOUT_FAIL_CASE,
+                    item.span,
+                    "this function marked with #[test] has no way to fail",
+                    |diag| {
+                        diag.note("make sure that something is checked in this test");
+                    },
+                );
+            }
+        }
+    }
+}
+
+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<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, '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) {
+            if 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 nested_visit_map(&mut self) -> Self::Map {
+        self.cx.tcx.hir()
+    }
+}

tests/ui/test_without_fail_case.rs

@@ -0,0 +1,45 @@
+#![allow(unused)]
+#![allow(clippy::tests_outside_test_module)]
+#![allow(clippy::unnecessary_literal_unwrap)]
+#![warn(clippy::test_without_fail_case)]
+
+// Should lint
+#[test]
+fn test_without_fail() {
+    // This test cannot fail.
+    let x = 5;
+    let y = x + 2;
+    println!("y: {}", y);
+}
+//~^ ERROR: this function marked with #[test] does not have a way to fail.
+//~^ NOTE: Ensure that something is being tested and asserted by this test.
+
+// Should not lint
+#[test]
+fn test_with_fail() {
+    // This test can fail.
+    assert_eq!(1 + 1, 2);
+}
+
+// Should not lint
+#[test]
+fn test_implicit_panic() {
+    implicit_panic()
+}
+
+fn implicit_panic() {
+    panic!("this is an implicit panic");
+}
+
+fn implicit_unwrap() {
+    let val: Option<u32> = None;
+    let _ = val.unwrap();
+}
+
+fn implicit_assert() {
+    assert_eq!(1, 2)
+}
+
+fn main() {
+    // Non-test code
+}

tests/ui/test_without_fail_case.stderr

@@ -0,0 +1,27 @@
+error: this function marked with #[test] has no way to fail
+  --> tests/ui/test_without_fail_case.rs:8:1
+   |
+LL | / fn test_without_fail() {
+LL | |     // This test cannot fail.
+LL | |     let x = 5;
+LL | |     let y = x + 2;
+LL | |     println!("y: {}", y);
+LL | | }
+   | |_^
+   |
+   = note: make sure that something is checked in this test
+   = note: `-D clippy::test-without-fail-case` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`
+
+error: this function marked with #[test] has no way to fail
+  --> tests/ui/test_without_fail_case.rs:26:1
+   |
+LL | / fn test_implicit_panic() {
+LL | |     implicit_panic()
+LL | | }
+   | |_^
+   |
+   = note: make sure that something is checked in this test
+
+error: aborting due to 2 previous errors
+