Merged in dmand/pylint/comparison-checker (pull request #280)

Add checker for comparisons to singleton values (True, False, None)

4 files changed, 101 insertions, 0 deletions

diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index f90172a..15ff292 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1436,6 +1436,44 @@ class LambdaForComprehensionChecker(_BasicChecker):
                 and infered.name in ['map', 'filter']):
             self.add_message('deprecated-lambda', node=node)
 
+class ComparisonChecker(_BasicChecker):
+    """checks for 'expr == True', 'expr == False' and 'expr == None'"""
+    msgs = {'C0121': ('Comparison to %s should be %s',
+                      'singleton-comparison',
+                      'Used when an expression is compared to singleton '
+                      'values like True, False or None.'),
+           }
+
+    def check_singleton_comparison(self, singleton, expr, root_node):
+        if singleton.value is True:
+            suggestion = "just 'expr' or 'expr is True'"
+            self.add_message('singleton-comparison',
+                             node=root_node,
+                             args=(True, suggestion))
+        elif singleton.value is False:
+            suggestion = "'not expr' or 'expr is False'"
+            self.add_message('singleton-comparison',
+                             node=root_node,
+                             args=(False, suggestion))
+        elif singleton.value is None:
+            self.add_message('singleton-comparison',
+                             node=root_node,
+                             args=(None, "'expr is None'"))
+
+    @check_messages('singleton-comparison')
+    def visit_compare(self, node):
+        # NOTE: this checker only works with binary comparisons like 'x == 42'
+        # but not 'x == y == 42'
+        if len(node.ops) != 1:
+            return
+
+        left = node.left
+        operator, right = node.ops[0]
+        if operator == '==' and isinstance(left, astroid.Const):
+            self.check_singleton_comparison(left, right, node)
+        elif operator == '==' and isinstance(right, astroid.Const):
+            self.check_singleton_comparison(right, left, node)
+
 
 def register(linter):
     """required method to auto register this checker"""
@@ -1445,3 +1483,4 @@ def register(linter):
     linter.register_checker(DocStringChecker(linter))
     linter.register_checker(PassChecker(linter))
     linter.register_checker(LambdaForComprehensionChecker(linter))
+    linter.register_checker(ComparisonChecker(linter))
diff --git a/pylint/test/functional/comparisons.py b/pylint/test/functional/comparisons.py
new file mode 100644
index 0000000..73067c8
--- /dev/null
+++ b/pylint/test/functional/comparisons.py
@@ -0,0 +1,11 @@
+# pylint: disable=missing-docstring, invalid-name
+x = 42
+a = x is None
+b = x == None  # [singleton-comparison]
+c = x == True  # [singleton-comparison]
+d = x == False  # [singleton-comparison]
+e = True == True  # [singleton-comparison]
+f = x is 1
+g = 123 is "123"
+h = None is x
+i = None == x  # [singleton-comparison]
diff --git a/pylint/test/functional/comparisons.txt b/pylint/test/functional/comparisons.txt
new file mode 100644
index 0000000..fd1fa98
--- /dev/null
+++ b/pylint/test/functional/comparisons.txt
@@ -0,0 +1,5 @@
+singleton-comparison:4::Comparison to None should be 'expr is None'
+singleton-comparison:5::Comparison to True should be just 'expr' or 'expr is True'
+singleton-comparison:6::Comparison to False should be 'not expr' or 'expr is False'
+singleton-comparison:7::Comparison to True should be just 'expr' or 'expr is True'
+singleton-comparison:11::Comparison to None should be 'expr is None'
diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py
index f19544f..c68f379 100644
--- a/pylint/test/unittest_checker_base.py
+++ b/pylint/test/unittest_checker_base.py
@@ -246,6 +246,52 @@ class MultiNamingStyleTest(CheckerTestCase):
                 self.checker.visit_functiondef(func)
             self.checker.leave_module(func.root)
 
+class ComparisonTest(CheckerTestCase):
+    CHECKER_CLASS = base.ComparisonChecker
+
+    def test_singleton_comparison(self):
+        node = test_utils.extract_node("foo == True")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(True, "just 'expr' or 'expr is True'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
+        node = test_utils.extract_node("foo == False")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(False, "'not expr' or 'expr is False'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
+        node = test_utils.extract_node("foo == None")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(None, "'expr is None'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
+        node = test_utils.extract_node("True == foo")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(True, "just 'expr' or 'expr is True'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
+        node = test_utils.extract_node("False == foo")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(False, "'not expr' or 'expr is False'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
+        node = test_utils.extract_node("None == foo")
+        message = Message('singleton-comparison',
+                          node=node,
+                          args=(None, "'expr is None'"))
+        with self.assertAddsMessages(message):
+            self.checker.visit_compare(node)
+
 
 if __name__ == '__main__':
     unittest.main()