Finished checking for cases of absolute values

Author: JarredAllen

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,
+                        );
                     }
                 }
             }

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

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