Add missing_assert_message lint

Co-authored-by: Weihang Lo <me@weihanglo.tw>
This commit is contained in:
Burak Varlı 2023-02-16 14:50:51 +00:00 committed by unexge
parent 41fa24cef8
commit ea2547b8c6
8 changed files with 313 additions and 2 deletions

View File

@ -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

View File

@ -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,

View File

@ -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`
}

View File

@ -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
}
}

View File

@ -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"];

View File

@ -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"];

View File

@ -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
}

View File

@ -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