Implement B004 (#638)

README.md

@@ -482,6 +482,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/
 | ---- | ---- | ------- | --- |
 | B002 | UnaryPrefixIncrement | Python does not support the unary prefix increment. |  |
 | B003 | AssignmentToOsEnviron | Assigning to `os.environ` doesn't clear the environment. |  |
+| B004 | UnreliableCallableCheck |  Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use `callable(x)` for consistent results. |  |
 | B006 | MutableArgumentDefault | Do not use mutable data structures for argument defaults. |  |
 | B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 |
 | B008 | FunctionCallArgumentDefault | Do not perform function calls in argument defaults. |  |
@@ -625,7 +626,7 @@ including:
 - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
 - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
-- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (15/32)
+- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (16/32)
 - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (11/34)
 - [`autoflake`](https://pypi.org/project/autoflake/) (1/7)
 
@@ -648,7 +649,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
 - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
 - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
-- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (15/32)
+- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (16/32)
 
 Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa),
 and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (11/34).

resources/test/fixtures/B004.py

@@ -0,0 +1,12 @@
+def this_is_a_bug():
+    o = object()
+    if hasattr(o, "__call__"):
+        print("Ooh, callable! Or is it?")
+    if getattr(o, "__call__", False):
+        print("Ooh, callable! Or is it?")
+
+
+def this_is_fine():
+    o = object()
+    if callable(o):
+        print("Ooh, this is actually callable.")

src/check_ast.rs

@@ -1042,6 +1042,10 @@ where
                     flake8_print::plugins::print_call(self, expr, func);
                 }
 
