diff --git a/CHANGELOG.md b/CHANGELOG.md index 095aaa4..b96bd55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Change Log +## Unreleased + +New error codes: +* Introduce Y091: Protocol method parameters should not be positional-or-keyword. + ## 24.9.0 Bugfixes diff --git a/ERRORCODES.md b/ERRORCODES.md index db82bbd..d1a353c 100644 --- a/ERRORCODES.md +++ b/ERRORCODES.md @@ -95,3 +95,4 @@ recommend only using `--extend-select`, never `--select`. | Code | Description | Code category |------|-------------|--------------- | Y090 | `tuple[int]` means "a tuple of length 1, in which the sole element is of type `int`". Consider using `tuple[int, ...]` instead, which means "a tuple of arbitrary (possibly 0) length, in which all elements are of type `int`". | Correctness +| Y091 | Protocol methods should not have positional-or-keyword parameters. Usually, a positional-only parameter is better. diff --git a/pyi.py b/pyi.py index 4fbb175..aa397cf 100644 --- a/pyi.py +++ b/pyi.py @@ -1742,7 +1742,9 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: self.generic_visit(node) self._check_class_bases(node.bases) self.enclosing_class_ctx = old_context + self.check_class_pass_and_ellipsis(node) + def check_class_pass_and_ellipsis(self, node: ast.ClassDef) -> None: # empty class body should contain "..." not "pass" if len(node.body) == 1: statement = node.body[0] @@ -2086,7 +2088,11 @@ def _check_class_method_for_bad_typevars( ): self._Y019_error(method, cls_typevar) - def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: + def check_self_typevars( + self, + node: ast.FunctionDef | ast.AsyncFunctionDef, + decorator_names: Container[str], + ) -> None: pos_or_keyword_args = node.args.posonlyargs + node.args.args if not pos_or_keyword_args: @@ -2100,12 +2106,6 @@ def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> N if not isinstance(first_arg_annotation, (ast.Name, ast.Subscript)): return - decorator_names = { - decorator.id - for decorator in node.decorator_list - if isinstance(decorator, ast.Name) - } - if "classmethod" in decorator_names or node.name == "__new__": self._check_class_method_for_bad_typevars( method=node, @@ -2121,6 +2121,20 @@ def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> N return_annotation=return_annotation, ) + def check_protocol_param_kinds( + self, + node: ast.FunctionDef | ast.AsyncFunctionDef, + decorator_names: Container[str], + ) -> None: + if "staticmethod" in decorator_names: + relevant_params = node.args.args + else: + relevant_params = node.args.args[1:] # exclude "self" + for pos_or_kw in relevant_params: + if pos_or_kw.arg.startswith("__"): + continue + self.error(pos_or_kw, Y091.format(arg=pos_or_kw.arg, method=node.name)) + @staticmethod def _is_positional_pre_570_argname(name: str) -> bool: # https://peps.python.org/pep-0484/#positional-only-arguments @@ -2175,7 +2189,14 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: self._check_pep570_syntax_used_where_applicable(node) if self.enclosing_class_ctx is not None: - self.check_self_typevars(node) + decorator_names = { + decorator.id + for decorator in node.decorator_list + if isinstance(decorator, ast.Name) + } + self.check_self_typevars(node, decorator_names) + if self.enclosing_class_ctx.is_protocol_class: + self.check_protocol_param_kinds(node, decorator_names) def visit_arg(self, node: ast.arg) -> None: if _is_NoReturn(node.annotation): @@ -2413,5 +2434,9 @@ def parse_options(options: argparse.Namespace) -> None: '"a tuple of length 1, in which the sole element is of type {typ!r}". ' 'Perhaps you meant "{new}"?' ) +Y091 = ( + 'Y091 Argument "{arg}" to protocol method "{method}" should probably not be positional-or-keyword. ' + "Make it positional-only, since usually you don't want to mandate a specific argument name" +) -DISABLED_BY_DEFAULT = ["Y090"] +DISABLED_BY_DEFAULT = ["Y090", "Y091"] diff --git a/tests/protocol_arg.pyi b/tests/protocol_arg.pyi new file mode 100644 index 0000000..4d31cc7 --- /dev/null +++ b/tests/protocol_arg.pyi @@ -0,0 +1,14 @@ +# flags: --extend-select=Y091 +from typing import Protocol + +class P(Protocol): + def method1(self, arg: int) -> None: ... # Y091 Argument "arg" to protocol method "method1" should probably not be positional-or-keyword. Make it positional-only, since usually you don't want to mandate a specific argument name + def method2(self, arg: str, /) -> None: ... + def method3(self, *, arg: str) -> None: ... + def method4(self, arg: int, /) -> None: ... + def method5(self, arg: int, /, *, foo: str) -> None: ... + # Ensure Y091 recognizes this as pos-only for the benefit of users still + # using the old syntax. + def method6(self, __arg: int) -> None: ... # Y063 Use PEP-570 syntax to indicate positional-only arguments + @staticmethod + def smethod1(arg: int) -> None: ... # Y091 Argument "arg" to protocol method "smethod1" should probably not be positional-or-keyword. Make it positional-only, since usually you don't want to mandate a specific argument name