Implement B009 (#669)

This commit is contained in:
Harutaka Kawamura 2022-11-11 03:52:20 +09:00 committed by GitHub
parent 9d8cd2d2fe
commit 1888f6d41b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 149 additions and 2 deletions

View File

@ -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. | | | B006 | MutableArgumentDefault | Do not use mutable data structures for argument defaults. | |
| B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 | | B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 |
| B008 | FunctionCallArgumentDefault | Do not perform function calls in argument defaults. | | | 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()` | 🛠 | | 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,):`. | | | B013 | RedundantTupleInExceptionHandler | A length-one tuple literal is redundant. Write `except ValueError:` instead of `except (ValueError,):`. | |
| B014 | DuplicateHandlerException | Exception handler with duplicate exception: `ValueError` | 🛠 | | B014 | DuplicateHandlerException | Exception handler with duplicate exception: `ValueError` | 🛠 |
@ -655,7 +656,7 @@ including:
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) - [`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) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34)
- [`autoflake`](https://pypi.org/project/autoflake/) (1/7) - [`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-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) - [`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), 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). and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34).

47
resources/test/fixtures/B009_B010.py vendored Normal file
View File

@ -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

View File

@ -1064,6 +1064,9 @@ where
if self.settings.enabled.contains(&CheckCode::B005) { if self.settings.enabled.contains(&CheckCode::B005) {
flake8_bugbear::plugins::strip_with_multi_characters(self, expr, func, args); 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) { if self.settings.enabled.contains(&CheckCode::B026) {
flake8_bugbear::plugins::star_arg_unpacking_after_keyword_arg( flake8_bugbear::plugins::star_arg_unpacking_after_keyword_arg(
self, args, keywords, self, args, keywords,

View File

@ -84,6 +84,7 @@ pub enum CheckCode {
B006, B006,
B007, B007,
B008, B008,
B009,
B011, B011,
B013, B013,
B014, B014,
@ -349,6 +350,7 @@ pub enum CheckKind {
MutableArgumentDefault, MutableArgumentDefault,
UnusedLoopControlVariable(String), UnusedLoopControlVariable(String),
FunctionCallArgumentDefault, FunctionCallArgumentDefault,
GetAttrWithConstant,
DoNotAssertFalse, DoNotAssertFalse,
RedundantTupleInExceptionHandler(String), RedundantTupleInExceptionHandler(String),
DuplicateHandlerException(Vec<String>), DuplicateHandlerException(Vec<String>),
@ -561,6 +563,7 @@ impl CheckCode {
CheckCode::B006 => CheckKind::MutableArgumentDefault, CheckCode::B006 => CheckKind::MutableArgumentDefault,
CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()), CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()),
CheckCode::B008 => CheckKind::FunctionCallArgumentDefault, CheckCode::B008 => CheckKind::FunctionCallArgumentDefault,
CheckCode::B009 => CheckKind::GetAttrWithConstant,
CheckCode::B011 => CheckKind::DoNotAssertFalse, CheckCode::B011 => CheckKind::DoNotAssertFalse,
CheckCode::B013 => { CheckCode::B013 => {
CheckKind::RedundantTupleInExceptionHandler("ValueError".to_string()) CheckKind::RedundantTupleInExceptionHandler("ValueError".to_string())
@ -779,6 +782,7 @@ impl CheckCode {
CheckCode::B006 => CheckCategory::Flake8Bugbear, CheckCode::B006 => CheckCategory::Flake8Bugbear,
CheckCode::B007 => CheckCategory::Flake8Bugbear, CheckCode::B007 => CheckCategory::Flake8Bugbear,
CheckCode::B008 => CheckCategory::Flake8Bugbear, CheckCode::B008 => CheckCategory::Flake8Bugbear,
CheckCode::B009 => CheckCategory::Flake8Bugbear,
CheckCode::B011 => CheckCategory::Flake8Bugbear, CheckCode::B011 => CheckCategory::Flake8Bugbear,
CheckCode::B013 => CheckCategory::Flake8Bugbear, CheckCode::B013 => CheckCategory::Flake8Bugbear,
CheckCode::B014 => CheckCategory::Flake8Bugbear, CheckCode::B014 => CheckCategory::Flake8Bugbear,
@ -961,6 +965,7 @@ impl CheckKind {
CheckKind::MutableArgumentDefault => &CheckCode::B006, CheckKind::MutableArgumentDefault => &CheckCode::B006,
CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007, CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007,
CheckKind::FunctionCallArgumentDefault => &CheckCode::B008, CheckKind::FunctionCallArgumentDefault => &CheckCode::B008,
CheckKind::GetAttrWithConstant => &CheckCode::B009,
CheckKind::DoNotAssertFalse => &CheckCode::B011, CheckKind::DoNotAssertFalse => &CheckCode::B011,
CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013, CheckKind::RedundantTupleInExceptionHandler(_) => &CheckCode::B013,
CheckKind::DuplicateHandlerException(_) => &CheckCode::B014, CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
@ -1264,6 +1269,10 @@ impl CheckKind {
CheckKind::FunctionCallArgumentDefault => { CheckKind::FunctionCallArgumentDefault => {
"Do not perform function calls in argument defaults.".to_string() "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 \ CheckKind::DoNotAssertFalse => "Do not `assert False` (`python -O` removes these \
calls), raise `AssertionError()`" calls), raise `AssertionError()`"
.to_string(), .to_string(),

View File

@ -43,6 +43,7 @@ pub enum CheckCodePrefix {
B006, B006,
B007, B007,
B008, B008,
B009,
B01, B01,
B011, B011,
B013, B013,
@ -336,6 +337,7 @@ impl CheckCodePrefix {
CheckCode::B006, CheckCode::B006,
CheckCode::B007, CheckCode::B007,
CheckCode::B008, CheckCode::B008,
CheckCode::B009,
CheckCode::B011, CheckCode::B011,
CheckCode::B013, CheckCode::B013,
CheckCode::B014, CheckCode::B014,
@ -354,6 +356,7 @@ impl CheckCodePrefix {
CheckCode::B006, CheckCode::B006,
CheckCode::B007, CheckCode::B007,
CheckCode::B008, CheckCode::B008,
CheckCode::B009,
CheckCode::B011, CheckCode::B011,
CheckCode::B013, CheckCode::B013,
CheckCode::B014, CheckCode::B014,
@ -372,6 +375,7 @@ impl CheckCodePrefix {
CheckCode::B006, CheckCode::B006,
CheckCode::B007, CheckCode::B007,
CheckCode::B008, CheckCode::B008,
CheckCode::B009,
], ],
CheckCodePrefix::B002 => vec![CheckCode::B002], CheckCodePrefix::B002 => vec![CheckCode::B002],
CheckCodePrefix::B003 => vec![CheckCode::B003], CheckCodePrefix::B003 => vec![CheckCode::B003],
@ -380,6 +384,7 @@ impl CheckCodePrefix {
CheckCodePrefix::B006 => vec![CheckCode::B006], CheckCodePrefix::B006 => vec![CheckCode::B006],
CheckCodePrefix::B007 => vec![CheckCode::B007], CheckCodePrefix::B007 => vec![CheckCode::B007],
CheckCodePrefix::B008 => vec![CheckCode::B008], CheckCodePrefix::B008 => vec![CheckCode::B008],
CheckCodePrefix::B009 => vec![CheckCode::B009],
CheckCodePrefix::B01 => vec![ CheckCodePrefix::B01 => vec![
CheckCode::B011, CheckCode::B011,
CheckCode::B013, CheckCode::B013,
@ -1051,6 +1056,7 @@ impl CheckCodePrefix {
CheckCodePrefix::B006 => PrefixSpecificity::Explicit, CheckCodePrefix::B006 => PrefixSpecificity::Explicit,
CheckCodePrefix::B007 => PrefixSpecificity::Explicit, CheckCodePrefix::B007 => PrefixSpecificity::Explicit,
CheckCodePrefix::B008 => PrefixSpecificity::Explicit, CheckCodePrefix::B008 => PrefixSpecificity::Explicit,
CheckCodePrefix::B009 => PrefixSpecificity::Explicit,
CheckCodePrefix::B01 => PrefixSpecificity::Tens, CheckCodePrefix::B01 => PrefixSpecificity::Tens,
CheckCodePrefix::B011 => PrefixSpecificity::Explicit, CheckCodePrefix::B011 => PrefixSpecificity::Explicit,
CheckCodePrefix::B013 => PrefixSpecificity::Explicit, CheckCodePrefix::B013 => PrefixSpecificity::Explicit,

View File

@ -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<Regex> =
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),
));
}
}
}
}
}
}

View File

@ -4,6 +4,7 @@ pub use assignment_to_os_environ::assignment_to_os_environ;
pub use cannot_raise_literal::cannot_raise_literal; pub use cannot_raise_literal::cannot_raise_literal;
pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions}; pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions};
pub use function_call_argument_default::function_call_argument_default; 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 mutable_argument_default::mutable_argument_default;
pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler; 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; 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 cannot_raise_literal;
mod duplicate_exceptions; mod duplicate_exceptions;
mod function_call_argument_default; mod function_call_argument_default;
mod getattr_with_constant;
mod mutable_argument_default; mod mutable_argument_default;
mod redundant_tuple_in_exception_handler; mod redundant_tuple_in_exception_handler;
mod star_arg_unpacking_after_keyword_arg; mod star_arg_unpacking_after_keyword_arg;

View File

@ -299,6 +299,7 @@ mod tests {
#[test_case(CheckCode::B006, Path::new("B006_B008.py"); "B006")] #[test_case(CheckCode::B006, Path::new("B006_B008.py"); "B006")]
#[test_case(CheckCode::B007, Path::new("B007.py"); "B007")] #[test_case(CheckCode::B007, Path::new("B007.py"); "B007")]
#[test_case(CheckCode::B008, Path::new("B006_B008.py"); "B008")] #[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::B011, Path::new("B011.py"); "B011")]
#[test_case(CheckCode::B013, Path::new("B013.py"); "B013")] #[test_case(CheckCode::B013, Path::new("B013.py"); "B013")]
#[test_case(CheckCode::B014, Path::new("B014.py"); "B014")] #[test_case(CheckCode::B014, Path::new("B014.py"); "B014")]

7
src/python/keyword.rs Normal file
View File

@ -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",
];

View File

@ -1,3 +1,4 @@
pub mod builtins; pub mod builtins;
pub mod future; pub mod future;
pub mod keyword;
pub mod typing; pub mod typing;

View File

@ -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: ~