Implement E713 and E714 (#111)

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 |

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),

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)

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)

resources/test/fixtures/pyproject.toml

@@ -6,6 +6,8 @@ select = [
     "E501",
     "E711",
     "E712",
+    "E713",
+    "E714",
     "E731",
     "E902",
     "F401",

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,

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,

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(

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,

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,
                 ])
             }),
         })