From 03775f86a80db5e40bd6850171bf7c1410db5285 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 15:06:58 +0000 Subject: [PATCH 01/18] Introduce `Y022` and `Y023`: checking various imports --- .flake8 | 2 + CHANGELOG.rst | 2 + README.rst | 6 + pyi.py | 109 ++++++++++++++++++ tests/del.pyi | 4 +- tests/imports.pyi | 105 +++++++++++++++++ .../vanilla_flake8_not_clean_forward_refs.pyi | 4 +- 7 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 tests/imports.pyi diff --git a/.flake8 b/.flake8 index 72e5313e..31c4d73e 100644 --- a/.flake8 +++ b/.flake8 @@ -5,3 +5,5 @@ ignore = E302, E501, E701, W503, E203 max-line-length = 80 max-complexity = 12 select = B,C,E,F,W,Y,B9 +per-file-ignores = + tests/imports.pyi: F401, F811 \ No newline at end of file diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 650d4625..0c4133c2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,6 +19,8 @@ unreleased * introduce Y016 (duplicate union member) * support Python 3.10 * discontinue support for Python 3.6 +* introduce Y022 (banning various imports) +* introduce Y023 (always alias ``collections.abc.Set``) 20.10.0 ~~~~~~~ diff --git a/README.rst b/README.rst index 3f555282..0cc5ba43 100644 --- a/README.rst +++ b/README.rst @@ -81,6 +81,12 @@ currently emitted: an instance of ``cls``, and ``__new__`` methods. * Y020: Quoted annotations should never be used in stubs. * Y021: Docstrings should not be included in stubs. +* Y022: Use typing-module aliases to stdlib objects as little as possible + (``builtins.list`` instead of ``typing.List``, etc.). For typing-only objects + such as ``ClassVar`` or ``NoReturn``, import objects from ``typing`` instead + of ``typing_extensions`` wherever possible. +* Y023: Always alias ``collections.abc.Set`` when importing it, so as to avoid + confusion with ``builtins.set``. The following warnings are disabled by default: diff --git a/pyi.py b/pyi.py index 7df4633f..0ee430e7 100644 --- a/pyi.py +++ b/pyi.py @@ -42,6 +42,66 @@ class TypeVarInfo(NamedTuple): name: str +# for both typing and typing_extensions +_BAD_COLLECTIONS_ALIASES = { + "Counter": "Counter", + "Deque": "deque", + "DefaultDict": "defaultdict", + "ChainMap": "ChainMap", + "OrderedDict": "OrderedDict", +} +_BAD_COLLECTIONS_ALIASES = { + alias: f'"collections.{cls}"' for alias, cls in _BAD_COLLECTIONS_ALIASES.items() +} + +# Just for typing +_BAD_BUILTINS_ALIASES = { + alias: f'"builtins.{alias.lower()}"' + for alias in ("Dict", "Frozenset", "List", "Set", "Tuple", "Type") +} + +# Just for typing_extensions +_BAD_CONTEXTLIB_ALIASES = { + alias: f'"contextlib.Abstract{alias}" or "typing.{alias}"' + for alias in ("ContextManager", "AsyncContextManager") +} +_BAD_COLLECTIONS_ABC_ALIASES = { + alias: f'"collections.abc.{alias}" or "typing.{alias}"' + for alias in ( + "Awaitable", + "Coroutine", + "AsyncIterable", + "AsyncIterator", + "AsyncGenerator", + ) +} +_TYPING_NOT_TYPING_EXTENSIONS = { + alias: f'"typing.{alias}"' + for alias in ( + "Protocol", + "runtime_checkable", + "ClassVar", + "NewType", + "overload", + "Text", + "NoReturn", + ) +} + + +# collections.abc.Set is dealt with separately as special cases +FORBIDDEN_IMPORTS_MAPPING = { + "typing": {**_BAD_BUILTINS_ALIASES, **_BAD_COLLECTIONS_ALIASES}, + "typing_extensions": { + **_BAD_COLLECTIONS_ABC_ALIASES, + **_BAD_CONTEXTLIB_ALIASES, + **_BAD_COLLECTIONS_ALIASES, + **_TYPING_NOT_TYPING_EXTENSIONS, + }, + "collections": {"namedtuple": '"typing.NamedTuple"'}, +} + + class PyiAwareFlakesChecker(FlakesChecker): def deferHandleNode(self, node, parent): self.deferFunction(lambda: self.handleNode(node, parent)) @@ -187,6 +247,50 @@ def in_class(self) -> bool: """Determine whether we are inside a `class` statement""" return bool(self._class_nesting) + def _Y022_check( + self, + node: ast.Attribute | ast.alias, + object_name: str, + module_name: str, + blacklist: dict[str, str], + ) -> None: + if object_name in blacklist: + error_message = Y022.format( + good_cls_name=blacklist[object_name], + bad_cls_alias=f'"{module_name}.{object_name}"', + ) + self.error(node, error_message) + + def visit_Attribute(self, node: ast.Attribute) -> None: + self.generic_visit(node) + thing = node.value + if not isinstance(thing, ast.Name): + return + thingname = thing.id + if thingname in FORBIDDEN_IMPORTS_MAPPING: + self._Y022_check( + node=node, + object_name=node.attr, + module_name=thingname, + blacklist=FORBIDDEN_IMPORTS_MAPPING[thingname], + ) + + def visit_ImportFrom(self, node: ast.ImportFrom) -> None: + module_name, imported_objects = node.module, node.names + if module_name in FORBIDDEN_IMPORTS_MAPPING: + forbidden_imports = FORBIDDEN_IMPORTS_MAPPING[module_name] + for obj in imported_objects: + self._Y022_check( + node=obj, + object_name=obj.name, + module_name=module_name, + blacklist=forbidden_imports, + ) + elif module_name == "collections.abc": + for obj in imported_objects: + if obj.name == "Set" and obj.asname != "AbstractSet": + self.error(obj, Y023) + def visit_Assign(self, node: ast.Assign) -> None: if self.in_function: # We error for unexpected things within functions separately. @@ -756,6 +860,11 @@ def should_warn(self, code): Y019 = 'Y019 Use "_typeshed.Self" instead of "{typevar_name}"' Y020 = "Y020 Quoted annotations should never be used in stubs" Y021 = "Y021 Docstrings should not be included in stubs" +Y022 = "Y022 Use {good_cls_name} instead of {bad_cls_alias}" +Y023 = ( + 'Y023 Use "from collections.abc import Set as AbstractSet"' + 'to avoid confusion with "builtins.set"' +) Y092 = "Y092 Top-level attribute must not have a default value" Y093 = "Y093 Use typing_extensions.TypeAlias for type aliases" diff --git a/tests/del.pyi b/tests/del.pyi index 0909c978..d92a95a0 100644 --- a/tests/del.pyi +++ b/tests/del.pyi @@ -1,7 +1,7 @@ -from typing import List, Union +from typing import Union -ManyStr = List[EitherStr] +ManyStr = list[EitherStr] EitherStr = Union[str, bytes] diff --git a/tests/imports.pyi b/tests/imports.pyi new file mode 100644 index 00000000..f35870c8 --- /dev/null +++ b/tests/imports.pyi @@ -0,0 +1,105 @@ +# NOTE: F401 & F811 are ignored in this file in the .flake8 config file + +# GOOD IMPORTS +from collections import ChainMap, Counter, OrderedDict, UserDict, UserList, UserString, defaultdict, deque +from collections.abc import ( + Awaitable, + Coroutine, + AsyncIterable, + AsyncIterator, + AsyncGenerator, + Hashable, + Iterable, + Iterator, + Generator, + Reversible, + Set as AbstractSet, + Sized, + Container, + Callable, + Collection, + MutableSet, + MutableMapping, + MappingView, + KeysView, + ItemsView, + ValuesView, + Sequence, + MutableSequence, + ByteString +) + +# Things that are of no use for stub files are intentionally omitted. +from typing import ( + Any, + Callable, + ClassVar, + Generic, + Optional, + Protocol, + TypeVar, + Union, + AbstractSet, + ByteString, + Container, + Hashable, + ItemsView, + Iterable, + Iterator, + KeysView, + Mapping, + MappingView, + MutableMapping, + MutableSequence, + MutableSet, + Sequence, + Sized, + ValuesView, + Awaitable, + AsyncIterator, + AsyncIterable, + Coroutine, + Collection, + AsyncGenerator, + Reversible, + SupportsAbs, + SupportsBytes, + SupportsComplex, + SupportsFloat, + SupportsIndex, + SupportsInt, + SupportsRound, + Generator, + BinaryIO, + IO, + NamedTuple, + Match, + Pattern, + TextIO, + AnyStr, + NewType, + NoReturn, + overload +) +from typing_extensions import ( + Concatenate, + Final, + ParamSpec, + SupportsIndex, + final, + Literal, + TypeAlias, + TypeGuard, + Annotated, + TypedDict +) + + +# BAD IMPORTS +from collections import namedtuple # Y022 Use "typing.NamedTuple" instead of "collections.namedtuple" +from typing import Dict # Y022 Use "builtins.dict" instead of "typing.Dict" +from typing import Counter # Y022 Use "collections.Counter" instead of "typing.Counter" +from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" +from typing_extensions import ClassVar # Y022 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" +from typing_extensions import Awaitable # Y022 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" +from typing_extensions import ContextManager # Y022 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" diff --git a/tests/vanilla_flake8_not_clean_forward_refs.pyi b/tests/vanilla_flake8_not_clean_forward_refs.pyi index 3702e208..379defb4 100644 --- a/tests/vanilla_flake8_not_clean_forward_refs.pyi +++ b/tests/vanilla_flake8_not_clean_forward_refs.pyi @@ -1,8 +1,8 @@ # flags: --no-pyi-aware-file-checker -from typing import List, Union +from typing import Union -ManyStr = List[EitherStr] # F821 undefined name 'EitherStr' +ManyStr = list[EitherStr] # F821 undefined name 'EitherStr' EitherStr = Union[str, bytes] From 08d6a611aa5719cbadf6d3370fff9887d20fe500 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 15:14:34 +0000 Subject: [PATCH 02/18] Add test, fix string --- pyi.py | 2 +- tests/imports.pyi | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pyi.py b/pyi.py index 0ee430e7..7c3eb29a 100644 --- a/pyi.py +++ b/pyi.py @@ -862,7 +862,7 @@ def should_warn(self, code): Y021 = "Y021 Docstrings should not be included in stubs" Y022 = "Y022 Use {good_cls_name} instead of {bad_cls_alias}" Y023 = ( - 'Y023 Use "from collections.abc import Set as AbstractSet"' + 'Y023 Use "from collections.abc import Set as AbstractSet" ' 'to avoid confusion with "builtins.set"' ) Y092 = "Y092 Top-level attribute must not have a default value" diff --git a/tests/imports.pyi b/tests/imports.pyi index f35870c8..67f0c64d 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -103,3 +103,4 @@ from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" from typing_extensions import ClassVar # Y022 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" from typing_extensions import Awaitable # Y022 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" from typing_extensions import ContextManager # Y022 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" +from collections.abc import Set # Y023 Use "from collections.abc import Set as AbstractSet" to avoid confusion with "builtins.set" From a3e49bebd006cf0d3dac76c0f5223494bef6131a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:17:37 +0000 Subject: [PATCH 03/18] Python 2 compat --- pyi.py | 27 ++++++++++++++++++++------- tests/imports.pyi | 9 +++++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pyi.py b/pyi.py index 7c3eb29a..56b80b2f 100644 --- a/pyi.py +++ b/pyi.py @@ -43,12 +43,18 @@ class TypeVarInfo(NamedTuple): # for both typing and typing_extensions +# +# OrderedDict is omitted: +# -- In Python 3, we'd rather import it from collections, not typing or typing_extensions +# -- But in Python 2, it cannot be imported from collections or typing, only from typing_extensions +# +# ChainMap does not exist in typing or typing_extensions in Python 2, +# so we can disallow importing it from anywhere except collections _BAD_COLLECTIONS_ALIASES = { "Counter": "Counter", "Deque": "deque", "DefaultDict": "defaultdict", "ChainMap": "ChainMap", - "OrderedDict": "OrderedDict", } _BAD_COLLECTIONS_ALIASES = { alias: f'"collections.{cls}"' for alias, cls in _BAD_COLLECTIONS_ALIASES.items() @@ -61,10 +67,10 @@ class TypeVarInfo(NamedTuple): } # Just for typing_extensions -_BAD_CONTEXTLIB_ALIASES = { - alias: f'"contextlib.Abstract{alias}" or "typing.{alias}"' - for alias in ("ContextManager", "AsyncContextManager") -} +# +# collections.abc aliases: none of these exist in typing or typing_extensions in Python 2, +# so we disallow importing them from typing_extensions. +# We can't disallow importing collections.abc aliases from typing yet due to mypy/pytype errors. _BAD_COLLECTIONS_ABC_ALIASES = { alias: f'"collections.abc.{alias}" or "typing.{alias}"' for alias in ( @@ -91,12 +97,19 @@ class TypeVarInfo(NamedTuple): # collections.abc.Set is dealt with separately as special cases FORBIDDEN_IMPORTS_MAPPING = { - "typing": {**_BAD_BUILTINS_ALIASES, **_BAD_COLLECTIONS_ALIASES}, + "typing": { + **_BAD_BUILTINS_ALIASES, + **_BAD_COLLECTIONS_ALIASES, + # AsyncContextManager doesn't exist at all in Python 2, so we can disallow importing it from typing + 'AsyncContextManager': '"contextlib.AbstractAsyncContextManager"' + }, "typing_extensions": { **_BAD_COLLECTIONS_ABC_ALIASES, - **_BAD_CONTEXTLIB_ALIASES, **_BAD_COLLECTIONS_ALIASES, **_TYPING_NOT_TYPING_EXTENSIONS, + # ContextManager doesn't exist in contextlib in Python 2, so we have to allow importing it from typing + # We can disallow importing it from typing_extensions, however + 'ContextManager': '"contextlib.AbstractContextManager" or "typing.ContextManager"' }, "collections": {"namedtuple": '"typing.NamedTuple"'}, } diff --git a/tests/imports.pyi b/tests/imports.pyi index 67f0c64d..f5e84ca9 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -79,7 +79,8 @@ from typing import ( AnyStr, NewType, NoReturn, - overload + overload, + ContextManager # ContextManager must be importable from typing (but not typing_extensions) for Python 2 compabitility ) from typing_extensions import ( Concatenate, @@ -91,7 +92,8 @@ from typing_extensions import ( TypeAlias, TypeGuard, Annotated, - TypedDict + TypedDict, + OrderedDict # OrderedDict must be importable from typing_extensions (but not typing) for Python 2 compatibility ) @@ -99,8 +101,11 @@ from typing_extensions import ( from collections import namedtuple # Y022 Use "typing.NamedTuple" instead of "collections.namedtuple" from typing import Dict # Y022 Use "builtins.dict" instead of "typing.Dict" from typing import Counter # Y022 Use "collections.Counter" instead of "typing.Counter" +from typing import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" +from typing import ChainMap # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" from typing_extensions import ClassVar # Y022 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" from typing_extensions import Awaitable # Y022 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" from typing_extensions import ContextManager # Y022 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" +from typing_extensions import ChainMap # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" from collections.abc import Set # Y023 Use "from collections.abc import Set as AbstractSet" to avoid confusion with "builtins.set" From 3c4fb425ff84180b991ca7828bc9487d4eaca1dd Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:31:30 +0000 Subject: [PATCH 04/18] `contextlib` details, improve comments --- pyi.py | 22 ++++++++++++++-------- tests/imports.pyi | 1 + 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/pyi.py b/pyi.py index 56b80b2f..df0f72b1 100644 --- a/pyi.py +++ b/pyi.py @@ -42,7 +42,7 @@ class TypeVarInfo(NamedTuple): name: str -# for both typing and typing_extensions +# The collections import blacklist is for typing & typing_extensions # # OrderedDict is omitted: # -- In Python 3, we'd rather import it from collections, not typing or typing_extensions @@ -60,16 +60,17 @@ class TypeVarInfo(NamedTuple): alias: f'"collections.{cls}"' for alias, cls in _BAD_COLLECTIONS_ALIASES.items() } -# Just for typing +# Just-for-typing blacklist _BAD_BUILTINS_ALIASES = { alias: f'"builtins.{alias.lower()}"' for alias in ("Dict", "Frozenset", "List", "Set", "Tuple", "Type") } -# Just for typing_extensions +# Just-for-typing_extensions blacklist # # collections.abc aliases: none of these exist in typing or typing_extensions in Python 2, # so we disallow importing them from typing_extensions. +# # We can't disallow importing collections.abc aliases from typing yet due to mypy/pytype errors. _BAD_COLLECTIONS_ABC_ALIASES = { alias: f'"collections.abc.{alias}" or "typing.{alias}"' @@ -95,20 +96,25 @@ class TypeVarInfo(NamedTuple): } -# collections.abc.Set is dealt with separately as special cases +# collections.abc.Set is dealt with separately as a special case +# +# AsyncContextManager doesn't exist at all in Python 2, +# so we can disallow importing it from typing & typing_extensions. +# +# ContextManager doesn't exist in contextlib in Python 2, but does exist in typing, +# so we have to allow importing it from typing. +# We *can* disallow importing it from typing_extensions, however FORBIDDEN_IMPORTS_MAPPING = { "typing": { **_BAD_BUILTINS_ALIASES, **_BAD_COLLECTIONS_ALIASES, - # AsyncContextManager doesn't exist at all in Python 2, so we can disallow importing it from typing 'AsyncContextManager': '"contextlib.AbstractAsyncContextManager"' - }, + }, "typing_extensions": { **_BAD_COLLECTIONS_ABC_ALIASES, **_BAD_COLLECTIONS_ALIASES, **_TYPING_NOT_TYPING_EXTENSIONS, - # ContextManager doesn't exist in contextlib in Python 2, so we have to allow importing it from typing - # We can disallow importing it from typing_extensions, however + 'AsyncContextManager': '"contextlib.AbstractAsyncContextManager"', 'ContextManager': '"contextlib.AbstractContextManager" or "typing.ContextManager"' }, "collections": {"namedtuple": '"typing.NamedTuple"'}, diff --git a/tests/imports.pyi b/tests/imports.pyi index f5e84ca9..14decf0d 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -106,6 +106,7 @@ from typing import ChainMap # Y022 Use "collections.ChainMap" instead of "typin from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" from typing_extensions import ClassVar # Y022 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" from typing_extensions import Awaitable # Y022 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" +from typing_extensions import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" from typing_extensions import ContextManager # Y022 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" from typing_extensions import ChainMap # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" from collections.abc import Set # Y023 Use "from collections.abc import Set as AbstractSet" to avoid confusion with "builtins.set" From 7dbd846b22e5f407ac2c7b01b3e9d1bca945621d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:37:25 +0000 Subject: [PATCH 05/18] Improve README description --- README.rst | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 0cc5ba43..af439c8c 100644 --- a/README.rst +++ b/README.rst @@ -81,10 +81,12 @@ currently emitted: an instance of ``cls``, and ``__new__`` methods. * Y020: Quoted annotations should never be used in stubs. * Y021: Docstrings should not be included in stubs. -* Y022: Use typing-module aliases to stdlib objects as little as possible - (``builtins.list`` instead of ``typing.List``, etc.). For typing-only objects - such as ``ClassVar`` or ``NoReturn``, import objects from ``typing`` instead - of ``typing_extensions`` wherever possible. +* Y022: Imports linting: use typing-module aliases to stdlib objects as little + as possible (e.g. use ``builtins.list`` instead of ``typing.List``). However, + use ``typing.NamedTuple`` instead of ``collections.namedtuple``, as it allows + for more precise type inference. For typing-only objects such as ``ClassVar`` + or ``NoReturn``, import objects from ``typing`` instead of + ``typing_extensions`` wherever possible. * Y023: Always alias ``collections.abc.Set`` when importing it, so as to avoid confusion with ``builtins.set``. From ac9421b73c9fc73421e70c32626561d5fc732761 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:42:59 +0000 Subject: [PATCH 06/18] Black --- pyi.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyi.py b/pyi.py index df0f72b1..6c37a24f 100644 --- a/pyi.py +++ b/pyi.py @@ -108,14 +108,14 @@ class TypeVarInfo(NamedTuple): "typing": { **_BAD_BUILTINS_ALIASES, **_BAD_COLLECTIONS_ALIASES, - 'AsyncContextManager': '"contextlib.AbstractAsyncContextManager"' + "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"', }, "typing_extensions": { **_BAD_COLLECTIONS_ABC_ALIASES, **_BAD_COLLECTIONS_ALIASES, **_TYPING_NOT_TYPING_EXTENSIONS, - 'AsyncContextManager': '"contextlib.AbstractAsyncContextManager"', - 'ContextManager': '"contextlib.AbstractContextManager" or "typing.ContextManager"' + "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"', + "ContextManager": '"contextlib.AbstractContextManager" or "typing.ContextManager"', }, "collections": {"namedtuple": '"typing.NamedTuple"'}, } From 1568c011f1b64f43df4ab04bf96618fed349a1e1 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:46:38 +0000 Subject: [PATCH 07/18] `alias` has no attribute `lineno` --- pyi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyi.py b/pyi.py index 6c37a24f..4de7f0c2 100644 --- a/pyi.py +++ b/pyi.py @@ -300,7 +300,7 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: forbidden_imports = FORBIDDEN_IMPORTS_MAPPING[module_name] for obj in imported_objects: self._Y022_check( - node=obj, + node=node, object_name=obj.name, module_name=module_name, blacklist=forbidden_imports, @@ -308,7 +308,7 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: elif module_name == "collections.abc": for obj in imported_objects: if obj.name == "Set" and obj.asname != "AbstractSet": - self.error(obj, Y023) + self.error(node, Y023) def visit_Assign(self, node: ast.Assign) -> None: if self.in_function: From e2602bdf78056a6be3868e787724c64c9560e42b Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 17:48:08 +0000 Subject: [PATCH 08/18] Correct type annotations --- pyi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyi.py b/pyi.py index 4de7f0c2..9ec366f7 100644 --- a/pyi.py +++ b/pyi.py @@ -268,7 +268,7 @@ def in_class(self) -> bool: def _Y022_check( self, - node: ast.Attribute | ast.alias, + node: ast.Attribute | ast.ImportFrom, object_name: str, module_name: str, blacklist: dict[str, str], From 09a330ed8d9956a056bb070bb73f20ab01d903a9 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 18:01:59 +0000 Subject: [PATCH 09/18] in the middle --- pyi.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pyi.py b/pyi.py index 9ec366f7..6ee8c12e 100644 --- a/pyi.py +++ b/pyi.py @@ -104,7 +104,7 @@ class TypeVarInfo(NamedTuple): # ContextManager doesn't exist in contextlib in Python 2, but does exist in typing, # so we have to allow importing it from typing. # We *can* disallow importing it from typing_extensions, however -FORBIDDEN_IMPORTS_MAPPING = { +FORBIDDEN_Y022_IMPORTS = { "typing": { **_BAD_BUILTINS_ALIASES, **_BAD_COLLECTIONS_ALIASES, @@ -116,10 +116,11 @@ class TypeVarInfo(NamedTuple): **_TYPING_NOT_TYPING_EXTENSIONS, "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"', "ContextManager": '"contextlib.AbstractContextManager" or "typing.ContextManager"', - }, - "collections": {"namedtuple": '"typing.NamedTuple"'}, + } } +FORBIDDEN_ + class PyiAwareFlakesChecker(FlakesChecker): def deferHandleNode(self, node, parent): @@ -276,7 +277,7 @@ def _Y022_check( if object_name in blacklist: error_message = Y022.format( good_cls_name=blacklist[object_name], - bad_cls_alias=f'"{module_name}.{object_name}"', + bad_cls_alias=object_name, ) self.error(node, error_message) @@ -879,9 +880,11 @@ def should_warn(self, code): Y019 = 'Y019 Use "_typeshed.Self" instead of "{typevar_name}"' Y020 = "Y020 Quoted annotations should never be used in stubs" Y021 = "Y021 Docstrings should not be included in stubs" -Y022 = "Y022 Use {good_cls_name} instead of {bad_cls_alias}" -Y023 = ( - 'Y023 Use "from collections.abc import Set as AbstractSet" ' +Y022 = 'Y022 Use {good_cls_name} instead of "typing.{bad_cls_alias}"' +Y023 = 'Y023 Use {good_cls_name} instead of "typing_extensions.{bad_cls_alias}"' +Y024 = 'Y024 Use "typing.NamedTuple" instead of "collections.namedtuple"' +Y025 = ( + 'Y025 Use "from collections.abc import Set as AbstractSet" ' 'to avoid confusion with "builtins.set"' ) Y092 = "Y092 Top-level attribute must not have a default value" From a3836cfa3462c3ebdfc35d83152a21c87d09856a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 23:41:25 +0000 Subject: [PATCH 10/18] Address review, resolve merge conflicts, add tests, fix bugs --- CHANGELOG.rst | 6 ++- README.rst | 12 +++-- pyi.py | 126 +++++++++++++++++++++++----------------------- tests/imports.pyi | 48 +++++++++++++++--- 4 files changed, 114 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 60ae5d02..4ae81f4e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,8 +20,10 @@ unreleased * introduce Y016 (duplicate union member) * support Python 3.10 * discontinue support for Python 3.6 -* introduce Y022 (banning various imports) -* introduce Y023 (always alias ``collections.abc.Set``) +* introduce Y022 (prefer stdlib classes over ``typing`` aliases) +* introduce Y023 (prefer ``typing`` over ``typing_extensions``) +* introduce Y024 (prefer ``typing.NamedTuple`` to ``collections.namedtuple``) +* introduce Y025 (always alias ``collections.abc.Set``) 20.10.0 ~~~~~~~ diff --git a/README.rst b/README.rst index 6d432428..2f28d05a 100644 --- a/README.rst +++ b/README.rst @@ -82,11 +82,13 @@ currently emitted: * Y020: Quoted annotations should never be used in stubs. * Y021: Docstrings should not be included in stubs. * Y022: Imports linting: use typing-module aliases to stdlib objects as little - as possible (e.g. use ``builtins.list`` instead of ``typing.List``). However, - use ``typing.NamedTuple`` instead of ``collections.namedtuple``, as it allows - for more precise type inference. For typing-only objects such as ``ClassVar`` - or ``NoReturn``, import objects from ``typing`` instead of - ``typing_extensions`` wherever possible. + as possible (e.g. ``builtins.list`` over ``typing.List``, + ``collections.Counter`` over ``typing.Counter``, etc.). +* Y023: Where there is no detriment to backwards compatibility, import objects + such as ``ClassVar`` and ``NoReturn`` from ``typing`` rather than + ``typing_extensions``. +* Y024: Use ``typing.NamedTuple`` instead of ``collections.namedtuple``, as it allows + for more precise type inference. * Y023: Always alias ``collections.abc.Set`` when importing it, so as to avoid confusion with ``builtins.set``. diff --git a/pyi.py b/pyi.py index a04f439a..83f8a822 100644 --- a/pyi.py +++ b/pyi.py @@ -66,8 +66,6 @@ class TypeVarInfo(NamedTuple): for alias in ("Dict", "Frozenset", "List", "Set", "Tuple", "Type") } -# Just-for-typing_extensions blacklist -# # collections.abc aliases: none of these exist in typing or typing_extensions in Python 2, # so we disallow importing them from typing_extensions. # @@ -96,32 +94,6 @@ class TypeVarInfo(NamedTuple): } -# collections.abc.Set is dealt with separately as a special case -# -# AsyncContextManager doesn't exist at all in Python 2, -# so we can disallow importing it from typing & typing_extensions. -# -# ContextManager doesn't exist in contextlib in Python 2, but does exist in typing, -# so we have to allow importing it from typing. -# We *can* disallow importing it from typing_extensions, however -FORBIDDEN_Y022_IMPORTS = { - "typing": { - **_BAD_BUILTINS_ALIASES, - **_BAD_COLLECTIONS_ALIASES, - "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"', - }, - "typing_extensions": { - **_BAD_COLLECTIONS_ABC_ALIASES, - **_BAD_COLLECTIONS_ALIASES, - **_TYPING_NOT_TYPING_EXTENSIONS, - "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"', - "ContextManager": '"contextlib.AbstractContextManager" or "typing.ContextManager"', - } -} - -FORBIDDEN_ - - class PyiAwareFlakesChecker(FlakesChecker): def deferHandleNode(self, node, parent): self.deferFunction(lambda: self.handleNode(node, parent)) @@ -267,49 +239,75 @@ def in_class(self) -> bool: """Determine whether we are inside a `class` statement""" return bool(self._class_nesting) - def _Y022_check( - self, - node: ast.Attribute | ast.ImportFrom, - object_name: str, - module_name: str, - blacklist: dict[str, str], + def _check_typing_object( + self, node: ast.Attribute | ast.ImportFrom, module_name: str, object_name: str ) -> None: - if object_name in blacklist: - error_message = Y022.format( - good_cls_name=blacklist[object_name], - bad_cls_alias=object_name, - ) - self.error(node, error_message) + if object_name in _BAD_COLLECTIONS_ALIASES: + error_code, blacklist = Y022, _BAD_COLLECTIONS_ALIASES + elif object_name == "AsyncContextManager": + error_code = Y022 + blacklist = { + "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"' + } + elif module_name == "typing": + if object_name not in _BAD_BUILTINS_ALIASES: + return + error_code, blacklist = Y022, _BAD_BUILTINS_ALIASES + elif object_name in _BAD_COLLECTIONS_ABC_ALIASES: + error_code, blacklist = Y023, _BAD_COLLECTIONS_ABC_ALIASES + elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: + error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS + elif object_name == "ContextManager": + error_code = Y023 + blacklist = { + "ContextManager": '"contextlib.AbstractContextManager" or "typing.ContextManager"' + } + else: + return + + error_message = error_code.format( + good_cls_name=blacklist[object_name], + bad_cls_alias=f"{module_name}.{object_name}", + ) + self.error(node, error_message) def visit_Attribute(self, node: ast.Attribute) -> None: self.generic_visit(node) thing = node.value if not isinstance(thing, ast.Name): return - thingname = thing.id - if thingname in FORBIDDEN_IMPORTS_MAPPING: - self._Y022_check( - node=node, - object_name=node.attr, - module_name=thingname, - blacklist=FORBIDDEN_IMPORTS_MAPPING[thingname], - ) + thingname, attribute = thing.id, node.attr + + if thingname == "collections" and attribute == "namedtuple": + return self.error(node, Y024) + elif thingname not in {"typing", "typing_extensions"}: + return + + self._check_typing_object( + node=node, module_name=thingname, object_name=attribute + ) def visit_ImportFrom(self, node: ast.ImportFrom) -> None: module_name, imported_objects = node.module, node.names - if module_name in FORBIDDEN_IMPORTS_MAPPING: - forbidden_imports = FORBIDDEN_IMPORTS_MAPPING[module_name] - for obj in imported_objects: - self._Y022_check( - node=node, - object_name=obj.name, - module_name=module_name, - blacklist=forbidden_imports, - ) - elif module_name == "collections.abc": - for obj in imported_objects: - if obj.name == "Set" and obj.asname != "AbstractSet": - self.error(node, Y023) + + if module_name == "collections.abc" and any( + obj.name == "Set" and obj.asname != "AbstractSet" + for obj in imported_objects + ): + return self.error(node, Y025) + + elif module_name == "collections" and any( + obj.name == "namedtuple" for obj in imported_objects + ): + return self.error(node, Y024) + + elif module_name not in {"typing", "typing_extensions"}: + return + + for obj in imported_objects: + self._check_typing_object( + node=node, module_name=module_name, object_name=obj.name + ) def visit_Assign(self, node: ast.Assign) -> None: if self.in_function: @@ -386,6 +384,7 @@ def visit_Expr(self, node: ast.Expr) -> None: self.generic_visit(node) def visit_AnnAssign(self, node: ast.AnnAssign) -> None: + self.generic_visit(node) if isinstance(node.annotation, ast.Name) and node.annotation.id == "TypeAlias": return if node.value and not isinstance(node.value, ast.Ellipsis): @@ -877,15 +876,14 @@ def should_warn(self, code): Y019 = 'Y019 Use "_typeshed.Self" instead of "{typevar_name}"' Y020 = "Y020 Quoted annotations should never be used in stubs" Y021 = "Y021 Docstrings should not be included in stubs" -Y022 = 'Y022 Use {good_cls_name} instead of "typing.{bad_cls_alias}"' -Y023 = 'Y023 Use {good_cls_name} instead of "typing_extensions.{bad_cls_alias}"' +Y022 = 'Y022 Use {good_cls_name} instead of "{bad_cls_alias}"' +Y023 = 'Y023 Use {good_cls_name} instead of "{bad_cls_alias}"' Y024 = 'Y024 Use "typing.NamedTuple" instead of "collections.namedtuple"' Y025 = ( 'Y025 Use "from collections.abc import Set as AbstractSet" ' 'to avoid confusion with "builtins.set"' ) Y092 = "Y092 Top-level attribute must not have a default value" ->>>>>>> origin/master Y093 = "Y093 Use typing_extensions.TypeAlias for type aliases" DISABLED_BY_DEFAULT = [Y093] diff --git a/tests/imports.pyi b/tests/imports.pyi index 14decf0d..0639f483 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -1,6 +1,10 @@ # NOTE: F401 & F811 are ignored in this file in the .flake8 config file # GOOD IMPORTS +import typing +import typing_extensions +import collections +import collections.abc from collections import ChainMap, Counter, OrderedDict, UserDict, UserList, UserString, defaultdict, deque from collections.abc import ( Awaitable, @@ -97,16 +101,46 @@ from typing_extensions import ( ) -# BAD IMPORTS -from collections import namedtuple # Y022 Use "typing.NamedTuple" instead of "collections.namedtuple" +# BAD IMPORTS (Y022 code) from typing import Dict # Y022 Use "builtins.dict" instead of "typing.Dict" from typing import Counter # Y022 Use "collections.Counter" instead of "typing.Counter" from typing import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" from typing import ChainMap # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" -from typing_extensions import ClassVar # Y022 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" -from typing_extensions import Awaitable # Y022 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" -from typing_extensions import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" -from typing_extensions import ContextManager # Y022 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" from typing_extensions import ChainMap # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" -from collections.abc import Set # Y023 Use "from collections.abc import Set as AbstractSet" to avoid confusion with "builtins.set" +from typing_extensions import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" + +# BAD IMPORTS (Y023 code) +from typing_extensions import ClassVar # Y023 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" +from typing_extensions import Awaitable # Y023 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" +from typing_extensions import ContextManager # Y023 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" + +# BAD IMPORTS: OTHER +from collections import namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple" +from collections.abc import Set # Y025 Use "from collections.abc import Set as AbstractSet" to avoid confusion with "builtins.set" + +# GOOD ATTRIBUTE ACCESS +foo: typing.SupportsIndex +bar: typing_extensions.Final[int] +baz: collections.abc.Sized +blah: collections.deque[int] + +# BAD ATTRIBUTE ACCESS (Y022 code) +a: typing.Dict[str, int] # Y022 Use "builtins.dict" instead of "typing.Dict" +b: typing.Counter[float] # Y022 Use "collections.Counter" instead of "typing.Counter" +c: typing.AsyncContextManager[None] # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" +d: typing.ChainMap[int, str] # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" +e: typing_extensions.DefaultDict[bytes, bytes] # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" +f: typing_extensions.ChainMap[str, str] # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" +g: typing_extensions.AsyncContextManager[Any] # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" + +# BAD ATTRIBUTE ACCESS (Y023 code) +class Foo: + attribute: typing_extensions.ClassVar[int] # Y023 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" + + +h: typing_extensions.Awaitable[float] # Y023 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" +i: typing_extensions.ContextManager[None] # Y023 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" + +# BAD ATTRIBUTE ACCESS: OTHER +j: collections.namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple" From 14b507fab74fa1dd6bd5e0970774f9588d000af7 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 17 Jan 2022 23:58:58 +0000 Subject: [PATCH 11/18] Better ALLCAPS names --- pyi.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pyi.py b/pyi.py index 83f8a822..59dbfebe 100644 --- a/pyi.py +++ b/pyi.py @@ -42,26 +42,25 @@ class TypeVarInfo(NamedTuple): name: str -# The collections import blacklist is for typing & typing_extensions -# -# OrderedDict is omitted: +# OrderedDict is omitted from this blacklist: # -- In Python 3, we'd rather import it from collections, not typing or typing_extensions # -- But in Python 2, it cannot be imported from collections or typing, only from typing_extensions # # ChainMap does not exist in typing or typing_extensions in Python 2, # so we can disallow importing it from anywhere except collections -_BAD_COLLECTIONS_ALIASES = { +_COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS = { "Counter": "Counter", "Deque": "deque", "DefaultDict": "defaultdict", "ChainMap": "ChainMap", } -_BAD_COLLECTIONS_ALIASES = { - alias: f'"collections.{cls}"' for alias, cls in _BAD_COLLECTIONS_ALIASES.items() +_COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS = { + alias: f'"collections.{cls}"' + for alias, cls in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS.items() } # Just-for-typing blacklist -_BAD_BUILTINS_ALIASES = { +_BUILTINS_NOT_TYPING = { alias: f'"builtins.{alias.lower()}"' for alias in ("Dict", "Frozenset", "List", "Set", "Tuple", "Type") } @@ -70,7 +69,7 @@ class TypeVarInfo(NamedTuple): # so we disallow importing them from typing_extensions. # # We can't disallow importing collections.abc aliases from typing yet due to mypy/pytype errors. -_BAD_COLLECTIONS_ABC_ALIASES = { +_COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS = { alias: f'"collections.abc.{alias}" or "typing.{alias}"' for alias in ( "Awaitable", @@ -242,19 +241,20 @@ def in_class(self) -> bool: def _check_typing_object( self, node: ast.Attribute | ast.ImportFrom, module_name: str, object_name: str ) -> None: - if object_name in _BAD_COLLECTIONS_ALIASES: - error_code, blacklist = Y022, _BAD_COLLECTIONS_ALIASES + if object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: + error_code, blacklist = Y022, _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS elif object_name == "AsyncContextManager": error_code = Y022 blacklist = { "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"' } elif module_name == "typing": - if object_name not in _BAD_BUILTINS_ALIASES: + if object_name not in _BUILTINS_NOT_TYPING: return - error_code, blacklist = Y022, _BAD_BUILTINS_ALIASES - elif object_name in _BAD_COLLECTIONS_ABC_ALIASES: - error_code, blacklist = Y023, _BAD_COLLECTIONS_ABC_ALIASES + error_code, blacklist = Y022, _BUILTINS_NOT_TYPING + elif object_name in _COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS: + error_code = Y023 + blacklist = _COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS elif object_name == "ContextManager": From a23878d57d8c397f15a8445e4c5e415731f57f0f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 00:22:27 +0000 Subject: [PATCH 12/18] Better syntax suggestions for `contextlib`/`collections.abc` --- pyi.py | 23 +++++++++-------------- tests/imports.pyi | 8 ++++---- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/pyi.py b/pyi.py index 59dbfebe..0185d193 100644 --- a/pyi.py +++ b/pyi.py @@ -69,19 +69,16 @@ class TypeVarInfo(NamedTuple): # so we disallow importing them from typing_extensions. # # We can't disallow importing collections.abc aliases from typing yet due to mypy/pytype errors. -_COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS = { - alias: f'"collections.abc.{alias}" or "typing.{alias}"' +_TYPING_NOT_TYPING_EXTENSIONS = { + alias: f'"typing.{alias}"' for alias in ( + # collections.abc aliases "Awaitable", "Coroutine", "AsyncIterable", "AsyncIterator", "AsyncGenerator", - ) -} -_TYPING_NOT_TYPING_EXTENSIONS = { - alias: f'"typing.{alias}"' - for alias in ( + # typing aliases "Protocol", "runtime_checkable", "ClassVar", @@ -252,16 +249,14 @@ def _check_typing_object( if object_name not in _BUILTINS_NOT_TYPING: return error_code, blacklist = Y022, _BUILTINS_NOT_TYPING - elif object_name in _COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS: - error_code = Y023 - blacklist = _COLLECTIONSABC_OR_TYPING_NOT_TYPING_EXTENSIONS elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS elif object_name == "ContextManager": - error_code = Y023 - blacklist = { - "ContextManager": '"contextlib.AbstractContextManager" or "typing.ContextManager"' - } + suggested_syntax = ( + '"contextlib.AbstractContextManager" ' + '(or "typing.ContextManager" in Python 2-compatible code)' + ) + error_code, blacklist = Y023, {"ContextManager": suggested_syntax} else: return diff --git a/tests/imports.pyi b/tests/imports.pyi index 0639f483..2c7b07e0 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -112,8 +112,8 @@ from typing_extensions import AsyncContextManager # Y022 Use "contextlib.Abstra # BAD IMPORTS (Y023 code) from typing_extensions import ClassVar # Y023 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" -from typing_extensions import Awaitable # Y023 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" -from typing_extensions import ContextManager # Y023 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" +from typing_extensions import Awaitable # Y023 Use "typing.Awaitable" instead of "typing_extensions.Awaitable" +from typing_extensions import ContextManager # Y023 Use "contextlib.AbstractContextManager" (or "typing.ContextManager" in Python 2-compatible code) instead of "typing_extensions.ContextManager" # BAD IMPORTS: OTHER from collections import namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple" @@ -139,8 +139,8 @@ class Foo: attribute: typing_extensions.ClassVar[int] # Y023 Use "typing.ClassVar" instead of "typing_extensions.ClassVar" -h: typing_extensions.Awaitable[float] # Y023 Use "collections.abc.Awaitable" or "typing.Awaitable" instead of "typing_extensions.Awaitable" -i: typing_extensions.ContextManager[None] # Y023 Use "contextlib.AbstractContextManager" or "typing.ContextManager" instead of "typing_extensions.ContextManager" +h: typing_extensions.Awaitable[float] # Y023 Use "typing.Awaitable" instead of "typing_extensions.Awaitable" +i: typing_extensions.ContextManager[None] # Y023 Use "contextlib.AbstractContextManager" (or "typing.ContextManager" in Python 2-compatible code) instead of "typing_extensions.ContextManager" # BAD ATTRIBUTE ACCESS: OTHER j: collections.namedtuple # Y024 Use "typing.NamedTuple" instead of "collections.namedtuple" From 7d5468df068b1a037b231cbd8511e761a1b7f575 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 00:43:06 +0000 Subject: [PATCH 13/18] Code review: Improve control flow readability --- pyi.py | 80 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/pyi.py b/pyi.py index 0185d193..dee6db77 100644 --- a/pyi.py +++ b/pyi.py @@ -89,6 +89,10 @@ class TypeVarInfo(NamedTuple): ) } +_ASYNC_CONTEXTMANAGER_BLACKLIST = { + "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"' +} + class PyiAwareFlakesChecker(FlakesChecker): def deferHandleNode(self, node, parent): @@ -235,28 +239,45 @@ def in_class(self) -> bool: """Determine whether we are inside a `class` statement""" return bool(self._class_nesting) - def _check_typing_object( + def _check_import_or_attribute( self, node: ast.Attribute | ast.ImportFrom, module_name: str, object_name: str ) -> None: - if object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: - error_code, blacklist = Y022, _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS - elif object_name == "AsyncContextManager": + if module_name == "collections": + if object_name == "namedtuple": + self.error(node, Y024) + return + + if module_name == "typing": error_code = Y022 - blacklist = { - "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"' - } - elif module_name == "typing": - if object_name not in _BUILTINS_NOT_TYPING: + + if object_name in _BUILTINS_NOT_TYPING: + blacklist = _BUILTINS_NOT_TYPING + elif object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: + blacklist = _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS + elif object_name == "AsyncContextManager": + blacklist = _ASYNC_CONTEXTMANAGER_BLACKLIST + else: return - error_code, blacklist = Y022, _BUILTINS_NOT_TYPING - elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: - error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS - elif object_name == "ContextManager": - suggested_syntax = ( - '"contextlib.AbstractContextManager" ' - '(or "typing.ContextManager" in Python 2-compatible code)' - ) - error_code, blacklist = Y023, {"ContextManager": suggested_syntax} + + elif module_name == "typing_extensions": + if object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: + error_code, blacklist = ( + Y022, + _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS, + ) + elif object_name == "AsyncContextManager": + error_code, blacklist = Y022, _ASYNC_CONTEXTMANAGER_BLACKLIST + elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: + error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS + elif object_name == "ContextManager": + suggested_syntax = ( + '"contextlib.AbstractContextManager" ' + '(or "typing.ContextManager" in Python 2-compatible code)' + ) + error_code, blacklist = Y023, {"ContextManager": suggested_syntax} + else: + return + else: return @@ -271,36 +292,25 @@ def visit_Attribute(self, node: ast.Attribute) -> None: thing = node.value if not isinstance(thing, ast.Name): return - thingname, attribute = thing.id, node.attr - if thingname == "collections" and attribute == "namedtuple": - return self.error(node, Y024) - elif thingname not in {"typing", "typing_extensions"}: - return - - self._check_typing_object( - node=node, module_name=thingname, object_name=attribute + self._check_import_or_attribute( + node=node, module_name=thing.id, object_name=node.attr ) def visit_ImportFrom(self, node: ast.ImportFrom) -> None: module_name, imported_objects = node.module, node.names + if module_name is None: + return + if module_name == "collections.abc" and any( obj.name == "Set" and obj.asname != "AbstractSet" for obj in imported_objects ): return self.error(node, Y025) - elif module_name == "collections" and any( - obj.name == "namedtuple" for obj in imported_objects - ): - return self.error(node, Y024) - - elif module_name not in {"typing", "typing_extensions"}: - return - for obj in imported_objects: - self._check_typing_object( + self._check_import_or_attribute( node=node, module_name=module_name, object_name=obj.name ) From c221fa595e11977541c1502296ad9ea15d7d9df6 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 00:56:57 +0000 Subject: [PATCH 14/18] Update README.rst Co-authored-by: Jelle Zijlstra --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 2f28d05a..f9f9ab05 100644 --- a/README.rst +++ b/README.rst @@ -89,7 +89,7 @@ currently emitted: ``typing_extensions``. * Y024: Use ``typing.NamedTuple`` instead of ``collections.namedtuple``, as it allows for more precise type inference. -* Y023: Always alias ``collections.abc.Set`` when importing it, so as to avoid +* Y025: Always alias ``collections.abc.Set`` when importing it, so as to avoid confusion with ``builtins.set``. The following warnings are disabled by default: From e3d3eb391132d487013876b0c452be671c077833 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 17 Jan 2022 16:58:20 -0800 Subject: [PATCH 15/18] Update pyi.py --- pyi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyi.py b/pyi.py index dee6db77..8c24d900 100644 --- a/pyi.py +++ b/pyi.py @@ -888,7 +888,6 @@ def should_warn(self, code): 'Y025 Use "from collections.abc import Set as AbstractSet" ' 'to avoid confusion with "builtins.set"' ) -Y092 = "Y092 Top-level attribute must not have a default value" Y093 = "Y093 Use typing_extensions.TypeAlias for type aliases" DISABLED_BY_DEFAULT = [Y093] From 2d20a055dd7489a8b74fe93db95f4265f5838927 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 11:29:47 +0000 Subject: [PATCH 16/18] Improve tests to cover attribute access inside functions and methods --- tests/imports.pyi | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/imports.pyi b/tests/imports.pyi index 2c7b07e0..f80fa687 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -127,12 +127,29 @@ blah: collections.deque[int] # BAD ATTRIBUTE ACCESS (Y022 code) a: typing.Dict[str, int] # Y022 Use "builtins.dict" instead of "typing.Dict" -b: typing.Counter[float] # Y022 Use "collections.Counter" instead of "typing.Counter" -c: typing.AsyncContextManager[None] # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" -d: typing.ChainMap[int, str] # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" -e: typing_extensions.DefaultDict[bytes, bytes] # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" -f: typing_extensions.ChainMap[str, str] # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" -g: typing_extensions.AsyncContextManager[Any] # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" + +def func1() -> typing.Counter[float]: # Y022 Use "collections.Counter" instead of "typing.Counter" + ... + + +def func2(c: typing.AsyncContextManager[None]) -> None: # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" + ... + + +def func3(d: typing.ChainMap[int, str] = ...) -> None: # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" + ... + + +class Spam: + def meth1() -> typing_extensions.DefaultDict[bytes, bytes]: # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" + ... + + def meth2(f: typing_extensions.ChainMap[str, str]) -> None: # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" + ... + + def meth3(g: typing_extensions.AsyncContextManager[Any] = ...) -> None: # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" + ... + # BAD ATTRIBUTE ACCESS (Y023 code) class Foo: From 7644212f20242a86c9a46ce4cf8d8611b1b9c11c Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 18:12:41 +0000 Subject: [PATCH 17/18] Disallow importing `Type` from `typing_extensions`, improve tests more --- pyi.py | 10 +++++----- tests/imports.pyi | 20 +++++++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pyi.py b/pyi.py index 8c24d900..c74124f5 100644 --- a/pyi.py +++ b/pyi.py @@ -260,11 +260,11 @@ def _check_import_or_attribute( return elif module_name == "typing_extensions": - if object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: - error_code, blacklist = ( - Y022, - _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS, - ) + if object_name == "Type": + error_code, blacklist = Y022, {"Type": '"builtins.type"'} + elif object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: + error_code = Y022 + blacklist = _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS elif object_name == "AsyncContextManager": error_code, blacklist = Y022, _ASYNC_CONTEXTMANAGER_BLACKLIST elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: diff --git a/tests/imports.pyi b/tests/imports.pyi index f80fa687..ecb80b6f 100644 --- a/tests/imports.pyi +++ b/tests/imports.pyi @@ -106,6 +106,7 @@ from typing import Dict # Y022 Use "builtins.dict" instead of "typing.Dict" from typing import Counter # Y022 Use "collections.Counter" instead of "typing.Counter" from typing import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing.AsyncContextManager" from typing import ChainMap # Y022 Use "collections.ChainMap" instead of "typing.ChainMap" +from typing_extensions import Type # Y022 Use "builtins.type" instead of "typing_extensions.Type" from typing_extensions import DefaultDict # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" from typing_extensions import ChainMap # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" from typing_extensions import AsyncContextManager # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" @@ -121,9 +122,18 @@ from collections.abc import Set # Y025 Use "from collections.abc import Set as # GOOD ATTRIBUTE ACCESS foo: typing.SupportsIndex -bar: typing_extensions.Final[int] -baz: collections.abc.Sized -blah: collections.deque[int] + +@typing_extensions.final +def bar(arg: collections.abc.Sized) -> typing_extensions.Literal[True]: + ... + + +class Fish: + blah: collections.deque[int] + + def method(self, arg: typing.SupportsInt = ...) -> None: + ... + # BAD ATTRIBUTE ACCESS (Y022 code) a: typing.Dict[str, int] # Y022 Use "builtins.dict" instead of "typing.Dict" @@ -144,10 +154,10 @@ class Spam: def meth1() -> typing_extensions.DefaultDict[bytes, bytes]: # Y022 Use "collections.defaultdict" instead of "typing_extensions.DefaultDict" ... - def meth2(f: typing_extensions.ChainMap[str, str]) -> None: # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" + def meth2(self, f: typing_extensions.ChainMap[str, str]) -> None: # Y022 Use "collections.ChainMap" instead of "typing_extensions.ChainMap" ... - def meth3(g: typing_extensions.AsyncContextManager[Any] = ...) -> None: # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" + def meth3(self, g: typing_extensions.AsyncContextManager[Any] = ...) -> None: # Y022 Use "contextlib.AbstractAsyncContextManager" instead of "typing_extensions.AsyncContextManager" ... From c644a6c9060f32f6f42c2b71e6612a834a342481 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 18 Jan 2022 21:38:53 +0000 Subject: [PATCH 18/18] Finish addressing review --- pyi.py | 107 +++++++++++++++++++++++++++------------------------------ 1 file changed, 50 insertions(+), 57 deletions(-) diff --git a/pyi.py b/pyi.py index c74124f5..bf74a4ca 100644 --- a/pyi.py +++ b/pyi.py @@ -48,30 +48,35 @@ class TypeVarInfo(NamedTuple): # # ChainMap does not exist in typing or typing_extensions in Python 2, # so we can disallow importing it from anywhere except collections -_COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS = { - "Counter": "Counter", - "Deque": "deque", - "DefaultDict": "defaultdict", - "ChainMap": "ChainMap", -} -_COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS = { - alias: f'"collections.{cls}"' - for alias, cls in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS.items() -} - -# Just-for-typing blacklist -_BUILTINS_NOT_TYPING = { - alias: f'"builtins.{alias.lower()}"' - for alias in ("Dict", "Frozenset", "List", "Set", "Tuple", "Type") +_BAD_Y022_IMPORTS = { + # typing aliases for collections + "typing.Counter": "collections.Counter", + "typing.Deque": "collections.deque", + "typing.DefaultDict": "collections.defaultdict", + "typing.ChainMap": "collections.ChainMap", + # typing aliases for builtins + "typing.Dict": "builtins.dict", + "typing.FrozenSet": "builtins.frozenset", + "typing.List": "builtins.list", + "typing.Set": "builtins.set", + "typing.Tuple": "builtins.tuple", + "typing.Type": "builtins.type", + # One typing alias for contextlib + "typing.AsyncContextManager": "contextlib.AbstractAsyncContextManager", + # typing_extensions aliases for collections + "typing_extensions.Counter": "collections.Counter", + "typing_extensions.Deque": "collections.deque", + "typing_extensions.DefaultDict": "collections.defaultdict", + "typing_extensions.ChainMap": "collections.ChainMap", + # One typing_extensions alias for a builtin + "typing_extensions.Type": "builtins.type", + # one typing_extensions alias for contextlib + "typing_extensions.AsyncContextManager": "contextlib.AbstractAsyncContextManager", } -# collections.abc aliases: none of these exist in typing or typing_extensions in Python 2, -# so we disallow importing them from typing_extensions. -# -# We can't disallow importing collections.abc aliases from typing yet due to mypy/pytype errors. -_TYPING_NOT_TYPING_EXTENSIONS = { - alias: f'"typing.{alias}"' - for alias in ( +# typing_extensions.ContextManager is omitted from this collection - special-cased +_BAD_Y023_IMPORTS = frozenset( + { # collections.abc aliases "Awaitable", "Coroutine", @@ -86,12 +91,8 @@ class TypeVarInfo(NamedTuple): "overload", "Text", "NoReturn", - ) -} - -_ASYNC_CONTEXTMANAGER_BLACKLIST = { - "AsyncContextManager": '"contextlib.AbstractAsyncContextManager"' -} + } +) class PyiAwareFlakesChecker(FlakesChecker): @@ -242,49 +243,41 @@ def in_class(self) -> bool: def _check_import_or_attribute( self, node: ast.Attribute | ast.ImportFrom, module_name: str, object_name: str ) -> None: - if module_name == "collections": - if object_name == "namedtuple": - self.error(node, Y024) - return + fullname = f"{module_name}.{object_name}" - if module_name == "typing": - error_code = Y022 - - if object_name in _BUILTINS_NOT_TYPING: - blacklist = _BUILTINS_NOT_TYPING - elif object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: - blacklist = _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS - elif object_name == "AsyncContextManager": - blacklist = _ASYNC_CONTEXTMANAGER_BLACKLIST - else: - return + # Y022 errors + if fullname in _BAD_Y022_IMPORTS: + error_message = Y022.format( + good_cls_name=f'"{_BAD_Y022_IMPORTS[fullname]}"', + bad_cls_alias=fullname, + ) + # Y023 errors elif module_name == "typing_extensions": - if object_name == "Type": - error_code, blacklist = Y022, {"Type": '"builtins.type"'} - elif object_name in _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS: - error_code = Y022 - blacklist = _COLLECTIONS_NOT_TYPING_OR_TYPING_EXTENSIONS - elif object_name == "AsyncContextManager": - error_code, blacklist = Y022, _ASYNC_CONTEXTMANAGER_BLACKLIST - elif object_name in _TYPING_NOT_TYPING_EXTENSIONS: - error_code, blacklist = Y023, _TYPING_NOT_TYPING_EXTENSIONS + if object_name in _BAD_Y023_IMPORTS: + error_message = Y023.format( + good_cls_name=f'"typing.{object_name}"', + bad_cls_alias=f"typing_extensions.{object_name}", + ) elif object_name == "ContextManager": suggested_syntax = ( '"contextlib.AbstractContextManager" ' '(or "typing.ContextManager" in Python 2-compatible code)' ) - error_code, blacklist = Y023, {"ContextManager": suggested_syntax} + error_message = Y023.format( + good_cls_name=suggested_syntax, + bad_cls_alias="typing_extensions.ContextManager", + ) else: return + # Y024 errors + elif fullname == "collections.namedtuple": + error_message = Y024 + else: return - error_message = error_code.format( - good_cls_name=blacklist[object_name], - bad_cls_alias=f"{module_name}.{object_name}", - ) self.error(node, error_message) def visit_Attribute(self, node: ast.Attribute) -> None: