From 1692337cb919eada8596160488d2bd12cb2f2f03 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 12:56:58 +0000 Subject: [PATCH 1/8] Introduce a check disallowing module-level defaults, and improve detection of unused TypeVars --- CHANGELOG.rst | 7 ++++++ README.rst | 2 ++ pyi.py | 43 +++++++++++++++++++++++++-------- tests/attribute_annotations.pyi | 13 ++++++++-- tests/typevar.pyi | 3 +++ 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fd7eb938..b90c41d0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,13 @@ Change Log ---------- +Unreleased +~~~~~~~~~~ + +* introduce Y032 (disallow default values for most module-level assignments). + Similar to the Y092 check that was removed in 22.1.0. However, this check is + enabled by default, whereas Y092 was disabled by default. + 22.1.0 ~~~~~~ diff --git a/README.rst b/README.rst index 430742bf..6212fa1b 100644 --- a/README.rst +++ b/README.rst @@ -107,6 +107,8 @@ currently emitted: syntax wherever possible. (In situations where this is not possible, such as if a field is a Python keyword or an invalid identifier, this error will not be raised.) +* Y032: Unless the object is used elswhere in the same file as an annotation, + a module-level attribute should not have a default value. Many error codes enforce modern conventions, and some cannot yet be used in all cases: diff --git a/pyi.py b/pyi.py index a6b51fc2..0d6a0171 100644 --- a/pyi.py +++ b/pyi.py @@ -7,7 +7,6 @@ import optparse import re import sys -from collections import Counter from collections.abc import Iterable, Iterator, Sequence from contextlib import contextmanager from copy import deepcopy @@ -296,11 +295,12 @@ def __init__(self, filename: Path = Path("none")) -> None: self.errors: list[Error] = [] # Mapping of all private TypeVars/ParamSpecs/TypeVarTuples to the nodes where they're defined self.typevarlike_defs: dict[TypeVarInfo, ast.Assign] = {} - # Mapping of each name in the file to the no. of occurrences - self.all_name_occurrences: Counter[str] = Counter() + self.suspicious_global_assignments: dict[str, ast.AnnAssign] = {} + self.all_names_in_annotations: set[str] = set() self.string_literals_allowed = NestingCounter() self.in_function = NestingCounter() self.in_class = NestingCounter() + self.visiting_function_arguments = NestingCounter() def __repr__(self) -> str: return f"{self.__class__.__name__}(filename={self.filename!r})" @@ -423,8 +423,12 @@ def visit_Assign(self, node: ast.Assign) -> None: ): self.error(node, Y026) + def _visit_annotation(self, annotation: ast.Name) -> None: + self.all_names_in_annotations.add(annotation.id) + def visit_Name(self, node: ast.Name) -> None: - self.all_name_occurrences[node.id] += 1 + if self.visiting_function_arguments.active: + self._visit_annotation(node) def visit_Call(self, node: ast.Call) -> None: function = node.func @@ -472,10 +476,17 @@ def visit_Expr(self, node: ast.Expr) -> None: 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): - self.error(node.value, Y015) + if isinstance(node.annotation, ast.Name): + self._visit_annotation(node.annotation) + if node.annotation.id == "TypeAlias": + return + if isinstance(node.target, ast.Name): + if self.in_class.active: + if node.value and not isinstance(node.value, ast.Ellipsis): + self.error(node.value, Y015) + else: + if node.value: + self.suspicious_global_assignments[node.target.id] = node def _check_union_members(self, members: Sequence[ast.expr]) -> None: members_by_dump: dict[str, list[ast.expr]] = {} @@ -850,6 +861,8 @@ def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> N def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: with self.in_function.enabled(): self.generic_visit(node) + if isinstance(node.returns, ast.Name): + self._visit_annotation(node.returns) for i, statement in enumerate(node.body): if i == 0: @@ -869,7 +882,8 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: self.check_self_typevars(node) def visit_arguments(self, node: ast.arguments) -> None: - self.generic_visit(node) + with self.visiting_function_arguments.enabled(): + self.generic_visit(node) args = node.args[-len(node.defaults) :] for arg, default in chain( zip(args, node.defaults), zip(node.kwonlyargs, node.kw_defaults) @@ -886,11 +900,19 @@ def run(self, tree: ast.AST) -> Iterable[Error]: self.errors.clear() self.visit(tree) for (cls_name, typevar_name), def_node in self.typevarlike_defs.items(): - if self.all_name_occurrences[typevar_name] == 1: + if typevar_name not in self.all_names_in_annotations: self.error( def_node, Y018.format(typevarlike_cls=cls_name, typevar_name=typevar_name), ) + # Only raise Y032 if the name is not used anywhere within the same file, + # as otherwise stock flake8 raises spurious errors about the name being undefined. + # See https://github.com/python/typeshed/pull/6930 + for thing_name, assign_node in self.suspicious_global_assignments.items(): + if thing_name not in self.all_names_in_annotations: + self.error(assign_node, Y032) + elif not isinstance(assign_node.value, ast.Ellipsis): + self.error(assign_node, Y015) yield from self.errors @@ -980,3 +1002,4 @@ def parse_options(cls, optmanager, options, extra_args) -> None: Y029 = "Y029 Defining __repr__ or __str__ in a stub is almost always redundant" Y030 = "Y030 Multiple Literal members in a union. {suggestion}" Y031 = "Y031 Use class-based syntax for TypedDicts where possible" +Y032 = "Y032 Default value unnecessary for module-level attribute" diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 5662faa9..69efa35b 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,10 +1,18 @@ -field1: int -field2: int = ... +field0: int +field1: int = ... # Y032 Default value unnecessary for module-level attribute +field2: int = 0 # Y032 Default value unnecessary for module-level attribute 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 "..." field7 = b"" # Y015 Attribute must not have a default value other than "..." +field8: str = ... + +thing_using_field3: field3 +thing_using_field4: field4 +thing_using_field5: field5 +thing_using_field6: field6 +thing_using_field7: field7 class Foo: field1: int @@ -14,3 +22,4 @@ class Foo: 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 "..." field7 = b"" # Y015 Attribute must not have a default value other than "..." + thing_using_field8_in_class: field8 diff --git a/tests/typevar.pyi b/tests/typevar.pyi index 35c7955b..f283588a 100644 --- a/tests/typevar.pyi +++ b/tests/typevar.pyi @@ -15,6 +15,8 @@ def func(arg: _UsedTypeVar) -> _UsedTypeVar: ... _TypeVarUsedInBinOp = TypeVar("_TypeVarUsedInBinOp", bound=str) def func2(arg: _TypeVarUsedInBinOp | int) -> _TypeVarUsedInBinOp | int: ... +_UnusedTypeVarWithSameNameAsClassAttributeElswhere = TypeVar("_UnusedTypeVarWithSameNameAsClassAttributeElswhere") # Y018 TypeVar "_UnusedTypeVarWithSameNameAsClassAttributeElswhere" is not used + _S = TypeVar("_S") class BadClass: @@ -33,3 +35,4 @@ class GoodClass: def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... + _UnusedTypeVarWithSameNameAsClassAttributeElswhere: str From 53f237184090980eb45381fd68fce26b040410d0 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 14:14:16 +0000 Subject: [PATCH 2/8] Simplify and fix regressions --- pyi.py | 21 +++++++-------------- tests/attribute_annotations.pyi | 2 -- tests/typevar.pyi | 6 +++--- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/pyi.py b/pyi.py index 0d6a0171..eb17953f 100644 --- a/pyi.py +++ b/pyi.py @@ -7,6 +7,7 @@ import optparse import re import sys +from collections import Counter from collections.abc import Iterable, Iterator, Sequence from contextlib import contextmanager from copy import deepcopy @@ -295,12 +296,12 @@ def __init__(self, filename: Path = Path("none")) -> None: self.errors: list[Error] = [] # Mapping of all private TypeVars/ParamSpecs/TypeVarTuples to the nodes where they're defined self.typevarlike_defs: dict[TypeVarInfo, ast.Assign] = {} + # Mapping of each name in the file to the no. of occurrences + self.all_name_occurrences: Counter[str] = Counter() self.suspicious_global_assignments: dict[str, ast.AnnAssign] = {} - self.all_names_in_annotations: set[str] = set() self.string_literals_allowed = NestingCounter() self.in_function = NestingCounter() self.in_class = NestingCounter() - self.visiting_function_arguments = NestingCounter() def __repr__(self) -> str: return f"{self.__class__.__name__}(filename={self.filename!r})" @@ -423,12 +424,8 @@ def visit_Assign(self, node: ast.Assign) -> None: ): self.error(node, Y026) - def _visit_annotation(self, annotation: ast.Name) -> None: - self.all_names_in_annotations.add(annotation.id) - def visit_Name(self, node: ast.Name) -> None: - if self.visiting_function_arguments.active: - self._visit_annotation(node) + self.all_name_occurrences[node.id] += 1 def visit_Call(self, node: ast.Call) -> None: function = node.func @@ -477,7 +474,6 @@ def visit_Expr(self, node: ast.Expr) -> None: def visit_AnnAssign(self, node: ast.AnnAssign) -> None: self.generic_visit(node) if isinstance(node.annotation, ast.Name): - self._visit_annotation(node.annotation) if node.annotation.id == "TypeAlias": return if isinstance(node.target, ast.Name): @@ -861,8 +857,6 @@ def check_self_typevars(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> N def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: with self.in_function.enabled(): self.generic_visit(node) - if isinstance(node.returns, ast.Name): - self._visit_annotation(node.returns) for i, statement in enumerate(node.body): if i == 0: @@ -882,8 +876,7 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: self.check_self_typevars(node) def visit_arguments(self, node: ast.arguments) -> None: - with self.visiting_function_arguments.enabled(): - self.generic_visit(node) + self.generic_visit(node) args = node.args[-len(node.defaults) :] for arg, default in chain( zip(args, node.defaults), zip(node.kwonlyargs, node.kw_defaults) @@ -900,7 +893,7 @@ def run(self, tree: ast.AST) -> Iterable[Error]: self.errors.clear() self.visit(tree) for (cls_name, typevar_name), def_node in self.typevarlike_defs.items(): - if typevar_name not in self.all_names_in_annotations: + if self.all_name_occurrences[typevar_name] == 1: self.error( def_node, Y018.format(typevarlike_cls=cls_name, typevar_name=typevar_name), @@ -909,7 +902,7 @@ def run(self, tree: ast.AST) -> Iterable[Error]: # as otherwise stock flake8 raises spurious errors about the name being undefined. # See https://github.com/python/typeshed/pull/6930 for thing_name, assign_node in self.suspicious_global_assignments.items(): - if thing_name not in self.all_names_in_annotations: + if self.all_name_occurrences[thing_name] == 1: self.error(assign_node, Y032) elif not isinstance(assign_node.value, ast.Ellipsis): self.error(assign_node, Y015) diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 69efa35b..b741141d 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -15,8 +15,6 @@ thing_using_field6: field6 thing_using_field7: field7 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 "..." diff --git a/tests/typevar.pyi b/tests/typevar.pyi index f283588a..9a4d8bd7 100644 --- a/tests/typevar.pyi +++ b/tests/typevar.pyi @@ -12,11 +12,12 @@ _Ts = TypeVarTuple("_Ts") # Y018 TypeVarTuple "_Ts" is not used _UsedTypeVar = TypeVar("_UsedTypeVar") def func(arg: _UsedTypeVar) -> _UsedTypeVar: ... +_UsedOnlyInClassDef = TypeVar("_UsedOnlyInClassDef") +class Foo(list[_UsedOnlyInClassDef]): ... + _TypeVarUsedInBinOp = TypeVar("_TypeVarUsedInBinOp", bound=str) def func2(arg: _TypeVarUsedInBinOp | int) -> _TypeVarUsedInBinOp | int: ... -_UnusedTypeVarWithSameNameAsClassAttributeElswhere = TypeVar("_UnusedTypeVarWithSameNameAsClassAttributeElswhere") # Y018 TypeVar "_UnusedTypeVarWithSameNameAsClassAttributeElswhere" is not used - _S = TypeVar("_S") class BadClass: @@ -35,4 +36,3 @@ class GoodClass: def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... - _UnusedTypeVarWithSameNameAsClassAttributeElswhere: str From c49bc2f96b573e85ef8cc2fb289480fbcac4fce4 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 15:10:02 +0000 Subject: [PATCH 3/8] Update README.rst Co-authored-by: Akuli --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 6212fa1b..314274d4 100644 --- a/README.rst +++ b/README.rst @@ -107,7 +107,7 @@ currently emitted: syntax wherever possible. (In situations where this is not possible, such as if a field is a Python keyword or an invalid identifier, this error will not be raised.) -* Y032: Unless the object is used elswhere in the same file as an annotation, +* Y032: Unless the object is used elsewhere in the same file, a module-level attribute should not have a default value. Many error codes enforce modern conventions, and some cannot yet be used in From 17e041e4826a49c5ccbda98b2ff4cf9030576424 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 15:32:43 +0000 Subject: [PATCH 4/8] Address review --- pyi.py | 9 ++++----- tests/attribute_annotations.pyi | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pyi.py b/pyi.py index eb17953f..57fdd3a6 100644 --- a/pyi.py +++ b/pyi.py @@ -473,9 +473,8 @@ def visit_Expr(self, node: ast.Expr) -> None: def visit_AnnAssign(self, node: ast.AnnAssign) -> None: self.generic_visit(node) - if isinstance(node.annotation, ast.Name): - if node.annotation.id == "TypeAlias": - return + if isinstance(node.annotation, ast.Name) and node.annotation.id == "TypeAlias": + return if isinstance(node.target, ast.Name): if self.in_class.active: if node.value and not isinstance(node.value, ast.Ellipsis): @@ -901,8 +900,8 @@ def run(self, tree: ast.AST) -> Iterable[Error]: # Only raise Y032 if the name is not used anywhere within the same file, # as otherwise stock flake8 raises spurious errors about the name being undefined. # See https://github.com/python/typeshed/pull/6930 - for thing_name, assign_node in self.suspicious_global_assignments.items(): - if self.all_name_occurrences[thing_name] == 1: + for symbol, assign_node in self.suspicious_global_assignments.items(): + if self.all_name_occurrences[symbol] == 1: self.error(assign_node, Y032) elif not isinstance(assign_node.value, ast.Ellipsis): self.error(assign_node, Y015) diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index b741141d..ca47ec58 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,18 +1,24 @@ +from typing import TypeAlias + field0: int field1: int = ... # Y032 Default value unnecessary for module-level attribute field2: int = 0 # Y032 Default value unnecessary for module-level attribute 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 "..." + +# TODO: Ideally the error code for field6 would be Y032, +# but it isn't because the `Foo` class has a separate attribute that is also named "field6", +# and the PyiVisitor plays it safe by only raising Y032 if a symbol only appears once in a file. field6 = 0 # Y015 Attribute must not have a default value other than "..." + field7 = b"" # Y015 Attribute must not have a default value other than "..." field8: str = ... -thing_using_field3: field3 -thing_using_field4: field4 -thing_using_field5: field5 -thing_using_field6: field6 -thing_using_field7: field7 +thing_using_field3: TypeAlias = field3.to_bytes +thing_using_field4: TypeAlias = field4.to_bytes +thing_using_field5: TypeAlias = field5.to_bytes +thing_using_field7: TypeAlias = field7.decode class Foo: field3 = ... # type: int @@ -20,4 +26,4 @@ class Foo: 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 "..." field7 = b"" # Y015 Attribute must not have a default value other than "..." - thing_using_field8_in_class: field8 + thing_using_field8_in_class: TypeAlias = field8.startswith From 44f1d3b69bc1b444508f33f0c26ec53fc333fec9 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 16:02:59 +0000 Subject: [PATCH 5/8] Improve error messages --- pyi.py | 55 ++++++++++++++++++++++++--------- tests/attribute_annotations.pyi | 20 ++++++------ 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/pyi.py b/pyi.py index 57fdd3a6..d0c97a6a 100644 --- a/pyi.py +++ b/pyi.py @@ -415,7 +415,7 @@ def visit_Assign(self, node: ast.Assign) -> None: else: self.error(target, Y001.format(cls_name)) if isinstance(node.value, (ast.Num, ast.Str, ast.Bytes)): - self.error(node.value, Y015) + self._Y015_error(node) # 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. @@ -478,7 +478,7 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None: if isinstance(node.target, ast.Name): if self.in_class.active: if node.value and not isinstance(node.value, ast.Ellipsis): - self.error(node.value, Y015) + self._Y015_error(node) else: if node.value: self.suspicious_global_assignments[node.target.id] = node @@ -885,26 +885,51 @@ def visit_arguments(self, node: ast.arguments) -> None: if not isinstance(default, ast.Ellipsis): self.error(default, (Y014 if arg.annotation is None else Y011)) - def error(self, node: ast.AST, message: str) -> None: - self.errors.append(Error(node.lineno, node.col_offset, message, PyiTreeChecker)) - - def run(self, tree: ast.AST) -> Iterable[Error]: - self.errors.clear() - self.visit(tree) + def _check_for_unused_typevars(self) -> None: + """After the PyiVisitor has visited the tree, check for unused TypeVars.""" for (cls_name, typevar_name), def_node in self.typevarlike_defs.items(): if self.all_name_occurrences[typevar_name] == 1: self.error( def_node, Y018.format(typevarlike_cls=cls_name, typevar_name=typevar_name), ) - # Only raise Y032 if the name is not used anywhere within the same file, - # as otherwise stock flake8 raises spurious errors about the name being undefined. - # See https://github.com/python/typeshed/pull/6930 + + def _Y015_error(self, node: ast.Assign | ast.AnnAssign) -> None: + copy_of_node = deepcopy(node) + copy_of_node.value = ast.Constant(value=...) + old_syntax, new_syntax = unparse(node), unparse(copy_of_node) + error_message = Y015.format(old_syntax=old_syntax, new_syntax=new_syntax) + self.error(node, error_message) + + def _check_global_assignments(self) -> None: + """After the PyiVisitor has visited the tree, check for Y032 and Y015 errors. + + Only raise Y032 if the name is not used anywhere within the same file, + as otherwise stock flake8 raises spurious errors about the name being undefined. + See https://github.com/python/typeshed/pull/6930. + + Only raise Y015 if Y032 is not applicable. + """ for symbol, assign_node in self.suspicious_global_assignments.items(): if self.all_name_occurrences[symbol] == 1: - self.error(assign_node, Y032) + copy_of_node = deepcopy(assign_node) + copy_of_node.value = None + old_syntax, new_syntax = unparse(assign_node), unparse(copy_of_node) + error_message = Y032.format( + old_syntax=old_syntax, new_syntax=new_syntax + ) + self.error(assign_node, error_message) elif not isinstance(assign_node.value, ast.Ellipsis): - self.error(assign_node, Y015) + self._Y015_error(assign_node) + + def error(self, node: ast.AST, message: str) -> None: + self.errors.append(Error(node.lineno, node.col_offset, message, PyiTreeChecker)) + + def run(self, tree: ast.AST) -> Iterable[Error]: + self.errors.clear() + self.visit(tree) + self._check_for_unused_typevars() + self._check_global_assignments() yield from self.errors @@ -974,7 +999,7 @@ def parse_options(cls, optmanager, options, extra_args) -> None: Y012 = 'Y012 Class body must not contain "pass"' Y013 = 'Y013 Non-empty class body must not contain "..."' Y014 = 'Y014 Default values for arguments must be "..."' -Y015 = 'Y015 Attribute must not have a default value other than "..."' +Y015 = 'Y015 Bad default value. Use "{new_syntax}" instead of "{old_syntax}"' Y016 = 'Y016 Duplicate union member "{}"' Y017 = "Y017 Only simple assignments allowed" Y018 = 'Y018 {typevarlike_cls} "{typevar_name}" is not used' @@ -994,4 +1019,4 @@ def parse_options(cls, optmanager, options, extra_args) -> None: Y029 = "Y029 Defining __repr__ or __str__ in a stub is almost always redundant" Y030 = "Y030 Multiple Literal members in a union. {suggestion}" Y031 = "Y031 Use class-based syntax for TypedDicts where possible" -Y032 = "Y032 Default value unnecessary for module-level attribute" +Y032 = 'Y032 Default value unnecessary. Use "{new_syntax}" instead of "{old_syntax}"' diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index ca47ec58..9dc26647 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,18 +1,18 @@ from typing import TypeAlias field0: int -field1: int = ... # Y032 Default value unnecessary for module-level attribute -field2: int = 0 # Y032 Default value unnecessary for module-level attribute +field1: int = ... # Y032 Default value unnecessary. Use "field1: int" instead of "field1: int = ..." +field2: int = 0 # Y032 Default value unnecessary. Use "field2: int" instead of "field2: int = 0" 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 "..." +field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0" +field5 = 0 # type: int # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0" # TODO: Ideally the error code for field6 would be Y032, # but it isn't because the `Foo` class has a separate attribute that is also named "field6", # and the PyiVisitor plays it safe by only raising Y032 if a symbol only appears once in a file. -field6 = 0 # Y015 Attribute must not have a default value other than "..." +field6 = 0 # Y015 Bad default value. Use "field6 = ..." instead of "field6 = 0" -field7 = b"" # Y015 Attribute must not have a default value other than "..." +field7 = b"" # Y015 Bad default value. Use "field7 = ..." instead of "field7 = b''" field8: str = ... thing_using_field3: TypeAlias = field3.to_bytes @@ -22,8 +22,8 @@ thing_using_field7: TypeAlias = field7.decode class Foo: 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 "..." - field7 = b"" # Y015 Attribute must not have a default value other than "..." + field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0" + field5 = 0 # type: int # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0" + field6 = 0 # Y015 Bad default value. Use "field6 = ..." instead of "field6 = 0" + field7 = b"" # Y015 Bad default value. Use "field7 = ..." instead of "field7 = b''" thing_using_field8_in_class: TypeAlias = field8.startswith From 398e703bf91f60e1c71a83b91dc64564b8bed49b Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 16:06:55 +0000 Subject: [PATCH 6/8] Fix for older Pythons? --- pyi.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pyi.py b/pyi.py index d0c97a6a..2b5b297c 100644 --- a/pyi.py +++ b/pyi.py @@ -897,8 +897,8 @@ def _check_for_unused_typevars(self) -> None: def _Y015_error(self, node: ast.Assign | ast.AnnAssign) -> None: copy_of_node = deepcopy(node) copy_of_node.value = ast.Constant(value=...) - old_syntax, new_syntax = unparse(node), unparse(copy_of_node) - error_message = Y015.format(old_syntax=old_syntax, new_syntax=new_syntax) + new_syntax = unparse(copy_of_node).replace("\n", "") + error_message = Y015.format(old_syntax=unparse(node), new_syntax=new_syntax) self.error(node, error_message) def _check_global_assignments(self) -> None: @@ -914,9 +914,9 @@ def _check_global_assignments(self) -> None: if self.all_name_occurrences[symbol] == 1: copy_of_node = deepcopy(assign_node) copy_of_node.value = None - old_syntax, new_syntax = unparse(assign_node), unparse(copy_of_node) + new_syntax = unparse(copy_of_node).replace("\n", "") error_message = Y032.format( - old_syntax=old_syntax, new_syntax=new_syntax + old_syntax=unparse(assign_node), new_syntax=new_syntax ) self.error(assign_node, error_message) elif not isinstance(assign_node.value, ast.Ellipsis): From 66c903497eb37a9606643d191b30899f9851201f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 16:13:18 +0000 Subject: [PATCH 7/8] Actually fix --- pyi.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pyi.py b/pyi.py index 2b5b297c..c01bf184 100644 --- a/pyi.py +++ b/pyi.py @@ -233,6 +233,11 @@ def visit_Index(self, node: ast.Index) -> ast.expr: return node.value +def _unparse_assign_node(node: ast.Assign | ast.AnnAssign) -> str: + """Unparse an Assign node, and remove any newlines in it""" + return unparse(node).replace("\n", "") + + def _is_list_of_str_nodes(seq: list[ast.expr | None]) -> TypeGuard[list[ast.Str]]: return all(isinstance(item, ast.Str) for item in seq) @@ -895,10 +900,11 @@ def _check_for_unused_typevars(self) -> None: ) def _Y015_error(self, node: ast.Assign | ast.AnnAssign) -> None: + old_syntax = _unparse_assign_node(node) copy_of_node = deepcopy(node) copy_of_node.value = ast.Constant(value=...) - new_syntax = unparse(copy_of_node).replace("\n", "") - error_message = Y015.format(old_syntax=unparse(node), new_syntax=new_syntax) + new_syntax = _unparse_assign_node(copy_of_node) + error_message = Y015.format(old_syntax=old_syntax, new_syntax=new_syntax) self.error(node, error_message) def _check_global_assignments(self) -> None: @@ -912,11 +918,12 @@ def _check_global_assignments(self) -> None: """ for symbol, assign_node in self.suspicious_global_assignments.items(): if self.all_name_occurrences[symbol] == 1: + old_syntax = _unparse_assign_node(assign_node) copy_of_node = deepcopy(assign_node) copy_of_node.value = None - new_syntax = unparse(copy_of_node).replace("\n", "") + new_syntax = _unparse_assign_node(copy_of_node) error_message = Y032.format( - old_syntax=unparse(assign_node), new_syntax=new_syntax + old_syntax=old_syntax, new_syntax=new_syntax ) self.error(assign_node, error_message) elif not isinstance(assign_node.value, ast.Ellipsis): From 3f61d27d1a729da984ca97281c2b257e6652ff3a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Jan 2022 20:23:31 +0000 Subject: [PATCH 8/8] Address review --- tests/attribute_annotations.pyi | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 9dc26647..a11b27bf 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,8 +1,8 @@ from typing import TypeAlias field0: int -field1: int = ... # Y032 Default value unnecessary. Use "field1: int" instead of "field1: int = ..." -field2: int = 0 # Y032 Default value unnecessary. Use "field2: int" instead of "field2: int = 0" +field_foo: int = ... # Y032 Default value unnecessary. Use "field_foo: int" instead of "field_foo: int = ..." +field_bar: int = 0 # Y032 Default value unnecessary. Use "field_bar: int" instead of "field_bar: int = 0" field3 = ... # type: int field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0" field5 = 0 # type: int # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0" @@ -21,6 +21,8 @@ thing_using_field5: TypeAlias = field5.to_bytes thing_using_field7: TypeAlias = field7.decode class Foo: + field1: int + field2: int = ... field3 = ... # type: int field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0" field5 = 0 # type: int # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0"