Skip to content

Commit 9bbfa8c

Browse files
author
SoulSniper1212
committed
feat: add check for simplifiable match with sequence or mapping patterns
Signed-off-by: SoulSniper1212 <[email protected]>
1 parent 613fbd1 commit 9bbfa8c

File tree

6 files changed

+341
-0
lines changed

6 files changed

+341
-0
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,3 +212,6 @@ fabric.properties
212212
# Used for testing:
213213
ex.py
214214
setup.py
215+
216+
# CodeRabbit logs:
217+
logs/
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import pytest
2+
3+
from wemake_python_styleguide.violations.consistency import (
4+
SimplifiableMatchWithSequenceOrMappingViolation,
5+
)
6+
from wemake_python_styleguide.visitors.ast.conditions import (
7+
SimplifiableMatchWithSequenceOrMappingVisitor,
8+
)
9+
10+
# Wrong: Simple sequence patterns
11+
simple_list_match = """
12+
match data:
13+
case [1, 2]:
14+
handle_pair()
15+
case _:
16+
ignore()
17+
"""
18+
19+
simple_nested_list_match = """
20+
match data:
21+
case [[1, 2], [3, 4]]:
22+
handle_nested()
23+
case _:
24+
ignore()
25+
"""
26+
27+
simple_tuple_match = """
28+
match data:
29+
case (1, "a"):
30+
handle_tuple()
31+
case _:
32+
ignore()
33+
"""
34+
35+
simple_dict_match = """
36+
match data:
37+
case {"key": "value"}:
38+
handle_dict()
39+
case _:
40+
ignore()
41+
"""
42+
43+
simple_nested_dict_match = """
44+
match data:
45+
case {"outer": {"inner": 42}}:
46+
handle_nested_dict()
47+
case _:
48+
ignore()
49+
"""
50+
51+
mixed_list_dict_match = """
52+
match data:
53+
case [{"key": "value"}, 42]:
54+
handle_mixed()
55+
case _:
56+
ignore()
57+
"""
58+
59+
# Correct: Complex patterns that should not be simplified
60+
complex_with_binding = """
61+
match data:
62+
case [x, y]:
63+
handle_with_binding(x, y)
64+
case _:
65+
ignore()
66+
"""
67+
68+
with_star_pattern = """
69+
match data:
70+
case [first, *rest]:
71+
handle_with_rest(first, rest)
72+
case _:
73+
ignore()
74+
"""
75+
76+
with_rest_name = """
77+
match data:
78+
case {"key": value, **rest}:
79+
handle_with_rest(value, rest)
80+
case _:
81+
ignore()
82+
"""
83+
84+
with_guard = """
85+
match data:
86+
case [1, 2] if condition > 0:
87+
handle_guarded()
88+
case _:
89+
ignore()
90+
"""
91+
92+
complex_match = """
93+
match data:
94+
case SomeClass(x):
95+
handle_class()
96+
case _:
97+
ignore()
98+
"""
99+
100+
no_wildcard = """
101+
match data:
102+
case [1, 2]:
103+
handle_first()
104+
case [3, 4]:
105+
handle_second()
106+
"""
107+
108+
more_than_two_cases = """
109+
match data:
110+
case [1, 2]:
111+
handle_first()
112+
case [3, 4]:
113+
handle_second()
114+
case _:
115+
handle_default()
116+
"""
117+
118+
119+
@pytest.mark.parametrize(
120+
'code',
121+
[
122+
simple_list_match,
123+
simple_nested_list_match,
124+
simple_tuple_match,
125+
simple_dict_match,
126+
simple_nested_dict_match,
127+
mixed_list_dict_match,
128+
],
129+
)
130+
def test_simplifiable_sequence_or_mapping_match(
131+
code,
132+
assert_errors,
133+
parse_ast_tree,
134+
default_options,
135+
):
136+
"""Test that simple sequence and mapping matches raise a violation."""
137+
tree = parse_ast_tree(code)
138+
visitor = SimplifiableMatchWithSequenceOrMappingVisitor(default_options, tree=tree)
139+
visitor.run()
140+
assert_errors(visitor, [SimplifiableMatchWithSequenceOrMappingViolation])
141+
142+
143+
@pytest.mark.parametrize(
144+
'template',
145+
[
146+
complex_with_binding,
147+
with_star_pattern,
148+
with_rest_name,
149+
with_guard,
150+
complex_match,
151+
no_wildcard,
152+
more_than_two_cases,
153+
],
154+
)
155+
def test_not_simplifiable_sequence_or_mapping_match(
156+
template,
157+
assert_errors,
158+
parse_ast_tree,
159+
default_options,
160+
):
161+
"""Test that complex or non-simplifiable matches do not raise violations."""
162+
tree = parse_ast_tree(template)
163+
visitor = SimplifiableMatchWithSequenceOrMappingVisitor(default_options, tree=tree)
164+
visitor.run()
165+
assert_errors(visitor, [])

