Skip to content

Commit

Permalink
[refurb] Implement bit-count (FURB161) (#9265)
Browse files Browse the repository at this point in the history
## 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`
  • Loading branch information
diceroll123 committed Dec 27, 2023
1 parent 5018701 commit c716acc
Show file tree
Hide file tree
Showing 9 changed files with 407 additions and 1 deletion.
22 changes: 22 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
x = 10

def ten() -> int:
return 10

count = bin(x).count("1") # FURB161
count = bin(10).count("1") # FURB161
count = bin(0b1010).count("1") # FURB161
count = bin(0xA).count("1") # FURB161
count = bin(0o12).count("1") # FURB161
count = bin(0x10 + 0x1000).count("1") # FURB161
count = bin(ten()).count("1") # FURB161
count = bin((10)).count("1") # FURB161
count = bin("10" "15").count("1") # FURB161

count = x.bit_count() # OK
count = (10).bit_count() # OK
count = 0b1010.bit_count() # OK
count = 0xA.bit_count() # OK
count = 0o12.bit_count() # OK
count = (0x10 + 0x1000).bit_count() # OK
count = ten().bit_count() # OK
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
}
if checker.enabled(Rule::BitCount) {
refurb::rules::bit_count(checker, call);
}
if checker.enabled(Rule::TypeOfPrimitive) {
pyupgrade::rules::type_of_primitive(checker, expr, func, args);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
(Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
(Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant),
(Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
(Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),
(Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::checkers::ast::Checker;
/// the `from __future__ import annotations` import statement has no effect
/// and should be omitted.
///
/// ## Resources
/// ## References
/// - [Static Typing with Python: Type Stubs](https://typing.readthedocs.io/en/latest/source/stubs.html)
#[violation]
pub struct FutureAnnotationsInStub;
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ mod tests {
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
#[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
#[test_case(Rule::BitCount, Path::new("FURB161.py"))]
#[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
#[test_case(Rule::RedundantLogBase, Path::new("FURB163.py"))]
Expand Down
185 changes: 185 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/bit_count.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, ExprAttribute, ExprCall};
use ruff_text_size::Ranged;

use crate::fix::snippet::SourceCodeSnippet;
use crate::{checkers::ast::Checker, settings::types::PythonVersion};

/// ## What it does
/// Checks for uses of `bin(...).count("1")` to perform a population count.
///
/// ## Why is this bad?
/// In Python 3.10, a `bit_count()` method was added to the `int` class,
/// which is more concise and efficient than converting to a binary
/// representation via `bin(...)`.
///
/// ## Example
/// ```python
/// x = bin(123).count("1")
/// y = bin(0b1111011).count("1")
/// ```
///
/// Use instead:
/// ```python
/// x = (123).bit_count()
/// y = 0b1111011.bit_count()
/// ```
///
/// ## Options
/// - `target-version`
///
/// ## References
/// - [Python documentation:`int.bit_count`](https://docs.python.org/3/library/stdtypes.html#int.bit_count)
#[violation]
pub struct BitCount {
existing: SourceCodeSnippet,
replacement: SourceCodeSnippet,
}

impl AlwaysFixableViolation for BitCount {
#[derive_message_formats]
fn message(&self) -> String {
let BitCount { existing, .. } = self;
let existing = existing.truncated_display();
format!("Use of `bin({existing}).count('1')`")
}

fn fix_title(&self) -> String {
let BitCount { replacement, .. } = self;
if let Some(replacement) = replacement.full_display() {
format!("Replace with `{replacement}`")
} else {
format!("Replace with `.bit_count()`")
}
}
}

/// FURB161
pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) {
// `int.bit_count()` was added in Python 3.10
if checker.settings.target_version < PythonVersion::Py310 {
return;
}

let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else {
return;
};

// Ensure that we're performing a `.count(...)`.
if attr.as_str() != "count" {
return;
}

if !call.arguments.keywords.is_empty() {
return;
};
let [arg] = call.arguments.args.as_slice() else {
return;
};

let Expr::StringLiteral(ast::ExprStringLiteral {
value: count_value, ..
}) = arg
else {
return;
};

// Ensure that we're performing a `.count("1")`.
if count_value != "1" {
return;
}

let Expr::Call(ExprCall {
func, arguments, ..
}) = value.as_ref()
else {
return;
};

// Ensure that we're performing a `bin(...)`.
if !checker
.semantic()
.resolve_call_path(func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "bin"]))
{
return;
}

if !arguments.keywords.is_empty() {
return;
};
let [arg] = arguments.args.as_slice() else {
return;
};

// Extract, e.g., `x` in `bin(x)`.
let literal_text = checker.locator().slice(arg);

// If we're calling a method on an integer, or an expression with lower precedence, parenthesize
// it.
let parenthesize = match arg {
Expr::NumberLiteral(ast::ExprNumberLiteral { .. }) => {
let mut chars = literal_text.chars();
!matches!(
(chars.next(), chars.next()),
(Some('0'), Some('b' | 'B' | 'x' | 'X' | 'o' | 'O'))
)
}

Expr::StringLiteral(inner) => inner.value.is_implicit_concatenated(),
Expr::BytesLiteral(inner) => inner.value.is_implicit_concatenated(),
Expr::FString(inner) => inner.value.is_implicit_concatenated(),

Expr::Await(_)
| Expr::Starred(_)
| Expr::UnaryOp(_)
| Expr::BinOp(_)
| Expr::BoolOp(_)
| Expr::IfExp(_)
| Expr::NamedExpr(_)
| Expr::Lambda(_)
| Expr::Slice(_)
| Expr::Yield(_)
| Expr::YieldFrom(_)
| Expr::Name(_)
| Expr::List(_)
| Expr::Compare(_)
| Expr::Tuple(_)
| Expr::GeneratorExp(_)
| Expr::IpyEscapeCommand(_) => true,

Expr::Call(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| Expr::EllipsisLiteral(_)
| Expr::Attribute(_)
| Expr::Subscript(_) => false,
};

let replacement = if parenthesize {
format!("({literal_text}).bit_count()")
} else {
format!("{literal_text}.bit_count()")
};

let mut diagnostic = Diagnostic::new(
BitCount {
existing: SourceCodeSnippet::from_str(literal_text),
replacement: SourceCodeSnippet::new(replacement.to_string()),
},
call.range(),
);

diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
replacement,
call.range(),
)));

checker.diagnostics.push(diagnostic);
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub(crate) use bit_count::*;
pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*;
pub(crate) use hashlib_digest_hex::*;
Expand All @@ -16,6 +17,7 @@ pub(crate) use slice_copy::*;
pub(crate) use type_none_comparison::*;
pub(crate) use unnecessary_enumerate::*;

mod bit_count;
mod check_and_remove_from_set;
mod delete_full_slice;
mod hashlib_digest_hex;
Expand Down

0 comments on commit c716acc

Please sign in to comment.