Add Y091: Protocol method arg should not be pos-or-kw

CHANGELOG.md

@@ -83,6 +83,7 @@ New error codes:
 * Y060, which flags redundant inheritance from `Generic[]`.
 * Y061: Do not use `None` inside a `Literal[]` slice.
   For example, use `Literal["foo"] | None` instead of `Literal["foo", None]`.
+* Introduce Y062: Protocol method parameters should not be positional-or-keyword.
 
 Other changes:
 * The undocumented `pyi.__version__` and `pyi.PyiTreeChecker.version`

ERRORCODES.md

@@ -80,6 +80,7 @@ The following warnings are currently emitted by default:
 | <a id="Y064" href="#Y064">Y064</a> | Use simpler syntax to define final literal types. For example, use `x: Final = 42` instead of `x: Final[Literal[42]]`. | Style
 | <a id="Y065" href="#Y065">Y065</a> | Don't use bare `Incomplete` in argument and return annotations. Instead, leave them unannotated. Omitting an annotation entirely from a function will cause some type checkers to view the parameter or return type as "untyped"; this may result in stricter type-checking on code that makes use of the stubbed function. | Style
 | <a id="Y066" href="#Y066">Y066</a> | When using if/else with `sys.version_info`, put the code for new Python versions first. | Style
+| <a id="Y067" href="#Y066">Y067</a> | Protocol methods should not have positional-or-keyword parameters. Usually, a positional-only parameter is better.
 
 ## Warnings disabled by default
 

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]
@@ -2121,6 +2123,14 @@ 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
+    ) -> None:
+        for pos_or_kw in node.args.args[1:]:  # exclude "self"
+            if pos_or_kw.arg.startswith("__"):
+                continue
+            self.error(pos_or_kw, Y067.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
@@ -2176,6 +2186,8 @@ 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)
+            if self.enclosing_class_ctx.is_protocol_class:
+                self.check_protocol_param_kinds(node)
 
     def visit_arg(self, node: ast.arg) -> None:
         if _is_NoReturn(node.annotation):
@@ -2408,6 +2420,10 @@ def parse_options(options: argparse.Namespace) -> None:
     "Y066 When using if/else with sys.version_info, "
     'put the code for new Python versions first, e.g. "{new_syntax}"'
 )
+Y067 = (
+    'Y067 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"
+)
 Y090 = (
     'Y090 "{original}" means '
     '"a tuple of length 1, in which the sole element is of type {typ!r}". '

tests/protocol_arg.pyi

@@ -0,0 +1,9 @@
+from typing import Protocol
+
+class P(Protocol):
+    def method1(self, arg: int) -> None: ...  # Y067 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: ...
+    def method6(self, arg, /, *, foo: str) -> None: ...