wemake_python_styleguide/logic/tree/pattern_matching.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,96 @@ def is_irrefutable_binding(pattern: ast.pattern) -> bool:
102102
and pattern.pattern is None
103103
and pattern.name is not None
104104
)
105+
106+
107+
def is_simple_sequence_or_mapping_pattern(pattern: ast.pattern) -> bool:
108+
"""
109+
Checks if a pattern is a simple sequence or mapping without
110+
variable bindings.
111+
112+
Supports:
113+
- Simple lists: ``[1, 2]``, ``["a", "b"]``, ``[const.A, const.B]``
114+
- Simple tuples: ``(1, 2)``, ``("a", "b")``
115+
- Simple dicts: ``{"key": "value"}``, ``{1: 2}``
116+
- No variable bindings (like ``[x, y]``) or starred patterns
117+
(like ``[first, *rest]``)
118+
- No guards allowed
119+
"""
120+
# Check for simple list or tuple patterns
121+
if isinstance(pattern, ast.MatchSequence):
122+
# Check that there are no starred patterns (*rest)
123+
if not _has_star_pattern(pattern):
124+
# Check that all elements are simple patterns (not binding vars)
125+
return all(
126+
_is_simple_pattern_element(element)
127+
for element in pattern.patterns
128+
)
129+
130+
# Check for simple dict patterns
131+
elif isinstance(pattern, ast.MatchMapping):
132+
# Check that all keys are simple (not variables) and all
133+
# values are simple patterns
134+
if any(
135+
key is None or not _is_simple_key_pattern(key)
136+
for key in pattern.keys
137+
):
138+
return False
139+
if pattern.patterns and any(
140+
not _is_simple_pattern_element(value) for value in pattern.patterns
141+
):
142+
return False
143+
# If rest name is present (has variable binding), it's not simple
144+
return pattern.rest is None
145+
146+
return False
147+
148+
149+
def _has_star_pattern(pattern: ast.MatchSequence) -> bool:
150+
"""Check if the sequence pattern contains starred patterns."""
151+
# ast.MatchStar has been added in Python 3.10 for starred patterns
152+
for sub_pattern in pattern.patterns:
153+
if isinstance(sub_pattern, ast.MatchStar):
154+
return True
155+
return False
156+
157+
158+
def _is_simple_pattern_element(pattern: ast.pattern) -> bool:
159+
"""Check if a pattern element is simple (not binding variables)."""
160+
# Handle simple value patterns (literals, constants, attributes)
161+
if isinstance(pattern, ast.MatchValue):
162+
return isinstance(
163+
pattern.value, (ast.Constant, ast.Name, ast.Attribute)
164+
)
165+
166+
# Handle simple singleton patterns (True, False, None)
167+
if isinstance(pattern, ast.MatchSingleton):
168+
return True
169+
170+
# Handle simple nested patterns
171+
if isinstance(pattern, (ast.MatchSequence, ast.MatchMapping)):
172+
return is_simple_sequence_or_mapping_pattern(pattern)
173+
174+
# Handle Union patterns (| operator)
175+
if isinstance(pattern, ast.MatchOr):
176+
return all(_is_simple_pattern_element(sub) for sub in pattern.patterns)
177+
178+
# Handle MatchAs patterns - not simple if it's binding a variable
179+
if isinstance(pattern, ast.MatchAs):
180+
# If pattern.name is not None, it's a binding (not simple)
181+
if pattern.name is not None:
182+
return False
183+
# If pattern.name is None but pattern.pattern is not None,
184+
# check if the inner pattern is simple
185+
if pattern.pattern is not None:
186+
return _is_simple_pattern_element(pattern.pattern)
187+
# This case should not happen in valid Python ASTs
188+
return False
189+
190+
# Other pattern types are not simple (like MatchClass with constructors)
191+
return False
192+
193+
194+
def _is_simple_key_pattern(key: ast.expr) -> bool:
195+
"""Check if a mapping key is simple (not a variable)."""
196+
# Keys should be constants or attributes, not variable names
197+
return isinstance(key, (ast.Constant, ast.Name, ast.Attribute))

