Catch more E713 and E714 with a regular expression; issue #236 continued

Author: florentx

CHANGES.txt

@@ -5,6 +5,9 @@ Changelog
 1.x (unreleased)
 ----------------
 
+* Report E713 and E714 when operators ``not in`` and ``is not`` are
+  recommended. (Issue #236)
+
 * Allow the checkers to report errors on empty files. (Issue #240)
 
 * Fix ignoring too many checks when ``--select`` is used with codes

pep8.py

@@ -99,6 +99,7 @@
 EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')
 WHITESPACE_AFTER_COMMA_REGEX = re.compile(r'[,;:]\s*(?:  |\t)')
 COMPARE_SINGLETON_REGEX = re.compile(r'([=!]=)\s*(None|False|True)')
+COMPARE_NEGATIVE_REGEX = re.compile(r'\b(not)\s+[^[({ ]+\s+(in|is)\s')
 COMPARE_TYPE_REGEX = re.compile(r'(?:[=!]=|is(?:\s+not)?)\s*type(?:s.\w+Type'
                                 r'|\s*\(\s*([^)]*[^ )])\s*\))')
 KEYWORD_REGEX = re.compile(r'(\s*)\b(?:%s)\b(\s*)' % r'|'.join(KEYWORDS))
@@ -937,6 +938,31 @@ def comparison_to_singleton(logical_line, noqa):
                                (code, singleton, msg))
 
 
+def comparison_negative(logical_line):
+    r"""
+    Negative comparison, either identity or membership, should be
+    done using "not in" and "is not".
+
+    Okay: if x not in y:\n    pass
+    Okay: assert (X in Y or X is Z)
+    Okay: if not (X in Y):\n    pass
+    Okay: zz = x is not y
+    E713: Z = not X in Y
+    E713: if not X.B in Y:\n    pass
+    E714: if not X is Y:\n    pass
+    E714: Z = not X.B is Y
+    """
+    match = COMPARE_NEGATIVE_REGEX.search(logical_line)
+    if match:
+        if match.group(2) == 'in':
+            msg = ("E713: Use the 'not in' "
+                   "operator for collection membership evaluation")
+        else:
+            msg = ("E714: Use the 'is not' "
+                   "operator when testing for unequal identities")
+        yield match.start(1), msg
+
+
 def comparison_type(logical_line):
     """
     Object type comparisons should always use isinstance() instead of
@@ -1018,40 +1044,6 @@ def python_3000_backticks(logical_line):
         yield pos, "W604 backticks are deprecated, use 'repr()'"
 
 
-def not_in(logical_line):
-    """
-    Check for use of "not in" for evaluating membership.
-
-    Okay: if x not in y:\n    pass
-    Okay: if not (X in Y or X is Z):\n    pass
-    Okay: if not (X in Y):\n    pass
-    E713: if not X in Y
-    E713: if not X.B in Y
-    """
-
-    split_line = logical_line.split()
-    if (len(split_line) == 5 and split_line[0] == 'if' and
-            split_line[1] == 'not' and split_line[3] == 'in' and not
-            split_line[2].startswith('(')):
-                yield (logical_line.find('not'), "E713: Use the 'not in' "
-                       "operator for collection membership evaluation")
-
-
-def is_not(logical_line):
-    """
-    Check for use of 'is not' for testing unequal identities.
-
-    Okay: if x is not y:\n    pass
-    E714: if not X is Y
-    E714: if not X.B is Y
-    """
-
-    split_line = logical_line.split()
-    if (len(split_line) == 5 and split_line[0] == 'if' and
-            split_line[1] == 'not' and split_line[3] == 'is'):
-                yield (logical_line.find('not'), "E714: Use the 'is not' "
-                       "operator when testing for unequal identities")
-
 ##############################################################################
 # Helper functions
 ##############################################################################

testsuite/E71.py

@@ -1,9 +1,41 @@
+#: E711
+if res == None:
+    pass
 #: E712
 if res == True:
     pass
 #: E712
 if res != False:
     pass
-#: E711
-if res == None:
+
+#
+#: E713
+if not X in Y:
+    pass
+#: E713
+if not X.B in Y:
+    pass
+#: E713
+if not X in Y and Z == "zero":
+    pass
+#: E713
+if X == "zero" or not Y in Z:
+    pass
+
+#
+#: E714
+if not X is Y:
+    pass
+#: E714
+if not X.B is Y:
+    pass
+#: Okay
+if x not in y:
+    pass
+if not (X in Y or X is Z):
+    pass
+if not (X in Y):
+    pass
+if x is not y:
     pass
+#: