Skip to content

Commit c716acc

Browse files
authored
[refurb] Implement bit-count (FURB161) (#9265)
## Summary Implements [`FURB161`/`use-bit-count`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/use_bit_count.py) See: #1348 ## Test Plan `cargo test`
1 parent 5018701 commit c716acc

File tree

9 files changed

+407
-1
lines changed

9 files changed

+407
-1
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
x = 10
2+
3+
def ten() -> int:
4+
return 10
5+
6+
count = bin(x).count("1") # FURB161
7+
count = bin(10).count("1") # FURB161
8+
count = bin(0b1010).count("1") # FURB161
9+
count = bin(0xA).count("1") # FURB161
10+
count = bin(0o12).count("1") # FURB161
11+
count = bin(0x10 + 0x1000).count("1") # FURB161
12+
count = bin(ten()).count("1") # FURB161
13+
count = bin((10)).count("1") # FURB161
14+
count = bin("10" "15").count("1") # FURB161
15+
16+
count = x.bit_count() # OK
17+
count = (10).bit_count() # OK
18+
count = 0b1010.bit_count() # OK
19+
count = 0xA.bit_count() # OK
20+
count = 0o12.bit_count() # OK
21+
count = (0x10 + 0x1000).bit_count() # OK
22+
count = ten().bit_count() # OK

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
438438
}
439439
}
440440
}
441+
if checker.enabled(Rule::BitCount) {
442+
refurb::rules::bit_count(checker, call);
443+
}
441444
if checker.enabled(Rule::TypeOfPrimitive) {
442445
pyupgrade::rules::type_of_primitive(checker, expr, func, args);
443446
}

crates/ruff_linter/src/codes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
964964
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
965965
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
966966
(Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant),
967+
(Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
967968
(Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),
968969
(Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
969970
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),

crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::checkers::ast::Checker;
1414
/// the `from __future__ import annotations` import statement has no effect
1515
/// and should be omitted.
1616
///
17-
/// ## Resources
17+
/// ## References
1818
/// - [Static Typing with Python: Type Stubs](https://typing.readthedocs.io/en/latest/source/stubs.html)
1919
#[violation]
2020
pub struct FutureAnnotationsInStub;

crates/ruff_linter/src/rules/refurb/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ mod tests {
2727
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
2828
#[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
2929
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
30+
#[test_case(Rule::BitCount, Path::new("FURB161.py"))]
3031
#[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
3132
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
3233
#[test_case(Rule::RedundantLogBase, Path::new("FURB163.py"))]
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
2+
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast::{self as ast, Expr, ExprAttribute, ExprCall};
4+
use ruff_text_size::Ranged;
5+
6+
use crate::fix::snippet::SourceCodeSnippet;
7+
use crate::{checkers::ast::Checker, settings::types::PythonVersion};
8+
9+
/// ## What it does
10+
/// Checks for uses of `bin(...).count("1")` to perform a population count.
11+
///
12+
/// ## Why is this bad?
13+
/// In Python 3.10, a `bit_count()` method was added to the `int` class,
14+
/// which is more concise and efficient than converting to a binary
15+
/// representation via `bin(...)`.
16+
///
17+
/// ## Example
18+
/// ```python
19+
/// x = bin(123).count("1")
20+
/// y = bin(0b1111011).count("1")
21+
/// ```
22+
///
23+
/// Use instead:
24+
/// ```python
25+
/// x = (123).bit_count()
26+
/// y = 0b1111011.bit_count()
27+
/// ```
28+
///
29+
/// ## Options
30+
/// - `target-version`
31+
///
32+
/// ## References
33+
/// - [Python documentation:`int.bit_count`](https://docs.python.org/3/library/stdtypes.html#int.bit_count)
34+
#[violation]
35+
pub struct BitCount {
36+
existing: SourceCodeSnippet,
37+
replacement: SourceCodeSnippet,
38+
}
39+
40+
impl AlwaysFixableViolation for BitCount {
41+
#[derive_message_formats]
42+
fn message(&self) -> String {
43+
let BitCount { existing, .. } = self;
44+
let existing = existing.truncated_display();
45+
format!("Use of `bin({existing}).count('1')`")
46+
}
47+
48+
fn fix_title(&self) -> String {
49+
let BitCount { replacement, .. } = self;
50+
if let Some(replacement) = replacement.full_display() {
51+
format!("Replace with `{replacement}`")
52+
} else {
53+
format!("Replace with `.bit_count()`")
54+
}
55+
}
56+
}
57+
58+
/// FURB161
59+
pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) {
60+
// `int.bit_count()` was added in Python 3.10
61+
if checker.settings.target_version < PythonVersion::Py310 {
62+
return;
63+
}
64+
65+
let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else {
66+
return;
67+
};
68+
69+
// Ensure that we're performing a `.count(...)`.
70+
if attr.as_str() != "count" {
71+
return;
72+
}
73+
74+
if !call.arguments.keywords.is_empty() {
75+
return;
76+
};
77+
let [arg] = call.arguments.args.as_slice() else {
78+
return;
79+
};
80+
81+
let Expr::StringLiteral(ast::ExprStringLiteral {
82+
value: count_value, ..
83+
}) = arg
84+
else {
85+
return;
86+
};
87+
88+
// Ensure that we're performing a `.count("1")`.
89+
if count_value != "1" {
90+
return;
91+
}
92+
93+
let Expr::Call(ExprCall {
94+
func, arguments, ..
95+
}) = value.as_ref()
96+
else {
97+
return;
98+
};
99+
100+
// Ensure that we're performing a `bin(...)`.
101+
if !checker
102+
.semantic()
103+
.resolve_call_path(func)
104+
.is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "bin"]))
105+
{
106+
return;
107+
}
108+
109+
if !arguments.keywords.is_empty() {
110+
return;
111+
};
112+
let [arg] = arguments.args.as_slice() else {
113+
return;
114+
};
115+
116+
// Extract, e.g., `x` in `bin(x)`.
117+
let literal_text = checker.locator().slice(arg);
118+
119+
// If we're calling a method on an integer, or an expression with lower precedence, parenthesize
120+
// it.
121+
let parenthesize = match arg {
122+
Expr::NumberLiteral(ast::ExprNumberLiteral { .. }) => {
123+
let mut chars = literal_text.chars();
124+
!matches!(
125+
(chars.next(), chars.next()),
126+
(Some('0'), Some('b' | 'B' | 'x' | 'X' | 'o' | 'O'))
127+
)
128+
}
129+
130+
Expr::StringLiteral(inner) => inner.value.is_implicit_concatenated(),
131+
Expr::BytesLiteral(inner) => inner.value.is_implicit_concatenated(),
132+
Expr::FString(inner) => inner.value.is_implicit_concatenated(),
133+
134+
Expr::Await(_)
135+
| Expr::Starred(_)
136+
| Expr::UnaryOp(_)
137+
| Expr::BinOp(_)
138+
| Expr::BoolOp(_)
139+
| Expr::IfExp(_)
140+
| Expr::NamedExpr(_)
141+
| Expr::Lambda(_)
142+
| Expr::Slice(_)
143+
| Expr::Yield(_)
144+
| Expr::YieldFrom(_)
145+
| Expr::Name(_)
146+
| Expr::List(_)
147+
| Expr::Compare(_)
148+
| Expr::Tuple(_)
149+
| Expr::GeneratorExp(_)
150+
| Expr::IpyEscapeCommand(_) => true,
151+
152+
Expr::Call(_)
153+
| Expr::Dict(_)
154+
| Expr::Set(_)
155+
| Expr::ListComp(_)
156+
| Expr::SetComp(_)
157+
| Expr::DictComp(_)
158+
| Expr::BooleanLiteral(_)
159+
| Expr::NoneLiteral(_)
160+
| Expr::EllipsisLiteral(_)
161+
| Expr::Attribute(_)
162+
| Expr::Subscript(_) => false,
163+
};
164+
165+
let replacement = if parenthesize {
166+
format!("({literal_text}).bit_count()")
167+
} else {
168+
format!("{literal_text}.bit_count()")
169+
};
170+
171+
let mut diagnostic = Diagnostic::new(
172+
BitCount {
173+
existing: SourceCodeSnippet::from_str(literal_text),
174+
replacement: SourceCodeSnippet::new(replacement.to_string()),
175+
},
176+
call.range(),
177+
);
178+
179+
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
180+
replacement,
181+
call.range(),
182+
)));
183+
184+
checker.diagnostics.push(diagnostic);
185+
}

crates/ruff_linter/src/rules/refurb/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub(crate) use bit_count::*;
12
pub(crate) use check_and_remove_from_set::*;
23
pub(crate) use delete_full_slice::*;
34
pub(crate) use hashlib_digest_hex::*;
@@ -16,6 +17,7 @@ pub(crate) use slice_copy::*;
1617
pub(crate) use type_none_comparison::*;
1718
pub(crate) use unnecessary_enumerate::*;
1819

20+
mod bit_count;
1921
mod check_and_remove_from_set;
2022
mod delete_full_slice;
2123
mod hashlib_digest_hex;

0 commit comments

Comments
 (0)