From 61b2d79839201035d1d6e6a27cbc5695bca25e03 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Sat, 4 Nov 2023 13:24:15 +0100 Subject: [PATCH 1/2] Add TrioZeroSleepCall violation --- .../test/fixtures/flake8_trio/TRIO115.py | 26 ++++ .../src/checkers/ast/analyze/expression.rs | 7 +- crates/ruff_linter/src/codes.rs | 1 + .../ruff_linter/src/rules/flake8_trio/mod.rs | 1 + .../src/rules/flake8_trio/rules/mod.rs | 2 + .../flake8_trio/rules/zero_sleep_call.rs | 109 ++++++++++++++++ ...lake8_trio__tests__TRIO115_TRIO115.py.snap | 118 ++++++++++++++++++ ruff.schema.json | 2 + 8 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py create mode 100644 crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs create mode 100644 crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py new file mode 100644 index 0000000000000..1571b1d576d36 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py @@ -0,0 +1,26 @@ +import trio +from trio import sleep + + +async def afoo(): + await trio.sleep(0) # TRIO115 + await trio.sleep(1) # Ok + await trio.sleep(0, 1) # Ok + await trio.sleep(...) # Ok + await trio.sleep() # Ok + + trio.sleep(0) # TRIO115 + foo = 0 + trio.sleep(foo) # TRIO115 + trio.sleep(1) # OK + time.sleep(0) # OK + + sleep(0) # TRIO115 + + +trio.sleep(0) # TRIO115 + + +def foo(): + trio.run(trio.sleep(0)) # TRIO115 + diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index aa000bcf5d569..be05080725652 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -15,8 +15,8 @@ use crate::rules::{ flake8_comprehensions, flake8_datetimez, flake8_debugger, flake8_django, flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_logging, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_self, - flake8_simplify, flake8_tidy_imports, flake8_use_pathlib, flynt, numpy, pandas_vet, - pep8_naming, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, + flake8_simplify, flake8_tidy_imports, flake8_trio, flake8_use_pathlib, flynt, numpy, + pandas_vet, pep8_naming, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, }; use crate::settings::types::PythonVersion; @@ -926,6 +926,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::ImplicitCwd) { refurb::rules::no_implicit_cwd(checker, call); } + if checker.enabled(Rule::TrioZeroSleepCall) { + flake8_trio::rules::zero_sleep_call(checker, call); + } } Expr::Dict( dict @ ast::ExprDict { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 9e0ceccfd7b9e..a08eede891815 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -292,6 +292,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, "115") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioZeroSleepCall), // flake8-builtins (Flake8Builtins, "001") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinVariableShadowing), diff --git a/crates/ruff_linter/src/rules/flake8_trio/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/mod.rs index cde1aae100e6f..71f12e7832019 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/mod.rs @@ -14,6 +14,7 @@ mod tests { use crate::test::test_path; #[test_case(Rule::TrioTimeoutWithoutAwait, Path::new("TRIO100.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()); let diagnostics = test_path( 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 73521d47fe224..abbd18e4b2984 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs @@ -1,3 +1,5 @@ pub(crate) use timeout_without_await::*; +pub(crate) use zero_sleep_call::*; mod timeout_without_await; +mod zero_sleep_call; diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs new file mode 100644 index 0000000000000..fa3706f0c7619 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs @@ -0,0 +1,109 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::Stmt; +use ruff_python_ast::{self as ast, Expr, ExprCall, Int}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; + +/// ## What it does +/// Checks for calls of `trio.sleep(0)` and suggests replacement with `trio.lowlevel.checkpoint()` +/// +/// ## Why is this bad? +/// Although equivalent to `trio.sleep(0)`, a call to `trio.lowlevel.checkpoint()` is more +/// suggestive of the underlying process and the author's intent. +/// +/// ## Example +/// ```python +/// async def func(): +/// await trio.sleep(0) +/// ``` +/// +/// Use instead: +/// ```python +/// async def func(): +/// await trio.lowlevel.checkpoint() +/// ``` +#[violation] +pub struct TrioZeroSleepCall; + +impl AlwaysFixableViolation for TrioZeroSleepCall { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)`") + } + + fn fix_title(&self) -> String { + format!("Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()`") + } +} + +/// TRIO115 +pub(crate) fn zero_sleep_call(checker: &mut Checker, call: &ExprCall) { + if !checker + .semantic() + .resolve_call_path(call.func.as_ref()) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["trio", "sleep"])) + { + return; + } + + if call.arguments.len() != 1 { + return; + } + + let Some(arg) = call.arguments.find_argument("seconds", 0) else { + return; + }; + + match arg { + Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => { + let Some(int) = value.as_int() else { return }; + if *int != Int::ZERO { + return; + } + } + Expr::Name(ast::ExprName { id, .. }) => { + let scope = checker.semantic().current_scope(); + if let Some(binding_id) = scope.get(id) { + let binding = checker.semantic().binding(binding_id); + if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { + if let Some(parent_id) = binding.source { + let parent = checker.semantic().statement(parent_id); + if let Stmt::Assign(ast::StmtAssign { value, .. }) + | Stmt::AnnAssign(ast::StmtAnnAssign { + value: Some(value), .. + }) + | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent + { + if let Expr::NumberLiteral(ast::ExprNumberLiteral { + value: num, .. + }) = value.as_ref() + { + let Some(int) = num.as_int() else { return }; + if *int != Int::ZERO { + return; + } + } + } + } + } + } + } + _ => return, + } + + let mut diagnostic = Diagnostic::new(TrioZeroSleepCall, call.range()); + diagnostic.try_set_fix(|| { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("trio", "lowlevel.checkpoint"), + call.func.start(), + checker.semantic(), + )?; + let reference_edit = Edit::range_replacement(binding, call.func.range()); + let arg_edit = Edit::range_deletion(call.arguments.range); + Ok(Fix::unsafe_edits(import_edit, [reference_edit, arg_edit])) + }); + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap new file mode 100644 index 0000000000000..78cc018407abc --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap @@ -0,0 +1,118 @@ +--- +source: crates/ruff_linter/src/rules/flake8_trio/mod.rs +--- +TRIO115.py:6:11: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +5 | async def afoo(): +6 | await trio.sleep(0) # TRIO115 + | ^^^^^^^^^^^^^ TRIO115 +7 | await trio.sleep(1) # Ok +8 | await trio.sleep(0, 1) # Ok + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +3 3 | +4 4 | +5 5 | async def afoo(): +6 |- await trio.sleep(0) # TRIO115 + 6 |+ await trio.lowlevel.checkpoint # TRIO115 +7 7 | await trio.sleep(1) # Ok +8 8 | await trio.sleep(0, 1) # Ok +9 9 | await trio.sleep(...) # Ok + +TRIO115.py:12:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +10 | await trio.sleep() # Ok +11 | +12 | trio.sleep(0) # TRIO115 + | ^^^^^^^^^^^^^ TRIO115 +13 | foo = 0 +14 | trio.sleep(foo) # TRIO115 + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +9 9 | await trio.sleep(...) # Ok +10 10 | await trio.sleep() # Ok +11 11 | +12 |- trio.sleep(0) # TRIO115 + 12 |+ trio.lowlevel.checkpoint # TRIO115 +13 13 | foo = 0 +14 14 | trio.sleep(foo) # TRIO115 +15 15 | trio.sleep(1) # OK + +TRIO115.py:14:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +12 | trio.sleep(0) # TRIO115 +13 | foo = 0 +14 | trio.sleep(foo) # TRIO115 + | ^^^^^^^^^^^^^^^ TRIO115 +15 | trio.sleep(1) # OK +16 | time.sleep(0) # OK + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +11 11 | +12 12 | trio.sleep(0) # TRIO115 +13 13 | foo = 0 +14 |- trio.sleep(foo) # TRIO115 + 14 |+ trio.lowlevel.checkpoint # TRIO115 +15 15 | trio.sleep(1) # OK +16 16 | time.sleep(0) # OK +17 17 | + +TRIO115.py:18:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +16 | time.sleep(0) # OK +17 | +18 | sleep(0) # TRIO115 + | ^^^^^^^^ TRIO115 + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +15 15 | trio.sleep(1) # OK +16 16 | time.sleep(0) # OK +17 17 | +18 |- sleep(0) # TRIO115 + 18 |+ trio.lowlevel.checkpoint # TRIO115 +19 19 | +20 20 | +21 21 | trio.sleep(0) # TRIO115 + +TRIO115.py:21:1: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +21 | trio.sleep(0) # TRIO115 + | ^^^^^^^^^^^^^ TRIO115 + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +18 18 | sleep(0) # TRIO115 +19 19 | +20 20 | +21 |-trio.sleep(0) # TRIO115 + 21 |+trio.lowlevel.checkpoint # TRIO115 +22 22 | +23 23 | +24 24 | def foo(): + +TRIO115.py:25:14: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` + | +24 | def foo(): +25 | trio.run(trio.sleep(0)) # TRIO115 + | ^^^^^^^^^^^^^ TRIO115 + | + = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + +ℹ Suggested fix +22 22 | +23 23 | +24 24 | def foo(): +25 |- trio.run(trio.sleep(0)) # TRIO115 + 25 |+ trio.run(trio.lowlevel.checkpoint) # TRIO115 +26 26 | + + diff --git a/ruff.schema.json b/ruff.schema.json index 1886b55e838d2..64949d6489183 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3472,6 +3472,8 @@ "TRIO1", "TRIO10", "TRIO100", + "TRIO11", + "TRIO115", "TRY", "TRY0", "TRY00", From 3380e00190c5aff357148dc97a60ef101ef3e1a1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 5 Nov 2023 20:12:23 -0500 Subject: [PATCH 2/2] Minor tweaks --- .../test/fixtures/flake8_trio/TRIO115.py | 13 +++-- .../flake8_trio/rules/zero_sleep_call.rs | 10 ++-- ...lake8_trio__tests__TRIO115_TRIO115.py.snap | 51 +++++++++---------- 3 files changed, 36 insertions(+), 38 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py index 1571b1d576d36..117c2270f2940 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py @@ -2,12 +2,12 @@ from trio import sleep -async def afoo(): +async def func(): await trio.sleep(0) # TRIO115 - await trio.sleep(1) # Ok - await trio.sleep(0, 1) # Ok - await trio.sleep(...) # Ok - await trio.sleep() # Ok + await trio.sleep(1) # OK + await trio.sleep(0, 1) # OK + await trio.sleep(...) # OK + await trio.sleep() # OK trio.sleep(0) # TRIO115 foo = 0 @@ -21,6 +21,5 @@ async def afoo(): trio.sleep(0) # TRIO115 -def foo(): +def func(): trio.run(trio.sleep(0)) # TRIO115 - diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs index fa3706f0c7619..efb85e2120306 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs @@ -8,11 +8,11 @@ use crate::checkers::ast::Checker; use crate::importer::ImportRequest; /// ## What it does -/// Checks for calls of `trio.sleep(0)` and suggests replacement with `trio.lowlevel.checkpoint()` +/// Checks for uses of `trio.sleep(0)`. /// /// ## Why is this bad? -/// Although equivalent to `trio.sleep(0)`, a call to `trio.lowlevel.checkpoint()` is more -/// suggestive of the underlying process and the author's intent. +/// `trio.sleep(0)` is equivalent to calling `trio.lowlevel.checkpoint()`. +/// However, the latter better conveys the intent of the code. /// /// ## Example /// ```python @@ -35,7 +35,7 @@ impl AlwaysFixableViolation for TrioZeroSleepCall { } fn fix_title(&self) -> String { - format!("Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()`") + format!("Replace with `trio.lowlevel.checkpoint()`") } } @@ -103,7 +103,7 @@ pub(crate) fn zero_sleep_call(checker: &mut Checker, call: &ExprCall) { )?; let reference_edit = Edit::range_replacement(binding, call.func.range()); let arg_edit = Edit::range_deletion(call.arguments.range); - Ok(Fix::unsafe_edits(import_edit, [reference_edit, arg_edit])) + Ok(Fix::safe_edits(import_edit, [reference_edit, arg_edit])) }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap index 78cc018407abc..b273e0333b8e4 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO115_TRIO115.py.snap @@ -3,38 +3,38 @@ source: crates/ruff_linter/src/rules/flake8_trio/mod.rs --- TRIO115.py:6:11: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` | -5 | async def afoo(): +5 | async def func(): 6 | await trio.sleep(0) # TRIO115 | ^^^^^^^^^^^^^ TRIO115 -7 | await trio.sleep(1) # Ok -8 | await trio.sleep(0, 1) # Ok +7 | await trio.sleep(1) # OK +8 | await trio.sleep(0, 1) # OK | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix +ℹ Fix 3 3 | 4 4 | -5 5 | async def afoo(): +5 5 | async def func(): 6 |- await trio.sleep(0) # TRIO115 6 |+ await trio.lowlevel.checkpoint # TRIO115 -7 7 | await trio.sleep(1) # Ok -8 8 | await trio.sleep(0, 1) # Ok -9 9 | await trio.sleep(...) # Ok +7 7 | await trio.sleep(1) # OK +8 8 | await trio.sleep(0, 1) # OK +9 9 | await trio.sleep(...) # OK TRIO115.py:12:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` | -10 | await trio.sleep() # Ok +10 | await trio.sleep() # OK 11 | 12 | trio.sleep(0) # TRIO115 | ^^^^^^^^^^^^^ TRIO115 13 | foo = 0 14 | trio.sleep(foo) # TRIO115 | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix -9 9 | await trio.sleep(...) # Ok -10 10 | await trio.sleep() # Ok +ℹ Fix +9 9 | await trio.sleep(...) # OK +10 10 | await trio.sleep() # OK 11 11 | 12 |- trio.sleep(0) # TRIO115 12 |+ trio.lowlevel.checkpoint # TRIO115 @@ -51,9 +51,9 @@ TRIO115.py:14:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 15 | trio.sleep(1) # OK 16 | time.sleep(0) # OK | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix +ℹ Fix 11 11 | 12 12 | trio.sleep(0) # TRIO115 13 13 | foo = 0 @@ -70,9 +70,9 @@ TRIO115.py:18:5: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 18 | sleep(0) # TRIO115 | ^^^^^^^^ TRIO115 | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix +ℹ Fix 15 15 | trio.sleep(1) # OK 16 16 | time.sleep(0) # OK 17 17 | @@ -87,9 +87,9 @@ TRIO115.py:21:1: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 21 | trio.sleep(0) # TRIO115 | ^^^^^^^^^^^^^ TRIO115 | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix +ℹ Fix 18 18 | sleep(0) # TRIO115 19 19 | 20 20 | @@ -97,22 +97,21 @@ TRIO115.py:21:1: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.s 21 |+trio.lowlevel.checkpoint # TRIO115 22 22 | 23 23 | -24 24 | def foo(): +24 24 | def func(): TRIO115.py:25:14: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)` | -24 | def foo(): +24 | def func(): 25 | trio.run(trio.sleep(0)) # TRIO115 | ^^^^^^^^^^^^^ TRIO115 | - = help: Replace `trio.sleep(0)` with `trio.lowlevel.checkpoint()` + = help: Replace with `trio.lowlevel.checkpoint()` -ℹ Suggested fix +ℹ Fix 22 22 | 23 23 | -24 24 | def foo(): +24 24 | def func(): 25 |- trio.run(trio.sleep(0)) # TRIO115 25 |+ trio.run(trio.lowlevel.checkpoint) # TRIO115 -26 26 |