Skip to content

Forbid imports of collections.abc aliases from typing #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@
# E701 multiple statements on one line (colon) -- disallows "..." on the same line
# E704 multiple statements on one line (def) -- disallows function body on the same line as the def
#
# tests/imports.pyi adds the following ignore codes for that specific file:
# F401 module imported but unused
# F811 redefinition of unused name
#
# tests/type_comments.pyi adds the following ignore codes for that specific file:
# F723 syntax error in type comment
# E261 at least two spaces before inline comment
Expand All @@ -32,5 +28,4 @@ select = B,C,E,F,W,Y,B9
per-file-ignores =
*.py: E203, E501, W503, W291, W293
*.pyi: E301, E302, E305, E501, E701, E704, W503
tests/imports.pyi: E301, E302, E305, E501, E701, E704, F401, F811, W503
tests/type_comments.pyi: E261, E262, E301, E302, E305, E501, E701, E704, F723, W503
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

Features:
* Expand Y027 check to prohibit importing any objects from the `typing` module that are
aliases for objects living `collections.abc` (except for `typing.AbstractSet`, which
is special-cased).
* Introduce Y038: Use `from collections.abc import Set as AbstractSet` instead of
`from typing import AbstractSet`.

Bugfixes:
* Improve inaccurate error messages for Y036.

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,12 @@ currently emitted:
| Y035 | `__all__` and `__match_args__` in a stub file should always have values, as these special variables in a `.pyi` file have identical semantics to `__all__` and `__match_args__` in a `.py` file. E.g. write `__all__ = ["foo", "bar"]` instead of `__all__: list[str]`.
| Y036 | Y036 detects common errors in `__exit__` and `__aexit__` methods. For example, the first argument in an `__exit__` method should either be annotated with `object` or `type[BaseException] \| None`.
| Y037 | Use PEP 604 syntax instead of `typing.Union` and `typing.Optional`. E.g. use `str \| int` instead of `Union[str, int]`, and use `str \| None` instead of `Optional[str]`.
| Y038 | Use `from collections.abc import Set as AbstractSet` instead of `from typing import AbstractSet`. Like Y027, this error code should be switched off in your config file if your stubs support Python 2.

Many error codes enforce modern conventions, and some cannot yet be used in
all cases:

* Y027 is incompatible with Python 2 and should only be used in stubs
* Y027 and Y038 are incompatible with Python 2. These should only be used in stubs
that are meant only for Python 3.
* Y037 (enforcing PEP 604 syntax everywhere) is not yet fully compatible with
the mypy type checker, which has
Expand Down
166 changes: 112 additions & 54 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,68 +58,93 @@ class TypeVarInfo(NamedTuple):
_MAPPING_SLICE = "KeyType, ValueType"
_TYPE_SLICE = "MyClass"
_COUNTER_SLICE = "KeyType"
_CONTEXTLIB_SLICE = "T"
_SET_SLICE = "T"
_SEQUENCE_SLICE = "T"
_COROUTINE_SLICE = "YieldType, SendType, ReturnType"
_ASYNCGEN_SLICE = "YieldType, SendType"


