Skip to content

Commit dfec946

Browse files
IDrokin117Igor DrokinntBre
authored
[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 <drokinii1017@gmail.com> Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
1 parent ff677a9 commit dfec946

File tree

3 files changed

+80
-8
lines changed

3 files changed

+80
-8
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B004.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,16 @@ def this_is_fine():
3939
"__call__", # comment 4
4040
# comment 5
4141
)
42+
43+
import operator
44+
45+
assert hasattr(operator, "__call__")
46+
assert callable(operator) is False
47+
48+
49+
class A:
50+
def __init__(self): self.__call__ = None
51+
52+
53+
assert hasattr(A(), "__call__")
54+
assert callable(A()) is False

crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,29 @@ use crate::{Edit, Fix, FixAvailability, Violation};
2828
/// ```
2929
///
3030
/// ## Fix safety
31-
/// This rule's fix is marked as unsafe if there's comments in the `hasattr` call
32-
/// expression, as comments may be removed.
31+
/// This rule's fix is marked as unsafe because the replacement may not be semantically
32+
/// equivalent to the original expression, potentially changing the behavior of the code.
3333
///
34-
/// For example, the fix would be marked as unsafe in the following case:
34+
/// For example, an imported module may have a `__call__` attribute but is not considered
35+
/// a callable object:
36+
/// ```python
37+
/// import operator
38+
///
39+
/// assert hasattr(operator, "__call__")
40+
/// assert callable(operator) is False
41+
/// ```
42+
/// Additionally, `__call__` may be defined only as an instance method:
43+
/// ```python
44+
/// class A:
45+
/// def __init__(self):
46+
/// self.__call__ = None
47+
///
48+
///
49+
/// assert hasattr(A(), "__call__")
50+
/// assert callable(A()) is False
51+
/// ```
52+
///
53+
/// Additionally, if there are comments in the `hasattr` call expression, they may be removed:
3554
/// ```python
3655
/// hasattr(
3756
/// # comment 1
@@ -103,11 +122,7 @@ pub(crate) fn unreliable_callable_check(
103122
Ok(Fix::applicable_edits(
104123
binding_edit,
105124
import_edit,
106-
if checker.comment_ranges().intersects(expr.range()) {
107-
Applicability::Unsafe
108-
} else {
109-
Applicability::Safe
110-
},
125+
Applicability::Unsafe,
111126
))
112127
});
113128
}

crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B004_B004.py.snap

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ help: Replace with `callable()`
1919
4 | print("Ooh, callable! Or is it?")
2020
5 | if getattr(o, "__call__", False):
2121
6 | print("Ooh, callable! Or is it?")
22+
note: This is an unsafe fix and may change runtime behavior
2223

2324
B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
2425
--> B004.py:5:8
@@ -50,6 +51,7 @@ help: Replace with `callable()`
5051
13 | print("B U G")
5152
14 | if builtins.getattr(o, "__call__", False):
5253
15 | print("B U G")
54+
note: This is an unsafe fix and may change runtime behavior
5355

5456
B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
5557
--> B004.py:14:8
@@ -85,6 +87,7 @@ help: Replace with `callable()`
8587
26 | print("STILL a bug!")
8688
27 |
8789
28 |
90+
note: This is an unsafe fix and may change runtime behavior
8891

8992
B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
9093
--> B004.py:35:1
@@ -99,6 +102,8 @@ B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable.
99102
40 | | # comment 5
100103
41 | | )
101104
| |_^
105+
42 |
106+
43 | import operator
102107
|
103108
help: Replace with `callable()`
104109
32 |
@@ -112,4 +117,43 @@ help: Replace with `callable()`
112117
- # comment 5
113118
- )
114119
35 + callable(obj)
120+
36 |
121+
37 | import operator
122+
38 |
123+
note: This is an unsafe fix and may change runtime behavior
124+
125+
B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
126+
--> B004.py:45:8
127+
|
128+
43 | import operator
129+
44 |
130+
45 | assert hasattr(operator, "__call__")
131+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
132+
46 | assert callable(operator) is False
133+
|
134+
help: Replace with `callable()`
135+
42 |
136+
43 | import operator
137+
44 |
138+
- assert hasattr(operator, "__call__")
139+
45 + assert callable(operator)
140+
46 | assert callable(operator) is False
141+
47 |
142+
48 |
143+
note: This is an unsafe fix and may change runtime behavior
144+
145+
B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
146+
--> B004.py:53:8
147+
|
148+
53 | assert hasattr(A(), "__call__")
149+
| ^^^^^^^^^^^^^^^^^^^^^^^^
150+
54 | assert callable(A()) is False
151+
|
152+
help: Replace with `callable()`
153+
50 | def __init__(self): self.__call__ = None
154+
51 |
155+
52 |
156+
- assert hasattr(A(), "__call__")
157+
53 + assert callable(A())
158+
54 | assert callable(A()) is False
115159
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)