Skip to content

Commit

Permalink
Add support for different orders of expression
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sour1emon
Sour1emon committed Sep 6, 2024
1 parent 28b6d7f commit 348a7f8
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 51 deletions.
154 changes: 104 additions & 50 deletions clippy_lints/src/manual_is_power_of_two.rs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
Expand All @@ -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)
}
6 changes: 6 additions & 0 deletions tests/ui/manual_is_power_of_two.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/manual_is_power_of_two.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 25 additions & 1 deletion tests/ui/manual_is_power_of_two.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 348a7f8

Please sign in to comment.