diff --git a/README.md b/README.md index b64e47c52c..a81f87362a 100644 --- a/README.md +++ b/README.md @@ -464,6 +464,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | B002 | UnaryPrefixIncrement | Python does not support the unary prefix increment. | | | 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. | | | B011 | DoNotAssertFalse | Do not `assert False` (`python -O` removes these calls), raise `AssertionError()` | 🛠 | | B013 | RedundantTupleInExceptionHandler | A length-one tuple literal is redundant. Write `except ValueError:` instead of `except (ValueError,):`. | | | B014 | DuplicateHandlerException | Exception handler with duplicate exception: `ValueError` | 🛠 | @@ -577,7 +578,7 @@ including: - [`flake8-print`](https://pypi.org/project/flake8-print/) - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (12/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (13/32) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (10/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -599,7 +600,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-print`](https://pypi.org/project/flake8-print/) - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (12/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (13/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/) (10/34). diff --git a/src/ast/types.rs b/src/ast/types.rs index 77abc8e4e0..b44fd869f6 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -36,6 +36,7 @@ pub enum ScopeKind { Function(FunctionScope), Generator, Module, + Arg, } #[derive(Clone, Debug)] diff --git a/src/check_ast.rs b/src/check_ast.rs index e1c32935a0..4e0b46107b 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1663,6 +1663,9 @@ where if self.settings.enabled.contains(&CheckCode::B006) { flake8_bugbear::plugins::mutable_argument_default(self, arguments) } + if self.settings.enabled.contains(&CheckCode::B008) { + flake8_bugbear::plugins::function_call_argument_default(self, arguments) + } // Bind, but intentionally avoid walking default expressions, as we handle them // upstream. diff --git a/src/checks.rs b/src/checks.rs index 70501e1ec6..a8b8ad0592 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -80,6 +80,7 @@ pub enum CheckCode { B002, B006, B007, + B008, B011, B013, B014, @@ -297,6 +298,7 @@ pub enum CheckKind { UnaryPrefixIncrement, MutableArgumentDefault, UnusedLoopControlVariable(String), + FunctionCallArgumentDefault, DoNotAssertFalse, RedundantTupleInExceptionHandler(String), DuplicateHandlerException(Vec), @@ -484,6 +486,7 @@ impl CheckCode { CheckCode::B002 => CheckKind::UnaryPrefixIncrement, CheckCode::B006 => CheckKind::MutableArgumentDefault, CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()), + CheckCode::B008 => CheckKind::FunctionCallArgumentDefault, CheckCode::B011 => CheckKind::DoNotAssertFalse, CheckCode::B013 => { CheckKind::RedundantTupleInExceptionHandler("ValueError".to_string()) @@ -680,6 +683,7 @@ impl CheckCode { CheckCode::B002 => CheckCategory::Flake8Bugbear, CheckCode::B006 => CheckCategory::Flake8Bugbear, CheckCode::B007 => CheckCategory::Flake8Bugbear, + CheckCode::B008 => CheckCategory::Flake8Bugbear, CheckCode::B011 => CheckCategory::Flake8Bugbear, CheckCode::B013 => CheckCategory::Flake8Bugbear, CheckCode::B014 => CheckCategory::Flake8Bugbear, @@ -841,6 +845,7 @@ impl CheckKind { CheckKind::UnaryPrefixIncrement => &CheckCode::B002, CheckKind::MutableArgumentDefault => &CheckCode::B006, CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007, + CheckKind::FunctionCallArgumentDefault => &CheckCode::B008, CheckKind::DoNotAssertFalse => &CheckCode::B011, CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013, CheckKind::DuplicateHandlerException(_) => &CheckCode::B014, @@ -1113,6 +1118,9 @@ impl CheckKind { "Loop control variable `{name}` not used within the loop body. If this is \ intended, start the name with an underscore." ), + CheckKind::FunctionCallArgumentDefault => { + "Do not perform function calls in argument defaults.".to_string() + } CheckKind::DoNotAssertFalse => "Do not `assert False` (`python -O` removes these \ calls), raise `AssertionError()`" .to_string(), diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 7a78a968d0..3f6571dc3d 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -19,6 +19,7 @@ pub enum CheckCodePrefix { B002, B006, B007, + B008, B01, B011, B013, @@ -257,6 +258,7 @@ impl CheckCodePrefix { CheckCode::B002, CheckCode::B006, CheckCode::B007, + CheckCode::B008, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -269,6 +271,7 @@ impl CheckCodePrefix { CheckCode::B002, CheckCode::B006, CheckCode::B007, + CheckCode::B008, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -277,10 +280,11 @@ impl CheckCodePrefix { CheckCode::B018, CheckCode::B025, ], - CheckCodePrefix::B00 => vec![CheckCode::B002, CheckCode::B006, CheckCode::B007], + CheckCodePrefix::B00 => vec![CheckCode::B002, CheckCode::B006, CheckCode::B008], CheckCodePrefix::B002 => vec![CheckCode::B002], CheckCodePrefix::B006 => vec![CheckCode::B006], CheckCodePrefix::B007 => vec![CheckCode::B007], + CheckCodePrefix::B008 => vec![CheckCode::B008], CheckCodePrefix::B01 => vec![ CheckCode::B011, CheckCode::B013, @@ -913,6 +917,7 @@ impl CheckCodePrefix { CheckCodePrefix::B002 => PrefixSpecificity::Explicit, CheckCodePrefix::B006 => PrefixSpecificity::Explicit, CheckCodePrefix::B007 => PrefixSpecificity::Explicit, + CheckCodePrefix::B008 => PrefixSpecificity::Explicit, CheckCodePrefix::B01 => PrefixSpecificity::Tens, CheckCodePrefix::B011 => PrefixSpecificity::Explicit, CheckCodePrefix::B013 => PrefixSpecificity::Explicit, diff --git a/src/flake8_bugbear/plugins/function_call_argument_default.rs b/src/flake8_bugbear/plugins/function_call_argument_default.rs new file mode 100644 index 0000000000..290dee3e58 --- /dev/null +++ b/src/flake8_bugbear/plugins/function_call_argument_default.rs @@ -0,0 +1,96 @@ +use rustpython_ast::{Arguments, Constant, Expr, ExprKind}; + +use crate::ast::helpers::compose_call_path; +use crate::ast::types::{CheckLocator, Range}; +use crate::ast::visitor; +use crate::ast::visitor::Visitor; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; +use crate::flake8_bugbear::plugins::mutable_argument_default::is_mutable_func; + +const IMMUTABLE_FUNCS: [&str; 11] = [ + "tuple", + "frozenset", + "operator.attrgetter", + "operator.itemgetter", + "operator.methodcaller", + "attrgetter", + "itemgetter", + "methodcaller", + "types.MappingProxyType", + "MappingProxyType", + "re.compile", +]; + +fn is_immutable_func(expr: &Expr) -> bool { + compose_call_path(expr).map_or_else(|| false, |func| IMMUTABLE_FUNCS.contains(&func.as_str())) +} + +struct ArgumentDefaultVisitor { + checks: Vec<(CheckKind, Range)>, +} + +impl<'a, 'b> Visitor<'b> for ArgumentDefaultVisitor +where + 'b: 'a, +{ + fn visit_expr(&mut self, expr: &'a Expr) { + match &expr.node { + ExprKind::Call { func, args, .. } => { + if !is_mutable_func(func) + && !is_immutable_func(func) + && !is_nan_or_infinity(func, args) + { + self.checks.push(( + CheckKind::FunctionCallArgumentDefault, + Range::from_located(expr), + )) + } + visitor::walk_expr(self, expr) + } + ExprKind::Lambda { .. } => {} + _ => visitor::walk_expr(self, expr), + } + } +} + +fn is_nan_or_infinity(expr: &Expr, args: &[Expr]) -> bool { + if let ExprKind::Name { id, .. } = &expr.node { + if id == "float" { + if let Some(arg) = args.first() { + if let ExprKind::Constant { + value: Constant::Str(value), + .. + } = &arg.node + { + let lowercased = value.to_lowercase(); + return lowercased == "nan" + || lowercased == "+nan" + || lowercased == "-nan" + || lowercased == "inf" + || lowercased == "+inf" + || lowercased == "-inf" + || lowercased == "infinity" + || lowercased == "+infinity" + || lowercased == "-infinity"; + } + } + } + } + false +} + +/// B008 +pub fn function_call_argument_default(checker: &mut Checker, arguments: &Arguments) { + let mut visitor = ArgumentDefaultVisitor { checks: vec![] }; + for expr in arguments + .defaults + .iter() + .chain(arguments.kw_defaults.iter()) + { + visitor.visit_expr(expr); + } + for (check, range) in visitor.checks { + checker.add_check(Check::new(check, checker.locate_check(range))); + } +} diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 3ec5ea8a83..b2e597c31a 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/src/flake8_bugbear/plugins/mod.rs @@ -1,6 +1,7 @@ pub use assert_false::assert_false; pub use assert_raises_exception::assert_raises_exception; pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions}; +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; @@ -11,6 +12,7 @@ pub use useless_expression::useless_expression; mod assert_false; mod assert_raises_exception; mod duplicate_exceptions; +mod function_call_argument_default; mod mutable_argument_default; mod redundant_tuple_in_exception_handler; mod unary_prefix_increment; diff --git a/src/flake8_bugbear/plugins/mutable_argument_default.rs b/src/flake8_bugbear/plugins/mutable_argument_default.rs index d02255200a..3a443d40f2 100644 --- a/src/flake8_bugbear/plugins/mutable_argument_default.rs +++ b/src/flake8_bugbear/plugins/mutable_argument_default.rs @@ -1,9 +1,34 @@ -use rustpython_ast::{Arguments, ExprKind}; +use rustpython_ast::{Arguments, Expr, ExprKind}; use crate::ast::types::{CheckLocator, Range}; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; +pub fn is_mutable_func(expr: &Expr) -> bool { + match &expr.node { + ExprKind::Name { id, .. } + if id == "dict" + || id == "list" + || id == "set" + || id == "Counter" + || id == "OrderedDict" + || id == "defaultdict" + || id == "deque" => + { + true + } + ExprKind::Attribute { value, attr, .. } + if (attr == "Counter" + || attr == "OrderedDict" + || attr == "defaultdict" + || attr == "deque") => + { + matches!(&value.node, ExprKind::Name { id, .. } if id == "collections") + } + _ => false, + } +} + /// B006 pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { for expr in arguments @@ -23,39 +48,14 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { checker.locate_check(Range::from_located(expr)), )); } - ExprKind::Call { func, .. } => match &func.node { - ExprKind::Name { id, .. } - if id == "dict" - || id == "list" - || id == "set" - || id == "Counter" - || id == "OrderedDict" - || id == "defaultdict" - || id == "deque" => - { + ExprKind::Call { func, .. } => { + if is_mutable_func(func) { checker.add_check(Check::new( CheckKind::MutableArgumentDefault, checker.locate_check(Range::from_located(expr)), )); } - ExprKind::Attribute { value, attr, .. } - if (attr == "Counter" - || attr == "OrderedDict" - || attr == "defaultdict" - || attr == "deque") => - { - match &value.node { - ExprKind::Name { id, .. } if id == "collections" => { - checker.add_check(Check::new( - CheckKind::MutableArgumentDefault, - checker.locate_check(Range::from_located(expr)), - )); - } - _ => {} - } - } - _ => {} - }, + } _ => {} } } diff --git a/src/linter.rs b/src/linter.rs index 57fa5c726f..bb49364e81 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -295,6 +295,7 @@ mod tests { #[test_case(CheckCode::B002, Path::new("B002.py"); "B002")] #[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")] #[test_case(CheckCode::B011, Path::new("B011.py"); "B011")] #[test_case(CheckCode::B013, Path::new("B013.py"); "B013")] #[test_case(CheckCode::B014, Path::new("B014.py"); "B014")] diff --git a/src/snapshots/ruff__linter__tests__B008_B006_B008.py.snap b/src/snapshots/ruff__linter__tests__B008_B006_B008.py.snap new file mode 100644 index 0000000000..86da86506d --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B008_B006_B008.py.snap @@ -0,0 +1,125 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: FunctionCallArgumentDefault + location: + row: 85 + column: 60 + end_location: + row: 85 + column: 68 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 89 + column: 63 + end_location: + row: 89 + column: 71 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 93 + column: 59 + end_location: + row: 93 + column: 67 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 109 + column: 38 + end_location: + row: 109 + column: 49 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 113 + column: 11 + end_location: + row: 113 + column: 28 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 113 + column: 31 + end_location: + row: 113 + column: 51 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 117 + column: 29 + end_location: + row: 117 + column: 44 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 155 + column: 33 + end_location: + row: 155 + column: 47 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 160 + column: 29 + end_location: + row: 160 + column: 37 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 164 + column: 44 + end_location: + row: 164 + column: 57 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 170 + column: 20 + end_location: + row: 170 + column: 28 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 170 + column: 30 + end_location: + row: 170 + column: 47 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 176 + column: 21 + end_location: + row: 176 + column: 62 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 181 + column: 18 + end_location: + row: 181 + column: 59 + fix: ~ +- kind: FunctionCallArgumentDefault + location: + row: 181 + column: 36 + end_location: + row: 181 + column: 53 + fix: ~ +