diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py new file mode 100644 index 0000000000000..a18600c8b7eb7 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py @@ -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 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index f42f577ea8b67..e5bb598e73085 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -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); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 550960ce275af..8614dcda65ad0 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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), diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs index bdbaf4a274c53..41ea12a377feb 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/future_annotations_in_stub.rs @@ -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; diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 5ce76cd3420bd..35950e2a72b09 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -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"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs new file mode 100644 index 0000000000000..6a1bbf3dddd9e --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs @@ -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); +} diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index c5536a029b7b2..ff4c02360f7f4 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -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::*; @@ -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; diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB161_FURB161.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB161_FURB161.py.snap new file mode 100644 index 0000000000000..0085558579ae4 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB161_FURB161.py.snap @@ -0,0 +1,191 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB161.py:6:9: FURB161 [*] Use of `bin(x).count('1')` + | +4 | return 10 +5 | +6 | count = bin(x).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^ FURB161 +7 | count = bin(10).count("1") # FURB161 +8 | count = bin(0b1010).count("1") # FURB161 + | + = help: Replace with `(x).bit_count()` + +ℹ Safe fix +3 3 | def ten() -> int: +4 4 | return 10 +5 5 | +6 |-count = bin(x).count("1") # FURB161 + 6 |+count = (x).bit_count() # FURB161 +7 7 | count = bin(10).count("1") # FURB161 +8 8 | count = bin(0b1010).count("1") # FURB161 +9 9 | count = bin(0xA).count("1") # FURB161 + +FURB161.py:7:9: FURB161 [*] Use of `bin(10).count('1')` + | +6 | count = bin(x).count("1") # FURB161 +7 | count = bin(10).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^ FURB161 +8 | count = bin(0b1010).count("1") # FURB161 +9 | count = bin(0xA).count("1") # FURB161 + | + = help: Replace with `(10).bit_count()` + +ℹ Safe fix +4 4 | return 10 +5 5 | +6 6 | count = bin(x).count("1") # FURB161 +7 |-count = bin(10).count("1") # FURB161 + 7 |+count = (10).bit_count() # FURB161 +8 8 | count = bin(0b1010).count("1") # FURB161 +9 9 | count = bin(0xA).count("1") # FURB161 +10 10 | count = bin(0o12).count("1") # FURB161 + +FURB161.py:8:9: FURB161 [*] Use of `bin(0b1010).count('1')` + | + 6 | count = bin(x).count("1") # FURB161 + 7 | count = bin(10).count("1") # FURB161 + 8 | count = bin(0b1010).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^^^ FURB161 + 9 | count = bin(0xA).count("1") # FURB161 +10 | count = bin(0o12).count("1") # FURB161 + | + = help: Replace with `0b1010.bit_count()` + +ℹ Safe fix +5 5 | +6 6 | count = bin(x).count("1") # FURB161 +7 7 | count = bin(10).count("1") # FURB161 +8 |-count = bin(0b1010).count("1") # FURB161 + 8 |+count = 0b1010.bit_count() # FURB161 +9 9 | count = bin(0xA).count("1") # FURB161 +10 10 | count = bin(0o12).count("1") # FURB161 +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 + +FURB161.py:9:9: FURB161 [*] Use of `bin(0xA).count('1')` + | + 7 | count = bin(10).count("1") # FURB161 + 8 | count = bin(0b1010).count("1") # FURB161 + 9 | count = bin(0xA).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^ FURB161 +10 | count = bin(0o12).count("1") # FURB161 +11 | count = bin(0x10 + 0x1000).count("1") # FURB161 + | + = help: Replace with `0xA.bit_count()` + +ℹ Safe fix +6 6 | count = bin(x).count("1") # FURB161 +7 7 | count = bin(10).count("1") # FURB161 +8 8 | count = bin(0b1010).count("1") # FURB161 +9 |-count = bin(0xA).count("1") # FURB161 + 9 |+count = 0xA.bit_count() # FURB161 +10 10 | count = bin(0o12).count("1") # FURB161 +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 12 | count = bin(ten()).count("1") # FURB161 + +FURB161.py:10:9: FURB161 [*] Use of `bin(0o12).count('1')` + | + 8 | count = bin(0b1010).count("1") # FURB161 + 9 | count = bin(0xA).count("1") # FURB161 +10 | count = bin(0o12).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^ FURB161 +11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 | count = bin(ten()).count("1") # FURB161 + | + = help: Replace with `0o12.bit_count()` + +ℹ Safe fix +7 7 | count = bin(10).count("1") # FURB161 +8 8 | count = bin(0b1010).count("1") # FURB161 +9 9 | count = bin(0xA).count("1") # FURB161 +10 |-count = bin(0o12).count("1") # FURB161 + 10 |+count = 0o12.bit_count() # FURB161 +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 12 | count = bin(ten()).count("1") # FURB161 +13 13 | count = bin((10)).count("1") # FURB161 + +FURB161.py:11:9: FURB161 [*] Use of `bin(0x10 + 0x1000).count('1')` + | + 9 | count = bin(0xA).count("1") # FURB161 +10 | count = bin(0o12).count("1") # FURB161 +11 | count = bin(0x10 + 0x1000).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB161 +12 | count = bin(ten()).count("1") # FURB161 +13 | count = bin((10)).count("1") # FURB161 + | + = help: Replace with `(0x10 + 0x1000).bit_count()` + +ℹ Safe fix +8 8 | count = bin(0b1010).count("1") # FURB161 +9 9 | count = bin(0xA).count("1") # FURB161 +10 10 | count = bin(0o12).count("1") # FURB161 +11 |-count = bin(0x10 + 0x1000).count("1") # FURB161 + 11 |+count = (0x10 + 0x1000).bit_count() # FURB161 +12 12 | count = bin(ten()).count("1") # FURB161 +13 13 | count = bin((10)).count("1") # FURB161 +14 14 | count = bin("10" "15").count("1") # FURB161 + +FURB161.py:12:9: FURB161 [*] Use of `bin(ten()).count('1')` + | +10 | count = bin(0o12).count("1") # FURB161 +11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 | count = bin(ten()).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^^ FURB161 +13 | count = bin((10)).count("1") # FURB161 +14 | count = bin("10" "15").count("1") # FURB161 + | + = help: Replace with `ten().bit_count()` + +ℹ Safe fix +9 9 | count = bin(0xA).count("1") # FURB161 +10 10 | count = bin(0o12).count("1") # FURB161 +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 |-count = bin(ten()).count("1") # FURB161 + 12 |+count = ten().bit_count() # FURB161 +13 13 | count = bin((10)).count("1") # FURB161 +14 14 | count = bin("10" "15").count("1") # FURB161 +15 15 | + +FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')` + | +11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 | count = bin(ten()).count("1") # FURB161 +13 | count = bin((10)).count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^ FURB161 +14 | count = bin("10" "15").count("1") # FURB161 + | + = help: Replace with `(10).bit_count()` + +ℹ Safe fix +10 10 | count = bin(0o12).count("1") # FURB161 +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 12 | count = bin(ten()).count("1") # FURB161 +13 |-count = bin((10)).count("1") # FURB161 + 13 |+count = (10).bit_count() # FURB161 +14 14 | count = bin("10" "15").count("1") # FURB161 +15 15 | +16 16 | count = x.bit_count() # OK + +FURB161.py:14:9: FURB161 [*] Use of `bin("10" "15").count('1')` + | +12 | count = bin(ten()).count("1") # FURB161 +13 | count = bin((10)).count("1") # FURB161 +14 | count = bin("10" "15").count("1") # FURB161 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ FURB161 +15 | +16 | count = x.bit_count() # OK + | + = help: Replace with `("10" "15").bit_count()` + +ℹ Safe fix +11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 +12 12 | count = bin(ten()).count("1") # FURB161 +13 13 | count = bin((10)).count("1") # FURB161 +14 |-count = bin("10" "15").count("1") # FURB161 + 14 |+count = ("10" "15").bit_count() # FURB161 +15 15 | +16 16 | count = x.bit_count() # OK +17 17 | count = (10).bit_count() # OK + + diff --git a/ruff.schema.json b/ruff.schema.json index 3761d705180f7..fb7e2af47f587 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2924,6 +2924,7 @@ "FURB15", "FURB152", "FURB16", + "FURB161", "FURB163", "FURB168", "FURB169",