Skip to content

Commit

Permalink
feat: implement test_without_fail case lint
Browse files Browse the repository at this point in the history
  • Loading branch information
kayagokalp committed Oct 26, 2024
1 parent 5678531 commit 1ab9ae6
Show file tree
Hide file tree
Showing 20 changed files with 710 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion book/src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
41 changes: 41 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
Loading

0 comments on commit 1ab9ae6

Please sign in to comment.