From abb732c81071dec3cd60b462babfc281e0074467 Mon Sep 17 00:00:00 2001
From: jakkdl <h6+github@pm.me>
Date: Mon, 24 Jun 2024 11:27:42 +0200
Subject: [PATCH 1/5] add b040: exception with note added not reraised or used

---
 README.rst            |   4 ++
 bugbear.py            |  59 +++++++++++++++++-
 tests/b040.py         | 135 ++++++++++++++++++++++++++++++++++++++++++
 tests/test_bugbear.py |  13 ++++
 4 files changed, 208 insertions(+), 3 deletions(-)
 create mode 100644 tests/b040.py

diff --git a/README.rst b/README.rst
index ce0cfcb..89b35ce 100644
--- a/README.rst
+++ b/README.rst
@@ -351,6 +351,10 @@ MIT
 Change Log
 ----------
 
+Future
+~~~~~~
+* Add B040: Exception with added note not reraised. (#474)
+
 24.4.26
 ~~~~~~~
 
diff --git a/bugbear.py b/bugbear.py
index c2a3032..c1a8a5b 100644
--- a/bugbear.py
+++ b/bugbear.py
@@ -428,11 +428,15 @@ def visit(self, node):
 
         self.check_for_b018(node)
 
-    def visit_ExceptHandler(self, node):
+    def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
         if node.type is None:
             self.errors.append(B001(node.lineno, node.col_offset))
             self.generic_visit(node)
             return
+
+        # used by B040 to check for add_note exceptions that are unraised
+        self.exceptions_tracked = {node.name: None}
+
         handlers = _flatten_excepthandler(node.type)
         names = []
         bad_handlers = []
@@ -471,6 +475,15 @@ def visit_ExceptHandler(self, node):
 
         self.generic_visit(node)
 
+        for _exc_name, exc_status in self.exceptions_tracked.items():
+            if exc_status is None:
+                continue
+            # TODO: if we only track the caught exception, we can directly
+            # point to `node.name.lineno`. If we track more exceptions we should
+            # store the node of the variable in `exceptions_tracked` to point to here.
+            self.errors.append(B040(node.lineno, node.col_offset))
+        self.exceptions_tracked.clear()
+
     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]:
@@ -481,6 +494,7 @@ def visit_UAdd(self, node):
     def visit_Call(self, node):
         if isinstance(node.func, ast.Attribute):
             self.check_for_b005(node)
+            self.check_for_b040_add_note(node.func)
         else:
             with suppress(AttributeError, IndexError):
                 if (
@@ -507,15 +521,19 @@ def visit_Call(self, node):
         self.check_for_b026(node)
         self.check_for_b028(node)
         self.check_for_b034(node)
+        self.check_for_b040_usage(node.args)
+        self.check_for_b040_usage(node.keywords)
         self.check_for_b905(node)
         self.generic_visit(node)
 
     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))
@@ -587,7 +605,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.exceptions_tracked.clear()
+        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)
@@ -604,6 +627,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):
@@ -1054,6 +1078,31 @@ 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) -> None:
+        if (
+            node.attr == "add_note"
+            and isinstance(node.value, ast.Name)
+            and node.value.id in self.exceptions_tracked
+        ):
+            self.exceptions_tracked[node.value.id] = True
+
+    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.exceptions_tracked or node is None:
+            return
+        for name in (
+            n.id
+            for n in superwalk(node)
+            if isinstance(n, ast.Name) and n.id in self.exceptions_tracked
+        ):
+            del self.exceptions_tracked[name]
+
     def _get_assigned_names(self, loop_node):
         loop_targets = (ast.For, ast.AsyncFor, ast.comprehension)
         for node in children_in_scope(loop_node):
@@ -2166,6 +2215,10 @@ def visit_Lambda(self, node):
     message="B037 Class `__init__` methods must not return or yield and any values."
 )
 
