Skip to content

Commit f7a741a

Browse files
authored
[flake8-bugbear] Mark autofix for B004 as unsafe if the hasattr call expr contains comments (astral-sh#18755)
1 parent 4c8d612 commit f7a741a

File tree

3 files changed

+64
-2
lines changed

3 files changed

+64
-2
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,13 @@ def this_is_fine():
2929
o = object()
3030
if callable(o):
3131
print("Ooh, this is actually callable.")
32+
33+
# https://github.com/astral-sh/ruff/issues/18741
34+
# The autofix for this is unsafe due to the comments.
35+
hasattr(
36+
# comment 1
37+
obj, # comment 2
38+
# comment 3
39+
"__call__", # comment 4
40+
# comment 5
41+
)

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use ruff_diagnostics::Applicability;
12
use ruff_macros::{ViolationMetadata, derive_message_formats};
23
use ruff_python_ast::{self as ast, Expr};
34
use ruff_text_size::Ranged;
@@ -26,6 +27,21 @@ use crate::{Edit, Fix, FixAvailability, Violation};
2627
/// callable(obj)
2728
/// ```
2829
///
30+
/// ## 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.
33+
///
34+
/// For example, the fix would be marked as unsafe in the following case:
35+
/// ```python
36+
/// hasattr(
37+
/// # comment 1
38+
/// obj, # comment 2
39+
/// # comment 3
40+
/// "__call__", # comment 4
41+
/// # comment 5
42+
/// )
43+
/// ```
44+
///
2945
/// ## References
3046
/// - [Python documentation: `callable`](https://docs.python.org/3/library/functions.html#callable)
3147
/// - [Python documentation: `hasattr`](https://docs.python.org/3/library/functions.html#hasattr)
@@ -84,7 +100,15 @@ pub(crate) fn unreliable_callable_check(
84100
format!("{binding}({})", checker.locator().slice(obj)),
85101
expr.range(),
86102
);
87-
Ok(Fix::safe_edits(binding_edit, import_edit))
103+
Ok(Fix::applicable_edits(
104+
binding_edit,
105+
import_edit,
106+
if checker.comment_ranges().intersects(expr.range()) {
107+
Applicability::Unsafe
108+
} else {
109+
Applicability::Safe
110+
},
111+
))
88112
});
89113
}
90114
}

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,32 @@ B004.py:24:8: B004 [*] Using `hasattr(x, "__call__")` to test if x is callable i
8585
25 |+ if builtins.callable(o):
8686
25 26 | print("STILL a bug!")
8787
26 27 |
88-
27 28 |
88+
27 28 |
89+
90+
B004.py:35:1: B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
91+
|
92+
33 | # https://github.com/astral-sh/ruff/issues/18741
93+
34 | # The autofix for this is unsafe due to the comments.
94+
35 | / hasattr(
95+
36 | | # comment 1
96+
37 | | obj, # comment 2
97+
38 | | # comment 3
98+
39 | | "__call__", # comment 4
99+
40 | | # comment 5
100+
41 | | )
101+
| |_^ B004
102+
|
103+
= help: Replace with `callable()`
104+
105+
Unsafe fix
106+
32 32 |
107+
33 33 | # https://github.com/astral-sh/ruff/issues/18741
108+
34 34 | # The autofix for this is unsafe due to the comments.
109+
35 |-hasattr(
110+
36 |- # comment 1
111+
37 |- obj, # comment 2
112+
38 |- # comment 3
113+
39 |- "__call__", # comment 4
114+
40 |- # comment 5
115+
41 |-)
116+
35 |+callable(obj)

0 commit comments

Comments
 (0)