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 b040: exception with note added not reraised or used #477

Merged
merged 6 commits into from
Jun 28, 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
3 changes: 3 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ second usage. Save the result to a list if the result is needed multiple times.

**B039**: ``ContextVar`` with mutable literal or function call as default. This is only evaluated once, and all subsequent calls to `.get()` would return the same instance of the default. This uses the same logic as B006 and B008, including ignoring values in ``extend-immutable-calls``.

**B040**: Caught exception with call to ``add_note`` not used. Did you forget to ``raise`` it?

Opinionated warnings
~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -357,6 +359,7 @@ FUTURE
~~~~~~

* Add B039, ``ContextVar`` with mutable literal or function call as default.
* Add B040: Exception with added note not reraised. (#474)

24.4.26
~~~~~~~
Expand Down
166 changes: 123 additions & 43 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from contextlib import suppress
from functools import lru_cache, partial
from keyword import iskeyword
from typing import Dict, List, Set, Union
from typing import Dict, Iterable, Iterator, List, Set, Union

import attr
import pycodestyle
Expand Down Expand Up @@ -55,7 +55,7 @@ class BugBearChecker:
filename = attr.ib(default="(none)")
lines = attr.ib(default=None)
max_line_length = attr.ib(default=79)
visitor = attr.ib(init=False, default=attr.Factory(lambda: BugBearVisitor))
visitor = attr.ib(init=False, factory=lambda: BugBearVisitor)
options = attr.ib(default=None)

def run(self):
Expand Down Expand Up @@ -227,7 +227,7 @@ def _is_identifier(arg):
return re.match(r"^[A-Za-z_][A-Za-z0-9_]*$", arg.value) is not None


def _flatten_excepthandler(node):
def _flatten_excepthandler(node: ast.expr | None) -> Iterator[ast.expr | None]:
if not isinstance(node, ast.Tuple):
yield node
return
Expand Down Expand Up @@ -356,16 +356,23 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
return super().generic_visit(node)


@attr.define
class B040CaughtException:
name: str
has_note: bool


@attr.s
class BugBearVisitor(ast.NodeVisitor):
filename = attr.ib()
lines = attr.ib()
b008_b039_extend_immutable_calls = attr.ib(default=attr.Factory(set))
b902_classmethod_decorators = attr.ib(default=attr.Factory(set))
node_window = attr.ib(default=attr.Factory(list))
errors = attr.ib(default=attr.Factory(list))
futures = attr.ib(default=attr.Factory(set))
contexts = attr.ib(default=attr.Factory(list))
b008_b039_extend_immutable_calls = attr.ib(factory=set)
b902_classmethod_decorators = attr.ib(factory=set)
node_window = attr.ib(factory=list)
errors = attr.ib(factory=list)
futures = attr.ib(factory=set)
contexts = attr.ib(factory=list)
b040_caught_exception: B040CaughtException | None = attr.ib(default=None)

NODE_WINDOW_SIZE = 4
_b023_seen = attr.ib(factory=set, init=False)
Expand Down Expand Up @@ -428,41 +435,20 @@ def visit(self, node):

self.check_for_b018(node)

def visit_ExceptHandler(self, node):
def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
if node.type is None:
self.errors.append(B001(node.lineno, node.col_offset))
self.generic_visit(node)
return
handlers = _flatten_excepthandler(node.type)
names = []
bad_handlers = []
ignored_handlers = []
for handler in handlers:
if isinstance(handler, (ast.Name, ast.Attribute)):
name = _to_name_str(handler)
if name is None:
ignored_handlers.append(handler)
else:
names.append(name)
elif isinstance(handler, (ast.Call, ast.Starred)):
ignored_handlers.append(handler)
else:
bad_handlers.append(handler)
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))
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))

old_b040_caught_exception = self.b040_caught_exception
if node.name is None:
self.b040_caught_exception = None
else:
maybe_error = _check_redundant_excepthandlers(names, node)
if maybe_error is not None:
self.errors.append(maybe_error)
self.b040_caught_exception = B040CaughtException(node.name, False)

names = self.check_for_b013_b029_b030(node)

