Skip to content

Commit

Permalink
[flake8-async] Sleep with >24 hour interval should usually sleep fore…
Browse files Browse the repository at this point in the history
…ver (ASYNC116) (#11498)

## Summary

Addresses #8451 by implementing rule 116 to add an unsafe fix when sleep
is used with a >24 hour interval to instead consider sleeping forever.

This rule is added as async instead as I my understanding was that these
trio rules would be moved to async anyway.

There are a couple of TODOs, which address further extending the rule by
adding support for lookups and evaluations, and also supporting `anyio`.
  • Loading branch information
ekohilas authored May 23, 2024
1 parent 9a93409 commit ebdaf57
Show file tree
Hide file tree
Showing 8 changed files with 321 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# type: ignore
# ASYNCIO_NO_ERROR - no asyncio.sleep_forever, so check intentionally doesn't trigger.
import math
from math import inf


async def import_trio():
import trio

# These examples are probably not meant to ever wake up:
await trio.sleep(100000) # error: 116, "async"

# 'inf literal' overflow trick
await trio.sleep(1e999) # error: 116, "async"

await trio.sleep(86399)
await trio.sleep(86400)
await trio.sleep(86400.01) # error: 116, "async"
await trio.sleep(86401) # error: 116, "async"

await trio.sleep(-1) # will raise a runtime error
await trio.sleep(0) # handled by different check

# these ones _definitely_ never wake up (TODO)
await trio.sleep(float("inf"))
await trio.sleep(math.inf)
await trio.sleep(inf)

# don't require inf to be in math (TODO)
await trio.sleep(np.inf)

# don't evaluate expressions (TODO)
one_day = 86401
await trio.sleep(86400 + 1)
await trio.sleep(60 * 60 * 24 + 1)
await trio.sleep(foo())
await trio.sleep(one_day)
await trio.sleep(86400 + foo())
await trio.sleep(86400 + ...)
await trio.sleep("hello")
await trio.sleep(...)


def not_async_fun():
import trio

# does not require the call to be awaited, nor in an async fun
trio.sleep(86401) # error: 116, "async"
# also checks that we don't break visit_Call
trio.run(trio.sleep(86401)) # error: 116, "async"


async def import_from_trio():
from trio import sleep

# catch from import
await sleep(86401) # error: 116, "async"
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 @@ -508,6 +508,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::BlockingOsCallInAsyncFunction) {
flake8_async::rules::blocking_os_call(checker, call);
}
if checker.enabled(Rule::SleepForeverCall) {
flake8_async::rules::sleep_forever_call(checker, call);
}
if checker.any_enabled(&[Rule::Print, Rule::PPrint]) {
flake8_print::rules::print_call(checker, call);
}
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 @@ -334,6 +334,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Async, "100") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingHttpCallInAsyncFunction),
(Flake8Async, "101") => (RuleGroup::Stable, rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction),
(Flake8Async, "102") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingOsCallInAsyncFunction),
(Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::SleepForeverCall),

// flake8-trio
(Flake8Trio, "100") => (RuleGroup::Stable, rules::flake8_trio::rules::TrioTimeoutWithoutAwait),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_async/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod tests {
#[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC100.py"))]
#[test_case(Rule::OpenSleepOrSubprocessInAsyncFunction, Path::new("ASYNC101.py"))]
#[test_case(Rule::BlockingOsCallInAsyncFunction, Path::new("ASYNC102.py"))]
#[test_case(Rule::SleepForeverCall, Path::new("ASYNC116.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_async/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
pub(crate) use blocking_http_call::*;
pub(crate) use blocking_os_call::*;
pub(crate) use open_sleep_or_subprocess_call::*;
pub(crate) use sleep_forever_call::*;

mod blocking_http_call;
mod blocking_os_call;
mod open_sleep_or_subprocess_call;
mod sleep_forever_call;
110 changes: 110 additions & 0 deletions crates/ruff_linter/src/rules/flake8_async/rules/sleep_forever_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprCall, ExprNumberLiteral, Number};
use ruff_python_semantic::Modules;
use ruff_text_size::Ranged;

use crate::{checkers::ast::Checker, importer::ImportRequest};

/// ## What it does
/// Checks for uses of `trio.sleep()` with an interval greater than 24 hours.
///
/// ## Why is this bad?
/// `trio.sleep()` with an interval greater than 24 hours is usually intended
/// to sleep indefinitely. Instead of using a large interval,
/// `trio.sleep_forever()` better conveys the intent.
///
///
/// ## Example
/// ```python
/// import trio
///
///
/// async def func():
/// await trio.sleep(86401)
/// ```
///
/// Use instead:
/// ```python
/// import trio
///
///
/// async def func():
/// await trio.sleep_forever()
/// ```
#[violation]
pub struct SleepForeverCall;

impl Violation for SleepForeverCall {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
format!("`trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`")
}

fn fix_title(&self) -> Option<String> {
Some(format!("Replace with `trio.sleep_forever()`"))
}
}

/// ASYNC116
pub(crate) fn sleep_forever_call(checker: &mut Checker, call: &ExprCall) {
if !checker.semantic().seen_module(Modules::TRIO) {
return;
}

if call.arguments.len() != 1 {
return;
}

let Some(arg) = call.arguments.find_argument("seconds", 0) else {
return;
};

if !checker
.semantic()
.resolve_qualified_name(call.func.as_ref())
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["trio", "sleep"]))
{
return;
}

let Expr::NumberLiteral(ExprNumberLiteral { value, .. }) = arg else {
return;
};

