diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/missing_assert_message.rs | 123 | ||||
| -rw-r--r-- | tests/ui/filter_map_next_fixable.fixed | 2 | ||||
| -rw-r--r-- | tests/ui/filter_map_next_fixable.rs | 2 | ||||
| -rw-r--r-- | tests/ui/missing_assert_message.rs | 84 | ||||
| -rw-r--r-- | tests/ui/missing_assert_message.stderr | 100 |
8 files changed, 313 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9508ab57cb8..62cc7437b82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4706,6 +4706,7 @@ Released 2018-09-13 [`mismatching_type_param_order`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatching_type_param_order [`misnamed_getters`]: https://rust-lang.github.io/rust-clippy/master/index.html#misnamed_getters [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op +[`missing_assert_message`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_assert_message [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items [`missing_enforced_import_renames`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_enforced_import_renames diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 470a2e79e47..462fcb6483d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -417,6 +417,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::misc_early::UNSEPARATED_LITERAL_SUFFIX_INFO, crate::misc_early::ZERO_PREFIXED_LITERAL_INFO, crate::mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER_INFO, + crate::missing_assert_message::MISSING_ASSERT_MESSAGE_INFO, crate::missing_const_for_fn::MISSING_CONST_FOR_FN_INFO, crate::missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS_INFO, crate::missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ae7fdd6be26..fe0229a814f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -193,6 +193,7 @@ mod minmax; mod misc; mod misc_early; mod mismatching_type_param_order; +mod missing_assert_message; mod missing_const_for_fn; mod missing_doc; mod missing_enforced_import_rename; @@ -926,6 +927,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi)); store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead)); + store.register_pre_expansion_pass(|| Box::new(missing_assert_message::MissingAssertMessage)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/missing_assert_message.rs b/clippy_lints/src/missing_assert_message.rs new file mode 100644 index 00000000000..a884b22370d --- /dev/null +++ b/clippy_lints/src/missing_assert_message.rs @@ -0,0 +1,123 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_ast::ast; +use rustc_ast::{ + token::{Token, TokenKind}, + tokenstream::TokenTree, +}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks assertions that doesn't have a custom panic message. + /// + /// ### Why is this bad? + /// If the assertion fails, a custom message may make it easier to debug what went wrong. + /// + /// ### Example + /// ```rust + /// let threshold = 50; + /// let num = 42; + /// assert!(num < threshold); + /// ``` + /// Use instead: + /// ```rust + /// let threshold = 50; + /// let num = 42; + /// assert!(num < threshold, "{num} is lower than threshold ({threshold})"); + /// ``` + #[clippy::version = "1.69.0"] + pub MISSING_ASSERT_MESSAGE, + pedantic, + "checks assertions that doesn't have a custom panic message" +} + +#[derive(Default, Clone, Debug)] +pub struct MissingAssertMessage { + // This field will be greater than zero if we are inside a `#[test]` or `#[cfg(test)]` + test_deepnes: usize, +} + +impl_lint_pass!(MissingAssertMessage => [MISSING_ASSERT_MESSAGE]); + +impl EarlyLintPass for MissingAssertMessage { + fn check_mac(&mut self, cx: &EarlyContext<'_>, mac_call: &ast::MacCall) { + if self.test_deepnes != 0 { + return; + } + + let Some(last_segment) = mac_call.path.segments.last() else { return; }; + let num_separators_needed = match last_segment.ident.as_str() { + "assert" | "debug_assert" => 1, + "assert_eq" | "assert_ne" | "debug_assert_eq" | "debug_assert_ne" => 2, + _ => return, + }; + let num_separators = num_commas_on_arguments(mac_call); + + if num_separators < num_separators_needed { + span_lint( + cx, + MISSING_ASSERT_MESSAGE, + mac_call.span(), + "assert without any message", + ); + } + } + + fn check_item(&mut self, _: &EarlyContext<'_>, item: &ast::Item) { + if item.attrs.iter().any(is_a_test_attribute) { + self.test_deepnes += 1; + } + } + + fn check_item_post(&mut self, _: &EarlyContext<'_>, item: &ast::Item) { + if item.attrs.iter().any(is_a_test_attribute) { + self.test_deepnes -= 1; + } + } +} + +// Returns number of commas (excluding trailing comma) from `MacCall`'s arguments. +fn num_commas_on_arguments(mac_call: &ast::MacCall) -> usize { + let mut num_separators = 0; + let mut is_trailing = false; + for tt in mac_call.args.tokens.trees() { + match tt { + TokenTree::Token( + Token { + kind: TokenKind::Comma, + span: _, + }, + _, + ) => { + num_separators += 1; + is_trailing = true; + }, + _ => { + is_trailing = false; + }, + } + } + if is_trailing { + num_separators -= 1; + } + num_separators +} + +// Returns true if the attribute is either a `#[test]` or a `#[cfg(test)]`. +fn is_a_test_attribute(attr: &ast::Attribute) -> bool { + if attr.has_name(sym::test) { + return true; + } + + if attr.has_name(sym::cfg) + && let Some(items) = attr.meta_item_list() + && let [item] = &*items + && item.has_name(sym::test) + { + true + } else { + false + } +} diff --git a/tests/ui/filter_map_next_fixable.fixed b/tests/ui/filter_map_next_fixable.fixed index 462d46169fc..0568eff41b5 100644 --- a/tests/ui/filter_map_next_fixable.fixed +++ b/tests/ui/filter_map_next_fixable.fixed @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::all, clippy::pedantic)] -#![allow(unused)] +#![allow(unused, clippy::missing_assert_message)] fn main() { let a = ["1", "lol", "3", "NaN", "5"]; diff --git a/tests/ui/filter_map_next_fixable.rs b/tests/ui/filter_map_next_fixable.rs index 2ea00cf7307..b0722ee8258 100644 --- a/tests/ui/filter_map_next_fixable.rs +++ b/tests/ui/filter_map_next_fixable.rs @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::all, clippy::pedantic)] -#![allow(unused)] +#![allow(unused, clippy::missing_assert_message)] fn main() { let a = ["1", "lol", "3", "NaN", "5"]; diff --git a/tests/ui/missing_assert_message.rs b/tests/ui/missing_assert_message.rs new file mode 100644 index 00000000000..89404ca8827 --- /dev/null +++ b/tests/ui/missing_assert_message.rs @@ -0,0 +1,84 @@ +#![allow(unused)] +#![warn(clippy::missing_assert_message)] + +macro_rules! bar { + ($( $x:expr ),*) => { + foo() + }; +} + +fn main() {} + +// Should trigger warning +fn asserts_without_message() { + assert!(foo()); + assert_eq!(foo(), foo()); + assert_ne!(foo(), foo()); + debug_assert!(foo()); + debug_assert_eq!(foo(), foo()); + debug_assert_ne!(foo(), foo()); +} + +// Should trigger warning +fn asserts_without_message_but_with_macro_calls() { + assert!(bar!(true)); + assert!(bar!(true, false)); + assert_eq!(bar!(true), foo()); + assert_ne!(bar!(true, true), bar!(true)); +} + +// Should trigger warning +fn asserts_with_trailing_commas() { + assert!(foo(),); + assert_eq!(foo(), foo(),); + assert_ne!(foo(), foo(),); + debug_assert!(foo(),); + debug_assert_eq!(foo(), foo(),); + debug_assert_ne!(foo(), foo(),); +} + +// Should not trigger warning +fn asserts_with_message_and_with_macro_calls() { + assert!(bar!(true), "msg"); + assert!(bar!(true, false), "msg"); + assert_eq!(bar!(true), foo(), "msg"); + assert_ne!(bar!(true, true), bar!(true), "msg"); +} + +// Should not trigger warning +fn asserts_with_message() { + assert!(foo(), "msg"); + assert_eq!(foo(), foo(), "msg"); + assert_ne!(foo(), foo(), "msg"); + debug_assert!(foo(), "msg"); + debug_assert_eq!(foo(), foo(), "msg"); + debug_assert_ne!(foo(), foo(), "msg"); +} + +// Should not trigger warning +#[test] +fn asserts_without_message_but_inside_a_test_function() { + assert!(foo()); + assert_eq!(foo(), foo()); + assert_ne!(foo(), foo()); + debug_assert!(foo()); + debug_assert_eq!(foo(), foo()); + debug_assert_ne!(foo(), foo()); +} + +// Should not trigger warning +#[cfg(test)] +mod tests { + fn asserts_without_message_but_inside_a_test_module() { + assert!(foo()); + assert_eq!(foo(), foo()); + assert_ne!(foo(), foo()); + debug_assert!(foo()); + debug_assert_eq!(foo(), foo()); + debug_assert_ne!(foo(), foo()); + } +} + +fn foo() -> bool { + true +} diff --git a/tests/ui/missing_assert_message.stderr b/tests/ui/missing_assert_message.stderr new file mode 100644 index 00000000000..900966500c8 --- /dev/null +++ b/tests/ui/missing_assert_message.stderr @@ -0,0 +1,100 @@ +error: assert without any message + --> $DIR/missing_assert_message.rs:14:5 + | +LL | assert!(foo()); + | ^^^^^^^^^^^^^^ + | + = note: `-D clippy::missing-assert-message` implied by `-D warnings` + +error: assert without any message + --> $DIR/missing_assert_message.rs:15:5 + | +LL | assert_eq!(foo(), foo()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:16:5 + | +LL | assert_ne!(foo(), foo()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:17:5 + | +LL | debug_assert!(foo()); + | ^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:18:5 + | +LL | debug_assert_eq!(foo(), foo()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:19:5 + | +LL | debug_assert_ne!(foo(), foo()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:24:5 + | +LL | assert!(bar!(true)); + | ^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:25:5 + | +LL | assert!(bar!(true, false)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:26:5 + | +LL | assert_eq!(bar!(true), foo()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:27:5 + | +LL | assert_ne!(bar!(true, true), bar!(true)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:32:5 + | +LL | assert!(foo(),); + | ^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:33:5 + | +LL | assert_eq!(foo(), foo(),); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:34:5 + | +LL | assert_ne!(foo(), foo(),); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:35:5 + | +LL | debug_assert!(foo(),); + | ^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:36:5 + | +LL | debug_assert_eq!(foo(), foo(),); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assert without any message + --> $DIR/missing_assert_message.rs:37:5 + | +LL | debug_assert_ne!(foo(), foo(),); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 16 previous errors + |