+B040 = Error(
+    message="B040 Exception with added note not used. Did you forget to raise it?"
+)
+
 # Warnings disabled by default.
 B901 = Error(
     message=(
diff --git a/tests/b040.py b/tests/b040.py
new file mode 100644
index 0000000..8cd6203
--- /dev/null
+++ b/tests/b040.py
@@ -0,0 +1,135 @@
+def arbitrary_fun(*args, **kwargs):
+    ...
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # error
+
+try:
+    ...
+except Exception as e:  # safe (handled by most type checkers)
+    pass
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...")
+    raise # safe
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...")
+    raise e # safe
+
+try:
+    ...
+except Exception as e:
+    f = ValueError()
+    e.add_note("...") # error
+    raise f
+
+try:
+    ...
+except Exception as e:
+    f = ValueError()
+    e.add_note("...") # safe
+    raise f from e
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    foo = e
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    # not that printing the exception is actually using it, but we treat
+    # it being used as a parameter to any function as "using" it
+    print(e)
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    list(e)
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    arbitrary_fun(kwarg=e)
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    arbitrary_fun(*(e,))
+
+try:
+    ...
+except Exception as e:
+    e.add_note("...") # safe
+    arbitrary_fun(**{"e":e})
+
+
+try:
+    ...
+except ValueError as e:
+    e.add_note("") # error
+except TypeError as e:
+    raise e
+
+mylist = []
+try:
+    ...
+except Exception as e:
+    mylist.append(e)
+    e.add_note("")
+
+
+def exc_add_note_not_in_except():
+    exc = ValueError()
+    exc.add_note("") # should maybe error?
+
+try:
+    ...
+except Exception as e:
+    e2 = ValueError()
+    e2.add_note("") # should maybe error?
+    raise e
+
+try:
+    ...
+except Exception as e:
+    e.add_note("")
+    e = ValueError()
+
+try:
+    ...
+except Exception as e: # should error, but we don't handle lambdas
+    e.add_note("")
+    f = lambda e: e
+
+try:
+    ...
+except Exception as e: # safe
+    e.add_note("")
+    ann_assign_target: Exception = e
+
+try:
+    ...
+except Exception as e: # should error
+    e.add_note(str(e))
+
+try:
+    ...
+except Exception as e:  # should error
+    e.add_note("")
+    try:
+        ...
+    except ValueError as f:
+        pass
diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py
index e84e4e8..b20dbb7 100644
--- a/tests/test_bugbear.py
+++ b/tests/test_bugbear.py
@@ -46,6 +46,7 @@
     B035,
     B036,
     B037,
+    B040,
     B901,
     B902,
     B903,
@@ -636,6 +637,18 @@ def test_b037(self) -> None:
         )
         self.assertEqual(errors, expected)
 
+    def test_b040(self) -> None:
+        filename = Path(__file__).absolute().parent / "b040.py"
+        bbc = BugBearChecker(filename=str(filename))
+        errors = list(bbc.run())
+        expected = self.errors(
+            B040(6, 0),
+            B040(28, 0),
+            B040(81, 0),
+            B040(107, 0),
+        )
+        self.assertEqual(errors, expected)
+
     def test_b908(self):
         filename = Path(__file__).absolute().parent / "b908.py"
         bbc = BugBearChecker(filename=str(filename))

From 48e3dffdff6d0a9a796c7687d6a655606f72b72b Mon Sep 17 00:00:00 2001
From: jakkdl <h6+github@pm.me>
Date: Mon, 24 Jun 2024 11:47:05 +0200
Subject: [PATCH 2/5] handle two more cases, with temp ugly code

---
 bugbear.py            | 30 ++++++++++++++++++++++++------
 tests/b040.py         |  5 +++--
 tests/test_bugbear.py |  2 ++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/bugbear.py b/bugbear.py
index c1a8a5b..373208a 100644
--- a/bugbear.py
+++ b/bugbear.py
@@ -366,6 +366,7 @@ class BugBearVisitor(ast.NodeVisitor):
     errors = attr.ib(default=attr.Factory(list))
     futures = attr.ib(default=attr.Factory(set))
     contexts = attr.ib(default=attr.Factory(list))
+    exceptions_tracked: dict[str, bool | None] = attr.ib(default=attr.Factory(dict))
 
     NODE_WINDOW_SIZE = 4
     _b023_seen = attr.ib(factory=set, init=False)
@@ -435,6 +436,7 @@ def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
             return
 
         # used by B040 to check for add_note exceptions that are unraised
+        old_exceptions_tracked = self.exceptions_tracked
         self.exceptions_tracked = {node.name: None}
 
         handlers = _flatten_excepthandler(node.type)
@@ -482,7 +484,7 @@ def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
             # point to `node.name.lineno`. If we track more exceptions we should
             # store the node of the variable in `exceptions_tracked` to point to here.
             self.errors.append(B040(node.lineno, node.col_offset))
-        self.exceptions_tracked.clear()
+        self.exceptions_tracked = old_exceptions_tracked
 
     def visit_UAdd(self, node):
         trailing_nodes = list(map(type, self.node_window[-4:]))
@@ -492,9 +494,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)
-            self.check_for_b040_add_note(node.func)
+            is_b040_add_note = self.check_for_b040_add_note(node.func)
         else:
             with suppress(AttributeError, IndexError):
                 if (
@@ -521,10 +524,19 @@ def visit_Call(self, node):
         self.check_for_b026(node)
         self.check_for_b028(node)
         self.check_for_b034(node)
-        self.check_for_b040_usage(node.args)
-        self.check_for_b040_usage(node.keywords)
         self.check_for_b905(node)
-        self.generic_visit(node)
+
+        if not is_b040_add_note:
+            self.check_for_b040_usage(node.args)
+            self.check_for_b040_usage(node.keywords)
+            self.generic_visit(node)
+        else:
+            # TODO: temp dirty hack to avoid nested calls within the parameter list
+            # using the variable itself. (i.e. `e.add_note(str(e))`)
+            # will clean up, complexity depends on decision of scope of the check
+            current_exceptions_tracked = self.exceptions_tracked.copy()
+            self.generic_visit(node)
+            self.exceptions_tracked = current_exceptions_tracked
 
     def visit_Module(self, node):
         self.generic_visit(node)
@@ -1078,13 +1090,15 @@ 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) -> None:
+    def check_for_b040_add_note(self, node: ast.Attribute) -> bool:
         if (
             node.attr == "add_note"
             and isinstance(node.value, ast.Name)
             and node.value.id in self.exceptions_tracked
         ):
             self.exceptions_tracked[node.value.id] = True
