From 022ff64d29f99e75c0374ead4d421334fc927f1a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 10 Oct 2022 10:55:55 -0400 Subject: [PATCH] Implement B011 from flake8-bugbear (#390) --- README.md | 6 +- resources/test/fixtures/B011.py | 4 ++ src/check_ast.rs | 5 +- src/checks.rs | 13 +++++ src/code_gen.rs | 2 +- src/linter.rs | 12 ++++ src/plugins.rs | 2 + src/plugins/assert_false.rs | 61 ++++++++++++++++++++ src/snapshots/ruff__linter__tests__b011.snap | 37 ++++++++++++ 9 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 resources/test/fixtures/B011.py create mode 100644 src/plugins/assert_false.rs create mode 100644 src/snapshots/ruff__linter__tests__b011.snap diff --git a/README.md b/README.md index f16cfc4cc0..4cc1cec2d1 100644 --- a/README.md +++ b/README.md @@ -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-super`](https://pypi.org/project/flake8-super/) - [`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) 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 | | | | A002 | BuiltinArgumentShadowing | Argument `...` 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 | | | | C401 | UnnecessaryGeneratorSet | Unnecessary generator - rewrite as a set 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 | | 🛠 | | U003 | TypeOfPrimitive | Use `str` instead of `type(...)` | | 🛠 | | 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 | | 🛠 | | U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | 🛠 | | U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | | 🛠 | diff --git a/resources/test/fixtures/B011.py b/resources/test/fixtures/B011.py new file mode 100644 index 0000000000..5dd14bd2c4 --- /dev/null +++ b/resources/test/fixtures/B011.py @@ -0,0 +1,4 @@ +assert 1 != 2 +assert False +assert 1 != 2, "message" +assert False, "message" diff --git a/src/check_ast.rs b/src/check_ast.rs index 9c3401b0c2..6528315020 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -548,10 +548,13 @@ where plugins::if_tuple(self, stmt, test); } } - StmtKind::Assert { test, .. } => { + StmtKind::Assert { test, msg } => { if self.settings.enabled.contains(&CheckCode::F631) { plugins::assert_tuple(self, stmt, test); } + if self.settings.enabled.contains(&CheckCode::B011) { + plugins::assert_false(self, stmt, test, msg); + } } StmtKind::Try { handlers, .. } => { if self.settings.enabled.contains(&CheckCode::F707) { diff --git a/src/checks.rs b/src/checks.rs index 564fc06477..35cd1aa042 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -121,6 +121,8 @@ pub enum CheckCode { A001, A002, A003, + // flake8-bugbear + B011, // flake8-comprehensions C400, C401, @@ -213,6 +215,8 @@ pub enum CheckKind { BuiltinVariableShadowing(String), BuiltinArgumentShadowing(String), BuiltinAttributeShadowing(String), + // flake8-bugbear + DoNotAssertFalse, // flake8-comprehensions UnnecessaryGeneratorList, UnnecessaryGeneratorSet, @@ -306,6 +310,8 @@ impl CheckCode { CheckCode::A001 => CheckKind::BuiltinVariableShadowing("...".to_string()), CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()), CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()), + // flake8-bugbear + CheckCode::B011 => CheckKind::DoNotAssertFalse, // flake8-comprehensions CheckCode::C400 => CheckKind::UnnecessaryGeneratorList, CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet, @@ -400,6 +406,8 @@ impl CheckKind { CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001, CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002, CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003, + // flake8-bugbear + CheckKind::DoNotAssertFalse => &CheckCode::B011, // flake8-comprehensions CheckKind::UnnecessaryGeneratorList => &CheckCode::C400, CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401, @@ -572,6 +580,10 @@ impl CheckKind { CheckKind::BuiltinAttributeShadowing(name) => { 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 CheckKind::UnnecessaryGeneratorList => { "Unnecessary generator - rewrite as a list comprehension".to_string() @@ -675,6 +687,7 @@ impl CheckKind { matches!( self, CheckKind::DeprecatedUnittestAlias(_, _) + | CheckKind::DoNotAssertFalse | CheckKind::PPrintFound | CheckKind::PrintFound | CheckKind::SuperCallWithParameters diff --git a/src/code_gen.rs b/src/code_gen.rs index 4e6c39fa69..0a7ab34dfc 100644 --- a/src/code_gen.rs +++ b/src/code_gen.rs @@ -115,7 +115,7 @@ impl SourceGenerator { Ok(()) } - fn unparse_stmt(&mut self, ast: &Stmt) -> fmt::Result { + pub fn unparse_stmt(&mut self, ast: &Stmt) -> fmt::Result { macro_rules! statement { ($body:block) => {{ self.newline()?; diff --git a/src/linter.rs b/src/linter.rs index 90fc6713f6..d679e17688 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -810,6 +810,18 @@ mod tests { 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] fn c400() -> Result<()> { let mut checks = check_path( diff --git a/src/plugins.rs b/src/plugins.rs index cf34eef8b1..da6aaaee90 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -1,3 +1,4 @@ +pub use assert_false::assert_false; pub use assert_tuple::assert_tuple; pub use deprecated_unittest_alias::deprecated_unittest_alias; 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_object_inheritance::useless_object_inheritance; +mod assert_false; mod assert_tuple; mod deprecated_unittest_alias; mod if_tuple; diff --git a/src/plugins/assert_false.rs b/src/plugins/assert_false.rs new file mode 100644 index 0000000000..c4393f36d9 --- /dev/null +++ b/src/plugins/assert_false.rs @@ -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>) -> 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>) { + 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); + } +} diff --git a/src/snapshots/ruff__linter__tests__b011.snap b/src/snapshots/ruff__linter__tests__b011.snap new file mode 100644 index 0000000000..bce0cf16c9 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__b011.snap @@ -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 +