Merge heads.

14 files changed, 119 insertions, 32 deletions

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 6c2a7e2..5409b8f 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -72,3 +72,5 @@ Order doesn't matter (not that much, at least ;)
 * Stéphane Wirtel: nonlocal-without-binding
 
 * Dmitry Pribysh: multiple-imports, not-iterable, not-a-mapping, various patches.
+
+* Laura Medioni (Logilab): misplaced-comparison-constant
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index 1b13b78..234737e 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1454,11 +1454,21 @@ class LambdaForComprehensionChecker(_BasicChecker):
             self.add_message('deprecated-lambda', node=node)
 
 class ComparisonChecker(_BasicChecker):
-    """checks for 'expr == True', 'expr == False' and 'expr == None'"""
+    """checks for singleton comparison and for yoda condition
+
+    - singleton comparison: 'expr == True', 'expr == False' and 'expr == None'
+    - yoda condition: 'const "comp" right' where comp can be '==', '!=', '<',
+      '<=', '>' or '>=', and right can be a variable, an attribute, a method or
+      a function
+    """
     msgs = {'C0121': ('Comparison to %s should be %s',
                       'singleton-comparison',
                       'Used when an expression is compared to singleton '
                       'values like True, False or None.'),
+            'W0151': ('Comparison should be %s',
+                      'misplaced-comparison-constant',
+                      'Used when the constant is placed on the left side'
+                      'of a comparison'),
            }
 
     def check_singleton_comparison(self, singleton, root_node):
@@ -1477,7 +1487,20 @@ class ComparisonChecker(_BasicChecker):
                              node=root_node,
                              args=(None, "'expr is None'"))
 
-    @check_messages('singleton-comparison')
+    def _check_misplaced_constant(self, node, left, right, operator):
+        suggestion = None
+        if isinstance(right, (astroid.Name, astroid.Attribute, astroid.Call)):
+            reverse_op = {'<': '>', '<=': '>=', '>': '<', '>=': '<='}
+            if operator in reverse_op:
+                operator = reverse_op[operator]
+            suggestion = '%s %s %s' % (right.as_string(), operator, left.value)
+        elif isinstance(right, astroid.Const):
+            suggestion = 'between a variable and a constant'
+        if suggestion:
+            self.add_message('misplaced-comparison-constant', node=node,
+                             args=(suggestion,))
+
+    @check_messages('singleton-comparison', 'misplaced-comparison-constant')
     def visit_compare(self, node):
         # NOTE: this checker only works with binary comparisons like 'x == 42'
         # but not 'x == y == 42'
@@ -1488,9 +1511,13 @@ class ComparisonChecker(_BasicChecker):
         operator, right = node.ops[0]
         if operator == '==':
             if isinstance(left, astroid.Const):
+                self._check_misplaced_constant(node, left, right, operator)
                 self.check_singleton_comparison(left, node)
             elif isinstance(right, astroid.Const):
                 self.check_singleton_comparison(right, node)
+        elif (operator in ('<', '<=', '>', '>=', '!=')
+              and isinstance(left, astroid.Const)):
+            self._check_misplaced_constant(node, left, right, operator)
 
 
 def register(linter):
diff --git a/pylint/test/functional/assert_on_tuple.py b/pylint/test/functional/assert_on_tuple.py
index 357e181..a612aa4 100644
--- a/pylint/test/functional/assert_on_tuple.py
+++ b/pylint/test/functional/assert_on_tuple.py
@@ -1,5 +1,6 @@
 '''Assert check example'''
 
+# pylint: disable=misplaced-comparison-constant
 assert (1 == 1, 2 == 2), "no error"
 assert (1 == 1, 2 == 2) # [assert-on-tuple]
 assert 1 == 1, "no error"
diff --git a/pylint/test/functional/assert_on_tuple.txt b/pylint/test/functional/assert_on_tuple.txt
index b1de73c..52d1ca5 100644
--- a/pylint/test/functional/assert_on_tuple.txt
+++ b/pylint/test/functional/assert_on_tuple.txt
@@ -1,2 +1,2 @@
-assert-on-tuple:4::Assert called on a 2-uple. Did you mean 'assert x,y'?
-assert-on-tuple:10::Assert called on a 2-uple. Did you mean 'assert x,y'?
+assert-on-tuple:5::Assert called on a 2-uple. Did you mean 'assert x,y'?
+assert-on-tuple:11::Assert called on a 2-uple. Did you mean 'assert x,y'?
diff --git a/pylint/test/functional/misplaced_bare_raise.py b/pylint/test/functional/misplaced_bare_raise.py
index ad60214..30b49bd 100644
--- a/pylint/test/functional/misplaced_bare_raise.py
+++ b/pylint/test/functional/misplaced_bare_raise.py
@@ -11,6 +11,7 @@ try:
 except Exception:

     raise

 

