[ty] Add a diagnostic for an unused awaitable#23650
Merged
charliermarsh merged 1 commit intomainfrom Mar 4, 2026
Merged
Conversation
Typing conformance resultsNo changes detected ✅ |
|
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unused-awaitable |
15 | 0 | 0 |
unused-type-ignore-comment |
0 | 2 | 0 |
| Total | 15 | 2 | 0 |
oconnor663
approved these changes
Mar 4, 2026
Contributor
oconnor663
left a comment
There was a problem hiding this comment.
LGTM overall, approving with comments.
| # error: [invalid-context-manager] "Object of type `Manager` cannot be used with `async with` because the method `__aenter__` may be missing" | ||
| async with Manager() as f: | ||
| reveal_type(f) # revealed: CoroutineType[Any, Any, str] | ||
| reveal_type(f) # revealed: CoroutineType[Any, Any, str] # ty: ignore[unused-awaitable] |
Contributor
There was a problem hiding this comment.
Should we special-case KnownFunction::RevealType in this lint?
Contributor
There was a problem hiding this comment.
It might make sense to do AssertType at the same time, that shows up in the ecosystem report for trio.
|
|
||
| async def main(): | ||
| coro = fetch() | ||
| await coro |
Contributor
There was a problem hiding this comment.
IIUC, the await coro line here is extra, and coro = fetch() by itself is enough by itself to suppress this warning. Ruff will give an unused variable warning about coro in that case, but Ty alone will not. Maybe remove the second line here and add a TODO if we expect Ty to eventually emit a warning here?
| from types import CoroutineType | ||
| from typing import Any | ||
|
|
||
| def get_coroutine(flag: bool) -> CoroutineType[Any, Any, int] | CoroutineType[Any, Any, str]: |
Contributor
There was a problem hiding this comment.
Any particular reason there's a bool arg here and in the next test case?
Comment on lines
+1012
to
+1015
| Type::Intersection(intersection) => intersection | ||
| .positive(db) | ||
| .iter() | ||
| .any(|ty| ty.is_awaitable(db)), |
Contributor
There was a problem hiding this comment.
We should probably have a test case for this branch. CoroutineType is @final, so it's tricky to write an intersection for it that doesn't simplify, but something like this seems to work for me:
diff --git i/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md w/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md
index 162e730d65..edfe302793 100644
--- i/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md
+++ w/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md
@@ -87,6 +87,25 @@ async def main():
get_maybe_coroutine(True)
```
+## Intersection with awaitable
+
+When an intersection type contains an awaitable element, the lint should fire.
+
+```py
+from collections.abc import Coroutine
+from types import CoroutineType
+from ty_extensions import Intersection
+
+class Foo: ...
+class Bar: ...
+
+def get_coroutine() -> Intersection[Coroutine[Foo, Foo, Foo], CoroutineType[Bar, Bar, Bar]]:
+ raise NotImplementedError
+
+async def main():
+ get_coroutine() # error: [unused-awaitable]
+```
+
## Non-awaitable expression statement
Regular non-awaitable expression statements should not trigger this lint.
(Formatting diffs with ``` in them is hard...)
|
|
||
| /// Returns `true` if this type is an awaitable that should be awaited before being discarded. | ||
| /// | ||
| /// Currently checks for instances of `types.CoroutineType` (returned by `async def` calls). |
Contributor
There was a problem hiding this comment.
Out of curiosity, did you consider checking for "assignability to Awaitable"? My instinct is that that's too expensive to evaluate on every single expression statement, but I'm not sure.
31011da to
184e2cc
Compare
Member
|
This was probably one of the first "lintish" rule that we added to ty that isn't strictly necessary to be implemented in type inference because it doesn't change the inferred type. If we end up adding more such rules, it probably makes sense to consider introducing a dedicated linter pass for checks that aren't part of type inference (and may also get access to broader settings). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is a warning by default.
Closes astral-sh/ty#2791.