Add support for different orders of expression
This commit is contained in:
parent
6ecb48f6d0
commit
f994797e46
@ -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 @@
|
||||
|
||||
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)
|
||||
}
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user