Lint expressions of the form x.log(b) / y.log(b)

krishna-veerareddy · krishna-veerareddy · commit de7973392402 · 2020-02-23T22:20:33.000-08:00
diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs
@@ -64,29 +64,31 @@ declare_clippy_lint! {
 
 declare_lint_pass!(FloatingPointArithmetic => [FLOATING_POINT_IMPROVEMENTS]);
 
-fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec<Expr>) {
-    let arg = sugg::Sugg::hir(cx, &args[0], "..").maybe_par();
-
-    if let Some((value, _)) = constant(cx, cx.tables, &args[1]) {
-        let method;
-
+// Returns the specialized log method for a given base if base is constant
+// and is one of 2, 10 and e
+fn get_specialized_log_method(cx: &LateContext<'_, '_>, base: &Expr) -> Option<&'static str> {
+    if let Some((value, _)) = constant(cx, cx.tables, base) {
         if F32(2.0) == value || F64(2.0) == value {
-            method = "log2";
+            return Some("log2");
         } else if F32(10.0) == value || F64(10.0) == value {
-            method = "log10";
+            return Some("log10");
         } else if F32(f32_consts::E) == value || F64(f64_consts::E) == value {
-            method = "ln";
-        } else {
-            return;
+            return Some("ln");
         }
+    }
+
+    None
+}
 
+fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec<Expr>) {
+    if let Some(method) = get_specialized_log_method(cx, &args[1]) {
         span_lint_and_sugg(
             cx,
             FLOATING_POINT_IMPROVEMENTS,
             expr.span,
             "logarithm for bases 2, 10 and e can be computed more accurately",
             "consider using",
-            format!("{}.{}()", arg, method),
+            format!("{}.{}()", sugg::Sugg::hir(cx, &args[0], ".."), method),
             Applicability::MachineApplicable,
         );
     }
@@ -232,6 +234,82 @@ fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr) {
     }
 }
 
