From f8061e8b99585fb9affe02ec23c193f920fc96ca Mon Sep 17 00:00:00 2001 From: Vasco Schiavo <115561717+VascoSch92@users.noreply.github.com> Date: Fri, 18 Apr 2025 19:46:01 +0200 Subject: [PATCH] [`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 --- .../resources/test/fixtures/refurb/FURB161.py | 1 + .../src/rules/refurb/rules/bit_count.rs | 25 ++++++++--- ...es__refurb__tests__FURB161_FURB161.py.snap | 43 ++++++++++++++----- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py index 0736b9dc3a..6dda8a11c9 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB161.py @@ -12,6 +12,7 @@ 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 = bin("123").count("1") # FURB161 count = x.bit_count() # OK count = (10).bit_count() # OK diff --git a/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs index 863ea3cdd4..4de3c44a96 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/bit_count.rs @@ -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; @@ -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` /// @@ -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 { @@ -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); } 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 index 7888f39f7d..2bff822228 100644 --- 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 @@ -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 | @@ -137,7 +137,7 @@ 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 @@ -145,7 +145,7 @@ FURB161.py:12:9: FURB161 [*] Use of `bin(ten()).count('1')` 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')` | @@ -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()` @@ -164,8 +165,8 @@ 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')` | @@ -173,17 +174,37 @@ FURB161.py:14:9: FURB161 [*] Use of `bin("10" "15").count('1')` 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