diff --git a/README.md b/README.md index ab5b5b970a..dcb165bb57 100644 --- a/README.md +++ b/README.md @@ -501,7 +501,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. | | +| 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,):`. | | diff --git a/resources/test/fixtures/B009_B010.py b/resources/test/fixtures/B009_B010.py index e002b0f0f1..8b0362681d 100644 --- a/resources/test/fixtures/B009_B010.py +++ b/resources/test/fixtures/B009_B010.py @@ -1,7 +1,7 @@ """ Should emit: -B009 - Line 17, 18, 19, 44 -B010 - Line 28, 29, 30 +B009 - Line 18, 19, 20, 21, 22 +B010 - Line 33, 34, 35, 36 """ # Valid getattr usage @@ -11,37 +11,26 @@ getattr(foo, "bar{foo}".format(foo="a"), None) getattr(foo, "bar{foo}".format(foo="a")) getattr(foo, bar, None) getattr(foo, "123abc") +getattr(foo, r"123\abc") getattr(foo, "except") # Invalid usage getattr(foo, "bar") getattr(foo, "_123abc") getattr(foo, "abc123") +getattr(foo, r"abc123") +_ = lambda x: getattr(x, "bar") # Valid setattr usage setattr(foo, bar, None) setattr(foo, "bar{foo}".format(foo="a"), None) setattr(foo, "123abc", None) +setattr(foo, r"123\abc", None) setattr(foo, "except", None) +_ = lambda x: setattr(x, "bar", 1) # 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 +setattr(foo, r"abc123", None) diff --git a/src/check_ast.rs b/src/check_ast.rs index e86d6c765d..c9d5f05dc7 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1075,11 +1075,11 @@ where flake8_bugbear::plugins::getattr_with_constant(self, expr, func, args); } if self.settings.enabled.contains(&CheckCode::B010) { - if !self + if self .scopes .iter() .rev() - .any(|scope| matches!(scope.kind, ScopeKind::Lambda)) + .all(|scope| !matches!(scope.kind, ScopeKind::Lambda)) { flake8_bugbear::plugins::setattr_with_constant(self, expr, func, args); } @@ -1473,7 +1473,6 @@ where } self.push_scope(Scope::new(ScopeKind::Lambda)) } - ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { if self.settings.enabled.contains(&CheckCode::C416) { if let Some(check) = flake8_comprehensions::checks::unnecessary_comprehension( @@ -1489,7 +1488,6 @@ where } self.push_scope(Scope::new(ScopeKind::Generator)) } - ExprKind::GeneratorExp { .. } | ExprKind::DictComp { .. } => { self.push_scope(Scope::new(ScopeKind::Generator)) } diff --git a/src/checks.rs b/src/checks.rs index 7b9ebf6741..5be7e259ce 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1743,6 +1743,7 @@ impl CheckKind { | CheckKind::DeprecatedUnittestAlias(_, _) | CheckKind::DoNotAssertFalse | CheckKind::DuplicateHandlerException(_) + | CheckKind::GetAttrWithConstant | CheckKind::IsLiteral | CheckKind::NewLineAfterLastParagraph | CheckKind::NewLineAfterSectionName(_) diff --git a/src/flake8_bugbear/plugins/getattr_with_constant.rs b/src/flake8_bugbear/plugins/getattr_with_constant.rs index 72cbdc9bb3..0ed1c0c3c4 100644 --- a/src/flake8_bugbear/plugins/getattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/getattr_with_constant.rs @@ -1,26 +1,50 @@ -use rustpython_ast::{Constant, Expr, ExprKind}; +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind}; use crate::ast::types::Range; +use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; +use crate::code_gen::SourceGenerator; use crate::flake8_bugbear::constants::IDENTIFIER_REGEX; use crate::python::keyword::KWLIST; -/// B009 +fn attribute(value: &Expr, attr: &str) -> Expr { + Expr::new( + Default::default(), + Default::default(), + ExprKind::Attribute { + value: Box::new(value.clone()), + attr: attr.to_string(), + ctx: ExprContext::Load, + }, + ) +} + 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 [obj, 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), - )); + let mut check = + Check::new(CheckKind::GetAttrWithConstant, Range::from_located(expr)); + if checker.patch() { + let mut generator = SourceGenerator::new(); + if let Ok(()) = generator.unparse_expr(&attribute(obj, value), 0) { + if let Ok(content) = generator.generate() { + check.amend(Fix::replacement( + content, + expr.location, + expr.end_location.unwrap(), + )); + } + } + } + checker.add_check(check); } } } diff --git a/src/flake8_bugbear/plugins/setattr_with_constant.rs b/src/flake8_bugbear/plugins/setattr_with_constant.rs index b86ee581dd..479014ece6 100644 --- a/src/flake8_bugbear/plugins/setattr_with_constant.rs +++ b/src/flake8_bugbear/plugins/setattr_with_constant.rs @@ -18,7 +18,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar { if IDENTIFIER_REGEX.is_match(value) && !KWLIST.contains(&value.as_str()) { checker.add_check(Check::new( - CheckKind::GetAttrWithConstant, + CheckKind::SetAttrWithConstant, Range::from_located(expr), )); } diff --git a/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap b/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap index ac91b7ba5e..996013df3c 100644 --- a/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap +++ b/src/snapshots/ruff__linter__tests__B009_B009_B010.py.snap @@ -4,34 +4,87 @@ expression: checks --- - kind: GetAttrWithConstant location: - row: 17 + row: 18 column: 0 end_location: - row: 17 + row: 18 column: 19 - fix: ~ + fix: + patch: + content: foo.bar + location: + row: 18 + column: 0 + end_location: + row: 18 + column: 19 + applied: false - kind: GetAttrWithConstant location: - row: 18 + row: 19 column: 0 end_location: - row: 18 + row: 19 column: 23 - fix: ~ + fix: + patch: + content: foo._123abc + location: + row: 19 + column: 0 + end_location: + row: 19 + column: 23 + applied: false - kind: GetAttrWithConstant location: - row: 19 + row: 20 column: 0 end_location: - row: 19 + row: 20 column: 22 - fix: ~ + fix: + patch: + content: foo.abc123 + location: + row: 20 + column: 0 + end_location: + row: 20 + column: 22 + applied: false - kind: GetAttrWithConstant location: - row: 45 + row: 21 + column: 0 + end_location: + row: 21 + column: 23 + fix: + patch: + content: foo.abc123 + location: + row: 21 + column: 0 + end_location: + row: 21 + column: 23 + applied: false +- kind: GetAttrWithConstant + location: + row: 22 column: 14 end_location: - row: 45 - column: 37 - fix: ~ + row: 22 + column: 31 + fix: + patch: + content: x.bar + location: + row: 22 + column: 14 + end_location: + row: 22 + column: 31 + applied: false diff --git a/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap b/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap index 1785bf57e2..60c615f917 100644 --- a/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap +++ b/src/snapshots/ruff__linter__tests__B010_B009_B010.py.snap @@ -2,28 +2,5 @@ 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: ~ +[]