Lint (x * x + y * y).sqrt() => x.hypot(y)

thiagoarrais · thiagoarrais · commit 149c3ead87a6 · 2020-06-01T16:06:29.000-03:00
diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs
@@ -2,10 +2,10 @@ use crate::consts::{
     constant, constant_simple, Constant,
     Constant::{Int, F32, F64},
 };
-use crate::utils::{higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq};
+use crate::utils::{get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
+use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -298,6 +298,17 @@ fn check_powf(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
 fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
     // Check argument
     if let Some((value, _)) = constant(cx, cx.tables, &args[1]) {
+        // TODO: need more specific check. this is too wide. remember also to include tests
+        if let Some(parent) = get_parent_expr(cx, expr) {
+            if let Some(grandparent) = get_parent_expr(cx, parent) {
+                if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = grandparent.kind {
+                    if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() {
+                        return;
+                    }
+                }
+            }
+        }
+
         let (lint, help, suggestion) = match value {
             Int(2) => (
                 IMPRECISE_FLOPS,
@@ -319,6 +330,57 @@ fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
     }
 }
 
+fn detect_hypot(cx: &LateContext<'_, '_>, args: &[Expr<'_>]) -> Option<String> {
+    if let ExprKind::Binary(
+        Spanned {
+            node: BinOpKind::Add, ..
+        },
+        ref add_lhs,
+        ref add_rhs,
+    ) = args[0].kind
+    {
+        // check if expression of the form x * x + y * y
+        if_chain! {
+            if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lmul_lhs, ref lmul_rhs) = add_lhs.kind;
+            if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref rmul_lhs, ref rmul_rhs) = add_rhs.kind;
+            if are_exprs_equal(cx, lmul_lhs, lmul_rhs);
+            if are_exprs_equal(cx, rmul_lhs, rmul_rhs);
+            then {
+                return Some(format!("{}.hypot({})", Sugg::hir(cx, &lmul_lhs, ".."), Sugg::hir(cx, &rmul_lhs, "..")));
+            }
+        }
+
+        // check if expression of the form x.powi(2) + y.powi(2)
+        if_chain! {
+            if let ExprKind::MethodCall(PathSegment { ident: lmethod_name, .. }, ref _lspan, ref largs) = add_lhs.kind;
+            if let ExprKind::MethodCall(PathSegment { ident: rmethod_name, .. }, ref _rspan, ref rargs) = add_rhs.kind;
+            if lmethod_name.as_str() == "powi" && rmethod_name.as_str() == "powi";
+            if let Some((lvalue, _)) = constant(cx, cx.tables, &largs[1]);
+            if let Some((rvalue, _)) = constant(cx, cx.tables, &rargs[1]);
+            if Int(2) == lvalue && Int(2) == rvalue;
+            then {
+                return Some(format!("{}.hypot({})", Sugg::hir(cx, &largs[0], ".."), Sugg::hir(cx, &rargs[0], "..")));
+            }
+        }
+    }
+
+    None
+}
+
+fn check_hypot(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
+    if let Some(message) = detect_hypot(cx, args) {
+        span_lint_and_sugg(
+            cx,
+            IMPRECISE_FLOPS,
+            expr.span,
+            "hypotenuse can be computed more accurately",
+            "consider using",
+            message,
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
 // TODO: Lint expressions of the form `x.exp() - y` where y > 1
 // and suggest usage of `x.exp_m1() - (y - 1)` instead
 fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
@@ -370,6 +432,14 @@ fn check_mul_add(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
         rhs,
     ) = &expr.kind
     {
+        if let Some(parent) = get_parent_expr(cx, expr) {
+            if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = parent.kind {
+                if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() {
+                    return;
+                }
+            }
+        }
+
         let (recv, arg1, arg2) = if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, lhs) {
             (inner_lhs, inner_rhs, rhs)
         } else if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, rhs) {
@@ -516,6 +586,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic {
                     "log" => check_log_base(cx, expr, args),
                     "powf" => check_powf(cx, expr, args),
                     "powi" => check_powi(cx, expr, args),
+                    "sqrt" => check_hypot(cx, expr, args),
                     _ => {},
                 }
             }
diff --git a/tests/ui/floating_point_hypot.fixed b/tests/ui/floating_point_hypot.fixed
@@ -0,0 +1,13 @@
+// run-rustfix
+#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)]
+
+fn main() {
+    let x = 3f32;
+    let y = 4f32;
+    let _ = x.hypot(y);
+    let _ = (x + 1f32).hypot(y);
+    let _ = x.hypot(y);
+    // Cases where the lint shouldn't be applied
+    let _ = x.mul_add(x, y * y).sqrt();
+    let _ = x.mul_add(4f32, y * y).sqrt();
+}
diff --git a/tests/ui/floating_point_hypot.rs b/tests/ui/floating_point_hypot.rs
@@ -0,0 +1,13 @@
+// run-rustfix
+#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)]
+
+fn main() {
+    let x = 3f32;
+    let y = 4f32;
+    let _ = (x * x + y * y).sqrt();
+    let _ = ((x + 1f32) * (x + 1f32) + y * y).sqrt();
+    let _ = (x.powi(2) + y.powi(2)).sqrt();
+    // Cases where the lint shouldn't be applied
+    let _ = x.mul_add(x, y * y).sqrt();
+    let _ = (x * 4f32 + y * y).sqrt();
+}
diff --git a/tests/ui/floating_point_hypot.stderr b/tests/ui/floating_point_hypot.stderr
@@ -0,0 +1,30 @@
+error: hypotenuse can be computed more accurately
+  --> $DIR/floating_point_hypot.rs:7:13
+   |
+LL |     let _ = (x * x + y * y).sqrt();
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)`
+   |
+   = note: `-D clippy::imprecise-flops` implied by `-D warnings`
+
+error: hypotenuse can be computed more accurately
+  --> $DIR/floating_point_hypot.rs:8:13
+   |
+LL |     let _ = ((x + 1f32) * (x + 1f32) + y * y).sqrt();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x + 1f32).hypot(y)`
+
+error: hypotenuse can be computed more accurately
+  --> $DIR/floating_point_hypot.rs:9:13
+   |
+LL |     let _ = (x.powi(2) + y.powi(2)).sqrt();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)`
+
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> $DIR/floating_point_hypot.rs:12:13
+   |
+LL |     let _ = (x * 4f32 + y * y).sqrt();
+   |             ^^^^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(4f32, y * y)`
+   |
+   = note: `-D clippy::suboptimal-flops` implied by `-D warnings`
+
+error: aborting due to 4 previous errors
+
diff --git a/tests/ui/floating_point_mul_add.fixed b/tests/ui/floating_point_mul_add.fixed
@@ -18,4 +18,9 @@ fn main() {
 
     let _ = a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c)) + c;
     let _ = 1234.567_f64.mul_add(45.67834_f64, 0.0004_f64);
+
+    let _ = a.mul_add(a, b).sqrt();
+
+    // Cases where the lint shouldn't be applied
+    let _ = (a * a + b * b).sqrt();
 }
diff --git a/tests/ui/floating_point_mul_add.rs b/tests/ui/floating_point_mul_add.rs
@@ -18,4 +18,9 @@ fn main() {
 
     let _ = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
     let _ = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
+
+    let _ = (a * a + b).sqrt();
+
+    // Cases where the lint shouldn't be applied
+    let _ = (a * a + b * b).sqrt();
 }
diff --git a/tests/ui/floating_point_mul_add.stderr b/tests/ui/floating_point_mul_add.stderr
@@ -54,5 +54,11 @@ error: multiply and add expressions can be calculated more efficiently and accur
 LL |     let _ = 1234.567_f64 * 45.67834_f64 + 0.0004_f64;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `1234.567_f64.mul_add(45.67834_f64, 0.0004_f64)`
 
-error: aborting due to 9 previous errors
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> $DIR/floating_point_mul_add.rs:22:13
+   |
+LL |     let _ = (a * a + b).sqrt();
+   |             ^^^^^^^^^^^ help: consider using: `a.mul_add(a, b)`
+
+error: aborting due to 10 previous errors
 
diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed
@@ -1,12 +1,17 @@
 // run-rustfix
-#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)]
+#![warn(clippy::imprecise_flops)]
 
 fn main() {
     let one = 1;
     let x = 3f32;
     let _ = x * x;
     let _ = x * x;
+
+    let y = 4f32;
+    let _ = (x * x + y).sqrt();
+    let _ = (x + y * y).sqrt();
     // Cases where the lint shouldn't be applied
     let _ = x.powi(3);
     let _ = x.powi(one + 1);
+    let _ = x.hypot(y);
 }
diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs
@@ -1,12 +1,17 @@
 // run-rustfix
-#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)]
+#![warn(clippy::imprecise_flops)]
 
 fn main() {
     let one = 1;
     let x = 3f32;
     let _ = x.powi(2);
     let _ = x.powi(1 + 1);
+
+    let y = 4f32;
+    let _ = (x.powi(2) + y).sqrt();
+    let _ = (x + y.powi(2)).sqrt();
     // Cases where the lint shouldn't be applied
     let _ = x.powi(3);
     let _ = x.powi(one + 1);
+    let _ = (x.powi(2) + y.powi(2)).sqrt();
 }
diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr
@@ -12,5 +12,23 @@ error: square can be computed more accurately
 LL |     let _ = x.powi(1 + 1);
    |             ^^^^^^^^^^^^^ help: consider using: `x * x`
 
-error: aborting due to 2 previous errors
+error: square can be computed more accurately
+  --> $DIR/floating_point_powi.rs:11:14
+   |
+LL |     let _ = (x.powi(2) + y).sqrt();
+   |              ^^^^^^^^^ help: consider using: `x * x`
+
+error: square can be computed more accurately
+  --> $DIR/floating_point_powi.rs:12:18
+   |
+LL |     let _ = (x + y.powi(2)).sqrt();
+   |                  ^^^^^^^^^ help: consider using: `y * y`
+
+error: hypotenuse can be computed more accurately
+  --> $DIR/floating_point_powi.rs:16:13
+   |
+LL |     let _ = (x.powi(2) + y.powi(2)).sqrt();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)`
+
+error: aborting due to 5 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -18,4 +18,9 @@ fn main() {`
`18`	`18`
`19`	`19`	`let _ = a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c)) + c;`
`20`	`20`	`let _ = 1234.567_f64.mul_add(45.67834_f64, 0.0004_f64);`
	`21`	`+`
	`22`	`+ let _ = a.mul_add(a, b).sqrt();`
	`23`	`+`
	`24`	`+ // Cases where the lint shouldn't be applied`
	`25`	`+ let _ = (a * a + b * b).sqrt();`
`21`	`26`	`}`