From 21c671be422e71204b6372f2009b6d3c9c9a5ba2 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Tue, 9 Jul 2024 18:27:41 -0400 Subject: [PATCH] update async110 to match upstream --- .../test/fixtures/flake8_async/ASYNC110.py | 32 +++++++ .../src/checkers/ast/analyze/statement.rs | 4 +- crates/ruff_linter/src/codes.rs | 2 +- .../src/rules/flake8_async/helpers.rs | 21 ++++ .../ruff_linter/src/rules/flake8_async/mod.rs | 3 +- .../flake8_async/rules/async_busy_wait.rs | 95 +++++++++++++++++++ .../src/rules/flake8_async/rules/mod.rs | 4 +- .../flake8_async/rules/unneeded_sleep.rs | 73 -------------- ...e8_async__tests__ASYNC110_ASYNC110.py.snap | 16 ++-- ..._tests__preview__ASYNC110_ASYNC110.py.snap | 47 +++++++++ 10 files changed, 210 insertions(+), 87 deletions(-) create mode 100644 crates/ruff_linter/src/rules/flake8_async/rules/async_busy_wait.rs delete mode 100644 crates/ruff_linter/src/rules/flake8_async/rules/unneeded_sleep.rs create mode 100644 crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC110_ASYNC110.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC110.py b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC110.py index b0f3abed4ab90..d09464b33eea3 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC110.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC110.py @@ -1,4 +1,6 @@ import trio +import anyio +import asyncio async def func(): @@ -14,3 +16,33 @@ async def func(): async def func(): while True: trio.sleep(10) + + +async def func(): + while True: + await anyio.sleep(10) + + +async def func(): + while True: + await anyio.sleep_until(10) + + +async def func(): + while True: + anyio.sleep(10) + + +async def func(): + while True: + await asyncio.sleep(10) + + +async def func(): + while True: + await asyncio.sleep_until(10) + + +async def func(): + while True: + asyncio.sleep(10) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index df40da16ad61f..be8ca358b8075 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1330,8 +1330,8 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::TryExceptInLoop) { perflint::rules::try_except_in_loop(checker, body); } - if checker.enabled(Rule::TrioUnneededSleep) { - flake8_async::rules::unneeded_sleep(checker, while_stmt); + if checker.enabled(Rule::AsyncBusyWait) { + flake8_async::rules::async_busy_wait(checker, while_stmt); } } Stmt::For( diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 1060ccaeba908..45e92b40e370a 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -296,7 +296,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Async, "100") => (RuleGroup::Stable, rules::flake8_async::rules::CancelScopeNoCheckpoint), (Flake8Async, "105") => (RuleGroup::Stable, rules::flake8_async::rules::TrioSyncCall), (Flake8Async, "109") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncFunctionWithTimeout), - (Flake8Async, "110") => (RuleGroup::Stable, rules::flake8_async::rules::TrioUnneededSleep), + (Flake8Async, "110") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncBusyWait), (Flake8Async, "115") => (RuleGroup::Stable, rules::flake8_async::rules::TrioZeroSleepCall), (Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::SleepForeverCall), (Flake8Async, "210") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingHttpCallInAsyncFunction), diff --git a/crates/ruff_linter/src/rules/flake8_async/helpers.rs b/crates/ruff_linter/src/rules/flake8_async/helpers.rs index b726d8fda8b84..99c7b2444021a 100644 --- a/crates/ruff_linter/src/rules/flake8_async/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_async/helpers.rs @@ -10,6 +10,27 @@ pub(super) enum AsyncModule { Trio, } +impl AsyncModule { + pub(super) fn try_from(qualified_name: &QualifiedName<'_>) -> Option { + match qualified_name.segments() { + ["asyncio", ..] => Some(Self::AsyncIo), + ["anyio", ..] => Some(Self::AnyIo), + ["trio", ..] => Some(Self::Trio), + _ => None, + } + } +} + +impl std::fmt::Display for AsyncModule { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AsyncModule::AnyIo => write!(f, "asyncio"), + AsyncModule::AsyncIo => write!(f, "anyio"), + AsyncModule::Trio => write!(f, "trio"), + } + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(super) enum MethodName { AsyncIoTimeout, diff --git a/crates/ruff_linter/src/rules/flake8_async/mod.rs b/crates/ruff_linter/src/rules/flake8_async/mod.rs index 2cff08d959414..bb74a7764a799 100644 --- a/crates/ruff_linter/src/rules/flake8_async/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_async/mod.rs @@ -19,7 +19,7 @@ mod tests { #[test_case(Rule::TrioSyncCall, Path::new("ASYNC105.py"))] #[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_0.py"))] #[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_1.py"))] - #[test_case(Rule::TrioUnneededSleep, Path::new("ASYNC110.py"))] + #[test_case(Rule::AsyncBusyWait, Path::new("ASYNC110.py"))] #[test_case(Rule::TrioZeroSleepCall, Path::new("ASYNC115.py"))] #[test_case(Rule::SleepForeverCall, Path::new("ASYNC116.py"))] #[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC210.py"))] @@ -41,6 +41,7 @@ mod tests { #[test_case(Rule::CancelScopeNoCheckpoint, Path::new("ASYNC100.py"))] #[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_0.py"))] #[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_1.py"))] + #[test_case(Rule::AsyncBusyWait, Path::new("ASYNC110.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_busy_wait.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_busy_wait.rs new file mode 100644 index 0000000000000..0254c23868c0c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_busy_wait.rs @@ -0,0 +1,95 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::rules::flake8_async::helpers::AsyncModule; +use crate::settings::types::PreviewMode; + +/// ## What it does +/// Checks for the use of an async sleep function in a `while` loop. +/// +/// ## Why is this bad? +/// Instead of sleeping in a `while` loop, and waiting for a condition +/// to become true, it's preferable to `await` on an `Event` object such +/// as: `asyncio.Event`, `trio.Event`, or `anyio.Event`. +/// +/// ## Example +/// ```python +/// DONE = False +/// +/// +/// async def func(): +/// while not DONE: +/// await asyncio.sleep(1) +/// ``` +/// +/// Use instead: +/// ```python +/// DONE = asyncio.Event() +/// +/// +/// async def func(): +/// await DONE.wait() +/// ``` +/// +/// [`asyncio` events]: https://docs.python.org/3/library/asyncio-sync.html#asyncio.Event +/// [`anyio` events]: https://trio.readthedocs.io/en/latest/reference-core.html#trio.Event +/// [`trio` events]: https://anyio.readthedocs.io/en/latest/api.html#anyio.Event +#[violation] +pub struct AsyncBusyWait { + module: AsyncModule, +} + +impl Violation for AsyncBusyWait { + #[derive_message_formats] + fn message(&self) -> String { + let Self { module } = self; + format!("Use `{module}.Event` instead of awaiting `{module}.sleep` in a `while` loop") + } +} + +/// ASYNC110 +pub(crate) fn async_busy_wait(checker: &mut Checker, while_stmt: &ast::StmtWhile) { + // The body should be a single `await` call. + let [stmt] = while_stmt.body.as_slice() else { + return; + }; + let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { + return; + }; + let Expr::Await(ast::ExprAwait { value, .. }) = value.as_ref() else { + return; + }; + let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else { + return; + }; + + let Some(qualified_name) = checker.semantic().resolve_qualified_name(func.as_ref()) else { + return; + }; + + if matches!(checker.settings.preview, PreviewMode::Disabled) { + if matches!(qualified_name.segments(), ["trio", "sleep" | "sleep_until"]) { + checker.diagnostics.push(Diagnostic::new( + AsyncBusyWait { + module: AsyncModule::Trio, + }, + while_stmt.range(), + )); + } + } else { + if matches!( + qualified_name.segments(), + ["trio" | "anyio", "sleep" | "sleep_until"] | ["asyncio", "sleep"] + ) { + checker.diagnostics.push(Diagnostic::new( + AsyncBusyWait { + module: AsyncModule::try_from(&qualified_name).unwrap(), + }, + while_stmt.range(), + )); + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs index 54ab5c2dc0541..f3af8a8dc1ce3 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/mod.rs @@ -1,3 +1,4 @@ +pub(crate) use async_busy_wait::*; pub(crate) use async_function_with_timeout::*; pub(crate) use blocking_http_call::*; pub(crate) use blocking_open_call::*; @@ -6,9 +7,9 @@ pub(crate) use blocking_sleep::*; pub(crate) use cancel_scope_no_checkpoint::*; pub(crate) use sleep_forever_call::*; pub(crate) use sync_call::*; -pub(crate) use unneeded_sleep::*; pub(crate) use zero_sleep_call::*; +mod async_busy_wait; mod async_function_with_timeout; mod blocking_http_call; mod blocking_open_call; @@ -17,5 +18,4 @@ mod blocking_sleep; mod cancel_scope_no_checkpoint; mod sleep_forever_call; mod sync_call; -mod unneeded_sleep; mod zero_sleep_call; diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/unneeded_sleep.rs b/crates/ruff_linter/src/rules/flake8_async/rules/unneeded_sleep.rs deleted file mode 100644 index aded4e23d1a75..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_async/rules/unneeded_sleep.rs +++ /dev/null @@ -1,73 +0,0 @@ -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr, Stmt}; -use ruff_python_semantic::Modules; -use ruff_text_size::Ranged; - -use crate::checkers::ast::Checker; - -/// ## What it does -/// Checks for the use of `trio.sleep` in a `while` loop. -/// -/// ## Why is this bad? -/// Instead of sleeping in a `while` loop, and waiting for a condition -/// to become true, it's preferable to `wait()` on a `trio.Event`. -/// -/// ## Example -/// ```python -/// DONE = False -/// -/// -/// async def func(): -/// while not DONE: -/// await trio.sleep(1) -/// ``` -/// -/// Use instead: -/// ```python -/// DONE = trio.Event() -/// -/// -/// async def func(): -/// await DONE.wait() -/// ``` -#[violation] -pub struct TrioUnneededSleep; - -impl Violation for TrioUnneededSleep { - #[derive_message_formats] - fn message(&self) -> String { - format!("Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop") - } -} - -/// ASYNC110 -pub(crate) fn unneeded_sleep(checker: &mut Checker, while_stmt: &ast::StmtWhile) { - if !checker.semantic().seen_module(Modules::TRIO) { - return; - } - - // The body should be a single `await` call. - let [stmt] = while_stmt.body.as_slice() else { - return; - }; - let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { - return; - }; - let Expr::Await(ast::ExprAwait { value, .. }) = value.as_ref() else { - return; - }; - let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() else { - return; - }; - - if checker - .semantic() - .resolve_qualified_name(func.as_ref()) - .is_some_and(|path| matches!(path.segments(), ["trio", "sleep" | "sleep_until"])) - { - checker - .diagnostics - .push(Diagnostic::new(TrioUnneededSleep, while_stmt.range())); - } -} diff --git a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC110_ASYNC110.py.snap b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC110_ASYNC110.py.snap index fe99c8f822450..e1f8905dd9c37 100644 --- a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC110_ASYNC110.py.snap +++ b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC110_ASYNC110.py.snap @@ -1,20 +1,20 @@ --- source: crates/ruff_linter/src/rules/flake8_async/mod.rs --- -ASYNC110.py:5:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop +ASYNC110.py:7:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop | -4 | async def func(): -5 | while True: +6 | async def func(): +7 | while True: | _____^ -6 | | await trio.sleep(10) +8 | | await trio.sleep(10) | |____________________________^ ASYNC110 | -ASYNC110.py:10:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop +ASYNC110.py:12:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop | - 9 | async def func(): -10 | while True: +11 | async def func(): +12 | while True: | _____^ -11 | | await trio.sleep_until(10) +13 | | await trio.sleep_until(10) | |__________________________________^ ASYNC110 | diff --git a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC110_ASYNC110.py.snap b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC110_ASYNC110.py.snap new file mode 100644 index 0000000000000..c878faddf086a --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC110_ASYNC110.py.snap @@ -0,0 +1,47 @@ +--- +source: crates/ruff_linter/src/rules/flake8_async/mod.rs +--- +ASYNC110.py:7:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop + | +6 | async def func(): +7 | while True: + | _____^ +8 | | await trio.sleep(10) + | |____________________________^ ASYNC110 + | + +ASYNC110.py:12:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop + | +11 | async def func(): +12 | while True: + | _____^ +13 | | await trio.sleep_until(10) + | |__________________________________^ ASYNC110 + | + +ASYNC110.py:22:5: ASYNC110 Use `asyncio.Event` instead of awaiting `asyncio.sleep` in a `while` loop + | +21 | async def func(): +22 | while True: + | _____^ +23 | | await anyio.sleep(10) + | |_____________________________^ ASYNC110 + | + +ASYNC110.py:27:5: ASYNC110 Use `asyncio.Event` instead of awaiting `asyncio.sleep` in a `while` loop + | +26 | async def func(): +27 | while True: + | _____^ +28 | | await anyio.sleep_until(10) + | |___________________________________^ ASYNC110 + | + +ASYNC110.py:37:5: ASYNC110 Use `anyio.Event` instead of awaiting `anyio.sleep` in a `while` loop + | +36 | async def func(): +37 | while True: + | _____^ +38 | | await asyncio.sleep(10) + | |_______________________________^ ASYNC110 + |