# ChainMap and AsyncContextManager do not exist in typing or typing_extensions in Python 2,
# so we can disallow importing them from anywhere except collections and contextlib respectively.
# A Python 2-compatible check
_BAD_Y022_IMPORTS = {
# typing aliases for collections
"typing.Counter": ("collections.Counter", _COUNTER_SLICE),
"typing.Deque": ("collections.deque", _SEQUENCE_SLICE),
"typing.Deque": ("collections.deque", "T"),
"typing.DefaultDict": ("collections.defaultdict", _MAPPING_SLICE),
"typing.ChainMap": ("collections.ChainMap", _MAPPING_SLICE),
# typing aliases for builtins
"typing.Dict": ("dict", _MAPPING_SLICE),
"typing.FrozenSet": ("frozenset", _SET_SLICE),
"typing.List": ("list", _SEQUENCE_SLICE),
"typing.Set": ("set", _SET_SLICE),
"typing.FrozenSet": ("frozenset", "T"),
"typing.List": ("list", "T"),
"typing.Set": ("set", "T"),
"typing.Tuple": ("tuple", "Foo, Bar"),
"typing.Type": ("type", _TYPE_SLICE),
# One typing alias for contextlib
"typing.AsyncContextManager": (
"contextlib.AbstractAsyncContextManager",
_CONTEXTLIB_SLICE,
),
"typing.AsyncContextManager": ("contextlib.AbstractAsyncContextManager", "T"),
# typing_extensions aliases for collections
"typing_extensions.Counter": ("collections.Counter", _COUNTER_SLICE),
"typing_extensions.Deque": ("collections.deque", _SEQUENCE_SLICE),
"typing_extensions.Deque": ("collections.deque", "T"),
"typing_extensions.DefaultDict": ("collections.defaultdict", _MAPPING_SLICE),
"typing_extensions.ChainMap": ("collections.ChainMap", _MAPPING_SLICE),
# One typing_extensions alias for a builtin
"typing_extensions.Type": ("type", _TYPE_SLICE),
# one typing_extensions alias for contextlib
"typing_extensions.AsyncContextManager": (
"contextlib.AbstractAsyncContextManager",
_CONTEXTLIB_SLICE,
"T",
),
}

