Forbid imports of collections.abc aliases from typing (#213)

A regression test for python/typeshed#7635

Refs #46 (but doesn't quite close it).

Changes made:
* Expand Y027 to cover all objects in typing that are aliases to objects in collections.abc, except for AbstractSet
* Add new Y038 error code to forbid import AbstractSet from typing
* Change the error message in Y023 to make it clear that imports from collections.abc are now preferred to imports from typing.
* Refactor Y023 logic in pyi.py so as to quell flake8 from complaining that _check_import_or_attribute was getting too complex.
* Some small refactorings of test files to make them compatible with the new checks.

Author: AlexWaygood

.flake8

@@ -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
@@ -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

CHANGELOG.md

@@ -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.
 

README.md

@@ -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

pyi.py

@@ -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",
 }
 
 
@@ -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:
@@ -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":
@@ -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]"'
@@ -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:
@@ -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)'

tests/classdefs.pyi

@@ -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
 
@@ -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

tests/exit_methods.pyi

@@ -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,
 )