Skip to content

Commit fb9913e

Browse files
borsbors
committed
Auto merge of rust-lang#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 rust-lang/rust-clippy#12894 ---- changelog: new [`manual_is_power_of_two`] lint
2 parents 7d7b298 + f994797 commit fb9913e

7 files changed

+227
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5626,6 +5626,7 @@ Released 2018-09-13
56265626
[`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
56275627
[`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite
56285628
[`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite
5629+
[`manual_is_power_of_two`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two
56295630
[`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and
56305631
[`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
56315632
[`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
306306
crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
307307
crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
308308
crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
309+
crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO,
309310
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
310311
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
311312
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ mod manual_div_ceil;
207207
mod manual_float_methods;
208208
mod manual_hash_one;
209209
mod manual_is_ascii_check;
210+
mod manual_is_power_of_two;
210211
mod manual_let_else;
211212
mod manual_main_separator_str;
212213
mod manual_non_exhaustive;
@@ -938,5 +939,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
938939
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
939940
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
940941
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
942+
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
941943
// add lints here, do not remove this comment, it's used in `new_lint`
942944
}

‎clippy_lints/src/manual_is_power_of_two.rs

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
3+
use clippy_utils::SpanlessEq;
4+
use rustc_ast::LitKind;
5+
use rustc_data_structures::packed::Pu128;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{BinOpKind, Expr, ExprKind};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::ty::Uint;
10+
use rustc_session::declare_lint_pass;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for expressions like `x.count_ones() == 1` or `x & (x - 1) == 0`, with x and unsigned integer, which are manual
15+
/// reimplementations of `x.is_power_of_two()`.
16+
/// ### Why is this bad?
17+
/// Manual reimplementations of `is_power_of_two` increase code complexity for little benefit.
18+
/// ### Example
19+
/// ```no_run
20+
/// let a: u32 = 4;
21+
/// let result = a.count_ones() == 1;
22+
/// ```
23+
/// Use instead:
24+
/// ```no_run
25+
/// let a: u32 = 4;
26+
/// let result = a.is_power_of_two();
27+
/// ```
28+
#[clippy::version = "1.82.0"]
29+
pub MANUAL_IS_POWER_OF_TWO,
30+
complexity,
31+
"manually reimplementing `is_power_of_two`"
32+
}
33+
34+
declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
35+
36+
impl LateLintPass<'_> for ManualIsPowerOfTwo {
37+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
38+
let mut applicability = Applicability::MachineApplicable;
39+
40+
if let ExprKind::Binary(bin_op, left, right) = expr.kind
41+
&& bin_op.node == BinOpKind::Eq
42+
{
43+
// a.count_ones() == 1
44+
if let ExprKind::MethodCall(method_name, reciever, _, _) = left.kind
45+
&& method_name.ident.as_str() == "count_ones"
46+
&& let &Uint(_) = cx.typeck_results().expr_ty(reciever).kind()
47+
&& check_lit(right, 1)
48+
{
49+
build_sugg(cx, expr, reciever, &mut applicability);
50+
}
51+
52+
// 1 == a.count_ones()
53+
if let ExprKind::MethodCall(method_name, reciever, _, _) = right.kind
54+
&& method_name.ident.as_str() == "count_ones"
55+
&& let &Uint(_) = cx.typeck_results().expr_ty(reciever).kind()
56+
&& check_lit(left, 1)
57+
{
58+
build_sugg(cx, expr, reciever, &mut applicability);
59+
}
60+
61+
// a & (a - 1) == 0
62+
if let ExprKind::Binary(op1, left1, right1) = left.kind
63+
&& op1.node == BinOpKind::BitAnd
64+
&& let ExprKind::Binary(op2, left2, right2) = right1.kind
65+
&& op2.node == BinOpKind::Sub
66+
&& check_eq_expr(cx, left1, left2)
67+
&& let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
68+
&& check_lit(right2, 1)
69+
&& check_lit(right, 0)
70+
{
71+
build_sugg(cx, expr, left1, &mut applicability);
72+
}
73+
74+
// (a - 1) & a == 0;
75+
if let ExprKind::Binary(op1, left1, right1) = left.kind
76+
&& op1.node == BinOpKind::BitAnd
77+
&& let ExprKind::Binary(op2, left2, right2) = left1.kind
78+
&& op2.node == BinOpKind::Sub
79+
&& check_eq_expr(cx, right1, left2)
80+
&& let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
81+
&& check_lit(right2, 1)
82+
&& check_lit(right, 0)
83+
{
84+
build_sugg(cx, expr, right1, &mut applicability);
85+
}
86+
87+
// 0 == a & (a - 1);
88+
if let ExprKind::Binary(op1, left1, right1) = right.kind
89+
&& op1.node == BinOpKind::BitAnd
90+
&& let ExprKind::Binary(op2, left2, right2) = right1.kind
91+
&& op2.node == BinOpKind::Sub
92+
&& check_eq_expr(cx, left1, left2)
93+
&& let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
94+
&& check_lit(right2, 1)
95+
&& check_lit(left, 0)
96+
{
97+
build_sugg(cx, expr, left1, &mut applicability);
98+
}
99+
100+
// 0 == (a - 1) & a
101+
if let ExprKind::Binary(op1, left1, right1) = right.kind
102+
&& op1.node == BinOpKind::BitAnd
103+
&& let ExprKind::Binary(op2, left2, right2) = left1.kind
104+
&& op2.node == BinOpKind::Sub
105+
&& check_eq_expr(cx, right1, left2)
106+
&& let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
107+
&& check_lit(right2, 1)
108+
&& check_lit(left, 0)
109+
{
110+
build_sugg(cx, expr, right1, &mut applicability);
111+
}
112+
}
113+
}
114+
}
115+
116+
fn build_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, reciever: &Expr<'_>, applicability: &mut Applicability) {
117+
let snippet = snippet_with_applicability(cx, reciever.span, "..", applicability);
118+
119+
span_lint_and_sugg(
120+
cx,
121+
MANUAL_IS_POWER_OF_TWO,
122+
expr.span,
123+
"manually reimplementing `is_power_of_two`",
124+
"consider using `.is_power_of_two()`",
125+
format!("{snippet}.is_power_of_two()"),
126+
*applicability,
127+
);
128+
}
129+
130+
fn check_lit(expr: &Expr<'_>, expected_num: u128) -> bool {
131+
if let ExprKind::Lit(lit) = expr.kind
132+
&& let LitKind::Int(Pu128(num), _) = lit.node
133+
&& num == expected_num
134+
{
135+
return true;
136+
}
137+
false
138+
}
139+
140+
fn check_eq_expr(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
141+
SpanlessEq::new(cx).eq_expr(lhs, rhs)
142+
}