// TODO(ekohilas): Replace with Duration::from_days(1).as_secs(); when available.
let one_day_in_secs = 60 * 60 * 24;
match value {
Number::Int(int_value) => {
let Some(int_value) = int_value.as_u64() else {
return;
};
if int_value <= one_day_in_secs {
return;
}
}
Number::Float(float_value) =>
{
#[allow(clippy::cast_precision_loss)]
if *float_value <= one_day_in_secs as f64 {
return;
}
}
Number::Complex { .. } => return,
}

let mut diagnostic = Diagnostic::new(SleepForeverCall, call.range());
let replacement_function = "sleep_forever";
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import_from("trio", replacement_function),
call.func.start(),
checker.semantic(),
)?;
let reference_edit = Edit::range_replacement(binding, call.func.range());
let arg_edit = Edit::range_replacement("()".to_string(), 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,145 @@
---
source: crates/ruff_linter/src/rules/flake8_async/mod.rs
---
ASYNC116.py:11:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
10 | # These examples are probably not meant to ever wake up:
11 | await trio.sleep(100000) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^^ ASYNC116
12 |
13 | # 'inf literal' overflow trick
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
8 8 | import trio
9 9 |
10 10 | # These examples are probably not meant to ever wake up:
11 |- await trio.sleep(100000) # error: 116, "async"
11 |+ await trio.sleep_forever() # error: 116, "async"
12 12 |
13 13 | # 'inf literal' overflow trick
14 14 | await trio.sleep(1e999) # error: 116, "async"

ASYNC116.py:14:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
13 | # 'inf literal' overflow trick
14 | await trio.sleep(1e999) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^ ASYNC116
15 |
16 | await trio.sleep(86399)
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
11 11 | await trio.sleep(100000) # error: 116, "async"
12 12 |
13 13 | # 'inf literal' overflow trick
14 |- await trio.sleep(1e999) # error: 116, "async"
14 |+ await trio.sleep_forever() # error: 116, "async"
15 15 |
16 16 | await trio.sleep(86399)
17 17 | await trio.sleep(86400)

ASYNC116.py:18:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
16 | await trio.sleep(86399)
17 | await trio.sleep(86400)
18 | await trio.sleep(86400.01) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^^^^ ASYNC116
19 | await trio.sleep(86401) # error: 116, "async"
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
15 15 |
16 16 | await trio.sleep(86399)
17 17 | await trio.sleep(86400)
18 |- await trio.sleep(86400.01) # error: 116, "async"
18 |+ await trio.sleep_forever() # error: 116, "async"
19 19 | await trio.sleep(86401) # error: 116, "async"
20 20 |
21 21 | await trio.sleep(-1) # will raise a runtime error

ASYNC116.py:19:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
17 | await trio.sleep(86400)
18 | await trio.sleep(86400.01) # error: 116, "async"
19 | await trio.sleep(86401) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^ ASYNC116
20 |
21 | await trio.sleep(-1) # will raise a runtime error
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
16 16 | await trio.sleep(86399)
17 17 | await trio.sleep(86400)
18 18 | await trio.sleep(86400.01) # error: 116, "async"
19 |- await trio.sleep(86401) # error: 116, "async"
19 |+ await trio.sleep_forever() # error: 116, "async"
20 20 |
21 21 | await trio.sleep(-1) # will raise a runtime error
22 22 | await trio.sleep(0) # handled by different check

ASYNC116.py:48:5: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
47 | # does not require the call to be awaited, nor in an async fun
48 | trio.sleep(86401) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^ ASYNC116
49 | # also checks that we don't break visit_Call
50 | trio.run(trio.sleep(86401)) # error: 116, "async"
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
45 45 | import trio
46 46 |
47 47 | # does not require the call to be awaited, nor in an async fun
48 |- trio.sleep(86401) # error: 116, "async"
48 |+ trio.sleep_forever() # error: 116, "async"
49 49 | # also checks that we don't break visit_Call
50 50 | trio.run(trio.sleep(86401)) # error: 116, "async"
51 51 |

ASYNC116.py:50:14: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
48 | trio.sleep(86401) # error: 116, "async"
49 | # also checks that we don't break visit_Call
50 | trio.run(trio.sleep(86401)) # error: 116, "async"
| ^^^^^^^^^^^^^^^^^ ASYNC116
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
47 47 | # does not require the call to be awaited, nor in an async fun
48 48 | trio.sleep(86401) # error: 116, "async"
49 49 | # also checks that we don't break visit_Call
50 |- trio.run(trio.sleep(86401)) # error: 116, "async"
50 |+ trio.run(trio.sleep_forever()) # error: 116, "async"
51 51 |
52 52 |
53 53 | async def import_from_trio():

ASYNC116.py:57:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
|
56 | # catch from import
57 | await sleep(86401) # error: 116, "async"
| ^^^^^^^^^^^^ ASYNC116
|
= help: Replace with `trio.sleep_forever()`

Unsafe fix
2 2 | # ASYNCIO_NO_ERROR - no asyncio.sleep_forever, so check intentionally doesn't trigger.
3 3 | import math
4 4 | from math import inf
5 |+from trio import sleep_forever
5 6 |
6 7 |
7 8 | async def import_trio():
--------------------------------------------------------------------------------
54 55 | from trio import sleep
55 56 |
56 57 | # catch from import
57 |- await sleep(86401) # error: 116, "async"
58 |+ await sleep_forever() # error: 116, "async"
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.

0 comments on commit ebdaf57

Please sign in to comment.