Skip to content

Commit

Permalink
feat: implement TRY006
Browse files Browse the repository at this point in the history
  • Loading branch information
guilatrova committed May 20, 2023
1 parent 64f7a9d commit 9635950
Show file tree
Hide file tree
Showing 14 changed files with 194 additions and 42 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ exclude = ["samples"]
ignore = ["TRY002", "TRY200", "TRY300"]
experimental = false
check_pickable = false
allowed_base_exceptions = ["MyAppBase"]
```

CLI flags always overwrite the config file.
Expand Down
13 changes: 7 additions & 6 deletions docs/violations/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

## `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 |
| [TRY005](TRY005.md) (**ENABLED through `check_pickable`**) | Define `__reduce__` to make exception pickable |
| 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 |
| [TRY006](TRY006.md) (**ENABLED through `allowed_base_exceptions`**) | Inheriting from non defined base exception |

## `TRY1xx` - General

Expand Down
5 changes: 5 additions & 0 deletions docs/violations/TRY005.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ It breaks if your exception takes custom args (not string or not optional).

Since not every project would care about it, this is an optional violation that can be enabled through `check_pickable`.

```toml
[tool.tryceratops]
check_pickable = true
```

## How it looks like

```py
Expand Down
34 changes: 34 additions & 0 deletions docs/violations/TRY006.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# `TRY006` - Inheriting from non defined base exception

## Why is it bad

It's a good idea to define a base exception for your project, you can define specific base types that should be used instead of the regular `Exception`.

## How to enable it

You need to define which base exceptions are intended to be used through `allowed_base_exceptions`.

```toml
[tool.tryceratops]
allowed_base_exceptions = ["MyAppException", "CriticalAppException"]
```

## How it looks like

```py
class MyAppException(Exception):
...

class SpecificException(Exception):
...
```

## How it should be

```py
class MyAppException(Exception):
...

