Skip to content
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

Type narrowing failure inside AsyncExitStack blocks #13936

Closed
AlexWaygood opened this issue Oct 22, 2022 · 1 comment · Fixed by #14151
Closed

Type narrowing failure inside AsyncExitStack blocks #13936

AlexWaygood opened this issue Oct 22, 2022 · 1 comment · Fixed by #14151
Labels
bug mypy got something wrong topic-async async, await, asyncio topic-type-context Type context / bidirectional inference topic-type-narrowing Conditional type narrowing / binder

Comments

@AlexWaygood
Copy link
Member

Bug Report

In the following synchronous code, mypy is correctly able to perform type narrowing to determine that the resource variable must be an instance of Resource when resource.do_thing() is called inside the foo() function:

from contextlib import AbstractContextManager
from typing import TypeVar
T = TypeVar("T")
Self = TypeVar("Self")

class ExitStack:
    def __enter__(self: Self) -> Self:
        return self
    def __exit__(self, *args: object) -> None:
        pass
    def enter_context(self, cm: AbstractContextManager[T]) -> T:
        return cm.__enter__()

class Resource:
    def __enter__(self: Self) -> Self:
        return self
    def __exit__(self, *args: object) -> None:
        pass
    def do_thing(self) -> None:
        pass

def foo(resource: Resource | None) -> None:
    with ExitStack() as stack:
        if resource is None:
            resource = stack.enter_context(Resource())
        resource.do_thing()

However, in an asynchronous version of this code, mypy is not able to narrow the type in the same way, leading to a false positive being emitted:

from contextlib import AbstractAsyncContextManager
from typing import TypeVar
T = TypeVar("T")
Self = TypeVar("Self")

class AsyncExitStack:
    async def __aenter__(self: Self) -> Self:
        return self
    async def __aexit__(self, *args: object) -> None:
        pass
    async def enter_async_context(self, cm: AbstractAsyncContextManager[T]) -> T:
        return await cm.__aenter__()

class AsyncResource:
    async def __aenter__(self: Self) -> Self:
        return self
    async def __aexit__(self, *args: object) -> None:
        pass
    def do_thing(self) -> None:
        pass

async def async_foo(resource: AsyncResource | None) -> None:
    async with AsyncExitStack() as stack:
        if resource is None:
            resource = await stack.enter_async_context(AsyncResource())
        resource.do_thing()  # error: Item "None" of "Optional[AsyncResource]" has no attribute "do_thing"

Curiously, if we rewrite async_foo() like this, the false-positive error goes away:

async def async_foo(resource: AsyncResource | None) -> None:
    async with AsyncExitStack() as stack:
        if resource is None:
            res = await stack.enter_async_context(AsyncResource())
            resource = res
        resource.do_thing()

Real-life use case

My use case is to write a function like this using aiohttp, in which a user can optionally supply an existing aiohttp.ClientSession() instance if they want to, or let the function create one on the fly if not.

import aiohttp
from contextlib import AsyncExitStack

async def foo(session: aiohttp.ClientSession | None = None):
    async with AsyncExitStack() as stack:
        if session is None:
            session = await stack.enter_async_context(aiohttp.ClientSession())
        async with session.get("https://foo.com/json") as response:
            data = await response.json()

To Reproduce

https://mypy-play.net/?mypy=latest&python=3.10&gist=0ee494765b5abc08fc9f161bc63656a7

Your Environment

  • Mypy version used: 0.982; also occurs on mypy master (commit 8d5b641)
@AlexWaygood AlexWaygood added bug mypy got something wrong topic-type-narrowing Conditional type narrowing / binder topic-async async, await, asyncio labels Oct 22, 2022
@ramvikrams
Copy link

ramvikrams commented Oct 26, 2022

async def async_foo(resource: AsyncResource ) -> None:
can we not remove the or operator here and later resource can be told if it is null

@hauntsaninja hauntsaninja added the topic-type-context Type context / bidirectional inference label Oct 26, 2022
ilevkivskyi added a commit that referenced this issue Nov 22, 2022
Fixes #4805
Fixes #13936

It is known that mypy can overuse outer type context sometimes
(especially when it is a union). This prevents a common use case for
narrowing types (see issue and test cases). This is a somewhat major
semantic change, but I think it should match better what a user would
expect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-async async, await, asyncio topic-type-context Type context / bidirectional inference topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants