user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

add manual_abs_diff lint

Browse source
This commit is contained in:
Yotam Ofek 2025-03-26 22:05:57 +00:00
parent 4c610e32ad
commit 52a3082056
10 changed files with 465 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -5784,6 +5784,7 @@ Released 2018-09-13
[`macro_metavars_in_unsafe`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_abs_diff`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits

1
book/src/lint_configuration.md
View file

@ -797,6 +797,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
* [`lines_filter_map_ok`](https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok)
* [`manual_abs_diff`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_abs_diff)
* [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)
* [`manual_c_str_literals`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals)
* [`manual_clamp`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp)

1
clippy_config/src/conf.rs
View file

@ -645,6 +645,7 @@ define_Conf! {
iter_kv_map,
legacy_numeric_constants,
lines_filter_map_ok,
manual_abs_diff,
manual_bits,
manual_c_str_literals,
manual_clamp,

1
clippy_lints/src/declared_lints.rs
View file

@ -313,6 +313,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::macro_metavars_in_unsafe::MACRO_METAVARS_IN_UNSAFE_INFO,
crate::macro_use::MACRO_USE_IMPORTS_INFO,
crate::main_recursion::MAIN_RECURSION_INFO,
crate::manual_abs_diff::MANUAL_ABS_DIFF_INFO,
crate::manual_assert::MANUAL_ASSERT_INFO,
crate::manual_async_fn::MANUAL_ASYNC_FN_INFO,
crate::manual_bits::MANUAL_BITS_INFO,

2
clippy_lints/src/lib.rs
View file

@ -205,6 +205,7 @@ mod loops;
mod macro_metavars_in_unsafe;
mod macro_use;
mod main_recursion;
mod manual_abs_diff;
mod manual_assert;
mod manual_async_fn;
mod manual_bits;
@ -878,6 +879,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(std_instead_of_core::StdReexports::new(conf)));
store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(conf)));
store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));
store.register_late_pass(move |_| Box::new(manual_abs_diff::ManualAbsDiff::new(conf)));
store.register_late_pass(move |_| Box::new(manual_clamp::ManualClamp::new(conf)));
store.register_late_pass(|_| Box::new(manual_string_new::ManualStringNew));
store.register_late_pass(|_| Box::new(unused_peekable::UnusedPeekable));

152
clippy_lints/src/manual_abs_diff.rs Normal file
View file

@ -0,0 +1,152 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::If;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::HasSession as _;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{eq_expr_value, peel_blocks, span_contains_comment};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty};
use rustc_session::impl_lint_pass;
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Detects patterns like `if a > b { a - b } else { b - a }` and suggests using `a.abs_diff(b)`.
///
/// ### Why is this bad?
/// Using `abs_diff` is shorter, more readable, and avoids control flow.
///
/// ### Examples
/// ```no_run
/// # let (a, b) = (5_usize, 3_usize);
/// if a > b {
/// a - b
/// } else {
/// b - a
/// }
/// # ;
/// ```
/// Use instead:
/// ```no_run
/// # let (a, b) = (5_usize, 3_usize);
/// a.abs_diff(b)
/// # ;
/// ```
#[clippy::version = "1.86.0"]
pub MANUAL_ABS_DIFF,
complexity,
"using an if-else pattern instead of `abs_diff`"
}
impl_lint_pass!(ManualAbsDiff => [MANUAL_ABS_DIFF]);
pub struct ManualAbsDiff {
msrv: Msrv,
}
impl ManualAbsDiff {
pub fn new(conf: &'static Conf) -> Self {
Self { msrv: conf.msrv }
}
}
impl<'tcx> LateLintPass<'tcx> for ManualAbsDiff {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !expr.span.from_expansion()
&& let Some(if_expr) = If::hir(expr)
&& let Some(r#else) = if_expr.r#else
&& let ExprKind::Binary(op, rhs, lhs) = if_expr.cond.kind
&& let (BinOpKind::Gt | BinOpKind::Ge, mut a, mut b) | (BinOpKind::Lt | BinOpKind::Le, mut b, mut a) =
(op.node, rhs, lhs)
&& let Some(ty) = self.are_ty_eligible(cx, a, b)
&& is_sub_expr(cx, if_expr.then, a, b, ty)
&& is_sub_expr(cx, r#else, b, a, ty)
{
span_lint_and_then(
cx,
MANUAL_ABS_DIFF,
expr.span,
"manual absolute difference pattern without using `abs_diff`",
|diag| {
if is_unsuffixed_numeral_lit(a) && !is_unsuffixed_numeral_lit(b) {
(a, b) = (b, a);
}
let applicability = {
let source_map = cx.sess().source_map();
if span_contains_comment(source_map, if_expr.then.span)
|| span_contains_comment(source_map, r#else.span)
{
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
}
};
let sugg = format!(
"{}.abs_diff({})",
Sugg::hir(cx, a, "..").maybe_paren(),
Sugg::hir(cx, b, "..")
);
diag.span_suggestion(expr.span, "replace with `abs_diff`", sugg, applicability);
},
);
}
}
}
impl ManualAbsDiff {
/// Returns a type if `a` and `b` are both of it, and this lint can be applied to that
/// type (currently, any primitive int, or a `Duration`)
fn are_ty_eligible<'tcx>(&self, cx: &LateContext<'tcx>, a: &Expr<'_>, b: &Expr<'_>) -> Option<Ty<'tcx>> {
let is_int = |ty: Ty<'_>| matches!(ty.kind(), ty::Uint(_) | ty::Int(_)) && self.msrv.meets(cx, msrvs::ABS_DIFF);
let is_duration =
|ty| is_type_diagnostic_item(cx, ty, sym::Duration) && self.msrv.meets(cx, msrvs::DURATION_ABS_DIFF);
let a_ty = cx.typeck_results().expr_ty(a).peel_refs();
(a_ty == cx.typeck_results().expr_ty(b).peel_refs() && (is_int(a_ty) || is_duration(a_ty))).then_some(a_ty)
}
}
/// Checks if the given expression is a subtraction operation between two expected expressions,
/// i.e. if `expr` is `{expected_a} - {expected_b}`.
///
/// If `expected_ty` is a signed primitive integer, this function will only return `Some` if the
/// subtraction expr is wrapped in a cast to the equivalent unsigned int.
fn is_sub_expr(
cx: &LateContext<'_>,
expr: &Expr<'_>,
expected_a: &Expr<'_>,
expected_b: &Expr<'_>,
expected_ty: Ty<'_>,
) -> bool {
let expr = peel_blocks(expr).kind;
if let ty::Int(ty) = expected_ty.kind() {
let unsigned = Ty::new_uint(cx.tcx, ty.to_unsigned());
return if let ExprKind::Cast(expr, cast_ty) = expr
&& cx.typeck_results().node_type(cast_ty.hir_id) == unsigned
{
is_sub_expr(cx, expr, expected_a, expected_b, unsigned)
} else {
false
};
}
if let ExprKind::Binary(op, a, b) = expr
&& let BinOpKind::Sub = op.node
&& eq_expr_value(cx, a, expected_a)
&& eq_expr_value(cx, b, expected_b)
{
true
} else {
false
}
}
fn is_unsuffixed_numeral_lit(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Lit(lit) if lit.node.is_numeric() && lit.node.is_unsuffixed())
}

