Skip to content

Commit

Permalink
Adds WPS242 which forbids to have too many cases in match block (#3218
Browse files Browse the repository at this point in the history
)
  • Loading branch information
pojknamn authored Dec 24, 2024
1 parent aa7b453 commit a63be9d
Show file tree
Hide file tree
Showing 12 changed files with 203 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Semantic versioning in our case means:
- Adds a new rule to detect duplicate conditions in `if`s and `elif`s, #2241
- Adds a new rule to detect duplicate `case` pattens in `match`, #3206
- Adds a new rule to find too many `match` subjects, #3201
- Adds a new rule to detect too many `case` statements, #3202
- 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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ norecursedirs = [
# Coverage configuration: https://coverage.readthedocs.io/

# We don't need to cover some files. They are fully checked with mypy.
# And don"t contain any logic.
# And don't contain any logic.
omit = [
# Does not contain runtime logic:
"wemake_python_styleguide/types.py",
Expand Down
19 changes: 19 additions & 0 deletions tests/fixtures/noqa/noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,25 @@ def many_raises_function(parameter): # noqa: WPS238
my_print('except')


match x: # noqa: WPS242
case 1:
my_print('first')
case 2:
my_print('second')
case 3:
my_print('third')
case 4:
my_print('fourth')
case 5:
my_print('fifth')
case 6:
my_print('sixth')
case 7:
my_print('seventh')
case 8:
my_print('eighth')


my_print("""
text
""") # noqa: WPS462
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 @@ -93,6 +93,7 @@
'WPS239': 1,
'WPS240': 0, # only triggers on 3.12+
'WPS241': 1,
'WPS242': 1,
'WPS300': 1,
'WPS301': 1,
'WPS302': 0, # disabled since 1.0.0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import pytest

from wemake_python_styleguide.violations.complexity import (
TooManyMatchCaseViolation,
)
from wemake_python_styleguide.visitors.ast.complexity.pm import (
MatchCasesVisitor,
)

match_cases1 = """
match x:
case _: ...
"""

match_cases4 = """
match x:
case 1: ...
case 2: ...
case 3: ...
case 4: ...
"""

match_cases6 = """
match x:
case 1: ...
case 2: ...
case 3: ...
case 4: ...
case 5: ...
case 6: ...
"""

match_cases7 = """
match x:
case 1: ...
case 2: ...
case 3: ...
case 4: ...
case 5: ...
case 6: ...
case 7: ...
case 8: ...
"""


@pytest.mark.parametrize(
'code',
[
match_cases7,
],
)
def test_match_cases_wrong_count(
assert_errors,
assert_error_text,
parse_ast_tree,
code,
default_options,
):
"""Testing that default settings raise a warning."""
tree = parse_ast_tree(code)

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

assert_errors(visitor, [TooManyMatchCaseViolation])
assert_error_text(visitor, '8', baseline=default_options.max_match_cases)


@pytest.mark.parametrize(
'code',
[
match_cases1,
match_cases6,
],
)
def test_match_cases_correct_count(
assert_errors,
parse_ast_tree,
code,
default_options,
):
"""Testing that default settings do not raise a warning."""
tree = parse_ast_tree(code)

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

assert_errors(visitor, [])


@pytest.mark.parametrize(
'code',
[
match_cases4,
match_cases6,
match_cases7,
],
)
def test_match_cases_configured_count(
assert_errors,
parse_ast_tree,
code,
options,
):
"""Testing that settings can reflect the change."""
tree = parse_ast_tree(code)

option_values = options(max_match_cases=2)
visitor = MatchCasesVisitor(option_values, tree=tree)
visitor.run()

assert_errors(visitor, [TooManyMatchCaseViolation])
8 changes: 8 additions & 0 deletions wemake_python_styleguide/options/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ class of violations that are forbidden to ignore inline, defaults to
- ``max-match-subjects`` - maximum number of subjects in a match statement,
defaults to
:str:`wemake_python_styleguide.options.defaults.MAX_MATCH_SUBJECTS`
- ``max-match-cases`` - maximum number of cases in a match block of code
defaults to
:str:`wemake_python_styleguide.options.defaults.MAX_MATCH_CASES`
.. rubric:: Formatter options
Expand Down Expand Up @@ -431,6 +434,11 @@ class Configuration:
defaults.MAX_MATCH_SUBJECTS,
'Maximum number of subjects in a match statement.',
),
_Option(
'--max-match-cases',
defaults.MAX_MATCH_CASES,
'Maximum number of match cases in a single match.',
),
# Formatter:
_Option(
'--show-violation-links',
Expand Down
3 changes: 3 additions & 0 deletions wemake_python_styleguide/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@
#: Maximum number of subjects in a ``match`` statement.
MAX_MATCH_SUBJECTS: Final = 7 # 7 +- 0, guessed

#: Maximum number of subjects in match statement.
MAX_MATCH_CASES: Final = 7 # guessed

# ==========
# Formatter:
# ==========
Expand Down
1 change: 1 addition & 0 deletions wemake_python_styleguide/options/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class _ValidatedOptions:
max_tuple_unpack_length: int = attr.ib(validator=[_min_max(min=1)])
max_type_params: int = attr.ib(validator=[_min_max(min=1)])
max_match_subjects: int = attr.ib(validator=[_min_max(min=1)])
max_match_cases: int = attr.ib(validator=[_min_max(min=1)])
show_violation_links: bool
exps_for_one_empty_line: int

Expand Down
1 change: 1 addition & 0 deletions wemake_python_styleguide/presets/topics/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@
calls.CallChainsVisitor,
annotations.AnnotationComplexityVisitor,
pm.MatchSubjectsVisitor,
pm.MatchCasesVisitor,
)
3 changes: 3 additions & 0 deletions wemake_python_styleguide/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,6 @@ def exps_for_one_empty_line(self) -> int: ...

@property
def max_match_subjects(self) -> int: ...

@property
def max_match_cases(self) -> int: ...
33 changes: 33 additions & 0 deletions wemake_python_styleguide/violations/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
TooManyExceptExceptionsViolation
TooManyTypeParamsViolation
TooManyMatchSubjectsViolation
TooManyMatchCaseViolation
Module complexity
-----------------
Expand Down Expand Up @@ -107,6 +108,7 @@
.. autoclass:: TooManyExceptExceptionsViolation
.. autoclass:: TooManyTypeParamsViolation
.. autoclass:: TooManyMatchSubjectsViolation
.. autoclass:: TooManyMatchCaseViolation
"""

Expand Down Expand Up @@ -1359,3 +1361,34 @@ class TooManyMatchSubjectsViolation(ASTViolation):

error_template = 'Found too many subjects in `match` statement: {0}'
code = 241


@final
class TooManyMatchCaseViolation(ASTViolation):
"""
Forbids to have too many match cases.
Reasoning:
Too many match cases means that you are probably overly complicate
the object that you are matching right now.
It would be really hard for users
to manually add all match cases.
Solution:
Refactor the ``match`` statement by breaking the logic into smaller,
focused functions. This will improve readability and maintainability.
Split complex logic into separate functions to keep each one concise,
reducing the size of the ``match`` block and making the code easier
to understand and modify.
Configuration:
This rule is configurable with ``--max-match-cases``.
Default:
:str:`wemake_python_styleguide.options.defaults.MAX_MATCH_CASES`
.. versionadded:: 1.0.0
"""

error_template = 'Found too many cases in `match` block: {0}'
code = 242
20 changes: 20 additions & 0 deletions wemake_python_styleguide/visitors/ast/complexity/pm.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,23 @@ def _check_match_subjects_count(self, node: ast.Match) -> None:
baseline=self.options.max_match_subjects,
)
)


@final
class MatchCasesVisitor(BaseNodeVisitor):
"""Finds excessive match cases in `match` statements."""

def visit_Match(self, node: ast.Match) -> None:
"""Finds all `match` statements and checks their cases."""
self._check_match_cases_count(node)
self.generic_visit(node)

def _check_match_cases_count(self, node: ast.Match) -> None:
if len(node.cases) > self.options.max_match_cases:
self.add_violation(
violation=complexity.TooManyMatchCaseViolation(
text=str(len(node.cases)),
node=node,
baseline=self.options.max_match_cases,
),
)

0 comments on commit a63be9d

Please sign in to comment.