Skip to content

Commit

Permalink
Error on unused awaitable expressions (#12279)
Browse files Browse the repository at this point in the history
Generates an error when an expression has a type which has a
defined __await__ in its MRO but is not used. This includes all
builtin awaitable objects (e.g. futures, coroutines, and tasks)
but also anything a user might define which is also awaitable.

A hint is attached to suggest awaiting.

This can be extended in the future to other types of values that
may lead to a resource leak if needed, or even exposed as a plugin.

We test simple and complex cases (coroutines and user defined classes).
We also test that __getattr__ does not create false positives for
awaitables.

Some tests require fixes, either because they were deliberately not
awaiting an awaitable to verify some other logic in mypy, or because
reveal_type returns the object, so it was generating an error we would
rather simply silence,
  • Loading branch information
jhance authored Mar 11, 2022
1 parent 3a5b2a9 commit 536bac0
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 5 deletions.
28 changes: 26 additions & 2 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
from mypy.scope import Scope
from mypy import state, errorcodes as codes
from mypy.traverser import has_return_statement, all_return_statements
from mypy.errorcodes import ErrorCode
from mypy.errorcodes import ErrorCode, UNUSED_AWAITABLE, UNUSED_COROUTINE
from mypy.util import is_typeshed_file, is_dunder, is_sunder

T = TypeVar('T')
Expand Down Expand Up @@ -3432,8 +3432,32 @@ def try_infer_partial_type_from_indexed_assignment(
[key_type, value_type])
del partial_types[var]

def type_requires_usage(self, typ: Type) -> Optional[Tuple[str, ErrorCode]]:
"""Some types require usage in all cases. The classic example is
an unused coroutine.
In the case that it does require usage, returns a note to attach
to the error message.
"""
proper_type = get_proper_type(typ)
if isinstance(proper_type, Instance):
# We use different error codes for generic awaitable vs coroutine.
# 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:
return ("Are you missing an await?", UNUSED_AWAITABLE)
return None

def visit_expression_stmt(self, s: ExpressionStmt) -> None:
self.expr_checker.accept(s.expr, allow_none_return=True, always_allow_any=True)
expr_type = self.expr_checker.accept(s.expr, allow_none_return=True, always_allow_any=True)
error_note_and_code = self.type_requires_usage(expr_type)
if error_note_and_code:
error_note, code = error_note_and_code
self.fail(
message_registry.TYPE_MUST_BE_USED.format(format_type(expr_type)), s, code=code
)
self.note(error_note, s, code=code)

def visit_return_stmt(self, s: ReturnStmt) -> None:
"""Type check a return statement."""
Expand Down
9 changes: 9 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ def __str__(self) -> str:
LITERAL_REQ: Final = ErrorCode(
"literal-required", "Check that value is a literal", 'General'
)
UNUSED_COROUTINE: Final = ErrorCode(
"unused-coroutine", "Ensure that all coroutines are used", "General"
)

# These error codes aren't enabled by default.
NO_UNTYPED_DEF: Final[ErrorCode] = ErrorCode(
Expand Down Expand Up @@ -147,6 +150,12 @@ def __str__(self) -> str:
"General",
default_enabled=False,
)
UNUSED_AWAITABLE: Final = ErrorCode(
"unused-awaitable",
"Ensure that all awaitable values are used",
"General",
default_enabled=False,
)


# Syntax errors are often blocking.
Expand Down
1 change: 1 addition & 0 deletions mypy/message_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def format(self, *args: object, **kwargs: object) -> "ErrorMessage":
PYTHON2_PRINT_FILE_TYPE: Final = (
'Argument "file" to "print" has incompatible type "{}"; expected "{}"'
)
TYPE_MUST_BE_USED: Final = 'Value of type {} must be used'

# Generic
GENERIC_INSTANCE_VAR_CLASS_ACCESS: Final = (
Expand Down
28 changes: 27 additions & 1 deletion test-data/unit/check-async-await.test
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ async def f() -> int:

async def f() -> int:
return 0
reveal_type(f()) # N: Revealed type is "typing.Coroutine[Any, Any, builtins.int]"
_ = reveal_type(f()) # N: Revealed type is "typing.Coroutine[Any, Any, builtins.int]"
[builtins fixtures/async_await.pyi]
[typing fixtures/typing-async.pyi]

Expand Down Expand Up @@ -799,3 +799,29 @@ async def precise2(futures: Iterable[Awaitable[int]]) -> None:

[builtins fixtures/async_await.pyi]
[typing fixtures/typing-async.pyi]

[case testUnusedAwaitable]
# flags: --show-error-codes --enable-error-code unused-awaitable
from typing import Iterable

async def foo() -> None:
pass

class A:
def __await__(self) -> Iterable[int]:
yield 5

# Things with __getattr__ should not simply be considered awaitable.
class B:
def __getattr__(self, attr) -> object:
return 0

def bar() -> None:
A() # E: Value of type "A" must be used [unused-awaitable] \
# N: Are you missing an await?
foo() # E: Value of type "Coroutine[Any, Any, None]" must be used [unused-coroutine] \
# N: Are you missing an await?
B()

[builtins fixtures/async_await.pyi]
[typing fixtures/typing-async.pyi]
2 changes: 1 addition & 1 deletion test-data/unit/check-class-namedtuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ class XRepr(NamedTuple):
return 0

reveal_type(XMeth(1).double()) # N: Revealed type is "builtins.int"
reveal_type(XMeth(1).asyncdouble()) # N: Revealed type is "typing.Coroutine[Any, Any, builtins.int]"
_ = reveal_type(XMeth(1).asyncdouble()) # N: Revealed type is "typing.Coroutine[Any, Any, builtins.int]"
reveal_type(XMeth(42).x) # N: Revealed type is "builtins.int"
reveal_type(XRepr(42).__str__()) # N: Revealed type is "builtins.str"
reveal_type(XRepr(1, 2).__sub__(XRepr(3))) # N: Revealed type is "builtins.int"
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ async def g() -> NoReturn:
await f()

async def h() -> NoReturn: # E: Implicit return in function which does not return
f()
# Purposely not evaluating coroutine
_ = f()
[builtins fixtures/dict.pyi]
[typing fixtures/typing-async.pyi]

Expand Down

0 comments on commit 536bac0

Please sign in to comment.