From dfec94608c3bbda687c48713e6b11991a16d331e Mon Sep 17 00:00:00 2001 From: Igor Drokin <41319097+IDrokin117@users.noreply.github.com> Date: Fri, 12 Sep 2025 22:27:17 +0300 Subject: [PATCH] [`flake8-bugbear`] Mark the fix for `unreliable-callable-check` as always unsafe (`B004`) (#20318) ## Summary Resolves #20282 Makes the rule fix always unsafe, because the replacement may not be semantically equivalent to the original expression, potentially changing the behavior of the code. Updated docstring with examples. ## Test Plan - Added two tests from issue and regenerated the snapshot --------- Co-authored-by: Igor Drokin Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/flake8_bugbear/B004.py | 13 ++++++ .../rules/unreliable_callable_check.rs | 31 +++++++++---- ...__flake8_bugbear__tests__B004_B004.py.snap | 44 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py index 3a81f8c28b..b064101ed4 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py @@ -39,3 +39,16 @@ hasattr( "__call__", # comment 4 # comment 5 ) + +import operator + +assert hasattr(operator, "__call__") +assert callable(operator) is False + + +class A: + def __init__(self): self.__call__ = None + + +assert hasattr(A(), "__call__") +assert callable(A()) is False diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index fbeea00d97..b432ed369d 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -28,10 +28,29 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// ``` /// /// ## Fix safety -/// This rule's fix is marked as unsafe if there's comments in the `hasattr` call -/// expression, as comments may be removed. +/// This rule's fix is marked as unsafe because the replacement may not be semantically +/// equivalent to the original expression, potentially changing the behavior of the code. /// -/// For example, the fix would be marked as unsafe in the following case: +/// For example, an imported module may have a `__call__` attribute but is not considered +/// a callable object: +/// ```python +/// import operator +/// +/// assert hasattr(operator, "__call__") +/// assert callable(operator) is False +/// ``` +/// Additionally, `__call__` may be defined only as an instance method: +/// ```python +/// class A: +/// def __init__(self): +/// self.__call__ = None +/// +/// +/// assert hasattr(A(), "__call__") +/// assert callable(A()) is False +/// ``` +/// +/// Additionally, if there are comments in the `hasattr` call expression, they may be removed: /// ```python /// hasattr( /// # comment 1 @@ -103,11 +122,7 @@ pub(crate) fn unreliable_callable_check( Ok(Fix::applicable_edits( binding_edit, import_edit, - if checker.comment_ranges().intersects(expr.range()) { - Applicability::Unsafe - } else { - Applicability::Safe - }, + Applicability::Unsafe, )) }); } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap index 45fa6d7593..8170d5bcad 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap @@ -19,6 +19,7 @@ help: Replace with `callable()` 4 | print("Ooh, callable! Or is it?") 5 | if getattr(o, "__call__", False): 6 | print("Ooh, callable! Or is it?") +note: This is an unsafe fix and may change runtime behavior B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. --> B004.py:5:8 @@ -50,6 +51,7 @@ help: Replace with `callable()` 13 | print("B U G") 14 | if builtins.getattr(o, "__call__", False): 15 | print("B U G") +note: This is an unsafe fix and may change runtime behavior B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. --> B004.py:14:8 @@ -85,6 +87,7 @@ help: Replace with `callable()` 26 | print("STILL a bug!") 27 | 28 | +note: This is an unsafe fix and may change runtime behavior B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. --> B004.py:35:1 @@ -99,6 +102,8 @@ B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. 40 | | # comment 5 41 | | ) | |_^ +42 | +43 | import operator | help: Replace with `callable()` 32 | @@ -112,4 +117,43 @@ help: Replace with `callable()` - # comment 5 - ) 35 + callable(obj) +36 | +37 | import operator +38 | +note: This is an unsafe fix and may change runtime behavior + +B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. + --> B004.py:45:8 + | +43 | import operator +44 | +45 | assert hasattr(operator, "__call__") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +46 | assert callable(operator) is False + | +help: Replace with `callable()` +42 | +43 | import operator +44 | + - assert hasattr(operator, "__call__") +45 + assert callable(operator) +46 | assert callable(operator) is False +47 | +48 | +note: This is an unsafe fix and may change runtime behavior + +B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results. + --> B004.py:53:8 + | +53 | assert hasattr(A(), "__call__") + | ^^^^^^^^^^^^^^^^^^^^^^^^ +54 | assert callable(A()) is False + | +help: Replace with `callable()` +50 | def __init__(self): self.__call__ = None +51 | +52 | + - assert hasattr(A(), "__call__") +53 + assert callable(A()) +54 | assert callable(A()) is False note: This is an unsafe fix and may change runtime behavior