class SpecificException(MyAppException): # 👈 Assuming `MyAppException` is defined as allowed
...
```
22 changes: 0 additions & 22 deletions src/tests/analyzers_class_test.py

This file was deleted.

47 changes: 47 additions & 0 deletions src/tests/analyzers_classdefs_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from functools import partial

from tryceratops import analyzers
from tryceratops.settings import GlobalSettings
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])


def test_inherit_from_allowed_exceptions():
tree = read_sample("class_base_allowed")
allowed_base_exceptions = {"AllowedExc", "AlsoAllowed"}
analyzer = analyzers.classdefs.InheritFromBaseAnalyzer(
GlobalSettings(
include_experimental=False,
exclude_dirs=[],
ignore_violations=[],
allowed_base_exceptions=allowed_base_exceptions,
)
)

assert_non_pickable = partial(assert_violation, codes.ALLOWED_BASE_EXCEPTION[0])
msg_base = codes.ALLOWED_BASE_EXCEPTION[1]
allowed_msg = ", ".join(allowed_base_exceptions)

violations = analyzer.check(tree, "filename")

assert len(violations) == 2

assert_non_pickable(msg_base.format("InvalidBase", allowed_msg), 19, 0, violations[0])
assert_non_pickable(msg_base.format("MultiInvalidBase", allowed_msg), 23, 0, violations[1])
32 changes: 32 additions & 0 deletions src/tests/samples/violations/class_base_allowed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""
Violation:
Forbid inherit from Exception if needed
"""


from typing import Any


class AllowedExc(Exception):
pass


class AlsoAllowed(Exception):
pass


class InvalidBase(Exception): # Err
pass


class MultiInvalidBase(ValueError, Exception): # Err
pass


class ThisIsFine(AllowedExc):
pass


class ThisIsAlsoFine(AlsoAllowed):
pass
3 changes: 2 additions & 1 deletion src/tryceratops/analyzers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
call.CallAvoidCheckingToContinueAnalyzer,
conditional.PreferTypeErrorAnalyzer,
classdefs.NonPickableAnalyzer,
classdefs.InheritFromBaseAnalyzer,
exception_block.ExceptReraiseWithoutCauseAnalyzer,
exception_block.ExceptVerboseReraiseAnalyzer,
exception_block.ExceptBroadPassAnalyzer,
Expand All @@ -29,7 +30,7 @@

def get_analyzer_chain(global_settings: GlobalSettings) -> Set[BaseAnalyzer]:
analyzers = {
analyzercls()
analyzercls(global_settings)
for analyzercls in ANALYZER_CLASSES
if global_settings.should_run_processor(analyzercls)
}
Expand Down
22 changes: 12 additions & 10 deletions src/tryceratops/analyzers/base.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
import ast
import typing as t
from abc import ABC, abstractmethod
from typing import Any, Callable, List, Protocol, Type

from tryceratops.processors import Processor
from tryceratops.settings import GlobalSettings
from tryceratops.violations import Violation

from .exceptions import AnalyzerVisitException


class BaseAnalyzer(ABC, Processor, ast.NodeVisitor):
violation_type: Type[Violation] = Violation
violation_type: t.Type[Violation] = Violation

def __init__(self) -> None:
self.violations: List[Violation] = []
def __init__(self, settings: t.Optional[GlobalSettings] = None) -> None:
self.violations: t.List[Violation] = []
self._settings = settings

def _mark_violation(self, *nodes: ast.AST, **kwargs: Any) -> None: # noqa: ANN401
def _mark_violation(self, *nodes: ast.AST, **kwargs: t.Any) -> None: # noqa: ANN401
klass = self.violation_type
for node in nodes:
violation = klass.build(self.filename, self.violation_code, node, **kwargs)
self.violations.append(violation)

def check(self, tree: ast.AST, filename: str) -> List[Violation]:
def check(self, tree: ast.AST, filename: str) -> t.List[Violation]:
self.filename = filename
self.violations = []

Expand All @@ -29,12 +31,12 @@ def check(self, tree: ast.AST, filename: str) -> List[Violation]:
return self.violations


class StmtBodyProtocol(Protocol):
body: List[ast.stmt]
class StmtBodyProtocol(t.Protocol):
body: t.List[ast.stmt]


def visit_error_handler(func: Callable[[BaseAnalyzer, Any], Any]): # noqa: ANN201
def _func(instance: Any, node: ast.stmt) -> None: # noqa: ANN401
def visit_error_handler(func: t.Callable[[BaseAnalyzer, t.Any], t.Any]): # noqa: ANN201
def _func(instance: t.Any, node: ast.stmt) -> None: # noqa: ANN401
try:
return func(instance, node)
except Exception as ex:
Expand Down
21 changes: 21 additions & 0 deletions src/tryceratops/analyzers/classdefs.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import ast
import typing as t

from tryceratops.settings import GlobalSettings
from tryceratops.violations import codes
from tryceratops.violations.violations import InheritFromNonBaseViolation

from .base import BaseAnalyzer, visit_error_handler

Expand Down Expand Up @@ -44,3 +46,22 @@ def visit_ClassDef(self, node: ast.ClassDef) -> t.Any:
):
# Pickle would break for non string args or for more than 1 arg
self._mark_violation(node)


class InheritFromBaseAnalyzer(BaseAnalyzer):
violation_code = codes.ALLOWED_BASE_EXCEPTION
violation_type = InheritFromNonBaseViolation

@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", None) == "Exception"])
if is_exc is False:
return self.generic_visit(node)

settings = t.cast(GlobalSettings, self._settings)
if node.name not in settings.allowed_base_exceptions:
self._mark_violation(
node,
class_name=node.name,
allowed_bases=settings.allowed_base_exceptions,
)
7 changes: 5 additions & 2 deletions src/tryceratops/settings.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import TYPE_CHECKING, Iterable, Optional, Type
from typing import TYPE_CHECKING, Iterable, Optional, Set, Type

from tryceratops.processors import Processor
from tryceratops.violations import Violation
Expand Down Expand Up @@ -55,6 +55,8 @@ class GlobalSettings:
include_experimental: bool
ignore_violations: Iterable[str]
exclude_dirs: Iterable[str]
allowed_base_exceptions: Set[str]

check_pickable: bool = False
autofix: bool = False

Expand Down Expand Up @@ -94,8 +96,9 @@ def create_from_config(cls, config: PyprojectConfig) -> GlobalSettings:
exclude = config.get("exclude", [])
autofix = config.get("autofix", False)
check_pickable = config.get("check_pickable", False)
allowed_base_exceptions = set(config.get("allowed_base_exceptions", set()))

return cls(experimental, ignore, exclude, check_pickable, autofix)
return cls(experimental, ignore, exclude, allowed_base_exceptions, check_pickable, autofix)

def overwrite_from_cli(
self,
Expand Down
1 change: 1 addition & 0 deletions src/tryceratops/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ class PyprojectConfig(t.TypedDict):
experimental: te.NotRequired[bool]
autofix: te.NotRequired[bool]
check_pickable: te.NotRequired[bool]
allowed_base_exceptions: te.NotRequired[bool]
5 changes: 5 additions & 0 deletions src/tryceratops/violations/codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
)
PREFER_TYPE_ERROR = ("TRY004", "Prefer TypeError exception for invalid type")
NON_PICKABLE_CLASS = ("TRY005", "Define __reduce__ to make exception pickable")
ALLOWED_BASE_EXCEPTION = (
"TRY006",
"Class '{}' should inherit from any of your defined base exceptions: {}",
)

# TRY1xx - General
CHECK_TO_CONTINUE = (
Expand Down Expand Up @@ -37,6 +41,7 @@
"TRY003",
"TRY004",
"TRY005",
"TRY006",
# TRY1xx - General
"TRY100",
"TRY101",
Expand Down
23 changes: 22 additions & 1 deletion src/tryceratops/violations/violations.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import ast
from dataclasses import dataclass
from typing import Any, Optional, Tuple
from typing import Any, Optional, Set, Tuple


@dataclass
Expand Down Expand Up @@ -68,3 +68,24 @@ def build(
return cls(
code, node.lineno, node.col_offset, msg, filename, node, except_node, exception_name
)


@dataclass
class InheritFromNonBaseViolation(Violation):
@classmethod
def build(
cls,
filename: str,
vio_details: Tuple[str, str],
node: ast.AST,
class_name: Optional[str] = None,
allowed_bases: Optional[Set[str]] = None,
*args: Any, # noqa: ANN401
**kwargs: Any, # noqa: ANN401
) -> InheritFromNonBaseViolation:
code, msg_base = vio_details

allowed_msg = ", ".join(allowed_bases or set())
msg = msg_base.format(class_name or "", allowed_msg)

return cls(code, node.lineno, node.col_offset, msg, filename, node)

0 comments on commit 9635950

Please sign in to comment.