Skip to content

Commit cbe3bf9

Browse files
charliermarshcharliermarsh
authored
Avoid asyncio-dangling-task violations on shadowed bindings (#9215)
## Summary Ensures that we avoid flagging cases like: ```python async def f(x: int): if x > 0: task = asyncio.create_task(make_request()) else: task = asyncio.create_task(make_request()) await task ``` Closes #9133.
1 parent 4b4160e commit cbe3bf9

4 files changed

Lines changed: 82 additions & 25 deletions

File tree

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF006.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,33 @@ async def f():
122122
# OK
123123
async def f():
124124
task[i] = asyncio.create_task(coordinator.ws_connect())
125+
126+
127+
# OK
128+
async def f(x: int):
129+
if x > 0:
130+
task = asyncio.create_task(make_request())
131+
else:
132+
task = asyncio.create_task(make_request())
133+
await task
134+
135+
136+
# OK
137+
async def f(x: bool):
138+
if x:
139+
t = asyncio.create_task(asyncio.sleep(1))
140+
else:
141+
t = None
142+
try:
143+
await asyncio.sleep(1)
144+
finally:
145+
if t:
146+
await t
147+
148+
149+
# Error
150+
async def f(x: bool):
151+
if x:
152+
t = asyncio.create_task(asyncio.sleep(1))
153+
else:
154+
t = None

‎crates/ruff_linter/src/checkers/ast/analyze/bindings.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ use ruff_text_size::Ranged;
33

44
use crate::checkers::ast::Checker;
55
use crate::codes::Rule;
6-
use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint, ruff};
6+
use crate::rules::{flake8_import_conventions, flake8_pyi, pyflakes, pylint};
77

88
/// Run lint rules over the [`Binding`]s.
99
pub(crate) fn bindings(checker: &mut Checker) {
1010
if !checker.any_enabled(&[
11-
Rule::AsyncioDanglingTask,
1211
Rule::InvalidAllFormat,
1312
Rule::InvalidAllObject,
1413
Rule::NonAsciiName,
@@ -72,12 +71,5 @@ pub(crate) fn bindings(checker: &mut Checker) {
7271
checker.diagnostics.push(diagnostic);
7372
}
7473
}
75-
if checker.enabled(Rule::AsyncioDanglingTask) {
76-
if let Some(diagnostic) =
77-
ruff::rules::asyncio_dangling_binding(binding, &checker.semantic)
78-
{
79-
checker.diagnostics.push(diagnostic);
80-
}
81-
}
8274
}
8375
}

‎crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@ use ruff_text_size::Ranged;
55

66
use crate::checkers::ast::Checker;
77
use crate::codes::Rule;
8-
use crate::rules::{flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint};
8+
use crate::rules::{
9+
flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint, ruff,
10+
};
911

1012
/// Run lint rules over all deferred scopes in the [`SemanticModel`].
1113
pub(crate) fn deferred_scopes(checker: &mut Checker) {
1214
if !checker.any_enabled(&[
15+
Rule::AsyncioDanglingTask,
1316
Rule::GlobalVariableNotAssigned,
1417
Rule::ImportShadowedByLoopVar,
18+
Rule::NoSelfUse,
1519
Rule::RedefinedArgumentFromLocal,
1620
Rule::RedefinedWhileUnused,
1721
Rule::RuntimeImportInTypeCheckingBlock,
@@ -32,7 +36,6 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
3236
Rule::UnusedPrivateTypedDict,
3337
Rule::UnusedStaticMethodArgument,
3438
Rule::UnusedVariable,
35-
Rule::NoSelfUse,
3639
]) {
3740
return;
3841
}
@@ -270,6 +273,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
270273
flake8_pyi::rules::unused_private_typed_dict(checker, scope, &mut diagnostics);
271274
}
272275

276+
if checker.enabled(Rule::AsyncioDanglingTask) {
277+
ruff::rules::asyncio_dangling_binding(scope, &checker.semantic, &mut diagnostics);
278+
}
279+
273280
if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Lambda(_)) {
274281
if checker.enabled(Rule::UnusedVariable) {
275282
pyflakes::rules::unused_variable(checker, scope, &mut diagnostics);

‎crates/ruff_linter/src/rules/ruff/rules/asyncio_dangling_task.rs‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ast::Stmt;
44
use ruff_diagnostics::{Diagnostic, Violation};
55
use ruff_macros::{derive_message_formats, violation};
66
use ruff_python_ast::{self as ast, Expr};
7-
use ruff_python_semantic::{analyze::typing, Binding, SemanticModel};
7+
use ruff_python_semantic::{analyze::typing, Scope, SemanticModel};
88
use ruff_text_size::Ranged;
99

1010
/// ## What it does
@@ -105,22 +105,50 @@ pub(crate) fn asyncio_dangling_task(expr: &Expr, semantic: &SemanticModel) -> Op
105105

106106
/// RUF006
107107
pub(crate) fn asyncio_dangling_binding(
108-
binding: &Binding,
108+
scope: &Scope,
109109
semantic: &SemanticModel,
110-
) -> Option<Diagnostic> {
111-
if binding.is_used() || !binding.kind.is_assignment() {
112-
return None;
113-
}
110+
diagnostics: &mut Vec<Diagnostic>,
111+
) {
112+
for binding_id in scope.binding_ids() {
113+
// If the binding itself is used, or it's not an assignment, skip it.
114+
let binding = semantic.binding(binding_id);
115+
if binding.is_used() || !binding.kind.is_assignment() {
116+
continue;
117+
}
118+
119+
// Otherwise, any dangling tasks, including those that are shadowed, as in:
120+
// ```python
121+
// if x > 0:
122+
// task = asyncio.create_task(make_request())
123+
// else:
124+
// task = asyncio.create_task(make_request())
125+
// ```
126+
for binding_id in
127+
std::iter::successors(Some(binding_id), |id| semantic.shadowed_binding(*id))
128+
{
129+
let binding = semantic.binding(binding_id);
130+
if binding.is_used() || !binding.kind.is_assignment() {
131+
continue;
132+
}
114133

115-
let source = binding.source?;
116-
match semantic.statement(source) {
117-
Stmt::Assign(ast::StmtAssign { value, targets, .. }) if targets.len() == 1 => {
118-
asyncio_dangling_task(value, semantic)
134+
let Some(source) = binding.source else {
135+
continue;
136+
};
137+
138+
let diagnostic = match semantic.statement(source) {
139+
Stmt::Assign(ast::StmtAssign { value, targets, .. }) if targets.len() == 1 => {
140+
asyncio_dangling_task(value, semantic)
141+
}
142+
Stmt::AnnAssign(ast::StmtAnnAssign {
143+
value: Some(value), ..
144+
}) => asyncio_dangling_task(value, semantic),
145+
_ => None,
146+
};
147+
148+
if let Some(diagnostic) = diagnostic {
149+
diagnostics.push(diagnostic);
150+
}
119151
}
120-
Stmt::AnnAssign(ast::StmtAnnAssign {
121-
value: Some(value), ..
122-
}) => asyncio_dangling_task(value, semantic),
123-
_ => None,
124152
}
125153
}
126154

0 commit comments

Comments
 (0)