+            return True
+        return False
 
     def check_for_b040_usage(self, node: ast.expr | None) -> None:
         def superwalk(node: ast.AST | list[ast.AST]):
@@ -1101,6 +1115,10 @@ def superwalk(node: ast.AST | list[ast.AST]):
             for n in superwalk(node)
             if isinstance(n, ast.Name) and n.id in self.exceptions_tracked
         ):
+            if hasattr(node, "lineno"):
+                print("deleting", node.lineno)
+            else:
+                print("deleting0", node[0].lineno)
             del self.exceptions_tracked[name]
 
     def _get_assigned_names(self, loop_node):
diff --git a/tests/b040.py b/tests/b040.py
index 8cd6203..476bc1f 100644
--- a/tests/b040.py
+++ b/tests/b040.py
@@ -122,12 +122,13 @@ def exc_add_note_not_in_except():
 
 try:
     ...
-except Exception as e: # should error
+except Exception as e: # error
     e.add_note(str(e))
 
+# check nesting
 try:
     ...
-except Exception as e:  # should error
+except Exception as e:  # error
     e.add_note("")
     try:
         ...
diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py
index b20dbb7..087191b 100644
--- a/tests/test_bugbear.py
+++ b/tests/test_bugbear.py
@@ -646,6 +646,8 @@ def test_b040(self) -> None:
             B040(28, 0),
             B040(81, 0),
             B040(107, 0),
+            B040(125, 0),
+            B040(131, 0),
         )
         self.assertEqual(errors, expected)
 

From d749c3dbdfd60507bfe392d80bb7af1fbe8627b7 Mon Sep 17 00:00:00 2001
From: jakkdl <h6+github@pm.me>
Date: Mon, 24 Jun 2024 12:11:59 +0200
Subject: [PATCH 3/5] add test case, clean up stray debug prints