# typing_extensions.ContextManager is omitted from the Y023 and Y027 collections - special-cased
# We use `None` to signify that the object shouldn't be parameterised.
_BAD_Y023_IMPORTS = {
# collections.abc aliases
# Objects you should import from collections.abc/typing instead of typing_extensions
# A Python 2-compatible check
_BAD_COLLECTIONSABC_Y023_IMPORTS = {
"Awaitable": "T",
"Coroutine": "YieldType, SendType, ReturnType",
"Coroutine": _COROUTINE_SLICE,
"AsyncIterable": "T",
"AsyncIterator": "T",
"AsyncGenerator": "YieldType, SendType",
# typing aliases
"Protocol": None,
"runtime_checkable": None,
"ClassVar": "T",
"NewType": None,
"overload": None,
"Text": None,
"NoReturn": None,
"AsyncGenerator": _ASYNCGEN_SLICE,
}
_BAD_TYPING_Y023_IMPORTS = frozenset(
{
"Protocol",
"runtime_checkable",
"NewType",
"overload",
"Text",
"NoReturn",
# ClassVar deliberately omitted, as it's the only one in this group that should be parameterised
# It is special-case elsewhere
}
)

# Objects you should import from collections.abc instead of typing(_extensions)
# A Python 2-incompatible check
# typing.AbstractSet is deliberately omitted (special-cased)
# We use `None` to signify that the object shouldn't be parameterised.
_BAD_Y027_IMPORTS = {
"typing.ContextManager": ("contextlib.AbstractContextManager", _CONTEXTLIB_SLICE),
"typing.OrderedDict": ("collections.OrderedDict", _MAPPING_SLICE),
"typing_extensions.OrderedDict": ("collections.OrderedDict", _MAPPING_SLICE),
"ByteString": None,
"Collection": "T",
"ItemsView": _MAPPING_SLICE,
"KeysView": "KeyType",
"Mapping": _MAPPING_SLICE,
"MappingView": None,
"MutableMapping": _MAPPING_SLICE,
"MutableSequence": "T",
"MutableSet": "T",
"Sequence": "T",
"ValuesView": "ValueType",
"Iterable": "T",
"Iterator": "T",
"Generator": "YieldType, SendType, ReturnType",
"Hashable": None,
"Reversible": "T",
"Sized": None,
"Coroutine": _COROUTINE_SLICE,
"AsyncGenerator": _ASYNCGEN_SLICE,
"AsyncIterator": "T",
"AsyncIterable": "T",
"Awaitable": "T",
"Callable": None,
"Container": "T",
}


Expand Down Expand Up @@ -586,6 +611,38 @@ def __init__(self, filename: Path | None = None) -> None:
def __repr__(self) -> str:
return f"{self.__class__.__name__}(filename={self.filename!r})"

@staticmethod
def _get_Y023_error_message(object_name: str) -> str | None:
"""
Return the appropriate error message for a bad import/attribute-access from typing_extensions.
Return `None` if it's an OK import/attribute-access.
"""
if object_name in _BAD_COLLECTIONSABC_Y023_IMPORTS:
slice_contents = _BAD_COLLECTIONSABC_Y023_IMPORTS[object_name]
suggestion = (
f'"collections.abc.{object_name}[{slice_contents}]" '
f'(or "typing.{object_name}[{slice_contents}]" '
f"in Python 2-compatible code)"
)
bad_syntax = f'"typing_extensions.{object_name}[{slice_contents}]"'
elif object_name in _BAD_TYPING_Y023_IMPORTS:
suggestion = f'"typing.{object_name}"'
bad_syntax = f'"typing_extensions.{object_name}"'
elif object_name == "ClassVar":
suggestion = '"typing.ClassVar[T]"'
bad_syntax = '"typing_extensions.ClassVar[T]"'
elif object_name == "ContextManager":
suggestion = (
'"contextlib.AbstractContextManager[T]" '
'(or "typing.ContextManager[T]" '
"in Python 2-compatible code)"
)
bad_syntax = '"typing_extensions.ContextManager[T]"'
else:
return None

return Y023.format(good_syntax=suggestion, bad_syntax=bad_syntax)

def _check_import_or_attribute(
self, node: ast.Attribute | ast.ImportFrom, module_name: str, object_name: str
) -> None:
Expand All @@ -600,35 +657,31 @@ def _check_import_or_attribute(
)

# Y027 errors
elif fullname in _BAD_Y027_IMPORTS:
good_cls_name, params = _BAD_Y027_IMPORTS[fullname]
elif module_name == "typing" and object_name in _BAD_Y027_IMPORTS:
slice_contents = _BAD_Y027_IMPORTS[object_name]
params = "" if slice_contents is None else f"[{slice_contents}]"
error_message = Y027.format(
good_syntax=f'"{good_cls_name}[{params}]"',
bad_syntax=f'"{fullname}[{params}]"',
good_syntax=f'"collections.abc.{object_name}{params}"',
bad_syntax=f'"typing.{object_name}{params}"',
)
elif module_name in _TYPING_MODULES and object_name == "OrderedDict":
error_message = Y027.format(
good_syntax=f'"collections.OrderedDict[{_MAPPING_SLICE}]"',
bad_syntax=f'"{fullname}[{_MAPPING_SLICE}]"',
)
elif fullname == "typing.ContextManager":
error_message = Y027.format(
good_syntax='"contextlib.AbstractContextManager[T]"',
bad_syntax='"typing.ContextManager[T]"',
)

# Y023 errors
elif module_name == "typing_extensions":
if object_name in _BAD_Y023_IMPORTS:
slice_contents = _BAD_Y023_IMPORTS[object_name]
params = "" if slice_contents is None else f"[{slice_contents}]"
error_message = Y023.format(
good_syntax=f'"typing.{object_name}{params}"',
bad_syntax=f'"typing_extensions.{object_name}{params}"',
)
elif object_name == "ContextManager":
suggested_syntax = (
f'"contextlib.AbstractContextManager[{_CONTEXTLIB_SLICE}]" '
f'(or "typing.ContextManager[{_CONTEXTLIB_SLICE}]" '
f"in Python 2-compatible code)"
)
error_message = Y023.format(
good_syntax=suggested_syntax,
bad_syntax=f'"typing_extensions.ContextManager[{_CONTEXTLIB_SLICE}]"',
)
error_message += " (PEP 585 syntax)"
else:
analysis = self._get_Y023_error_message(object_name)
if analysis is None:
return
else:
error_message = analysis

# Y024 errors
elif fullname == "collections.namedtuple":
Expand All @@ -639,7 +692,6 @@ def _check_import_or_attribute(
error_message = Y037.format(
old_syntax=fullname, example='"int | None" instead of "Optional[int]"'
)

elif fullname == "typing.Union":
error_message = Y037.format(
old_syntax=fullname, example='"int | str" instead of "Union[int, str]"'
Expand Down Expand Up @@ -677,6 +729,11 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
node=node, module_name=module_name, object_name=obj.name
)

if module_name == "typing" and any(
obj.name == "AbstractSet" for obj in imported_objects
):
self.error(node, Y038)

def _check_assignment_to_function(
self, node: ast.Assign, function: ast.expr, object_name: str
) -> None:
Expand Down Expand Up @@ -1531,3 +1588,4 @@ def parse_options(
Y035 = 'Y035 "{var}" in a stub file must have a value, as it has the same semantics as "{var}" at runtime.'
Y036 = "Y036 Badly defined {method_name} method: {details}"
Y037 = "Y037 Use PEP 604 union types instead of {old_syntax} (e.g. {example})."
Y038 = 'Y038 Use "from collections.abc import Set as AbstractSet" instead of "from typing import AbstractSet" (PEP 585 syntax)'
10 changes: 5 additions & 5 deletions tests/classdefs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import abc
import collections.abc
import typing
from abc import abstractmethod
from typing import Any, AsyncIterator, Iterator, overload
from collections.abc import AsyncIterator, Iterator
from typing import Any, overload

import typing_extensions
from _typeshed import Self
from typing_extensions import final

Expand Down Expand Up @@ -69,14 +69,14 @@ class InvalidButPluginDoesNotCrash:
class BadIterator1(Iterator[int]):
def __iter__(self) -> Iterator[int]: ... # Y034 "__iter__" methods in classes like "BadIterator1" usually return "self" at runtime. Consider using "_typeshed.Self" in "BadIterator1.__iter__", e.g. "def __iter__(self: Self) -> Self: ..."

class BadIterator2(typing.Iterator[int]):
class BadIterator2(typing.Iterator[int]): # Y027 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax)
def __iter__(self) -> Iterator[int]: ... # Y034 "__iter__" methods in classes like "BadIterator2" usually return "self" at runtime. Consider using "_typeshed.Self" in "BadIterator2.__iter__", e.g. "def __iter__(self: Self) -> Self: ..."

class BadIterator3(typing_extensions.Iterator[int]):
class BadIterator3(typing.Iterator[int]): # Y027 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax)
def __iter__(self) -> collections.abc.Iterator[int]: ... # Y034 "__iter__" methods in classes like "BadIterator3" usually return "self" at runtime. Consider using "_typeshed.Self" in "BadIterator3.__iter__", e.g. "def __iter__(self: Self) -> Self: ..."

class BadAsyncIterator(collections.abc.AsyncIterator[str]):
def __aiter__(self) -> typing.AsyncIterator[str]: ... # Y034 "__aiter__" methods in classes like "BadAsyncIterator" usually return "self" at runtime. Consider using "_typeshed.Self" in "BadAsyncIterator.__aiter__", e.g. "def __aiter__(self: Self) -> Self: ..."
def __aiter__(self) -> typing.AsyncIterator[str]: ... # Y034 "__aiter__" methods in classes like "BadAsyncIterator" usually return "self" at runtime. Consider using "_typeshed.Self" in "BadAsyncIterator.__aiter__", e.g. "def __aiter__(self: Self) -> Self: ..." # Y027 Use "collections.abc.AsyncIterator[T]" instead of "typing.AsyncIterator[T]" (PEP 585 syntax)

class Abstract(Iterator[str]):
@abstractmethod
Expand Down
2 changes: 1 addition & 1 deletion tests/exit_methods.pyi
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import types
import typing
from collections.abc import Awaitable
from types import TracebackType
from typing import ( # Y022 Use "type[MyClass]" instead of "typing.Type[MyClass]" (PEP 585 syntax)
Any,
Awaitable,
Type,
)

Expand Down
Loading