Add new check which finds duplicate except clauses (#284)

* Add new check which finds duplicate except clauses

Closes #254

* Add sorting

Author: kasium

README.rst

@@ -156,6 +156,10 @@ the loop, because `late-binding closures are a classic gotcha
 
 **B024**: Abstract base class with no abstract method. Remember to use @abstractmethod, @abstractclassmethod, and/or @abstractproperty decorators.
 
+**B025**: ``try-except`` block with duplicate exceptions found.
+This check identifies exception types that are specified in multiple ``except``
+clauses. The first specification is the only one ever considered, so all others can be removed.
+
 Opinionated warnings
 ~~~~~~~~~~~~~~~~~~~~
 

bugbear.py

@@ -421,6 +421,7 @@ def visit_ClassDef(self, node):
 
     def visit_Try(self, node):
         self.check_for_b012(node)
+        self.check_for_b025(node)
         self.generic_visit(node)
 
     def visit_Compare(self, node):
@@ -837,6 +838,24 @@ def check_for_b022(self, node):
         ):
             self.errors.append(B022(node.lineno, node.col_offset))
 
+    def check_for_b025(self, node):
+        seen = []
+        for handler in node.handlers:
+            if isinstance(handler.type, (ast.Name, ast.Attribute)):
+                name = ".".join(compose_call_path(handler.type))
+                seen.append(name)
+            elif isinstance(handler.type, ast.Tuple):
+                # to avoid checking the same as B014, remove duplicates per except
+                uniques = set()
+                for entry in handler.type.elts:
+                    name = ".".join(compose_call_path(entry))
+                    uniques.add(name)
+                seen.extend(uniques)
+        # sort to have a deterministic output
+        duplicates = sorted(set(x for x in seen if seen.count(x) > 1))
+        for duplicate in duplicates:
+            self.errors.append(B025(node.lineno, node.col_offset, vars=(duplicate,)))
+
 
 def compose_call_path(node):
     if isinstance(node, ast.Attribute):
@@ -1178,6 +1197,12 @@ def visit_Lambda(self, node):
         " decorators."
     )
 )
+B025 = Error(
+    message=(
+        "B025 Exception `{0}` has been caught multiple times. Only the first except"
+        " will be considered and all other except catches can be safely removed."
+    )
+)
 
 # Warnings disabled by default.
 B901 = Error(

tests/b025.py

@@ -0,0 +1,38 @@
+"""
+Should emit:
+B025 - on lines 15, 22, 31
+"""
+
+import pickle
+
+try:
+    a = 1
+except ValueError:
+    a = 2
+finally:
+    a = 3
+
+try:
+    a = 1
+except ValueError:
+    a = 2
+except ValueError:
+    a = 2
+
+try:
+    a = 1
+except pickle.PickleError:
+    a = 2
+except ValueError:
+    a = 2
+except pickle.PickleError:
+    a = 2
+
+try:
+    a = 1
+except (ValueError, TypeError):
+    a = 2
+except ValueError:
+    a = 2
+except (OSError, TypeError):
+    a = 2

tests/test_bugbear.py

@@ -35,6 +35,7 @@
     B022,
     B023,
     B024,
+    B025,
     B901,
     B902,
     B903,
@@ -366,6 +367,20 @@ def test_b024(self):
         )
         self.assertEqual(errors, expected)
 
+    def test_b025(self):
+        filename = Path(__file__).absolute().parent / "b025.py"
+        bbc = BugBearChecker(filename=str(filename))
+        errors = list(bbc.run())
+        self.assertEqual(
+            errors,
+            self.errors(
+                B025(15, 0, vars=("ValueError",)),
+                B025(22, 0, vars=("pickle.PickleError",)),
+                B025(31, 0, vars=("TypeError",)),
+                B025(31, 0, vars=("ValueError",)),
+            ),
+        )
+
     def test_b901(self):
         filename = Path(__file__).absolute().parent / "b901.py"
         bbc = BugBearChecker(filename=str(filename))