Add B904 - Use raise ... from err in except clauses (#181)

* Strip trailing whitespace
* Add B904 - use exc chaining
* Format with black
* Allow explicit re-raise

This is useful when you want to explicitly modify the traceback, as done by Hypothesis and Trio

* Disable raises check by default

.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

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
 ~~~~~~
 

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"]

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")

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))