Skip to content

Commit

Permalink
add except* support to B012&B025 (#500)
Browse files Browse the repository at this point in the history
* add except* support to B012&B025, add tests for any except-handling rules

* now prints except* in error messages
  • Loading branch information
jakkdl authored Dec 11, 2024
1 parent 4fed293 commit b960272
Show file tree
Hide file tree
Showing 11 changed files with 535 additions and 42 deletions.
5 changes: 5 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,11 @@ Change Log
----------


FUTURE
~~~~~~

* B012 and B025 now also handle try/except*

24.10.31
~~~~~~~~

Expand Down
63 changes: 45 additions & 18 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def _flatten_excepthandler(node: ast.expr | None) -> Iterator[ast.expr | None]:
yield expr


def _check_redundant_excepthandlers(names: Sequence[str], node):
def _check_redundant_excepthandlers(names: Sequence[str], node, in_trystar):
# See if any of the given exception names could be removed, e.g. from:
# (MyError, MyError) # duplicate names
# (MyError, BaseException) # everything derives from the Base
Expand Down Expand Up @@ -275,7 +275,7 @@ def _check_redundant_excepthandlers(names: Sequence[str], node):
return B014(
node.lineno,
node.col_offset,
vars=(", ".join(names), as_, desc),
vars=(", ".join(names), as_, desc, in_trystar),
)
return None

Expand Down Expand Up @@ -388,6 +388,9 @@ class BugBearVisitor(ast.NodeVisitor):
_b023_seen: set[error] = attr.ib(factory=set, init=False)
_b005_imports: set[str] = attr.ib(factory=set, init=False)

# set to "*" when inside a try/except*, for correctly printing errors
in_trystar: str = attr.ib(default="")

if False:
# Useful for tracing what the hell is going on.

Expand Down Expand Up @@ -457,7 +460,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
else:
self.b040_caught_exception = B040CaughtException(node.name, False)

names = self.check_for_b013_b029_b030(node)
names = self.check_for_b013_b014_b029_b030(node)

if (
"BaseException" in names
Expand Down Expand Up @@ -605,6 +608,12 @@ def visit_Try(self, node) -> None:
self.check_for_b025(node)
self.generic_visit(node)

def visit_TryStar(self, node) -> None:
outer_trystar = self.in_trystar
self.in_trystar = "*"
self.visit_Try(node)
self.in_trystar = outer_trystar

def visit_Compare(self, node) -> None:
self.check_for_b015(node)
self.generic_visit(node)
Expand Down Expand Up @@ -763,15 +772,17 @@ def _loop(node, bad_node_types) -> None:
bad_node_types = (ast.Return,)

elif isinstance(node, bad_node_types):
self.errors.append(B012(node.lineno, node.col_offset))
self.errors.append(
B012(node.lineno, node.col_offset, vars=(self.in_trystar,))
)

for child in ast.iter_child_nodes(node):
_loop(child, bad_node_types)

for child in node.finalbody:
_loop(child, (ast.Return, ast.Continue, ast.Break))

def check_for_b013_b029_b030(self, node: ast.ExceptHandler) -> list[str]:
def check_for_b013_b014_b029_b030(self, node: ast.ExceptHandler) -> list[str]:
handlers: Iterable[ast.expr | None] = _flatten_excepthandler(node.type)
names: list[str] = []
bad_handlers: list[object] = []
Expand All @@ -791,16 +802,27 @@ def check_for_b013_b029_b030(self, node: ast.ExceptHandler) -> list[str]:
if bad_handlers:
self.errors.append(B030(node.lineno, node.col_offset))
if len(names) == 0 and not bad_handlers and not ignored_handlers:
self.errors.append(B029(node.lineno, node.col_offset))
self.errors.append(
B029(node.lineno, node.col_offset, vars=(self.in_trystar,))
)
elif (
len(names) == 1
and not bad_handlers
and not ignored_handlers
and isinstance(node.type, ast.Tuple)
):
self.errors.append(B013(node.lineno, node.col_offset, vars=names))
self.errors.append(
B013(
node.lineno,
node.col_offset,
vars=(
*names,
self.in_trystar,
),
)
)
else:
maybe_error = _check_redundant_excepthandlers(names, node)
maybe_error = _check_redundant_excepthandlers(names, node, self.in_trystar)
if maybe_error is not None:
self.errors.append(maybe_error)
return names
Expand Down Expand Up @@ -1216,7 +1238,9 @@ def check_for_b904(self, node) -> 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))
self.errors.append(
B904(node.lineno, node.col_offset, vars=(self.in_trystar,))
)

def walk_function_body(self, node):
def _loop(parent, node):
Expand Down Expand Up @@ -1480,7 +1504,9 @@ def check_for_b025(self, node) -> None:
# sort to have a deterministic output
duplicates = sorted({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,)))
self.errors.append(
B025(node.lineno, node.col_offset, vars=(duplicate, self.in_trystar))
)

@staticmethod
def _is_infinite_iterator(node: ast.expr) -> bool:
Expand Down Expand Up @@ -2073,6 +2099,7 @@ def visit_Lambda(self, node) -> None:
error = namedtuple("error", "lineno col message type vars")
Error = partial(partial, error, type=BugBearChecker, vars=())

# note: bare except* is a syntax error, so B001 does not need to handle it
B001 = Error(
message=(
"B001 Do not use bare `except:`, it also catches unexpected "
Expand Down Expand Up @@ -2194,20 +2221,20 @@ def visit_Lambda(self, node) -> None:
B012 = Error(
message=(
"B012 return/continue/break inside finally blocks cause exceptions "
"to be silenced. Exceptions should be silenced in except blocks. Control "
"to be silenced. Exceptions should be silenced in except{0} blocks. Control "
"statements can be moved outside the finally block."
)
)
B013 = Error(
message=(
"B013 A length-one tuple literal is redundant. "
"Write `except {0}:` instead of `except ({0},):`."
"Write `except{1} {0}:` instead of `except{1} ({0},):`."
)
)
B014 = Error(
message=(
"B014 Redundant exception types in `except ({0}){1}:`. "
"Write `except {2}{1}:`, which catches exactly the same exceptions."
"B014 Redundant exception types in `except{3} ({0}){1}:`. "
"Write `except{3} {2}{1}:`, which catches exactly the same exceptions."
)
)
B014_REDUNDANT_EXCEPTIONS = {
Expand Down Expand Up @@ -2296,8 +2323,8 @@ def visit_Lambda(self, node) -> None:
)
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."
"B025 Exception `{0}` has been caught multiple times. Only the first except{0}"
" will be considered and all other except{0} catches can be safely removed."
)
)
B026 = Error(
Expand Down Expand Up @@ -2325,7 +2352,7 @@ def visit_Lambda(self, node) -> None:
)
B029 = Error(
message=(
"B029 Using `except ():` with an empty tuple does not handle/catch "
"B029 Using `except{0} ():` with an empty tuple does not handle/catch "
"anything. Add exceptions to handle."
)
)
Expand Down Expand Up @@ -2414,7 +2441,7 @@ def visit_Lambda(self, node) -> None:

B904 = Error(
message=(
"B904 Within an `except` clause, raise exceptions with `raise ... from err` or"
"B904 Within an `except{0}` 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."
Expand Down
29 changes: 29 additions & 0 deletions tests/b012_py311.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
def a():
try:
pass
except* Exception:
pass
finally:
return # warning


def b():
try:
pass
except* Exception:
pass
finally:
if 1 + 0 == 2 - 1:
return # warning


def c():
try:
pass
except* Exception:
pass
finally:
try:
return # warning
except* Exception:
pass
40 changes: 40 additions & 0 deletions tests/b013_py311.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"""
Should emit:
B013 - on lines 10 and 28
"""

import re

try:
pass
except* (ValueError,):
# pointless use of tuple
pass

# fmt: off
# Turn off black to keep brackets around
# single except*ion for testing purposes.
try:
pass
except* (ValueError):
# not using a tuple means it's OK (if odd)
pass
# fmt: on

try:
pass
except* ValueError:
# no warning here, all good
pass

try:
pass
except* (re.error,):
# pointless use of tuple with dotted attribute
pass

try:
pass
except* (a.b.c.d, b.c.d):
# attribute of attribute, etc.
pass
76 changes: 76 additions & 0 deletions tests/b014_py311.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"""
This is a copy of b014 but with except* instead. Should emit:
B014 - on lines 11, 17, 28, 42, 49, 56, and 74.
"""

import binascii
import re

try:
pass
except* (Exception, TypeError):
# TypeError is a subclass of Exception, so it doesn't add anything
pass

try:
pass
except* (OSError, OSError) as err:
# Duplicate exception types are useless
pass


class MyError(Exception):
pass


try:
pass
except* (MyError, MyError):
# Detect duplicate non-builtin errors
pass


try:
pass
except* (MyError, Exception) as e:
# Don't assume that we're all subclasses of Exception
pass


try:
pass
except* (MyError, BaseException) as e:
# But we *can* assume that everything is a subclass of BaseException
raise e


try:
pass
except* (re.error, re.error):
# Duplicate exception types as attributes
pass


try:
pass
except* (IOError, EnvironmentError, OSError):
# Detect if a primary exception and any its aliases are present.
#
# Since Python 3.3, IOError, EnvironmentError, WindowsError, mmap.error,
# socket.error and select.error are aliases of OSError. See PEP 3151 for
# more info.
pass


try:
pass
except* (MyException, NotImplemented):
# NotImplemented is not an exception, let's not crash on it.
pass


try:
pass
except* (ValueError, binascii.Error):
# binascii.Error is a subclass of ValueError.
pass
38 changes: 38 additions & 0 deletions tests/b025_py311.py
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions tests/b029_py311.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"""
Should emit:
B029 - on lines 8 and 13
"""

try:
pass
except* ():
pass

try:
pass
except* () as e:
pass
Loading

0 comments on commit b960272

Please sign in to comment.