Skip to content

Commit

Permalink
Forbids raise SystemError, closes #1786
Browse files Browse the repository at this point in the history
  • Loading branch information
sobolevn committed Dec 22, 2024
1 parent c377862 commit ae3e4c5
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ Semantic versioning in our case means:
- Adds a new rule to find too complex `except` with too many exceptions
- Adds a new rule to find too many `PEP695` type params
- Adds a new rule to find useless ternary expressions, #1706
- Adds a new rule to forbid `raise SystemError`, use `sys.exit` instead, #1786
- Adds support to run `wemake-python-styleguide` as a `pre-commit` hook, #2588
- GitHub Action can now use `cwd:` parameter to specify
where your configuration file is, #2474
Expand Down
4 changes: 4 additions & 0 deletions tests/fixtures/noqa/noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,10 @@ def set_attribute(self): # noqa: WPS615
a_list[slice(1)] = [1, 2] # noqa: WPS362


def function_with_systemerror():
raise SystemError(1) # noqa: WPS363


try:
cause_errors()
except ValueError or TypeError: # noqa: WPS455
Expand Down
1 change: 1 addition & 0 deletions tests/test_checker/test_noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
'WPS360': 0, # disabled since 1.0.0
'WPS361': 0, # disabled since 1.0.0
'WPS362': 2,
'WPS363': 1,
'WPS400': 0, # defined in ignored violations.
'WPS401': 0, # logically unacceptable.
'WPS402': 0, # defined in ignored violations.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import pytest

from wemake_python_styleguide.violations.consistency import (
RaiseSystemErrorViolation,
)
from wemake_python_styleguide.visitors.ast.keywords import WrongRaiseVisitor

template = 'raise {0}'


@pytest.mark.parametrize(
'code',
[
'SystemError',
'SystemError()',
'SystemError(0)',
'SystemError(code)',
],
)
def test_raise_system_error(
assert_errors,
parse_ast_tree,
code,
default_options,
):
"""Testing `raise SystemError` is restricted."""
tree = parse_ast_tree(template.format(code))

visitor = WrongRaiseVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [RaiseSystemErrorViolation])


@pytest.mark.parametrize(
'code',
[
'NotImplementedError',
'NotImplementedError()',
'CustomSystemError',
'CustomSystemError()',
'custom.SystemError',
'custom.SystemError()',
],
)
def test_raise_good_errors(
assert_errors,
parse_ast_tree,
code,
default_options,
):
"""Testing that other exceptions are allowed."""
tree = parse_ast_tree(template.format(code))

visitor = WrongRaiseVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])
5 changes: 2 additions & 3 deletions wemake_python_styleguide/logic/tree/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ def get_exception_name(node: ast.Raise) -> str | None:
if exception is None:
return None

exception_func = getattr(exception, 'func', None)
if exception_func:
exception = exception_func
if isinstance(exception, ast.Call):
exception = exception.func

return getattr(exception, 'id', None)

Expand Down
29 changes: 29 additions & 0 deletions wemake_python_styleguide/violations/consistency.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
RawStringNotNeededViolation
InconsistentComprehensionViolation
AssignToSliceViolation
RaiseSystemErrorViolation
Consistency checks
------------------
Expand Down Expand Up @@ -153,6 +154,7 @@
.. autoclass:: RawStringNotNeededViolation
.. autoclass:: InconsistentComprehensionViolation
.. autoclass:: AssignToSliceViolation
.. autoclass:: RaiseSystemErrorViolation
"""

Expand Down Expand Up @@ -2473,3 +2475,30 @@ class AssignToSliceViolation(ASTViolation):

error_template = 'Found assignment to a subscript slice'
code = 362


@final
class RaiseSystemErrorViolation(ASTViolation):
"""
Forbid raising :exc:`SystemError`.
Reasoning:
For consistency.
Solution:
Use :func:`sys.exit`.
Example::
# Correct:
sys.exit(code)
# Wrong:
raise SystemError(code)
.. versionadded:: 1.0.0
"""

error_template = 'Found `raise SystemError`, instead of using `sys.exit`'
code = 363
17 changes: 13 additions & 4 deletions wemake_python_styleguide/visitors/ast/keywords.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
InconsistentReturnViolation,
InconsistentYieldViolation,
IncorrectYieldFromTargetViolation,
RaiseSystemErrorViolation,
)
from wemake_python_styleguide.visitors.base import BaseNodeVisitor
from wemake_python_styleguide.visitors.decorators import alias
Expand All @@ -43,25 +44,33 @@
class WrongRaiseVisitor(BaseNodeVisitor):
"""Finds wrong ``raise`` keywords."""

_system_error_name: ClassVar[str] = 'SystemError'

def visit_Raise(self, node: ast.Raise) -> None:
"""Checks how ``raise`` keyword is used."""
self._check_bare_raise(node)
self._check_raise_from_itself(node)
self._check_raise_system_error(node)
self.generic_visit(node)

def _check_bare_raise(self, node: ast.Raise) -> None:
if node.exc is None:
parent_except = walk.get_closest_parent(node, ast.ExceptHandler)
if node.exc is not None:
return
if walk.get_closest_parent(node, ast.ExceptHandler):
return

if not parent_except:
self.add_violation(BareRaiseViolation(node))
self.add_violation(BareRaiseViolation(node))

def _check_raise_from_itself(self, node: ast.Raise) -> None:
raising_name = get_exception_name(node)
names_are_same = raising_name == get_cause_name(node)
if raising_name is not None and names_are_same:
self.add_violation(RaiseFromItselfViolation(node))

def _check_raise_system_error(self, node: ast.Raise) -> None:
if get_exception_name(node) == self._system_error_name:
self.add_violation(RaiseSystemErrorViolation(node))


@final
@alias(
Expand Down

0 comments on commit ae3e4c5

Please sign in to comment.