Skip to content

Commit

Permalink
Implement B008 (#594)
Browse files Browse the repository at this point in the history
  • Loading branch information
harupy authored Nov 5, 2022
1 parent c544d84 commit f7780eb
Show file tree
Hide file tree
Showing 10 changed files with 274 additions and 32 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` | 🛠 |
Expand Down Expand Up @@ -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)
Expand All @@ -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).
Expand Down
1 change: 1 addition & 0 deletions src/ast/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub enum ScopeKind {
Function(FunctionScope),
Generator,
Module,
Arg,
}

#[derive(Clone, Debug)]
Expand Down
3 changes: 3 additions & 0 deletions src/check_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub enum CheckCode {
B002,
B006,
B007,
B008,
B011,
B013,
B014,
Expand Down Expand Up @@ -297,6 +298,7 @@ pub enum CheckKind {
UnaryPrefixIncrement,
MutableArgumentDefault,
UnusedLoopControlVariable(String),
FunctionCallArgumentDefault,
DoNotAssertFalse,
RedundantTupleInExceptionHandler(String),
DuplicateHandlerException(Vec<String>),
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
7 changes: 6 additions & 1 deletion src/checks_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub enum CheckCodePrefix {
B002,
B006,
B007,
B008,
B01,
B011,
B013,
Expand Down Expand Up @@ -257,6 +258,7 @@ impl CheckCodePrefix {
CheckCode::B002,
CheckCode::B006,
CheckCode::B007,
CheckCode::B008,
CheckCode::B011,
CheckCode::B013,
CheckCode::B014,
Expand All @@ -269,6 +271,7 @@ impl CheckCodePrefix {
CheckCode::B002,
CheckCode::B006,
CheckCode::B007,
CheckCode::B008,
CheckCode::B011,
CheckCode::B013,
CheckCode::B014,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
96 changes: 96 additions & 0 deletions src/flake8_bugbear/plugins/function_call_argument_default.rs
Original file line number Diff line number Diff line change
@@ -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)));
}
}
2 changes: 2 additions & 0 deletions src/flake8_bugbear/plugins/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
58 changes: 29 additions & 29 deletions src/flake8_bugbear/plugins/mutable_argument_default.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)),
));
}
_ => {}
}
}
_ => {}
},
}
_ => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
Loading

0 comments on commit f7780eb

Please sign in to comment.