Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 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,9 +63,21 @@ def f():
tasks = [asyncio.create_task(task) for task in tasks]


# OK (false negative)
# Error
def f():
task = asyncio.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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::rules::{flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint};
use crate::rules::{
flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint, ruff,
};

/// Run lint rules over all deferred scopes in the [`SemanticModel`].
pub(crate) fn deferred_scopes(checker: &mut Checker) {
Expand All @@ -31,6 +33,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
Rule::UnusedPrivateTypedDict,
Rule::UnusedStaticMethodArgument,
Rule::UnusedVariable,
Rule::AsyncioDanglingTask,
Rule::NoSelfUse,
]) {
return;
Expand Down Expand Up @@ -294,6 +297,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
}
}

if checker.enabled(Rule::AsyncioDanglingTask) {
ruff::rules::asyncio_dangling_task_unused(checker, scope, &mut diagnostics);
}

if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) {
if enforce_typing_imports {
let runtime_imports: Vec<&Binding> = checker
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,9 @@ 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);
let mut diagnostics: Vec<Diagnostic> = vec![];
ruff::rules::asyncio_dangling_task(checker, value, &mut diagnostics);
checker.diagnostics.extend(diagnostics);
asafamr-mm marked this conversation as resolved.
Show resolved Hide resolved
}
if checker.enabled(Rule::RepeatedAppend) {
refurb::rules::repeated_append(checker, stmt);
Expand Down
34 changes: 30 additions & 4 deletions crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::fmt;

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

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::{analyze::typing, Scope};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -66,7 +67,11 @@ impl Violation for AsyncioDanglingTask {
}

/// RUF006
pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) {
pub(crate) fn asyncio_dangling_task(
checker: &Checker,
expr: &Expr,
diagnostics: &mut Vec<Diagnostic>,
) {
let Expr::Call(ast::ExprCall { func, .. }) = expr else {
return;
};
Expand All @@ -81,7 +86,7 @@ pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) {
_ => None,
})
{
checker.diagnostics.push(Diagnostic::new(
diagnostics.push(Diagnostic::new(
AsyncioDanglingTask { method },
expr.range(),
));
Expand All @@ -94,7 +99,7 @@ pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) {
if typing::resolve_assignment(value, checker.semantic()).is_some_and(|call_path| {
matches!(call_path.as_slice(), ["asyncio", "get_running_loop"])
}) {
checker.diagnostics.push(Diagnostic::new(
diagnostics.push(Diagnostic::new(
AsyncioDanglingTask {
method: Method::CreateTask,
},
Expand All @@ -105,6 +110,27 @@ pub(crate) fn asyncio_dangling_task(checker: &mut Checker, expr: &Expr) {
}
}

pub(crate) fn asyncio_dangling_task_unused(
checker: &Checker,
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
for binding in scope
.binding_ids()
.map(|binding_id| checker.semantic().binding(binding_id))
.filter(|binding| binding.kind.is_assignment() && !binding.is_used())
{
let Some(source) = binding.source else {
continue;
};
let Stmt::Assign(ast::StmtAssign { value, .. }) = checker.semantic().statement(source)
else {
continue;
};
asyncio_dangling_task(checker, value, diagnostics);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job figuring this out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try using typing::analyze::resolve_assignment here instead? It will also handle AnnAssign.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure how to do it without some refactoring.
I currently use assignment value expression which (if I'm not mistaken) is discarded after processing in resolve_assignment?

For now I've extended the pattern match to include AnnAssign as done in unnecessary_list_cast - but would gladly change it

}
}

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum Method {
CreateTask,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,19 @@ 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:91:5: RUF006 Store a reference to the return value of `asyncio.create_task`
|
89 | def f():
90 | loop = asyncio.get_running_loop()
91 | loop.create_task(coordinator.ws_connect()) # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF006
|

Expand Down
Loading