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
This commit is contained in:
Justin Prieto 2023-07-15 21:32:21 -04:00 committed by GitHub
parent 06b5c6c06f
commit f012ed2d77
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 12 deletions

View File

@ -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);
}

View File

@ -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()`