Add support for different orders of expression

clippy_lints/src/manual_is_power_of_two.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::SpanlessEq;
 use rustc_ast::LitKind;
 use rustc_data_structures::packed::Pu128;
 use rustc_errors::Applicability;
@@ -10,19 +11,19 @@ use rustc_session::declare_lint_pass;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for expressions like `x.count_ones() == 1` or `x & (x - 1) == 0`, which are manual
+    /// Checks for expressions like `x.count_ones() == 1` or `x & (x - 1) == 0`, with x and unsigned integer, which are manual
     /// reimplementations of `x.is_power_of_two()`.
     /// ### Why is this bad?
     /// Manual reimplementations of `is_power_of_two` increase code complexity for little benefit.
     /// ### Example
     /// ```no_run
-    /// let x: u32 = 1;
-    /// let result = x.count_ones() == 1;
+    /// let a: u32 = 4;
+    /// let result = a.count_ones() == 1;
     /// ```
     /// Use instead:
     /// ```no_run
-    /// let x: u32 = 1;
-    /// let result = x.is_power_of_two();
+    /// let a: u32 = 4;
+    /// let result = a.is_power_of_two();
     /// ```
     #[clippy::version = "1.82.0"]
     pub MANUAL_IS_POWER_OF_TWO,
@@ -36,53 +37,106 @@ impl LateLintPass<'_> for ManualIsPowerOfTwo {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
         let mut applicability = Applicability::MachineApplicable;
 
-        // x.count_ones() == 1
-        if let ExprKind::Binary(op, left, right) = expr.kind
-            && BinOpKind::Eq == op.node
-            && let ExprKind::MethodCall(method_name, reciever, _, _) = left.kind
-            && method_name.ident.as_str() == "count_ones"
-            && let ExprKind::Lit(lit) = right.kind
-            && let LitKind::Int(Pu128(1), _) = lit.node
-            && let &Uint(_) = cx.typeck_results().expr_ty(reciever).kind()
+        if let ExprKind::Binary(bin_op, left, right) = expr.kind
+            && bin_op.node == BinOpKind::Eq
         {
-            let snippet = snippet_with_applicability(cx, reciever.span, "..", &mut applicability);
-            let sugg = format!("{snippet}.is_power_of_two()");
-            span_lint_and_sugg(
-                cx,
-                MANUAL_IS_POWER_OF_TWO,
-                expr.span,
-                "manually reimplementing `is_power_of_two`",
-                "consider using `.is_power_of_two()`",
-                sugg,
-                applicability,
-            );
-        }
+            // a.count_ones() == 1
+            if let ExprKind::MethodCall(method_name, reciever, _, _) = left.kind
+                && method_name.ident.as_str() == "count_ones"
+                && let &Uint(_) = cx.typeck_results().expr_ty(reciever).kind()
+                && check_lit(right, 1)
+            {
+                build_sugg(cx, expr, reciever, &mut applicability);
+            }
 
