From ca45952e33c6ab23774b21a1112cc93d60d1ee5f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 3 Sep 2022 22:53:59 -0400 Subject: [PATCH] Implement R002 (NoAssertEquals) --- README.md | 3 +- examples/generate_rules_table.rs | 1 + resources/test/fixtures/{R0205.py => R001.py} | 0 resources/test/fixtures/R002.py | 3 ++ resources/test/fixtures/pyproject.toml | 3 +- src/check_ast.rs | 40 ++++++++++++++-- src/checks.rs | 43 ++++++++++++++--- src/linter.rs | 47 +++++++++++++++++-- src/pyproject.rs | 3 +- 9 files changed, 126 insertions(+), 17 deletions(-) rename resources/test/fixtures/{R0205.py => R001.py} (100%) create mode 100644 resources/test/fixtures/R002.py diff --git a/README.md b/README.md index 85b702165a..63114ff219 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,8 @@ OPTIONS: | F831 | DuplicateArgumentName | Duplicate argument name in function definition | | F841 | UnusedVariable | Local variable `...` is assigned to but never used | | F901 | RaiseNotImplemented | `raise NotImplemented` should be `raise NotImplementedError` | -| R0205 | UselessObjectInheritance | Class `...` inherits from object | +| R001 | UselessObjectInheritance | Class `...` inherits from object | +| R002 | NoAssertEquals | `assertEquals` is deprecated, use `assertEqual` instead | ## Development diff --git a/examples/generate_rules_table.rs b/examples/generate_rules_table.rs index fdd7b0d57e..5959966e4a 100644 --- a/examples/generate_rules_table.rs +++ b/examples/generate_rules_table.rs @@ -11,6 +11,7 @@ fn main() { CheckKind::ImportStarUsage, CheckKind::LineTooLong, CheckKind::ModuleImportNotAtTopOfFile, + CheckKind::NoAssertEquals, CheckKind::RaiseNotImplemented, CheckKind::ReturnOutsideFunction, CheckKind::UndefinedExport("...".to_string()), diff --git a/resources/test/fixtures/R0205.py b/resources/test/fixtures/R001.py similarity index 100% rename from resources/test/fixtures/R0205.py rename to resources/test/fixtures/R001.py diff --git a/resources/test/fixtures/R002.py b/resources/test/fixtures/R002.py new file mode 100644 index 0000000000..779eb5689c --- /dev/null +++ b/resources/test/fixtures/R002.py @@ -0,0 +1,3 @@ +self.assertEquals (1, 2) +self.assertEquals(1, 2) +self.assertEqual(3, 4) diff --git a/resources/test/fixtures/pyproject.toml b/resources/test/fixtures/pyproject.toml index c058053dfa..135ca6f6fd 100644 --- a/resources/test/fixtures/pyproject.toml +++ b/resources/test/fixtures/pyproject.toml @@ -18,5 +18,6 @@ select = [ "F831", "F841", "F901", - "R0205", + "R001", + "R002", ] diff --git a/src/check_ast.rs b/src/check_ast.rs index fe4d87943e..20bf59a67c 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -2,8 +2,8 @@ use std::collections::BTreeSet; use std::path::Path; use rustpython_parser::ast::{ - Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt, - StmtKind, Suite, + Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, + Location, Stmt, StmtKind, Suite, }; use rustpython_parser::parser; @@ -11,7 +11,7 @@ use crate::ast_ops::{ extract_all_names, Binding, BindingKind, Scope, ScopeKind, SourceCodeLocator, }; use crate::builtins::{BUILTINS, MAGIC_GLOBALS}; -use crate::checks::{Check, CheckCode, CheckKind}; +use crate::checks::{Check, CheckCode, CheckKind, Fix}; use crate::settings::Settings; use crate::visitor::{walk_excepthandler, Visitor}; use crate::{autofix, fixer, visitor}; @@ -132,7 +132,7 @@ impl Visitor for Checker<'_> { decorator_list, .. } => { - if self.settings.select.contains(&CheckCode::R0205) { + if self.settings.select.contains(&CheckCode::R001) { for expr in bases { if let ExprKind::Name { id, .. } = &expr.node { if id == "object" { @@ -159,6 +159,7 @@ impl Visitor for Checker<'_> { ) { check.amend(fix); } + } else { } self.checks.push(check); } @@ -442,6 +443,37 @@ impl Visitor for Checker<'_> { ExprContext::Store => self.handle_node_store(expr, parent), ExprContext::Del => self.handle_node_delete(expr), }, + ExprKind::Call { func, .. } => { + if self.settings.select.contains(&CheckCode::R002) { + if let ExprKind::Attribute { value, attr, .. } = &func.node { + if attr == "assertEquals" { + if let ExprKind::Name { id, .. } = &value.node { + if id == "self" { + let mut check = + Check::new(CheckKind::NoAssertEquals, expr.location); + if matches!(self.autofix, autofix::Mode::Generate) + || matches!(self.autofix, autofix::Mode::Apply) + { + check.amend(Fix { + content: "assertEqual".to_string(), + start: Location::new( + func.location.row(), + func.location.column() + 1, + ), + end: Location::new( + func.location.row(), + func.location.column() + 1 + "assertEquals".len(), + ), + applied: false, + }); + } + self.checks.push(check); + } + } + } + } + } + } ExprKind::GeneratorExp { .. } | ExprKind::ListComp { .. } | ExprKind::DictComp { .. } diff --git a/src/checks.rs b/src/checks.rs index 5d905719ed..424d2443ef 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -24,7 +24,8 @@ pub enum CheckCode { F831, F841, F901, - R0205, + R001, + R002, } impl FromStr for CheckCode { @@ -48,7 +49,8 @@ impl FromStr for CheckCode { "F831" => Ok(CheckCode::F831), "F841" => Ok(CheckCode::F841), "F901" => Ok(CheckCode::F901), - "R0205" => Ok(CheckCode::R0205), + "R001" => Ok(CheckCode::R001), + "R002" => Ok(CheckCode::R002), _ => Err(anyhow::anyhow!("Unknown check code: {s}")), } } @@ -68,12 +70,13 @@ impl CheckCode { CheckCode::F706 => "F706", CheckCode::F707 => "F707", CheckCode::F821 => "F821", - CheckCode::F823 => "F823", CheckCode::F822 => "F822", + CheckCode::F823 => "F823", CheckCode::F831 => "F831", CheckCode::F841 => "F841", CheckCode::F901 => "F901", - CheckCode::R0205 => "R0205", + CheckCode::R001 => "R001", + CheckCode::R002 => "R002", } } @@ -96,7 +99,8 @@ impl CheckCode { CheckCode::F831 => &LintSource::AST, CheckCode::F841 => &LintSource::AST, CheckCode::F901 => &LintSource::AST, - CheckCode::R0205 => &LintSource::AST, + CheckCode::R001 => &LintSource::AST, + CheckCode::R002 => &LintSource::AST, } } } @@ -117,6 +121,7 @@ pub enum CheckKind { ImportStarUsage, LineTooLong, ModuleImportNotAtTopOfFile, + NoAssertEquals, RaiseNotImplemented, ReturnOutsideFunction, UndefinedExport(String), @@ -140,6 +145,7 @@ impl CheckKind { CheckKind::ImportStarUsage => "ImportStarUsage", CheckKind::LineTooLong => "LineTooLong", CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile", + CheckKind::NoAssertEquals => "NoAssertEquals", CheckKind::RaiseNotImplemented => "RaiseNotImplemented", CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction", CheckKind::UndefinedExport(_) => "UndefinedExport", @@ -163,6 +169,7 @@ impl CheckKind { CheckKind::ImportStarUsage => &CheckCode::F403, CheckKind::LineTooLong => &CheckCode::E501, CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402, + CheckKind::NoAssertEquals => &CheckCode::R002, CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::ReturnOutsideFunction => &CheckCode::F706, CheckKind::UndefinedExport(_) => &CheckCode::F822, @@ -170,7 +177,7 @@ impl CheckKind { CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UnusedImport(_) => &CheckCode::F401, CheckKind::UnusedVariable(_) => &CheckCode::F841, - CheckKind::UselessObjectInheritance(_) => &CheckCode::R0205, + CheckKind::UselessObjectInheritance(_) => &CheckCode::R001, CheckKind::YieldOutsideFunction => &CheckCode::F704, } } @@ -196,6 +203,9 @@ impl CheckKind { CheckKind::ModuleImportNotAtTopOfFile => { "Module level import not at top of file".to_string() } + CheckKind::NoAssertEquals => { + "`assertEquals` is deprecated, use `assertEqual` instead".to_string() + } CheckKind::RaiseNotImplemented => { "`raise NotImplemented` should be `raise NotImplementedError`".to_string() } @@ -226,7 +236,26 @@ impl CheckKind { /// Whether the check kind is (potentially) fixable. pub fn fixable(&self) -> bool { - matches!(self, CheckKind::UselessObjectInheritance(_)) + match self { + CheckKind::AssertTuple => false, + CheckKind::DefaultExceptNotLast => false, + CheckKind::DuplicateArgumentName => false, + CheckKind::FStringMissingPlaceholders => false, + CheckKind::IfTuple => false, + CheckKind::ImportStarUsage => false, + CheckKind::LineTooLong => false, + CheckKind::ModuleImportNotAtTopOfFile => false, + CheckKind::NoAssertEquals => true, + CheckKind::RaiseNotImplemented => false, + CheckKind::ReturnOutsideFunction => false, + CheckKind::UndefinedExport(_) => false, + CheckKind::UndefinedLocal(_) => false, + CheckKind::UndefinedName(_) => false, + CheckKind::UnusedImport(_) => false, + CheckKind::UnusedVariable(_) => false, + CheckKind::UselessObjectInheritance(_) => true, + CheckKind::YieldOutsideFunction => false, + } } } diff --git a/src/linter.rs b/src/linter.rs index 2ba8ad6aa7..290816b21e 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -591,13 +591,13 @@ mod tests { } #[test] - fn r0205() -> Result<()> { + fn r001() -> Result<()> { let actual = check_path( - Path::new("./resources/test/fixtures/R0205.py"), + Path::new("./resources/test/fixtures/R001.py"), &settings::Settings { line_length: 88, exclude: vec![], - select: BTreeSet::from([CheckCode::R0205]), + select: BTreeSet::from([CheckCode::R001]), }, &autofix::Mode::Generate, )?; @@ -810,4 +810,45 @@ mod tests { Ok(()) } + + #[test] + fn r002() -> Result<()> { + let actual = check_path( + Path::new("./resources/test/fixtures/R002.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::R002]), + }, + &autofix::Mode::Generate, + )?; + let expected = vec![ + Check { + kind: CheckKind::NoAssertEquals, + location: Location::new(1, 19), + fix: Some(Fix { + content: "assertEqual".to_string(), + start: Location::new(1, 6), + end: Location::new(1, 18), + applied: false, + }), + }, + Check { + kind: CheckKind::NoAssertEquals, + location: Location::new(2, 18), + fix: Some(Fix { + content: "assertEqual".to_string(), + start: Location::new(2, 6), + end: Location::new(2, 18), + applied: false, + }), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } } diff --git a/src/pyproject.rs b/src/pyproject.rs index cc946182ab..e40a6b4fbc 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -253,7 +253,8 @@ other-attribute = 1 CheckCode::F831, CheckCode::F841, CheckCode::F901, - CheckCode::R0205, + CheckCode::R001, + CheckCode::R002, ])), } );