+                if self.settings.enabled.contains(&CheckCode::B004) {
+                    flake8_bugbear::plugins::unreliable_callable_check(self, expr, func, args);
+                }
+
                 // flake8-comprehensions
                 if self.settings.enabled.contains(&CheckCode::C400) {
                     if let Some(check) = flake8_comprehensions::checks::unnecessary_generator_list(

src/checks.rs

@@ -79,6 +79,7 @@ pub enum CheckCode {
     // flake8-bugbear
     B002,
     B003,
+    B004,
     B006,
     B007,
     B008,
@@ -337,6 +338,7 @@ pub enum CheckKind {
     // flake8-bugbear
     UnaryPrefixIncrement,
     AssignmentToOsEnviron,
+    UnreliableCallableCheck,
     MutableArgumentDefault,
     UnusedLoopControlVariable(String),
     FunctionCallArgumentDefault,
@@ -541,6 +543,7 @@ impl CheckCode {
             // flake8-bugbear
             CheckCode::B002 => CheckKind::UnaryPrefixIncrement,
             CheckCode::B003 => CheckKind::AssignmentToOsEnviron,
+            CheckCode::B004 => CheckKind::UnreliableCallableCheck,
             CheckCode::B006 => CheckKind::MutableArgumentDefault,
             CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()),
             CheckCode::B008 => CheckKind::FunctionCallArgumentDefault,
@@ -752,6 +755,7 @@ impl CheckCode {
             CheckCode::A003 => CheckCategory::Flake8Builtins,
             CheckCode::B002 => CheckCategory::Flake8Bugbear,
             CheckCode::B003 => CheckCategory::Flake8Bugbear,
+            CheckCode::B004 => CheckCategory::Flake8Bugbear,
             CheckCode::B006 => CheckCategory::Flake8Bugbear,
             CheckCode::B007 => CheckCategory::Flake8Bugbear,
             CheckCode::B008 => CheckCategory::Flake8Bugbear,
@@ -927,6 +931,7 @@ impl CheckKind {
             // flake8-bugbear
             CheckKind::UnaryPrefixIncrement => &CheckCode::B002,
             CheckKind::AssignmentToOsEnviron => &CheckCode::B003,
+            CheckKind::UnreliableCallableCheck => &CheckCode::B004,
             CheckKind::MutableArgumentDefault => &CheckCode::B006,
             CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007,
             CheckKind::FunctionCallArgumentDefault => &CheckCode::B008,
@@ -1211,6 +1216,10 @@ impl CheckKind {
             CheckKind::AssignmentToOsEnviron => {
                 "Assigning to `os.environ` doesn't clear the environment.".to_string()
             }
+            CheckKind::UnreliableCallableCheck => " Using `hasattr(x, '__call__')` to test if x \
+                                                   is callable is unreliable. Use `callable(x)` \
+                                                   for consistent results."
+                .to_string(),
             CheckKind::MutableArgumentDefault => {
                 "Do not use mutable data structures for argument defaults.".to_string()
             }

src/checks_gen.rs

@@ -35,6 +35,7 @@ pub enum CheckCodePrefix {
     B00,
     B002,
     B003,
+    B004,
     B006,
     B007,
     B008,
@@ -317,6 +318,7 @@ impl CheckCodePrefix {
             CheckCodePrefix::B => vec![
                 CheckCode::B002,
                 CheckCode::B003,
+                CheckCode::B004,
                 CheckCode::B006,
                 CheckCode::B007,
                 CheckCode::B008,
@@ -332,6 +334,7 @@ impl CheckCodePrefix {
             CheckCodePrefix::B0 => vec![
                 CheckCode::B002,
                 CheckCode::B003,
+                CheckCode::B004,
                 CheckCode::B006,
                 CheckCode::B007,
                 CheckCode::B008,
@@ -347,12 +350,14 @@ impl CheckCodePrefix {
             CheckCodePrefix::B00 => vec![
                 CheckCode::B002,
                 CheckCode::B003,
+                CheckCode::B004,
                 CheckCode::B006,
                 CheckCode::B007,
                 CheckCode::B008,
             ],
             CheckCodePrefix::B002 => vec![CheckCode::B002],
             CheckCodePrefix::B003 => vec![CheckCode::B003],
+            CheckCodePrefix::B004 => vec![CheckCode::B004],
             CheckCodePrefix::B006 => vec![CheckCode::B006],
             CheckCodePrefix::B007 => vec![CheckCode::B007],
             CheckCodePrefix::B008 => vec![CheckCode::B008],
@@ -1010,6 +1015,7 @@ impl CheckCodePrefix {
             CheckCodePrefix::B00 => PrefixSpecificity::Tens,
             CheckCodePrefix::B002 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B003 => PrefixSpecificity::Explicit,
+            CheckCodePrefix::B004 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B006 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B007 => PrefixSpecificity::Explicit,
             CheckCodePrefix::B008 => PrefixSpecificity::Explicit,

src/flake8_bugbear/plugins/mod.rs

@@ -7,6 +7,7 @@ pub use function_call_argument_default::function_call_argument_default;
 pub use mutable_argument_default::mutable_argument_default;
 pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler;
 pub use unary_prefix_increment::unary_prefix_increment;
+pub use unreliable_callable_check::unreliable_callable_check;
 pub use unused_loop_control_variable::unused_loop_control_variable;
 pub use useless_comparison::useless_comparison;
 pub use useless_expression::useless_expression;
@@ -20,6 +21,7 @@ mod function_call_argument_default;
 mod mutable_argument_default;
 mod redundant_tuple_in_exception_handler;
 mod unary_prefix_increment;
+mod unreliable_callable_check;
 mod unused_loop_control_variable;
 mod useless_comparison;
 mod useless_expression;

src/flake8_bugbear/plugins/unreliable_callable_check.rs

@@ -0,0 +1,27 @@
+use rustpython_ast::{Constant, Expr, ExprKind};
+
+use crate::ast::types::Range;
+use crate::check_ast::Checker;
+use crate::checks::{Check, CheckKind};
+
+/// B004
+pub fn unreliable_callable_check(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
+    if let ExprKind::Name { id, .. } = &func.node {
+        if id == "getattr" || id == "hasattr" {
+            if args.len() >= 2 {
+                if let ExprKind::Constant {
+                    value: Constant::Str(s),
+                    ..
+                } = &args[1].node
+                {
+                    if s == "__call__" {
+                        checker.add_check(Check::new(
+                            CheckKind::UnreliableCallableCheck,
+                            Range::from_located(expr),
+                        ));
+                    }
+                }
+            }
+        }
+    }
+}

src/linter.rs

@@ -294,6 +294,7 @@ mod tests {
     #[test_case(CheckCode::A003, Path::new("A003.py"); "A003")]
     #[test_case(CheckCode::B002, Path::new("B002.py"); "B002")]
     #[test_case(CheckCode::B003, Path::new("B003.py"); "B003")]
+    #[test_case(CheckCode::B004, Path::new("B004.py"); "B004")]
     #[test_case(CheckCode::B006, Path::new("B006_B008.py"); "B006")]
     #[test_case(CheckCode::B007, Path::new("B007.py"); "B007")]
     #[test_case(CheckCode::B008, Path::new("B006_B008.py"); "B008")]

src/snapshots/ruff__linter__tests__B004_B004.py.snap

@@ -0,0 +1,21 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind: UnreliableCallableCheck
+  location:
+    row: 3
+    column: 7
+  end_location:
+    row: 3
+    column: 29
+  fix: ~
+- kind: UnreliableCallableCheck
+  location:
+    row: 5
+    column: 7
+  end_location:
+    row: 5
+    column: 36
+  fix: ~
+