Skip to content

Commit

Permalink
Implement asyncio-dangling-task to track asyncio.create_task calls
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 15, 2023
1 parent 08e9d12 commit d606c9b
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,7 @@ For more, see [tryceratops](https://pypi.org/project/tryceratops/1.1.0/) on PyPI
| RUF003 | ambiguous-unicode-character-comment | Comment contains ambiguous unicode character `{confusable}` (did you mean `{representant}`?) | 🛠 |
| RUF004 | keyword-argument-before-star-argument | Keyword argument `{name}` must come after starred arguments | |
| RUF005 | unpack-instead-of-concatenating-to-collection-literal | Consider `{expr}` instead of concatenation | 🛠 |
| RUF006 | [asyncio-dangling-task](https://beta.ruff.rs/docs/rules/asyncio-dangling-task/) | Dangling task | |
| RUF100 | unused-noqa | Unused `noqa` directive | 🛠 |

<!-- End auto-generated sections. -->
Expand Down
52 changes: 52 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF006.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import asyncio


# Error
def f():
asyncio.create_task(coordinator.ws_connect()) # Error


# OK
def f():
background_tasks = set()

for i in range(10):
task = asyncio.create_task(some_coro(param=i))

# Add task to the set. This creates a strong reference.
background_tasks.add(task)

# To prevent keeping references to finished tasks forever,
# make each task remove its own reference from the set after
# completion:
task.add_done_callback(background_tasks.discard)


# OK
def f():
ctx.task = asyncio.create_task(make_request())


# OK
def f():
tasks.append(asyncio.create_task(self._populate_collection(coll, coll_info)))


# OK
def f():
asyncio.wait([asyncio.create_task(client.close()) for client in clients.values()])


# OK
def f():
tasks = [asyncio.create_task(task) for task in tasks]


# Ok (false negative)
def f():
task = asyncio.create_task(coordinator.ws_connect())


# Ok (potential false negative)
def f():
do_nothing_with_the_task(asyncio.create_task(coordinator.ws_connect()))
7 changes: 7 additions & 0 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,13 @@ where
{
flake8_simplify::rules::use_capital_environment_variables(self, value);
}
if self.settings.rules.enabled(&Rule::AsyncioDanglingTask) {
if let Some(diagnostic) = ruff::rules::asyncio_dangling_task(value, |expr| {
self.resolve_call_path(expr)
}) {
self.diagnostics.push(diagnostic);
}
}
}
_ => {}
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Ruff, "003") => Rule::AmbiguousUnicodeCharacterComment,
(Ruff, "004") => Rule::KeywordArgumentBeforeStarArgument,
(Ruff, "005") => Rule::UnpackInsteadOfConcatenatingToCollectionLiteral,
(Ruff, "006") => Rule::AsyncioDanglingTask,
(Ruff, "100") => Rule::UnusedNOQA,

// flake8-django
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ ruff_macros::register_rules!(
rules::ruff::rules::AmbiguousUnicodeCharacterComment,
rules::ruff::rules::KeywordArgumentBeforeStarArgument,
rules::ruff::rules::UnpackInsteadOfConcatenatingToCollectionLiteral,
rules::ruff::rules::AsyncioDanglingTask,
rules::ruff::rules::UnusedNOQA,
// flake8-django
rules::flake8_django::rules::NullableModelStringField,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod tests {

#[test_case(Rule::KeywordArgumentBeforeStarArgument, Path::new("RUF004.py"); "RUF004")]
#[test_case(Rule::UnpackInsteadOfConcatenatingToCollectionLiteral, Path::new("RUF005.py"); "RUF005")]
#[test_case(Rule::AsyncioDanglingTask, Path::new("RUF006.py"); "RUF006")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
78 changes: 78 additions & 0 deletions crates/ruff/src/rules/ruff/rules/asyncio_dangling_task.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use rustpython_parser::ast::{Expr, ExprKind};

use ruff_macros::{define_violation, derive_message_formats};

use crate::ast::types::{CallPath, Range};
use crate::registry::Diagnostic;
use crate::violation::Violation;

define_violation!(
/// ## What it does
/// Checks for `asyncio.create_task` calls that do not store a reference
/// to the returned result.
///
/// ## Why is this bad?
/// Per the `asyncio` documentation, the event loop only retains a weak
/// reference to tasks. If the task returned by `asyncio.create_task` is
/// not stored in a variable, or a collection, or otherwise referenced, it
/// may be garbage collected at any time. This can lead to unexpected and
/// inconsistent behavior, as your tasks may or may not run to completion.
///
/// ## Example
/// ```python
/// import asyncio
///
/// for i in range(10):
/// # This creates a weak reference to the task, which may be garbage
/// # collected at any time.
/// asyncio.create_task(some_coro(param=i))
/// ```
///
/// Use instead:
/// ```python
/// import asyncio
///
/// background_tasks = set()
///
/// for i in range(10):
/// task = asyncio.create_task(some_coro(param=i))
///
/// # Add task to the set. This creates a strong reference.
/// background_tasks.add(task)
///
/// # To prevent keeping references to finished tasks forever,
/// # make each task remove its own reference from the set after
/// # completion:
/// task.add_done_callback(background_tasks.discard)
/// ```
///
/// ## References
/// * [_The Heisenbug lurking in your async code_](https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/)
/// * [`asyncio.create_task`](https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task)
pub struct AsyncioDanglingTask;
);

impl Violation for AsyncioDanglingTask {
#[derive_message_formats]
fn message(&self) -> String {
format!("Dangling task")
}
}

/// RUF006
pub fn asyncio_dangling_task<'a, F>(expr: &'a Expr, resolve_call_path: F) -> Option<Diagnostic>
where
F: FnOnce(&'a Expr) -> Option<CallPath<'a>>,
{
if let ExprKind::Call { func, .. } = &expr.node {
if resolve_call_path(func).map_or(false, |call_path| {
call_path.as_slice() == ["asyncio", "create_task"]
}) {
return Some(Diagnostic::new(
AsyncioDanglingTask,
Range::from_located(expr),
));
}
}
None
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod ambiguous_unicode_character;
mod asyncio_dangling_task;
mod keyword_argument_before_star_argument;
mod unpack_instead_of_concatenating_to_collection_literal;
mod unused_noqa;
Expand All @@ -7,6 +8,7 @@ pub use ambiguous_unicode_character::{
ambiguous_unicode_character, AmbiguousUnicodeCharacterComment,
AmbiguousUnicodeCharacterDocstring, AmbiguousUnicodeCharacterString,
};
pub use asyncio_dangling_task::{asyncio_dangling_task, AsyncioDanglingTask};
pub use keyword_argument_before_star_argument::{
keyword_argument_before_star_argument, KeywordArgumentBeforeStarArgument,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: crates/ruff/src/rules/ruff/mod.rs
expression: diagnostics
---
- kind:
AsyncioDanglingTask: ~
location:
row: 6
column: 4
end_location:
row: 6
column: 49
fix: ~
parent: ~

1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1864,6 +1864,7 @@
"RUF003",
"RUF004",
"RUF005",
"RUF006",
"RUF1",
"RUF10",
"RUF100",
Expand Down

0 comments on commit d606c9b

Please sign in to comment.