Minor improvements to Y029 and Y036 checks (#216)

Author: AlexWaygood

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 Features:
 * Introduce Y039: Use `str` instead of `typing.Text` for Python 3 stubs.
+* Teach the Y036 check that `builtins.object` (as well as the unqualified `object`) is
+  acceptable as an annotation for an `__(a)exit__` method argument.
+* Teach the Y029 check to emit errors for `__repr__` and `__str__` methods that return
+  `builtins.str` (as opposed to the unqualified `str`).
 
 ## 22.4.1
 

pyi.py

@@ -296,7 +296,7 @@ def _is_name(node: ast.expr | None, name: str) -> bool:
 _TYPING_MODULES = frozenset({"typing", "typing_extensions"})
 
 
-def _is_object(node: ast.expr, name: str, *, from_: Container[str]) -> bool:
+def _is_object(node: ast.expr | None, name: str, *, from_: Container[str]) -> bool:
     """Determine whether `node` is an ast representation of `name`.
 
     Return True if `node` is either:
@@ -334,6 +334,7 @@ def _is_object(node: ast.expr, name: str, *, from_: Container[str]) -> bool:
 _is_Final = partial(_is_object, name="Final", from_=_TYPING_MODULES)
 _is_Self = partial(_is_object, name="Self", from_=({"_typeshed"} | _TYPING_MODULES))
 _is_TracebackType = partial(_is_object, name="TracebackType", from_={"types"})
+_is_builtins_object = partial(_is_object, name="object", from_={"builtins"})
 
 
 def _is_type_or_Type(node: ast.expr) -> bool:
@@ -1165,7 +1166,8 @@ def error_for_bad_exit_method(details: str) -> None:
             if varargs:
                 varargs_annotation = varargs.annotation
                 if not (
-                    varargs_annotation is None or _is_name(varargs_annotation, "object")
+                    varargs_annotation is None
+                    or _is_builtins_object(varargs_annotation)
                 ):
                     error_for_bad_exit_method(
                         f'Star-args in an {method_name} method should be annotated with "object", '
@@ -1211,7 +1213,7 @@ def error_for_bad_annotation(
 
         if num_args >= 2:
             arg1_annotation = non_kw_only_args[1].annotation
-            if arg1_annotation is None or _is_name(arg1_annotation, "object"):
+            if arg1_annotation is None or _is_builtins_object(arg1_annotation):
                 pass
             elif _is_PEP_604_union(arg1_annotation):
                 is_union_with_None, non_None_part = _analyse_exit_method_arg(
@@ -1229,7 +1231,7 @@ def error_for_bad_annotation(
 
         if num_args >= 3:
             arg2_annotation = non_kw_only_args[2].annotation
-            if arg2_annotation is None or _is_name(arg2_annotation, "object"):
+            if arg2_annotation is None or _is_builtins_object(arg2_annotation):
                 pass
             elif _is_PEP_604_union(arg2_annotation):
                 is_union_with_None, non_None_part = _analyse_exit_method_arg(
@@ -1242,7 +1244,7 @@ def error_for_bad_annotation(
 
         if num_args >= 4:
             arg3_annotation = non_kw_only_args[3].annotation
-            if arg3_annotation is None or _is_name(arg3_annotation, "object"):
+            if arg3_annotation is None or _is_builtins_object(arg3_annotation):
                 pass
             elif _is_PEP_604_union(arg3_annotation):
                 is_union_with_None, non_None_part = _analyse_exit_method_arg(
@@ -1299,7 +1301,7 @@ def _visit_synchronous_method(self, node: ast.FunctionDef) -> None:
         if method_name in {"__repr__", "__str__"}:
             if (
                 len(non_kw_only_args) == 1
-                and _is_name(node.returns, "str")
+                and _is_object(node.returns, "str", from_={"builtins"})
                 and not any(_is_abstractmethod(deco) for deco in node.decorator_list)
             ):
                 self.error(node, Y029)
@@ -1374,6 +1376,8 @@ def _check_class_method_for_bad_typevars(
         else:
             return
 
+        # Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]`
+        # These are edge cases, and it's hard to give good error messages for them.
         if not _is_name(first_arg_annotation.value, "type"):
             return
 

tests/classdefs.pyi

@@ -1,4 +1,5 @@
 import abc
+import builtins
 import collections.abc
 import typing
 from abc import abstractmethod
@@ -11,7 +12,7 @@ from typing_extensions import final
 class Bad:
     def __new__(cls, *args: Any, **kwargs: Any) -> Bad: ...  # Y034 "__new__" methods usually return "self" at runtime. Consider using "_typeshed.Self" in "Bad.__new__", e.g. "def __new__(cls: type[Self], *args: Any, **kwargs: Any) -> Self: ..."
     def __repr__(self) -> str: ...  # Y029 Defining __repr__ or __str__ in a stub is almost always redundant
-    def __str__(self) -> str: ...  # Y029 Defining __repr__ or __str__ in a stub is almost always redundant
+    def __str__(self) -> builtins.str: ...  # Y029 Defining __repr__ or __str__ in a stub is almost always redundant
     def __eq__(self, other: Any) -> bool: ...  # Y032 Prefer "object" to "Any" for the second parameter in "__eq__" methods
     def __ne__(self, other: typing.Any) -> typing.Any: ...  # Y032 Prefer "object" to "Any" for the second parameter in "__ne__" methods
     def __enter__(self) -> Bad: ...  # Y034 "__enter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "_typeshed.Self" in "Bad.__enter__", e.g. "def __enter__(self: Self) -> Self: ..."

tests/exit_methods.pyi

@@ -1,3 +1,4 @@
+import builtins
 import types
 import typing
 from collections.abc import Awaitable
@@ -15,7 +16,7 @@ class One:
     async def __aexit__(self, *args) -> str: ...
 
 class Two:
-    def __exit__(self, typ: type[BaseException] | None, *args: object) -> bool | None: ...
+    def __exit__(self, typ: type[BaseException] | None, *args: builtins.object) -> bool | None: ...
     async def __aexit__(self, typ: Type[BaseException] | None, *args: object) -> bool: ...
 
 class Three:
@@ -31,8 +32,8 @@ class Five:
     def __aexit__(self, typ: type[BaseException] | None, exc: BaseException | None, tb: TracebackType | None) -> Awaitable[None]: ...
 
 class FiveAndAHalf:
-    def __exit__(self, typ: object, exc: object, tb: object) -> None: ...
-    async def __aexit__(self, typ: object, exc: object, tb: object) -> None: ...
+    def __exit__(self, typ: object, exc: builtins.object, tb: object) -> None: ...
+    async def __aexit__(self, typ: object, exc: object, tb: builtins.object) -> None: ...
 
 def __exit__(foo: str, bar: int) -> list[str]: ...
 async def __aexit__(spam: bytes, eggs: dict[str, Any]) -> set[int]: ...