+# pylint: disable=misplaced-comparison-constant

 try:

     pass

 except Exception:

diff --git a/pylint/test/functional/misplaced_bare_raise.txt b/pylint/test/functional/misplaced_bare_raise.txt
index c60f3ca..fb39640 100644
--- a/pylint/test/functional/misplaced_bare_raise.txt
+++ b/pylint/test/functional/misplaced_bare_raise.txt
@@ -1,7 +1,7 @@
 misplaced-bare-raise:5::The raise statement is not inside an except clause

-misplaced-bare-raise:35:test1.best:The raise statement is not inside an except clause

-misplaced-bare-raise:38:test1:The raise statement is not inside an except clause

-misplaced-bare-raise:39::The raise statement is not inside an except clause

-misplaced-bare-raise:48::The raise statement is not inside an except clause

-misplaced-bare-raise:56:A:The raise statement is not inside an except clause

-misplaced-bare-raise:67::The raise statement is not inside an except clause
\ No newline at end of file
+misplaced-bare-raise:36:test1.best:The raise statement is not inside an except clause

+misplaced-bare-raise:39:test1:The raise statement is not inside an except clause

+misplaced-bare-raise:40::The raise statement is not inside an except clause

+misplaced-bare-raise:49::The raise statement is not inside an except clause

+misplaced-bare-raise:57:A:The raise statement is not inside an except clause

+misplaced-bare-raise:68::The raise statement is not inside an except clause

diff --git a/pylint/test/functional/misplaced_comparison_constant.py b/pylint/test/functional/misplaced_comparison_constant.py
new file mode 100644
index 0000000..df3f40e
--- /dev/null
+++ b/pylint/test/functional/misplaced_comparison_constant.py
@@ -0,0 +1,38 @@
+"""Check that the constants are on the right side of the comparisons"""
+
+# pylint: disable=singleton-comparison, missing-docstring, too-few-public-methods
+
+class MyClass(object):
+    def __init__(self):
+        self.attr = 1
+
+    def dummy_return(self):
+        return self.attr
+
+def dummy_return():
+    return 2
+
+def bad_comparisons():
+    """this is not ok"""
+    instance = MyClass()
+    for i in range(10):
+        if 5 <= i:  # [misplaced-comparison-constant]
+            print "foo"
+        if True == True:  # [misplaced-comparison-constant]
+            pass
+        if 'bar' != 'foo':  # [misplaced-comparison-constant]
+            pass
+        if 1 == i:  # [misplaced-comparison-constant]
+            print "bar"
+        if 3 < dummy_return():  # [misplaced-comparison-constant]
+            pass
+        if 4 != instance.dummy_return():  # [misplaced-comparison-constant]
+            pass
+        if 1 == instance.attr:  # [misplaced-comparison-constant]
+            pass
+
+def good_comparison():
+    """this is ok"""
+    for i in range(10):
+        if i == 5:
+            print "foo"
diff --git a/pylint/test/functional/misplaced_comparison_constant.txt b/pylint/test/functional/misplaced_comparison_constant.txt
new file mode 100644
index 0000000..2e63dce
--- /dev/null
+++ b/pylint/test/functional/misplaced_comparison_constant.txt
@@ -0,0 +1,7 @@
+misplaced-comparison-constant:19:bad_comparisons:Comparison should be i >= 5
+misplaced-comparison-constant:21:bad_comparisons:Comparison should be between a variable and a constant
+misplaced-comparison-constant:23:bad_comparisons:Comparison should be between a variable and a constant
+misplaced-comparison-constant:25:bad_comparisons:Comparison should be i == 1
+misplaced-comparison-constant:27:bad_comparisons:Comparison should be dummy_return() > 3
+misplaced-comparison-constant:29:bad_comparisons:Comparison should be instance.dummy_return() != 4
+misplaced-comparison-constant:31:bad_comparisons:Comparison should be instance.attr == 1
diff --git a/pylint/test/functional/singleton_comparison.py b/pylint/test/functional/singleton_comparison.py
index 73067c8..59738f8 100644
--- a/pylint/test/functional/singleton_comparison.py
+++ b/pylint/test/functional/singleton_comparison.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, invalid-name
+# pylint: disable=missing-docstring, invalid-name, misplaced-comparison-constant
 x = 42
 a = x is None
 b = x == None  # [singleton-comparison]
