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 all commits
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
25 changes: 25 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,25 @@
import trio
from trio import sleep


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

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 func():
trio.run(trio.sleep(0)) # TRIO115
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::TrioSyncCall) {
flake8_trio::rules::sync_call(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 @@ -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, "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 @@ -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::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,5 +1,7 @@
pub(crate) use sync_call::*;
pub(crate) use timeout_without_await::*;
pub(crate) use zero_sleep_call::*;

mod sync_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 uses of `trio.sleep(0)`.
///
/// ## Why is this bad?
/// `trio.sleep(0)` is equivalent to calling `trio.lowlevel.checkpoint()`.
/// However, the latter better conveys the intent of the code.
///
/// ## 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 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::safe_edits(import_edit, [reference_edit, arg_edit]))
});
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
---
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 func():
6 | await trio.sleep(0) # TRIO115
| ^^^^^^^^^^^^^ TRIO115
7 | await trio.sleep(1) # OK
8 | await trio.sleep(0, 1) # OK
|
= help: Replace with `trio.lowlevel.checkpoint()`

ℹ Fix
3 3 |
4 4 |
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

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 with `trio.lowlevel.checkpoint()`

ℹ 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 with `trio.lowlevel.checkpoint()`

ℹ 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 with `trio.lowlevel.checkpoint()`

ℹ 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 with `trio.lowlevel.checkpoint()`

ℹ 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 func():

TRIO115.py:25:14: TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)`
|
24 | def func():
25 | trio.run(trio.sleep(0)) # TRIO115
| ^^^^^^^^^^^^^ TRIO115
|
= help: Replace with `trio.lowlevel.checkpoint()`

ℹ Fix
22 22 |
23 23 |
24 24 | def func():
25 |- trio.run(trio.sleep(0)) # TRIO115
25 |+ trio.run(trio.lowlevel.checkpoint) # TRIO115


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.