Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TRIO] Add TRIO115: TrioZeroSleepCall #8486

Merged
merged 3 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO115.py
Original file line number Diff line number Diff line change
@@ -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

7 changes: 5 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_trio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub(crate) use timeout_without_await::*;
pub(crate) use zero_sleep_call::*;

mod timeout_without_await;
mod zero_sleep_call;
109 changes: 109 additions & 0 deletions crates/ruff_linter/src/rules/flake8_trio/rules/zero_sleep_call.rs
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interpreted from upstream plugin, couldn't find additional info in docs or PRs on the reason. All input is welcome

/// 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();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a more compact way to do this yet. In case there is one somewhere in the repo already, happy to hear about it and refactor this.

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);
}
Original file line number Diff line number Diff line change
@@ -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 |


2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading