mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 21:35:58 +00:00
Implement B011 from flake8-bugbear (#390)
This commit is contained in:
parent
5a06fb28fd
commit
022ff64d29
9 changed files with 138 additions and 4 deletions
|
@ -215,7 +215,8 @@ ruff also implements some of the most popular Flake8 plugins natively, including
|
||||||
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
|
- [`flake8-builtins`](https://pypi.org/project/flake8-builtins/)
|
||||||
- [`flake8-super`](https://pypi.org/project/flake8-super/)
|
- [`flake8-super`](https://pypi.org/project/flake8-super/)
|
||||||
- [`flake8-print`](https://pypi.org/project/flake8-print/)
|
- [`flake8-print`](https://pypi.org/project/flake8-print/)
|
||||||
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (partial)
|
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (11/16)
|
||||||
|
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (1/32)
|
||||||
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (partial)
|
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (partial)
|
||||||
|
|
||||||
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
|
Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
|
||||||
|
@ -278,6 +279,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
|
||||||
| A001 | BuiltinVariableShadowing | Variable `...` is shadowing a python builtin | | |
|
| A001 | BuiltinVariableShadowing | Variable `...` is shadowing a python builtin | | |
|
||||||
| A002 | BuiltinArgumentShadowing | Argument `...` is shadowing a python builtin | | |
|
| A002 | BuiltinArgumentShadowing | Argument `...` is shadowing a python builtin | | |
|
||||||
| A003 | BuiltinAttributeShadowing | Class attribute `...` is shadowing a python builtin | | |
|
| A003 | BuiltinAttributeShadowing | Class attribute `...` is shadowing a python builtin | | |
|
||||||
|
| B011 | DoNotAssertFalse | Do not `assert False` (`python -O` removes these calls). Instead, raise `AssertionError()`. | | 🛠 |
|
||||||
| C400 | UnnecessaryGeneratorList | Unnecessary generator - rewrite as a list comprehension | | |
|
| C400 | UnnecessaryGeneratorList | Unnecessary generator - rewrite as a list comprehension | | |
|
||||||
| C401 | UnnecessaryGeneratorSet | Unnecessary generator - rewrite as a set comprehension | | |
|
| C401 | UnnecessaryGeneratorSet | Unnecessary generator - rewrite as a set comprehension | | |
|
||||||
| C402 | UnnecessaryGeneratorDict | Unnecessary generator - rewrite as a dict comprehension | | |
|
| C402 | UnnecessaryGeneratorDict | Unnecessary generator - rewrite as a dict comprehension | | |
|
||||||
|
@ -295,7 +297,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
|
||||||
| U002 | UnnecessaryAbspath | `abspath(__file__)` is unnecessary in Python 3.9 and later | | 🛠 |
|
| U002 | UnnecessaryAbspath | `abspath(__file__)` is unnecessary in Python 3.9 and later | | 🛠 |
|
||||||
| U003 | TypeOfPrimitive | Use `str` instead of `type(...)` | | 🛠 |
|
| U003 | TypeOfPrimitive | Use `str` instead of `type(...)` | | 🛠 |
|
||||||
| U004 | UselessObjectInheritance | Class `...` inherits from object | | 🛠 |
|
| U004 | UselessObjectInheritance | Class `...` inherits from object | | 🛠 |
|
||||||
| U005 | NoAssertEquals | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 |
|
| U005 | DeprecatedUnittestAlias | `assertEquals` is deprecated, use `assertEqual` instead | | 🛠 |
|
||||||
| U006 | UsePEP585Annotation | Use `list` instead of `List` for type annotations | | 🛠 |
|
| U006 | UsePEP585Annotation | Use `list` instead of `List` for type annotations | | 🛠 |
|
||||||
| U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 |
|
| U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 |
|
||||||
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |
|
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 |
|
||||||
|
|
4
resources/test/fixtures/B011.py
vendored
Normal file
4
resources/test/fixtures/B011.py
vendored
Normal file
|
@ -0,0 +1,4 @@
|
||||||
|
assert 1 != 2
|
||||||
|
assert False
|
||||||
|
assert 1 != 2, "message"
|
||||||
|
assert False, "message"
|
|
@ -548,10 +548,13 @@ where
|
||||||
plugins::if_tuple(self, stmt, test);
|
plugins::if_tuple(self, stmt, test);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
StmtKind::Assert { test, .. } => {
|
StmtKind::Assert { test, msg } => {
|
||||||
if self.settings.enabled.contains(&CheckCode::F631) {
|
if self.settings.enabled.contains(&CheckCode::F631) {
|
||||||
plugins::assert_tuple(self, stmt, test);
|
plugins::assert_tuple(self, stmt, test);
|
||||||
}
|
}
|
||||||
|
if self.settings.enabled.contains(&CheckCode::B011) {
|
||||||
|
plugins::assert_false(self, stmt, test, msg);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
StmtKind::Try { handlers, .. } => {
|
StmtKind::Try { handlers, .. } => {
|
||||||
if self.settings.enabled.contains(&CheckCode::F707) {
|
if self.settings.enabled.contains(&CheckCode::F707) {
|
||||||
|
|
|
@ -121,6 +121,8 @@ pub enum CheckCode {
|
||||||
A001,
|
A001,
|
||||||
A002,
|
A002,
|
||||||
A003,
|
A003,
|
||||||
|
// flake8-bugbear
|
||||||
|
B011,
|
||||||
// flake8-comprehensions
|
// flake8-comprehensions
|
||||||
C400,
|
C400,
|
||||||
C401,
|
C401,
|
||||||
|
@ -213,6 +215,8 @@ pub enum CheckKind {
|
||||||
BuiltinVariableShadowing(String),
|
BuiltinVariableShadowing(String),
|
||||||
BuiltinArgumentShadowing(String),
|
BuiltinArgumentShadowing(String),
|
||||||
BuiltinAttributeShadowing(String),
|
BuiltinAttributeShadowing(String),
|
||||||
|
// flake8-bugbear
|
||||||
|
DoNotAssertFalse,
|
||||||
// flake8-comprehensions
|
// flake8-comprehensions
|
||||||
UnnecessaryGeneratorList,
|
UnnecessaryGeneratorList,
|
||||||
UnnecessaryGeneratorSet,
|
UnnecessaryGeneratorSet,
|
||||||
|
@ -306,6 +310,8 @@ impl CheckCode {
|
||||||
CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()),
|
CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()),
|
||||||
CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()),
|
CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()),
|
||||||
CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
|
CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
|
||||||
|
// flake8-bugbear
|
||||||
|
CheckCode::B011 => CheckKind::DoNotAssertFalse,
|
||||||
// flake8-comprehensions
|
// flake8-comprehensions
|
||||||
CheckCode::C400 => CheckKind::UnnecessaryGeneratorList,
|
CheckCode::C400 => CheckKind::UnnecessaryGeneratorList,
|
||||||
CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet,
|
CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet,
|
||||||
|
@ -400,6 +406,8 @@ impl CheckKind {
|
||||||
CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
|
CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
|
||||||
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
|
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
|
||||||
CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
|
CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
|
||||||
|
// flake8-bugbear
|
||||||
|
CheckKind::DoNotAssertFalse => &CheckCode::B011,
|
||||||
// flake8-comprehensions
|
// flake8-comprehensions
|
||||||
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
|
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
|
||||||
CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401,
|
CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401,
|
||||||
|
@ -572,6 +580,10 @@ impl CheckKind {
|
||||||
CheckKind::BuiltinAttributeShadowing(name) => {
|
CheckKind::BuiltinAttributeShadowing(name) => {
|
||||||
format!("Class attribute `{name}` is shadowing a python builtin")
|
format!("Class attribute `{name}` is shadowing a python builtin")
|
||||||
}
|
}
|
||||||
|
// flake8-bugbear
|
||||||
|
CheckKind::DoNotAssertFalse => {
|
||||||
|
"Do not `assert False` (`python -O` removes these calls). Instead, raise `AssertionError()`.".to_string()
|
||||||
|
}
|
||||||
// flake8-comprehensions
|
// flake8-comprehensions
|
||||||
CheckKind::UnnecessaryGeneratorList => {
|
CheckKind::UnnecessaryGeneratorList => {
|
||||||
"Unnecessary generator - rewrite as a list comprehension".to_string()
|
"Unnecessary generator - rewrite as a list comprehension".to_string()
|
||||||
|
@ -675,6 +687,7 @@ impl CheckKind {
|
||||||
matches!(
|
matches!(
|
||||||
self,
|
self,
|
||||||
CheckKind::DeprecatedUnittestAlias(_, _)
|
CheckKind::DeprecatedUnittestAlias(_, _)
|
||||||
|
| CheckKind::DoNotAssertFalse
|
||||||
| CheckKind::PPrintFound
|
| CheckKind::PPrintFound
|
||||||
| CheckKind::PrintFound
|
| CheckKind::PrintFound
|
||||||
| CheckKind::SuperCallWithParameters
|
| CheckKind::SuperCallWithParameters
|
||||||
|
|
|
@ -115,7 +115,7 @@ impl SourceGenerator {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
fn unparse_stmt<U>(&mut self, ast: &Stmt<U>) -> fmt::Result {
|
pub fn unparse_stmt<U>(&mut self, ast: &Stmt<U>) -> fmt::Result {
|
||||||
macro_rules! statement {
|
macro_rules! statement {
|
||||||
($body:block) => {{
|
($body:block) => {{
|
||||||
self.newline()?;
|
self.newline()?;
|
||||||
|
|
|
@ -810,6 +810,18 @@ mod tests {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn b011() -> Result<()> {
|
||||||
|
let mut checks = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/B011.py"),
|
||||||
|
&settings::Settings::for_rule(CheckCode::B011),
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
checks.sort_by_key(|check| check.location);
|
||||||
|
insta::assert_yaml_snapshot!(checks);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn c400() -> Result<()> {
|
fn c400() -> Result<()> {
|
||||||
let mut checks = check_path(
|
let mut checks = check_path(
|
||||||
|
|
|
@ -1,3 +1,4 @@
|
||||||
|
pub use assert_false::assert_false;
|
||||||
pub use assert_tuple::assert_tuple;
|
pub use assert_tuple::assert_tuple;
|
||||||
pub use deprecated_unittest_alias::deprecated_unittest_alias;
|
pub use deprecated_unittest_alias::deprecated_unittest_alias;
|
||||||
pub use if_tuple::if_tuple;
|
pub use if_tuple::if_tuple;
|
||||||
|
@ -11,6 +12,7 @@ pub use use_pep604_annotation::use_pep604_annotation;
|
||||||
pub use useless_metaclass_type::useless_metaclass_type;
|
pub use useless_metaclass_type::useless_metaclass_type;
|
||||||
pub use useless_object_inheritance::useless_object_inheritance;
|
pub use useless_object_inheritance::useless_object_inheritance;
|
||||||
|
|
||||||
|
mod assert_false;
|
||||||
mod assert_tuple;
|
mod assert_tuple;
|
||||||
mod deprecated_unittest_alias;
|
mod deprecated_unittest_alias;
|
||||||
mod if_tuple;
|
mod if_tuple;
|
||||||
|
|
61
src/plugins/assert_false.rs
Normal file
61
src/plugins/assert_false.rs
Normal file
|
@ -0,0 +1,61 @@
|
||||||
|
use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};
|
||||||
|
|
||||||
|
use crate::ast::types::Range;
|
||||||
|
use crate::autofix::fixer;
|
||||||
|
use crate::check_ast::Checker;
|
||||||
|
use crate::checks::{Check, CheckKind, Fix};
|
||||||
|
use crate::code_gen::SourceGenerator;
|
||||||
|
|
||||||
|
fn assertion_error(msg: &Option<Box<Expr>>) -> Stmt {
|
||||||
|
Stmt::new(
|
||||||
|
Default::default(),
|
||||||
|
Default::default(),
|
||||||
|
StmtKind::Raise {
|
||||||
|
exc: Some(Box::new(Expr::new(
|
||||||
|
Default::default(),
|
||||||
|
Default::default(),
|
||||||
|
ExprKind::Call {
|
||||||
|
func: Box::new(Expr::new(
|
||||||
|
Default::default(),
|
||||||
|
Default::default(),
|
||||||
|
ExprKind::Name {
|
||||||
|
id: "AssertionError".to_string(),
|
||||||
|
ctx: ExprContext::Load,
|
||||||
|
},
|
||||||
|
)),
|
||||||
|
args: if let Some(msg) = msg {
|
||||||
|
vec![*msg.clone()]
|
||||||
|
} else {
|
||||||
|
vec![]
|
||||||
|
},
|
||||||
|
keywords: vec![],
|
||||||
|
},
|
||||||
|
))),
|
||||||
|
cause: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: &Option<Box<Expr>>) {
|
||||||
|
if let ExprKind::Constant {
|
||||||
|
value: Constant::Bool(false),
|
||||||
|
..
|
||||||
|
} = &test.node
|
||||||
|
{
|
||||||
|
let mut check = Check::new(CheckKind::DoNotAssertFalse, Range::from_located(test));
|
||||||
|
if matches!(checker.autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
|
||||||
|
let mut generator = SourceGenerator::new();
|
||||||
|
if let Ok(()) = generator.unparse_stmt(&assertion_error(msg)) {
|
||||||
|
if let Ok(content) = generator.generate() {
|
||||||
|
check.amend(Fix {
|
||||||
|
content,
|
||||||
|
location: stmt.location,
|
||||||
|
end_location: stmt.end_location,
|
||||||
|
applied: false,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
checker.add_check(check);
|
||||||
|
}
|
||||||
|
}
|
37
src/snapshots/ruff__linter__tests__b011.snap
Normal file
37
src/snapshots/ruff__linter__tests__b011.snap
Normal file
|
@ -0,0 +1,37 @@
|
||||||
|
---
|
||||||
|
source: src/linter.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: DoNotAssertFalse
|
||||||
|
location:
|
||||||
|
row: 2
|
||||||
|
column: 8
|
||||||
|
end_location:
|
||||||
|
row: 2
|
||||||
|
column: 13
|
||||||
|
fix:
|
||||||
|
content: raise AssertionError()
|
||||||
|
location:
|
||||||
|
row: 2
|
||||||
|
column: 1
|
||||||
|
end_location:
|
||||||
|
row: 2
|
||||||
|
column: 13
|
||||||
|
applied: false
|
||||||
|
- kind: DoNotAssertFalse
|
||||||
|
location:
|
||||||
|
row: 4
|
||||||
|
column: 8
|
||||||
|
end_location:
|
||||||
|
row: 4
|
||||||
|
column: 13
|
||||||
|
fix:
|
||||||
|
content: "raise AssertionError(\"message\")"
|
||||||
|
location:
|
||||||
|
row: 4
|
||||||
|
column: 1
|
||||||
|
end_location:
|
||||||
|
row: 4
|
||||||
|
column: 24
|
||||||
|
applied: false
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue