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

The PR fixes #16457 .

Specifically, `FURB161` is marked safe, but the rule generates safe
fixes only in specific cases. Therefore, we attempt to mark the fix as
unsafe when we are not in one of these cases.

For instances, the fix is marked as aunsafe just in case of strings (as
pointed out in the issue). Let me know if I should change something.

---------

Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
Vasco Schiavo 2025-04-18 19:46:01 +02:00 committed by GitHub
parent 27a315b740
commit f8061e8b99
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 52 additions and 17 deletions

View file

@ -12,6 +12,7 @@ count = bin(0x10 + 0x1000).count("1") # FURB161
count = bin(ten()).count("1") # FURB161 count = bin(ten()).count("1") # FURB161
count = bin((10)).count("1") # FURB161 count = bin((10)).count("1") # FURB161
count = bin("10" "15").count("1") # FURB161 count = bin("10" "15").count("1") # FURB161
count = bin("123").count("1") # FURB161
count = x.bit_count() # OK count = x.bit_count() # OK
count = (10).bit_count() # OK count = (10).bit_count() # OK

View file

@ -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_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::PythonVersion; use ruff_python_ast::PythonVersion;
use ruff_python_ast::{self as ast, Expr, ExprAttribute, ExprCall}; 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 ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -27,6 +28,11 @@ use crate::fix::snippet::SourceCodeSnippet;
/// y = 0b1111011.bit_count() /// 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 /// ## Options
/// - `target-version` /// - `target-version`
/// ///
@ -163,6 +169,14 @@ pub(crate) fn bit_count(checker: &Checker, call: &ExprCall) {
| Expr::Subscript(_) => false, | 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 { let replacement = if parenthesize {
format!("({literal_text}).bit_count()") format!("({literal_text}).bit_count()")
} else { } else {
@ -176,11 +190,10 @@ pub(crate) fn bit_count(checker: &Checker, call: &ExprCall) {
}, },
call.range(), call.range(),
); );
diagnostic.set_fix(Fix::applicable_edit(
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( Edit::range_replacement(replacement, call.range()),
replacement, applicability,
call.range(), ));
)));
checker.report_diagnostic(diagnostic); checker.report_diagnostic(diagnostic);
} }

View file

@ -12,7 +12,7 @@ FURB161.py:6:9: FURB161 [*] Use of `bin(x).count('1')`
| |
= help: Replace with `(x).bit_count()` = help: Replace with `(x).bit_count()`
Safe fix Unsafe fix
3 3 | def ten() -> int: 3 3 | def ten() -> int:
4 4 | return 10 4 4 | return 10
5 5 | 5 5 |
@ -137,7 +137,7 @@ FURB161.py:12:9: FURB161 [*] Use of `bin(ten()).count('1')`
| |
= help: Replace with `ten().bit_count()` = help: Replace with `ten().bit_count()`
Safe fix Unsafe fix
9 9 | count = bin(0xA).count("1") # FURB161 9 9 | count = bin(0xA).count("1") # FURB161
10 10 | count = bin(0o12).count("1") # FURB161 10 10 | count = bin(0o12).count("1") # FURB161
11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161
@ -145,7 +145,7 @@ FURB161.py:12:9: FURB161 [*] Use of `bin(ten()).count('1')`
12 |+count = ten().bit_count() # FURB161 12 |+count = ten().bit_count() # FURB161
13 13 | count = bin((10)).count("1") # FURB161 13 13 | count = bin((10)).count("1") # FURB161
14 14 | count = bin("10" "15").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')` FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
| |
@ -154,6 +154,7 @@ FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
13 | count = bin((10)).count("1") # FURB161 13 | count = bin((10)).count("1") # FURB161
| ^^^^^^^^^^^^^^^^^^^^ FURB161 | ^^^^^^^^^^^^^^^^^^^^ FURB161
14 | count = bin("10" "15").count("1") # FURB161 14 | count = bin("10" "15").count("1") # FURB161
15 | count = bin("123").count("1") # FURB161
| |
= help: Replace with `(10).bit_count()` = help: Replace with `(10).bit_count()`
@ -164,8 +165,8 @@ FURB161.py:13:9: FURB161 [*] Use of `bin(10).count('1')`
13 |-count = bin((10)).count("1") # FURB161 13 |-count = bin((10)).count("1") # FURB161
13 |+count = (10).bit_count() # FURB161 13 |+count = (10).bit_count() # FURB161
14 14 | count = bin("10" "15").count("1") # FURB161 14 14 | count = bin("10" "15").count("1") # FURB161
15 15 | 15 15 | count = bin("123").count("1") # FURB161
16 16 | count = x.bit_count() # OK 16 16 |
FURB161.py:14:9: FURB161 [*] Use of `bin("10" "15").count('1')` FURB161.py:14:9: FURB161 [*] Use of `bin("10" "15").count('1')`
| |
@ -173,17 +174,37 @@ FURB161.py:14:9: FURB161 [*] Use of `bin("10" "15").count('1')`
13 | count = bin((10)).count("1") # FURB161 13 | count = bin((10)).count("1") # FURB161
14 | count = bin("10" "15").count("1") # FURB161 14 | count = bin("10" "15").count("1") # FURB161
| ^^^^^^^^^^^^^^^^^^^^^^^^^ FURB161 | ^^^^^^^^^^^^^^^^^^^^^^^^^ FURB161
15 | 15 | count = bin("123").count("1") # FURB161
16 | count = x.bit_count() # OK
| |
= help: Replace with `("10" "15").bit_count()` = help: Replace with `("10" "15").bit_count()`
Safe fix Unsafe fix
11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161 11 11 | count = bin(0x10 + 0x1000).count("1") # FURB161
12 12 | count = bin(ten()).count("1") # FURB161 12 12 | count = bin(ten()).count("1") # FURB161
13 13 | count = bin((10)).count("1") # FURB161 13 13 | count = bin((10)).count("1") # FURB161
14 |-count = bin("10" "15").count("1") # FURB161 14 |-count = bin("10" "15").count("1") # FURB161
14 |+count = ("10" "15").bit_count() # FURB161 14 |+count = ("10" "15").bit_count() # FURB161
15 15 | 15 15 | count = bin("123").count("1") # FURB161
16 16 | count = x.bit_count() # OK 16 16 |
17 17 | count = (10).bit_count() # OK 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