Skip to content

Commit 4d400a2

Browse files
committed
feat(manual_assert_eq): new lint
1 parent a10cafe commit 4d400a2

33 files changed

+542
-92
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6528,6 +6528,7 @@ Released 2018-09-13
65286528
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
65296529
[`manual_abs_diff`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff
65306530
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
6531+
[`manual_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq
65316532
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
65326533
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
65336534
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
66

7-
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
7+
[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
88

99
Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
1010
You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
A collection of lints to catch common mistakes and improve your
66
[Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
Lints are divided into categories, each with a default [lint
1111
level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_lints/src/declared_lints.rs

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

clippy_lints/src/eta_reduction.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ fn has_late_bound_to_non_late_bound_regions(from_sig: FnSig<'_>, to_sig: FnSig<'
371371
}
372372
}
373373

374-
assert!(from_sig.inputs_and_output.len() == to_sig.inputs_and_output.len());
374+
assert_eq!(from_sig.inputs_and_output.len(), to_sig.inputs_and_output.len());
375375
from_sig
376376
.inputs_and_output
377377
.iter()

clippy_lints/src/functions/renamed_function_params.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl RenamedFnArgs {
5757
{
5858
let mut renamed: Vec<(Span, String)> = vec![];
5959

60-
debug_assert!(default_idents.size_hint() == current_idents.size_hint());
60+
debug_assert_eq!(default_idents.size_hint(), current_idents.size_hint());
6161
for (default_ident, current_ident) in iter::zip(default_idents, current_idents) {
6262
let has_name_to_check = |ident: Option<Ident>| {
6363
ident

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;
@@ -850,6 +851,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
850851
Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
851852
Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
852853
Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
854+
Box::new(|_| Box::new(manual_assert_eq::ManualAssertEq)),
853855
// add late passes here, used by `cargo dev new_lint`
854856
];
855857
store.late_passes.extend(late_lints);
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
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::ty::implements_trait;
5+
use clippy_utils::{is_in_const_context, sym};
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{BinOpKind, Expr, ExprKind};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_session::declare_lint_pass;
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Checks for `assert!` and `debug_assert!` that consist of only an (in)equality check
14+
///
15+
/// ### Why is this bad?
16+
/// `assert_{eq,ne}!` and `debug_assert_{eq,ne}!` achieves the same goal, and provides some
17+
/// additional debug information
18+
///
19+
/// ### Example
20+
/// ```no_run
21+
/// assert!(2 * 2 == 4);
22+
/// assert!(2 * 2 != 5);
23+
/// debug_assert!(2 * 2 == 4);
24+
/// debug_assert!(2 * 2 != 5);
25+
/// ```
26+
/// Use instead:
27+
/// ```no_run
28+
/// assert_eq!(2 * 2, 4);
29+
/// assert_ne!(2 * 2, 5);
30+
/// debug_assert_eq!(2 * 2, 4);
31+
/// debug_assert_ne!(2 * 2, 5);
32+
/// ```
33+
#[clippy::version = "1.93.0"]
34+
pub MANUAL_ASSERT_EQ,
35+
pedantic,
36+
"checks for assertions consisting of an (in)equality check"
37+
}
38+
declare_lint_pass!(ManualAssertEq => [MANUAL_ASSERT_EQ]);
39+
40+
impl LateLintPass<'_> for ManualAssertEq {
41+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
42+
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
43+
&& let macro_name = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
44+
Some(sym::assert_macro) => "assert",
45+
Some(sym::debug_assert_macro) => "debug_assert",
46+
_ => return,
47+
}
48+
// `assert_eq` isn't allowed in const context because it calls non-const `core::panicking::assert_failed`
49+
// XXX: this might change in the future, so might want to relax this restriction
50+
&& !is_in_const_context(cx)
51+
&& let Some((cond, _)) = find_assert_args(cx, expr, macro_call.expn)
52+
&& let ExprKind::Binary(op, lhs, rhs) = cond.kind
53+
&& matches!(op.node, BinOpKind::Eq | BinOpKind::Ne)
54+
&& !cond.span.from_expansion()
55+
&& let Some(debug_trait) = cx.tcx.get_diagnostic_item(sym::Debug)
56+
&& implements_trait(cx, cx.typeck_results().expr_ty(lhs), debug_trait, &[])
57+
&& implements_trait(cx, cx.typeck_results().expr_ty(rhs), debug_trait, &[])
58+
{
59+
span_lint_and_then(
60+
cx,
61+
MANUAL_ASSERT_EQ,
62+
macro_call.span,
63+
format!("used `{macro_name}!` with an equality comparison"),
64+
|diag| {
65+
let kind = if op.node == BinOpKind::Eq { "eq" } else { "ne" };
66+
let new_name = format!("{macro_name}_{kind}");
67+
let msg = format!("replace it with `{new_name}!(..)`");
68+
69+
let ctxt = cond.span.ctxt();
70+
if let Some(lhs_span) = walk_span_to_context(lhs.span, ctxt)
71+
&& let Some(rhs_span) = walk_span_to_context(rhs.span, ctxt)
72+
{
73+
let macro_name_span = cx.sess().source_map().span_until_char(macro_call.span, '!');
74+
let eq_span = cond.span.with_lo(lhs_span.hi()).with_hi(rhs_span.lo());
75+
let suggestions = vec![
76+
(macro_name_span.shrink_to_hi(), format!("_{kind}")),
77+
(eq_span, ", ".to_string()),
78+
];
79+
80+
diag.multipart_suggestion(msg, suggestions, Applicability::MachineApplicable);
81+
} else {
82+
diag.span_help(expr.span, msg);
83+
}
84+
},
85+
);
86+
}
87+
}
88+
}

clippy_utils/src/ty/mod.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -915,14 +915,20 @@ fn assert_generic_args_match<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, args: &[Generi
915915
.chain(&g.own_params)
916916
.map(|x| &x.kind);
917917

918-
assert!(
919-
count == args.len(),
920-
"wrong number of arguments for `{did:?}`: expected `{count}`, found {}\n\
918+
#[expect(
919+
clippy::manual_assert_eq,
920+
reason = "the message contains `assert_eq!`-like formatting itself"
921+
)]
922+
{
923+
assert!(
924+
count == args.len(),
925+
"wrong number of arguments for `{did:?}`: expected `{count}`, found {}\n\
921926
note: the expected arguments are: `[{}]`\n\
922927
the given arguments are: `{args:#?}`",
923-
args.len(),
924-
params.clone().map(ty::GenericParamDefKind::descr).format(", "),
925-
);
928+
args.len(),
929+
params.clone().map(ty::GenericParamDefKind::descr).format(", "),
930+
);
931+
}
926932

927933
if let Some((idx, (param, arg))) =
928934
params

lintcheck/src/output.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ fn print_stats(old_stats: HashMap<String, usize>, new_stats: HashMap<&String, us
228228

229229
// remove duplicates from both hashmaps
230230
for (k, v) in &same_in_both_hashmaps {
231-
assert!(old_stats_deduped.remove(k) == Some(*v));
232-
assert!(new_stats_deduped.remove(k) == Some(*v));
231+
assert_eq!(old_stats_deduped.remove(k), Some(*v));
232+
assert_eq!(new_stats_deduped.remove(k), Some(*v));
233233
}
234234

235235
println!("\nStats:");

0 commit comments

Comments
 (0)