Detect `unused-asyncio-dangling-task` (`RUF006`) on unused assignments (#9060)

## Summary

Fixes #8863 : Detect asyncio-dangling-task (RUF006) when discarding
return value

## Test Plan

added new two testcases, changed result of an old one that was made more
specific
This commit is contained in:
asafamr-mm 2023-12-09 23:10:38 +02:00 committed by GitHub
parent cb8a2f5615
commit 6c2613b44e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 107 additions and 26 deletions

View File

@ -63,11 +63,29 @@ def f():
tasks = [asyncio.create_task(task) for task in tasks] tasks = [asyncio.create_task(task) for task in tasks]
# OK (false negative) # Error
def f(): def f():
task = asyncio.create_task(coordinator.ws_connect()) task = asyncio.create_task(coordinator.ws_connect())
# Error
def f():
loop = asyncio.get_running_loop()
task: asyncio.Task = loop.create_task(coordinator.ws_connect())
# OK (potential false negative)
def f():
task = asyncio.create_task(coordinator.ws_connect())
background_tasks.add(task)
# OK
async def f():
task = asyncio.create_task(coordinator.ws_connect())
await task
# OK (potential false negative) # OK (potential false negative)
def f(): def f():
do_nothing_with_the_task(asyncio.create_task(coordinator.ws_connect())) do_nothing_with_the_task(asyncio.create_task(coordinator.ws_connect()))
@ -88,3 +106,19 @@ def f():
def f(): def f():
loop = asyncio.get_running_loop() loop = asyncio.get_running_loop()
loop.do_thing(coordinator.ws_connect()) loop.do_thing(coordinator.ws_connect())
# OK
async def f():
task = unused = asyncio.create_task(coordinator.ws_connect())
await task
# OK (false negative)
async def f():
task = unused = asyncio.create_task(coordinator.ws_connect())
# OK
async def f():
task[i] = asyncio.create_task(coordinator.ws_connect())

View File

@ -3,11 +3,12 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::codes::Rule; use crate::codes::Rule;
use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint}; use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint, ruff};
/// Run lint rules over the [`Binding`]s. /// Run lint rules over the [`Binding`]s.
pub(crate) fn bindings(checker: &mut Checker) { pub(crate) fn bindings(checker: &mut Checker) {
if !checker.any_enabled(&[ if !checker.any_enabled(&[
Rule::AsyncioDanglingTask,
Rule::InvalidAllFormat, Rule::InvalidAllFormat,
Rule::InvalidAllObject, Rule::InvalidAllObject,
Rule::NonAsciiName, Rule::NonAsciiName,
@ -71,5 +72,12 @@ pub(crate) fn bindings(checker: &mut Checker) {
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
} }
if checker.enabled(Rule::AsyncioDanglingTask) {
if let Some(diagnostic) =
ruff::rules::asyncio_dangling_binding(binding, &checker.semantic)
{
checker.diagnostics.push(diagnostic);
}
}
} }
} }

View File

@ -1571,7 +1571,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::named_expr_without_context(checker, value); pylint::rules::named_expr_without_context(checker, value);
} }
if checker.enabled(Rule::AsyncioDanglingTask) { if checker.enabled(Rule::AsyncioDanglingTask) {
ruff::rules::asyncio_dangling_task(checker, value); if let Some(diagnostic) =
ruff::rules::asyncio_dangling_task(value, checker.semantic())
{
checker.diagnostics.push(diagnostic);
}
} }
if checker.enabled(Rule::RepeatedAppend) { if checker.enabled(Rule::RepeatedAppend) {
refurb::rules::repeated_append(checker, stmt); refurb::rules::repeated_append(checker, stmt);

View File

@ -1,14 +1,12 @@
use std::fmt; use std::fmt;
use ruff_python_ast::{self as ast, Expr}; use ast::Stmt;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::analyze::typing; use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::{analyze::typing, Binding, SemanticModel};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does /// ## What it does
/// Checks for `asyncio.create_task` and `asyncio.ensure_future` calls /// Checks for `asyncio.create_task` and `asyncio.ensure_future` calls
/// that do not store a reference to the returned result. /// that do not store a reference to the returned result.
@ -66,35 +64,34 @@ impl Violation for AsyncioDanglingTask {
} }
/// RUF006 /// RUF006
pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) { pub(crate) fn asyncio_dangling_task(expr: &Expr, semantic: &SemanticModel) -> Option<Diagnostic> {
let Expr::Call(ast::ExprCall { func, .. }) = expr else { let Expr::Call(ast::ExprCall { func, .. }) = expr else {
return; return None;
}; };
// Ex) `asyncio.create_task(...)` // Ex) `asyncio.create_task(...)`
if let Some(method) = checker if let Some(method) =
.semantic() semantic
.resolve_call_path(func) .resolve_call_path(func)
.and_then(|call_path| match call_path.as_slice() { .and_then(|call_path| match call_path.as_slice() {
["asyncio", "create_task"] => Some(Method::CreateTask), ["asyncio", "create_task"] => Some(Method::CreateTask),
["asyncio", "ensure_future"] => Some(Method::EnsureFuture), ["asyncio", "ensure_future"] => Some(Method::EnsureFuture),
_ => None, _ => None,
}) })
{ {
checker.diagnostics.push(Diagnostic::new( return Some(Diagnostic::new(
AsyncioDanglingTask { method }, AsyncioDanglingTask { method },
expr.range(), expr.range(),
)); ));
return;
} }
// Ex) `loop = asyncio.get_running_loop(); loop.create_task(...)` // Ex) `loop = asyncio.get_running_loop(); loop.create_task(...)`
if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() { if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() {
if attr == "create_task" { if attr == "create_task" {
if typing::resolve_assignment(value, checker.semantic()).is_some_and(|call_path| { if typing::resolve_assignment(value, semantic).is_some_and(|call_path| {
matches!(call_path.as_slice(), ["asyncio", "get_running_loop"]) matches!(call_path.as_slice(), ["asyncio", "get_running_loop"])
}) { }) {
checker.diagnostics.push(Diagnostic::new( return Some(Diagnostic::new(
AsyncioDanglingTask { AsyncioDanglingTask {
method: Method::CreateTask, method: Method::CreateTask,
}, },
@ -103,6 +100,28 @@ pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) {
} }
} }
} }
None
}
/// RUF006
pub(crate) fn asyncio_dangling_binding(
binding: &Binding,
semantic: &SemanticModel,
) -> Option<Diagnostic> {
if binding.is_used() || !binding.kind.is_assignment() {
return None;
}
let source = binding.source?;
match semantic.statement(source) {
Stmt::Assign(ast::StmtAssign { value, targets, .. }) if targets.len() == 1 => {
asyncio_dangling_task(value, semantic)
}
Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value), ..
}) => asyncio_dangling_task(value, semantic),
_ => None,
}
} }
#[derive(Debug, PartialEq, Eq, Copy, Clone)] #[derive(Debug, PartialEq, Eq, Copy, Clone)]

View File

@ -17,11 +17,27 @@ RUF006.py:11:5: RUF006 Store a reference to the return value of `asyncio.ensure_
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006
| |
RUF006.py:79:5: RUF006 Store a reference to the return value of `asyncio.create_task` RUF006.py:68:12: RUF006 Store a reference to the return value of `asyncio.create_task`
| |
77 | def f(): 66 | # Error
78 | loop = asyncio.get_running_loop() 67 | def f():
79 | loop.create_task(coordinator.ws_connect()) # Error 68 | task = asyncio.create_task(coordinator.ws_connect())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006
|
RUF006.py:74:26: RUF006 Store a reference to the return value of `asyncio.create_task`
|
72 | def f():
73 | loop = asyncio.get_running_loop()
74 | task: asyncio.Task = loop.create_task(coordinator.ws_connect())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006
|
RUF006.py:97:5: RUF006 Store a reference to the return value of `asyncio.create_task`
|
95 | def f():
96 | loop = asyncio.get_running_loop()
97 | loop.create_task(coordinator.ws_connect()) # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006
| |