Auto merge of #13327 - Sour1emon:master, r=llogiq

Add new lint `manual_is_power_of_two`

Suggest using `is_power_of_two()` instead of the manual implementations for some basic cases

Part of https://github.com/rust-lang/rust-clippy/issues/12894

----

changelog: new [`manual_is_power_of_two`] lint

CHANGELOG.md

@@ -5626,6 +5626,7 @@ Released 2018-09-13
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite
 [`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite
+[`manual_is_power_of_two`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two
 [`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and
 [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
 [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str

clippy_lints/src/declared_lints.rs

@@ -306,6 +306,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
     crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
     crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
+    crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO,
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,

clippy_lints/src/lib.rs

@@ -207,6 +207,7 @@ mod manual_div_ceil;
 mod manual_float_methods;
 mod manual_hash_one;
 mod manual_is_ascii_check;
+mod manual_is_power_of_two;
 mod manual_let_else;
 mod manual_main_separator_str;
 mod manual_non_exhaustive;
@@ -938,5 +939,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
     store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
     store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
+    store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/manual_is_power_of_two.rs

@@ -0,0 +1,142 @@
+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;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Uint;
+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`, 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 a: u32 = 4;
+    /// let result = a.count_ones() == 1;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let a: u32 = 4;
+    /// let result = a.is_power_of_two();
+    /// ```
+    #[clippy::version = "1.82.0"]
+    pub MANUAL_IS_POWER_OF_TWO,
+    complexity,
+    "manually reimplementing `is_power_of_two`"
+}
+
+declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
+
+impl LateLintPass<'_> for ManualIsPowerOfTwo {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        let mut applicability = Applicability::MachineApplicable;
+
+        if let ExprKind::Binary(bin_op, left, right) = expr.kind
+            && bin_op.node == BinOpKind::Eq
+        {
+            // 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);
+            }
+
+            // 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

@@ -0,0 +1,20 @@
+#![warn(clippy::manual_is_power_of_two)]
+
+fn main() {
+    let a = 16_u64;
+
+    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
+    let _ = b.count_ones() == 1;
+    let _ = b & (b - 1) == 0;
+}

tests/ui/manual_is_power_of_two.rs

@@ -0,0 +1,20 @@
+#![warn(clippy::manual_is_power_of_two)]
+
+fn main() {
+    let a = 16_u64;
+
+    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
+    let _ = b.count_ones() == 1;
+    let _ = b & (b - 1) == 0;
+}

tests/ui/manual_is_power_of_two.stderr

@@ -0,0 +1,41 @@
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:6:13
+   |
+LL |     let _ = a.count_ones() == 1;
+   |             ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+   |
+   = note: `-D clippy::manual-is-power-of-two` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_is_power_of_two)]`
+
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:7:13
+   |
+LL |     let _ = a & (a - 1) == 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: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
+