Skip to content

Commit 0bd6326

Browse files
committed
feat(WPS366): match statement sequence or mapping can be simplified to if, refs #3527
1 parent ff14c22 commit 0bd6326

File tree

9 files changed

+337
-61
lines changed

9 files changed

+337
-61
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Semantic versioning in our case means:
2929
### Features
3030

3131
- Adds `WPS365`: match statement can be simplified to `if`, #3520
32+
- Adds `WPS366`: match sequence or mapping can be simplified to `if`, #3527
3233

3334

3435
## 1.4.0

tests/fixtures/noqa/noqa.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,17 @@ def many_raises_function(parameter): # noqa: WPS238
622622
case _:
623623
...
624624

625+
match subject: # noqa: WPS366
626+
case [1, 2]: # noqa: WPS366
627+
...
628+
case _:
629+
...
630+
631+
match subject: # noqa: WPS366
632+
case {'x': 1, 'y': 2}:
633+
...
634+
case _:
635+
...
625636

626637
my_print("""
627638
text

tests/test_checker/test_noqa.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@
167167
'WPS363': 1,
168168
'WPS364': 1,
169169
'WPS365': 1,
170+
'WPS366': 2,
170171
'WPS400': 0, # defined in ignored violations.
171172
'WPS401': 0, # logically unacceptable.
172173
'WPS402': 0, # defined in ignored violations.
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import pytest
2+
3+
from wemake_python_styleguide.violations.consistency import (
4+
SimplifiableSequenceOrMappingMatchViolation,
5+
)
6+
from wemake_python_styleguide.visitors.ast.conditions import (
7+
SimplifiableSequenceOrMappingMatchVisitor,
8+
)
9+
10+
# Wrong:
11+
simple_sequence = """
12+
match data:
13+
case {0}:
14+
pass
15+
case _:
16+
pass
17+
"""
18+
19+
simple_mapping = """
20+
match data:
21+
case {0}:
22+
pass
23+
case _:
24+
pass
25+
"""
26+
27+
28+
# Correct:
29+
binding_sequence = """
30+
match data:
31+
case [x]:
32+
pass
33+
case _:
34+
pass
35+
"""
36+
37+
starred_sequence = """
38+
match data:
39+
case [1, *rest]:
40+
pass
41+
case _:
42+
pass
43+
"""
44+
45+
mapping_with_binding = """
46+
match data:
47+
case {"x": value}:
48+
pass
49+
case _:
50+
pass
51+
"""
52+
53+
with_guard = """
54+
match data:
55+
case [1, 2] if flag:
56+
pass
57+
case _:
58+
pass
59+
"""
60+
61+
as_binding = """
62+
match data:
63+
case [1, 2] as x:
64+
pass
65+
case _:
66+
pass
67+
"""
68+
69+
complex_sequence = """
70+
match data:
71+
case [1, [3, 4]]:
72+
pass
73+
case _:
74+
pass
75+
"""
76+
77+
complex_name = """
78+
match data:
79+
case [some_var]:
80+
pass
81+
case _:
82+
pass
83+
"""
84+
85+
three_cases = """
86+
match data:
87+
case [1]:
88+
pass
89+
case [2]:
90+
pass
91+
case _:
92+
pass
93+
"""
94+
95+
96+
@pytest.mark.parametrize(
97+
'code',
98+
[
99+
[1, 'a'],
100+
[2, 3],
101+
['x', 'y'],
102+
],
103+
)
104+
def test_simplifiable_sequence_match_violation(
105+
code, assert_errors, parse_ast_tree, default_options
106+
):
107+
"""Test that simple sequence match raises a violation."""
108+
tree = parse_ast_tree(simple_sequence.format(code))
109+
visitor = SimplifiableSequenceOrMappingMatchVisitor(
110+
default_options, tree=tree
111+
)
112+
visitor.run()
113+
assert_errors(visitor, [SimplifiableSequenceOrMappingMatchViolation])
114+
115+
116+
@pytest.mark.parametrize(
117+
'code',
118+
[
119+
{'a': 1},
120+
{'x': 2, 'y': 3},
121+
{4: 'b'},
122+
],
123+
)
124+
def test_simplifiable_mapping_match_violation(
125+
code, assert_errors, parse_ast_tree, default_options
126+
):
127+
"""Test that simple mapping match raises a violation."""
128+
tree = parse_ast_tree(simple_mapping.format(code))
129+
visitor = SimplifiableSequenceOrMappingMatchVisitor(
130+
default_options, tree=tree
131+
)
132+
visitor.run()
133+
assert_errors(visitor, [SimplifiableSequenceOrMappingMatchViolation])
134+
135+
136+
@pytest.mark.parametrize(
137+
'code',
138+
[
139+
binding_sequence,
140+
starred_sequence,
141+
mapping_with_binding,
142+
with_guard,
143+
as_binding,
144+
complex_sequence,
145+
complex_name,
146+
three_cases,
147+
],
148+
)
149+
def test_not_simplifiable_structural_match(
150+
code, assert_errors, parse_ast_tree, default_options
151+
):
152+
"""Test that complex or non-simplifiable matches do not raise violations."""
153+
tree = parse_ast_tree(code)
154+
visitor = SimplifiableSequenceOrMappingMatchVisitor(
155+
default_options, tree=tree
156+
)
157+
visitor.run()
158+
assert_errors(visitor, [])

wemake_python_styleguide/logic/tree/pattern_matching.py

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -45,60 +45,3 @@ def is_constant_subject(condition: ast.AST | list[ast.expr]) -> bool:
4545
and is_constant_subject(node.values)
4646
)
4747
return False
48-
49-
50-
def is_wildcard_pattern(case: ast.match_case) -> bool:
51-
"""Returns True only for `case _:`."""
52-
pattern = case.pattern
53-
return (
54-
isinstance(pattern, ast.MatchAs)
55-
and pattern.pattern is None
56-
and pattern.name is None
57-
)
58-
59-
60-
def is_simple_pattern(pattern: ast.pattern) -> bool:
61-
"""Returns True if the pattern is simple enough to replace with `==`."""
62-
return _is_simple_value_or_singleton(pattern) or _is_simple_composite(
63-
pattern
64-
)
65-
66-
67-
def _is_simple_composite(pattern: ast.pattern) -> bool:
68-
"""Returns True/False for MatchOr and MatchAs, None otherwise."""
69-
if isinstance(pattern, ast.MatchOr):
70-
return all(is_simple_pattern(sub) for sub in pattern.patterns)
71-
if isinstance(pattern, ast.MatchAs):
72-
inner = pattern.pattern
73-
return inner is not None and is_simple_pattern(inner)
74-
return False
75-
76-
77-
def _is_simple_value_or_singleton(pattern: ast.pattern) -> bool:
78-
"""
79-
Checks if a pattern is a simple literal or singleton.
80-
81-
Supports:
82-
- Single values: ``1``, ``"text"``, ``ns.CONST``.
83-
- Singleton values: ``True``, ``False``, ``None``.
84-
"""
85-
if isinstance(pattern, ast.MatchSingleton):
86-
return True
87-
if isinstance(pattern, ast.MatchValue):
88-
return isinstance(
89-
pattern.value, (ast.Constant, ast.Name, ast.Attribute)
90-
)
91-
return False
92-
93-
94-
def is_irrefutable_binding(pattern: ast.pattern) -> bool:
95-
"""
96-
Returns True for patterns like ``case x:`` or ``case data:``.
97-
98-
These always match and just bind the subject to a name.
99-
"""
100-
return (
101-
isinstance(pattern, ast.MatchAs)
102-
and pattern.pattern is None
103-
and pattern.name is not None
104-
)
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import ast
2+
3+
4+
def is_wildcard_pattern(case: ast.match_case) -> bool:
5+
"""Returns True only for `case _:`."""
6+
pattern = case.pattern
7+
return (
8+
isinstance(pattern, ast.MatchAs)
9+
and pattern.pattern is None
10+
and pattern.name is None
11+
)
12+
13+
14+
def is_simple_pattern(pattern: ast.pattern) -> bool:
15+
"""Returns True if the pattern is simple enough to replace with `==`."""
16+
return _is_simple_value_or_singleton(pattern) or _is_simple_composite(
17+
pattern
18+
)
19+
20+
21+
def _is_simple_composite(pattern: ast.pattern) -> bool:
22+
"""Returns True/False for MatchOr and MatchAs, None otherwise."""
23+
if isinstance(pattern, ast.MatchOr):
24+
return all(is_simple_pattern(sub) for sub in pattern.patterns)
25+
if isinstance(pattern, ast.MatchAs):
26+
inner = pattern.pattern
27+
return inner is not None and is_simple_pattern(inner)
28+
return False
29+
30+
31+
def _is_simple_value_or_singleton(pattern: ast.pattern) -> bool:
32+
"""
33+
Checks if a pattern is a simple literal or singleton.
34+
35+
Supports:
36+
- Single values: ``1``, ``"text"``, ``ns.CONST``.
37+
- Singleton values: ``True``, ``False``, ``None``.
38+
"""
39+
if isinstance(pattern, ast.MatchSingleton):
40+
return True
41+
if isinstance(pattern, ast.MatchValue):
42+
return isinstance(
43+
pattern.value, (ast.Constant, ast.Name, ast.Attribute)
44+
)
45+
return False
46+
47+
48+
def is_irrefutable_binding(pattern: ast.pattern) -> bool:
49+
"""
50+
Returns True for patterns like ``case x:`` or ``case data:``.
51+
52+
These always match and just bind the subject to a name.
53+
"""
54+
return (
55+
isinstance(pattern, ast.MatchAs)
56+
and pattern.pattern is None
57+
and pattern.name is not None
58+
)
59+
60+
61+
def is_simple_sequence_or_mapping_pattern(pattern: ast.pattern) -> bool:
62+
"""
63+
Check that all elements in sequence/mapping are simple.
64+
65+
Simple is (literals, constants, names).
66+
"""
67+
if isinstance(pattern, ast.MatchSequence):
68+
return all(is_simple_pattern(pattern) for pattern in pattern.patterns)
69+
if isinstance(pattern, ast.MatchMapping):
70+
return all(is_simple_pattern(pattern) for pattern in pattern.patterns)
71+
return False

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.SimplifiableSequenceOrMappingMatchVisitor,
7475
conditions.LeakingForLoopVisitor,
7576
iterables.IterableUnpackingVisitor,
7677
blocks.AfterBlockVariablesVisitor,

wemake_python_styleguide/violations/consistency.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,3 +2460,57 @@ class SimplifiableMatchViolation(ASTViolation):
24602460
'Found simplifiable `match` statement that can be just `if`'
24612461
)
24622462
code = 365
2463+
2464+
2465+
@final
2466+
class SimplifiableSequenceOrMappingMatchViolation(ASTViolation):
2467+
"""
2468+
Some ``match`` list/dict statements can be simplified to ``if`` statements.
2469+
2470+
Reasoning:
2471+
Using ``match`` to check for an exact sequence (like ``[1, 2]``)
2472+
or mapping (like ``{"x": 1}``)
2473+
pattern with no destructuring is unnecessarily verbose.
2474+
Such cases are better expressed
2475+
with a direct equality comparison (``==``),
2476+
which is more readable and performant.
2477+
2478+
While ``match`` excels at deconstruction and partial matching,
2479+
using it for full structural identity checks of literals is overkill.
2480+
2481+
Solution:
2482+
Replace the ``match`` statement with an ``if/else``.
2483+
2484+
When is this violation raised?
2485+
- There are exactly two cases, and the second is ``case _:``.
2486+
- The first case uses a simple sequence (e.g. ``[1, "a"]``)
2487+
or mapping (e.g. ``{"key": 1}``).
2488+
- No variable bindings (e.g. ``case [x]:``),
2489+
starred patterns (``*rest``),or keyword rest (``**extra``).
2490+
- No guards (``if ...``).
2491+
- All elements are literals, constants, or names (not expressions).
2492+
2493+
2494+
Example::
2495+
2496+
# Correct:
2497+
if data == [1, 2]:
2498+
handle_pair()
2499+
else:
2500+
ignore()
2501+
2502+
# Wrong:
2503+
match
2504+
case [1, 2]:
2505+
handle_pair()
2506+
case _:
2507+
ignore()
2508+
2509+
.. versionadded:: 1.5.0
2510+
2511+
"""
2512+
2513+
error_template = (
2514+
'Found simplifiable sequence/mapping `match` that can be `if`'
2515+
)
2516+
code = 366

0 commit comments

Comments
 (0)