3
clippy_utils/src/msrvs.rs
View file

@ -27,7 +27,7 @@ msrv_aliases! {
1,84,0 { CONST_OPTION_AS_SLICE }
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION, DURATION_ABS_DIFF }
1,80,0 { BOX_INTO_ITER, LAZY_CELL }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
@ -40,6 +40,7 @@ msrv_aliases! {
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
1,63,0 { CLONE_INTO }
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN }
1,60,0 { ABS_DIFF }
1,59,0 { THREAD_LOCAL_CONST_INIT }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY, CONST_RAW_PTR_DEREF }
1,57,0 { MAP_WHILE }

106
tests/ui/manual_abs_diff.fixed Normal file
View file

@ -0,0 +1,106 @@
#![warn(clippy::manual_abs_diff)]
use std::time::Duration;
fn main() {
let a: usize = 5;
let b: usize = 3;
let c: usize = 8;
let d: usize = 11;
let _ = a.abs_diff(b);
//~^ manual_abs_diff
let _ = b.abs_diff(a);
//~^ manual_abs_diff
let _ = b.abs_diff(5);
//~^ manual_abs_diff
let _ = b.abs_diff(5);
//~^ manual_abs_diff
let _ = a.abs_diff(b);
//~^ manual_abs_diff
let _ = b.abs_diff(a);
//~^ manual_abs_diff
#[allow(arithmetic_overflow)]
{
let _ = if a > b { b - a } else { a - b };
let _ = if a < b { a - b } else { b - a };
}
let _ = (a + b).abs_diff(c + d);
let _ = (c + d).abs_diff(a + b);
const A: usize = 5;
const B: usize = 3;
// check const context
const _: usize = A.abs_diff(B);
//~^ manual_abs_diff
let a = Duration::from_secs(3);
let b = Duration::from_secs(5);
let _ = a.abs_diff(b);
//~^ manual_abs_diff
let a: i32 = 3;
let b: i32 = -5;
let _ = if a > b { a - b } else { b - a };
let _ = a.abs_diff(b);
//~^ manual_abs_diff
}
// FIXME: bunch of patterns that should be linted
fn fixme() {
let a: usize = 5;
let b: usize = 3;
let c: usize = 8;
let d: usize = 11;
{
let out;
if a > b {
out = a - b;
} else {
out = b - a;
}
}
{
let mut out = 0;
if a > b {
out = a - b;
} else if a < b {
out = b - a;
}
}
#[allow(clippy::implicit_saturating_sub)]
let _ = if a > b {
a - b
} else if a < b {
b - a
} else {
0
};
let a: i32 = 3;
let b: i32 = 5;
let _: u32 = if a > b { a - b } else { b - a } as u32;
}
fn non_primitive_ty() {
#[derive(Eq, PartialEq, PartialOrd)]
struct S(i32);
impl std::ops::Sub for S {
type Output = S;
fn sub(self, rhs: Self) -> Self::Output {
Self(self.0 - rhs.0)
}
}
let (a, b) = (S(10), S(20));
let _ = if a < b { b - a } else { a - b };
}

