diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 970aeef623e..8a6ae10ab0b 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -387,6 +387,18 @@ fn is_testing_positive(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_ } } +fn is_testing_negative(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool { + if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind { + match op { + BinOpKind::Gt | BinOpKind::Ge => is_zero(left) && are_exprs_equal(cx, right, test), + BinOpKind::Lt | BinOpKind::Le => is_zero(right) && are_exprs_equal(cx, left, test), + _ => false, + } + } else { + false + } +} + fn are_exprs_equal(cx: &LateContext<'_, '_>, expr1: &Expr<'_>, expr2: &Expr<'_>) -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(expr1, expr2) } @@ -410,30 +422,9 @@ fn is_zero(expr: &Expr<'_>) -> bool { fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { if let Some((cond, body, Some(else_body))) = higher::if_block(&expr) { - if let ExprKind::Block( - Block { - stmts: [], - expr: - Some(Expr { - kind: ExprKind::Unary(UnOp::UnNeg, else_expr), - .. - }), - .. - }, - _, - ) = else_body.kind - { - if let ExprKind::Block( - Block { - stmts: [], - expr: Some(body), - .. - }, - _, - ) = &body.kind - { + if let ExprKind::Block( Block { stmts: [], expr: Some(Expr { kind: ExprKind::Unary(UnOp::UnNeg, else_expr), .. }), .. }, _,) = else_body.kind { + if let ExprKind::Block( Block { stmts: [], expr: Some(body), .. }, _,) = &body.kind { if are_exprs_equal(cx, else_expr, body) { - dbg!("if (cond) body else -body\nbody: {:?}", &body.kind); if is_testing_positive(cx, cond, body) { span_lint_and_sugg( cx, @@ -444,6 +435,44 @@ fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { format!("{}.abs()", Sugg::hir(cx, body, "..")), Applicability::MachineApplicable, ); + } else if is_testing_negative(cx, cond, body) { + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "This looks like you've implemented your own negative absolute value function", + "try", + format!("-{}.abs()", Sugg::hir(cx, body, "..")), + Applicability::MachineApplicable, + ); + } + } + } + } + if let ExprKind::Block( Block { stmts: [], expr: Some(Expr { kind: ExprKind::Unary(UnOp::UnNeg, else_expr), .. }), .. }, _,) = &body.kind + { + if let ExprKind::Block( Block { stmts: [], expr: Some(body), .. }, _,) = &else_body.kind { + if are_exprs_equal(cx, else_expr, body) { + if is_testing_negative(cx, cond, body) { + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "This looks like you've implemented your own absolute value function", + "try", + format!("{}.abs()", Sugg::hir(cx, body, "..")), + Applicability::MachineApplicable, + ); + } else if is_testing_positive(cx, cond, body) { + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "This looks like you've implemented your own negative absolute value function", + "try", + format!("-{}.abs()", Sugg::hir(cx, body, "..")), + Applicability::MachineApplicable, + ); } } } diff --git a/tests/ui/floating_point_abs.rs b/tests/ui/floating_point_abs.rs index 0efc7092899..40d2ff7e859 100644 --- a/tests/ui/floating_point_abs.rs +++ b/tests/ui/floating_point_abs.rs @@ -1,4 +1,5 @@ -#[warn(clippy::suboptimal_flops)] +#![warn(clippy::suboptimal_flops)] + struct A { a: f64, b: f64 @@ -28,6 +29,22 @@ fn fake_abs3(a: A) -> f64 { } } +fn fake_abs4(num: f64) -> f64 { + if 0.0 >= num { + -num + } else { + num + } +} + +fn fake_abs5(a: A) -> f64 { + if a.a < 0.0 { + -a.a + } else { + a.a + } +} + fn fake_nabs1(num: f64) -> f64 { if num < 0.0 { num @@ -46,9 +63,9 @@ fn fake_nabs2(num: f64) -> f64 { fn fake_nabs3(a: A) -> A { A { a: if a.a >= 0.0 { - a.a - } else { -a.a + } else { + a.a }, b: a.b } } diff --git a/tests/ui/floating_point_abs.stderr b/tests/ui/floating_point_abs.stderr index bbd67de17c0..dd648a8a272 100644 --- a/tests/ui/floating_point_abs.stderr +++ b/tests/ui/floating_point_abs.stderr @@ -1,5 +1,5 @@ error: This looks like you've implemented your own absolute value function - --> $DIR/floating_point_abs.rs:4:5 + --> $DIR/floating_point_abs.rs:9:5 | LL | / if num >= 0.0 { LL | | num @@ -10,5 +10,76 @@ LL | | } | = note: `-D clippy::suboptimal-flops` implied by `-D warnings` -error: aborting due to previous error +error: This looks like you've implemented your own absolute value function + --> $DIR/floating_point_abs.rs:17:5 + | +LL | / if 0.0 < num { +LL | | num +LL | | } else { +LL | | -num +LL | | } + | |_____^ help: try: `num.abs()` + +error: This looks like you've implemented your own absolute value function + --> $DIR/floating_point_abs.rs:25:5 + | +LL | / if a.a > 0.0 { +LL | | a.a +LL | | } else { +LL | | -a.a +LL | | } + | |_____^ help: try: `a.a.abs()` + +error: This looks like you've implemented your own absolute value function + --> $DIR/floating_point_abs.rs:33:5 + | +LL | / if 0.0 >= num { +LL | | -num +LL | | } else { +LL | | num +LL | | } + | |_____^ help: try: `num.abs()` + +error: This looks like you've implemented your own absolute value function + --> $DIR/floating_point_abs.rs:41:5 + | +LL | / if a.a < 0.0 { +LL | | -a.a +LL | | } else { +LL | | a.a +LL | | } + | |_____^ help: try: `a.a.abs()` + +error: This looks like you've implemented your own negative absolute value function + --> $DIR/floating_point_abs.rs:49:5 + | +LL | / if num < 0.0 { +LL | | num +LL | | } else { +LL | | -num +LL | | } + | |_____^ help: try: `-num.abs()` + +error: This looks like you've implemented your own negative absolute value function + --> $DIR/floating_point_abs.rs:57:5 + | +LL | / if 0.0 >= num { +LL | | num +LL | | } else { +LL | | -num +LL | | } + | |_____^ help: try: `-num.abs()` + +error: This looks like you've implemented your own negative absolute value function + --> $DIR/floating_point_abs.rs:65:12 + | +LL | A { a: if a.a >= 0.0 { + | ____________^ +LL | | -a.a +LL | | } else { +LL | | a.a +LL | | }, b: a.b } + | |_________^ help: try: `-a.a.abs()` + +error: aborting due to 8 previous errors