diff --git a/README.md b/README.md index b4fda35a65..fa659237cf 100644 --- a/README.md +++ b/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). diff --git a/resources/test/fixtures/B004.py b/resources/test/fixtures/B004.py new file mode 100644 index 0000000000..6a6e8c86e6 --- /dev/null +++ b/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.") diff --git a/src/check_ast.rs b/src/check_ast.rs index 5b44e4ae80..66cbebbc56 100644 --- a/src/check_ast.rs +++ b/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( diff --git a/src/checks.rs b/src/checks.rs index 8a8671a30f..a4078ce064 100644 --- a/src/checks.rs +++ b/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() } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index dc324b70fe..a0b4f99ee2 100644 --- a/src/checks_gen.rs +++ b/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, diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index d47e27cdb1..94585f756b 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/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; diff --git a/src/flake8_bugbear/plugins/unreliable_callable_check.rs b/src/flake8_bugbear/plugins/unreliable_callable_check.rs new file mode 100644 index 0000000000..827f1ef06b --- /dev/null +++ b/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), + )); + } + } + } + } + } +} diff --git a/src/linter.rs b/src/linter.rs index ddd516234a..3d90730488 100644 --- a/src/linter.rs +++ b/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")] diff --git a/src/snapshots/ruff__linter__tests__B004_B004.py.snap b/src/snapshots/ruff__linter__tests__B004_B004.py.snap new file mode 100644 index 0000000000..ca80cc54ce --- /dev/null +++ b/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: ~ +