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

krishna-veerareddy · krishna-veerareddy · commit 20319e8608ba · 2019-12-19T20:13:41.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,74 @@ fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr) {
     }
 }
 
+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];
+
+            if left_method == "log" {
+                if_chain! {
+                    // Checks whether the bases are constant and equal
+                    if let Some((numerator_base, _)) = constant(cx, cx.tables, &left_args[1]);
+                    if let Some((denominator_base, _)) = constant(cx, cx.tables, &right_args[1]);
+                    if numerator_base == denominator_base;
+                    then { }
+                    else {
+                        if_chain! {
+                            // Checks whether the bases are the same variable
+                            if let ExprKind::Path(ref left_base_qpath) = &left_args[1].kind;
+                            if let QPath::Resolved(_, ref left_base_path) = *left_base_qpath;
+                            if left_base_path.segments.len() == 1;
+                            if let def::Res::Local(left_local_id) = qpath_res(cx, left_base_qpath, expr.hir_id);
+                            if let ExprKind::Path(ref right_base_qpath) = &right_args[1].kind;
+                            if let QPath::Resolved(_, ref right_base_path) = *right_base_qpath;
+                            if right_base_path.segments.len() == 1;
+                            if let def::Res::Local(right_local_id) = qpath_res(cx, right_base_qpath, expr.hir_id);
+                            if left_local_id == right_local_id;
+                            then { }
+                            else {
+                                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 expressed more succinctly",
+                "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 +317,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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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 expressed more succinctly
+  --> $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
 
