diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md index 49238a4ced..db814df8f0 100644 --- a/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/unused_awaitable.md @@ -2,7 +2,10 @@ ## Basic coroutine not awaited -Calling an `async def` function produces a coroutine that must be awaited. +Calling an `async def` function produces a coroutine. If that coroutine is discarded without being +awaited, ty reports an `unused-awaitable` diagnostic. Inside an asynchronous function, this +diagnostic includes an autofix that adds `await`. The fix is marked unsafe because awaiting the +coroutine executes its body and can suspend the calling function. ```py async def fetch() -> int: @@ -12,10 +15,26 @@ async def fetch_complex(x) -> int: return 42 async def main(): - fetch() # error: [unused-awaitable] + fetch() # snapshot: unused-awaitable fetch_complex(lambda: None) # error: [unused-awaitable] ``` +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:8:5 + | +8 | fetch() # snapshot: unused-awaitable + | ^^^^^^^ +help: Did you mean to `await` this expression? + | +7 | async def main(): + - fetch() # snapshot: unused-awaitable +8 + await fetch() # snapshot: unused-awaitable +9 | fetch_complex(lambda: None) # error: [unused-awaitable] + | +note: This is an unsafe fix and may change runtime behavior +``` + ## Awaited coroutine is fine ```py @@ -52,13 +71,157 @@ async def main(): ## Top-level coroutine call -The lint fires even outside of `async def`, since the coroutine is still discarded. +The lint fires even outside of `async def`, since the coroutine is still discarded. No fix is +offered because Python modules do not allow top-level `await`. ```py async def fetch() -> int: return 42 -fetch() # error: [unused-awaitable] +fetch() # snapshot: unused-awaitable +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:4:1 + | +4 | fetch() # snapshot: unused-awaitable + | ^^^^^^^ +``` + +## Synchronous function nested in an asynchronous function + +A synchronous function cannot use `await`, even when it is defined inside an asynchronous function. +This diagnostic therefore has no fix. + +```py +async def fetch() -> int: + return 42 + +async def main(): + def inner(): + fetch() # snapshot: unused-awaitable +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:6:9 + | +6 | fetch() # snapshot: unused-awaitable + | ^^^^^^^ +``` + +## Class nested in an asynchronous function + +A class body cannot use `await`, even when the class is defined inside an asynchronous function. +This diagnostic therefore has no fix. + +```py +async def fetch() -> int: + return 42 + +async def main(): + class Inner: + fetch() # snapshot: unused-awaitable +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:6:9 + | +6 | fetch() # snapshot: unused-awaitable + | ^^^^^^^ +``` + +## Coroutine variables + +A fix is also offered when a coroutine variable is used as an expression statement. + +```py +from types import CoroutineType +from typing import Any + +async def main(value: CoroutineType[Any, Any, int]): + value # snapshot: unused-awaitable +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:5:5 + | +5 | value # snapshot: unused-awaitable + | ^^^^^ +help: Did you mean to `await` this expression? + | +4 | async def main(value: CoroutineType[Any, Any, int]): + - value # snapshot: unused-awaitable +5 + await value # snapshot: unused-awaitable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Conditional expressions + +When a conditional expression returns an unused coroutine, ty offers an autofix that awaits the +selected coroutine. The fix parenthesizes the expression so that `await` applies to either branch. +Existing parentheses and comments are preserved. + +```py +async def fetch() -> int: + return 42 + +async def main(flag: bool): + ( # Keep the expression on multiple lines. + fetch() if flag else fetch() # snapshot: unused-awaitable + ) +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:6:9 + | +6 | fetch() if flag else fetch() # snapshot: unused-awaitable + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: Did you mean to `await` this expression? + | +5 | ( # Keep the expression on multiple lines. + - fetch() if flag else fetch() # snapshot: unused-awaitable +6 + await (fetch() if flag else fetch()) # snapshot: unused-awaitable +7 | ) + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Already-awaited expressions + +Awaiting a coroutine can return another coroutine that is then discarded. In this case, ty reports +`unused-awaitable` and offers an autofix that awaits the returned coroutine. The fix parenthesizes +the existing `await` expression before adding the second `await`. + +```py +from types import CoroutineType +from typing import Any + +async def fetch() -> CoroutineType[Any, Any, int]: + raise NotImplementedError + +async def main(): + await fetch() # snapshot: unused-awaitable +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.py:8:5 + | +8 | await fetch() # snapshot: unused-awaitable + | ^^^^^^^^^^^^^ +help: Did you mean to `await` this expression? + | +7 | async def main(): + - await fetch() # snapshot: unused-awaitable +8 + await (await fetch()) # snapshot: unused-awaitable + | +note: This is an unsafe fix and may change runtime behavior ``` ## Union of awaitables @@ -153,3 +316,47 @@ def get_any() -> Any: async def main(): get_any() ``` + +## Notebook cells + +Notebook cells allow top-level `await`, so ty can offer an `await` autofix for unused coroutines at +module scope. + +```ipynb +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "async def fetch() -> int:\n", + " return 42\n", + "\n", + "fetch() # snapshot: unused-awaitable\n" + ] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 4 +} +``` + +```snapshot +warning[unused-awaitable]: Object of type `CoroutineType[Any, Any, int]` is not awaited + --> src/mdtest_snippet.ipynb:cell 1:4:1 + | +4 | fetch() # snapshot: unused-awaitable + | ^^^^^^^ +help: Did you mean to `await` this expression? + ::: cell 1 + | +3 | + - fetch() # snapshot: unused-awaitable +4 + await fetch() # snapshot: unused-awaitable +5 | + | +note: This is an unsafe fix and may change runtime behavior +``` diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index c9c72e823c..adfaf66353 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -74,8 +74,8 @@ use crate::types::diagnostic::{ INVALID_TYPE_VARIABLE_CONSTRAINTS, INVALID_TYPE_VARIABLE_DEFAULT, POSSIBLY_MISSING_IMPLICIT_CALL, POSSIBLY_MISSING_SUBMODULE, TypeCheckDiagnostics, UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL, UNRESOLVED_REFERENCE, UNSOUND_ASSIGNMENT, - UNSOUND_YIELD, UNSUPPORTED_OPERATOR, UNUSED_AWAITABLE, YieldKind, - autofix_with_notimplementederror, hint_if_stdlib_attribute_exists_on_other_versions, + UNSOUND_YIELD, UNSUPPORTED_OPERATOR, YieldKind, autofix_with_notimplementederror, + hint_if_stdlib_attribute_exists_on_other_versions, report_attempted_instantiation_of_abstract_class, report_attempted_protocol_instantiation, report_bad_dunder_delattr_call, report_bad_dunder_delete_call, report_call_to_abstract_method, report_cannot_pop_required_field_on_typed_dict, report_dynamic_function_decorator_return, @@ -162,6 +162,7 @@ use ty_python_core::{ExpressionNodeKey, Statement}; mod annotation_expression; mod attribute_assignment; +mod awaitable; mod binary_expressions; mod class; mod dict; @@ -997,23 +998,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { previous } - /// Returns `true` if `expr` is a call to a known diagnostic function - /// (e.g., `reveal_type` or `assert_type`) whose return value should not - /// trigger the `unused-awaitable` lint. - fn is_known_function_call(&self, expr: &ast::Expr) -> bool { - let ast::Expr::Call(call) = expr else { - return false; - }; - matches!( - self.expression_type(&call.func), - Type::FunctionLiteral(f) - if matches!( - f.known(self.db()), - Some(KnownFunction::RevealType | KnownFunction::AssertType) - ) - ) - } - /// Get the already-inferred type of an expression node, or Unknown. fn expression_type(&self, expr: &ast::Expr) -> Type<'db> { self.try_expression_type(expr).unwrap_or_else(Type::unknown) @@ -2160,7 +2144,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } fn infer_body(&mut self, suite: &[ast::Stmt]) { - let db = self.db(); for statement in suite { self.infer_maybe_standalone_statement(statement); @@ -2170,17 +2153,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { value, }) = statement { - let ty = self.expression_type(value); - if ty.is_awaitable(self.db()) && !self.is_known_function_call(value) { - if let Some(builder) = - self.context.report_lint(&UNUSED_AWAITABLE, value.as_ref()) - { - builder.into_diagnostic(format_args!( - "Object of type `{}` is not awaited", - ty.display(db, self.program_environment()), - )); - } - } + self.check_unused_awaitable(value); } } @@ -10016,30 +9989,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } - fn infer_await_expression( - &mut self, - await_expression: &ast::ExprAwait, - tcx: TypeContext<'db>, - ) -> Type<'db> { - let db = self.db(); - let env = self.program_environment(); - let ast::ExprAwait { - range: _, - node_index: _, - value, - } = await_expression; - - let expr_type = self.infer_expression( - value, - tcx.map(|tcx| KnownClass::Awaitable.to_specialized_instance(db, env, &[tcx])), - ); - - expr_type.try_await(db, env).unwrap_or_else(|err| { - err.report_diagnostic(&self.context, expr_type, value.as_ref().into()); - Type::unknown() - }) - } - // Perform narrowing with applicable constraints between the current scope and the enclosing scope. fn narrow_place_with_applicable_constraints( &self, diff --git a/crates/ty_python_semantic/src/types/infer/builder/awaitable.rs b/crates/ty_python_semantic/src/types/infer/builder/awaitable.rs new file mode 100644 index 0000000000..36fead9b12 --- /dev/null +++ b/crates/ty_python_semantic/src/types/infer/builder/awaitable.rs @@ -0,0 +1,159 @@ +//! Type inference for `await` expressions and diagnostics for unused awaitables. +//! +//! Shared helpers check where `await` is permitted and construct fixes that add it. + +use ruff_db::source::source_text; +use ruff_diagnostics::{Edit, Fix}; +use ruff_python_ast::{self as ast, PythonVersion}; +use ruff_text_size::Ranged; +use ty_python_core::scope::{NodeWithScopeKind, ScopeKind}; + +use super::TypeInferenceBuilder; +use crate::types::diagnostic::UNUSED_AWAITABLE; +use crate::types::function::KnownFunction; +use crate::types::{KnownClass, Type, TypeContext}; + +impl<'db> TypeInferenceBuilder<'db, '_> { + pub(super) fn infer_await_expression( + &mut self, + await_expression: &ast::ExprAwait, + tcx: TypeContext<'db>, + ) -> Type<'db> { + let db = self.db(); + let env = self.program_environment(); + let ast::ExprAwait { + range: _, + node_index: _, + value, + } = await_expression; + + let expr_type = self.infer_expression( + value, + tcx.map(|tcx| KnownClass::Awaitable.to_specialized_instance(db, env, &[tcx])), + ); + + expr_type.try_await(db, env).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, expr_type, value.as_ref().into()); + Type::unknown() + }) + } + + pub(super) fn check_unused_awaitable(&self, expression: &ast::Expr) { + let db = self.db(); + let ty = self.expression_type(expression); + if ty.is_awaitable(db) + && !self.is_known_function_call(expression) + && let Some(builder) = self.context.report_lint(&UNUSED_AWAITABLE, expression) + { + let mut diagnostic = builder.into_diagnostic(format_args!( + "Object of type `{}` is not awaited", + ty.display(db, self.program_environment()), + )); + if let Some(fix) = self.await_expression_fix(expression) { + diagnostic.help("Did you mean to `await` this expression?"); + diagnostic.set_fix(fix); + } + } + } + + /// Returns `true` if `expr` is a call to a known diagnostic function + /// (e.g., `reveal_type` or `assert_type`) whose return value should not + /// trigger the `unused-awaitable` lint. + fn is_known_function_call(&self, expr: &ast::Expr) -> bool { + let ast::Expr::Call(call) = expr else { + return false; + }; + matches!( + self.expression_type(&call.func), + Type::FunctionLiteral(f) + if matches!( + f.known(self.db()), + Some(KnownFunction::RevealType | KnownFunction::AssertType) + ) + ) + } + + /// Returns `true` if adding `await` at `expression` would produce valid Python. + /// + /// Accounts for asynchronous functions, notebook cells, annotation restrictions, enclosing + /// scopes, and the different scoping behavior of comprehensions and generator expressions. + pub(super) fn can_await_here(&self, expression: &ast::Expr) -> bool { + let Some(expression_scope) = self.index.try_expression_scope_id(expression) else { + return false; + }; + let annotation_parent_scope = self + .index + .annotation_parent_scope_id(self.module(), expression); + + let db = self.db(); + + let mut in_eager_comprehension = false; + + for (scope_id, scope) in self.index.ancestor_scopes(expression_scope) { + // The first iterable of a comprehension stays in the annotation's enclosing scope. + // Eager comprehensions also inherit the restriction, but a generator body can allow + // `await` before we reach the scope enclosing its annotation. + // Conservatively reject annotations on every Python version, even though some allow + // `await` before Python 3.14 without `from __future__ import annotations`. Avoiding + // invalid syntax matters more than offering every possible fix in this rare context. + if Some(scope_id) == annotation_parent_scope { + return false; + } + + // Before Python 3.11, awaiting in a nested list, set, or dict comprehension cannot + // implicitly make its containing comprehension or generator expression asynchronous. + if in_eager_comprehension + && scope.kind() == ScopeKind::Comprehension + && self.program_environment().python_version(db) < PythonVersion::PY311 + && !scope_id.is_async_comprehension(self.index) + { + return false; + } + + match scope.node() { + NodeWithScopeKind::Function(function) => { + return function.node(self.module()).is_async; + } + NodeWithScopeKind::Lambda(_) + | NodeWithScopeKind::Class(_) + | NodeWithScopeKind::ClassTypeParameters(_) + | NodeWithScopeKind::FunctionTypeParameters(_) + | NodeWithScopeKind::TypeAliasTypeParameters(_) + | NodeWithScopeKind::TypeAlias(_) => { + return false; + } + NodeWithScopeKind::GeneratorExpression(_) => { + return true; + } + NodeWithScopeKind::Module => { + return source_text(db, self.file()).is_notebook(); + } + NodeWithScopeKind::DictComprehension(_) + | NodeWithScopeKind::ListComprehension(_) + | NodeWithScopeKind::SetComprehension(_) => { + in_eager_comprehension = true; + } + } + } + + false + } + + /// Suggest awaiting an expression, adding parentheses if its precedence requires them. + pub(super) fn await_expression_fix(&self, expression: &ast::Expr) -> Option { + if !self.can_await_here(expression) { + return None; + } + + Some( + if expression.precedence() <= ast::OperatorPrecedence::Await { + Fix::unsafe_edits( + Edit::insertion("await (".to_string(), expression.start()), + [Edit::insertion(")".to_string(), expression.end())], + ) + } else { + Fix::unsafe_edit(Edit::insertion("await ".to_string(), expression.start())) + }, + ) + } +} diff --git a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs index d1b57ff797..e34baa33c3 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs @@ -23,7 +23,6 @@ use ty_python_core::{ definition::DefinitionKind, place::PlaceExpr, predicate::{Predicate, PredicateNode}, - scope::{NodeWithScopeKind, ScopeKind}, }; use crate::{ @@ -461,18 +460,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let mut diagnostic = builder.into_diagnostic("Condition is always truthy"); add_always_truthy_concise_message(&mut diagnostic); annotate_inferred_type(&mut diagnostic); - if test_type.try_await(db, env).is_ok() && self.can_await_here(test) { + if test_type.try_await(db, env).is_ok() + && let Some(fix) = self.await_expression_fix(test) + { diagnostic.help("Did you mean to `await` this expression?"); - - let fix = if test.precedence() <= ast::OperatorPrecedence::Await { - Fix::unsafe_edits( - Edit::insertion("await (".to_string(), test.start()), - [Edit::insertion(")".to_string(), test.end())], - ) - } else { - Fix::unsafe_edit(Edit::insertion("await ".to_string(), test.start())) - }; - diagnostic.set_fix(fix); } else if let Type::NominalInstance(instance) = test_type { let class = instance.class(db, env); @@ -800,72 +791,6 @@ impl<'db> TypeInferenceBuilder<'db, '_> { } } - /// Returns `true` if adding `await` at `expression` would produce valid Python. - /// - /// Accounts for asynchronous functions, notebook cells, annotation restrictions, enclosing - /// scopes, and the different scoping behavior of comprehensions and generator expressions. - fn can_await_here(&self, expression: &ast::Expr) -> bool { - let Some(expression_scope) = self.index.try_expression_scope_id(expression) else { - return false; - }; - let annotation_parent_scope = self - .index - .annotation_parent_scope_id(self.module(), expression); - - let db = self.db(); - - let mut in_eager_comprehension = false; - - for (scope_id, scope) in self.index.ancestor_scopes(expression_scope) { - // The first iterable of a comprehension stays in the annotation's enclosing scope. - // Eager comprehensions also inherit the restriction, but a generator body can allow - // `await` before we reach the scope enclosing its annotation. - // Conservatively reject annotations on every Python version, even though some allow - // `await` before Python 3.14 without `from __future__ import annotations`. Avoiding - // invalid syntax matters more than offering every possible fix in this rare context. - if Some(scope_id) == annotation_parent_scope { - return false; - } - - // Before Python 3.11, awaiting in a nested list, set, or dict comprehension cannot - // implicitly make its containing comprehension or generator expression asynchronous. - if in_eager_comprehension - && scope.kind() == ScopeKind::Comprehension - && self.program_environment().python_version(db) < PythonVersion::PY311 - && !scope_id.is_async_comprehension(self.index) - { - return false; - } - - match scope.node() { - NodeWithScopeKind::Function(function) => { - return function.node(self.module()).is_async; - } - NodeWithScopeKind::Lambda(_) - | NodeWithScopeKind::Class(_) - | NodeWithScopeKind::ClassTypeParameters(_) - | NodeWithScopeKind::FunctionTypeParameters(_) - | NodeWithScopeKind::TypeAliasTypeParameters(_) - | NodeWithScopeKind::TypeAlias(_) => { - return false; - } - NodeWithScopeKind::GeneratorExpression(_) => { - return true; - } - NodeWithScopeKind::Module => { - return source_text(db, self.file()).is_notebook(); - } - NodeWithScopeKind::DictComprehension(_) - | NodeWithScopeKind::ListComprehension(_) - | NodeWithScopeKind::SetComprehension(_) => { - in_eager_comprehension = true; - } - } - } - - false - } - /// Return a [`TextRange`] spanning from `branch_start` up to and including /// the offset of the first newline character after the start of `first_statement`. ///