From bd3b40688fdaa9ca69219552616e415648c5d404 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Sat, 12 Nov 2022 00:26:37 +0900 Subject: [PATCH] Implement B010 (#683) --- README.md | 5 ++-- src/ast/types.rs | 1 + src/check_ast.rs | 14 ++++++++- src/checks.rs | 9 ++++++ src/checks_gen.rs | 6 ++++ src/flake8_bugbear/constants.rs | 5 ++++ src/flake8_bugbear/mod.rs | 1 + .../plugins/getattr_with_constant.rs | 6 +--- src/flake8_bugbear/plugins/mod.rs | 2 ++ .../plugins/setattr_with_constant.rs | 29 +++++++++++++++++++ src/linter.rs | 1 + ...uff__linter__tests__B010_B009_B010.py.snap | 29 +++++++++++++++++++ 12 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 src/flake8_bugbear/constants.rs create mode 100644 src/flake8_bugbear/plugins/setattr_with_constant.rs create mode 100644 src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap diff --git a/README.md b/README.md index 61b102016a..ab5b5b970a 100644 --- a/README.md +++ b/README.md @@ -502,6 +502,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/ | B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 | | B008 | FunctionCallArgumentDefault | Do not perform function calls in argument defaults. | | | B009 | GetAttrWithConstant | Do not call `getattr` with a constant attribute value, it is not any safer than normal property access. | | +| B010 | SetAttrWithConstant | Do not call setattr with a constant attribute value, it is not any safer than normal property access. | | | 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` | 🛠 | @@ -665,7 +666,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/) (19/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (20/32) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -688,7 +689,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/) (19/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (20/32) Ruff can also replace [`isort`](https://pypi.org/project/isort/), [`yesqa`](https://github.com/asottile/yesqa), and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34). diff --git a/src/ast/types.rs b/src/ast/types.rs index bb4a9c558c..ee694b00f0 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -46,6 +46,7 @@ pub enum ScopeKind<'a> { Generator, Module, Arg, + Lambda, } #[derive(Clone, Debug)] diff --git a/src/check_ast.rs b/src/check_ast.rs index 6f4bdf3027..e86d6c765d 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1074,6 +1074,16 @@ where if self.settings.enabled.contains(&CheckCode::B009) { flake8_bugbear::plugins::getattr_with_constant(self, expr, func, args); } + if self.settings.enabled.contains(&CheckCode::B010) { + if !self + .scopes + .iter() + .rev() + .any(|scope| matches!(scope.kind, ScopeKind::Lambda)) + { + flake8_bugbear::plugins::setattr_with_constant(self, expr, func, args); + } + } if self.settings.enabled.contains(&CheckCode::B026) { flake8_bugbear::plugins::star_arg_unpacking_after_keyword_arg( self, args, keywords, @@ -1461,6 +1471,7 @@ where for expr in &args.defaults { self.visit_expr(expr); } + self.push_scope(Scope::new(ScopeKind::Lambda)) } ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { @@ -1649,7 +1660,8 @@ where // Post-visit. match &expr.node { - ExprKind::GeneratorExp { .. } + ExprKind::Lambda { .. } + | ExprKind::GeneratorExp { .. } | ExprKind::ListComp { .. } | ExprKind::DictComp { .. } | ExprKind::SetComp { .. } => { diff --git a/src/checks.rs b/src/checks.rs index 0970cdcc7a..7b9ebf6741 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -85,6 +85,7 @@ pub enum CheckCode { B007, B008, B009, + B010, B011, B013, B014, @@ -357,6 +358,7 @@ pub enum CheckKind { UnusedLoopControlVariable(String), FunctionCallArgumentDefault, GetAttrWithConstant, + SetAttrWithConstant, DoNotAssertFalse, RedundantTupleInExceptionHandler(String), DuplicateHandlerException(Vec), @@ -573,6 +575,7 @@ impl CheckCode { CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()), CheckCode::B008 => CheckKind::FunctionCallArgumentDefault, CheckCode::B009 => CheckKind::GetAttrWithConstant, + CheckCode::B010 => CheckKind::SetAttrWithConstant, CheckCode::B011 => CheckKind::DoNotAssertFalse, CheckCode::B013 => { CheckKind::RedundantTupleInExceptionHandler("ValueError".to_string()) @@ -794,6 +797,7 @@ impl CheckCode { CheckCode::B007 => CheckCategory::Flake8Bugbear, CheckCode::B008 => CheckCategory::Flake8Bugbear, CheckCode::B009 => CheckCategory::Flake8Bugbear, + CheckCode::B010 => CheckCategory::Flake8Bugbear, CheckCode::B011 => CheckCategory::Flake8Bugbear, CheckCode::B013 => CheckCategory::Flake8Bugbear, CheckCode::B014 => CheckCategory::Flake8Bugbear, @@ -978,6 +982,7 @@ impl CheckKind { CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007, CheckKind::FunctionCallArgumentDefault => &CheckCode::B008, CheckKind::GetAttrWithConstant => &CheckCode::B009, + CheckKind::SetAttrWithConstant => &CheckCode::B010, CheckKind::DoNotAssertFalse => &CheckCode::B011, CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013, CheckKind::DuplicateHandlerException(_) => &CheckCode::B014, @@ -1287,6 +1292,10 @@ impl CheckKind { value, it is not any safer than normal property \ access." .to_string(), + CheckKind::SetAttrWithConstant => "Do not call setattr with a constant attribute \ + value, it is not any safer than normal property \ + access." + .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 a37154b9dd..e720cd9f37 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -45,6 +45,7 @@ pub enum CheckCodePrefix { B008, B009, B01, + B010, B011, B013, B014, @@ -342,6 +343,7 @@ impl CheckCodePrefix { CheckCode::B007, CheckCode::B008, CheckCode::B009, + CheckCode::B010, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -361,6 +363,7 @@ impl CheckCodePrefix { CheckCode::B007, CheckCode::B008, CheckCode::B009, + CheckCode::B010, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -390,6 +393,7 @@ impl CheckCodePrefix { CheckCodePrefix::B008 => vec![CheckCode::B008], CheckCodePrefix::B009 => vec![CheckCode::B009], CheckCodePrefix::B01 => vec![ + CheckCode::B010, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -398,6 +402,7 @@ impl CheckCodePrefix { CheckCode::B017, CheckCode::B018, ], + CheckCodePrefix::B010 => vec![CheckCode::B010], CheckCodePrefix::B011 => vec![CheckCode::B011], CheckCodePrefix::B013 => vec![CheckCode::B013], CheckCodePrefix::B014 => vec![CheckCode::B014], @@ -1066,6 +1071,7 @@ impl CheckCodePrefix { CheckCodePrefix::B008 => PrefixSpecificity::Explicit, CheckCodePrefix::B009 => PrefixSpecificity::Explicit, CheckCodePrefix::B01 => PrefixSpecificity::Tens, + CheckCodePrefix::B010 => PrefixSpecificity::Explicit, CheckCodePrefix::B011 => PrefixSpecificity::Explicit, CheckCodePrefix::B013 => PrefixSpecificity::Explicit, CheckCodePrefix::B014 => PrefixSpecificity::Explicit, diff --git a/src/flake8_bugbear/constants.rs b/src/flake8_bugbear/constants.rs new file mode 100644 index 0000000000..34d366aab6 --- /dev/null +++ b/src/flake8_bugbear/constants.rs @@ -0,0 +1,5 @@ +use once_cell::sync::Lazy; +use regex::Regex; + +pub static IDENTIFIER_REGEX: Lazy = + Lazy::new(|| Regex::new(r"^[A-Za-z_][A-Za-z0-9_]*$").unwrap()); diff --git a/src/flake8_bugbear/mod.rs b/src/flake8_bugbear/mod.rs index 25a06164e5..8556dc6fed 100644 --- a/src/flake8_bugbear/mod.rs +++ b/src/flake8_bugbear/mod.rs @@ -1 +1,2 @@ +mod constants; pub mod plugins; diff --git a/src/flake8_bugbear/plugins/getattr_with_constant.rs b/src/flake8_bugbear/plugins/getattr_with_constant.rs index 01a0310b4f..72cbdc9bb3 100644 --- a/src/flake8_bugbear/plugins/getattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/getattr_with_constant.rs @@ -1,15 +1,11 @@ -use once_cell::sync::Lazy; -use regex::Regex; use rustpython_ast::{Constant, Expr, ExprKind}; use crate::ast::types::Range; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; +use crate::flake8_bugbear::constants::IDENTIFIER_REGEX; use crate::python::keyword::KWLIST; -static IDENTIFIER_REGEX: Lazy = - Lazy::new(|| Regex::new(r"^[A-Za-z_][A-Za-z0-9_]*$").unwrap()); - /// B009 pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let ExprKind::Name { id, .. } = &func.node { diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 4f386f3ae8..2b2781690b 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 getattr_with_constant::getattr_with_constant; pub use mutable_argument_default::mutable_argument_default; pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler; +pub use setattr_with_constant::setattr_with_constant; pub use star_arg_unpacking_after_keyword_arg::star_arg_unpacking_after_keyword_arg; pub use strip_with_multi_characters::strip_with_multi_characters; pub use unary_prefix_increment::unary_prefix_increment; @@ -24,6 +25,7 @@ mod function_call_argument_default; mod getattr_with_constant; mod mutable_argument_default; mod redundant_tuple_in_exception_handler; +mod setattr_with_constant; mod star_arg_unpacking_after_keyword_arg; mod strip_with_multi_characters; mod unary_prefix_increment; diff --git a/src/flake8_bugbear/plugins/setattr_with_constant.rs b/src/flake8_bugbear/plugins/setattr_with_constant.rs new file mode 100644 index 0000000000..b86ee581dd --- /dev/null +++ b/src/flake8_bugbear/plugins/setattr_with_constant.rs @@ -0,0 +1,29 @@ +use rustpython_ast::{Constant, Expr, ExprKind}; + +use crate::ast::types::Range; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; +use crate::flake8_bugbear::constants::IDENTIFIER_REGEX; +use crate::python::keyword::KWLIST; + +/// B010 +pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { + if let ExprKind::Name { id, .. } = &func.node { + if id == "setattr" { + if let [_, arg, _] = args { + if let ExprKind::Constant { + value: Constant::Str(value), + .. + } = &arg.node + { + if IDENTIFIER_REGEX.is_match(value) && !KWLIST.contains(&value.as_str()) { + checker.add_check(Check::new( + CheckKind::GetAttrWithConstant, + Range::from_located(expr), + )); + } + } + } + } + } +} diff --git a/src/linter.rs b/src/linter.rs index 13f2bd1e30..b728965a80 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -305,6 +305,7 @@ mod tests { #[test_case(CheckCode::B007, Path::new("B007.py"); "B007")] #[test_case(CheckCode::B008, Path::new("B006_B008.py"); "B008")] #[test_case(CheckCode::B009, Path::new("B009_B010.py"); "B009")] + #[test_case(CheckCode::B010, Path::new("B009_B010.py"); "B010")] #[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__B010_B009_B010.py.snap b/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap new file mode 100644 index 0000000000..1785bf57e2 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap @@ -0,0 +1,29 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: GetAttrWithConstant + location: + row: 28 + column: 0 + end_location: + row: 28 + column: 25 + fix: ~ +- kind: GetAttrWithConstant + location: + row: 29 + column: 0 + end_location: + row: 29 + column: 29 + fix: ~ +- kind: GetAttrWithConstant + location: + row: 30 + column: 0 + end_location: + row: 30 + column: 28 + fix: ~ +