From f012ed2d77272bfdd75f6988c2658e36fbf63102 Mon Sep 17 00:00:00 2001
From: Justin Prieto <density@users.noreply.github.com>
Date: Sat, 15 Jul 2023 21:32:21 -0400
Subject: [PATCH] Add autofix for B004 (#5788)

## Summary

Adds autofix for `hasattr` case of B004. I don't think it's safe (or
simple) to implement it for the `getattr` case because, inter alia,
calling `getattr` may have side effects.

Fixes #3545

## Test Plan

Existing tests were sufficient. Updated snapshots
---
 .../rules/unreliable_callable_check.rs        | 37 ++++++++++++++-----
 ...__flake8_bugbear__tests__B004_B004.py.snap | 15 +++++++-
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/crates/ruff/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs
index 34e209ee48..64e0066be3 100644
--- a/crates/ruff/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs
+++ b/crates/ruff/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs
@@ -1,9 +1,10 @@
 use rustpython_parser::ast::{self, Constant, Expr, Ranged};
 
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 
 use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
 
 /// ## What it does
 /// Checks for uses of `hasattr` to test if an object is callable (e.g.,
@@ -35,13 +36,19 @@ use crate::checkers::ast::Checker;
 pub struct UnreliableCallableCheck;
 
 impl Violation for UnreliableCallableCheck {
+    const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         format!(
-            "Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use \
+            "Using `hasattr(x, \"__call__\")` to test if x is callable is unreliable. Use \
              `callable(x)` for consistent results."
         )
     }
+
+    fn autofix_title(&self) -> Option<String> {
+        Some(format!("Replace with `callable()`"))
+    }
 }
 
 /// B004
@@ -54,23 +61,33 @@ pub(crate) fn unreliable_callable_check(
     let Expr::Name(ast::ExprName { id, .. }) = func else {
         return;
     };
-    if id != "getattr" && id != "hasattr" {
+    if !matches!(id.as_str(), "hasattr" | "getattr") {
         return;
     }
-    if args.len() < 2 {
+    let [obj, attr, ..] = args else {
         return;
     };
     let Expr::Constant(ast::ExprConstant {
-        value: Constant::Str(s),
+        value: Constant::Str(string),
         ..
-    }) = &args[1]
+    }) = attr
     else {
         return;
     };
-    if s != "__call__" {
+    if string != "__call__" {
         return;
     }
-    checker
-        .diagnostics
-        .push(Diagnostic::new(UnreliableCallableCheck, expr.range()));
+
+    let mut diagnostic = Diagnostic::new(UnreliableCallableCheck, expr.range());
+    if checker.patch(diagnostic.kind.rule()) {
+        if id == "hasattr" {
+            if checker.semantic().is_builtin("callable") {
+                diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
+                    format!("callable({})", checker.locator.slice(obj.range())),
+                    expr.range(),
+                )));
+            }
+        }
+    }
+    checker.diagnostics.push(diagnostic);
 }
diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B004_B004.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B004_B004.py.snap
index 8715d504c8..8ed1636e2f 100644
--- a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B004_B004.py.snap
+++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B004_B004.py.snap
@@ -1,7 +1,7 @@
 ---
 source: crates/ruff/src/rules/flake8_bugbear/mod.rs
 ---
-B004.py:3:8: B004 Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
+B004.py:3:8: B004 [*] Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
   |
 1 | def this_is_a_bug():
 2 |     o = object()
@@ -10,8 +10,18 @@ B004.py:3:8: B004 Using `hasattr(x, '__call__')` to test if x is callable is unr
 4 |         print("Ooh, callable! Or is it?")
 5 |     if getattr(o, "__call__", False):
   |
+  = help: Replace with `callable()`
 
-B004.py:5:8: B004 Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
+ℹ Fix
+1 1 | def this_is_a_bug():
+2 2 |     o = object()
+3   |-    if hasattr(o, "__call__"):
+  3 |+    if callable(o):
+4 4 |         print("Ooh, callable! Or is it?")
+5 5 |     if getattr(o, "__call__", False):
+6 6 |         print("Ooh, callable! Or is it?")
+
+B004.py:5:8: B004 Using `hasattr(x, "__call__")` to test if x is callable is unreliable. Use `callable(x)` for consistent results.
   |
 3 |     if hasattr(o, "__call__"):
 4 |         print("Ooh, callable! Or is it?")
@@ -19,5 +29,6 @@ B004.py:5:8: B004 Using `hasattr(x, '__call__')` to test if x is callable is unr
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B004
 6 |         print("Ooh, callable! Or is it?")
   |
+  = help: Replace with `callable()`