diff --git a/README.md b/README.md index 50b0e5580a..cf3d70c095 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,8 @@ lint rules that are obviated by Black (e.g., stylistic rules). | E501 | LineTooLong | Line too long | | E711 | NoneComparison | Comparison to `None` should be `cond is None` | | E712 | TrueFalseComparison | Comparison to `True` should be `cond is True` | +| E713 | NotInTest | Test for membership should be `not in` | +| E714 | NotIsTest | Test for object identity should be `is not` | | E731 | DoNotAssignLambda | Do not assign a lambda expression, use a def | | E902 | IOError | No such file or directory: `...` | | F401 | UnusedImport | `...` imported but unused | diff --git a/examples/generate_rules_table.rs b/examples/generate_rules_table.rs index c7af58839a..6cd4a54bb1 100644 --- a/examples/generate_rules_table.rs +++ b/examples/generate_rules_table.rs @@ -15,6 +15,8 @@ fn main() { CheckKind::ModuleImportNotAtTopOfFile, CheckKind::NoAssertEquals, CheckKind::NoneComparison(RejectedCmpop::Eq), + CheckKind::NotInTest, + CheckKind::NotIsTest, CheckKind::RaiseNotImplemented, CheckKind::ReturnOutsideFunction, CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq), diff --git a/resources/test/fixtures/E713.py b/resources/test/fixtures/E713.py new file mode 100644 index 0000000000..216a99c68b --- /dev/null +++ b/resources/test/fixtures/E713.py @@ -0,0 +1,7 @@ +my_list = [1, 2, 3] +if not num in my_list: + print(num) + +my_list = [1, 2, 3] +if num not in my_list: + print(num) diff --git a/resources/test/fixtures/E714.py b/resources/test/fixtures/E714.py new file mode 100644 index 0000000000..fb692f0a19 --- /dev/null +++ b/resources/test/fixtures/E714.py @@ -0,0 +1,5 @@ +if not user is None: + print(user.name) + +if user is not None: + print(user.name) diff --git a/resources/test/fixtures/pyproject.toml b/resources/test/fixtures/pyproject.toml index ba44ffa78e..b2ab76ee56 100644 --- a/resources/test/fixtures/pyproject.toml +++ b/resources/test/fixtures/pyproject.toml @@ -6,6 +6,8 @@ select = [ "E501", "E711", "E712", + "E713", + "E714", "E731", "E902", "F401", diff --git a/src/check_ast.rs b/src/check_ast.rs index d99fc19a4d..43f0fa04b8 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -4,7 +4,7 @@ use std::path::Path; use itertools::izip; use rustpython_parser::ast::{ Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, - Location, Stmt, StmtKind, Suite, + Location, Stmt, StmtKind, Suite, Unaryop, }; use rustpython_parser::parser; @@ -523,6 +523,27 @@ impl Visitor for Checker<'_> { } self.in_f_string = true; } + ExprKind::UnaryOp { op, operand } => { + if matches!(op, Unaryop::Not) { + if let ExprKind::Compare { ops, .. } = &operand.node { + match ops[..] { + [Cmpop::In] => { + if self.settings.select.contains(CheckKind::NotInTest.code()) { + self.checks + .push(Check::new(CheckKind::NotInTest, operand.location)); + } + } + [Cmpop::Is] => { + if self.settings.select.contains(CheckKind::NotIsTest.code()) { + self.checks + .push(Check::new(CheckKind::NotIsTest, operand.location)); + } + } + _ => {} + } + } + } + } ExprKind::Compare { left, ops, diff --git a/src/checks.rs b/src/checks.rs index d98661cd87..a6a2e36fa9 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -12,6 +12,8 @@ pub enum CheckCode { E501, E711, E712, + E713, + E714, E731, E902, F401, @@ -41,6 +43,8 @@ impl FromStr for CheckCode { "E501" => Ok(CheckCode::E501), "E711" => Ok(CheckCode::E711), "E712" => Ok(CheckCode::E712), + "E713" => Ok(CheckCode::E713), + "E714" => Ok(CheckCode::E714), "E731" => Ok(CheckCode::E731), "E902" => Ok(CheckCode::E902), "F401" => Ok(CheckCode::F401), @@ -71,6 +75,8 @@ impl CheckCode { CheckCode::E501 => "E501", CheckCode::E711 => "E711", CheckCode::E712 => "E712", + CheckCode::E713 => "E713", + CheckCode::E714 => "E714", CheckCode::E731 => "E731", CheckCode::E902 => "E902", CheckCode::F401 => "F401", @@ -99,6 +105,8 @@ impl CheckCode { CheckCode::E501 => &LintSource::Lines, CheckCode::E711 => &LintSource::AST, CheckCode::E712 => &LintSource::AST, + CheckCode::E713 => &LintSource::AST, + CheckCode::E714 => &LintSource::AST, CheckCode::E731 => &LintSource::AST, CheckCode::E902 => &LintSource::FileSystem, CheckCode::F401 => &LintSource::AST, @@ -138,16 +146,18 @@ pub enum RejectedCmpop { pub enum CheckKind { AssertTuple, DefaultExceptNotLast, + DoNotAssignLambda, DuplicateArgumentName, FStringMissingPlaceholders, IOError(String), IfTuple, ImportStarUsage, LineTooLong, - DoNotAssignLambda, ModuleImportNotAtTopOfFile, NoAssertEquals, NoneComparison(RejectedCmpop), + NotInTest, + NotIsTest, RaiseNotImplemented, ReturnOutsideFunction, TrueFalseComparison(bool, RejectedCmpop), @@ -176,6 +186,8 @@ impl CheckKind { CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile", CheckKind::NoAssertEquals => "NoAssertEquals", CheckKind::NoneComparison(_) => "NoneComparison", + CheckKind::NotInTest => "NotInTest", + CheckKind::NotIsTest => "NotIsTest", CheckKind::RaiseNotImplemented => "RaiseNotImplemented", CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction", CheckKind::TrueFalseComparison(_, _) => "TrueFalseComparison", @@ -204,6 +216,8 @@ impl CheckKind { CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402, CheckKind::NoAssertEquals => &CheckCode::R002, CheckKind::NoneComparison(_) => &CheckCode::E711, + CheckKind::NotInTest => &CheckCode::E713, + CheckKind::NotIsTest => &CheckCode::E714, CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::ReturnOutsideFunction => &CheckCode::F706, CheckKind::TrueFalseComparison(_, _) => &CheckCode::E712, @@ -253,6 +267,8 @@ impl CheckKind { "Comparison to `None` should be `cond is not None`".to_string() } }, + CheckKind::NotInTest => "Test for membership should be `not in`".to_string(), + CheckKind::NotIsTest => "Test for object identity should be `is not`".to_string(), CheckKind::RaiseNotImplemented => { "`raise NotImplemented` should be `raise NotImplementedError`".to_string() } @@ -313,6 +329,8 @@ impl CheckKind { CheckKind::LineTooLong => false, CheckKind::ModuleImportNotAtTopOfFile => false, CheckKind::NoAssertEquals => true, + CheckKind::NotInTest => false, + CheckKind::NotIsTest => false, CheckKind::NoneComparison(_) => false, CheckKind::RaiseNotImplemented => false, CheckKind::ReturnOutsideFunction => false, diff --git a/src/linter.rs b/src/linter.rs index a19c1d3ede..cd731c9d17 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -209,6 +209,54 @@ mod tests { Ok(()) } + #[test] + fn e713() -> Result<()> { + let actual = check_path( + Path::new("./resources/test/fixtures/E713.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::E713]), + }, + &autofix::Mode::Generate, + )?; + let expected = vec![Check { + kind: CheckKind::NotInTest, + location: Location::new(2, 12), + fix: None, + }]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn e714() -> Result<()> { + let actual = check_path( + Path::new("./resources/test/fixtures/E714.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::E714]), + }, + &autofix::Mode::Generate, + )?; + let expected = vec![Check { + kind: CheckKind::NotIsTest, + location: Location::new(1, 13), + fix: None, + }]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + #[test] fn e731() -> Result<()> { let actual = check_path( diff --git a/src/pyproject.rs b/src/pyproject.rs index 11f3a126f3..4af4d0b77b 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -241,6 +241,8 @@ other-attribute = 1 CheckCode::E501, CheckCode::E711, CheckCode::E712, + CheckCode::E713, + CheckCode::E714, CheckCode::E731, CheckCode::E902, CheckCode::F401, diff --git a/src/settings.rs b/src/settings.rs index 2a81bc7a16..6597178128 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -46,6 +46,10 @@ impl Settings { BTreeSet::from([ CheckCode::E402, CheckCode::E501, + CheckCode::E711, + CheckCode::E712, + CheckCode::E713, + CheckCode::E714, CheckCode::E731, CheckCode::E902, CheckCode::F401, @@ -53,11 +57,18 @@ impl Settings { CheckCode::F541, CheckCode::F631, CheckCode::F634, + CheckCode::F704, CheckCode::F706, CheckCode::F707, + CheckCode::F821, + CheckCode::F822, CheckCode::F823, CheckCode::F831, + CheckCode::F841, CheckCode::F901, + // Disable refactoring codes by default. + // CheckCode::R001, + // CheckCode::R002, ]) }), })