Skip to content

[refurb] Mark the FURB161 fix unsafe except for integers and booleans #17240

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

Merged
merged 7 commits into from
Apr 18, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def ten() -> int:
count = bin(ten()).count("1") # FURB161
count = bin((10)).count("1") # FURB161
count = bin("10" "15").count("1") # FURB161
count = bin("123").count("1") # FURB161

count = x.bit_count() # OK
count = (10).bit_count() # OK
Expand Down
25 changes: 19 additions & 6 deletions crates/ruff_linter/src/rules/refurb/rules/bit_count.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::PythonVersion;
use ruff_python_ast::{self as ast, Expr, ExprAttribute, ExprCall};
use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand All @@ -27,6 +28,11 @@ use crate::fix::snippet::SourceCodeSnippet;
/// y = 0b1111011.bit_count()
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe unless the argument to `bin` can be inferred as
/// an instance of a type that implements the `__index__` and `bit_count` methods because this can
/// change the exception raised at runtime for an invalid argument.
///
/// ## Options
/// - `target-version`
///
Expand Down Expand Up @@ -163,6 +169,14 @@ pub(crate) fn bit_count(checker: &Checker, call: &ExprCall) {
| Expr::Subscript(_) => false,
};

// check if the fix is safe or not
let applicability: Applicability = match ResolvedPythonType::from(arg) {
ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer | NumberLike::Bool)) => {
Applicability::Safe
}
_ => Applicability::Unsafe,
};

let replacement = if parenthesize {
format!("({literal_text}).bit_count()")
} else {
Expand All @@ -176,11 +190,10 @@ pub(crate) fn bit_count(checker: &Checker, call: &ExprCall) {
},
call.range(),
);

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

checker.report_diagnostic(diagnostic);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ FURB161.py:6:9: FURB161 [*] Use of `bin(x).count('1')`
|
= help: Replace with `(x).bit_count()`

Safe fix
Unsafe fix
3 3 | def ten() -> int:
4 4 | return 10
5 5 |
Expand Down Expand Up @@ -137,15 +137,15 @@ FURB161.py:12:9: FURB161 [*] Use of `bin(ten()).count('1')`
|
= help: Replace with `ten().bit_count()`

Safe fix
Unsafe 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 |
15 15 | count = bin("123").count("1") # FURB161

FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
|
Expand All @@ -154,6 +154,7 @@ FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
13 | count = bin((10)).count("1") # FURB161
| ^^^^^^^^^^^^^^^^^^^^ FURB161
14 | count = bin("10" "15").count("1") # FURB161
15 | count = bin("123").count("1") # FURB161
|
= help: Replace with `(10).bit_count()`

Expand All @@ -164,26 +165,46 @@ FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
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
15 15 | count = bin("123").count("1") # FURB161
16 16 |

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
15 | count = bin("123").count("1") # FURB161
|
= help: Replace with `("10" "15").bit_count()`

Safe fix
Unsafe 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
15 15 | count = bin("123").count("1") # FURB161
16 16 |
17 17 | count = x.bit_count() # OK

FURB161.py:15:9: FURB161 [*] Use of `bin("123").count('1')`
|
13 | count = bin((10)).count("1") # FURB161
14 | count = bin("10" "15").count("1") # FURB161
15 | count = bin("123").count("1") # FURB161
| ^^^^^^^^^^^^^^^^^^^^^ FURB161
16 |
17 | count = x.bit_count() # OK
|
= help: Replace with `"123".bit_count()`

ℹ Unsafe fix
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
15 |-count = bin("123").count("1") # FURB161
15 |+count = "123".bit_count() # FURB161
16 16 |
17 17 | count = x.bit_count() # OK
18 18 | count = (10).bit_count() # OK
Loading