diff --git a/README.rst b/README.rst index 24c86d4..6dce0aa 100644 --- a/README.rst +++ b/README.rst @@ -292,7 +292,12 @@ second usage. Save the result to a list if the result is needed multiple times. .. _B042: -**B042**: Remember to call super().__init__() in custom exceptions initalizer. +**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. +Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. +If you define `__init__` yourself without passing all arguments to `super().__init__` it is very easy to break pickling, especially if they pass keyword arguments which both +`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. +Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. +If you define `__str__/__reduce__` in super classes this check is unable to detect it, and we advise disabling it. .. _B043: diff --git a/bugbear.py b/bugbear.py index 029c713..279a955 100644 --- a/bugbear.py +++ b/bugbear.py @@ -1756,10 +1756,39 @@ def is_exception(s: str): else: return + # if the user defines __str__ + a pickle dunder they're probably in the clear. + has_pickle_dunder = False + has_str = False + for fun in node.body: + if isinstance(fun, ast.FunctionDef) and fun.name in ( + "__getnewargs_ex__", + "__getnewargs__", + "__getstate__", + "__setstate__", + "__reduce__", + "__reduce_ex__", + ): + if has_str: + return + has_pickle_dunder = True + elif isinstance(fun, ast.FunctionDef) and fun.name == "__str__": + if has_pickle_dunder: + return + has_str = True + # iterate body nodes looking for __init__ for fun in node.body: if not (isinstance(fun, ast.FunctionDef) and fun.name == "__init__"): continue + if any( + (isinstance(decorator, ast.Name) and decorator.id == "overload") + or ( + isinstance(decorator, ast.Attribute) + and decorator.attr == "overload" + ) + for decorator in fun.decorator_list + ): + continue if fun.args.kwonlyargs or fun.args.kwarg: # kwargs cannot be passed to super().__init__() self.add_error("B042", fun) @@ -2418,7 +2447,7 @@ def __call__(self, lineno: int, col: int, vars: tuple[object, ...] = ()) -> erro "B042": Error( message=( "B042 Exception class with `__init__` should pass all args to " - "`super().__init__()` in order to work with `copy.copy()`. " + "`super().__init__()` to work in edge cases of `pickle` and `copy.copy()`. " "It should also not take any kwargs." ) ), diff --git a/tests/eval_files/b042.py b/tests/eval_files/b042.py index d1efcc3..952f029 100644 --- a/tests/eval_files/b042.py +++ b/tests/eval_files/b042.py @@ -1,3 +1,7 @@ +import typing +from typing import overload + + class MyError_no_args(Exception): def __init__(self): # safe ... @@ -46,6 +50,25 @@ class MyError_posonlyargs(Exception): def __init__(self, x, /, y): super().__init__(x, y) +# ignore overloaded __init__ +class MyException(Exception): + @overload + def __init__(self, x: int): ... + @overload + def __init__(self, x: float): ... + + def __init__(self, x): + super().__init__(x) + +class MyException2(Exception): + @typing.overload + def __init__(self, x: int): ... + @typing.overload + def __init__(self, x: float): ... + + def __init__(self, x): + super().__init__(x) + # triggers if class name ends with, or # if it inherits from a class whose name ends with, any of # 'Error', 'Exception', 'ExceptionGroup', 'Warning', 'ExceptionGroup' @@ -70,5 +93,23 @@ def __init__(self, x): ... # B042: 4 class ExceptionHandler(Anything): def __init__(self, x): ... # safe -class FooException: +class FooException: # safe, doesn't inherit from anything + def __init__(self, x): ... + +### Ignore classes that define __str__ + any pickle dunder +class HasReduceStr(Exception): + def __reduce__(self): ... + def __str__(self): ... + def __init__(self, x): ... + +class HasReduce(Exception): + def __reduce__(self): ... + def __init__(self, x): ... # B042: 4 +class HasStr(Exception): + def __str__(self): ... + def __init__(self, x): ... # B042: 4 + +class HasStrReduceEx(Exception): + def __reduce_ex__(self, protocol): ... + def __str__(self): ... def __init__(self, x): ...