diff --git a/pylint/test/functional/superfluous_parens.py b/pylint/test/functional/superfluous_parens.py
index 8bb3dd6..417163e 100644
--- a/pylint/test/functional/superfluous_parens.py
+++ b/pylint/test/functional/superfluous_parens.py
@@ -1,9 +1,10 @@
 """Test the superfluous-parens warning."""
 from __future__ import print_function
 
-if (3 == 5):  # [superfluous-parens]
+i = 3
+if (i == 5):  # [superfluous-parens]
     pass
-if not (3 == 5):  # [superfluous-parens]
+if not (i == 5):  # [superfluous-parens]
     pass
 if not (3 or 5):
     pass
diff --git a/pylint/test/functional/superfluous_parens.txt b/pylint/test/functional/superfluous_parens.txt
index 9b203ee..57e1019 100644
--- a/pylint/test/functional/superfluous_parens.txt
+++ b/pylint/test/functional/superfluous_parens.txt
@@ -1,5 +1,5 @@
-superfluous-parens:4::Unnecessary parens after 'if' keyword
-superfluous-parens:6::Unnecessary parens after 'not' keyword
-superfluous-parens:10::Unnecessary parens after 'for' keyword
-superfluous-parens:12::Unnecessary parens after 'if' keyword
-superfluous-parens:17::Unnecessary parens after 'del' keyword
+superfluous-parens:5::Unnecessary parens after 'if' keyword
+superfluous-parens:7::Unnecessary parens after 'not' keyword
+superfluous-parens:11::Unnecessary parens after 'for' keyword
+superfluous-parens:13::Unnecessary parens after 'if' keyword
+superfluous-parens:18::Unnecessary parens after 'del' keyword
diff --git a/pylint/test/functional/using_constant_test.py b/pylint/test/functional/using_constant_test.py
index 3e7b4ad..fcaae72 100644
--- a/pylint/test/functional/using_constant_test.py
+++ b/pylint/test/functional/using_constant_test.py
@@ -108,6 +108,7 @@ if not 3:
 if instance.method():

     pass

 

+# pylint: disable=misplaced-comparison-constant

 if 2 < 3:

     pass

 

diff --git a/pylint/test/input/func_excess_escapes.py b/pylint/test/input/func_excess_escapes.py
index 178ace8..7b69832 100644
--- a/pylint/test/input/func_excess_escapes.py
+++ b/pylint/test/input/func_excess_escapes.py
@@ -1,4 +1,4 @@
-# pylint:disable=W0105, W0511
+# pylint:disable=W0105, W0511, W0151
 """Stray backslash escapes may be missing a raw-string prefix."""
 
 __revision__ = '$Id$'
diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py
index 2094314..9901765 100644
--- a/pylint/test/unittest_checker_base.py
+++ b/pylint/test/unittest_checker_base.py
@@ -248,7 +248,7 @@ class MultiNamingStyleTest(CheckerTestCase):
 class ComparisonTest(CheckerTestCase):
     CHECKER_CLASS = base.ComparisonChecker
 
-    def test_singleton_comparison(self):
+    def test_comparison(self):
         node = test_utils.extract_node("foo == True")
         message = Message('singleton-comparison',
                           node=node,
@@ -271,24 +271,33 @@ class ComparisonTest(CheckerTestCase):
             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):
+        messages = (Message('misplaced-comparison-constant',
+                            node=node,
+                            args=('foo == True',)),
+                    Message('singleton-comparison',
+                            node=node,
+                            args=(True, "just 'expr' or 'expr is True'")))
+        with self.assertAddsMessages(*messages):
             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):
+        messages = (Message('misplaced-comparison-constant',
+                            node=node,
+                            args=('foo == False',)),
+                    Message('singleton-comparison',
+                            node=node,
+                            args=(False, "'not expr' or 'expr is False'")))
+        with self.assertAddsMessages(*messages):
             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):
+        messages = (Message('misplaced-comparison-constant',
+                            node=node,
+                            args=('foo == None',)),
+                    Message('singleton-comparison',
+                            node=node,
+                            args=(None, "'expr is None'")))
+        with self.assertAddsMessages(*messages):
             self.checker.visit_compare(node)