+// Checks whether two expressions evaluate to the same value
+fn are_exprs_equivalent(cx: &LateContext<'_, '_>, left: &Expr, right: &Expr) -> bool {
+    // Checks whether the values are constant and equal
+    if_chain! {
+        if let Some((left_value, _)) = constant(cx, cx.tables, left);
+        if let Some((right_value, _)) = constant(cx, cx.tables, right);
+        if left_value == right_value;
+        then {
+            return true;
+        }
+    }
+
+    // Checks whether the expressions resolve to the same variable
+    if_chain! {
+        if let ExprKind::Path(ref left_qpath) = left.kind;
+        if let QPath::Resolved(_, ref left_path) = *left_qpath;
+        if left_path.segments.len() == 1;
+        if let def::Res::Local(left_local_id) = qpath_res(cx, left_qpath, left.hir_id);
+        if let ExprKind::Path(ref right_qpath) = right.kind;
+        if let QPath::Resolved(_, ref right_path) = *right_qpath;
+        if right_path.segments.len() == 1;
+        if let def::Res::Local(right_local_id) = qpath_res(cx, right_qpath, right.hir_id);
+        if left_local_id == right_local_id;
+        then {
+            return true;
+        }
+    }
+
+    false
+}
+
+fn check_log_division(cx: &LateContext<'_, '_>, expr: &Expr) {
+    let log_methods = ["log", "log2", "log10", "ln"];
+
+    if_chain! {
+        if let ExprKind::Binary(op, ref lhs, ref rhs) = expr.kind;
+        if op.node == BinOpKind::Div;
+        if cx.tables.expr_ty(lhs).is_floating_point();
+        if let ExprKind::MethodCall(left_path, _, left_args) = &lhs.kind;
+        if let ExprKind::MethodCall(right_path, _, right_args) = &rhs.kind;
+        let left_method = left_path.ident.name.as_str();
+        if left_method == right_path.ident.name.as_str();
+        if log_methods.iter().any(|&method| left_method == method);
+        then {
+            let left_recv = &left_args[0];
+            let right_recv = &right_args[0];
+
+            // Return early when bases are not equal
+            if left_method == "log" && !are_exprs_equivalent(cx, &left_args[1], &right_args[1]) {
+                return;
+            }
+
+            // Reduce the expression further for bases 2, 10 and e
+            let suggestion = if let Some(method) = get_specialized_log_method(cx, right_recv) {
+                format!("{}.{}()", sugg::Sugg::hir(cx, left_recv, ".."), method)
+            } else {
+                format!(
+                    "{}.log({})",
+                    sugg::Sugg::hir(cx, left_recv, ".."),
+                    sugg::Sugg::hir(cx, right_recv, "..")
+                )
+            };
+
+            span_lint_and_sugg(
+                cx,
+                FLOATING_POINT_IMPROVEMENTS,
+                expr.span,
+                "x.log(b) / y.log(b) can be reduced to x.log(y)",
+                "consider using",
+                suggestion,
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         if let ExprKind::MethodCall(ref path, _, args) = &expr.kind {
@@ -247,6 +325,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic {
             }
         } else {
             check_expm1(cx, expr);
+            check_log_division(cx, expr);
         }
     }
 }
diff --git a/tests/ui/floating_point_arithmetic.rs b/tests/ui/floating_point_arithmetic.rs
@@ -82,4 +82,30 @@ fn check_expm1() {
     let _ = x.exp() - 1.0 * 2.0;
 }
 
+fn check_log_division() {
+    let x = 3f32;
+    let y = 2f32;
+    let b = 4f32;
+
+    let _ = x.log2() / y.log2();
+    let _ = x.log10() / y.log10();
+    let _ = x.ln() / y.ln();
+    let _ = x.log(4.0) / y.log(4.0);
+    let _ = x.log(b) / y.log(b);
+    let _ = x.log(b) / y.log(x);
+    let _ = x.log(b) / 2f32.log(b);
+
+    let x = 3f64;
+    let y = 2f64;
+    let b = 4f64;
+
+    let _ = x.log2() / y.log2();
+    let _ = x.log10() / y.log10();
+    let _ = x.ln() / y.ln();
+    let _ = x.log(4.0) / y.log(4.0);
+    let _ = x.log(b) / y.log(b);
+    let _ = x.log(b) / y.log(x);
+    let _ = x.log(b) / 2f64.log(b);
+}
+
 fn main() {}
diff --git a/tests/ui/floating_point_arithmetic.stderr b/tests/ui/floating_point_arithmetic.stderr
@@ -192,5 +192,77 @@ error: (e.pow(x) - 1) can be computed more accurately
 LL |     let _ = x.exp() - 1.0 + 2.0;
    |             ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()`
 
-error: aborting due to 32 previous errors
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:90:13
+   |
+LL |     let _ = x.log2() / y.log2();
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:91:13
+   |
+LL |     let _ = x.log10() / y.log10();
+   |             ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:92:13
+   |
+LL |     let _ = x.ln() / y.ln();
+   |             ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:93:13
+   |
+LL |     let _ = x.log(4.0) / y.log(4.0);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:94:13
+   |
+LL |     let _ = x.log(b) / y.log(b);
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:96:13
+   |
+LL |     let _ = x.log(b) / 2f32.log(b);
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log2()`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:102:13
+   |
+LL |     let _ = x.log2() / y.log2();
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:103:13
+   |
+LL |     let _ = x.log10() / y.log10();
+   |             ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:104:13
+   |
+LL |     let _ = x.ln() / y.ln();
+   |             ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:105:13
+   |
+LL |     let _ = x.log(4.0) / y.log(4.0);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:106:13
+   |
+LL |     let _ = x.log(b) / y.log(b);
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)`
+
+error: x.log(b) / y.log(b) can be reduced to x.log(y)
+  --> $DIR/floating_point_arithmetic.rs:108:13
+   |
+LL |     let _ = x.log(b) / 2f64.log(b);
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log2()`
+
+error: aborting due to 44 previous errors
 
