mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-03 15:15:33 +00:00
Implement autofix for B009 (#684)
This commit is contained in:
parent
bd3b40688f
commit
e727c24f79
8 changed files with 111 additions and 69 deletions
|
@ -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,):`. | |
|
||||
|
|
27
resources/test/fixtures/B009_B010.py
vendored
27
resources/test/fixtures/B009_B010.py
vendored
|
@ -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)
|
||||
|
|
|
@ -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))
|
||||
}
|
||||
|
|
|
@ -1743,6 +1743,7 @@ impl CheckKind {
|
|||
| CheckKind::DeprecatedUnittestAlias(_, _)
|
||||
| CheckKind::DoNotAssertFalse
|
||||
| CheckKind::DuplicateHandlerException(_)
|
||||
| CheckKind::GetAttrWithConstant
|
||||
| CheckKind::IsLiteral
|
||||
| CheckKind::NewLineAfterLastParagraph
|
||||
| CheckKind::NewLineAfterSectionName(_)
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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),
|
||||
));
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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: ~
|
||||
[]
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue