summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJon Dufresne <jon.dufresne@gmail.com>2019-01-13 16:41:31 -0800
committerIan Stapleton Cordasco <graffatcolmingov@gmail.com>2019-01-19 07:23:05 -0600
commit7272b1048ee319cc058bf83d4b78563573583ce9 (patch)
treee69b90e1a4d60c5fa2b166e249c42e346b6ec408
parent1f58890b3ea7a8683bc21594a7dc773443102893 (diff)
downloadpyflakes-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.py16
-rw-r--r--pyflakes/messages.py4
-rw-r--r--pyflakes/test/test_is_literal.py200
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)