From 7ca91c57126a8bf22c80425c63f1361e3bc03fc5 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 18 Jan 2022 20:00:59 -0800 Subject: [PATCH 1/5] Enable TypeAlias check by default And delete the machinery for disabling errors. This made this check run on all tests, which helped me find a bug (it triggered on __all__). I can split the test changes into a separate PR if preferred. I'll submit a separate PR to typeshed to disable this check for now. Fixes #75. Fixes #86. --- CHANGELOG.rst | 3 +- README.rst | 5 +-- pyi.py | 36 +++---------------- tests/aliases.pyi | 3 +- tests/attribute_annotations.pyi | 2 -- tests/del.pyi | 6 ++-- tests/forward_refs.pyi | 14 ++++---- tests/forward_refs_annassign.pyi | 6 ++-- tests/quotes.pyi | 4 +-- .../vanilla_flake8_not_clean_forward_refs.pyi | 6 ++-- 10 files changed, 27 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a4b2fd50..1a709d54 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,7 +15,7 @@ unreleased statements). The previous checks were disabled by default. * detect usage of non-integer indices in sys.version_info checks * extend Y010 to check async functions in addition to normal functions -* introduce Y093 (require using TypeAlias for type aliases) +* introduce Y026 (require using TypeAlias for type aliases) * introduce Y017 (disallows assignments with multiple targets or non-name targets) * extend Y001 to cover ParamSpec and TypeVarTuple in addition to TypeVar * introduce Y018 (detect unused TypeVars) @@ -27,6 +27,7 @@ unreleased * introduce Y023 (prefer ``typing`` over ``typing_extensions``) * introduce Y024 (prefer ``typing.NamedTuple`` to ``collections.namedtuple``) * introduce Y025 (always alias ``collections.abc.Set``) +* all errors are now enabled by default 20.10.0 ~~~~~~~ diff --git a/README.rst b/README.rst index f9f9ab05..9584d619 100644 --- a/README.rst +++ b/README.rst @@ -91,10 +91,7 @@ currently emitted: for more precise type inference. * Y025: Always alias ``collections.abc.Set`` when importing it, so as to avoid confusion with ``builtins.set``. - -The following warnings are disabled by default: - -* Y093: Type aliases should be explicitly demarcated with ``typing.TypeAlias``. +* Y026: Type aliases should be explicitly demarcated with ``typing.TypeAlias``. License ------- diff --git a/pyi.py b/pyi.py index e6cbf777..dd69db02 100644 --- a/pyi.py +++ b/pyi.py @@ -346,14 +346,14 @@ def visit_Assign(self, node: ast.Assign) -> None: self.typevarlike_defs[target_info] = node else: self.error(target, Y001.format(cls_name)) - # We avoid triggering Y093 in this case because there are various + # We avoid triggering Y026 in this case because there are various # unusual cases where assignment to the result of a call is legitimate # in stubs. return if isinstance(node.value, (ast.Num, ast.Str, ast.Bytes)): self.error(node.value, Y015) - else: - self.error(node, Y093) + elif target_name != "__all__": + self.error(node, Y026) def visit_Name(self, node: ast.Name) -> None: self.all_name_occurrences[node.id] += 1 @@ -791,8 +791,7 @@ def run(self): if path.suffix == ".pyi": visitor = PyiVisitor(filename=path) for error in visitor.run(self.tree): - if self.should_warn(error.message[:4]): - yield error + yield error @classmethod def add_options(cls, parser): @@ -818,7 +817,6 @@ def add_options(cls, parser): except optparse.OptionConflictError: # In tests, sometimes this option gets called twice for some reason. pass - parser.extend_default_ignore(DISABLED_BY_DEFAULT) @classmethod def parse_options(cls, optmanager, options, extra_args): @@ -826,28 +824,6 @@ def parse_options(cls, optmanager, options, extra_args): if not options.no_pyi_aware_file_checker: checker.FileChecker = PyiAwareFileChecker - # Functionality to ignore some warnings. Adapted from flake8-bugbear. - def should_warn(self, code): - """Returns `True` if flake8-pyi should emit a particular warning. - flake8 overrides default ignores when the user specifies - `ignore = ` in configuration. This is problematic because it means - specifying anything in `ignore = ` implicitly enables all optional - warnings. This function is a workaround for this behavior. - Users should explicitly enable these warnings. - """ - if code[:3] != "Y09": - # Normal warnings are safe for emission. - return True - - if self.options is None: - return True - - for i in range(2, len(code) + 1): - if code[:i] in self.options.select: - return True - - return False - # Please keep error code lists in README and CHANGELOG up to date Y001 = "Y001 Name of private {} must start with _" @@ -880,6 +856,4 @@ def should_warn(self, code): 'Y025 Use "from collections.abc import Set as AbstractSet" ' 'to avoid confusion with "builtins.set"' ) -Y093 = "Y093 Use typing_extensions.TypeAlias for type aliases" - -DISABLED_BY_DEFAULT = [Y093] +Y026 = "Y026 Use typing_extensions.TypeAlias for type aliases" diff --git a/tests/aliases.pyi b/tests/aliases.pyi index 6e4f93fc..193addef 100644 --- a/tests/aliases.pyi +++ b/tests/aliases.pyi @@ -1,7 +1,6 @@ -# flags: --select=Y017,Y093 from typing import TypeAlias, ParamSpec as _ParamSpec, _Alias, TypedDict -X = int # Y093 Use typing_extensions.TypeAlias for type aliases +X = int # Y026 Use typing_extensions.TypeAlias for type aliases X: TypeAlias = int a = b = int # Y017 Only simple assignments allowed diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 5662faa9..49ebecb0 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,6 +1,5 @@ field1: int field2: int = ... -field3 = ... # type: int field4: int = 0 # Y015 Attribute must not have a default value other than "..." field5 = 0 # type: int # Y015 Attribute must not have a default value other than "..." field6 = 0 # Y015 Attribute must not have a default value other than "..." @@ -9,7 +8,6 @@ field7 = b"" # Y015 Attribute must not have a default value other than "..." class Foo: field1: int field2: int = ... - field3 = ... # type: int field4: int = 0 # Y015 Attribute must not have a default value other than "..." field5 = 0 # type: int # Y015 Attribute must not have a default value other than "..." field6 = 0 # Y015 Attribute must not have a default value other than "..." diff --git a/tests/del.pyi b/tests/del.pyi index d92a95a0..f2f11d0b 100644 --- a/tests/del.pyi +++ b/tests/del.pyi @@ -1,8 +1,8 @@ -from typing import Union +from typing import Union, TypeAlias -ManyStr = list[EitherStr] -EitherStr = Union[str, bytes] +ManyStr: TypeAlias = list[EitherStr] +EitherStr: TypeAlias = Union[str, bytes] def function(accepts: EitherStr) -> None: diff --git a/tests/forward_refs.pyi b/tests/forward_refs.pyi index e3497fd8..a90ad1b5 100644 --- a/tests/forward_refs.pyi +++ b/tests/forward_refs.pyi @@ -1,10 +1,10 @@ -from typing import Optional, Union +from typing import Optional, Union, TypeAlias -MaybeCStr = Optional[CStr] -CStr = Union[C, str] -__version__ = ... # type: str -__author__ = ... # type: str +MaybeCStr: TypeAlias = Optional[CStr] +CStr: TypeAlias = Union[C, str] +__version__: str +__author__: str def make_default_c() -> C: @@ -12,14 +12,14 @@ def make_default_c() -> C: class D(C): - parent = None # type: C + parent: C def __init__(self) -> None: ... class C: - other = None # type: C + other: C def __init__(self) -> None: ... diff --git a/tests/forward_refs_annassign.pyi b/tests/forward_refs_annassign.pyi index 8323693f..70573c87 100644 --- a/tests/forward_refs_annassign.pyi +++ b/tests/forward_refs_annassign.pyi @@ -1,8 +1,8 @@ -from typing import Optional, Union +from typing import Optional, Union, TypeAlias -MaybeCStr = Optional[CStr] -CStr = Union[C, str] +MaybeCStr: TypeAlias = Optional[CStr] +CStr: TypeAlias = Union[C, str] __version__: str __author__: str diff --git a/tests/quotes.pyi b/tests/quotes.pyi index e7303cc4..38f53a1f 100644 --- a/tests/quotes.pyi +++ b/tests/quotes.pyi @@ -1,4 +1,4 @@ -from typing import TypeVar, Literal, Annotated +from typing import TypeVar, Literal, Annotated, TypeAlias __all__ = ["f", "g"] @@ -22,7 +22,7 @@ def j() -> "int": # Y020 Quoted annotations should never be used in stubs ... -Alias = list["int"] # Y020 Quoted annotations should never be used in stubs +Alias: TypeAlias = list["int"] # Y020 Quoted annotations should never be used in stubs class Child(list["int"]): # Y020 Quoted annotations should never be used in stubs """Documented and guaranteed useful.""" # Y021 Docstrings should not be included in stubs diff --git a/tests/vanilla_flake8_not_clean_forward_refs.pyi b/tests/vanilla_flake8_not_clean_forward_refs.pyi index 379defb4..67d65b3c 100644 --- a/tests/vanilla_flake8_not_clean_forward_refs.pyi +++ b/tests/vanilla_flake8_not_clean_forward_refs.pyi @@ -1,9 +1,9 @@ # flags: --no-pyi-aware-file-checker -from typing import Union +from typing import Union, TypeAlias -ManyStr = list[EitherStr] # F821 undefined name 'EitherStr' -EitherStr = Union[str, bytes] +ManyStr: TypeAlias = list[EitherStr] # F821 undefined name 'EitherStr' +EitherStr: TypeAlias = Union[str, bytes] def function(accepts: EitherStr) -> None: From 98c5ebec2c43e697d73d8171ae407e57c7226f6d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 18 Jan 2022 20:27:14 -0800 Subject: [PATCH 2/5] Ellipsis is not a type alias --- pyi.py | 2 +- tests/attribute_annotations.pyi | 2 ++ tests/forward_refs.pyi | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pyi.py b/pyi.py index dd69db02..af215778 100644 --- a/pyi.py +++ b/pyi.py @@ -352,7 +352,7 @@ def visit_Assign(self, node: ast.Assign) -> None: return if isinstance(node.value, (ast.Num, ast.Str, ast.Bytes)): self.error(node.value, Y015) - elif target_name != "__all__": + elif target_name != "__all__" and not isinstance(node.value, ast.Ellipsis): self.error(node, Y026) def visit_Name(self, node: ast.Name) -> None: diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 49ebecb0..5662faa9 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,5 +1,6 @@ field1: int field2: int = ... +field3 = ... # type: int field4: int = 0 # Y015 Attribute must not have a default value other than "..." field5 = 0 # type: int # Y015 Attribute must not have a default value other than "..." field6 = 0 # Y015 Attribute must not have a default value other than "..." @@ -8,6 +9,7 @@ field7 = b"" # Y015 Attribute must not have a default value other than "..." class Foo: field1: int field2: int = ... + field3 = ... # type: int field4: int = 0 # Y015 Attribute must not have a default value other than "..." field5 = 0 # type: int # Y015 Attribute must not have a default value other than "..." field6 = 0 # Y015 Attribute must not have a default value other than "..." diff --git a/tests/forward_refs.pyi b/tests/forward_refs.pyi index a90ad1b5..e40d189d 100644 --- a/tests/forward_refs.pyi +++ b/tests/forward_refs.pyi @@ -3,8 +3,8 @@ from typing import Optional, Union, TypeAlias MaybeCStr: TypeAlias = Optional[CStr] CStr: TypeAlias = Union[C, str] -__version__: str -__author__: str +__version__ = ... # type: str +__author__ = ... # type: str def make_default_c() -> C: From 292712780c037a1c0148b9db0d452c1ede97678a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 19 Jan 2022 06:57:13 -0800 Subject: [PATCH 3/5] add note about compat --- README.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.rst b/README.rst index c611622c..fbeeda2d 100644 --- a/README.rst +++ b/README.rst @@ -95,6 +95,13 @@ currently emitted: * Y028: Always use class-based syntax for ``typing.NamedTuple``, instead of assignment-based syntax. +Many error codes enforce modern conventions, and some cannot yet be used in +all cases: + +* Y026 is incompatible with the pytype type checker and should be turned + off for stubs that need to be compatible with pytype. A fix is tracked + `here `_. + License ------- From 16bce26bd2c116b468d15d1a10af80e855e31959 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 19 Jan 2022 06:59:19 -0800 Subject: [PATCH 4/5] calls to typing.NamedTuple() also are not type aliases --- pyi.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pyi.py b/pyi.py index b23839e6..a07c5139 100644 --- a/pyi.py +++ b/pyi.py @@ -346,13 +346,12 @@ def visit_Assign(self, node: ast.Assign) -> None: self.typevarlike_defs[target_info] = node else: self.error(target, Y001.format(cls_name)) - # We avoid triggering Y026 in this case because there are various - # unusual cases where assignment to the result of a call is legitimate - # in stubs. - return if isinstance(node.value, (ast.Num, ast.Str, ast.Bytes)): self.error(node.value, Y015) - elif target_name != "__all__" and not isinstance(node.value, ast.Ellipsis): + # We avoid triggering Y026 for calls and = ... because there are various + # unusual cases where assignment to the result of a call is legitimate + # in stubs. + elif target_name != "__all__" and not isinstance(node.value, (ast.Ellipsis, ast.Call)): self.error(node, Y026) def visit_Name(self, node: ast.Name) -> None: From 7f021d8a2506d7ce8bded1966f799c44d0f37e05 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 19 Jan 2022 07:01:56 -0800 Subject: [PATCH 5/5] black --- pyi.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pyi.py b/pyi.py index a07c5139..b1cebd5a 100644 --- a/pyi.py +++ b/pyi.py @@ -351,7 +351,9 @@ def visit_Assign(self, node: ast.Assign) -> None: # We avoid triggering Y026 for calls and = ... because there are various # unusual cases where assignment to the result of a call is legitimate # in stubs. - elif target_name != "__all__" and not isinstance(node.value, (ast.Ellipsis, ast.Call)): + elif target_name != "__all__" and not isinstance( + node.value, (ast.Ellipsis, ast.Call) + ): self.error(node, Y026) def visit_Name(self, node: ast.Name) -> None: