From 1888f6d41badd6d72e79d586239ba0cdd88cbbcf Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Fri, 11 Nov 2022 03:52:20 +0900 Subject: [PATCH] Implement B009 (#669) --- README.md | 5 +- resources/test/fixtures/B009_B010.py | 47 +++++++++++++++++++ src/check_ast.rs | 3 ++ src/checks.rs | 9 ++++ src/checks_gen.rs | 6 +++ .../plugins/getattr_with_constant.rs | 33 +++++++++++++ src/flake8_bugbear/plugins/mod.rs | 2 + src/linter.rs | 1 + src/python/keyword.rs | 7 +++ src/python/mod.rs | 1 + ...uff__linter__tests__B009_B009_B010.py.snap | 37 +++++++++++++++ 11 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 resources/test/fixtures/B009_B010.py create mode 100644 src/flake8_bugbear/plugins/getattr_with_constant.rs create mode 100644 src/python/keyword.rs create mode 100644 src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap diff --git a/README.md b/README.md index d023953b29..0f98e6223f 100644 --- a/README.md +++ b/README.md @@ -492,6 +492,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/ | 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. | | +| B009 | GetAttrWithConstant | Do not call `getattr` 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` | 🛠 | @@ -655,7 +656,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/) (18/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (19/32) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -678,7 +679,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/) (18/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (19/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/) (14/34). diff --git a/resources/test/fixtures/B009_B010.py b/resources/test/fixtures/B009_B010.py new file mode 100644 index 0000000000..e002b0f0f1 --- /dev/null +++ b/resources/test/fixtures/B009_B010.py @@ -0,0 +1,47 @@ +""" +Should emit: +B009 - Line 17, 18, 19, 44 +B010 - Line 28, 29, 30 +""" + +# Valid getattr usage +getattr(foo, bar) +getattr(foo, "bar", None) +getattr(foo, "bar{foo}".format(foo="a"), None) +getattr(foo, "bar{foo}".format(foo="a")) +getattr(foo, bar, None) +getattr(foo, "123abc") +getattr(foo, "except") + +# Invalid usage +getattr(foo, "bar") +getattr(foo, "_123abc") +getattr(foo, "abc123") + +# Valid setattr usage +setattr(foo, bar, None) +setattr(foo, "bar{foo}".format(foo="a"), None) +setattr(foo, "123abc", None) +setattr(foo, "except", None) + +# Invalid usage +setattr(foo, "bar", None) +setattr(foo, "_123abc", None) +setattr(foo, "abc123", None) + +# Allow use of setattr within lambda expression +# since assignment is not valid in this context. +c = lambda x: setattr(x, "some_attr", 1) + + +class FakeCookieStore: + def __init__(self, has_setter): + self.cookie_filter = None + if has_setter: + self.setCookieFilter = lambda func: setattr(self, "cookie_filter", func) + + +# getattr is still flagged within lambda though +c = lambda x: getattr(x, "some_attr") +# should be replaced with +c = lambda x: x.some_attr diff --git a/src/check_ast.rs b/src/check_ast.rs index 853d988679..5860664922 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1064,6 +1064,9 @@ where if self.settings.enabled.contains(&CheckCode::B005) { flake8_bugbear::plugins::strip_with_multi_characters(self, expr, func, args); } + if self.settings.enabled.contains(&CheckCode::B009) { + flake8_bugbear::plugins::getattr_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, diff --git a/src/checks.rs b/src/checks.rs index 078bd925fb..4a88b68da8 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -84,6 +84,7 @@ pub enum CheckCode { B006, B007, B008, + B009, B011, B013, B014, @@ -349,6 +350,7 @@ pub enum CheckKind { MutableArgumentDefault, UnusedLoopControlVariable(String), FunctionCallArgumentDefault, + GetAttrWithConstant, DoNotAssertFalse, RedundantTupleInExceptionHandler(String), DuplicateHandlerException(Vec), @@ -561,6 +563,7 @@ impl CheckCode { CheckCode::B006 => CheckKind::MutableArgumentDefault, CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()), CheckCode::B008 => CheckKind::FunctionCallArgumentDefault, + CheckCode::B009 => CheckKind::GetAttrWithConstant, CheckCode::B011 => CheckKind::DoNotAssertFalse, CheckCode::B013 => { CheckKind::RedundantTupleInExceptionHandler("ValueError".to_string()) @@ -779,6 +782,7 @@ impl CheckCode { CheckCode::B006 => CheckCategory::Flake8Bugbear, CheckCode::B007 => CheckCategory::Flake8Bugbear, CheckCode::B008 => CheckCategory::Flake8Bugbear, + CheckCode::B009 => CheckCategory::Flake8Bugbear, CheckCode::B011 => CheckCategory::Flake8Bugbear, CheckCode::B013 => CheckCategory::Flake8Bugbear, CheckCode::B014 => CheckCategory::Flake8Bugbear, @@ -961,6 +965,7 @@ impl CheckKind { CheckKind::MutableArgumentDefault => &CheckCode::B006, CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007, CheckKind::FunctionCallArgumentDefault => &CheckCode::B008, + CheckKind::GetAttrWithConstant => &CheckCode::B009, CheckKind::DoNotAssertFalse => &CheckCode::B011, CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013, CheckKind::DuplicateHandlerException(_) => &CheckCode::B014, @@ -1264,6 +1269,10 @@ impl CheckKind { CheckKind::FunctionCallArgumentDefault => { "Do not perform function calls in argument defaults.".to_string() } + CheckKind::GetAttrWithConstant => "Do not call `getattr` 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 1c929ee18e..81e92849e5 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -43,6 +43,7 @@ pub enum CheckCodePrefix { B006, B007, B008, + B009, B01, B011, B013, @@ -336,6 +337,7 @@ impl CheckCodePrefix { CheckCode::B006, CheckCode::B007, CheckCode::B008, + CheckCode::B009, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -354,6 +356,7 @@ impl CheckCodePrefix { CheckCode::B006, CheckCode::B007, CheckCode::B008, + CheckCode::B009, CheckCode::B011, CheckCode::B013, CheckCode::B014, @@ -372,6 +375,7 @@ impl CheckCodePrefix { CheckCode::B006, CheckCode::B007, CheckCode::B008, + CheckCode::B009, ], CheckCodePrefix::B002 => vec![CheckCode::B002], CheckCodePrefix::B003 => vec![CheckCode::B003], @@ -380,6 +384,7 @@ impl CheckCodePrefix { CheckCodePrefix::B006 => vec![CheckCode::B006], CheckCodePrefix::B007 => vec![CheckCode::B007], CheckCodePrefix::B008 => vec![CheckCode::B008], + CheckCodePrefix::B009 => vec![CheckCode::B009], CheckCodePrefix::B01 => vec![ CheckCode::B011, CheckCode::B013, @@ -1051,6 +1056,7 @@ impl CheckCodePrefix { CheckCodePrefix::B006 => PrefixSpecificity::Explicit, CheckCodePrefix::B007 => PrefixSpecificity::Explicit, CheckCodePrefix::B008 => PrefixSpecificity::Explicit, + CheckCodePrefix::B009 => PrefixSpecificity::Explicit, CheckCodePrefix::B01 => PrefixSpecificity::Tens, CheckCodePrefix::B011 => PrefixSpecificity::Explicit, CheckCodePrefix::B013 => PrefixSpecificity::Explicit, diff --git a/src/flake8_bugbear/plugins/getattr_with_constant.rs b/src/flake8_bugbear/plugins/getattr_with_constant.rs new file mode 100644 index 0000000000..01a0310b4f --- /dev/null +++ b/src/flake8_bugbear/plugins/getattr_with_constant.rs @@ -0,0 +1,33 @@ +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::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 { + if id == "getattr" { + 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/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 02cf75a14e..4f386f3ae8 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/src/flake8_bugbear/plugins/mod.rs @@ -4,6 +4,7 @@ pub use assignment_to_os_environ::assignment_to_os_environ; pub use cannot_raise_literal::cannot_raise_literal; pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions}; 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 star_arg_unpacking_after_keyword_arg::star_arg_unpacking_after_keyword_arg; @@ -20,6 +21,7 @@ mod assignment_to_os_environ; mod cannot_raise_literal; mod duplicate_exceptions; mod function_call_argument_default; +mod getattr_with_constant; mod mutable_argument_default; mod redundant_tuple_in_exception_handler; mod star_arg_unpacking_after_keyword_arg; diff --git a/src/linter.rs b/src/linter.rs index 81dfdce620..756a7d0aa9 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -299,6 +299,7 @@ mod tests { #[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::B009, Path::new("B009_B010.py"); "B009")] #[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/python/keyword.rs b/src/python/keyword.rs new file mode 100644 index 0000000000..f60ceda8f4 --- /dev/null +++ b/src/python/keyword.rs @@ -0,0 +1,7 @@ +// See: https://github.com/python/cpython/blob/9d692841691590c25e6cf5b2250a594d3bf54825/Lib/keyword.py#L18 +pub const KWLIST: [&str; 35] = [ + "False", "None", "True", "and", "as", "assert", "async", "await", "break", "class", "continue", + "def", "del", "elif", "else", "except", "finally", "for", "from", "global", "if", "import", + "in", "is", "lambda", "nonlocal", "not", "or", "pass", "raise", "return", "try", "while", + "with", "yield", +]; diff --git a/src/python/mod.rs b/src/python/mod.rs index b68ea3f32d..611cd0691a 100644 --- a/src/python/mod.rs +++ b/src/python/mod.rs @@ -1,3 +1,4 @@ pub mod builtins; pub mod future; +pub mod keyword; pub mod typing; diff --git a/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap b/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap new file mode 100644 index 0000000000..ac91b7ba5e --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap @@ -0,0 +1,37 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: GetAttrWithConstant + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 19 + fix: ~ +- kind: GetAttrWithConstant + location: + row: 18 + column: 0 + end_location: + row: 18 + column: 23 + fix: ~ +- kind: GetAttrWithConstant + location: + row: 19 + column: 0 + end_location: + row: 19 + column: 22 + fix: ~ +- kind: GetAttrWithConstant + location: + row: 45 + column: 14 + end_location: + row: 45 + column: 37 + fix: ~ +