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 except* support to B012&B025 #500

Merged
merged 2 commits into from
Dec 11, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,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 @@ -604,6 +607,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 @@ -762,15 +771,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 @@ -790,16 +801,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 @@ -1215,7 +1237,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 @@ -1479,7 +1503,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 @@ -2058,6 +2084,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 @@ -2179,20 +2206,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 @@ -2281,8 +2308,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 @@ -2310,7 +2337,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 @@ -2399,7 +2426,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
Loading