From 0fb7d8dcadeea6505e1c1506d10fe5f86f44c95e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 8 Jun 2016 14:29:17 -0700 Subject: [PATCH] [16.6.0] Add B002, B301, B302, B303, B304, B305 --- README.rst | 51 +++++++++++++++++-- bugbear.py | 108 +++++++++++++++++++++++++++++++++++++++- setup.py | 2 +- tests/b002.py | 19 +++++++ tests/b301_b302_b305.py | 46 +++++++++++++++++ tests/b303_b304.py | 28 +++++++++++ tests/test_bugbear.py | 32 +++++++++++- 7 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 tests/b002.py create mode 100644 tests/b301_b302_b305.py create mode 100644 tests/b303_b304.py diff --git a/README.rst b/README.rst index 62a8c63..29f68a2 100644 --- a/README.rst +++ b/README.rst @@ -19,14 +19,50 @@ program. Contains warnings that don't belong in pyflakes and pep8:: List of warnings ---------------- -B001 -~~~~ - -Do not use bare ``except:``, it also catches unexpected events like -memory errors, interrupts, system exit, and so on. Prefer ``except +**B001**: Do not use bare ``except:``, it also catches unexpected events +like memory errors, interrupts, system exit, and so on. Prefer ``except Exception:``. If you're sure what you're doing, be explicit and write ``except BaseException:``. +**B002**: Python does not support the unary prefix increment. Writing +``++n`` is equivalent to ``+(+(n))``, which equals ``n``. You meant ``n ++= 1``. + +Python 3 compatibility warnings +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +These have higher risk of false positives but discover regressions that +are dangerous to slip through when test coverage is not great. Let me +know if a popular library is triggering any of the following warnings +for valid code. + +**B301**: Python 3 does not include ``.iter*`` methods on dictionaries. +The default behavior is to return iterables. Simply remove the ``iter`` +prefix from the method. For Python 2 compatibility, also prefer the +Python 3 equivalent if you expect that the size of the dict to be small +and bounded. The performance regression on Python 2 will be negligible +and the code is going to be the clearest. Alternatively, use +``six.iter*`` or ``future.utils.iter*``. + +**B302**: Python 3 does not include ``.view*`` methods on dictionaries. +The default behavior is to return viewables. Simply remove the ``view`` +prefix from the method. For Python 2 compatibility, also prefer the +Python 3 equivalent if you expect that the size of the dict to be small +and bounded. The performance regression on Python 2 will be negligible +and the code is going to be the clearest. Alternatively, use +``six.view*`` or ``future.utils.view*``. + +**B303**: The ``__metaclass__`` attribute on a class definition does +nothing on Python 3. Use ``class MyClass(BaseClass, metaclass=...)``. +For Python 2 compatibility, use ``six.add_metaclass``. + +**B304**: ``sys.maxint`` is not a thing on Python 3. Use +``sys.maxsize``. + +**B305**: ``.next()`` is not a thing on Python 3. Use the ``next()`` +builtin. For Python 2 compatibility, use ``six.next()``. + + Tests ----- @@ -63,6 +99,11 @@ MIT Change Log ---------- +16.6.0 +~~~~~~ + +* introduced B002, B301, B302, B303, B304, and B305 + 16.4.2 ~~~~~~ diff --git a/bugbear.py b/bugbear.py index 41f1162..2bab86b 100644 --- a/bugbear.py +++ b/bugbear.py @@ -6,7 +6,7 @@ import pep8 -__version__ = '16.4.2' +__version__ = '16.6.0' @attr.s @@ -56,9 +56,12 @@ class BugBearVisitor(ast.NodeVisitor): filename = attr.ib() lines = attr.ib() node_stack = attr.ib(default=attr.Factory(list)) + node_window = attr.ib(default=attr.Factory(list)) errors = attr.ib(default=attr.Factory(list)) futures = attr.ib(default=attr.Factory(set)) + NODE_WINDOW_SIZE = 4 + if False: # Useful for tracing what the hell is going on. @@ -68,6 +71,8 @@ def __getattr__(self, name): def visit(self, node): self.node_stack.append(node) + self.node_window.append(node) + self.node_window = self.node_window[-self.NODE_WINDOW_SIZE:] super().visit(node) self.node_stack.pop() @@ -78,8 +83,53 @@ def visit_ExceptHandler(self, node): ) self.generic_visit(node) + def visit_UAdd(self, node): + trailing_nodes = list(map(type, self.node_window[-4:])) + if trailing_nodes == [ast.UnaryOp, ast.UAdd, ast.UnaryOp, ast.UAdd]: + originator = self.node_window[-4] + self.errors.append( + B002(originator.lineno, originator.col_offset) + ) + self.generic_visit(node) + + def visit_Call(self, node): + if isinstance(node.func, ast.Attribute): + for bug in (B301, B302, B305): + if node.func.attr in bug.methods: + call_path = '.'.join(self.compose_call_path(node.func.value)) + if call_path not in bug.valid_paths: + self.errors.append( + bug(node.lineno, node.col_offset) + ) + break + self.generic_visit(node) + + def visit_Attribute(self, node): + path = '.'.join(self.compose_call_path(node)) + if path == 'sys.maxint': + self.errors.append( + B304(node.lineno, node.col_offset) + ) + + def visit_Assign(self, node): + if isinstance(self.node_stack[-2], ast.ClassDef): + assign_targets = {t.id for t in node.targets} + if '__metaclass__' in assign_targets: + self.errors.append( + B303(node.lineno, node.col_offset) + ) + self.generic_visit(node) + + def compose_call_path(self, node): + if isinstance(node, ast.Attribute): + yield from self.compose_call_path(node.value) + yield node.attr + elif isinstance(node, ast.Name): + yield node.id + error = namedtuple('error', 'lineno col message type') + B001 = partial( error, message="B001: Do not use bare `except:`, it also catches unexpected " @@ -88,3 +138,59 @@ def visit_ExceptHandler(self, node): "be explicit and write `except BaseException:`.", type=BugBearChecker, ) + +B002 = partial( + error, + message="B002: Python does not support the unary prefix increment. Writing " + "++n is equivalent to +(+(n)), which equals n. You meant n += 1.", + type=BugBearChecker, +) + +# Those could be false positives but it's more dangerous to let them slip +# through if they're not. +B301 = partial( + error, + message="B301: Python 3 does not include .iter* methods on dictionaries. " + "Remove the ``iter`` prefix from the method name. For Python 2 " + "compatibility, prefer the Python 3 equivalent unless you expect " + "the size of the container to be large or unbounded. Then use " + "`six.iter*` or `future.utils.iter*`.", + type=BugBearChecker, +) +B301.methods = {'iterkeys', 'itervalues', 'iteritems', 'iterlists'} +B301.valid_paths = {'six', 'future.utils', 'builtins'} + +B302 = partial( + error, + message="B302: Python 3 does not include .view* methods on dictionaries. " + "Remove the ``view`` prefix from the method name. For Python 2 " + "compatibility, prefer the Python 3 equivalent unless you expect " + "the size of the container to be large or unbounded. Then use " + "`six.view*` or `future.utils.view*`.", + type=BugBearChecker, +) +B302.methods = {'viewkeys', 'viewvalues', 'viewitems', 'viewlists'} +B302.valid_paths = {'six', 'future.utils', 'builtins'} + +B303 = partial( + error, + message="B303: __metaclass__ does nothing on Python 3. Use " + "`class MyClass(BaseClass, metaclass=...)`. For Python 2 " + "compatibility, use `six.add_metaclass`.", + type=BugBearChecker, +) + +B304 = partial( + error, + message="B304: sys.maxint is not a thing on Python 3. Use `sys.maxsize`.", + type=BugBearChecker, +) + +B305 = partial( + error, + message="B305: .next() is not a thing on Python 3. Use the `next()` " + "builtin. For Python 2 compatibility, use ``six.next()``.", + type=BugBearChecker, +) +B305.methods = {'next'} +B305.valid_paths = {'six', 'future.utils', 'builtins'} diff --git a/setup.py b/setup.py index 9270f32..9645473 100644 --- a/setup.py +++ b/setup.py @@ -48,7 +48,7 @@ ], entry_points={ 'flake8.extension': [ - 'B00 = bugbear:BugBearChecker', + 'B = bugbear:BugBearChecker', ], }, ) diff --git a/tests/b002.py b/tests/b002.py new file mode 100644 index 0000000..1d84fe8 --- /dev/null +++ b/tests/b002.py @@ -0,0 +1,19 @@ +""" +Should emit: +B002 - on lines 13 and 17 +""" + +def this_is_all_fine(n): + x = n + 1 + y = 1 + n + z = + x + y + return +z + +def this_is_buggy(n): + x = ++n + return x + +def this_is_buggy_too(n): + return (+ + +n + ) diff --git a/tests/b301_b302_b305.py b/tests/b301_b302_b305.py new file mode 100644 index 0000000..a7b51d0 --- /dev/null +++ b/tests/b301_b302_b305.py @@ -0,0 +1,46 @@ +""" +Should emit: +B301 - on lines 33-36 +B302 - on lines 37-40 +B305 - on lines 37-40 +""" + +import builtins # future's builtins really +import future.utils +import six +from six import iterkeys +from future.utils import itervalues + +def this_is_okay(): + d = {} + iterkeys(d) + six.iterkeys(d) + six.itervalues(d) + six.iteritems(d) + six.iterlists(d) + six.viewkeys(d) + six.viewvalues(d) + six.viewlists(d) + itervalues(d) + future.utils.iterkeys(d) + future.utils.itervalues(d) + future.utils.iteritems(d) + future.utils.iterlists(d) + future.utils.viewkeys(d) + future.utils.viewvalues(d) + future.utils.viewlists(d) + six.next(d) + builtins.next(d) + +def everything_else_is_wrong(): + d = None # note: bugbear is no type checker + d.iterkeys() + d.itervalues() + d.iteritems() + d.iterlists() # Djangoism + d.viewkeys() + d.viewvalues() + d.viewitems() + d.viewlists() # Djangoism + d.next() + d.keys().next() diff --git a/tests/b303_b304.py b/tests/b303_b304.py new file mode 100644 index 0000000..4470bd0 --- /dev/null +++ b/tests/b303_b304.py @@ -0,0 +1,28 @@ +""" +Should emit: +B303 - on line 21 +B304 - on line 28 +""" + +import sys +import something_else + +def this_is_okay(): + something_else.maxint + maxint = 3 + maxint + +maxint = 3 + +def this_is_also_okay(): + maxint + +class CustomClassWithBrokenMetaclass: + __metaclass__ = type + maxint = 5 # this is okay + + def this_is_also_fine(self): + self.maxint + +def this_is_wrong(): + sys.maxint diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 1c93ee4..729b0e5 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -1,7 +1,8 @@ from pathlib import Path import unittest -from bugbear import BugBearChecker, B001 +from bugbear import BugBearChecker +from bugbear import B001, B002, B301, B302, B303, B304, B305 class BugbearTestCase(unittest.TestCase): @@ -16,6 +17,35 @@ def test_b001(self): [B001(8, 0), B001(40, 4)], ) + def test_b002(self): + filename = Path(__file__).absolute().parent / 'b002.py' + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + self.assertEqual( + errors, + [B002(13, 8), B002(17, 12)], + ) + + def test_b301_b302_b305(self): + filename = Path(__file__).absolute().parent / 'b301_b302_b305.py' + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + self.assertEqual( + errors, + [B301(37, 4), B301(38, 4), B301(39, 4), B301(40, 4)] + + [B302(41, 4), B302(42, 4), B302(43, 4), B302(44, 4)] + + [B305(45, 4), B305(46, 4)] + ) + + def test_b303_b304(self): + filename = Path(__file__).absolute().parent / 'b303_b304.py' + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + self.assertEqual( + errors, + [B303(21, 4), B304(28, 4)], + ) + if __name__ == '__main__': unittest.main()