Skip to content

Commit

Permalink
Detect unused-asyncio-dangling-task (RUF006) on unused assignments (
Browse files Browse the repository at this point in the history
#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
  • Loading branch information
asafamr-mm authored Dec 9, 2023
1 parent cb8a2f5 commit 6c2613b
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 26 deletions.
36 changes: 35 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/ruff/RUF006.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,29 @@ def f():
tasks = [asyncio.create_task(task) for task in tasks]


# OK (false negative)
# Error
def f():
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)
def f():
do_nothing_with_the_task(asyncio.create_task(coordinator.ws_connect()))
Expand All @@ -88,3 +106,19 @@ def f():
def f():
loop = asyncio.get_running_loop()
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())
10 changes: 9 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
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.
pub(crate) fn bindings(checker: &mut Checker) {
if !checker.any_enabled(&[
Rule::AsyncioDanglingTask,
Rule::InvalidAllFormat,
Rule::InvalidAllObject,
Rule::NonAsciiName,
Expand Down Expand Up @@ -71,5 +72,12 @@ pub(crate) fn bindings(checker: &mut Checker) {
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);
}
}
}
}
6 changes: 5 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pylint::rules::named_expr_without_context(checker, value);
}
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) {
refurb::rules::repeated_append(checker, stmt);
Expand Down
57 changes: 38 additions & 19 deletions crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
use std::fmt;

use ruff_python_ast::{self as ast, Expr};

use ast::Stmt;
use ruff_diagnostics::{Diagnostic, 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 crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `asyncio.create_task` and `asyncio.ensure_future` calls
/// that do not store a reference to the returned result.
Expand Down Expand Up @@ -66,35 +64,34 @@ impl Violation for AsyncioDanglingTask {
}

/// 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 {
return;
return None;
};

// Ex) `asyncio.create_task(...)`
if let Some(method) = checker
.semantic()
.resolve_call_path(func)
.and_then(|call_path| match call_path.as_slice() {
["asyncio", "create_task"] => Some(Method::CreateTask),
["asyncio", "ensure_future"] => Some(Method::EnsureFuture),
_ => None,
})
if let Some(method) =
semantic
.resolve_call_path(func)
.and_then(|call_path| match call_path.as_slice() {
["asyncio", "create_task"] => Some(Method::CreateTask),
["asyncio", "ensure_future"] => Some(Method::EnsureFuture),
_ => None,
})
{
checker.diagnostics.push(Diagnostic::new(
return Some(Diagnostic::new(
AsyncioDanglingTask { method },
expr.range(),
));
return;
}

// Ex) `loop = asyncio.get_running_loop(); loop.create_task(...)`
if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() {
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"])
}) {
checker.diagnostics.push(Diagnostic::new(
return Some(Diagnostic::new(
AsyncioDanglingTask {
method: Method::CreateTask,
},
Expand All @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,27 @@ RUF006.py:11:5: RUF006 Store a reference to the return value of `asyncio.ensure_
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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():
78 | loop = asyncio.get_running_loop()
79 | loop.create_task(coordinator.ws_connect()) # Error
66 | # Error
67 | def f():
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
|

Expand Down

0 comments on commit 6c2613b

Please sign in to comment.