From 35d578f042614189e394890d51163ae6bb56e97a Mon Sep 17 00:00:00 2001 From: Steve C Date: Sat, 23 Dec 2023 16:50:31 -0500 Subject: [PATCH] [refurb] - implement `FURB161`/`use-bit-count` --- .../resources/test/fixtures/refurb/FURB161.py | 20 +++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/refurb/mod.rs | 1 + .../src/rules/refurb/rules/bit_count.rs | 140 ++++++++++++++++ .../ruff_linter/src/rules/refurb/rules/mod.rs | 2 + ...es__refurb__tests__FURB161_FURB161.py.snap | 149 ++++++++++++++++++ ruff.schema.json | 1 + 8 files changed, 317 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py create mode 100644 crates/ruff_linter/src/rules/refurb/rules/bit_count.rs create mode 100644 crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB161_FURB161.py.snap 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 00000000000000..6d4620e4991565 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py @@ -0,0 +1,20 @@ +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 = 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 \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index f42f577ea8b672..e5bb598e73085e 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 550960ce275afa..8614dcda65ad0a 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/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 5ce76cd3420bdc..35950e2a72b098 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 00000000000000..c655f76bb7dfca --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs @@ -0,0 +1,140 @@ +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, TextRange, TextSize}; + +use crate::{checkers::ast::Checker, settings::types::PythonVersion}; + +/// ## What it does +/// Checks for the use of `bin(x).count("1")` as a population count. +/// +/// ## Why is this bad? +/// Python 3.10 added the `int.bit_count()` method, which is more efficient. +/// +/// ## Example +/// ```python +/// x = bin(123).count("1") +/// y = bin(0b1111011).count("1") +/// ``` +/// +/// Use instead: +/// ```python +/// x = (123).bit_count() +/// y = 0b1111011.bit_count() +/// ``` +#[violation] +pub struct BitCount { + original_argument: String, + replacement: String, +} + +impl AlwaysFixableViolation for BitCount { + #[derive_message_formats] + fn message(&self) -> String { + let BitCount { + original_argument, .. + } = self; + format!("Use of bin({original_argument}).count('1')") + } + + fn fix_title(&self) -> String { + let BitCount { replacement, .. } = self; + format!("Replace with `{replacement}`") + } +} + +/// FURB161 +pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) { + if checker.settings.target_version < PythonVersion::Py310 { + // `int.bit_count()` was added in Python 3.10 + return; + } + + let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else { + return; + }; + + // make sure we're doing count + if attr.as_str() != "count" { + return; + } + + let Some(arg) = call.arguments.args.first() else { + return; + }; + + let Expr::StringLiteral(ast::ExprStringLiteral { + value: count_value, .. + }) = arg + else { + return; + }; + + // make sure we're doing count("1") + if count_value != "1" { + return; + } + + let Expr::Call(ExprCall { + func, arguments, .. + }) = value.as_ref() + else { + return; + }; + + let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { + return; + }; + + // make sure we're doing bin() + if id.as_str() != "bin" { + return; + } + + if arguments.len() != 1 { + return; + } + + let Some(arg) = arguments.args.first() else { + return; + }; + + let literal_text = checker.locator().slice(arg.range()); + + let replacement = match arg { + Expr::Name(ast::ExprName { id, .. }) => { + format!("{id}.bit_count()") + } + Expr::NumberLiteral(ast::ExprNumberLiteral { .. }) => { + let first_two_chars = checker + .locator() + .slice(TextRange::new(arg.start(), arg.start() + TextSize::from(2))); + + match first_two_chars { + "0b" | "0B" | "0x" | "0X" | "0o" | "0O" => format!("{literal_text}.bit_count()"), + _ => format!("({literal_text}).bit_count()"), + } + } + Expr::Call(ast::ExprCall { .. }) => { + format!("{literal_text}.bit_count()") + } + _ => { + format!("({literal_text}).bit_count()") + } + }; + + let mut diagnostic = Diagnostic::new( + BitCount { + original_argument: literal_text.to_string(), + replacement: replacement.clone(), + }, + 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 c5536a029b7b2d..ff4c02360f7f49 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 00000000000000..1ddfda9fb13141 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB161_FURB161.py.snap @@ -0,0 +1,149 @@ +--- +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 | + +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 + | + = 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 | +14 14 | count = x.bit_count() # OK + +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 | +14 | count = x.bit_count() # OK + | + = 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 | +14 14 | count = x.bit_count() # OK +15 15 | count = (10).bit_count() # OK + + diff --git a/ruff.schema.json b/ruff.schema.json index 3761d705180f7a..fb7e2af47f5873 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2924,6 +2924,7 @@ "FURB15", "FURB152", "FURB16", + "FURB161", "FURB163", "FURB168", "FURB169",