Y093: Detect pseudo-protocols

CHANGELOG.md

@@ -21,6 +21,7 @@ flake8-pyi uses Calendar Versioning (CalVer).
 ### New Error Codes
 
 * Y068: Don't use `@override` in stub files
+* Y092: Pseudo-protocols should not be used as argument types.
 
 ### Other changes
 

ERRORCODES.md

@@ -97,4 +97,5 @@ recommend only using `--extend-select`, never `--select`.
 | Code | Description | Code category
 |------|-------------|---------------
 | <a id="Y090" href="#Y090">Y090</a> | `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
-| <a id="Y091" href="#Y091">Y091</a> | Protocol methods should not have positional-or-keyword parameters. Usually, a positional-only parameter is better.
+| <a id="Y091" href="#Y091">Y091</a> | Protocol methods should not have positional-or-keyword parameters. Usually, a positional-only parameter is better. | Correctness
+| <a id="Y093" href="#Y093">Y093</a> | Pseudo-protocols like `Sequence` or `Mapping` should not be used as argument types. Using a protocol is preferred. | Correctness

flake8_pyi/errors.py

@@ -138,5 +138,8 @@ class Error(NamedTuple):
     '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"
 )
+Y093 = (
+    'Y093 Don\'t use pseudo-protocol "{arg}" as parameter type. Use a protocol instead.'
+)
 
-DISABLED_BY_DEFAULT = ["Y090", "Y091"]
+DISABLED_BY_DEFAULT = ["Y090", "Y091", "Y093"]

flake8_pyi/visitor.py

@@ -139,6 +139,15 @@ def all_equal(iterable: Iterable[object]) -> bool:
     }
 )
 
+PSEUDO_PROTOCOLS = {
+    "Sequence",
+    "MutableSequence",
+    "Mapping",
+    "MutableMapping",
+    "Set",
+    "MutableSet",
+}
+
 
 def _ast_node_for(string: str) -> ast.AST:
     """Helper function for doctests."""
@@ -743,15 +752,18 @@ def _is_valid_default_value_with_annotation(
             return False
 
 
+def _is_pep_604_union(node: ast.AST | None) -> TypeGuard[ast.BinOp]:
+    return isinstance(node, ast.BinOp) and isinstance(node.op, ast.BitOr)
+
+
 def _is_valid_pep_604_union_member(node: ast.expr) -> bool:
     return _is_None(node) or isinstance(node, (ast.Name, ast.Attribute, ast.Subscript))
 
 
 def _is_valid_pep_604_union(node: ast.expr) -> TypeGuard[ast.BinOp]:
     """Does `node` represent a valid PEP-604 union (e.g. `int | str`)?"""
     return (
-        isinstance(node, ast.BinOp)
-        and isinstance(node.op, ast.BitOr)
+        _is_pep_604_union(node)
         and (
             _is_valid_pep_604_union_member(node.left)
             or _is_valid_pep_604_union(node.left)
@@ -2069,6 +2081,7 @@ def visit_arg(self, node: ast.arg) -> None:
             self.error(node, errors.Y050)
         if _is_Incomplete(node.annotation):
             self.error(node, errors.Y065.format(what=f'parameter "{node.arg}"'))
+        self._check_pseudo_protocol(node.annotation)
         with self.visiting_arg.enabled():
             self.generic_visit(node)
 
@@ -2084,6 +2097,19 @@ def visit_arguments(self, node: ast.arguments) -> None:
         if node.kwarg is not None:
             self.visit(node.kwarg)
 
+    def _check_pseudo_protocol(self, node: ast.expr | None) -> None:
+        if node is None:
+            return
+        if isinstance(node, ast.Subscript):
+            self._check_pseudo_protocol(node.value)
+            self._check_pseudo_protocol(node.slice)
+        if _is_pep_604_union(node):
+            self._check_pseudo_protocol(node.left)
+            self._check_pseudo_protocol(node.right)
+        for name in PSEUDO_PROTOCOLS:
+            if _is_object(node, name, from_=_TYPING_OR_COLLECTIONS_ABC):
+                self.error(node, errors.Y093.format(arg=name))
+
     def check_arg_default(self, arg: ast.arg, default: ast.expr | None) -> None:
         self.visit(arg)
         if default is not None:

tests/pseudo_protocols.pyi

@@ -0,0 +1,34 @@
+# flags: --extend-select=Y093
+
+# Tests for pseudo-protocols like `Sequence`, `Mapping`, or `MutableMapping`
+# imported from collections.abc.
+#
+# We're explicitly not testing for imports from typing as that should already
+# trigger Y022 (import from collections.abc instead from typing).
+
+import collections.abc
+from collections.abc import (
+    Iterable,
+    Mapping,
+    Mapping as MyMapping,
+    MutableMapping,
+    Sequence,
+)
+
+def test_sequence(seq: Sequence[int]) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.
+def test_mapping(mapping: Mapping[str, int]) -> None: ...  # Y093 Don't use pseudo-protocol "Mapping" as parameter type. Use a protocol instead.
+def test_mutable_mapping(mapping: MutableMapping[str, int]) -> None: ...  # Y093 Don't use pseudo-protocol "MutableMapping" as parameter type. Use a protocol instead.
+def test_import_alias(mapping: MyMapping[str, int]) -> None: ...  # TODO: import aliases are currently not supported.
+def test_plain(seq: Sequence) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.
+def test_union(arg: Sequence[int] | int) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.
+def test_nested(arg: list[Sequence[int]]) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.
+def test_full_type(seq: collections.abc.Sequence[int]) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.
+
+x: Sequence[int]  # ok
+def test_iterable(it: Iterable[str]) -> None: ...  # ok
+def test_as_return_type() -> Sequence[int]: ...  # ok
+
+class Foo:
+    x: Sequence[int]  # ok
+
+    def test_method(self, seq: Sequence[int]) -> None: ...  # Y093 Don't use pseudo-protocol "Sequence" as parameter type. Use a protocol instead.