Skip to content

Commit 5856d0e

Browse files
committed
feat(manual_assert_eq): new lint
1 parent ad7fc4f commit 5856d0e

29 files changed

+462
-88
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6526,6 +6526,7 @@ Released 2018-09-13
65266526
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
65276527
[`manual_abs_diff`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff
65286528
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
6529+
[`manual_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq
65296530
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
65306531
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
65316532
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
292292
crate::main_recursion::MAIN_RECURSION_INFO,
293293
crate::manual_abs_diff::MANUAL_ABS_DIFF_INFO,
294294
crate::manual_assert::MANUAL_ASSERT_INFO,
295+
crate::manual_assert_eq::MANUAL_ASSERT_EQ_INFO,
295296
crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
296297
crate::manual_bits::MANUAL_BITS_INFO,
297298
crate::manual_clamp::MANUAL_CLAMP_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ mod macro_use;
195195
mod main_recursion;
196196
mod manual_abs_diff;
197197
mod manual_assert;
198+
mod manual_assert_eq;
198199
mod manual_async_fn;
199200
mod manual_bits;
200201
mod manual_clamp;
@@ -819,5 +820,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
819820
store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
820821
store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
821822
store.register_late_pass(|_| Box::new(replace_box::ReplaceBox::default()));
823+
store.register_late_pass(|_| Box::new(manual_assert_eq::ManualAssertEq));
822824
// add lints here, do not remove this comment, it's used in `new_lint`
823825
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::macros::{find_assert_args, root_macro_call_first_node};
3+
use clippy_utils::source::walk_span_to_context;
4+
use clippy_utils::{is_in_const_context, sym};
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{BinOpKind, Expr, ExprKind};
7+
use rustc_lint::{LateContext, LateLintPass, LintContext};
8+
use rustc_session::declare_lint_pass;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for `assert!` and `debug_assert!` that consist of only an (in)equality check
13+
///
14+
/// ### Why is this bad?
15+
/// `assert_{eq,ne}!` and `debug_assert_{eq,ne}!` achieves the same goal, and provides some
16+
/// additional debug information
17+
///
18+
/// ### Example
19+
/// ```no_run
20+
/// assert!(2 * 2 == 4);
21+
/// assert!(2 * 2 != 5);
22+
/// debug_assert!(2 * 2 == 4);
23+
/// debug_assert!(2 * 2 != 5);
24+
/// ```
25+
/// Use instead:
26+
/// ```no_run
27+
/// assert_eq!(2 * 2, 4);
28+
/// assert_ne!(2 * 2, 5);
29+
/// debug_assert_eq!(2 * 2, 4);
30+
/// debug_assert_ne!(2 * 2, 5);
31+
/// ```
32+
#[clippy::version = "1.92.0"]
33+
pub MANUAL_ASSERT_EQ,
34+
complexity,
35+
"checks for assertions consisting of an (in)equality check"
36+
}
37+
declare_lint_pass!(ManualAssertEq => [MANUAL_ASSERT_EQ]);
38+
39+
impl LateLintPass<'_> for ManualAssertEq {
40+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
41+
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
42+
&& let macro_name = cx.tcx.item_name(macro_call.def_id)
43+
&& matches!(macro_name, sym::assert | sym::debug_assert)
44+
// `assert_eq` isn't allowed in const context because it calls non-const `core::panicking::assert_failed`
45+
// XXX: this might change in the future, so might want to relax this restriction
46+
&& !is_in_const_context(cx)
47+
&& let Some((cond, _)) = find_assert_args(cx, expr, macro_call.expn)
48+
&& let ExprKind::Binary(op, lhs, rhs) = cond.kind
49+
&& matches!(op.node, BinOpKind::Eq | BinOpKind::Ne)
50+
&& !cond.span.from_expansion()
51+
{
52+
let macro_name = macro_name.as_str();
53+
span_lint_and_then(
54+
cx,
55+
MANUAL_ASSERT_EQ,
56+
macro_call.span,
57+
format!("used `{macro_name}!` with an equality comparison"),
58+
|diag| {
59+
let kind = if op.node == BinOpKind::Eq { "eq" } else { "ne" };
60+
let new_name = format!("{macro_name}_{kind}");
61+
let msg = format!("replace it with `{new_name}!(..)`");
62+
63+
let ctxt = cond.span.ctxt();
64+
if let Some(lhs_span) = walk_span_to_context(lhs.span, ctxt)
65+
&& let Some(rhs_span) = walk_span_to_context(rhs.span, ctxt)
66+
{
67+
let macro_name_span = cx.sess().source_map().span_until_char(macro_call.span, '!');
68+
let eq_span = cond.span.with_lo(lhs_span.hi()).with_hi(rhs_span.lo());
69+
let suggestions = vec![
70+
(macro_name_span.shrink_to_hi(), format!("_{kind}")),
71+
(eq_span, ", ".to_string()),
72+
];
73+
74+
diag.multipart_suggestion(msg, suggestions, Applicability::MachineApplicable);
75+
} else {
76+
diag.span_help(expr.span, msg);
77+
}
78+
},
79+
);
80+
}
81+
}
82+
}

clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ generate! {
133133
cycle,
134134
cyclomatic_complexity,
135135
de,
136+
debug_assert,
136137
deprecated_in_future,
137138
diagnostics,
138139
disallowed_types,

tests/ui/assertions_on_constants.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ fn main() {
5454
const _: () = assert!(true);
5555
//~^ assertions_on_constants
5656

57-
assert!(8 == (7 + 1));
58-
//~^ assertions_on_constants
57+
#[allow(clippy::manual_assert_eq, reason = "tests `assert!` specifically")]
58+
{
59+
assert!(8 == (7 + 1));
60+
//~^ assertions_on_constants
61+
}
5962

6063
// Don't lint if the value is dependent on a defined constant:
6164
const N: usize = 1024;
@@ -68,8 +71,11 @@ const _: () = {
6871
assert!(true);
6972
//~^ assertions_on_constants
7073

71-
assert!(8 == (7 + 1));
72-
//~^ assertions_on_constants
74+
#[allow(clippy::manual_assert_eq, reason = "tests `assert!` specifically")]
75+
{
76+
assert!(8 == (7 + 1));
77+
//~^ assertions_on_constants
78+
}
7379

7480
assert!(C);
7581
};

tests/ui/assertions_on_constants.stderr

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,47 +89,47 @@ LL | const _: () = assert!(true);
8989
= help: remove the assertion
9090

9191
error: this assertion is always `true`
92-
--> tests/ui/assertions_on_constants.rs:57:5
92+
--> tests/ui/assertions_on_constants.rs:59:9
9393
|
94-
LL | assert!(8 == (7 + 1));
95-
| ^^^^^^^^^^^^^^^^^^^^^
94+
LL | assert!(8 == (7 + 1));
95+
| ^^^^^^^^^^^^^^^^^^^^^
9696
|
9797
= help: remove the assertion
9898

9999
error: this assertion is always `true`
100-
--> tests/ui/assertions_on_constants.rs:68:5
100+
--> tests/ui/assertions_on_constants.rs:71:5
101101
|
102102
LL | assert!(true);
103103
| ^^^^^^^^^^^^^
104104
|
105105
= help: remove the assertion
106106

107107
error: this assertion is always `true`
108-
--> tests/ui/assertions_on_constants.rs:71:5
108+
--> tests/ui/assertions_on_constants.rs:76:9
109109
|
110-
LL | assert!(8 == (7 + 1));
111-
| ^^^^^^^^^^^^^^^^^^^^^
110+
LL | assert!(8 == (7 + 1));
111+
| ^^^^^^^^^^^^^^^^^^^^^
112112
|
113113
= help: remove the assertion
114114

115115
error: this assertion has a constant value
116-
--> tests/ui/assertions_on_constants.rs:79:5
116+
--> tests/ui/assertions_on_constants.rs:85:5
117117
|
118118
LL | assert!(C);
119119
| ^^^^^^^^^^
120120
|
121121
= help: consider moving this to an anonymous constant: `const _: () = { assert!(..); }`
122122

123123
error: this assertion has a constant value
124-
--> tests/ui/assertions_on_constants.rs:90:5
124+
--> tests/ui/assertions_on_constants.rs:96:5
125125
|
126126
LL | assert!(C);
127127
| ^^^^^^^^^^
128128
|
129129
= help: consider moving this into a const block: `const { assert!(..) }`
130130

131131
error: this assertion has a constant value
132-
--> tests/ui/assertions_on_constants.rs:96:5
132+
--> tests/ui/assertions_on_constants.rs:102:5
133133
|
134134
LL | assert!(C);
135135
| ^^^^^^^^^^

tests/ui/cmp_null.fixed

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ fn main() {
3333
//~^ cmp_null
3434
}
3535

36+
#[allow(
37+
clippy::manual_assert_eq,
38+
reason = "the lint only works on `assert`, not `assert_eq`"
39+
)]
3640
fn issue15010() {
3741
let f: *mut i32 = std::ptr::null_mut();
3842
debug_assert!(!f.is_null());

tests/ui/cmp_null.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ fn main() {
3333
//~^ cmp_null
3434
}
3535

36+
#[allow(
37+
clippy::manual_assert_eq,
38+
reason = "the lint only works on `assert`, not `assert_eq`"
39+
)]
3640
fn issue15010() {
3741
let f: *mut i32 = std::ptr::null_mut();
3842
debug_assert!(f != std::ptr::null_mut());

tests/ui/cmp_null.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ LL | let _ = x as *const () == ptr::null();
3232
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(x as *const ()).is_null()`
3333

3434
error: comparing with null is better expressed by the `.is_null()` method
35-
--> tests/ui/cmp_null.rs:38:19
35+
--> tests/ui/cmp_null.rs:42:19
3636
|
3737
LL | debug_assert!(f != std::ptr::null_mut());
3838
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!f.is_null()`

0 commit comments

Comments
 (0)