116
tests/ui/manual_abs_diff.rs Normal file
View file

@ -0,0 +1,116 @@
#![warn(clippy::manual_abs_diff)]
use std::time::Duration;
fn main() {
let a: usize = 5;
let b: usize = 3;
let c: usize = 8;
let d: usize = 11;
let _ = if a > b { a - b } else { b - a };
//~^ manual_abs_diff
let _ = if a < b { b - a } else { a - b };
//~^ manual_abs_diff
let _ = if 5 > b { 5 - b } else { b - 5 };
//~^ manual_abs_diff
let _ = if b > 5 { b - 5 } else { 5 - b };
//~^ manual_abs_diff
let _ = if a >= b { a - b } else { b - a };
//~^ manual_abs_diff
let _ = if a <= b { b - a } else { a - b };
//~^ manual_abs_diff
#[allow(arithmetic_overflow)]
{
let _ = if a > b { b - a } else { a - b };
let _ = if a < b { a - b } else { b - a };
}
let _ = if (a + b) > (c + d) {
//~^ manual_abs_diff
(a + b) - (c + d)
} else {
(c + d) - (a + b)
};
let _ = if (a + b) < (c + d) {
//~^ manual_abs_diff
(c + d) - (a + b)
} else {
(a + b) - (c + d)
};
const A: usize = 5;
const B: usize = 3;
// check const context
const _: usize = if A > B { A - B } else { B - A };
//~^ manual_abs_diff
let a = Duration::from_secs(3);
let b = Duration::from_secs(5);
let _ = if a > b { a - b } else { b - a };
//~^ manual_abs_diff
let a: i32 = 3;
let b: i32 = -5;
let _ = if a > b { a - b } else { b - a };
let _ = if a > b { (a - b) as u32 } else { (b - a) as u32 };
//~^ manual_abs_diff
}
// FIXME: bunch of patterns that should be linted
fn fixme() {
let a: usize = 5;
let b: usize = 3;
let c: usize = 8;
let d: usize = 11;
{
let out;
if a > b {
out = a - b;
} else {
out = b - a;
}
}
{
let mut out = 0;
if a > b {
out = a - b;
} else if a < b {
out = b - a;
}
}
#[allow(clippy::implicit_saturating_sub)]
let _ = if a > b {
a - b
} else if a < b {
b - a
} else {
0
};
let a: i32 = 3;
let b: i32 = 5;
let _: u32 = if a > b { a - b } else { b - a } as u32;
}
fn non_primitive_ty() {
#[derive(Eq, PartialEq, PartialOrd)]
struct S(i32);
impl std::ops::Sub for S {
type Output = S;
fn sub(self, rhs: Self) -> Self::Output {
Self(self.0 - rhs.0)
}
}
let (a, b) = (S(10), S(20));
let _ = if a < b { b - a } else { a - b };
}

83
tests/ui/manual_abs_diff.stderr Normal file
View file

@ -0,0 +1,83 @@
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:11:13
|
LL | let _ = if a > b { a - b } else { b - a };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `a.abs_diff(b)`
|
= note: `-D clippy::manual-abs-diff` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_abs_diff)]`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:13:13
|
LL | let _ = if a < b { b - a } else { a - b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `b.abs_diff(a)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:16:13
|
LL | let _ = if 5 > b { 5 - b } else { b - 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `b.abs_diff(5)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:18:13
|
LL | let _ = if b > 5 { b - 5 } else { 5 - b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `b.abs_diff(5)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:21:13
|
LL | let _ = if a >= b { a - b } else { b - a };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `a.abs_diff(b)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:23:13
|
LL | let _ = if a <= b { b - a } else { a - b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `b.abs_diff(a)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:32:13
|
LL | let _ = if (a + b) > (c + d) {
| _____________^
LL | |
LL | | (a + b) - (c + d)
LL | | } else {
LL | | (c + d) - (a + b)
LL | | };
| |_____^ help: replace with `abs_diff`: `(a + b).abs_diff(c + d)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:38:13
|
LL | let _ = if (a + b) < (c + d) {
| _____________^
LL | |
LL | | (c + d) - (a + b)
LL | | } else {
LL | | (a + b) - (c + d)
LL | | };
| |_____^ help: replace with `abs_diff`: `(c + d).abs_diff(a + b)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:48:22
|
LL | const _: usize = if A > B { A - B } else { B - A };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `A.abs_diff(B)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:53:13
|
LL | let _ = if a > b { a - b } else { b - a };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `a.abs_diff(b)`
error: manual absolute difference pattern without using `abs_diff`
--> tests/ui/manual_abs_diff.rs:59:13
|
LL | let _ = if a > b { (a - b) as u32 } else { (b - a) as u32 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `a.abs_diff(b)`
error: aborting due to 11 previous errors