Skip to content

Commit

Permalink
Review
Browse files Browse the repository at this point in the history
  • Loading branch information
sobolevn committed Dec 24, 2024
1 parent 944265b commit 58ce2f3
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 43 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +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 many `match` subjects, #3201
- 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
2 changes: 1 addition & 1 deletion tests/test_checker/test_noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
'WPS238': 1,
'WPS239': 1,
'WPS240': 0, # only triggers on 3.12+
'WPS241': 0,
'WPS241': 1,
'WPS300': 1,
'WPS301': 1,
'WPS302': 0, # disabled since 1.0.0
Expand Down
80 changes: 78 additions & 2 deletions tests/test_visitors/test_ast/test_compares/test_conditionals.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ def test_valid_conditional(
set_comprehension,
dict_comprehension,
gen_comprehension,
match_statement,
],
)
@pytest.mark.parametrize(
Expand All @@ -120,7 +119,8 @@ def test_valid_conditional(
'{test : "1"}',
'{"set"}',
'("tuple",)',
'["list"]',
'[]',
'[variable]',
'variable or False',
'variable and False',
'variable or True',
Expand All @@ -147,3 +147,79 @@ def test_constant_condition(
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]',
'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,
mode,
):
"""Testing that violations are when using invalid conditional in PM."""
tree = parse_ast_tree(
mode(create_variable.format(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}',
'{**keys, "data": None}',
'variable or other',
'variable and other',
],
)
def test_regular_condition_in_match(
assert_errors,
parse_ast_tree,
comparators,
default_options,
mode,
):
"""Testing correct conditional in PM."""
tree = parse_ast_tree(
mode(create_variable.format(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
Expand Up @@ -7,40 +7,50 @@
MatchSubjectsVisitor,
)

match_subjects9 = """
match a, b, c, d, e, f, g, h, i:
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 a, b, c, d, e, f, g, h:
match a1, b2, c3, d4, e5, f6, g7, h8:
case 1: ...
"""

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


@pytest.mark.parametrize(
'code',
[match_subjects9],
)
def test_match_subjects_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)
tree = parse_ast_tree(match_subjects8)

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

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


@pytest.mark.parametrize(
'code',
[match_subjects8],
[
match_subjects7,
match_call_expr,
match_case_long_tuple,
],
)
def test_match_subjects_correct_count(
assert_errors,
Expand All @@ -60,8 +70,8 @@ def test_match_subjects_correct_count(
@pytest.mark.parametrize(
'code',
[
match_subjects9,
match_subjects8,
match_subjects7,
],
)
def test_match_subjects_configured_count(
Expand All @@ -73,7 +83,7 @@ def test_match_subjects_configured_count(
"""Testing that settings can reflect the change."""
tree = parse_ast_tree(code)

option_values = options(max_match_subjects=3)
option_values = options(max_match_subjects=1)
visitor = MatchSubjectsVisitor(option_values, tree=tree)
visitor.run()

Expand Down
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
4 changes: 2 additions & 2 deletions wemake_python_styleguide/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +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 = 8 # guessed
MAX_MATCH_SUBJECTS: Final = 7 # 7 +- 0, guessed

# ==========
# Formatter:
Expand Down
19 changes: 15 additions & 4 deletions wemake_python_styleguide/visitors/ast/compares.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
compares,
functions,
operators,
pattern_matching,
)
from wemake_python_styleguide.logic.walrus import get_assigned_expr
from wemake_python_styleguide.types import AnyIf, AnyNodes
Expand Down Expand Up @@ -202,17 +203,27 @@ def visit_comprehension(self, node: ast.comprehension) -> None:

def visit_Match(self, node: ast.Match) -> None:
"""Ensures that ``match`` nodes are using valid conditionals."""
self._check_constant_condition(node.subject)
self._check_constant_condition(node.subject, is_match=True)
self.generic_visit(node)

def _check_constant_condition(self, node: ast.AST) -> None:
def _check_constant_condition(
self,
node: ast.AST,
*,
is_match: bool = False,
) -> None:
if isinstance(node, ast.BoolOp):
for condition in node.values:
self._check_constant_condition(condition)
else:
real_node = operators.unwrap_unary_node(get_assigned_expr(node))
if isinstance(real_node, self._forbidden_nodes):
self.add_violation(ConstantConditionViolation(node))
if is_match and not pattern_matching.is_constant_subject(real_node):
return
if not is_match and not isinstance(
real_node, self._forbidden_nodes
):
return
self.add_violation(ConstantConditionViolation(node))

def _check_nested_ifexpr(self, node: AnyIf) -> None:
is_nested_in_if = bool(
Expand Down
31 changes: 13 additions & 18 deletions wemake_python_styleguide/visitors/ast/complexity/pm.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,28 @@
import ast

from typing_extensions import final
from typing import final

from wemake_python_styleguide.violations import complexity
from wemake_python_styleguide.visitors.base import BaseNodeVisitor
from wemake_python_styleguide.visitors.decorators import alias


@final
@alias(
'visit_match',
('visit_Match',),
)
class MatchSubjectsVisitor(BaseNodeVisitor):
"""Finds excessive match subjects in `match` statements."""

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

def _check_match_subjects_count(self, node: ast.Match) -> None:
if isinstance(node.subject, ast.Tuple):
elts = node.subject.elts
if len(elts) > self.options.max_match_subjects:
self.add_violation(
complexity.TooManyMatchSubjectsViolation(
node,
text=str(len(elts)),
baseline=self.options.max_match_subjects,
)
)
if not isinstance(node.subject, ast.Tuple):
return
if len(node.subject.elts) <= self.options.max_match_subjects:
return
self.add_violation(
complexity.TooManyMatchSubjectsViolation(
node,
text=str(len(node.subject.elts)),
baseline=self.options.max_match_subjects,
)
)

0 comments on commit 58ce2f3

Please sign in to comment.