‎tests/ui/manual_is_power_of_two.fixed

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![warn(clippy::manual_is_power_of_two)]
2+
3+
fn main() {
4+
let a = 16_u64;
5+
6+
let _ = a.is_power_of_two();
7+
let _ = a.is_power_of_two();
8+
9+
// Test different orders of expression
10+
let _ = a.is_power_of_two();
11+
let _ = a.is_power_of_two();
12+
let _ = a.is_power_of_two();
13+
let _ = a.is_power_of_two();
14+
15+
let b = 4_i64;
16+
17+
// is_power_of_two only works for unsigned integers
18+
let _ = b.count_ones() == 1;
19+
let _ = b & (b - 1) == 0;
20+
}

‎tests/ui/manual_is_power_of_two.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![warn(clippy::manual_is_power_of_two)]
2+
3+
fn main() {
4+
let a = 16_u64;
5+
6+
let _ = a.count_ones() == 1;
7+
let _ = a & (a - 1) == 0;
8+
9+
// Test different orders of expression
10+
let _ = 1 == a.count_ones();
11+
let _ = (a - 1) & a == 0;
12+
let _ = 0 == a & (a - 1);
13+
let _ = 0 == (a - 1) & a;
14+
15+
let b = 4_i64;
16+
17+
// is_power_of_two only works for unsigned integers
18+
let _ = b.count_ones() == 1;
19+
let _ = b & (b - 1) == 0;
20+
}

‎tests/ui/manual_is_power_of_two.stderr

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
error: manually reimplementing `is_power_of_two`
2+
--> tests/ui/manual_is_power_of_two.rs:6:13
3+
|
4+
LL | let _ = a.count_ones() == 1;
5+
| ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
6+
|
7+
= note: `-D clippy::manual-is-power-of-two` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::manual_is_power_of_two)]`
9+
10+
error: manually reimplementing `is_power_of_two`
11+
--> tests/ui/manual_is_power_of_two.rs:7:13
12+
|
13+
LL | let _ = a & (a - 1) == 0;
14+
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
15+
16+
error: manually reimplementing `is_power_of_two`
17+
--> tests/ui/manual_is_power_of_two.rs:10:13
18+
|
19+
LL | let _ = 1 == a.count_ones();
20+
| ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
21+
22+
error: manually reimplementing `is_power_of_two`
23+
--> tests/ui/manual_is_power_of_two.rs:11:13
24+
|
25+
LL | let _ = (a - 1) & a == 0;
26+
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
27+
28+
error: manually reimplementing `is_power_of_two`
29+
--> tests/ui/manual_is_power_of_two.rs:12:13
30+
|
31+
LL | let _ = 0 == a & (a - 1);
32+
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
33+
34+
error: manually reimplementing `is_power_of_two`
35+
--> tests/ui/manual_is_power_of_two.rs:13:13
36+
|
37+
LL | let _ = 0 == (a - 1) & a;
38+
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
39+
40+
error: aborting due to 6 previous errors
41+

0 commit comments

Comments
 (0)