---
 bugbear.py            | 4 ----
 tests/b040.py         | 7 +++++++
 tests/test_bugbear.py | 4 ++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/bugbear.py b/bugbear.py
index 373208a..10f02af 100644
--- a/bugbear.py
+++ b/bugbear.py
@@ -1115,10 +1115,6 @@ def superwalk(node: ast.AST | list[ast.AST]):
             for n in superwalk(node)
             if isinstance(n, ast.Name) and n.id in self.exceptions_tracked
         ):
-            if hasattr(node, "lineno"):
-                print("deleting", node.lineno)
-            else:
-                print("deleting0", node[0].lineno)
             del self.exceptions_tracked[name]
 
     def _get_assigned_names(self, loop_node):
diff --git a/tests/b040.py b/tests/b040.py
index 476bc1f..1f37f58 100644
--- a/tests/b040.py
+++ b/tests/b040.py
@@ -114,6 +114,13 @@ def exc_add_note_not_in_except():
     e.add_note("")
     f = lambda e: e
 
+try:
+    ...
+except Exception as e: # should error, but we don't handle nested functions
+    e.add_note("")
+    def habla(e):
+        raise e
+
 try:
     ...
 except Exception as e: # safe
diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py
index 087191b..5669222 100644
--- a/tests/test_bugbear.py
+++ b/tests/test_bugbear.py
@@ -646,8 +646,8 @@ def test_b040(self) -> None:
             B040(28, 0),
             B040(81, 0),
             B040(107, 0),
-            B040(125, 0),
-            B040(131, 0),
+            B040(132, 0),
+            B040(138, 0),
         )
         self.assertEqual(errors, expected)
 

From 3260555c3239ba8577774e0891875e65fd5b298d Mon Sep 17 00:00:00 2001
From: jakkdl <h6+github@pm.me>
Date: Wed, 26 Jun 2024 16:45:38 +0200
Subject: [PATCH 4/5] break out b013,b029,b303 handler, clean up test file

---
 bugbear.py            |  70 +++++++++++----------
 tests/b040.py         | 137 +++++++++++++++++++++++++++---------------
 tests/test_bugbear.py |  10 +--
 3 files changed, 132 insertions(+), 85 deletions(-)

diff --git a/bugbear.py b/bugbear.py
index 10f02af..ad53311 100644
--- a/bugbear.py
+++ b/bugbear.py
@@ -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
@@ -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
@@ -439,36 +439,8 @@ def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
         old_exceptions_tracked = self.exceptions_tracked
         self.exceptions_tracked = {node.name: None}
 
-        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))
-        else:
-            maybe_error = _check_redundant_excepthandlers(names, node)
-            if maybe_error is not None:
-                self.errors.append(maybe_error)
+        names = self.check_for_b013_b029_b030(node)
+
         if (
             "BaseException" in names
             and not ExceptBaseExceptionVisitor(node).re_raised()
@@ -728,6 +700,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))
diff --git a/tests/b040.py b/tests/b040.py
index 1f37f58..1f5d6ec 100644
--- a/tests/b040.py
+++ b/tests/b040.py
@@ -1,52 +1,51 @@
-def arbitrary_fun(*args, **kwargs):
-    ...
+def arbitrary_fun(*args, **kwargs): ...
+
 
+# classic case
 try:
     ...
 except Exception as e:
-    e.add_note("...") # error
+    e.add_note("...")  # error
 
 try:
     ...
-except Exception as e:  # safe (handled by most type checkers)
+except Exception as e:  # safe (other linters will catch this)
     pass
 
 try:
     ...
 except Exception as e:
     e.add_note("...")
-    raise # safe
-
-try:
-    ...
-except Exception as e:
-    e.add_note("...")
-    raise e # safe
+    raise  # safe
 
+# other exception raised
 try:
     ...
 except Exception as e:
     f = ValueError()
-    e.add_note("...") # error
+    e.add_note("...")  # error
     raise f
 
+# raised as cause
 try:
     ...
 except Exception as e:
     f = ValueError()
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     raise f from e
 
+# assigned to other variable
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     foo = e
 
