Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new check which finds duplicate except clauses #284

Merged
merged 2 commits into from Sep 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.rst
Expand Up @@ -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
~~~~~~~~~~~~~~~~~~~~

Expand Down
25 changes: 25 additions & 0 deletions bugbear.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down
38 changes: 38 additions & 0 deletions 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
15 changes: 15 additions & 0 deletions tests/test_bugbear.py
Expand Up @@ -35,6 +35,7 @@
B022,
B023,
B024,
B025,
B901,
B902,
B903,
Expand Down Expand Up @@ -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))
Expand Down