Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,6 @@ fabric.properties
# Used for testing:
ex.py
setup.py

# CodeRabbit logs:
logs/
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import pytest

from wemake_python_styleguide.violations.consistency import (
SimplifiableMatchWithSequenceOrMappingViolation,
)
from wemake_python_styleguide.visitors.ast.conditions import (
SimplifiableMatchWithSequenceOrMappingVisitor,
)

# Wrong: Simple sequence patterns
simple_list_match = """
match data:
case [1, 2]:
handle_pair()
case _:
ignore()
"""

simple_nested_list_match = """
match data:
case [[1, 2], [3, 4]]:
handle_nested()
case _:
ignore()
"""

simple_tuple_match = """
match data:
case (1, "a"):
handle_tuple()
case _:
ignore()
"""

simple_dict_match = """
match data:
case {"key": "value"}:
handle_dict()
case _:
ignore()
"""

simple_nested_dict_match = """
match data:
case {"outer": {"inner": 42}}:
handle_nested_dict()
case _:
ignore()
"""

mixed_list_dict_match = """
match data:
case [{"key": "value"}, 42]:
handle_mixed()
case _:
ignore()
"""

# Correct: Complex patterns that should not be simplified
complex_with_binding = """
match data:
case [x, y]:
handle_with_binding(x, y)
case _:
ignore()
"""

with_star_pattern = """
match data:
case [first, *rest]:
handle_with_rest(first, rest)
case _:
ignore()
"""

with_rest_name = """
match data:
case {"key": value, **rest}:
handle_with_rest(value, rest)
case _:
ignore()
"""

with_guard = """
match data:
case [1, 2] if condition > 0:
handle_guarded()
case _:
ignore()
"""

complex_match = """
match data:
case SomeClass(x):
handle_class()
case _:
ignore()
"""

no_wildcard = """
match data:
case [1, 2]:
handle_first()
case [3, 4]:
handle_second()
"""

more_than_two_cases = """
match data:
case [1, 2]:
handle_first()
case [3, 4]:
handle_second()
case _:
handle_default()
"""


@pytest.mark.parametrize(
'code',
[
simple_list_match,
simple_nested_list_match,
simple_tuple_match,
simple_dict_match,
simple_nested_dict_match,
mixed_list_dict_match,
],
)
def test_simplifiable_sequence_or_mapping_match(
code,
assert_errors,
parse_ast_tree,
default_options,
):
"""Test that simple sequence and mapping matches raise a violation."""
tree = parse_ast_tree(code)
visitor = SimplifiableMatchWithSequenceOrMappingVisitor(
default_options, tree=tree
)
visitor.run()
assert_errors(visitor, [SimplifiableMatchWithSequenceOrMappingViolation])


@pytest.mark.parametrize(
'template',
[
complex_with_binding,
with_star_pattern,
with_rest_name,
with_guard,
complex_match,
no_wildcard,
more_than_two_cases,
],
)
def test_not_simplifiable_sequence_or_mapping_match(
template,
assert_errors,
parse_ast_tree,
default_options,
):
"""Test that complex or non-simplifiable matches do not raise violations."""
tree = parse_ast_tree(template)
visitor = SimplifiableMatchWithSequenceOrMappingVisitor(
default_options, tree=tree
)
visitor.run()
assert_errors(visitor, [])
93 changes: 93 additions & 0 deletions wemake_python_styleguide/logic/tree/pattern_matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,96 @@ def is_irrefutable_binding(pattern: ast.pattern) -> bool:
and pattern.pattern is None
and pattern.name is not None
)


def is_simple_sequence_or_mapping_pattern(pattern: ast.pattern) -> bool:
"""
Checks if a pattern is a simple sequence or mapping without
variable bindings.

Supports:
- Simple lists: ``[1, 2]``, ``["a", "b"]``, ``[const.A, const.B]``
- Simple tuples: ``(1, 2)``, ``("a", "b")``
- Simple dicts: ``{"key": "value"}``, ``{1: 2}``
- No variable bindings (like ``[x, y]``) or starred patterns
(like ``[first, *rest]``)
- No guards allowed
"""
# Check for simple list or tuple patterns
if isinstance(pattern, ast.MatchSequence):
# Check that there are no starred patterns (*rest)
if not _has_star_pattern(pattern):
# Check that all elements are simple patterns (not binding vars)
return all(
_is_simple_pattern_element(element)
for element in pattern.patterns
)

# Check for simple dict patterns
elif isinstance(pattern, ast.MatchMapping):
# Check that all keys are simple (not variables) and all
# values are simple patterns
if any(
key is None or not _is_simple_key_pattern(key)
for key in pattern.keys
):
return False
if pattern.patterns and any(
not _is_simple_pattern_element(value) for value in pattern.patterns
):
return False
# If rest name is present (has variable binding), it's not simple
return pattern.rest is None

return False