+# "used" in function call
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     # not that printing the exception is actually using it, but we treat
     # it being used as a parameter to any function as "using" it
     print(e)
@@ -54,90 +53,132 @@ def arbitrary_fun(*args, **kwargs):
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     list(e)
 
+# kwarg
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     arbitrary_fun(kwarg=e)
 
+# stararg
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
+    e.add_note("...")  # safe
     arbitrary_fun(*(e,))
 
 try:
     ...
 except Exception as e:
-    e.add_note("...") # safe
-    arbitrary_fun(**{"e":e})
+    e.add_note("...")  # safe
+    arbitrary_fun(**{"e": e})
 
 
+# multiple ExceptHandlers
 try:
     ...
 except ValueError as e:
-    e.add_note("") # error
+    e.add_note("")  # error
 except TypeError as e:
     raise e
 
+# exception variable used before `add_note`
 mylist = []
 try:
     ...
-except Exception as e:
+except Exception as e: # safe
     mylist.append(e)
     e.add_note("")
 
 
-def exc_add_note_not_in_except():
-    exc = ValueError()
-    exc.add_note("") # should maybe error?
+# AnnAssign
+try:
+    ...
+except Exception as e:  # safe
+    e.add_note("")
+    ann_assign_target: Exception = e
 
+# special case: e is only used in the `add_note` call itself
 try:
     ...
-except Exception as e:
-    e2 = ValueError()
-    e2.add_note("") # should maybe error?
-    raise e
+except Exception as e:  # error
+    e.add_note(str(e))
+    e.add_note(str(e))
 
+# check nesting
 try:
     ...
-except Exception as e:
+except Exception as e:  # error
     e.add_note("")
-    e = ValueError()
+    try:
+        ...
+    except ValueError as e:
+        raise
 
+# questionable if this should error
 try:
     ...
-except Exception as e: # should error, but we don't handle lambdas
+except Exception as e:
     e.add_note("")
-    f = lambda e: e
+    e = ValueError()
+
+# *** unhandled cases ***
+
 
+# This should ideally error
+def exc_add_note_not_in_except():
+    exc = ValueError()
+    exc.add_note("")
+
+
+# but not this
+def exc_add_note_not_in_except_safe(exc: ValueError):
+    exc.add_note("")
+
+
+# we currently only check the target of the except handler, even though this clearly
+# is hitting the same general pattern. But handling this case without a lot of false
+# alarms is very tricky without more infrastructure.
 try:
     ...
-except Exception as e: # should error, but we don't handle nested functions
-    e.add_note("")
-    def habla(e):
-        raise e
+except Exception as e:
+    e2 = ValueError()  # should error
+    e2.add_note(str(e))
+
+
+def foo_add_note_in_excepthandler():
+    e2 = ValueError()
+    try:
+        ...
+    except Exception as e:
+        e2.add_note(str(e))  # safe
+    raise e2
+
 
+# We don't currently handle lambdas or function definitions inside the exception
+# handler. This isn't conceptually difficult, but not really worth it with current
+# infrastructure
 try:
     ...
-except Exception as e: # safe
+except Exception as e:  # should error
     e.add_note("")
-    ann_assign_target: Exception = e
+    f = lambda e: e
 
 try:
     ...
-except Exception as e: # error
-    e.add_note(str(e))
+except Exception as e:  # should error
+    e.add_note("")
 
-# check nesting
+    def habla(e):
+        raise e
+
+
+# We also don't track other variables
 try:
     ...
-except Exception as e:  # error
-    e.add_note("")
-    try:
-        ...
-    except ValueError as f:
-        pass
+except Exception as e:
+    e3 = e
+    e3.add_note("")  # should error
diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py
index 5669222..63fff6a 100644
--- a/tests/test_bugbear.py
+++ b/tests/test_bugbear.py
@@ -642,12 +642,12 @@ def test_b040(self) -> None:
         bbc = BugBearChecker(filename=str(filename))
         errors = list(bbc.run())
         expected = self.errors(
-            B040(6, 0),
-            B040(28, 0),
-            B040(81, 0),
+            B040(7, 0),
+            B040(24, 0),
+            B040(83, 0),
             B040(107, 0),
-            B040(132, 0),
-            B040(138, 0),
+            B040(114, 0),
+            B040(124, 0),
         )
         self.assertEqual(errors, expected)
 