if (
"BaseException" in names
and not ExceptBaseExceptionVisitor(node).re_raised()
Expand All @@ -471,6 +457,13 @@ def visit_ExceptHandler(self, node):

self.generic_visit(node)

if (
self.b040_caught_exception is not None
and self.b040_caught_exception.has_note
):
self.errors.append(B040(node.lineno, node.col_offset))
self.b040_caught_exception = old_b040_caught_exception

def visit_UAdd(self, node):
trailing_nodes = list(map(type, self.node_window[-4:]))
if trailing_nodes == [ast.UnaryOp, ast.UAdd, ast.UnaryOp, ast.UAdd]:
Expand All @@ -479,8 +472,10 @@ def visit_UAdd(self, node):
self.generic_visit(node)

def visit_Call(self, node):
is_b040_add_note = False
if isinstance(node.func, ast.Attribute):
self.check_for_b005(node)
is_b040_add_note = self.check_for_b040_add_note(node.func)
else:
with suppress(AttributeError, IndexError):
if (
Expand Down Expand Up @@ -509,14 +504,28 @@ def visit_Call(self, node):
self.check_for_b034(node)
self.check_for_b039(node)
self.check_for_b905(node)

# no need for copying, if used in nested calls it will be set to None
current_b040_caught_exception = self.b040_caught_exception
if not is_b040_add_note:
self.check_for_b040_usage(node.args)
self.check_for_b040_usage(node.keywords)

self.generic_visit(node)

if is_b040_add_note:
# Avoid nested calls within the parameter list using the variable itself.
# e.g. `e.add_note(str(e))`
self.b040_caught_exception = current_b040_caught_exception

def visit_Module(self, node):
self.generic_visit(node)

def visit_Assign(self, node):
def visit_Assign(self, node: ast.Assign) -> None:
self.check_for_b040_usage(node.value)
if len(node.targets) == 1:
t = node.targets[0]

if isinstance(t, ast.Attribute) and isinstance(t.value, ast.Name):
if (t.value.id, t.attr) == ("os", "environ"):
self.errors.append(B003(node.lineno, node.col_offset))
Expand Down Expand Up @@ -588,7 +597,12 @@ def visit_Compare(self, node):
self.check_for_b015(node)
self.generic_visit(node)

def visit_Raise(self, node):
def visit_Raise(self, node: ast.Raise):
if node.exc is None:
self.b040_caught_exception = None
else:
self.check_for_b040_usage(node.exc)
self.check_for_b040_usage(node.cause)
self.check_for_b016(node)
self.check_for_b904(node)
self.generic_visit(node)
Expand All @@ -605,6 +619,7 @@ def visit_JoinedStr(self, node):

def visit_AnnAssign(self, node):
self.check_for_b032(node)
self.check_for_b040_usage(node.value)
self.generic_visit(node)

def visit_Import(self, node):
Expand Down Expand Up @@ -719,6 +734,40 @@ def _loop(node, 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]:
handlers: Iterable[ast.expr | None] = _flatten_excepthandler(node.type)
names: list[str] = []
bad_handlers: list[object] = []
ignored_handlers: list[ast.Name | ast.Attribute | ast.Call | ast.Starred] = []

for handler in handlers:
if isinstance(handler, (ast.Name, ast.Attribute)):
name = _to_name_str(handler)
if name is None:
ignored_handlers.append(handler)
else:
names.append(name)
elif isinstance(handler, (ast.Call, ast.Starred)):
ignored_handlers.append(handler)
else:
bad_handlers.append(handler)
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))
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))
else:
maybe_error = _check_redundant_excepthandlers(names, node)
if maybe_error is not None:
self.errors.append(maybe_error)
return names

def check_for_b015(self, node):
if isinstance(self.node_stack[-2], ast.Expr):
self.errors.append(B015(node.lineno, node.col_offset))
Expand Down Expand Up @@ -1081,6 +1130,33 @@ def check_for_b035(self, node: ast.DictComp):
B035(node.key.lineno, node.key.col_offset, vars=(node.key.id,))
)

def check_for_b040_add_note(self, node: ast.Attribute) -> bool:
if (
node.attr == "add_note"
and isinstance(node.value, ast.Name)
and self.b040_caught_exception
and node.value.id == self.b040_caught_exception.name
):
self.b040_caught_exception.has_note = True
return True
return False

def check_for_b040_usage(self, node: ast.expr | None) -> None:
def superwalk(node: ast.AST | list[ast.AST]):
if isinstance(node, list):
for n in node:
yield from ast.walk(n)
else:
yield from ast.walk(node)

if not self.b040_caught_exception or node is None:
return

for n in superwalk(node):
if isinstance(n, ast.Name) and n.id == self.b040_caught_exception.name:
self.b040_caught_exception = None
break

def _get_assigned_names(self, loop_node):
loop_targets = (ast.For, ast.AsyncFor, ast.comprehension)
for node in children_in_scope(loop_node):
Expand Down Expand Up @@ -1766,7 +1842,7 @@ class NameFinder(ast.NodeVisitor):
key is name string, value is the node (useful for location purposes).
"""

names: Dict[str, List[ast.Name]] = attr.ib(default=attr.Factory(dict))
names: Dict[str, List[ast.Name]] = attr.ib(factory=dict)

def visit_Name( # noqa: B906 # names don't contain other names
self, node: ast.Name
Expand All @@ -1791,7 +1867,7 @@ class NamedExprFinder(ast.NodeVisitor):
key is name string, value is the node (useful for location purposes).
"""

names: Dict[str, List[ast.Name]] = attr.ib(default=attr.Factory(dict))
names: Dict[str, List[ast.Name]] = attr.ib(factory=dict)

def visit_NamedExpr(self, node: ast.NamedExpr):
self.names.setdefault(node.target.id, []).append(node.target)
Expand Down Expand Up @@ -2218,6 +2294,10 @@ def visit_Lambda(self, node):
)
)

B040 = Error(
message="B040 Exception with added note not used. Did you forget to raise it?"
)

# Warnings disabled by default.
B901 = Error(
message=(
Expand Down
Loading