diff --git a/.flake8 b/.flake8 index dae0ba2..066df22 100644 --- a/.flake8 +++ b/.flake8 @@ -2,7 +2,7 @@ # Keep in sync with setup.cfg which is used for source packages. [flake8] -ignore = E203, E302, E501, E999 +ignore = E203, E302, E501, E999, W503 max-line-length = 88 max-complexity = 12 select = B,C,E,F,W,B9 diff --git a/README.rst b/README.rst index e8934f3..7f81cc5 100644 --- a/README.rst +++ b/README.rst @@ -123,9 +123,9 @@ waste CPU instructions. Either prepend ``assert`` or remove it. **B016**: Cannot raise a literal. Did you intend to return it or raise an Exception? -**B017**: ``self.assertRaises(Exception):`` should be considered evil. It can lead -to your test passing even if the code being tested is never executed due to a typo. -Either assert for a more specific exception (builtin or custom), use +**B017**: ``self.assertRaises(Exception):`` should be considered evil. It can lead +to your test passing even if the code being tested is never executed due to a typo. +Either assert for a more specific exception (builtin or custom), use ``assertRaisesRegex``, or use the context manager form of assertRaises (``with self.assertRaises(Exception) as ex:``) with an assertion against the data available in ``ex``. @@ -157,6 +157,11 @@ nothing else. If the attributes should be mutable, define the attributes in ``__slots__`` to save per-instance memory and to prevent accidentally creating additional attributes on instances. +**B904**: Within an ``except`` clause, raise exceptions with ``raise ... from err`` +or ``raise ... from None`` to distinguish them from errors in exception handling. +See [the exception chaining tutorial](https://docs.python.org/3/tutorial/errors.html#exception-chaining) +for details. + **B950**: Line too long. This is a pragmatic equivalent of ``pycodestyle``'s E501: it considers "max-line-length" but only triggers when the value has been exceeded by **more than 10%**. You will no @@ -217,10 +222,15 @@ MIT Change Log ---------- +Future +~~~~~~ + +* Add B904: check for ``raise`` without ``from`` in an ``except`` clause + 21.4.3 ~~~~~~ -* Verify the element in item_context.args is of type ast.Name for b017 +* Verify the element in item_context.args is of type ast.Name for b017 21.4.2 ~~~~~~ @@ -231,7 +241,7 @@ Change Log ~~~~~~ * Add B017: check for gotta-catch-em-all assertRaises(Exception) - + 21.3.2 ~~~~~~ diff --git a/bugbear.py b/bugbear.py index 68b09db..44cf9d4 100644 --- a/bugbear.py +++ b/bugbear.py @@ -292,6 +292,7 @@ def visit_Compare(self, node): def visit_Raise(self, node): self.check_for_b016(node) + self.check_for_b904(node) self.generic_visit(node) def visit_With(self, node): @@ -426,6 +427,21 @@ def check_for_b017(self, node): ): self.errors.append(B017(node.lineno, node.col_offset)) + def check_for_b904(self, node): + """Checks `raise` without `from` inside an `except` clause. + + In these cases, you should use explicit exception chaining from the + earlier error, or suppress it with `raise ... from None`. See + https://docs.python.org/3/tutorial/errors.html#exception-chaining + """ + if ( + node.cause is None + and node.exc is not None + and not (isinstance(node.exc, ast.Name) and node.exc.id.islower()) + and any(isinstance(n, ast.ExceptHandler) for n in self.node_stack) + ): + self.errors.append(B904(node.lineno, node.col_offset)) + def walk_function_body(self, node): def _loop(parent, node): if isinstance(node, (ast.AsyncFunctionDef, ast.FunctionDef)): @@ -775,6 +791,14 @@ def visit(self, node): ) ) +B904 = Error( + message=( + "B904 Within an `except` clause, raise exceptions with `raise ... from err` " + "or `raise ... from None` to distinguish them from errors in exception handling. " + "See https://docs.python.org/3/tutorial/errors.html#exception-chaining for details." + ) +) + B950 = Error(message="B950 line too long ({} > {} characters)") -disabled_by_default = ["B901", "B902", "B903", "B950"] +disabled_by_default = ["B901", "B902", "B903", "B904", "B950"] diff --git a/tests/b904.py b/tests/b904.py new file mode 100644 index 0000000..104fda8 --- /dev/null +++ b/tests/b904.py @@ -0,0 +1,21 @@ +""" +Should emit: +B904 - on lines 10, 11 and 16 +""" + +try: + raise ValueError +except ValueError: + if "abc": + raise TypeError + raise UserWarning +except AssertionError: + raise # Bare `raise` should not be an error +except Exception as err: + assert err + raise Exception("No cause here...") +except BaseException as base_err: + # Might use this instead of bare raise with the `.with_traceback()` method + raise base_err +finally: + raise Exception("Nothing to chain from, so no warning here") diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 0807f19..3f91edc 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -28,6 +28,7 @@ B015, B016, B017, + B904, B901, B902, B903, @@ -260,6 +261,17 @@ def test_b903(self): errors = list(bbc.run()) self.assertEqual(errors, self.errors(B903(32, 0), B903(38, 0))) + def test_b904(self): + filename = Path(__file__).absolute().parent / "b904.py" + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + expected = [ + B904(10, 8), + B904(11, 4), + B904(16, 4), + ] + self.assertEqual(errors, self.errors(*expected)) + def test_b950(self): filename = Path(__file__).absolute().parent / "b950.py" bbc = BugBearChecker(filename=str(filename))