Skip to content

Commit 3246459

Browse files
Add simplifiable match rule (wemake-services#3526)
Co-authored-by: sobolevn <[email protected]>
1 parent 27093cf commit 3246459

File tree

10 files changed

+333
-2
lines changed

10 files changed

+333
-2
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ Semantic versioning in our case means:
1717
change the client facing API, change code conventions significantly, etc.
1818

1919

20+
## 1.5.0
21+
22+
### Features
23+
24+
- Adds `WPS365`: match statement can be simplified to `if`, #3520
25+
26+
2027
## 1.4.0
2128

2229
### Features

docs/pages/usage/setup.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Required configuration
2929
----------------------
3030

3131
You must either provide ``--select=WPS`` to all your ``flake8`` calls,
32-
or add ``select = WPS`` into your ``flake8`` configration file.
32+
or add ``select = WPS`` into your ``flake8`` configuration file.
3333

3434
Running
3535
-------

tests/fixtures/noqa/noqa.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,12 @@ def many_raises_function(parameter): # noqa: WPS238
616616
case 8:
617617
my_print('eighth')
618618

619+
match subject: # noqa: WPS365
620+
case 1:
621+
...
622+
case _:
623+
...
624+
619625

620626
my_print("""
621627
text

tests/test_checker/test_noqa.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@
166166
'WPS362': 2,
167167
'WPS363': 1,
168168
'WPS364': 1,
169+
'WPS365': 1,
169170
'WPS400': 0, # defined in ignored violations.
170171
'WPS401': 0, # logically unacceptable.
171172
'WPS402': 0, # defined in ignored violations.
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
import pytest
2+
3+
from wemake_python_styleguide.violations.consistency import (
4+
SimplifiableMatchViolation,
5+
)
6+
from wemake_python_styleguide.visitors.ast.conditions import (
7+
SimplifiableMatchVisitor,
8+
)
9+
10+
# Wrong:
11+
simplifiable_match_match = """
12+
match subject:
13+
case {0}{1}:
14+
pass
15+
case _:
16+
pass
17+
"""
18+
19+
simplifiable_union_match_match = """
20+
match subject:
21+
case {0} | {1}{2}:
22+
pass
23+
case _:
24+
pass
25+
"""
26+
27+
simplifiable_guard_match = """
28+
match subject:
29+
case x if x > 0:
30+
pass
31+
case _:
32+
pass
33+
"""
34+
35+
# Correct:
36+
complex_match = """
37+
match subject:
38+
case State.FIRST:
39+
pass
40+
case State.SECOND:
41+
pass
42+
case _:
43+
pass
44+
"""
45+
46+
no_wildcard_match = """
47+
match subject:
48+
case State.FIRST:
49+
pass
50+
case State.SECOND:
51+
pass
52+
"""
53+
54+
guard_match = """
55+
match subject:
56+
case [x] if x > 0:
57+
pass
58+
case _:
59+
pass
60+
"""
61+
62+
class_with_args_match = """
63+
match subject:
64+
case SomeClass(x):
65+
pass
66+
case _:
67+
pass
68+
"""
69+
70+
class_with_kwargs_match = """
71+
match subject:
72+
case SomeClass(name=x):
73+
pass
74+
case _:
75+
pass
76+
"""
77+
78+
sequences_match = """
79+
match subject:
80+
case [x, *rest]:
81+
pass
82+
case _:
83+
pass
84+
"""
85+
86+
mappings_match = """
87+
match subject:
88+
case {"key": x}:
89+
pass
90+
case _:
91+
pass
92+
"""
93+
94+
95+
@pytest.mark.parametrize(
96+
'code',
97+
[
98+
'1',
99+
'True',
100+
'None',
101+
'"string"',
102+
'ns.CONST',
103+
'State.REJECTED',
104+
],
105+
)
106+
@pytest.mark.parametrize(
107+
'as_binding',
108+
['', ' as x'],
109+
)
110+
def test_simplifiable_single_match(
111+
code,
112+
as_binding,
113+
assert_errors,
114+
parse_ast_tree,
115+
default_options,
116+
):
117+
"""Test that simple single-case match raises a violation."""
118+
tree = parse_ast_tree(simplifiable_match_match.format(code, as_binding))
119+
visitor = SimplifiableMatchVisitor(default_options, tree=tree)
120+
visitor.run()
121+
assert_errors(visitor, [SimplifiableMatchViolation])
122+
123+
124+
def test_simplifiable_guarded_match(
125+
assert_errors,
126+
parse_ast_tree,
127+
default_options,
128+
):
129+
"""Test that guarded irrefutable match is simplified."""
130+
tree = parse_ast_tree(simplifiable_guard_match)
131+
visitor = SimplifiableMatchVisitor(default_options, tree=tree)
132+
visitor.run()
133+
assert_errors(visitor, [SimplifiableMatchViolation])
134+
135+
136+
@pytest.mark.parametrize(
137+
('left', 'right'),
138+
[
139+
('1', '2'),
140+
('True', 'False'),
141+
('"a"', '"b"'),
142+
('State.OK', 'State.ERROR'),
143+
('"first" | "second"', '"third"'),
144+
],
145+
)
146+
@pytest.mark.parametrize(
147+
'as_binding',
148+
['', ' as x'],
149+
)
150+
def test_simplifiable_union_match(
151+
left,
152+
right,
153+
as_binding,
154+
assert_errors,
155+
parse_ast_tree,
156+
default_options,
157+
):
158+
"""Test that union pattern raises violation."""
159+
tree = parse_ast_tree(
160+
simplifiable_union_match_match.format(left, right, as_binding)
161+
)
162+
visitor = SimplifiableMatchVisitor(default_options, tree=tree)
163+
visitor.run()
164+
assert_errors(visitor, [SimplifiableMatchViolation])
165+
166+
167+
@pytest.mark.parametrize(
168+
'template',
169+
[
170+
complex_match,
171+
no_wildcard_match,
172+
guard_match,
173+
class_with_args_match,
174+
class_with_kwargs_match,
175+
sequences_match,
176+
mappings_match,
177+
],
178+
)
179+
def test_not_simplifiable_match_templates(
180+
template,
181+
assert_errors,
182+
parse_ast_tree,
183+
default_options,
184+
):
185+
"""Test that complex or non-simplifiable matches do not raise violations."""
186+
tree = parse_ast_tree(template)
187+
visitor = SimplifiableMatchVisitor(default_options, tree=tree)
188+
visitor.run()
189+
assert_errors(visitor, [])

wemake_python_styleguide/logic/tree/pattern_matching.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,60 @@ 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+
)

wemake_python_styleguide/presets/types/tree.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
conditions.BooleanConditionVisitor,
7171
conditions.MatchVisitor,
7272
conditions.ChainedIsVisitor,
73+
conditions.SimplifiableMatchVisitor,
7374
iterables.IterableUnpackingVisitor,
7475
blocks.AfterBlockVariablesVisitor,
7576
subscripts.SubscriptVisitor,

wemake_python_styleguide/violations/consistency.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,3 +2415,48 @@ class NotInWithUnaryOpViolation(ASTViolation):
24152415

24162416
error_template = 'Found legacy `not ... in`, use `... not in` instead'
24172417
code = 364
2418+
2419+
2420+
@final
2421+
class SimplifiableMatchViolation(ASTViolation):
2422+
"""
2423+
Some ``match`` statements can be simplified to ``if`` statements.
2424+
2425+
Reasoning:
2426+
Using ``match`` for simple two-case conditions
2427+
(including wildcard ``_``)
2428+
is unnecessarily verbose and less performant than a plain ``if/else``.
2429+
Simple conditions should prefer readability and simplicity.
2430+
2431+
Solution:
2432+
Replace violating ``match ... case _`` statements with ``if ... else``.
2433+
2434+
When is this violation is raised?
2435+
- When there are exactly two ``case`` statements
2436+
- When the first case uses a literal pattern
2437+
- When the second case is a wildcard: ``case _:``
2438+
2439+
2440+
Example::
2441+
2442+
# Correct:
2443+
if state == EventType.REJECT:
2444+
user = 'rejected'
2445+
else:
2446+
user = 'active'
2447+
2448+
# Wrong:
2449+
match state:
2450+
case EventType.REJECT:
2451+
user = 'rejected'
2452+
case _:
2453+
user = 'active'
2454+
2455+
.. versionadded:: 1.5.0
2456+
2457+
"""
2458+
2459+
error_template = (
2460+
'Found simplifiable `match` statement that can be just `if`'
2461+
)
2462+
code = 365

wemake_python_styleguide/visitors/ast/conditions.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
compares,
1010
ifs,
1111
operators,
12+
pattern_matching,
1213
)
1314
from wemake_python_styleguide.types import AnyIf, AnyNodes
1415
from wemake_python_styleguide.violations import (
1516
best_practices,
17+
consistency,
1618
refactoring,
1719
)
1820
from wemake_python_styleguide.visitors.base import BaseNodeVisitor
@@ -188,3 +190,26 @@ def visit_Compare(self, node: ast.Compare) -> None:
188190
self.add_violation(refactoring.ChainedIsViolation(node))
189191

190192
self.generic_visit(node)
193+
194+
195+
@final
196+
class SimplifiableMatchVisitor(BaseNodeVisitor):
197+
"""Checks for match statements that can be simplified to if/else."""
198+
199+
def visit_Match(self, node: ast.Match) -> None:
200+
"""Checks match statements."""
201+
self._check_simplifiable_match(node)
202+
self.generic_visit(node)
203+
204+
def _check_simplifiable_match(self, node: ast.Match) -> None:
205+
cases = node.cases
206+
if len(cases) == 2:
207+
first, second = cases
208+
209+
if not pattern_matching.is_wildcard_pattern(second):
210+
return
211+
212+
if pattern_matching.is_irrefutable_binding(
213+
first.pattern
214+
) or pattern_matching.is_simple_pattern(first.pattern):
215+
self.add_violation(consistency.SimplifiableMatchViolation(node))

0 commit comments

Comments
 (0)