def _has_star_pattern(pattern: ast.MatchSequence) -> bool:
"""Check if the sequence pattern contains starred patterns."""
# ast.MatchStar has been added in Python 3.10 for starred patterns
for sub_pattern in pattern.patterns:
if isinstance(sub_pattern, ast.MatchStar):
return True
return False


def _is_simple_pattern_element(pattern: ast.pattern) -> bool:
"""Check if a pattern element is simple (not binding variables)."""
# Handle simple value patterns (literals, constants, attributes)
if isinstance(pattern, ast.MatchValue):
return isinstance(
pattern.value, (ast.Constant, ast.Name, ast.Attribute)
)

# Handle simple singleton patterns (True, False, None)
if isinstance(pattern, ast.MatchSingleton):
return True

# Handle simple nested patterns
if isinstance(pattern, (ast.MatchSequence, ast.MatchMapping)):
return is_simple_sequence_or_mapping_pattern(pattern)

# Handle Union patterns (| operator)
if isinstance(pattern, ast.MatchOr):
return all(_is_simple_pattern_element(sub) for sub in pattern.patterns)

# Handle MatchAs patterns - not simple if it's binding a variable
if isinstance(pattern, ast.MatchAs):
# If pattern.name is not None, it's a binding (not simple)
if pattern.name is not None:
return False
# If pattern.name is None but pattern.pattern is not None,
# check if the inner pattern is simple
if pattern.pattern is not None:
return _is_simple_pattern_element(pattern.pattern)
# This case should not happen in valid Python ASTs
return False

# Other pattern types are not simple (like MatchClass with constructors)
return False


def _is_simple_key_pattern(key: ast.expr) -> bool:
"""Check if a mapping key is simple (not a variable)."""
# Keys should be constants or attributes, not variable names
return isinstance(key, (ast.Constant, ast.Name, ast.Attribute))
1 change: 1 addition & 0 deletions wemake_python_styleguide/presets/types/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
conditions.MatchVisitor,
conditions.ChainedIsVisitor,
conditions.SimplifiableMatchVisitor,
conditions.SimplifiableMatchWithSequenceOrMappingVisitor,
conditions.LeakingForLoopVisitor,
iterables.IterableUnpackingVisitor,
blocks.AfterBlockVariablesVisitor,
Expand Down
51 changes: 51 additions & 0 deletions wemake_python_styleguide/violations/consistency.py
Original file line number Diff line number Diff line change
Expand Up @@ -2460,3 +2460,54 @@ class SimplifiableMatchViolation(ASTViolation):
'Found simplifiable `match` statement that can be just `if`'
)
code = 365


@final
class SimplifiableMatchWithSequenceOrMappingViolation(ASTViolation):
"""
Some ``match`` statements with simple sequences or mappings can be.

Reasoning:
Using ``match`` for exact structural comparisons of simple literals
(like lists or dicts) is unnecessarily verbose. While match excels
at deconstruction, using it to check for an exact list or dict value
is better expressed with a direct equality comparison (``==``),
which is more readable and performant.

Solution:
Replace ``match`` statements that check for simple sequences or
mappings (with no deconstruction) with ``if`` statements using ``==``.

When is this violation is raised?
- When there are exactly two ``case`` statements
- When the first case uses a simple sequence or mapping pattern
- When the second case is a wildcard: ``case _:``
- When the pattern contains only literals/contants (no
variable bindings)
- When there are no guards or starred patterns


Example::

# Correct:
if data == [1, 2]:
handle_pair()
else:
ignore()

# Wrong:
match data:
case [1, 2]:
handle_pair()
case _:
ignore()

.. versionadded:: 1.5.0

"""

error_template = (
'Found simplifiable `match` statement with sequence or mapping '
'that can be just `if`'
)
code = 366
32 changes: 32 additions & 0 deletions wemake_python_styleguide/visitors/ast/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,38 @@ def _check_simplifiable_match(self, node: ast.Match) -> None:
self.add_violation(consistency.SimplifiableMatchViolation(node))


@final
class SimplifiableMatchWithSequenceOrMappingVisitor(BaseNodeVisitor):
"""Checks for match statements with simple sequences and mappings."""

def visit_Match(self, node: ast.Match) -> None:
"""Checks match statements with simple sequences and mappings."""
self._check_simplifiable_match_seq_or_map(node)
self.generic_visit(node)

def _check_simplifiable_match_seq_or_map(self, node: ast.Match) -> None:
cases = node.cases
if len(cases) == 2:
first, second = cases

if not pattern_matching.is_wildcard_pattern(second):
return

# Check if the first pattern is a simple sequence or mapping pattern
# without variable bindings AND no guard is present
if (
pattern_matching.is_simple_sequence_or_mapping_pattern(
first.pattern
)
and first.guard is None # No guard clause
):
self.add_violation(
consistency.SimplifiableMatchWithSequenceOrMappingViolation(
node
)
)


@final
class LeakingForLoopVisitor(BaseNodeVisitor):
"""Finds 'for' loops directly inside class or module bodies."""
Expand Down
Loading