diff options
author | Laura M?dioni <laura.medioni@logilab.fr> | 2015-10-15 17:02:52 +0200 |
---|---|---|
committer | Laura M?dioni <laura.medioni@logilab.fr> | 2015-10-15 17:02:52 +0200 |
commit | d2fa848c02f0364466b46a511c89c33b6884ce29 (patch) | |
tree | 51e2c5d88ba858feb710f4c8c02390ff0b769f6e | |
parent | 4e82b39a1e0e2329de4df11cd79740a5455b7b5d (diff) | |
download | pylint-d2fa848c02f0364466b46a511c89c33b6884ce29.tar.gz |
add a new rule looking for yoda conditions
-rw-r--r-- | CONTRIBUTORS.txt | 2 | ||||
-rw-r--r-- | pylint/checkers/base.py | 28 | ||||
-rw-r--r-- | pylint/test/functional/assert_on_tuple.py | 1 | ||||
-rw-r--r-- | pylint/test/functional/assert_on_tuple.txt | 4 | ||||
-rw-r--r-- | pylint/test/functional/misplaced_bare_raise.py | 1 | ||||
-rw-r--r-- | pylint/test/functional/misplaced_bare_raise.txt | 12 | ||||
-rw-r--r-- | pylint/test/functional/misplaced_comparison_constant.py | 20 | ||||
-rw-r--r-- | pylint/test/functional/misplaced_comparison_constant.txt | 4 | ||||
-rw-r--r-- | pylint/test/functional/singleton_comparison.py | 2 | ||||
-rw-r--r-- | pylint/test/functional/superfluous_parens.py | 5 | ||||
-rw-r--r-- | pylint/test/functional/superfluous_parens.txt | 10 | ||||
-rw-r--r-- | pylint/test/functional/using_constant_test.py | 1 | ||||
-rw-r--r-- | pylint/test/input/func_excess_escapes.py | 2 | ||||
-rw-r--r-- | pylint/test/unittest_checker_base.py | 35 |
14 files changed, 95 insertions, 32 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index ee64a35..5b21f01 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. + +* Laura Medioni (Logilab): misplaced-comparison-constant diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py index 927e77b..2e5613a 100644 --- a/pylint/checkers/base.py +++ b/pylint/checkers/base.py @@ -1443,11 +1443,20 @@ 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" var' where comp can be '==', '!=', '<', + '<=', '>' or '>=' + """ 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): @@ -1466,7 +1475,18 @@ class ComparisonChecker(_BasicChecker): node=root_node, args=(None, "'expr is None'")) - @check_messages('singleton-comparison') + def _warn_misplaced_constant(self, node, left, right, operator): + if isinstance(right, astroid.Name): + reverse_op = {'<': '>', '<=': '>=', '>': '<', '>=': '<='} + if operator in reverse_op: + operator = reverse_op[operator] + suggestion = '%s %s %s' % (right.name, operator, left.value) + else: + suggestion = 'between a variable and a constant' + 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' @@ -1477,9 +1497,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_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) 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..fb6e40d --- /dev/null +++ b/pylint/test/functional/misplaced_comparison_constant.py @@ -0,0 +1,20 @@ +"""Check that the constants are on the right side of the comparisons""" + +# pylint: disable=singleton-comparison +def bad_comparisons(): + """this is not ok: 5 should be on the right""" + 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" + +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..99bc174 --- /dev/null +++ b/pylint/test/functional/misplaced_comparison_constant.txt @@ -0,0 +1,4 @@ +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 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 c68f379..ec3ae2b 100644 --- a/pylint/test/unittest_checker_base.py +++ b/pylint/test/unittest_checker_base.py @@ -249,7 +249,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, @@ -272,24 +272,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) |