From 42b289de150f47714f130da0fe53860e42f2d166 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 24 Sep 2025 22:23:25 +0200 Subject: [PATCH 1/8] Add additional checks for match class patterns --- .../messages/m/match-class-bind-self/bad.py | 14 ++++ .../messages/m/match-class-bind-self/good.py | 14 ++++ .../m/match-class-bind-self/related.rst | 1 + .../match-class-positional-attributes/bad.py | 12 ++++ .../match-class-positional-attributes/good.py | 12 ++++ .../related.rst | 1 + doc/user_guide/checkers/features.rst | 6 ++ doc/user_guide/messages/messages_overview.rst | 2 + doc/whatsnew/fragments/10586.new_check | 7 ++ pylint/checkers/match_statements_checker.py | 66 +++++++++++++++++++ tests/functional/m/match_class_pattern.py | 26 ++++++++ tests/functional/m/match_class_pattern.txt | 20 ++++-- 12 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 doc/data/messages/m/match-class-bind-self/bad.py create mode 100644 doc/data/messages/m/match-class-bind-self/good.py create mode 100644 doc/data/messages/m/match-class-bind-self/related.rst create mode 100644 doc/data/messages/m/match-class-positional-attributes/bad.py create mode 100644 doc/data/messages/m/match-class-positional-attributes/good.py create mode 100644 doc/data/messages/m/match-class-positional-attributes/related.rst create mode 100644 doc/whatsnew/fragments/10586.new_check diff --git a/doc/data/messages/m/match-class-bind-self/bad.py b/doc/data/messages/m/match-class-bind-self/bad.py new file mode 100644 index 0000000000..a2919b72ff --- /dev/null +++ b/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] + ... diff --git a/doc/data/messages/m/match-class-bind-self/good.py b/doc/data/messages/m/match-class-bind-self/good.py new file mode 100644 index 0000000000..df2eceda98 --- /dev/null +++ b/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): + ... diff --git a/doc/data/messages/m/match-class-bind-self/related.rst b/doc/data/messages/m/match-class-bind-self/related.rst new file mode 100644 index 0000000000..f10342b564 --- /dev/null +++ b/doc/data/messages/m/match-class-bind-self/related.rst @@ -0,0 +1 @@ +- `Python documentation `_ diff --git a/doc/data/messages/m/match-class-positional-attributes/bad.py b/doc/data/messages/m/match-class-positional-attributes/bad.py new file mode 100644 index 0000000000..69013b3f56 --- /dev/null +++ b/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] + ... diff --git a/doc/data/messages/m/match-class-positional-attributes/good.py b/doc/data/messages/m/match-class-positional-attributes/good.py new file mode 100644 index 0000000000..93c567bbf1 --- /dev/null +++ b/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): + ... diff --git a/doc/data/messages/m/match-class-positional-attributes/related.rst b/doc/data/messages/m/match-class-positional-attributes/related.rst new file mode 100644 index 0000000000..f10342b564 --- /dev/null +++ b/doc/data/messages/m/match-class-positional-attributes/related.rst @@ -0,0 +1 @@ +- `Python documentation `_ diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index 66a20c5492..074a87136b 100644 --- a/doc/user_guide/checkers/features.rst +++ b/doc/user_guide/checkers/features.rst @@ -697,6 +697,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' 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* + Keyword attributes are more explicit and slightly faster since CPython can + skip the `__match_args__` lookup. Method Args checker diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst index f71738c83c..5b1378474e 100644 --- a/doc/user_guide/messages/messages_overview.rst +++ b/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 diff --git a/doc/whatsnew/fragments/10586.new_check b/doc/whatsnew/fragments/10586.new_check new file mode 100644 index 0000000000..fa64fbaf80 --- /dev/null +++ b/doc/whatsnew/fragments/10586.new_check @@ -0,0 +1,7 @@ +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 diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index 70788803e8..8c4ae0c82f 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/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' 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", + "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), + 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=(f"{cls_name.name}() as {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 [""] 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", ) @@ -131,6 +184,19 @@ def visit_matchclass(self, node: nodes.MatchClass) -> None: attrs: set[str] = set() dups: set[str] = set() + if node.patterns: + if isinstance(node, nodes.MatchClass) and isinstance(node.cls, nodes.Name): + inferred = safe_infer(node.cls) + if not ( + isinstance(inferred, nodes.ClassDef) + and inferred.qname() in MATCH_CLASS_SELF_NAMES + ): + self.add_message( + "match-class-positional-attributes", + node=node, + confidence=HIGH, + ) + if ( node.patterns and (match_args := self.get_match_args_for_class(node.cls)) is not None diff --git a/tests/functional/m/match_class_pattern.py b/tests/functional/m/match_class_pattern.py index 0e1cd3826a..25cadaa455 100644 --- a/tests/functional/m/match_class_pattern.py +++ b/tests/functional/m/match_class_pattern.py @@ -1,4 +1,5 @@ # pylint: disable=missing-docstring,unused-variable,too-few-public-methods +# pylint: disable=match-class-positional-attributes # -- Check __match_args__ definitions -- class A: @@ -25,6 +26,8 @@ def f1(x): case A(1, 2): ... # [too-many-positional-sub-patterns] case B(1, 2): ... case B(1, 2, 3): ... # [too-many-positional-sub-patterns] + case int(1): ... + case int(1, 2): ... # [too-many-positional-sub-patterns] def f2(x): """Check multiple sub-patterns for attribute""" @@ -38,3 +41,26 @@ def f2(x): # If class name is undefined, we can't get __match_args__ case NotDefined(1, x=1): ... # [undefined-variable] + +def f3(x): + """Check class pattern with name binding to self.""" + match x: + case int(y): ... # [match-class-bind-self] + case int() as y: ... + case int(2 as y): ... + case str(y): ... # [match-class-bind-self] + case str() as y: ... + case str("Hello" as y): ... + case tuple(y): ... # [match-class-bind-self] + case tuple() as y: ... + +def f4(x): + """Check for positional attributes if keywords could be used.""" + # pylint: enable=match-class-positional-attributes + match x: + case int(2): ... + case bool(True): ... + case A(1): ... # [match-class-positional-attributes] + case A(x=1): ... + case B(1, 2): ... # [match-class-positional-attributes] + case B(x=1, y=2): ... diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index c77d5d2891..fda549d2c3 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -1,7 +1,13 @@ -invalid-match-args-definition:11:21:11:31:C:`__match_args__` must be a tuple of strings.:HIGH -invalid-match-args-definition:14:21:14:29:D:`__match_args__` must be a tuple of strings.:HIGH -too-many-positional-sub-patterns:25:13:25:20:f1:A expects 1 positional sub-patterns (given 2):INFERENCE -too-many-positional-sub-patterns:27:13:27:23:f1:B expects 2 positional sub-patterns (given 3):INFERENCE -multiple-class-sub-patterns:32:13:32:22:f2:Multiple sub-patterns for attribute x:INFERENCE -multiple-class-sub-patterns:34:13:34:29:f2:Multiple sub-patterns for attribute x:INFERENCE -undefined-variable:40:13:40:23:f2:Undefined variable 'NotDefined':UNDEFINED +invalid-match-args-definition:12:21:12:31:C:`__match_args__` must be a tuple of strings.:HIGH +invalid-match-args-definition:15:21:15:29:D:`__match_args__` must be a tuple of strings.:HIGH +too-many-positional-sub-patterns:26:13:26:20:f1:A expects 1 positional sub-patterns (given 2):INFERENCE +too-many-positional-sub-patterns:28:13:28:23:f1:B expects 2 positional sub-patterns (given 3):INFERENCE +too-many-positional-sub-patterns:30:13:30:22:f1:int expects 1 positional sub-patterns (given 2):INFERENCE +multiple-class-sub-patterns:35:13:35:22:f2:Multiple sub-patterns for attribute x:INFERENCE +multiple-class-sub-patterns:37:13:37:29:f2:Multiple sub-patterns for attribute x:INFERENCE +undefined-variable:43:13:43:23:f2:Undefined variable 'NotDefined':UNDEFINED +match-class-bind-self:48:17:48:18:f3:Use 'int() as y' instead:HIGH +match-class-bind-self:51:17:51:18:f3:Use 'str() as y' instead:HIGH +match-class-bind-self:54:19:54:20:f3:Use 'tuple() as y' instead:HIGH +match-class-positional-attributes:63:13:63:17:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:65:13:65:20:f4:Use keyword attributes instead of positional ones:HIGH From 279e8ccc6d3609d63a38c718898051514c91fbaa Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 24 Sep 2025 22:23:25 +0200 Subject: [PATCH 2/8] Fix functional tests --- tests/functional/ext/mccabe/mccabe.py | 4 ++-- tests/functional/p/pattern_matching.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/functional/ext/mccabe/mccabe.py b/tests/functional/ext/mccabe/mccabe.py index 9cb69735f0..851fb6db7f 100644 --- a/tests/functional/ext/mccabe/mccabe.py +++ b/tests/functional/ext/mccabe/mccabe.py @@ -327,9 +327,9 @@ def nested_match_case(data): # [too-complex] match data: case {"type": "user", "data": user_data}: match user_data: # Nested match adds complexity - case {"name": str(name)}: + case {"name": str() as name}: return f"User: {name}" - case {"id": int(user_id)}: + case {"id": int() as user_id}: return f"User ID: {user_id}" case _: return "Unknown user format" diff --git a/tests/functional/p/pattern_matching.py b/tests/functional/p/pattern_matching.py index 4a5884812b..d590748208 100644 --- a/tests/functional/p/pattern_matching.py +++ b/tests/functional/p/pattern_matching.py @@ -1,4 +1,5 @@ # pylint: disable=missing-docstring,invalid-name,too-few-public-methods +# pylint: disable=match-class-positional-attributes class Point2D: __match_args__ = ("x", "y") From 3ce613a842e5b5e72512013b29f97b758c6d57fd Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 24 Sep 2025 23:38:08 +0200 Subject: [PATCH 3/8] Fix list in changelog entry --- doc/whatsnew/fragments/10586.new_check | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/whatsnew/fragments/10586.new_check b/doc/whatsnew/fragments/10586.new_check index fa64fbaf80..4d1e2ae17d 100644 --- a/doc/whatsnew/fragments/10586.new_check +++ b/doc/whatsnew/fragments/10586.new_check @@ -1,4 +1,5 @@ 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 From a8e71f16f9860feacec796ba43952df5ae4ca865 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 25 Sep 2025 00:46:31 +0200 Subject: [PATCH 4/8] Improve match self detection --- pylint/checkers/match_statements_checker.py | 8 ++++--- tests/functional/m/match_class_pattern.py | 16 +++++++++++-- tests/functional/m/match_class_pattern.txt | 26 ++++++++++----------- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index 8c4ae0c82f..68b1a69e9f 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/pylint/checkers/match_statements_checker.py @@ -20,19 +20,21 @@ # List of builtin classes which match self +# Exclude `dict`, `list` and `tuple` as those could also +# be matched by the mapping or sequence patterns. # https://docs.python.org/3/reference/compound_stmts.html#class-patterns MATCH_CLASS_SELF_NAMES = { "builtins.bool", "builtins.bytearray", "builtins.bytes", - "builtins.dict", + # "builtins.dict", "builtins.float", "builtins.frozenset", "builtins.int", - "builtins.list", + # "builtins.list", "builtins.set", "builtins.str", - "builtins.tuple", + # "builtins.tuple", } diff --git a/tests/functional/m/match_class_pattern.py b/tests/functional/m/match_class_pattern.py index 25cadaa455..8a6ea662ec 100644 --- a/tests/functional/m/match_class_pattern.py +++ b/tests/functional/m/match_class_pattern.py @@ -1,6 +1,8 @@ # pylint: disable=missing-docstring,unused-variable,too-few-public-methods # pylint: disable=match-class-positional-attributes +from typing import NamedTuple + # -- Check __match_args__ definitions -- class A: __match_args__ = ("x",) @@ -19,6 +21,12 @@ def f(self): __match_args__ = ["x"] +class Result(NamedTuple): + # inherits from tuple -> match self + x: int + y: int + + def f1(x): """Check too many positional sub-patterns""" match x: @@ -28,6 +36,10 @@ def f1(x): case B(1, 2, 3): ... # [too-many-positional-sub-patterns] case int(1): ... case int(1, 2): ... # [too-many-positional-sub-patterns] + case tuple(1): ... + case tuple(1, 2): ... + case tuple((1, 2)): ... + case Result(1, 2): ... def f2(x): """Check multiple sub-patterns for attribute""" @@ -51,8 +63,6 @@ def f3(x): case str(y): ... # [match-class-bind-self] case str() as y: ... case str("Hello" as y): ... - case tuple(y): ... # [match-class-bind-self] - case tuple() as y: ... def f4(x): """Check for positional attributes if keywords could be used.""" @@ -64,3 +74,5 @@ def f4(x): case A(x=1): ... case B(1, 2): ... # [match-class-positional-attributes] case B(x=1, y=2): ... + case Result(1, 2): ... # [match-class-positional-attributes] + case Result(x=1, y=2): ... diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index fda549d2c3..9284c427ae 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -1,13 +1,13 @@ -invalid-match-args-definition:12:21:12:31:C:`__match_args__` must be a tuple of strings.:HIGH -invalid-match-args-definition:15:21:15:29:D:`__match_args__` must be a tuple of strings.:HIGH -too-many-positional-sub-patterns:26:13:26:20:f1:A expects 1 positional sub-patterns (given 2):INFERENCE -too-many-positional-sub-patterns:28:13:28:23:f1:B expects 2 positional sub-patterns (given 3):INFERENCE -too-many-positional-sub-patterns:30:13:30:22:f1:int expects 1 positional sub-patterns (given 2):INFERENCE -multiple-class-sub-patterns:35:13:35:22:f2:Multiple sub-patterns for attribute x:INFERENCE -multiple-class-sub-patterns:37:13:37:29:f2:Multiple sub-patterns for attribute x:INFERENCE -undefined-variable:43:13:43:23:f2:Undefined variable 'NotDefined':UNDEFINED -match-class-bind-self:48:17:48:18:f3:Use 'int() as y' instead:HIGH -match-class-bind-self:51:17:51:18:f3:Use 'str() as y' instead:HIGH -match-class-bind-self:54:19:54:20:f3:Use 'tuple() as y' instead:HIGH -match-class-positional-attributes:63:13:63:17:f4:Use keyword attributes instead of positional ones:HIGH -match-class-positional-attributes:65:13:65:20:f4:Use keyword attributes instead of positional ones:HIGH +invalid-match-args-definition:14:21:14:31:C:`__match_args__` must be a tuple of strings.:HIGH +invalid-match-args-definition:17:21:17:29:D:`__match_args__` must be a tuple of strings.:HIGH +too-many-positional-sub-patterns:34:13:34:20:f1:A expects 1 positional sub-patterns (given 2):INFERENCE +too-many-positional-sub-patterns:36:13:36:23:f1:B expects 2 positional sub-patterns (given 3):INFERENCE +too-many-positional-sub-patterns:38:13:38:22:f1:int expects 1 positional sub-patterns (given 2):INFERENCE +multiple-class-sub-patterns:47:13:47:22:f2:Multiple sub-patterns for attribute x:INFERENCE +multiple-class-sub-patterns:49:13:49:29:f2:Multiple sub-patterns for attribute x:INFERENCE +undefined-variable:55:13:55:23:f2:Undefined variable 'NotDefined':UNDEFINED +match-class-bind-self:60:17:60:18:f3:Use 'int() as y' instead:HIGH +match-class-bind-self:63:17:63:18:f3:Use 'str() as y' instead:HIGH +match-class-positional-attributes:73:13:73:17:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:75:13:75:20:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:77:13:77:25:f4:Use keyword attributes instead of positional ones:HIGH From 768c4c96f7ef3223f7b204b06a8a652e11c33c35 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 25 Sep 2025 00:59:05 +0200 Subject: [PATCH 5/8] Exclude tuple subclasses --- pylint/checkers/match_statements_checker.py | 13 +++++++------ tests/functional/m/match_class_pattern.py | 4 ++-- tests/functional/m/match_class_pattern.txt | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index 68b1a69e9f..558712af53 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/pylint/checkers/match_statements_checker.py @@ -20,21 +20,19 @@ # List of builtin classes which match self -# Exclude `dict`, `list` and `tuple` as those could also -# be matched by the mapping or sequence patterns. # https://docs.python.org/3/reference/compound_stmts.html#class-patterns MATCH_CLASS_SELF_NAMES = { "builtins.bool", "builtins.bytearray", "builtins.bytes", - # "builtins.dict", + "builtins.dict", "builtins.float", "builtins.frozenset", "builtins.int", - # "builtins.list", + "builtins.list", "builtins.set", "builtins.str", - # "builtins.tuple", + "builtins.tuple", } @@ -191,7 +189,10 @@ def visit_matchclass(self, node: nodes.MatchClass) -> None: inferred = safe_infer(node.cls) if not ( isinstance(inferred, nodes.ClassDef) - and inferred.qname() in MATCH_CLASS_SELF_NAMES + and ( + inferred.qname() in MATCH_CLASS_SELF_NAMES + or "tuple" in inferred.basenames + ) ): self.add_message( "match-class-positional-attributes", diff --git a/tests/functional/m/match_class_pattern.py b/tests/functional/m/match_class_pattern.py index 8a6ea662ec..f718149a5e 100644 --- a/tests/functional/m/match_class_pattern.py +++ b/tests/functional/m/match_class_pattern.py @@ -37,7 +37,7 @@ def f1(x): case int(1): ... case int(1, 2): ... # [too-many-positional-sub-patterns] case tuple(1): ... - case tuple(1, 2): ... + case tuple(1, 2): ... # [too-many-positional-sub-patterns] case tuple((1, 2)): ... case Result(1, 2): ... @@ -74,5 +74,5 @@ def f4(x): case A(x=1): ... case B(1, 2): ... # [match-class-positional-attributes] case B(x=1, y=2): ... - case Result(1, 2): ... # [match-class-positional-attributes] + case Result(1, 2): ... case Result(x=1, y=2): ... diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index 9284c427ae..45189e530b 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -3,6 +3,7 @@ invalid-match-args-definition:17:21:17:29:D:`__match_args__` must be a tuple of too-many-positional-sub-patterns:34:13:34:20:f1:A expects 1 positional sub-patterns (given 2):INFERENCE too-many-positional-sub-patterns:36:13:36:23:f1:B expects 2 positional sub-patterns (given 3):INFERENCE too-many-positional-sub-patterns:38:13:38:22:f1:int expects 1 positional sub-patterns (given 2):INFERENCE +too-many-positional-sub-patterns:40:13:40:24:f1:tuple expects 1 positional sub-patterns (given 2):INFERENCE multiple-class-sub-patterns:47:13:47:22:f2:Multiple sub-patterns for attribute x:INFERENCE multiple-class-sub-patterns:49:13:49:29:f2:Multiple sub-patterns for attribute x:INFERENCE undefined-variable:55:13:55:23:f2:Undefined variable 'NotDefined':UNDEFINED @@ -10,4 +11,3 @@ match-class-bind-self:60:17:60:18:f3:Use 'int() as y' instead:HIGH match-class-bind-self:63:17:63:18:f3:Use 'str() as y' instead:HIGH match-class-positional-attributes:73:13:73:17:f4:Use keyword attributes instead of positional ones:HIGH match-class-positional-attributes:75:13:75:20:f4:Use keyword attributes instead of positional ones:HIGH -match-class-positional-attributes:77:13:77:25:f4:Use keyword attributes instead of positional ones:HIGH From 5502cd3a5788a1d83bd0d12080bc08c9ae2b285c Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 25 Sep 2025 06:00:47 +0200 Subject: [PATCH 6/8] Minor improvement --- pylint/checkers/match_statements_checker.py | 2 +- tests/functional/m/match_class_pattern.py | 2 ++ tests/functional/m/match_class_pattern.txt | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index 558712af53..8baaf7e46d 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/pylint/checkers/match_statements_checker.py @@ -120,7 +120,7 @@ def visit_match(self, node: nodes.Match) -> None: def visit_matchas(self, node: nodes.MatchAs) -> None: match node: case nodes.MatchAs( - parent=nodes.MatchClass(cls=nodes.Name() as cls_name), + parent=nodes.MatchClass(cls=nodes.Name() as cls_name, patterns=[_]), name=nodes.AssignName(name=name), pattern=None, ): diff --git a/tests/functional/m/match_class_pattern.py b/tests/functional/m/match_class_pattern.py index f718149a5e..3b227c9fdc 100644 --- a/tests/functional/m/match_class_pattern.py +++ b/tests/functional/m/match_class_pattern.py @@ -63,6 +63,8 @@ def f3(x): case str(y): ... # [match-class-bind-self] case str() as y: ... case str("Hello" as y): ... + case tuple(y, 2): ... # pylint: disable=too-many-positional-sub-patterns + case tuple((y, 2)): ... def f4(x): """Check for positional attributes if keywords could be used.""" diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index 45189e530b..76a1759536 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -9,5 +9,5 @@ multiple-class-sub-patterns:49:13:49:29:f2:Multiple sub-patterns for attribute x undefined-variable:55:13:55:23:f2:Undefined variable 'NotDefined':UNDEFINED match-class-bind-self:60:17:60:18:f3:Use 'int() as y' instead:HIGH match-class-bind-self:63:17:63:18:f3:Use 'str() as y' instead:HIGH -match-class-positional-attributes:73:13:73:17:f4:Use keyword attributes instead of positional ones:HIGH -match-class-positional-attributes:75:13:75:20:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:75:13:75:17:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:77:13:77:20:f4:Use keyword attributes instead of positional ones:HIGH From 9ba694be04ccafea5f6def6af00b7156b49d9352 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 25 Sep 2025 11:49:55 +0200 Subject: [PATCH 7/8] Code review --- doc/user_guide/checkers/features.rst | 2 +- pylint/checkers/match_statements_checker.py | 6 +++--- tests/functional/m/match_class_pattern.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index bf159221d5..be6a4dac50 100644 --- a/doc/user_guide/checkers/features.rst +++ b/doc/user_guide/checkers/features.rst @@ -694,7 +694,7 @@ 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' instead* +: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* diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index 8baaf7e46d..f3c4d67d87 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/pylint/checkers/match_statements_checker.py @@ -64,7 +64,7 @@ class MatchStatementChecker(BaseChecker): "attribute in a class pattern.", ), "R1905": ( - "Use '%s' instead", + "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__` " @@ -132,7 +132,7 @@ def visit_matchas(self, node: nodes.MatchAs) -> None: self.add_message( "match-class-bind-self", node=node, - args=(f"{cls_name.name}() as {name}",), + args=(cls_name.name, name), confidence=HIGH, ) @@ -197,7 +197,7 @@ def visit_matchclass(self, node: nodes.MatchClass) -> None: self.add_message( "match-class-positional-attributes", node=node, - confidence=HIGH, + confidence=INFERENCE, ) if ( diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index 76a1759536..affbeb0979 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -9,5 +9,5 @@ multiple-class-sub-patterns:49:13:49:29:f2:Multiple sub-patterns for attribute x undefined-variable:55:13:55:23:f2:Undefined variable 'NotDefined':UNDEFINED match-class-bind-self:60:17:60:18:f3:Use 'int() as y' instead:HIGH match-class-bind-self:63:17:63:18:f3:Use 'str() as y' instead:HIGH -match-class-positional-attributes:75:13:75:17:f4:Use keyword attributes instead of positional ones:HIGH -match-class-positional-attributes:77:13:77:20:f4:Use keyword attributes instead of positional ones:HIGH +match-class-positional-attributes:75:13:75:17:f4:Use keyword attributes instead of positional ones:INFERENCE +match-class-positional-attributes:77:13:77:20:f4:Use keyword attributes instead of positional ones:INFERENCE From f420f288e8e5f0a641f642fddcd8a9cc191922ec Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Thu, 25 Sep 2025 13:06:02 +0200 Subject: [PATCH 8/8] Suggest keyword attributes --- doc/user_guide/checkers/features.rst | 2 +- pylint/checkers/match_statements_checker.py | 34 ++++++++++----------- tests/functional/m/match_class_pattern.txt | 4 +-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index be6a4dac50..ef2424522d 100644 --- a/doc/user_guide/checkers/features.rst +++ b/doc/user_guide/checkers/features.rst @@ -697,7 +697,7 @@ Match Statements checker Messages :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* +: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. diff --git a/pylint/checkers/match_statements_checker.py b/pylint/checkers/match_statements_checker.py index f3c4d67d87..011fb3235a 100644 --- a/pylint/checkers/match_statements_checker.py +++ b/pylint/checkers/match_statements_checker.py @@ -71,7 +71,7 @@ class MatchStatementChecker(BaseChecker): "can be avoided.", ), "R1906": ( - "Use keyword attributes instead of positional ones", + "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.", @@ -184,22 +184,6 @@ def visit_matchclass(self, node: nodes.MatchClass) -> None: attrs: set[str] = set() dups: set[str] = set() - if node.patterns: - if isinstance(node, nodes.MatchClass) and isinstance(node.cls, nodes.Name): - inferred = safe_infer(node.cls) - if not ( - isinstance(inferred, nodes.ClassDef) - and ( - inferred.qname() in MATCH_CLASS_SELF_NAMES - or "tuple" in inferred.basenames - ) - ): - self.add_message( - "match-class-positional-attributes", - node=node, - confidence=INFERENCE, - ) - if ( node.patterns and (match_args := self.get_match_args_for_class(node.cls)) is not None @@ -213,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) diff --git a/tests/functional/m/match_class_pattern.txt b/tests/functional/m/match_class_pattern.txt index affbeb0979..2ab0727468 100644 --- a/tests/functional/m/match_class_pattern.txt +++ b/tests/functional/m/match_class_pattern.txt @@ -9,5 +9,5 @@ multiple-class-sub-patterns:49:13:49:29:f2:Multiple sub-patterns for attribute x undefined-variable:55:13:55:23:f2:Undefined variable 'NotDefined':UNDEFINED match-class-bind-self:60:17:60:18:f3:Use 'int() as y' instead:HIGH match-class-bind-self:63:17:63:18:f3:Use 'str() as y' instead:HIGH -match-class-positional-attributes:75:13:75:17:f4:Use keyword attributes instead of positional ones:INFERENCE -match-class-positional-attributes:77:13:77:20:f4:Use keyword attributes instead of positional ones:INFERENCE +match-class-positional-attributes:75:13:75:17:f4:Use keyword attributes instead of positional ones ('x'):INFERENCE +match-class-positional-attributes:77:13:77:20:f4:Use keyword attributes instead of positional ones ('x', 'y'):INFERENCE