Implement B024 and B027 (#738)

This commit is contained in:
Harutaka Kawamura
2022-11-15 03:12:23 +09:00
committed by GitHub
parent d170388b7b
commit 9047bf680d
11 changed files with 530 additions and 2 deletions

View File

@@ -452,6 +452,14 @@ where
flake8_bugbear::plugins::useless_expression(self, body);
}
if self.settings.enabled.contains(&CheckCode::B024)
|| self.settings.enabled.contains(&CheckCode::B027)
{
flake8_bugbear::plugins::abstract_base_class(
self, stmt, name, bases, keywords, body,
);
}
self.check_builtin_shadowing(name, Range::from_located(stmt), false);
for expr in bases {

View File

@@ -97,8 +97,10 @@ pub enum CheckCode {
B019,
B021,
B022,
B024,
B025,
B026,
B027,
// flake8-comprehensions
C400,
C401,
@@ -399,8 +401,10 @@ pub enum CheckKind {
CachedInstanceMethod,
FStringDocstring,
UselessContextlibSuppress,
AbstractBaseClassWithoutAbstractMethod(String),
DuplicateTryBlockException(String),
StarArgUnpackingAfterKeywordArg,
EmptyMethodWithoutAbstractDecorator(String),
// flake8-comprehensions
UnnecessaryGeneratorList,
UnnecessaryGeneratorSet,
@@ -643,8 +647,10 @@ impl CheckCode {
CheckCode::B019 => CheckKind::CachedInstanceMethod,
CheckCode::B021 => CheckKind::FStringDocstring,
CheckCode::B022 => CheckKind::UselessContextlibSuppress,
CheckCode::B024 => CheckKind::AbstractBaseClassWithoutAbstractMethod("...".to_string()),
CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()),
CheckCode::B026 => CheckKind::StarArgUnpackingAfterKeywordArg,
CheckCode::B027 => CheckKind::EmptyMethodWithoutAbstractDecorator("...".to_string()),
// flake8-comprehensions
CheckCode::C400 => CheckKind::UnnecessaryGeneratorList,
CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet,
@@ -886,8 +892,10 @@ impl CheckCode {
CheckCode::B019 => CheckCategory::Flake8Bugbear,
CheckCode::B021 => CheckCategory::Flake8Bugbear,
CheckCode::B022 => CheckCategory::Flake8Bugbear,
CheckCode::B024 => CheckCategory::Flake8Bugbear,
CheckCode::B025 => CheckCategory::Flake8Bugbear,
CheckCode::B026 => CheckCategory::Flake8Bugbear,
CheckCode::B027 => CheckCategory::Flake8Bugbear,
CheckCode::C400 => CheckCategory::Flake8Comprehensions,
CheckCode::C401 => CheckCategory::Flake8Comprehensions,
CheckCode::C402 => CheckCategory::Flake8Comprehensions,
@@ -1092,8 +1100,10 @@ impl CheckKind {
CheckKind::CachedInstanceMethod => &CheckCode::B019,
CheckKind::FStringDocstring => &CheckCode::B021,
CheckKind::UselessContextlibSuppress => &CheckCode::B022,
CheckKind::AbstractBaseClassWithoutAbstractMethod(_) => &CheckCode::B024,
CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025,
CheckKind::StarArgUnpackingAfterKeywordArg => &CheckCode::B026,
CheckKind::EmptyMethodWithoutAbstractDecorator(_) => &CheckCode::B027,
// flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401,
@@ -1470,6 +1480,9 @@ impl CheckKind {
and therefore this context manager is redundant"
.to_string()
}
CheckKind::AbstractBaseClassWithoutAbstractMethod(name) => {
format!("`{name}` is an abstract base class, but it has no abstract methods")
}
CheckKind::DuplicateTryBlockException(name) => {
format!("try-except block with duplicate exception `{name}`")
}
@@ -1479,6 +1492,12 @@ impl CheckKind {
unpacked sequence, and this change of ordering can surprise and mislead readers."
.to_string()
}
CheckKind::EmptyMethodWithoutAbstractDecorator(name) => {
format!(
"`{name}` is an empty method in an abstract base class, but has no abstract \
decorator"
)
}
// flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => {
"Unnecessary generator (rewrite as a `list` comprehension)".to_string()

View File

@@ -58,8 +58,10 @@ pub enum CheckCodePrefix {
B02,
B021,
B022,
B024,
B025,
B026,
B027,
C,
C4,
C40,
@@ -386,8 +388,10 @@ impl CheckCodePrefix {
CheckCode::B019,
CheckCode::B021,
CheckCode::B022,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
CheckCode::B027,
],
CheckCodePrefix::B0 => vec![
CheckCode::B002,
@@ -410,8 +414,10 @@ impl CheckCodePrefix {
CheckCode::B019,
CheckCode::B021,
CheckCode::B022,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
CheckCode::B027,
],
CheckCodePrefix::B00 => vec![
CheckCode::B002,
@@ -456,13 +462,17 @@ impl CheckCodePrefix {
CheckCodePrefix::B02 => vec![
CheckCode::B021,
CheckCode::B022,
CheckCode::B024,
CheckCode::B025,
CheckCode::B026,
CheckCode::B027,
],
CheckCodePrefix::B021 => vec![CheckCode::B021],
CheckCodePrefix::B022 => vec![CheckCode::B022],
CheckCodePrefix::B024 => vec![CheckCode::B024],
CheckCodePrefix::B025 => vec![CheckCode::B025],
CheckCodePrefix::B026 => vec![CheckCode::B026],
CheckCodePrefix::B027 => vec![CheckCode::B027],
CheckCodePrefix::C => vec![
CheckCode::C400,
CheckCode::C401,
@@ -1205,8 +1215,10 @@ impl CheckCodePrefix {
CheckCodePrefix::B02 => PrefixSpecificity::Tens,
CheckCodePrefix::B021 => PrefixSpecificity::Explicit,
CheckCodePrefix::B022 => PrefixSpecificity::Explicit,
CheckCodePrefix::B024 => PrefixSpecificity::Explicit,
CheckCodePrefix::B025 => PrefixSpecificity::Explicit,
CheckCodePrefix::B026 => PrefixSpecificity::Explicit,
CheckCodePrefix::B027 => PrefixSpecificity::Explicit,
CheckCodePrefix::C => PrefixSpecificity::Category,
CheckCodePrefix::C4 => PrefixSpecificity::Hundreds,
CheckCodePrefix::C40 => PrefixSpecificity::Tens,

View File

@@ -0,0 +1,111 @@
use fnv::{FnvHashMap, FnvHashSet};
use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind};
use crate::ast::helpers::{compose_call_path, match_call_path};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
fn is_abc_class(
bases: &[Expr],
keywords: &[Keyword],
from_imports: &FnvHashMap<&str, FnvHashSet<&str>>,
) -> bool {
keywords.iter().any(|keyword| {
keyword
.node
.arg
.as_ref()
.map(|a| a == "metaclass")
.unwrap_or(false)
&& compose_call_path(&keyword.node.value)
.map(|call_path| match_call_path(&call_path, "abc.ABCMeta", from_imports))
.unwrap_or(false)
}) || bases.iter().any(|base| {
compose_call_path(base)
.map(|call_path| match_call_path(&call_path, "abc.ABC", from_imports))
.unwrap_or(false)
})
}
fn is_empty_body(body: &[Stmt]) -> bool {
body.iter().all(|stmt| match &stmt.node {
StmtKind::Pass => true,
StmtKind::Expr { value } => match &value.node {
ExprKind::Constant { value, .. } => {
matches!(value, Constant::Str(..) | Constant::Ellipsis)
}
_ => false,
},
_ => false,
})
}
fn is_abstractmethod(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool {
compose_call_path(expr)
.map(|call_path| match_call_path(&call_path, "abc.abstractmethod", from_imports))
.unwrap_or(false)
}
fn is_overload(expr: &Expr, from_imports: &FnvHashMap<&str, FnvHashSet<&str>>) -> bool {
compose_call_path(expr)
.map(|call_path| match_call_path(&call_path, "typing.overload", from_imports))
.unwrap_or(false)
}
pub fn abstract_base_class(
checker: &mut Checker,
stmt: &Stmt,
name: &str,
bases: &[Expr],
keywords: &[Keyword],
body: &[Stmt],
) {
if bases.len() + keywords.len() == 1 && is_abc_class(bases, keywords, &checker.from_imports) {
let mut has_abstract_method = false;
for stmt in body {
// https://github.com/PyCQA/flake8-bugbear/issues/293
// Ignore abc's that declares a class attribute that must be set
if let StmtKind::AnnAssign { .. } | StmtKind::Assign { .. } = &stmt.node {
has_abstract_method = true;
continue;
}
if let StmtKind::FunctionDef {
decorator_list,
body,
..
}
| StmtKind::AsyncFunctionDef {
decorator_list,
body,
..
} = &stmt.node
{
let has_abstract_decorator = decorator_list
.iter()
.any(|d| is_abstractmethod(d, &checker.from_imports));
has_abstract_method |= has_abstract_decorator;
if !has_abstract_decorator
&& is_empty_body(body)
&& !decorator_list
.iter()
.any(|d| is_overload(d, &checker.from_imports))
{
checker.add_check(Check::new(
CheckKind::EmptyMethodWithoutAbstractDecorator(name.to_string()),
Range::from_located(stmt),
));
}
}
}
if !has_abstract_method {
checker.add_check(Check::new(
CheckKind::AbstractBaseClassWithoutAbstractMethod(name.to_string()),
Range::from_located(stmt),
));
}
}
}

View File

@@ -1,3 +1,4 @@
pub use abstract_base_class::abstract_base_class;
pub use assert_false::assert_false;
pub use assert_raises_exception::assert_raises_exception;
pub use assignment_to_os_environ::assignment_to_os_environ;
@@ -20,6 +21,7 @@ pub use useless_comparison::useless_comparison;
pub use useless_contextlib_suppress::useless_contextlib_suppress;
pub use useless_expression::useless_expression;
mod abstract_base_class;
mod assert_false;
mod assert_raises_exception;
mod assignment_to_os_environ;

View File

@@ -346,8 +346,10 @@ mod tests {
#[test_case(CheckCode::B019, Path::new("B019.py"); "B019")]
#[test_case(CheckCode::B021, Path::new("B021.py"); "B021")]
#[test_case(CheckCode::B022, Path::new("B022.py"); "B022")]
#[test_case(CheckCode::B024, Path::new("B024.py"); "B024")]
#[test_case(CheckCode::B025, Path::new("B025.py"); "B025")]
#[test_case(CheckCode::B026, Path::new("B026.py"); "B026")]
#[test_case(CheckCode::B027, Path::new("B027.py"); "B027")]
#[test_case(CheckCode::C400, Path::new("C400.py"); "C400")]
#[test_case(CheckCode::C401, Path::new("C401.py"); "C401")]
#[test_case(CheckCode::C402, Path::new("C402.py"); "C402")]

View File

@@ -0,0 +1,86 @@
---
source: src/linter.rs
expression: checks
---
- kind:
AbstractBaseClassWithoutAbstractMethod: Base_1
location:
row: 17
column: 0
end_location:
row: 22
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: Base_4
location:
row: 34
column: 0
end_location:
row: 40
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: Base_5
location:
row: 40
column: 0
end_location:
row: 46
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: Base_6
location:
row: 46
column: 0
end_location:
row: 52
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: Base_7
location:
row: 52
column: 0
end_location:
row: 58
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: MetaBase_1
location:
row: 58
column: 0
end_location:
row: 63
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: abc_Base_1
location:
row: 69
column: 0
end_location:
row: 74
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: abc_Base_2
location:
row: 74
column: 0
end_location:
row: 79
column: 0
fix: ~
- kind:
AbstractBaseClassWithoutAbstractMethod: abc_set_class_variable_4
location:
row: 128
column: 0
end_location:
row: 130
column: 0
fix: ~

View File

@@ -0,0 +1,68 @@
---
source: src/linter.rs
expression: checks
---
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 13
column: 4
end_location:
row: 16
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 16
column: 4
end_location:
row: 19
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 19
column: 4
end_location:
row: 23
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 23
column: 4
end_location:
row: 30
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 31
column: 4
end_location:
row: 34
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 80
column: 4
end_location:
row: 83
column: 4
fix: ~
- kind:
EmptyMethodWithoutAbstractDecorator: AbstractClass
location:
row: 84
column: 4
end_location:
row: 87
column: 4
fix: ~