Skip to content

Commit

Permalink
Adds WPS241 which forbids to have too many subjects in match stat…
Browse files Browse the repository at this point in the history
…ements (#3214)

Co-authored-by: sobolevn <[email protected]>
  • Loading branch information
anywindblows and sobolevn authored Dec 24, 2024
1 parent a47422d commit 70b018e
Show file tree
Hide file tree
Showing 15 changed files with 292 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Semantic versioning in our case means:
- Adds a new rule to check problematic function params, #1343
- 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 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
5 changes: 5 additions & 0 deletions tests/fixtures/noqa/noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,11 @@ def many_raises_function(parameter): # noqa: WPS238
my_print('except')


match inst1, inst2, inst3, inst4, inst5, inst6, inst7, inst8: # noqa: WPS241
case 1:
my_print('except')


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 @@ -92,6 +92,7 @@
'WPS238': 1,
'WPS239': 1,
'WPS240': 0, # only triggers on 3.12+
'WPS241': 1,
'WPS300': 1,
'WPS301': 1,
'WPS302': 0, # disabled since 1.0.0
Expand Down
90 changes: 79 additions & 11 deletions tests/test_visitors/test_ast/test_compares/test_conditionals.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@
WrongConditionalVisitor,
)

create_variable = """
variable = 1
{0}
"""

if_statement = 'if {0}: ...'
ternary = 'ternary = 0 if {0} else 1'

Expand Down Expand Up @@ -83,9 +78,7 @@ def test_valid_conditional(
mode,
):
"""Testing that conditionals work well."""
tree = parse_ast_tree(
mode(create_variable.format(code.format(comparators))),
)
tree = parse_ast_tree(mode(code.format(comparators)))

visitor = WrongConditionalVisitor(default_options, tree=tree)
visitor.run()
Expand All @@ -102,7 +95,6 @@ def test_valid_conditional(
set_comprehension,
dict_comprehension,
gen_comprehension,
match_statement,
],
)
@pytest.mark.parametrize(
Expand All @@ -120,7 +112,9 @@ def test_valid_conditional(
'{test : "1"}',
'{"set"}',
'("tuple",)',
'["list"]',
'[]',
'[variable]',
'(1, var)',
'variable or False',
'variable and False',
'variable or True',
Expand All @@ -139,11 +133,85 @@ def test_constant_condition(
mode,
):
"""Testing that violations are when using invalid conditional."""
tree = parse_ast_tree(mode(code.format(comparators)))

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

assert_errors(visitor, [ConstantConditionViolation])


@pytest.mark.parametrize(
'comparators',
[
'True',
'False',
'None',
'4',
'-4.8',
'--0.0',
'"test"',
"b'bytes'",
'("string in brackets")',
'{1 : "1"}',
'{"set"}',
'("tuple",)',
'[]',
'[1, 2]',
'(1, 2)',
'variable or False',
'variable and False',
'variable or True',
'variable and True',
'(unique := True)',
'(unique := -1)',
'...',
],
)
def test_constant_condition_in_match(
assert_errors,
parse_ast_tree,
comparators,
default_options,
):
"""Testing that violations are when using invalid conditional in PM."""
tree = parse_ast_tree(
mode(create_variable.format(code.format(comparators))),
match_statement.format(comparators),
)

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

assert_errors(visitor, [ConstantConditionViolation])


@pytest.mark.parametrize(
'comparators',
[
'variable',
'(x := y)',
'-number',
'[x, y]',
'{test : "1"}',
'{test}',
'(x, y)',
'{**keys, "data": None}',
'variable or other',
'variable and other',
],
)
def test_regular_condition_in_match(
assert_errors,
parse_ast_tree,
comparators,
default_options,
):
"""Testing correct conditional in PM."""
tree = parse_ast_tree(
match_statement.format(comparators),
)

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

assert_errors(visitor, [])
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import pytest

from wemake_python_styleguide.violations.complexity import (
TooManyMatchSubjectsViolation,
)
from wemake_python_styleguide.visitors.ast.complexity.pm import (
MatchSubjectsVisitor,
)

match_call_expr = """
match some_call():
case 1: ...
"""

match_case_long_tuple = """
match some_call['a']:
case (a, b, c, d, e, f, g, h, i): ...
"""

match_subjects8 = """
match a1, b2, c3, d4, e5, f6, g7, h8:
case 1: ...
"""

match_subjects7 = """
match a1, b2, c3, d4, e5, f6, g7:
case 1: ...
"""


def test_match_subjects_wrong_count(
assert_errors,
assert_error_text,
parse_ast_tree,
default_options,
):
"""Testing that default settings raise a warning."""
tree = parse_ast_tree(match_subjects8)

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

assert_errors(visitor, [TooManyMatchSubjectsViolation])
assert_error_text(visitor, '8', baseline=default_options.max_match_subjects)


@pytest.mark.parametrize(
'code',
[
match_subjects7,
match_call_expr,
match_case_long_tuple,
],
)
def test_match_subjects_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 = MatchSubjectsVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])


@pytest.mark.parametrize(
'code',
[
match_subjects8,
match_subjects7,
],
)
def test_match_subjects_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_subjects=1)
visitor = MatchSubjectsVisitor(option_values, tree=tree)
visitor.run()

assert_errors(visitor, [TooManyMatchSubjectsViolation])
3 changes: 1 addition & 2 deletions wemake_python_styleguide/logic/tree/compares.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
import types
from collections import defaultdict
from collections.abc import Mapping
from typing import Final, TypeAlias
from typing import Final, TypeAlias, final

import attr
from typing_extensions import final

from wemake_python_styleguide.logic import source

Expand Down
24 changes: 24 additions & 0 deletions wemake_python_styleguide/logic/tree/pattern_matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
from collections.abc import Iterable

from wemake_python_styleguide.logic.nodes import get_parent
from wemake_python_styleguide.logic.tree import (
operators,
)
from wemake_python_styleguide.logic.walk import get_subnodes_by_type
from wemake_python_styleguide.logic.walrus import get_assigned_expr


def get_explicit_as_names(
Expand All @@ -21,3 +25,23 @@ def get_explicit_as_names(
and match_as.name
):
yield match_as


def is_constant_subject(condition: ast.AST | list[ast.expr]) -> bool:
"""Detect constant subjects for `ast.Match` nodes."""
if isinstance(condition, list):
return all(is_constant_subject(node) for node in condition)
node = operators.unwrap_unary_node(get_assigned_expr(condition))
if isinstance(node, ast.Constant):
return True
if isinstance(node, ast.Tuple | ast.List | ast.Set):
return is_constant_subject(node.elts)
if isinstance(node, ast.Dict):
return (
not any(dict_key is None for dict_key in node.keys)
and is_constant_subject([
dict_key for dict_key in node.keys if dict_key is not None
])
and is_constant_subject(node.values)
)
return False
8 changes: 8 additions & 0 deletions wemake_python_styleguide/options/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ class of violations that are forbidden to ignore inline, defaults to
- ``max-type-params`` - maximum number of PEP695 type parameters,
defaults to
:str:`wemake_python_styleguide.options.defaults.MAX_TYPE_PARAMS`
- ``max-match-subjects`` - maximum number of subjects in a match statement,
defaults to
:str:`wemake_python_styleguide.options.defaults.MAX_MATCH_SUBJECTS`
.. rubric:: Formatter options
Expand Down Expand Up @@ -423,6 +426,11 @@ class Configuration:
defaults.MAX_TYPE_PARAMS,
'Maximum number of PEP695 type parameters.',
),
_Option(
'--max-match-subjects',
defaults.MAX_MATCH_SUBJECTS,
'Maximum number of subjects in a match statement.',
),
# Formatter:
_Option(
'--show-violation-links',
Expand Down
5 changes: 4 additions & 1 deletion wemake_python_styleguide/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@
MAX_TUPLE_UNPACK_LENGTH: Final = 4 # guessed

#: Maximum number of PEP695 type parameters.
MAX_TYPE_PARAMS: Final = 6 # guessed
MAX_TYPE_PARAMS: Final = 6 # 7-1, guessed

#: Maximum number of subjects in a ``match`` statement.
MAX_MATCH_SUBJECTS: Final = 7 # 7 +- 0, 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 @@ -98,6 +98,7 @@ class _ValidatedOptions:
max_import_from_members: int = attr.ib(validator=[_min_max(min=1)])
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)])
show_violation_links: bool
exps_for_one_empty_line: int

Expand Down
2 changes: 2 additions & 0 deletions wemake_python_styleguide/presets/topics/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
nested,
offset,
overuses,
pm,
)

#: Used to store all complexity related visitors to be later passed to checker:
Expand All @@ -36,4 +37,5 @@
access.AccessVisitor,
calls.CallChainsVisitor,
annotations.AnnotationComplexityVisitor,
pm.MatchSubjectsVisitor,
)
3 changes: 3 additions & 0 deletions wemake_python_styleguide/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,6 @@ def show_violation_links(self) -> bool: ...

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

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

0 comments on commit 70b018e

Please sign in to comment.