Pyright: Warn about missing await when calling async functions

[shamrin]

Pyright doesn't help me when I forget to put await in front of async functions. This is relevant in all async frameworks like asyncio and Tornado. But this is especially interesting when using Trio, because there are no cases where you want to leave out await in front of async calls, including user defined functions.

import time
import trio

async def sleep():
    start = time.perf_counter()
    trio.sleep(1)
    duration = time.perf_counter() - start
    print(f'woke up after {duration:.2f} seconds')

trio.run(sleep)

$ python test.py
/home/user/src/test.py:6: RuntimeWarning: coroutine 'sleep' was never awaited
  trio.sleep(1)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
woke up after 0.00 seconds

Possible solution

Pyright should warn about missing await when calling async functions like trio.sleep, potentially behind a configuration option.

Additional context

• Trio documentation has a whole section warning people about missing await gotcha: https://trio.readthedocs.io/en/stable/tutorial.html#warning-don-t-forget-that-await
  > Why does this happen? In Trio, every time we use await it’s to call an async function, and every time we call an async function we use await. But Python’s trying to keep its options open for other libraries that are ahem a little less organized about things.
• Thread in Trio issue tracker discussing potential solutions involving mypy, pylint etc: https://github.com/python-trio/trio/issues/671
• https://github.com/microsoft/pyright/issues/607

[erictraut]

The challenge is that there is no need to immediately "await" an Awaitable. There are legitimate reasons why one might not want to do this. Pyright would generate many false positives if it assumed that all calls that returned an Awaitable required an "await" immediately.

Perhaps we could say that any Awaitable object that is dropped on the floor (not assigned to a variable, passed as an argument, returned from the function, etc.) should always require an await? Does that sound like it would cover most of the common cases where await is omitted unintentionally? And would that avoid all (or at least almost all) false positive errors?

[shamrin]

Interesting idea! Your suggestion seems safe and it should help in two of the three cases mentioned in python-trio/trio#671 description.

It probably wouldn't help for the remaining case - broken cpython bot example:

https://github.com/python/cpython/pull/9168#issuecomment-420361009

But of course, some protection is better than no protection at all.

[njsmith]

The challenge is that there is no need to immediately "await" an Awaitable

Yeah, that's why the issue over on the trio tracker mentions this as something for a pylint-trio/mypy-trio/etc. plugin to do... if you're using asyncio, then passing around Awaitables is common, but in trio programs, it's always an error.

Perhaps we could say that any Awaitable object that is dropped on the floor (not assigned to a variable, passed as an argument, returned from the function, etc.) should always require an await?

It's definitely an error to drop a coroutine object on the floor, for all users. This isn't true of all Awaitables – in particular it's not necessarily an error to drop an asyncio.Future on the floor. But it is true of the specific Awaitables which are coroutine objects returned from async def functions. So if it's possible to warn on that, it would certainly be a useful feature – it won't catch all cases, but at least it will catch some of them, and will never give false positives.

It would also be nice if there was some kind of configuration option to enable "strict" mode, where it becomes an error to do anything with a coroutine object besides await it. Ironically this is probably easier to implement than the safer version :-). The issue is that it wouldn't be useful to asyncio users, which is why you wouldn't want to enable it by default. But it would be very useful to trio users.

[erictraut]

The type system doesn't distinguish between a Coroutine that originated from an async def function and one that did not, so we can't use that in the test. I don't think it's safe to say that all Coroutine returned objects need to be awaited immediately, right? If my assumption is correct, I don't see a way to accurately distinguish between an Awaitable that must be "await"ed immediately and one that doesn't. It sounds like this may be a semantic that is specific to the trio library. If so, then it probably doesn't make sense to incorporate the rule into a general-purpose type checker or language server.

There is an existing diagnostic check in pyright called reportUnusedCallResult. It's off by default, but if enabled, it reports all cases where a function call returns a non-None value that is ignored. All non-None return values must be assigned to a variable, passed as an argument, returned, awaited, etc. This might be sufficient to catch these common errors for trio users. Please give this a try and let me know if it addresses your needs.

[njsmith]

The type system doesn't distinguish between a Coroutine that originated from an async def function and one that did not, so we can't use that in the test

Sure it does :-). Python's typing has both Coroutine and Awaitable types, where Coroutine is a subtype of Awaitable. Coroutines always come from async def (directly or indirectly). There are also other types like asyncio.Future that are Awaitables, but not Coroutines.

Sometimes it's okay to drop an Awaitable on the floor, but dropping a Coroutine on the floor is always an error, no matter what -- in fact the interpreter prints a warning whenever this happens, and the only reason it doesn't raise a proper exception is because of technical limitations. So it would be a strict improvement for pyright to unconditionally enable reportedUnusedCallResult for all functions whose return type is Coroutine. Enabling reportUnusedCallResult for all functions would cause tons of false positives, so I don't think it's tenable. Enabling it for just Coroutine-returning functions would have false negatives, but never any false positives.

That part is true for all Python users. The part that's specific to Trio users is that we could go one step further and eliminate the false negatives as well, by reporting an error whenever a Coroutine return value is consumed by something other than an await. I do get how you might not want to put special features into pyright for a single library like that -- though it would be nice if there was some kind of config or plugin that could enable it. (Some of our users already use a pylint plugin to implement this.)

[erictraut]

Ah, I didn't realize that Coroutine was generated only by an async def. Then it makes sense to use that as a distinguisher. The new diagnostic rule reportUnusedCoroutine would be similar to reportUnusedCallResult except that it would be limited specifically to functions that return a Coroutine object.

Pyright has two type checking modes: basic and strict. These modes affect which diagnostic rules are enabled and their resulting severity (warning vs. error). (Defaults in both modes can be overridden individually.) Basic mode is usually more conservative than strict, but if we're confident that we won't get any false positives, I think it would be fine to enable reportUnusedCoroutine for both basic and strict and make it an error in both cases.

[erictraut]

I've added the new diagnostic rule and enabled it by default for both basic and strict modes. Here's the PR. This will be included in the next release of pyright and pylance. Thanks for the suggestion and the detailed advice.

[shamrin]

Thank you @erictraut! This is very nice 👍

I wonder if this would cover most problems now. Some other forgotten await cases Pyright already covers:

async def coro(): return 1
def addone(n: int): return n + 1

async def main():
    # "Coroutine[Any, Any, Literal[1]]" is incompatible with "int"
    result = addone(coro())  
    # Operator "+" not supported for types "Coroutine[Any, Any, Literal[1]]" and "Literal[1]"
    result = coro() + 1      

In another case it seems impossible to implement without failing for commonly used patterns:

async def coro(): return 1

async def main():
    n = coro()
    # No error, but it's useful to debug print things in Python, including coroutines
    print(f'n={n}')  

(I'm guessing the last example is similar to what happened above with cpython bot.)

[erictraut]

This is now addressed in Pyright 1.1.98, which I just published. It will also be in the next release of Pylance.