diff options
author | Jon Dufresne <jon.dufresne@gmail.com> | 2019-01-13 16:41:31 -0800 |
---|---|---|
committer | Ian Stapleton Cordasco <graffatcolmingov@gmail.com> | 2019-01-19 07:23:05 -0600 |
commit | 7272b1048ee319cc058bf83d4b78563573583ce9 (patch) | |
tree | e69b90e1a4d60c5fa2b166e249c42e346b6ec408 | |
parent | 1f58890b3ea7a8683bc21594a7dc773443102893 (diff) | |
download | pyflakes-7272b1048ee319cc058bf83d4b78563573583ce9.tar.gz |
Add check for use of 'is' operator with str, bytes, and int literals
While it often appears to "work", it is considered bad practice to
compare against string, bytes, and int literals using the `is` operator.
This is because it relies on a CPython implementation detail that may
not hold true for other interpreters. For example:
# Bad
if x is 'foo':
...
# Good
if x == 'foo':
...
This can easily be caught during static analysis to promote best
practices. This is discussed in the Python bug:
https://bugs.python.org/issue34850
-rw-r--r-- | pyflakes/checker.py | 16 | ||||
-rw-r--r-- | pyflakes/messages.py | 4 | ||||
-rw-r--r-- | pyflakes/test/test_is_literal.py | 200 |
3 files changed, 219 insertions, 1 deletions
diff --git a/pyflakes/checker.py b/pyflakes/checker.py index e94bc71..7b68b21 100644 --- a/pyflakes/checker.py +++ b/pyflakes/checker.py @@ -1206,7 +1206,7 @@ class Checker(object): # "expr" type nodes BOOLOP = BINOP = UNARYOP = IFEXP = SET = \ - COMPARE = CALL = REPR = ATTRIBUTE = SUBSCRIPT = \ + CALL = REPR = ATTRIBUTE = SUBSCRIPT = \ STARRED = NAMECONSTANT = handleChildren NUM = STR = BYTES = ELLIPSIS = CONSTANT = ignore @@ -1665,3 +1665,17 @@ class Checker(object): if node.value: # If the assignment has value, handle the *value* now. self.handleNode(node.value, node) + + def COMPARE(self, node): + literals = (ast.Str, ast.Num) + if not PY2: + literals += (ast.Bytes,) + + left = node.left + for op, right in zip(node.ops, node.comparators): + if (isinstance(op, (ast.Is, ast.IsNot)) and + (isinstance(left, literals) or isinstance(right, literals))): + self.report(messages.IsLiteral, node) + left = right + + self.handleChildren(node) diff --git a/pyflakes/messages.py b/pyflakes/messages.py index 4d3c7d7..f73ba46 100644 --- a/pyflakes/messages.py +++ b/pyflakes/messages.py @@ -262,3 +262,7 @@ class RaiseNotImplemented(Message): class InvalidPrintSyntax(Message): message = 'use of >> is invalid with print function' + + +class IsLiteral(Message): + message = 'use ==/!= to compare str, bytes, and int literals' diff --git a/pyflakes/test/test_is_literal.py b/pyflakes/test/test_is_literal.py new file mode 100644 index 0000000..9280cda --- /dev/null +++ b/pyflakes/test/test_is_literal.py @@ -0,0 +1,200 @@ +from pyflakes.messages import IsLiteral +from pyflakes.test.harness import TestCase + + +class Test(TestCase): + def test_is_str(self): + self.flakes(""" + x = 'foo' + if x is 'foo': + pass + """, IsLiteral) + + def test_is_bytes(self): + self.flakes(""" + x = b'foo' + if x is b'foo': + pass + """, IsLiteral) + + def test_is_unicode(self): + self.flakes(""" + x = u'foo' + if x is u'foo': + pass + """, IsLiteral) + + def test_is_int(self): + self.flakes(""" + x = 10 + if x is 10: + pass + """, IsLiteral) + + def test_is_true(self): + self.flakes(""" + x = True + if x is True: + pass + """) + + def test_is_false(self): + self.flakes(""" + x = False + if x is False: + pass + """) + + def test_is_not_str(self): + self.flakes(""" + x = 'foo' + if x is not 'foo': + pass + """, IsLiteral) + + def test_is_not_bytes(self): + self.flakes(""" + x = b'foo' + if x is not b'foo': + pass + """, IsLiteral) + + def test_is_not_unicode(self): + self.flakes(""" + x = u'foo' + if x is not u'foo': + pass + """, IsLiteral) + + def test_is_not_int(self): + self.flakes(""" + x = 10 + if x is not 10: + pass + """, IsLiteral) + + def test_is_not_true(self): + self.flakes(""" + x = True + if x is not True: + pass + """) + + def test_is_not_false(self): + self.flakes(""" + x = False + if x is not False: + pass + """) + + def test_left_is_str(self): + self.flakes(""" + x = 'foo' + if 'foo' is x: + pass + """, IsLiteral) + + def test_left_is_bytes(self): + self.flakes(""" + x = b'foo' + if b'foo' is x: + pass + """, IsLiteral) + + def test_left_is_unicode(self): + self.flakes(""" + x = u'foo' + if u'foo' is x: + pass + """, IsLiteral) + + def test_left_is_int(self): + self.flakes(""" + x = 10 + if 10 is x: + pass + """, IsLiteral) + + def test_left_is_true(self): + self.flakes(""" + x = True + if True is x: + pass + """) + + def test_left_is_false(self): + self.flakes(""" + x = False + if False is x: + pass + """) + + def test_left_is_not_str(self): + self.flakes(""" + x = 'foo' + if 'foo' is not x: + pass + """, IsLiteral) + + def test_left_is_not_bytes(self): + self.flakes(""" + x = b'foo' + if b'foo' is not x: + pass + """, IsLiteral) + + def test_left_is_not_unicode(self): + self.flakes(""" + x = u'foo' + if u'foo' is not x: + pass + """, IsLiteral) + + def test_left_is_not_int(self): + self.flakes(""" + x = 10 + if 10 is not x: + pass + """, IsLiteral) + + def test_left_is_not_true(self): + self.flakes(""" + x = True + if True is not x: + pass + """) + + def test_left_is_not_false(self): + self.flakes(""" + x = False + if False is not x: + pass + """) + + def test_chained_operators_is_true(self): + self.flakes(""" + x = 5 + if x is True < 4: + pass + """) + + def test_chained_operators_is_str(self): + self.flakes(""" + x = 5 + if x is 'foo' < 4: + pass + """, IsLiteral) + + def test_chained_operators_is_true_end(self): + self.flakes(""" + x = 5 + if 4 < x is True: + pass + """) + + def test_chained_operators_is_str_end(self): + self.flakes(""" + x = 5 + if 4 < x is 'foo': + pass + """, IsLiteral) |