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..314274d4 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 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 all cases: diff --git a/pyi.py b/pyi.py index a6b51fc2..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) @@ -298,6 +303,7 @@ def __init__(self, filename: Path = Path("none")) -> None: 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.string_literals_allowed = NestingCounter() self.in_function = NestingCounter() self.in_class = NestingCounter() @@ -414,7 +420,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. @@ -474,8 +480,13 @@ 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.target, ast.Name): + if self.in_class.active: + if node.value and not isinstance(node.value, ast.Ellipsis): + self._Y015_error(node) + 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]] = {} @@ -879,18 +890,53 @@ 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), ) + + 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_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: + """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: + old_syntax = _unparse_assign_node(assign_node) + copy_of_node = deepcopy(assign_node) + copy_of_node.value = None + new_syntax = _unparse_assign_node(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._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 @@ -960,7 +1006,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' @@ -980,3 +1026,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. Use "{new_syntax}" instead of "{old_syntax}"' diff --git a/tests/attribute_annotations.pyi b/tests/attribute_annotations.pyi index 5662faa9..a11b27bf 100644 --- a/tests/attribute_annotations.pyi +++ b/tests/attribute_annotations.pyi @@ -1,16 +1,31 @@ -field1: int -field2: int = ... +from typing import TypeAlias + +field0: int +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 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" + +# 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 Bad default value. Use "field6 = ..." instead of "field6 = 0" + +field7 = b"" # Y015 Bad default value. Use "field7 = ..." instead of "field7 = b''" +field8: str = ... + +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: 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 "..." - 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 diff --git a/tests/typevar.pyi b/tests/typevar.pyi index 35c7955b..9a4d8bd7 100644 --- a/tests/typevar.pyi +++ b/tests/typevar.pyi @@ -12,6 +12,9 @@ _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: ...