Skip to content

stubtest: relax async checking for abstract methods #12343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions mypy/stubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more idiomatic way of figuring out whether the return type of a method in the stub is a subtype of Awaitable[Any]?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the unused awaitable stuff does something similar.

Copy link
Member Author

@AlexWaygood AlexWaygood Mar 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've modified my PR to add a helper property in mypy.types to avoid code duplication

)
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
Expand Down
53 changes: 52 additions & 1 deletion mypy/test/teststubtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]): ...
Expand Down Expand Up @@ -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: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to allow this? Doesn't seem particularly useful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember, and I'm not sure I care anymore. I guess I'll just close this PR :)

""",
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]:
Expand Down