wemake_python_styleguide/presets/types/tree.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
conditions.MatchVisitor,
7272
conditions.ChainedIsVisitor,
7373
conditions.SimplifiableMatchVisitor,
74+
conditions.SimplifiableMatchWithSequenceOrMappingVisitor,
7475
conditions.LeakingForLoopVisitor,
7576
iterables.IterableUnpackingVisitor,
7677
blocks.AfterBlockVariablesVisitor,

wemake_python_styleguide/violations/consistency.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,3 +2460,54 @@ class SimplifiableMatchViolation(ASTViolation):
24602460
'Found simplifiable `match` statement that can be just `if`'
24612461
)
24622462
code = 365
2463+
2464+
2465+
@final
2466+
class SimplifiableMatchWithSequenceOrMappingViolation(ASTViolation):
2467+
"""
2468+
Some ``match`` statements with simple sequences or mappings can be.
2469+
2470+
Reasoning:
2471+
Using ``match`` for exact structural comparisons of simple literals
2472+
(like lists or dicts) is unnecessarily verbose. While match excels
2473+
at deconstruction, using it to check for an exact list or dict value
2474+
is better expressed with a direct equality comparison (``==``),
2475+
which is more readable and performant.
2476+
2477+
Solution:
2478+
Replace ``match`` statements that check for simple sequences or
2479+
mappings (with no deconstruction) with ``if`` statements using ``==``.
2480+
2481+
When is this violation is raised?
2482+
- When there are exactly two ``case`` statements
2483+
- When the first case uses a simple sequence or mapping pattern
2484+
- When the second case is a wildcard: ``case _:``
2485+
- When the pattern contains only literals/contants (no
2486+
variable bindings)
2487+
- When there are no guards or starred patterns
2488+
2489+
2490+
Example::
2491+
2492+
# Correct:
2493+
if data == [1, 2]:
2494+
handle_pair()
2495+
else:
2496+
ignore()
2497+
2498+
# Wrong:
2499+
match data:
2500+
case [1, 2]:
2501+
handle_pair()
2502+
case _:
2503+
ignore()
2504+
2505+
.. versionadded:: 1.5.0
2506+
2507+
"""
2508+
2509+
error_template = (
2510+
'Found simplifiable `match` statement with sequence or mapping '
2511+
'that can be just `if`'
2512+
)
2513+
code = 366

wemake_python_styleguide/visitors/ast/conditions.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,34 @@ def _check_simplifiable_match(self, node: ast.Match) -> None:
220220
self.add_violation(consistency.SimplifiableMatchViolation(node))
221221

222222

223+
@final
224+
class SimplifiableMatchWithSequenceOrMappingVisitor(BaseNodeVisitor):
225+
"""Checks for match statements with simple sequences and mappings."""
226+
227+
def visit_Match(self, node: ast.Match) -> None:
228+
"""Checks match statements with simple sequences and mappings."""
229+
self._check_simplifiable_match_seq_or_map(node)
230+
self.generic_visit(node)
231+
232+
def _check_simplifiable_match_seq_or_map(self, node: ast.Match) -> None:
233+
cases = node.cases
234+
if len(cases) == 2:
235+
first, second = cases
236+
237+
if not pattern_matching.is_wildcard_pattern(second):
238+
return
239+
240+
# Check if the first pattern is a simple sequence or mapping pattern
241+
# without variable bindings AND no guard is present
242+
if (
243+
pattern_matching.is_simple_sequence_or_mapping_pattern(
244+
first.pattern
245+
)
246+
and first.guard is None # No guard clause
247+
):
248+
self.add_violation(consistency.SimplifiableMatchWithSequenceOrMappingViolation(node))
249+
250+
223251
@final
224252
class LeakingForLoopVisitor(BaseNodeVisitor):
225253
"""Finds 'for' loops directly inside class or module bodies."""

0 commit comments

Comments
 (0)