Add additional checks for match class patterns (#10587)

Author: cdce8p

doc/data/messages/m/match-class-bind-self/bad.py

@@ -0,0 +1,14 @@
+class Book:
+    __match_args__ = ("title", "year")
+
+    def __init__(self, title, year):
+        self.title = title
+        self.year = year
+
+
+def func(item: Book):
+    match item:
+        case Book(title=str(title)):  # [match-class-bind-self]
+            ...
+        case Book(year=int(year)):  # [match-class-bind-self]
+            ...

doc/data/messages/m/match-class-bind-self/good.py

@@ -0,0 +1,14 @@
+class Book:
+    __match_args__ = ("title", "year")
+
+    def __init__(self, title, year):
+        self.title = title
+        self.year = year
+
+
+def func(item: Book):
+    match item:
+        case Book(title=str() as title):
+            ...
+        case Book(year=int() as year):
+            ...

doc/data/messages/m/match-class-bind-self/related.rst

@@ -0,0 +1 @@
+- `Python documentation <https://docs.python.org/3/reference/compound_stmts.html#class-patterns>`_

doc/data/messages/m/match-class-positional-attributes/bad.py

@@ -0,0 +1,12 @@
+class Book:
+    __match_args__ = ("title", "year")
+
+    def __init__(self, title, year):
+        self.title = title
+        self.year = year
+
+
+def func(item: Book):
+    match item:
+        case Book("abc", 2000):  # [match-class-positional-attributes]
+            ...

doc/data/messages/m/match-class-positional-attributes/good.py

@@ -0,0 +1,12 @@
+class Book:
+    __match_args__ = ("title", "year")
+
+    def __init__(self, title, year):
+        self.title = title
+        self.year = year
+
+
+def func(item: Book):
+    match item:
+        case Book(title="abc", year=2000):
+            ...

doc/data/messages/m/match-class-positional-attributes/related.rst

@@ -0,0 +1 @@
+- `Python documentation <https://docs.python.org/3/reference/compound_stmts.html#class-patterns>`_

doc/user_guide/checkers/features.rst

@@ -694,6 +694,12 @@ Match Statements checker Messages
 :multiple-class-sub-patterns (E1904): *Multiple sub-patterns for attribute %s*
   Emitted when there is more than one sub-pattern for a specific attribute in
   a class pattern.
+:match-class-bind-self (R1905): *Use '%s() as %s' instead*
+  Match class patterns are faster if the name binding happens for the whole
+  pattern and any lookup for `__match_args__` can be avoided.
+:match-class-positional-attributes (R1906): *Use keyword attributes instead of positional ones (%s)*
+  Keyword attributes are more explicit and slightly faster since CPython can
+  skip the `__match_args__` lookup.
 
 
 Method Args checker

doc/user_guide/messages/messages_overview.rst

@@ -518,6 +518,8 @@ All messages in the refactor category:
    refactor/inconsistent-return-statements
    refactor/literal-comparison
    refactor/magic-value-comparison
+   refactor/match-class-bind-self
+   refactor/match-class-positional-attributes
    refactor/no-classmethod-decorator
    refactor/no-else-break
    refactor/no-else-continue

doc/whatsnew/fragments/10586.new_check

@@ -0,0 +1,8 @@
+Add additional checks for suboptimal uses of class patterns in :keyword:`match`.
+
+* :ref:`match-class-bind-self` is emitted if a name is bound to ``self`` instead of
+  using an ``as`` pattern.
+* :ref:`match-class-positional-attributes` is emitted if a class pattern has positional
+  attributes when keywords could be used.
+
+Refs #10586

pylint/checkers/match_statements_checker.py

@@ -19,6 +19,23 @@
     from pylint.lint import PyLinter
 
 
+# List of builtin classes which match self
+# https://docs.python.org/3/reference/compound_stmts.html#class-patterns
+MATCH_CLASS_SELF_NAMES = {
+    "builtins.bool",
+    "builtins.bytearray",
+    "builtins.bytes",
+    "builtins.dict",
+    "builtins.float",
+    "builtins.frozenset",
+    "builtins.int",
+    "builtins.list",
+    "builtins.set",
+    "builtins.str",
+    "builtins.tuple",
+}
+
+
 class MatchStatementChecker(BaseChecker):
     name = "match_statements"
     msgs = {
@@ -46,6 +63,19 @@ class MatchStatementChecker(BaseChecker):
             "Emitted when there is more than one sub-pattern for a specific "
             "attribute in a class pattern.",
         ),
+        "R1905": (
+            "Use '%s() as %s' instead",
+            "match-class-bind-self",
+            "Match class patterns are faster if the name binding happens "
+            "for the whole pattern and any lookup for `__match_args__` "
+            "can be avoided.",
+        ),
+        "R1906": (
+            "Use keyword attributes instead of positional ones (%s)",
+            "match-class-positional-attributes",
+            "Keyword attributes are more explicit and slightly faster "
+            "since CPython can skip the `__match_args__` lookup.",
+        ),
     }
 
     @only_required_for_messages("invalid-match-args-definition")
@@ -86,6 +116,26 @@ def visit_match(self, node: nodes.Match) -> None:
                         confidence=HIGH,
                     )
 
+    @only_required_for_messages("match-class-bind-self")
+    def visit_matchas(self, node: nodes.MatchAs) -> None:
+        match node:
+            case nodes.MatchAs(
+                parent=nodes.MatchClass(cls=nodes.Name() as cls_name, patterns=[_]),
+                name=nodes.AssignName(name=name),
+                pattern=None,
+            ):
+                inferred = safe_infer(cls_name)
+                if (
+                    isinstance(inferred, nodes.ClassDef)
+                    and inferred.qname() in MATCH_CLASS_SELF_NAMES
+                ):
+                    self.add_message(
+                        "match-class-bind-self",
+                        node=node,
+                        args=(cls_name.name, name),
+                        confidence=HIGH,
+                    )
+
     @staticmethod
     def get_match_args_for_class(node: nodes.NodeNG) -> list[str] | None:
         """Infer __match_args__ from class name."""
@@ -95,6 +145,8 @@ def get_match_args_for_class(node: nodes.NodeNG) -> list[str] | None:
         try:
             match_args = inferred.getattr("__match_args__")
         except astroid.exceptions.NotFoundError:
+            if inferred.qname() in MATCH_CLASS_SELF_NAMES:
+                return ["<self>"]
             return None
 
         match match_args:
@@ -124,6 +176,7 @@ def check_duplicate_sub_patterns(
             attrs.add(name)
 
     @only_required_for_messages(
+        "match-class-positional-attributes",
         "multiple-class-sub-patterns",
         "too-many-positional-sub-patterns",
     )
@@ -144,6 +197,22 @@ def visit_matchclass(self, node: nodes.MatchClass) -> None:
                 )
                 return
 
+            inferred = safe_infer(node.cls)
+            if not (
+                isinstance(inferred, nodes.ClassDef)
+                and (
+                    inferred.qname() in MATCH_CLASS_SELF_NAMES
+                    or "tuple" in inferred.basenames
+                )
+            ):
+                attributes = [f"'{attr}'" for attr in match_args[: len(node.patterns)]]
+                self.add_message(
+                    "match-class-positional-attributes",
+                    node=node,
+                    args=(", ".join(attributes),),
+                    confidence=INFERENCE,
+                )
+
             for i in range(len(node.patterns)):
                 name = match_args[i]
                 self.check_duplicate_sub_patterns(name, node, attrs=attrs, dups=dups)