feat: implement new TRY005 violation

README.md

@@ -133,7 +133,8 @@ Example:
 [tool.tryceratops]
 exclude = ["samples"]
 ignore = ["TRY002", "TRY200", "TRY300"]
-experimental = true
+experimental = false
+check_pickable = false
 ```
 
 CLI flags always overwrite the config file.

docs/violations/README.md

@@ -2,11 +2,12 @@
 
 ## `TRY0xx` - Exception Classes
 
-| Code                | Description                                                |
-| ------------------- | ---------------------------------------------------------- |
-| [TRY002](TRY002.md) | Create your own exception                                  |
-| [TRY003](TRY003.md) | Avoid specifying long messages outside the exception class |
-| [TRY004](TRY004.md) | Prefer `TypeError` exception for invalid type              |
+| Code                                                       | Description                                                |
+| ---------------------------------------------------------- | ---------------------------------------------------------- |
+| [TRY002](TRY002.md)                                        | Create your own exception                                  |
+| [TRY003](TRY003.md)                                        | Avoid specifying long messages outside the exception class |
+| [TRY004](TRY004.md)                                        | Prefer `TypeError` exception for invalid type              |
+| [TRY005](TRY005.md) (**ENABLED through `check_pickable`**) | Define `__reduce__` to make exception pickable             |
 
 ## `TRY1xx` - General
 

docs/violations/TRY005.md

@@ -0,0 +1,56 @@
+# `TRY005` - Define `__reduce__` to make exception pickable
+
+## Why is it bad
+
+When using multiprocessing (or anything that serializes with Pickle), and you raise an exception [Pickle uses `__reduce__`](https://docs.python.org/2/library/pickle.html#object.__reduce__) to serialize it.
+
+It breaks if your exception takes custom args (not string or not optional).
+
+[Stack Overflow question](https://stackoverflow.com/questions/16244923/how-to-make-a-custom-exception-class-with-multiple-init-args-pickleable)
+
+## How to enable it
+
+Since not every project would care about it, this is an optional violation that can be enabled through `check_pickable`.
+
+## How it looks like
+
+```py
+class ManyArgsMissingReduce(Exception):
+    def __init__(self, val1: str, val2: str) -> None:  # Requires pickable
+        super().__init__(f"{val1} {val2}")
+
+
+class CustomMissingReduce(Exception):
+    def __init__(self, age: int) -> None:  # Requires pickable
+        super().__init__(f"You're not old enough: {age}")
+```
+
+## How it should be
+
+```py
+# Generic implementation:
+class GenericReduceException(Exception)
+    def __init__(self, *args, **kwargs) -> None:
+        self.args = tuple([*args, *kwargs.values()]) # Saves all args/kwargs
+        super().__init__(*args)
+
+    def __reduce__(self) -> str | tuple[Any, ...]:
+        return (self.__class__, self.args) # Return them here
+
+# You can also be a bit more verbose:
+class ManyArgsWITHReduce(Exception):
+    def __init__(self, val1: str, val2: str) -> None:
+        self.val1, self.val2 = val1, val2
+        super().__init__(f"{val1} {val2}")
+
+    def __reduce__(self) -> str | tuple[Any, ...]:
+        return (ManyArgsWITHReduce, (self.val1, self.val2))
+```
+
+
+## When this is fine
+
+This is ok if you don't care about pickable exceptions 🤷 or either if you have an exception:
+
+- Without a custom `__init__` defined; or
+- With `__init__` that receives only one string argument

pyproject.toml

@@ -67,14 +67,15 @@ astpretty = "^3.0.0"
 python-semantic-release = "^7.33.5"
 mypy = "^1.3.0"
 types-toml = "^0.1.3"
+typing-extensions = { version = "^4.5.0", python = "<3.10" }
 
 [tool.black]
 line-length = 100
 
 [tool.isort]
 profile = "black"
 line_length = 100
-extra_standard_library = ["pytest", "toml", "click"]
+extra_standard_library = ["pytest", "toml", "click", "typing_extensions"]
 
 [tool.mypy]
 python_version = 3.8

src/tests/analyzers_class_test.py

@@ -0,0 +1,22 @@
+from functools import partial
+
+from tryceratops import analyzers
+from tryceratops.violations import codes
+
+from .analyzer_helpers import assert_violation, read_sample
+
+
+def test_non_pickable_error():
+    tree = read_sample("class_non_pickable")
+    analyzer = analyzers.classdefs.NonPickableAnalyzer()
+
+    assert_non_pickable = partial(
+        assert_violation, codes.NON_PICKABLE_CLASS[0], codes.NON_PICKABLE_CLASS[1]
+    )
+
+    violations = analyzer.check(tree, "filename")
+
+    assert len(violations) == 2
+
+    assert_non_pickable(24, 0, violations[0])
+    assert_non_pickable(29, 0, violations[1])

src/tests/samples/violations/class_non_pickable.py

@@ -0,0 +1,49 @@
+"""
+Violation:
+
+Implement __reduce__ to enforce class is pickable.
+"""
+
+
+from typing import Any
+
+
+class RandomClass:
+    """This is not even an exception"""
+
+
+class RegularException(Exception):
+    pass
+
+
+class AnotherOkException(Exception):
+    def __init__(self, val1: str) -> None:
+        super().__init__(val1)
+
+
+class ManyArgsMissingReduce(Exception):
+    def __init__(self, val1: str, val2: str) -> None:  # Requires pickable
+        super().__init__(f"{val1} {val2}")
+
+
+class CustomMissingReduce(Exception):
+    def __init__(self, age: int) -> None:  # Requires pickable
+        super().__init__(f"You're not old enough: {age}")
+
+
+class ManyArgsWITHReduce(Exception):
+    def __init__(self, val1: str, val2: str) -> None:
+        self.val1, self.val2 = val1, val2
+        super().__init__(f"{val1} {val2}")
+
+    def __reduce__(self) -> str | tuple[Any, ...]:
+        return (ManyArgsWITHReduce, (self.val1, self.val2))
+
+
+class CustomWITHReduce(Exception):
+    def __init__(self, age: int) -> None:
+        self.age = age
+        super().__init__(f"You're not old enough: {age}")
+
+    def __reduce__(self) -> str | tuple[Any, ...]:
+        return (CustomMissingReduce, (self.age,))

src/tryceratops/analyzers/__init__.py

@@ -1,8 +1,8 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Set, Type, cast
+from typing import TYPE_CHECKING, Set, Type
 
-from . import call, conditional, exception_block, try_block
+from . import call, classdefs, conditional, exception_block, try_block
 from .base import BaseAnalyzer
 
 if TYPE_CHECKING:
@@ -15,6 +15,7 @@
     call.CallRaiseLongArgsAnalyzer,
     call.CallAvoidCheckingToContinueAnalyzer,
     conditional.PreferTypeErrorAnalyzer,
+    classdefs.NonPickableAnalyzer,
     exception_block.ExceptReraiseWithoutCauseAnalyzer,
     exception_block.ExceptVerboseReraiseAnalyzer,
     exception_block.ExceptBroadPassAnalyzer,

src/tryceratops/analyzers/classdefs.py

@@ -0,0 +1,46 @@
+import ast
+import typing as t
+
+from tryceratops.violations import codes
+
+from .base import BaseAnalyzer, visit_error_handler
+
+
+class NonPickableAnalyzer(BaseAnalyzer):
+    violation_code = codes.NON_PICKABLE_CLASS
+
+    def _find_method(self, node: ast.ClassDef, name: str) -> t.Optional[ast.FunctionDef]:
+        for method in node.body:
+            if isinstance(method, ast.FunctionDef) and method.name == name:
+                return method
+
+        return None
+
+    @visit_error_handler
+    def visit_ClassDef(self, node: ast.ClassDef) -> t.Any:
+        is_exc = any([base for base in node.bases if getattr(base, "id") == "Exception"])
+        if is_exc is False:
+            return self.generic_visit(node)
+
+        init_method = self._find_method(node, "__init__")
+        if init_method is None:
+            return self.generic_visit(node)
+
+        reduce_method = self._find_method(node, "__reduce__")
+        if reduce_method is not None:
+            # Good enough to say this is not a violation
+            return self.generic_visit(node)
+
+        # First arg would be self
+        has_more_than_one_arg = len(init_method.args.args) > 1
+        if has_more_than_one_arg is False:
+            return self.generic_visit(node)
+
+        _, second_arg, *remaining_args = init_method.args.args
+        if (
+            len(remaining_args) > 0
+            or second_arg.annotation
+            and getattr(second_arg.annotation, "id") != "str"
+        ):
+            # Pickle would break for non string args or for more than 1 arg
+            self._mark_violation(node)

src/tryceratops/filters.py

@@ -5,6 +5,7 @@
 
 from tryceratops.processors import Processor
 from tryceratops.violations import Violation
+from tryceratops.violations.codes import NON_PICKABLE_CLASS
 
 if TYPE_CHECKING:
     from tryceratops.types import PyprojectConfig
@@ -54,13 +55,17 @@ class GlobalSettings:
     include_experimental: bool
     ignore_violations: Iterable[str]
     exclude_dirs: Iterable[str]
+    check_pickable: bool = False
     autofix: bool = False
 
     def _self_check(self) -> None:
         self.exclude_dirs = [excluded for excluded in self.exclude_dirs if excluded]
 
     def __post_init__(self) -> None:
         self._self_check()
+        if self.check_pickable is False:
+            self.ignore_violations = list(self.ignore_violations)
+            self.ignore_violations.append(NON_PICKABLE_CLASS[0])
 
     @property
     def exclude_experimental(self) -> bool:
@@ -88,8 +93,9 @@ def create_from_config(cls, config: PyprojectConfig) -> GlobalSettings:
         ignore = config.get("ignore", [])
         exclude = config.get("exclude", [])
         autofix = config.get("autofix", False)
+        check_pickable = config.get("check_pickable", False)
 
-        return cls(experimental, ignore, exclude, autofix)
+        return cls(experimental, ignore, exclude, autofix, check_pickable)
 
     def overwrite_from_cli(
         self,

src/tryceratops/types.py

@@ -1,21 +1,23 @@
 import ast
-from typing import Collection, List, Tuple, TypedDict
+import typing as t
+import typing_extensions as te
 
 from tryceratops.filters import FileFilter
 
-ParsedFileType = Tuple[str, ast.AST, FileFilter]
-ParsedFilesType = Collection[ParsedFileType]
+ParsedFileType = t.Tuple[str, ast.AST, FileFilter]
+ParsedFilesType = t.Collection[ParsedFileType]
 
 
-class PyprojectConfig(TypedDict):
+class PyprojectConfig(t.TypedDict):
     """
     Represents the expected pyproject config to be loaded
         exclude: a list of path patterns to be excluded e.g. [/tests, /fixtures]
         ignore: a list of violations to be completely ignored e.g. [TRY002, TRY300]
         experimental: whether to enable experimental analyzers
     """
 
-    exclude: List[str]
-    ignore: List[str]
-    experimental: bool
-    autofix: bool
+    exclude: te.NotRequired[t.List[str]]
+    ignore: te.NotRequired[t.List[str]]
+    experimental: te.NotRequired[bool]
+    autofix: te.NotRequired[bool]
+    check_pickable: te.NotRequired[bool]

src/tryceratops/violations/codes.py

@@ -5,6 +5,7 @@
     "Avoid specifying long messages outside the exception class",
 )
 PREFER_TYPE_ERROR = ("TRY004", "Prefer TypeError exception for invalid type")
+NON_PICKABLE_CLASS = ("TRY005", "Define __reduce__ to make exception pickable")
 
 # TRY1xx - General
 CHECK_TO_CONTINUE = (
@@ -31,16 +32,23 @@
 VERBOSE_LOG_MESSAGE = ("TRY401", "Do not log exception object, give context instead")
 
 CODE_CHOICES = {
+    # TRY0xx - Exceptions
     "TRY002",
     "TRY003",
+    "TRY004",
+    "TRY005",
+    # TRY1xx - General
     "TRY100",
     "TRY101",
+    # TRY2xx - Except
     "TRY200",
     "TRY201",
     "TRY202",
     "TRY203",
+    # TRY3xx - Try
     "TRY300",
     "TRY301",
+    # TRY4xx - Logging
     "TRY400",
     "TRY401",
 }