From 0126f74c29c5647295bebf4b42d79f12b7ffd8e4 Mon Sep 17 00:00:00 2001 From: Kar Petrosyan <92274156+karpetrosyan@users.noreply.github.com> Date: Wed, 8 Nov 2023 01:27:19 +0400 Subject: [PATCH] Add TRIO110 rule (#8537) ## Summary Adds TRIO110 from the [flake8-trio plugin](https://github.com/Zac-HD/flake8-trio). Relates to: https://github.com/astral-sh/ruff/issues/8451 --- .../test/fixtures/flake8_trio/TRIO110.py | 16 +++++ .../src/checkers/ast/analyze/statement.rs | 5 +- crates/ruff_linter/src/codes.rs | 1 + .../ruff_linter/src/rules/flake8_trio/mod.rs | 1 + .../src/rules/flake8_trio/rules/mod.rs | 2 + .../rules/flake8_trio/rules/unneeded_sleep.rs | 68 +++++++++++++++++++ ...lake8_trio__tests__TRIO110_TRIO110.py.snap | 22 ++++++ ruff.schema.json | 1 + 8 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO110.py create mode 100644 crates/ruff_linter/src/rules/flake8_trio/rules/unneeded_sleep.rs create mode 100644 crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO110_TRIO110.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO110.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO110.py new file mode 100644 index 0000000000000..b0f3abed4ab90 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO110.py @@ -0,0 +1,16 @@ +import trio + + +async def func(): + while True: + await trio.sleep(10) + + +async def func(): + while True: + await trio.sleep_until(10) + + +async def func(): + while True: + trio.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 51863307ea29d..6cf80b7f870dd 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1206,7 +1206,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { flake8_trio::rules::timeout_without_await(checker, with_stmt, items); } } - Stmt::While(ast::StmtWhile { body, orelse, .. }) => { + Stmt::While(while_stmt @ ast::StmtWhile { body, orelse, .. }) => { if checker.enabled(Rule::FunctionUsesLoopVariable) { flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Stmt(stmt)); } @@ -1216,6 +1216,9 @@ 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_trio::rules::unneeded_sleep(checker, while_stmt); + } } Stmt::For( for_stmt @ ast::StmtFor { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5f84914aaee35..36c0caf53b59b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -293,6 +293,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { // flake8-trio (Flake8Trio, "100") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioTimeoutWithoutAwait), (Flake8Trio, "105") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioSyncCall), + (Flake8Trio, "110") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioUnneededSleep), (Flake8Trio, "115") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioZeroSleepCall), // flake8-builtins diff --git a/crates/ruff_linter/src/rules/flake8_trio/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/mod.rs index f68d797581856..20724ac7e6c9c 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/mod.rs @@ -16,6 +16,7 @@ mod tests { #[test_case(Rule::TrioTimeoutWithoutAwait, Path::new("TRIO100.py"))] #[test_case(Rule::TrioSyncCall, Path::new("TRIO105.py"))] + #[test_case(Rule::TrioUnneededSleep, Path::new("TRIO110.py"))] #[test_case(Rule::TrioZeroSleepCall, Path::new("TRIO115.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs index ed3806b3f241f..267b80231f8cc 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs @@ -1,7 +1,9 @@ pub(crate) use sync_call::*; pub(crate) use timeout_without_await::*; +pub(crate) use unneeded_sleep::*; pub(crate) use zero_sleep_call::*; mod sync_call; mod timeout_without_await; +mod unneeded_sleep; mod zero_sleep_call; diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/unneeded_sleep.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/unneeded_sleep.rs new file mode 100644 index 0000000000000..d7387f5835d96 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/unneeded_sleep.rs @@ -0,0 +1,68 @@ +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; + +/// ## 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") + } +} + +/// TRIO110 +pub(crate) fn unneeded_sleep(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; + }; + + if checker + .semantic() + .resolve_call_path(func.as_ref()) + .is_some_and(|path| matches!(path.as_slice(), ["trio", "sleep" | "sleep_until"])) + { + checker + .diagnostics + .push(Diagnostic::new(TrioUnneededSleep, while_stmt.range())); + } +} diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO110_TRIO110.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO110_TRIO110.py.snap new file mode 100644 index 0000000000000..d95be0a65d321 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO110_TRIO110.py.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff_linter/src/rules/flake8_trio/mod.rs +--- +TRIO110.py:5:5: TRIO110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop + | +4 | async def func(): +5 | while True: + | _____^ +6 | | await trio.sleep(10) + | |____________________________^ TRIO110 + | + +TRIO110.py:10:5: TRIO110 Use `trio.Event` instead of awaiting `trio.sleep` in a `while` loop + | + 9 | async def func(): +10 | while True: + | _____^ +11 | | await trio.sleep_until(10) + | |__________________________________^ TRIO110 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 30f4b342889de..34cfe384aa67d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3476,6 +3476,7 @@ "TRIO100", "TRIO105", "TRIO11", + "TRIO110", "TRIO115", "TRY", "TRY0",