diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bdbc91db939..a3b814bcdc16 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index ec76a6dfb08e..1690e2beb16f 100644 --- a/README.md +++ b/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. diff --git a/book/src/README.md b/book/src/README.md index 7bdfb97c3acf..23527ba896af 100644 --- a/book/src/README.md +++ b/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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 43b551ae2161..351e0dd33b03 100644 --- a/book/src/lint_configuration.md +++ b/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 diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 4757c0b1339a..8b59d9718753 100644 --- a/clippy_config/src/conf.rs +++ b/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 = + 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 = + 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, Vec)> { 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( diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d8918d37afa9..42252de3209e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a0ff8316d5cd..c649f3097f70 100644 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/clippy_lints/src/test_without_fail_case.rs b/clippy_lints/src/test_without_fail_case.rs new file mode 100644 index 000000000000..e11381446430 --- /dev/null +++ b/clippy_lints/src/test_without_fail_case.rs @@ -0,0 +1,270 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::macros::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_data_structures::fx::FxHashSet; +use rustc_hir::def::Res; +use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::{self, Visitor}; +use rustc_hir::{AnonConst, Expr, ExprKind, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty; +use rustc_session::impl_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for test functions that cannot fail. + /// + /// ### Why is this bad? + /// A test function that cannot fail might indicate that it does not actually test anything. + /// It could lead to false positives in test suites, giving a false sense of security. + /// + /// ### Example + /// ```rust + /// #[test] + /// fn my_test() { + /// let x = 2; + /// let y = 2; + /// if x + y != 4 { + /// eprintln!("this is not a correct test") + /// } + /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// #[test] + /// fn my_test() { + /// let x = 2; + /// let y = 2; + /// assert_eq!(x + y, 4); + /// } + /// ``` + #[clippy::version = "1.82.0"] + pub TEST_WITHOUT_FAIL_CASE, + restriction, + "test function cannot fail because it does not anyway to panic or assert" +} + +pub struct TestWithoutFailCase { + config: SearchConfig, +} + +impl TestWithoutFailCase { + pub fn new(conf: &Conf) -> Self { + Self { + config: SearchConfig { + indexing_fallible: conf.test_without_fail_case_include_indexing_as_fallible, + fallible_paths: conf.test_without_fail_case_fallible_paths.iter().cloned().collect(), + non_fallible_paths: conf.test_without_fail_case_non_fallible_paths.iter().cloned().collect(), + }, + } + } +} + +impl_lint_pass!(TestWithoutFailCase => [TEST_WITHOUT_FAIL_CASE]); + +impl<'tcx> LateLintPass<'tcx> for TestWithoutFailCase { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + // Only interested in functions that are annotated with `#[test]`. + 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 typeck_results = cx.tcx.typeck(item.owner_id); + let fail_found = SearchFailIntraFunction::find_fail(cx, typeck_results, &self.config, body); + if !fail_found { + // No way to panic for this test function + span_lint_and_note( + cx, + TEST_WITHOUT_FAIL_CASE, + item.span, + "this function marked with `#[test]` cannot fail and will always succeed", + None, + "consider adding assertions or panics to test failure cases", + ); + } + } + } +} + +/// Set of options that user provivdes through configs, to modify the lint behaviour +/// according to their repo. +struct SearchConfig { + /// If search should consider indexing as fallible. + indexing_fallible: bool, + /// Set of paths that are marked as fallible. + fallible_paths: FxHashSet, + /// Set of paths that are marked as non fallible. + non_fallible_paths: FxHashSet, +} + +/// Visitor that searches for expressions that could cause a panic, such as `panic!`, +/// `assert!`, `unwrap()`, or calls to functions that can panic. +struct SearchFailIntraFunction<'a, 'tcx> { + /// The lint context + cx: &'a LateContext<'tcx>, + /// Whether a way to fail is found or not. + fail_found: bool, + /// Type checking results for the current body + typeck_results: &'tcx ty::TypeckResults<'tcx>, + /// Set of function `DefId`s that have been visited to avoid infinite recursion + visited_functions: FxHashSet, + /// Search configs containing the set of user provided configurations. + search_config: &'a SearchConfig, +} + +impl<'a, 'tcx> SearchFailIntraFunction<'a, 'tcx> { + pub fn new( + cx: &'a LateContext<'tcx>, + typeck_results: &'tcx ty::TypeckResults<'tcx>, + search_config: &'a SearchConfig, + ) -> Self { + Self { + cx, + fail_found: false, + typeck_results, + visited_functions: FxHashSet::default(), + search_config, + } + } + + /// Searches for a way to panic in the given body and returns the span if found + pub fn find_fail( + cx: &'a LateContext<'tcx>, + typeck_results: &'tcx ty::TypeckResults<'tcx>, + search_config: &'a SearchConfig, + body: impl Visitable<'tcx>, + ) -> bool { + let mut visitor = Self::new(cx, typeck_results, search_config); + body.visit(&mut visitor); + visitor.fail_found + } + + /// Checks the called function to see if it contains a panic + fn check_called_function(&mut self, def_id: DefId) { + // Avoid infinite recursion by checking if we've already visited this function + if !self.visited_functions.insert(def_id) { + return; + } + + if def_id.is_local() { + let hir = self.cx.tcx.hir(); + if let Some(local_def_id) = def_id.as_local() { + if let Some(body) = hir.maybe_body_owned_by(local_def_id) { + let typeck_results = self.cx.tcx.typeck(local_def_id); + let mut new_visitor = SearchFailIntraFunction { + cx: self.cx, + fail_found: false, + typeck_results, + visited_functions: self.visited_functions.clone(), + search_config: &self.search_config, + }; + body.visit(&mut new_visitor); + if new_visitor.fail_found { + self.fail_found = true; + } + } + } + } else { + // For external functions, assume they can panic + self.fail_found = true; + } + } +} + +impl<'tcx> Visitor<'tcx> for SearchFailIntraFunction<'_, 'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if self.fail_found { + // If we've already found a panic, no need to continue + return; + } + + match expr.kind { + ExprKind::Index(slice_expr, index_expr, _) => { + // If indexing into slices is considered fallible, we treat it as a potential failure point + if self.search_config.indexing_fallible { + self.fail_found = true; + return; + } + self.visit_expr(slice_expr); + self.visit_expr(index_expr); + }, + ExprKind::Call(callee, args) => { + if let ExprKind::Path(ref qpath) = callee.kind { + if let Res::Def(_, def_id) = self.cx.qpath_res(qpath, callee.hir_id) { + self.check_called_function(def_id); + if self.fail_found { + return; + } + } + } + self.visit_expr(callee); + for arg in args { + self.visit_expr(arg); + } + }, + ExprKind::MethodCall(_, receiver, args, _) => { + if let Some(def_id) = self.typeck_results.type_dependent_def_id(expr.hir_id) { + self.check_called_function(def_id); + if self.fail_found { + return; + } + } + self.visit_expr(receiver); + for arg in args { + self.visit_expr(arg); + } + }, + _ => { + if let Some(macro_call) = root_macro_call_first_node(self.cx, expr) { + let macro_with_path = self.cx.tcx.def_path_str(macro_call.def_id); + // Skip macros that are defined as `non_fallible` in the clippy.toml file. + // Some examples that would fit here can be `println!`, `print!`, `eprintln!`, + // `eprint!`. This is a special case, these macros can panic, but it is very + // unlikely that this is intended as the tests assertion. In the name of + // reducing false negatives we are giving out soundness. + // + // This decision can be justified as it is highly unlikely that this lint is sound + // without this additional check, and with this we are reducing the number of false + // negatives. + if self.search_config.non_fallible_paths.contains(¯o_with_path) { + return; + } + + if self.search_config.fallible_paths.contains(¯o_with_path) { + self.fail_found = true; + return; + } + } + + // TODO: also make these two configurable. + // Check for `unwrap` and `expect` method calls + 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.fail_found = true; + return; + } + } + + intravisit::walk_expr(self, expr); + }, + } + } + + // Do not visit anonymous constants, as panics in const contexts are compile-time errors + fn visit_anon_const(&mut self, _: &'tcx AnonConst) {} + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } +} diff --git a/tests/ui-toml/test_without_fail_case/auxiliary/test_macro.rs b/tests/ui-toml/test_without_fail_case/auxiliary/test_macro.rs new file mode 100644 index 000000000000..3b6aa2ed9e00 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/auxiliary/test_macro.rs @@ -0,0 +1,14 @@ +#[macro_export] +macro_rules! fallible_macro { + ( $x:expr ) => {{ + let _ = $x; + panic!("a"); + }}; +} + +#[macro_export] +macro_rules! non_fallible_macro { + ( $x:expr ) => {{ + let _ = $x; + }}; +} diff --git a/tests/ui-toml/test_without_fail_case/default/clippy.toml b/tests/ui-toml/test_without_fail_case/default/clippy.toml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/ui-toml/test_without_fail_case/fail_macro/clippy.toml b/tests/ui-toml/test_without_fail_case/fail_macro/clippy.toml new file mode 100644 index 000000000000..c53d722edf29 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/fail_macro/clippy.toml @@ -0,0 +1 @@ +test-without-fail-case-fallible-paths = ["test_macro::non_fallible_macro"] diff --git a/tests/ui-toml/test_without_fail_case/index_fail/clippy.toml b/tests/ui-toml/test_without_fail_case/index_fail/clippy.toml new file mode 100644 index 000000000000..12772c384a58 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/index_fail/clippy.toml @@ -0,0 +1 @@ +test-without-fail-case-include-indexing-as-fallible = true diff --git a/tests/ui-toml/test_without_fail_case/no_fail_macro/clippy.toml b/tests/ui-toml/test_without_fail_case/no_fail_macro/clippy.toml new file mode 100644 index 000000000000..27f20201256e --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/no_fail_macro/clippy.toml @@ -0,0 +1 @@ +test-without-fail-case-non-fallible-paths = ["test_macro::fallible_macro"] diff --git a/tests/ui-toml/test_without_fail_case/test_without_fail_case.default.stderr b/tests/ui-toml/test_without_fail_case/test_without_fail_case.default.stderr new file mode 100644 index 000000000000..b9aa4ff502f4 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/test_without_fail_case.default.stderr @@ -0,0 +1,45 @@ +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1 + | +LL | / fn test_custom_macro_fallible() { +LL | | println!("a") +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + = 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]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1 + | +LL | / fn test_indexing_fallible() { +LL | | let a = [1, 2, 3]; +LL | | let _ = a[0]; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1 + | +LL | / fn func_a() { +LL | | let _ = 1; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1 + | +LL | / fn should_not_lint_if_defined_as_fallible() { +LL | | non_fallible_macro!(1); +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: aborting due to 4 previous errors + diff --git a/tests/ui-toml/test_without_fail_case/test_without_fail_case.fail_macro.stderr b/tests/ui-toml/test_without_fail_case/test_without_fail_case.fail_macro.stderr new file mode 100644 index 000000000000..4af5b5626162 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/test_without_fail_case.fail_macro.stderr @@ -0,0 +1,35 @@ +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1 + | +LL | / fn test_custom_macro_fallible() { +LL | | println!("a") +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + = 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]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1 + | +LL | / fn test_indexing_fallible() { +LL | | let a = [1, 2, 3]; +LL | | let _ = a[0]; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1 + | +LL | / fn func_a() { +LL | | let _ = 1; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: aborting due to 3 previous errors + diff --git a/tests/ui-toml/test_without_fail_case/test_without_fail_case.index_fail.stderr b/tests/ui-toml/test_without_fail_case/test_without_fail_case.index_fail.stderr new file mode 100644 index 000000000000..3ca082a7d288 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/test_without_fail_case.index_fail.stderr @@ -0,0 +1,34 @@ +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1 + | +LL | / fn test_custom_macro_fallible() { +LL | | println!("a") +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + = 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]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1 + | +LL | / fn func_a() { +LL | | let _ = 1; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1 + | +LL | / fn should_not_lint_if_defined_as_fallible() { +LL | | non_fallible_macro!(1); +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: aborting due to 3 previous errors + diff --git a/tests/ui-toml/test_without_fail_case/test_without_fail_case.no_fail_macro.stderr b/tests/ui-toml/test_without_fail_case/test_without_fail_case.no_fail_macro.stderr new file mode 100644 index 000000000000..b1d4693a16b2 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/test_without_fail_case.no_fail_macro.stderr @@ -0,0 +1,45 @@ +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1 + | +LL | / fn test_indexing_fallible() { +LL | | let a = [1, 2, 3]; +LL | | let _ = a[0]; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + = 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]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1 + | +LL | / fn func_a() { +LL | | let _ = 1; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1 + | +LL | / fn should_not_lint_if_defined_as_fallible() { +LL | | non_fallible_macro!(1); +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:37:1 + | +LL | / fn should_lint_if_defined_as_non_fallible() { +LL | | fallible_macro!(1); +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: aborting due to 4 previous errors + diff --git a/tests/ui-toml/test_without_fail_case/test_without_fail_case.rs b/tests/ui-toml/test_without_fail_case/test_without_fail_case.rs new file mode 100644 index 000000000000..7a5edc8dcad2 --- /dev/null +++ b/tests/ui-toml/test_without_fail_case/test_without_fail_case.rs @@ -0,0 +1,41 @@ +//@aux-build:test_macro.rs +//@compile-flags: --test +//@revisions: default fail_macro no_fail_macro index_fail +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/default +//@[fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/fail_macro +//@[no_fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/no_fail_macro +//@[index_fail] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/index_fail +#![allow(unused)] +#![allow(clippy::tests_outside_test_module)] +#![warn(clippy::test_without_fail_case)] +use test_macro::{fallible_macro, non_fallible_macro}; + +// Should not lint because of the clippy.toml file that adds `test` as fallible. +#[test] +fn test_custom_macro_fallible() { + println!("a") +} + +// Should not lint unless the clippy.toml file makes indexing fallible. +#[test] +fn test_indexing_fallible() { + let a = [1, 2, 3]; + let _ = a[0]; +} + +#[test] +fn func_a() { + let _ = 1; +} + +#[test] +fn should_not_lint_if_defined_as_fallible() { + non_fallible_macro!(1); +} + +#[test] +fn should_lint_if_defined_as_non_fallible() { + fallible_macro!(1); +} + +fn main() {} diff --git a/tests/ui/test_without_fail_case.rs b/tests/ui/test_without_fail_case.rs new file mode 100644 index 000000000000..e7862307f5c3 --- /dev/null +++ b/tests/ui/test_without_fail_case.rs @@ -0,0 +1,99 @@ +#![allow(unused)] +#![allow(clippy::tests_outside_test_module)] +#![allow(clippy::unnecessary_literal_unwrap)] +#![warn(clippy::test_without_fail_case)] + +struct DummyStruct; + +impl DummyStruct { + fn panic_in_impl(self) { + panic!("a") + } + + fn assert_in_impl(self, a: bool) { + assert!(a) + } + + fn unwrap_in_impl(self, a: Option) { + let _ = a.unwrap(); + } +} + +#[test] +fn test_without_fail() { + let x = 5; + let y = x + 2; + println!("y: {}", y); +} + +// Should not lint +#[test] +fn impl_panic() { + let dummy_struct = DummyStruct; + dummy_struct.panic_in_impl(); +} + +// Should not lint +#[test] +fn impl_assert() { + let dummy_struct = DummyStruct; + dummy_struct.assert_in_impl(false); +} + +// Should not lint +#[test] +fn impl_unwrap() { + let dummy_struct = DummyStruct; + let a = Some(10i32); + dummy_struct.unwrap_in_impl(a); +} + +// 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() +} + +// Should not lint +#[test] +fn test_implicit_unwrap() { + implicit_unwrap(); +} + +// Should not lint +#[test] +fn test_implicit_assert() { + implicit_assert(); +} + +// Should lint with default config. +#[test] +fn test_slice_index() { + let a = [1, 2, 3, 4, 5, 6]; + // indexing into slice, this can fail but by default check for this is disabled. + let b = a[0]; +} + +fn implicit_panic() { + panic!("this is an implicit panic"); +} + +fn implicit_unwrap() { + let val: Option = None; + let _ = val.unwrap(); +} + +fn implicit_assert() { + assert_eq!(1, 2) +} + +fn main() { + // Non-test code +} diff --git a/tests/ui/test_without_fail_case.stderr b/tests/ui/test_without_fail_case.stderr new file mode 100644 index 000000000000..003a165b70d7 --- /dev/null +++ b/tests/ui/test_without_fail_case.stderr @@ -0,0 +1,28 @@ +error: this function marked with `#[test]` cannot fail and will always succeed + --> tests/ui/test_without_fail_case.rs:23:1 + | +LL | / fn test_without_fail() { +LL | | let x = 5; +LL | | let y = x + 2; +LL | | println!("y: {}", y); +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + = 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]` cannot fail and will always succeed + --> tests/ui/test_without_fail_case.rs:78:1 + | +LL | / fn test_slice_index() { +LL | | let a = [1, 2, 3, 4, 5, 6]; +LL | | // indexing into slice, this can fail but by default check for this is disabled. +LL | | let b = a[0]; +LL | | } + | |_^ + | + = note: consider adding assertions or panics to test failure cases + +error: aborting due to 2 previous errors +