Skip to content

Commit f6d8d9d

Browse files
committed
Add additional checks for match class patterns
1 parent 42c0131 commit f6d8d9d

File tree

12 files changed

+178
-11
lines changed

12 files changed

+178
-11
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class Book:
2+
__match_args__ = ("title", "year")
3+
4+
def __init__(self, title, year):
5+
self.title = title
6+
self.year = year
7+
8+
9+
def func(item: Book):
10+
match item:
11+
case Book(title=str(title)): # [match-class-bind-self]
12+
...
13+
case Book(year=int(year)): # [match-class-bind-self]
14+
...
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class Book:
2+
__match_args__ = ("title", "year")
3+
4+
def __init__(self, title, year):
5+
self.title = title
6+
self.year = year
7+
8+
9+
def func(item: Book):
10+
match item:
11+
case Book(title=str() as title):
12+
...
13+
case Book(year=int() as year):
14+
...
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `Python documentation <https://docs.python.org/3/reference/compound_stmts.html#class-patterns>`_
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class Book:
2+
__match_args__ = ("title", "year")
3+
4+
def __init__(self, title, year):
5+
self.title = title
6+
self.year = year
7+
8+
9+
def func(item: Book):
10+
match item:
11+
case Book("abc", 2000): # [match-class-positional-attributes]
12+
...
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class Book:
2+
__match_args__ = ("title", "year")
3+
4+
def __init__(self, title, year):
5+
self.title = title
6+
self.year = year
7+
8+
9+
def func(item: Book):
10+
match item:
11+
case Book(title="abc", year=2000):
12+
...
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `Python documentation <https://docs.python.org/3/reference/compound_stmts.html#class-patterns>`_

doc/user_guide/checkers/features.rst

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -692,11 +692,17 @@ Match Statements checker Messages
692692
:invalid-match-args-definition (E1902): *`__match_args__` must be a tuple of strings.*
693693
Emitted if `__match_args__` isn't a tuple of strings required for match.
694694
:too-many-positional-sub-patterns (E1903): *%s expects %d positional sub-patterns (given %d)*
695-
Emitted when the number of allowed positional sub-patterns exceeds the
696-
number of allowed sub-patterns specified in `__match_args__`.
695+
Emitted when the number of allowed positional sub-patterns exceeds the number
696+
of allowed sub-patterns specified in `__match_args__`.
697697
:multiple-class-sub-patterns (E1904): *Multiple sub-patterns for attribute %s*
698-
Emitted when there is more than one sub-pattern for a specific attribute in
699-
a class pattern.
698+
Emitted when there is more than one sub-pattern for a specific attribute in a
699+
class pattern.
700+
:match-class-bind-self (R1905): *Use '%s' instead*
701+
Match class patterns are faster if the name binding happens for the whole
702+
pattern and any lookup for `__match_args__` can be avoided.
703+
:match-class-positional-attributes (R1906): *Use keyword attributes instead of positional ones*
704+
Keyword attributes are more explicit and slightly faster since CPython can
705+
skip the `__match_args__` lookup.
700706

701707

702708
Method Args checker

doc/user_guide/messages/messages_overview.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,8 @@ All messages in the refactor category:
518518
refactor/inconsistent-return-statements
519519
refactor/literal-comparison
520520
refactor/magic-value-comparison
521+
refactor/match-class-bind-self
522+
refactor/match-class-positional-attributes
521523
refactor/no-classmethod-decorator
522524
refactor/no-else-break
523525
refactor/no-else-continue
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Add additional checks for suboptimal uses of class patterns in :keyword:`match`.
2+
* :ref:`match-class-bind-self` is emitted if a name is bound to ``self`` instead of
3+
using an ``as`` pattern.
4+
* :ref:`match-class-positional-attributes` is emitted if a class pattern has positional
5+
attributes when keywords could be used.
6+
7+
Refs #10586

pylint/checkers/match_statements_checker.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,23 @@
1919
from pylint.lint import PyLinter
2020

2121

22+
# List of builtin classes which match self
23+
# https://docs.python.org/3/reference/compound_stmts.html#class-patterns
24+
MATCH_CLASS_SELF_NAMES = {
25+
"builtins.bool",
26+
"builtins.bytearray",
27+
"builtins.bytes",
28+
"builtins.dict",
29+
"builtins.float",
30+
"builtins.frozenset",
31+
"builtins.int",
32+
"builtins.list",
33+
"builtins.set",
34+
"builtins.str",
35+
"builtins.tuple",
36+
}
37+
38+
2239
class MatchStatementChecker(BaseChecker):
2340
name = "match_statements"
2441
msgs = {
@@ -46,6 +63,19 @@ class MatchStatementChecker(BaseChecker):
4663
"Emitted when there is more than one sub-pattern for a specific "
4764
"attribute in a class pattern.",
4865
),
66+
"R1905": (
67+
"Use '%s' instead",
68+
"match-class-bind-self",
69+
"Match class patterns are faster if the name binding happens "
70+
"for the whole pattern and any lookup for `__match_args__` "
71+
"can be avoided.",
72+
),
73+
"R1906": (
74+
"Use keyword attributes instead of positional ones",
75+
"match-class-positional-attributes",
76+
"Keyword attributes are more explicit and slightly faster "
77+
"since CPython can skip the `__match_args__` lookup.",
78+
),
4979
}
5080

5181
@only_required_for_messages("invalid-match-args-definition")
@@ -86,6 +116,26 @@ def visit_match(self, node: nodes.Match) -> None:
86116
confidence=HIGH,
87117
)
88118

119+
@only_required_for_messages("match-class-bind-self")
120+
def visit_matchas(self, node: nodes.MatchAs) -> None:
121+
match node:
122+
case nodes.MatchAs(
123+
parent=nodes.MatchClass(cls=nodes.Name() as cls_name),
124+
name=nodes.AssignName(name=name),
125+
pattern=None,
126+
):
127+
inferred = safe_infer(cls_name)
128+
if (
129+
isinstance(inferred, nodes.ClassDef)
130+
and inferred.qname() in MATCH_CLASS_SELF_NAMES
131+
):
132+
self.add_message(
133+
"match-class-bind-self",
134+
node=node,
135+
args=(f"{cls_name.name}() as {name}",),
136+
confidence=HIGH,
137+
)
138+
89139
@staticmethod
90140
def get_match_args_for_class(node: nodes.NodeNG) -> list[str] | None:
91141
"""Infer __match_args__ from class name."""
@@ -95,6 +145,8 @@ def get_match_args_for_class(node: nodes.NodeNG) -> list[str] | None:
95145
try:
96146
match_args = inferred.getattr("__match_args__")
97147
except astroid.exceptions.NotFoundError:
148+
if inferred.qname() in MATCH_CLASS_SELF_NAMES:
149+
return ["<self>"]
98150
return None
99151

100152
match match_args:
@@ -124,13 +176,27 @@ def check_duplicate_sub_patterns(
124176
attrs.add(name)
125177

126178
@only_required_for_messages(
179+
"match-class-positional-attributes",
127180
"multiple-class-sub-patterns",
128181
"too-many-positional-sub-patterns",
129182
)
130183
def visit_matchclass(self, node: nodes.MatchClass) -> None:
131184
attrs: set[str] = set()
132185
dups: set[str] = set()
133186

187+
if node.patterns:
188+
if isinstance(node, nodes.MatchClass) and isinstance(node.cls, nodes.Name):
189+
inferred = safe_infer(node.cls)
190+
if not (
191+
isinstance(inferred, nodes.ClassDef)
192+
and inferred.qname() in MATCH_CLASS_SELF_NAMES
193+
):
194+
self.add_message(
195+
"match-class-positional-attributes",
196+
node=node,
197+
confidence=HIGH,
198+
)
199+
134200
if (
135201
node.patterns
136202
and (match_args := self.get_match_args_for_class(node.cls)) is not None

0 commit comments

Comments
 (0)