feat: implement test_without_fail case lint

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
@@ -6215,6 +6216,9 @@ Released 2018-09-13
 [`standard-macro-braces`]: https://doc.rust-lang.org/clippy/lint_configuration.html#standard-macro-braces
 [`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
 [`suppress-restriction-lint-in-const`]: https://doc.rust-lang.org/clippy/lint_configuration.html#suppress-restriction-lint-in-const
+[`test-without-fail-case-fallible-paths`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-fallible-paths
+[`test-without-fail-case-include-indexing-as-fallible`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-include-indexing-as-fallible
+[`test-without-fail-case-non-fallible-paths`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-non-fallible-paths
 [`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack
 [`too-many-arguments-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-arguments-threshold
 [`too-many-lines-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-lines-threshold

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

book/src/lint_configuration.md

@@ -831,6 +831,52 @@ if no suggestion can be made.
 * [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)
 
 
+## `test-without-fail-case-fallible-paths`
+List of full paths of macros and functions, that can fail. If a test, or a function
+that the test calls contains a call to any one of these, lint will mark the test fallible.
+
+**Default Value:** `["core::panic", "core::assert", "core::assert_eq", "core::assert_ne"]`
+
+---
+**Affected lints:**
+* [`test_without_fail_case`](https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case)
+
+
+## `test-without-fail-case-include-indexing-as-fallible`
+Whether to consider indexing as a fallible operation while assesing if a test can fail.
+Indexing is fallible, and thus the a test that is doing that can fail but it is likely
+that tests that fail this way were not intended.
+
+If set true, the lint will consider indexing into a slice a failable case
+and won't lint tests that has some sort of indexing. This analysis still done
+in a interprocedural manner. Meaning that any indexing opeartion done inside of
+a function that the test calls will still result the test getting marked fallible.
+
+By default this is set to `false`. That is because from a usability perspective,
+indexing an array is not the intended way to fail a test. So setting this `true`
+reduces false positives but makes the analysis more focused on possible byproducts
+of a test. That is the set of operations to get the point we assert something rather
+than the existance of asserting that thing.
+
+**Default Value:** `false`
+
+---
+**Affected lints:**
+* [`test_without_fail_case`](https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case)
+
+
+## `test-without-fail-case-non-fallible-paths`
+List of full paths of macros and functions, that we want to mark as "not going to fail".
+This allows us to make the lint more focused on actual short comings of our test suite
+by marking common routines non-fallible, even though they are fallible.
+
+**Default Value:** `["std::print", "std::println", "std::dbg", "std::eprint", "std::eprintln"]`
+
+---
+**Affected lints:**
+* [`test_without_fail_case`](https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case)
+
+
 ## `too-large-for-stack`
 The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap
 

clippy_config/src/conf.rs

@@ -46,6 +46,12 @@ const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z
 const DEFAULT_ALLOWED_PREFIXES: &[&str] = &["to", "as", "into", "from", "try_into", "try_from"];
 const DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS: &[&str] =
     &["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"];
+/// Default paths considered as fallible for `test_without_fail_case` lint.
+pub(crate) const DEFAULT_FALLIBLE_PATHS: &[&str] =
+    &["core::panic", "core::assert", "core::assert_eq", "core::assert_ne"];
+/// Default paths considered as non-fallible for `test_without_fail_case` lint.
+pub(crate) const DEFAULT_NONFALLIBLE_PATHS: &[&str] =
+    &["std::print", "std::println", "std::dbg", "std::eprint", "std::eprintln"];
 
 /// Conf with parse errors
 #[derive(Default)]
@@ -628,6 +634,33 @@ define_Conf! {
     /// if no suggestion can be made.
     #[lints(indexing_slicing)]
     suppress_restriction_lint_in_const: bool = false,
+    /// List of full paths of macros and functions, that can fail. If a test, or a function
+    /// that the test calls contains a call to any one of these, lint will mark the test fallible.
+    #[lints(test_without_fail_case)]
+    test_without_fail_case_fallible_paths: Vec<String> =
+        DEFAULT_FALLIBLE_PATHS.iter().map(ToString::to_string).collect(),
+    /// Whether to consider indexing as a fallible operation while assesing if a test can fail.
+    /// Indexing is fallible, and thus the a test that is doing that can fail but it is likely
+    /// that tests that fail this way were not intended.
+    ///
+    /// If set true, the lint will consider indexing into a slice a failable case
+    /// and won't lint tests that has some sort of indexing. This analysis still done
+    /// in a interprocedural manner. Meaning that any indexing opeartion done inside of
+    /// a function that the test calls will still result the test getting marked fallible.
+    ///
+    /// By default this is set to `false`. That is because from a usability perspective,
+    /// indexing an array is not the intended way to fail a test. So setting this `true`
+    /// reduces false positives but makes the analysis more focused on possible byproducts
+    /// of a test. That is the set of operations to get the point we assert something rather
+    /// than the existance of asserting that thing.
+    #[lints(test_without_fail_case)]
+    test_without_fail_case_include_indexing_as_fallible: bool = false,
+    /// List of full paths of macros and functions, that we want to mark as "not going to fail".
+    /// This allows us to make the lint more focused on actual short comings of our test suite
+    /// by marking common routines non-fallible, even though they are fallible.
+    #[lints(test_without_fail_case)]
+    test_without_fail_case_non_fallible_paths: Vec<String> =
+        DEFAULT_NONFALLIBLE_PATHS.iter().map(ToString::to_string).collect(),
     /// The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap
     #[lints(boxed_local, useless_vec)]
     too_large_for_stack: u64 = 200,
@@ -724,6 +757,14 @@ pub fn lookup_conf_file() -> io::Result<(Option<PathBuf>, Vec<String>)> {
 fn deserialize(file: &SourceFile) -> TryConf {
     match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(file)) {
         Ok(mut conf) => {
+            extend_vec_if_indicator_present(
+                &mut conf.conf.test_without_fail_case_fallible_paths,
+                DEFAULT_FALLIBLE_PATHS,
+            );
+            extend_vec_if_indicator_present(
+                &mut conf.conf.test_without_fail_case_non_fallible_paths,
+                DEFAULT_NONFALLIBLE_PATHS,
+            );
             extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
             extend_vec_if_indicator_present(&mut conf.conf.allowed_prefixes, DEFAULT_ALLOWED_PREFIXES);
             extend_vec_if_indicator_present(

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::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }