Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refurb] Implement bit-count (FURB161) #9265

Merged
merged 3 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading
Loading