Introduce a check disallowing module-level defaults, and improve Y015 error messages

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

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:

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}"'

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

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