Explicitely check for const to const comparison

Also add a check for comparison to a method, a function or an attribute and improve the functional test accordingly.

3 files changed, 40 insertions, 16 deletions

diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index 2e5613a..ac86d44 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1446,8 +1446,9 @@ class ComparisonChecker(_BasicChecker):
     """checks for singleton comparison and for yoda condition
 
     - singleton comparison: 'expr == True', 'expr == False' and 'expr == None'
-    - yoda condition: 'const "comp" var' where comp can be '==', '!=', '<',
-      '<=', '>' or '>='
+    - 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',
@@ -1475,16 +1476,18 @@ class ComparisonChecker(_BasicChecker):
                              node=root_node,
                              args=(None, "'expr is None'"))
 
-    def _warn_misplaced_constant(self, node, left, right, operator):
-        if isinstance(right, astroid.Name):
+    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.name, operator, left.value)
-        else:
+            suggestion = '%s %s %s' % (right.as_string(), operator, left.value)
+        elif isinstance(right, astroid.Const):
             suggestion = 'between a variable and a constant'
-        self.add_message('misplaced-comparison-constant', node=node,
-                         args=(suggestion,))
+        if suggestion:
+            self.add_message('misplaced-comparison-constant', node=node,
+                             args=(suggestion,))
 
     @check_messages('singleton-comparison', 'misplaced-comparison-constant')
     def visit_compare(self, node):
@@ -1497,13 +1500,13 @@ class ComparisonChecker(_BasicChecker):
         operator, right = node.ops[0]
         if operator == '==':
             if isinstance(left, astroid.Const):
-                self._warn_misplaced_constant(node, left, right, operator)
+                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._warn_misplaced_constant(node, left, right, operator)
+            self._check_misplaced_constant(node, left, right, operator)
 
 
 def register(linter):
diff --git a/pylint/test/functional/misplaced_comparison_constant.py b/pylint/test/functional/misplaced_comparison_constant.py
index fb6e40d..df3f40e 100644
--- a/pylint/test/functional/misplaced_comparison_constant.py
+++ b/pylint/test/functional/misplaced_comparison_constant.py
@@ -1,8 +1,20 @@
 """Check that the constants are on the right side of the comparisons"""
 
-# pylint: disable=singleton-comparison
+# 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: 5 should be on the right"""
+    """this is not ok"""
+    instance = MyClass()
     for i in range(10):
         if 5 <= i:  # [misplaced-comparison-constant]
             print "foo"
@@ -12,6 +24,12 @@ def bad_comparisons():
             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"""
diff --git a/pylint/test/functional/misplaced_comparison_constant.txt b/pylint/test/functional/misplaced_comparison_constant.txt
index 99bc174..2e63dce 100644
--- a/pylint/test/functional/misplaced_comparison_constant.txt
+++ b/pylint/test/functional/misplaced_comparison_constant.txt
@@ -1,4 +1,7 @@
-misplaced-comparison-constant:7:bad_comparisons:Comparison should be i >= 5
-misplaced-comparison-constant:9:bad_comparisons:Comparison should be between a variable and a constant
-misplaced-comparison-constant:11:bad_comparisons:Comparison should be between a variable and a constant
-misplaced-comparison-constant:13:bad_comparisons:Comparison should be i == 1
+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