From 30c7c9178315b8202f45bd330b8b606a9ace6637 Mon Sep 17 00:00:00 2001
From: jakkdl <h6+github@pm.me>
Date: Wed, 26 Jun 2024 17:16:40 +0200
Subject: [PATCH 5/5] simplify and clean up implementation. Replace
 `attr.ib(default=attr.Factory(...))` with `attr.ib(factory=...)`

---
 bugbear.py | 85 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 39 deletions(-)

diff --git a/bugbear.py b/bugbear.py
index 7f55e88..61aeaca 100644
--- a/bugbear.py
+++ b/bugbear.py
@@ -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):
@@ -356,17 +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))
-    exceptions_tracked: dict[str, bool | None] = attr.ib(default=attr.Factory(dict))
+    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)
@@ -429,15 +435,17 @@ def visit(self, node):
 
         self.check_for_b018(node)
 
-    def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
+    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
 
-        # used by B040 to check for add_note exceptions that are unraised
-        old_exceptions_tracked = self.exceptions_tracked
-        self.exceptions_tracked = {node.name: None}
+        old_b040_caught_exception = self.b040_caught_exception
+        if node.name is None:
+            self.b040_caught_exception = None
+        else:
+            self.b040_caught_exception = B040CaughtException(node.name, False)
 
         names = self.check_for_b013_b029_b030(node)
 
@@ -449,14 +457,12 @@ def visit_ExceptHandler(self, node):  # noqa: C901 # too complex
 
         self.generic_visit(node)
 
-        for _exc_name, exc_status in self.exceptions_tracked.items():
-            if exc_status is None:
-                continue
-            # TODO: if we only track the caught exception, we can directly
-            # point to `node.name.lineno`. If we track more exceptions we should
-            # store the node of the variable in `exceptions_tracked` to point to here.
+        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.exceptions_tracked = old_exceptions_tracked
+        self.b040_caught_exception = old_b040_caught_exception
 
     def visit_UAdd(self, node):
         trailing_nodes = list(map(type, self.node_window[-4:]))
@@ -499,17 +505,18 @@ def visit_Call(self, 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)
-        else:
-            # TODO: temp dirty hack to avoid nested calls within the parameter list
-            # using the variable itself. (i.e. `e.add_note(str(e))`)
-            # will clean up, complexity depends on decision of scope of the check
-            current_exceptions_tracked = self.exceptions_tracked.copy()
-            self.generic_visit(node)
-            self.exceptions_tracked = current_exceptions_tracked
+
+        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)
@@ -592,7 +599,7 @@ def visit_Compare(self, node):
 
     def visit_Raise(self, node: ast.Raise):
         if node.exc is None:
-            self.exceptions_tracked.clear()
+            self.b040_caught_exception = None
         else:
             self.check_for_b040_usage(node.exc)
             self.check_for_b040_usage(node.cause)
@@ -1127,9 +1134,10 @@ def check_for_b040_add_note(self, node: ast.Attribute) -> bool:
         if (
             node.attr == "add_note"
             and isinstance(node.value, ast.Name)
-            and node.value.id in self.exceptions_tracked
+            and self.b040_caught_exception
+            and node.value.id == self.b040_caught_exception.name
         ):
-            self.exceptions_tracked[node.value.id] = True
+            self.b040_caught_exception.has_note = True
             return True
         return False
 
@@ -1141,14 +1149,13 @@ def superwalk(node: ast.AST | list[ast.AST]):
             else:
                 yield from ast.walk(node)
 
-        if not self.exceptions_tracked or node is None:
+        if not self.b040_caught_exception or node is None:
             return
-        for name in (
-            n.id
-            for n in superwalk(node)
-            if isinstance(n, ast.Name) and n.id in self.exceptions_tracked
-        ):
-            del self.exceptions_tracked[name]
+
+        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)
@@ -1835,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
@@ -1860,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)