From f0a3e3599217e4c1fae9368de5c05590b530071d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 13 Mar 2022 19:34:06 +0000 Subject: [PATCH 1/9] stubtest: relax async checking for abstract methods --- mypy/stubtest.py | 33 ++++++++++++++++++------ mypy/test/teststubtest.py | 53 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index b568017cfa5f..3f934d49a3a5 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -734,14 +734,31 @@ def verify_funcitem( # That results in false positives. # See https://github.com/python/typeshed/issues/7344 if runtime_is_coroutine and not stub.is_coroutine: - yield Error( - object_path, - 'is an "async def" function at runtime, but not in the stub', - stub, - runtime, - stub_desc=stub_desc, - runtime_desc=runtime_sig_desc - ) + if ( + getattr(runtime, "__isabstractmethod__", False) + or (isinstance(stub, nodes.FuncDef) and stub.is_abstract) + ): + # Be more permissive if the method is an abstractmethod: + # Only error if the return type of the stub isn't awaitable + if isinstance(stub.type, mypy.types.CallableType): + ret_type = mypy.types.get_proper_type(stub.type.ret_type) + should_error = ( + isinstance(ret_type, mypy.types.Instance) + and "__await__" not in ret_type.type.names + ) + else: + should_error = False + else: + should_error = True + if should_error: + yield Error( + object_path, + 'is an "async def" function at runtime, but not in the stub', + stub, + runtime, + stub_desc=stub_desc, + runtime_desc=runtime_sig_desc + ) if not signature: return diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 759942c57ce7..30ea07ce89ad 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -54,8 +54,11 @@ def __init__(self, name, covariant: bool = ..., contravariant: bool = ...) -> No _S = TypeVar("_S", contravariant=True) _R = TypeVar("_R", covariant=True) -class Coroutine(Generic[_T_co, _S, _R]): ... class Iterable(Generic[_T_co]): ... +class Generator(Iterable[_T_co], Generic[_T_co, _S, _R]): ... +class Awaitable(Generic[_T_co]): + def __await__(self) -> Generator[Any, None, _T_co]: ... +class Coroutine(Generic[_T_co, _S, _R]): ... class Mapping(Generic[_K, _V]): ... class Sequence(Iterable[_T_co]): ... class Tuple(Sequence[_T_co]): ... @@ -229,6 +232,54 @@ def test_coroutines(self) -> Iterator[Case]: runtime="async def bingo(): return 5", error=None, ) + # Be more permissive if it's an abstractmethod in the stub... + yield Case( + stub=""" + from abc import abstractmethod + from typing import Awaitable + class Bar: + @abstractmethod + def abstract(self) -> Awaitable[int]: ... + """, + runtime=""" + class Bar: + async def abstract(self): return 5 + """, + error=None, + ) + # ...and/or an abstractmethod at runtime... + yield Case( + stub=""" + from typing import Generator, Any + class _CustomAwaitable: + def __await__(self) -> Generator[Any, Any, Any]: ... + class Foo: + def abstract(self) -> _CustomAwaitable: ... + """, + runtime=""" + from abc import abstractmethod + class Foo: + @abstractmethod + async def abstract(self): return 5 + """, + error=None, + ) + # ...but still error if the stub's return type isn't awaitable + yield Case( + stub=""" + from abc import abstractmethod + class Baz: + @abstractmethod + def abstract(self) -> int: ... + """, + runtime=""" + from abc import abstractmethod + class Baz: + @abstractmethod + async def abstract(self): return 5 + """, + error="Baz.abstract", + ) @collect_cases def test_arg_name(self) -> Iterator[Case]: From 1488e1451c8060108351f3894cd94d1a11d40413 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 13 Mar 2022 20:04:59 +0000 Subject: [PATCH 2/9] Fix --- mypy/stubtest.py | 9 +++++++-- mypy/test/teststubtest.py | 11 +++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 3f934d49a3a5..baa98b9ada4a 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -738,22 +738,27 @@ def verify_funcitem( getattr(runtime, "__isabstractmethod__", False) or (isinstance(stub, nodes.FuncDef) and stub.is_abstract) ): + error_msg = ( + "is an \"async def\" function at runtime, " + "but doesn't return an awaitable in the stub" + ) # Be more permissive if the method is an abstractmethod: # Only error if the return type of the stub isn't awaitable if isinstance(stub.type, mypy.types.CallableType): ret_type = mypy.types.get_proper_type(stub.type.ret_type) should_error = ( isinstance(ret_type, mypy.types.Instance) - and "__await__" not in ret_type.type.names + and not any("__await__" in clsdef.names for clsdef in ret_type.type.mro) ) else: should_error = False else: + error_msg = 'is an "async def" function at runtime, but not in the stub' should_error = True if should_error: yield Error( object_path, - 'is an "async def" function at runtime, but not in the stub', + error_msg, stub, runtime, stub_desc=stub_desc, diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 30ea07ce89ad..13e3cf162ebb 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -58,7 +58,7 @@ class Iterable(Generic[_T_co]): ... class Generator(Iterable[_T_co], Generic[_T_co, _S, _R]): ... class Awaitable(Generic[_T_co]): def __await__(self) -> Generator[Any, None, _T_co]: ... -class Coroutine(Generic[_T_co, _S, _R]): ... +class Coroutine(Awaitable[_T_co], Generic[_T_co, _S, _R]): ... class Mapping(Generic[_K, _V]): ... class Sequence(Iterable[_T_co]): ... class Tuple(Sequence[_T_co]): ... @@ -250,17 +250,24 @@ async def abstract(self): return 5 # ...and/or an abstractmethod at runtime... yield Case( stub=""" - from typing import Generator, Any + from typing import Generator, Any, Coroutine, Awaitable class _CustomAwaitable: def __await__(self) -> Generator[Any, Any, Any]: ... + class _CustomAwaitable2(Awaitable[Any]): ... class Foo: def abstract(self) -> _CustomAwaitable: ... + def abstract2(self) -> _CustomAwaitable2: ... + def abstract3(self) -> Coroutine[Any, Any, Any]: ... """, runtime=""" from abc import abstractmethod class Foo: @abstractmethod async def abstract(self): return 5 + @abstractmethod + async def abstract2(self): return 4 + @abstractmethod + async def abstract3(self): return 5 """, error=None, ) From 8db69eae3e7c588bed1247977f7ec760e05ce8e4 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 14 Mar 2022 18:32:27 +0000 Subject: [PATCH 3/9] Also fix error message broken on master --- mypy/stubtest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index baa98b9ada4a..45dabd298df0 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -726,7 +726,7 @@ def verify_funcitem( stub_sig = Signature.from_funcitem(stub) runtime_sig = Signature.from_inspect_signature(signature) runtime_sig_desc = f'{"async " if runtime_is_coroutine else ""}def {signature}' - stub_desc = f'def {stub_sig!r}' + stub_desc = str(stub_sig) else: runtime_sig_desc, stub_desc = None, None From c5fb13a04266a0f1ef5536b6f6bb59e104014ee5 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 22 Mar 2022 17:33:00 +0100 Subject: [PATCH 4/9] Add property to MyPy --- mypy/types.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mypy/types.py b/mypy/types.py index dadbb84fe1c8..a463fc8c1da8 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1121,6 +1121,10 @@ def copy_modified(self, *, def has_readable_member(self, name: str) -> bool: return self.type.has_readable_member(name) + + @property + def is_awaitable(self) -> bool: + return self.type.get("__await__") is not None class FunctionLike(ProperType): From 90779dbd0bea46892f40dde2583afe665e4ecc87 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 22 Mar 2022 17:36:15 +0100 Subject: [PATCH 5/9] Use property in stubtest --- mypy/stubtest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 45dabd298df0..ac7916a8f549 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -748,7 +748,7 @@ def verify_funcitem( ret_type = mypy.types.get_proper_type(stub.type.ret_type) should_error = ( isinstance(ret_type, mypy.types.Instance) - and not any("__await__" in clsdef.names for clsdef in ret_type.type.mro) + and not ret_type.is_awaitable ) else: should_error = False From 21fed7ae159ffde58289fca4428019f63654e15f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 22 Mar 2022 17:37:48 +0100 Subject: [PATCH 6/9] Use property in checker.py --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index e0f2398b67fc..9b3498aabef8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -3445,7 +3445,7 @@ def type_requires_usage(self, typ: Type) -> Optional[Tuple[str, ErrorCode]]: # Coroutines are on by default, whereas generic awaitables are not. if proper_type.type.fullname == "typing.Coroutine": return ("Are you missing an await?", UNUSED_COROUTINE) - if proper_type.type.get("__await__") is not None: + if proper_type.is_awaitable: return ("Are you missing an await?", UNUSED_AWAITABLE) return None From c81355b28a8c4727f576eed54adfc88ee5cba120 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 22 Mar 2022 17:42:40 +0100 Subject: [PATCH 7/9] Fix lint --- mypy/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/types.py b/mypy/types.py index a463fc8c1da8..f6e60138b7fe 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1121,7 +1121,7 @@ def copy_modified(self, *, def has_readable_member(self, name: str) -> bool: return self.type.has_readable_member(name) - + @property def is_awaitable(self) -> bool: return self.type.get("__await__") is not None From c6a17682c3f566807ba60c2563e86b0353b54036 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Wed, 27 Jul 2022 15:18:09 +0100 Subject: [PATCH 8/9] Blacken --- mypy/stubtest.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index fa9732caed75..06d67e295437 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -794,12 +794,11 @@ def verify_funcitem( # That results in false positives. # See https://github.com/python/typeshed/issues/7344 if runtime_is_coroutine and not stub.is_coroutine: - if ( - getattr(runtime, "__isabstractmethod__", False) - or (isinstance(stub, nodes.FuncDef) and stub.is_abstract) + if getattr(runtime, "__isabstractmethod__", False) or ( + isinstance(stub, nodes.FuncDef) and stub.is_abstract ): error_msg = ( - "is an \"async def\" function at runtime, " + 'is an "async def" function at runtime, ' "but doesn't return an awaitable in the stub" ) # Be more permissive if the method is an abstractmethod: @@ -807,8 +806,7 @@ def verify_funcitem( if isinstance(stub.type, mypy.types.CallableType): ret_type = mypy.types.get_proper_type(stub.type.ret_type) should_error = ( - isinstance(ret_type, mypy.types.Instance) - and not ret_type.is_awaitable + isinstance(ret_type, mypy.types.Instance) and not ret_type.is_awaitable ) else: should_error = False @@ -822,7 +820,7 @@ def verify_funcitem( stub, runtime, stub_desc=stub_desc, - runtime_desc=runtime_sig_desc + runtime_desc=runtime_sig_desc, ) if not signature: From a5cb05ffa9760cd36599b452ed5b0305c3fa2351 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Mon, 15 Aug 2022 07:55:17 +0100 Subject: [PATCH 9/9] Fix --- mypy/stubtest.py | 16 +++++++++++----- mypy/test/teststubtest.py | 4 ++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/mypy/stubtest.py b/mypy/stubtest.py index 1252c367267b..27b86f388fb4 100644 --- a/mypy/stubtest.py +++ b/mypy/stubtest.py @@ -835,10 +835,8 @@ def verify_funcitem( return if isinstance(stub, nodes.FuncDef): - stub_abstract = stub.abstract_status == nodes.IS_ABSTRACT - runtime_abstract = getattr(runtime, "__isabstractmethod__", False) # The opposite can exist: some implementations omit `@abstractmethod` decorators - if runtime_abstract and not stub_abstract: + if runtime_is_abstract(runtime) and not stub_is_abstract(stub): yield Error( object_path, "is inconsistent, runtime method is abstract but stub is not", @@ -864,8 +862,8 @@ def verify_funcitem( # That results in false positives. # See https://github.com/python/typeshed/issues/7344 if runtime_is_coroutine and not stub.is_coroutine: - if getattr(runtime, "__isabstractmethod__", False) or ( - isinstance(stub, nodes.FuncDef) and stub.is_abstract + if runtime_is_abstract(runtime) or ( + isinstance(stub, nodes.FuncDef) and stub_is_abstract(stub) ): error_msg = ( 'is an "async def" function at runtime, ' @@ -1280,6 +1278,14 @@ def is_read_only_property(runtime: object) -> bool: return isinstance(runtime, property) and runtime.fset is None +def stub_is_abstract(stub: nodes.FuncDef) -> bool: + return stub.abstract_status == nodes.IS_ABSTRACT + + +def runtime_is_abstract(runtime: object) -> bool: + return getattr(runtime, "__isabstractmethod__", False) + + def safe_inspect_signature(runtime: Any) -> Optional[inspect.Signature]: try: return inspect.signature(runtime) diff --git a/mypy/test/teststubtest.py b/mypy/test/teststubtest.py index 3e32f2f5a1aa..e13aafed3200 100644 --- a/mypy/test/teststubtest.py +++ b/mypy/test/teststubtest.py @@ -245,13 +245,17 @@ async def abstract(self): return 5 # ...and/or an abstractmethod at runtime... yield Case( stub=""" + from abc import abstractmethod from typing import Generator, Any, Coroutine, Awaitable class _CustomAwaitable: def __await__(self) -> Generator[Any, Any, Any]: ... class _CustomAwaitable2(Awaitable[Any]): ... class Foo: + @abstractmethod def abstract(self) -> _CustomAwaitable: ... + @abstractmethod def abstract2(self) -> _CustomAwaitable2: ... + @abstractmethod def abstract3(self) -> Coroutine[Any, Any, Any]: ... """, runtime="""