-        // x & (x - 1) == 0
-        if let ExprKind::Binary(op, left, right) = expr.kind
-            && BinOpKind::Eq == op.node
-            && let ExprKind::Binary(op1, left1, right1) = left.kind
-            && BinOpKind::BitAnd == op1.node
-            && let ExprKind::Binary(op2, left2, right2) = right1.kind
-            && BinOpKind::Sub == op2.node
-            && left1.span.eq_ctxt(left2.span)
-            && let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
-            && let ExprKind::Lit(lit) = right2.kind
-            && let LitKind::Int(Pu128(1), _) = lit.node
-            && let ExprKind::Lit(lit1) = right.kind
-            && let LitKind::Int(Pu128(0), _) = lit1.node
-        {
-            let snippet = snippet_with_applicability(cx, left1.span, "..", &mut applicability);
-            let sugg = format!("{snippet}.is_power_of_two()");
-            span_lint_and_sugg(
-                cx,
-                MANUAL_IS_POWER_OF_TWO,
-                expr.span,
-                "manually reimplementing `is_power_of_two`",
-                "consider using `.is_power_of_two()`",
-                sugg,
-                applicability,
-            );
+            // 1 == a.count_ones()
+            if let ExprKind::MethodCall(method_name, reciever, _, _) = right.kind
+                && method_name.ident.as_str() == "count_ones"
+                && let &Uint(_) = cx.typeck_results().expr_ty(reciever).kind()
+                && check_lit(left, 1)
+            {
+                build_sugg(cx, expr, reciever, &mut applicability);
+            }
+
+            // a & (a - 1) == 0
+            if let ExprKind::Binary(op1, left1, right1) = left.kind
+                && op1.node == BinOpKind::BitAnd
+                && let ExprKind::Binary(op2, left2, right2) = right1.kind
+                && op2.node == BinOpKind::Sub
+                && check_eq_expr(cx, left1, left2)
+                && let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
+                && check_lit(right2, 1)
+                && check_lit(right, 0)
+            {
+                build_sugg(cx, expr, left1, &mut applicability);
+            }
+
+            // (a - 1) & a == 0;
+            if let ExprKind::Binary(op1, left1, right1) = left.kind
+                && op1.node == BinOpKind::BitAnd
+                && let ExprKind::Binary(op2, left2, right2) = left1.kind
+                && op2.node == BinOpKind::Sub
+                && check_eq_expr(cx, right1, left2)
+                && let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
+                && check_lit(right2, 1)
+                && check_lit(right, 0)
+            {
+                build_sugg(cx, expr, right1, &mut applicability);
+            }
+
+            // 0 == a & (a - 1);
+            if let ExprKind::Binary(op1, left1, right1) = right.kind
+                && op1.node == BinOpKind::BitAnd
+                && let ExprKind::Binary(op2, left2, right2) = right1.kind
+                && op2.node == BinOpKind::Sub
+                && check_eq_expr(cx, left1, left2)
+                && let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
+                && check_lit(right2, 1)
+                && check_lit(left, 0)
+            {
+                build_sugg(cx, expr, left1, &mut applicability);
+            }
+
+            // 0 == (a - 1) & a
+            if let ExprKind::Binary(op1, left1, right1) = right.kind
+                && op1.node == BinOpKind::BitAnd
+                && let ExprKind::Binary(op2, left2, right2) = left1.kind
+                && op2.node == BinOpKind::Sub
+                && check_eq_expr(cx, right1, left2)
+                && let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
+                && check_lit(right2, 1)
+                && check_lit(left, 0)
+            {
+                build_sugg(cx, expr, right1, &mut applicability);
+            }
         }
     }
 }
+
+fn build_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, reciever: &Expr<'_>, applicability: &mut Applicability) {
+    let snippet = snippet_with_applicability(cx, reciever.span, "..", applicability);
+
+    span_lint_and_sugg(
+        cx,
+        MANUAL_IS_POWER_OF_TWO,
+        expr.span,
+        "manually reimplementing `is_power_of_two`",
+        "consider using `.is_power_of_two()`",
+        format!("{snippet}.is_power_of_two()"),
+        *applicability,
+    );
+}
+
+fn check_lit(expr: &Expr<'_>, expected_num: u128) -> bool {
+    if let ExprKind::Lit(lit) = expr.kind
+        && let LitKind::Int(Pu128(num), _) = lit.node
+        && num == expected_num
+    {
+        return true;
+    }
+    false
+}
+
+fn check_eq_expr(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
+    SpanlessEq::new(cx).eq_expr(lhs, rhs)
+}

tests/ui/manual_is_power_of_two.fixed

@@ -6,6 +6,12 @@ fn main() {
     let _ = a.is_power_of_two();
     let _ = a.is_power_of_two();
 
+    // Test different orders of expression
+    let _ = a.is_power_of_two();
+    let _ = a.is_power_of_two();
+    let _ = a.is_power_of_two();
+    let _ = a.is_power_of_two();
+
     let b = 4_i64;
 
     // is_power_of_two only works for unsigned integers

tests/ui/manual_is_power_of_two.rs

@@ -6,6 +6,12 @@ fn main() {
     let _ = a.count_ones() == 1;
     let _ = a & (a - 1) == 0;
 
+    // Test different orders of expression
+    let _ = 1 == a.count_ones();
+    let _ = (a - 1) & a == 0;
+    let _ = 0 == a & (a - 1);
+    let _ = 0 == (a - 1) & a;
+
     let b = 4_i64;
 
     // is_power_of_two only works for unsigned integers

tests/ui/manual_is_power_of_two.stderr

@@ -13,5 +13,29 @@ error: manually reimplementing `is_power_of_two`
 LL |     let _ = a & (a - 1) == 0;
    |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
-error: aborting due to 2 previous errors
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:10:13
+   |
+LL |     let _ = 1 == a.count_ones();
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:11:13
+   |
+LL |     let _ = (a - 1) & a == 0;
+   |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:12:13
+   |
+LL |     let _ = 0 == a & (a - 1);
+   |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:13:13
+   |
+LL |     let _ = 0 == (a - 1) & a;
+   |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+
+error: aborting due to 6 previous errors