diff --git a/CHANGELOG.md b/CHANGELOG.md index af8d8b0..8df8e17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,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. ## 25.5.0 diff --git a/ERRORCODES.md b/ERRORCODES.md index db2565c..a15c8ff 100644 --- a/ERRORCODES.md +++ b/ERRORCODES.md @@ -97,4 +97,5 @@ 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. +| Y091 | Protocol methods should not have positional-or-keyword parameters. Usually, a positional-only parameter is better. | Correctness +| Y093 | Pseudo-protocols like `Sequence` or `Mapping` should not be used as argument types. Using a protocol is preferred. | Correctness diff --git a/flake8_pyi/errors.py b/flake8_pyi/errors.py index 756b6a2..0f90c67 100644 --- a/flake8_pyi/errors.py +++ b/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"] diff --git a/flake8_pyi/visitor.py b/flake8_pyi/visitor.py index 4bb1cb6..7368aee 100644 --- a/flake8_pyi/visitor.py +++ b/flake8_pyi/visitor.py @@ -138,6 +138,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.""" @@ -748,6 +757,10 @@ 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)) @@ -755,8 +768,7 @@ def _is_valid_pep_604_union_member(node: ast.expr) -> bool: 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) @@ -2108,6 +2120,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) @@ -2124,6 +2137,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: diff --git a/tests/pseudo_protocols.pyi b/tests/pseudo_protocols.pyi new file mode 100644 index 0000000..66a7a60 --- /dev/null +++ b/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.