Skip to content

Commit

Permalink
[flake8-async] Update ASYNC116 to match upstream (#12266)
Browse files Browse the repository at this point in the history
Co-authored-by: Micha Reiser <micha@reiser.io>
  • Loading branch information
augustelalande and MichaReiser committed Jul 10, 2024
1 parent d365f1a commit 880c31d
Show file tree
Hide file tree
Showing 8 changed files with 448 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,56 @@ async def import_from_trio():

# catch from import
await sleep(86401) # error: 116, "async"


async def import_anyio():
import anyio

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

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

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

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

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

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

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


def not_async_fun():
import anyio

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


async def import_from_anyio():
from anyio import sleep

# catch from import
await sleep(86401) # error: 116, "async"
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::BlockingSleepInAsyncFunction) {
flake8_async::rules::blocking_sleep(checker, call);
}
if checker.enabled(Rule::SleepForeverCall) {
flake8_async::rules::sleep_forever_call(checker, call);
if checker.enabled(Rule::LongSleepNotForever) {
flake8_async::rules::long_sleep_not_forever(checker, call);
}
if checker.any_enabled(&[Rule::Print, Rule::PPrint]) {
flake8_print::rules::print_call(checker, call);
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Async, "109") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncFunctionWithTimeout),
(Flake8Async, "110") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncBusyWait),
(Flake8Async, "115") => (RuleGroup::Stable, rules::flake8_async::rules::AsyncZeroSleep),
(Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::SleepForeverCall),
(Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::LongSleepNotForever),
(Flake8Async, "210") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingHttpCallInAsyncFunction),
(Flake8Async, "220") => (RuleGroup::Stable, rules::flake8_async::rules::CreateSubprocessInAsyncFunction),
(Flake8Async, "221") => (RuleGroup::Stable, rules::flake8_async::rules::RunProcessInAsyncFunction),
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_linter/src/rules/flake8_async/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ mod tests {
#[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_1.py"))]
#[test_case(Rule::AsyncBusyWait, Path::new("ASYNC110.py"))]
#[test_case(Rule::AsyncZeroSleep, Path::new("ASYNC115.py"))]
#[test_case(Rule::SleepForeverCall, Path::new("ASYNC116.py"))]
#[test_case(Rule::LongSleepNotForever, Path::new("ASYNC116.py"))]
#[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC210.py"))]
#[test_case(Rule::CreateSubprocessInAsyncFunction, Path::new("ASYNC22x.py"))]
#[test_case(Rule::RunProcessInAsyncFunction, Path::new("ASYNC22x.py"))]
Expand All @@ -43,6 +43,7 @@ mod tests {
#[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_1.py"))]
#[test_case(Rule::AsyncBusyWait, Path::new("ASYNC110.py"))]
#[test_case(Rule::AsyncZeroSleep, Path::new("ASYNC115.py"))]
#[test_case(Rule::LongSleepNotForever, Path::new("ASYNC116.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@ 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};
use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::rules::flake8_async::helpers::AsyncModule;

/// ## What it does
/// Checks for uses of `trio.sleep()` with an interval greater than 24 hours.
/// Checks for uses of `trio.sleep()` or `anyio.sleep()` with a delay 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.
/// Calling `sleep()` with a delay greater than 24 hours is usually intended
/// to sleep indefinitely. Instead of using a large delay,
/// `trio.sleep_forever()` or `anyio.sleep_forever()` better conveys the intent.
///
///
/// ## Example
Expand All @@ -33,23 +35,31 @@ use crate::{checkers::ast::Checker, importer::ImportRequest};
/// await trio.sleep_forever()
/// ```
#[violation]
pub struct SleepForeverCall;
pub struct LongSleepNotForever {
module: AsyncModule,
}

impl Violation for SleepForeverCall {
impl Violation for LongSleepNotForever {
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()`")
let Self { module } = self;
format!(
"`{module}.sleep()` with >24 hour interval should usually be `{module}.sleep_forever()`"
)
}

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

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

Expand All @@ -61,14 +71,6 @@ pub(crate) fn sleep_forever_call(checker: &mut Checker, call: &ExprCall) {
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;
};
Expand All @@ -94,11 +96,34 @@ pub(crate) fn sleep_forever_call(checker: &mut Checker, call: &ExprCall) {
Number::Complex { .. } => return,
}

let mut diagnostic = Diagnostic::new(SleepForeverCall, call.range());
let Some(qualified_name) = checker
.semantic()
.resolve_qualified_name(call.func.as_ref())
else {
return;
};

let Some(module) = AsyncModule::try_from(&qualified_name) else {
return;
};

let is_relevant_module = if checker.settings.preview.is_enabled() {
matches!(module, AsyncModule::AnyIo | AsyncModule::Trio)
} else {
matches!(module, AsyncModule::Trio)
};

let is_sleep = is_relevant_module && matches!(qualified_name.segments(), [_, "sleep"]);

if !is_sleep {
return;
}

let mut diagnostic = Diagnostic::new(LongSleepNotForever { module }, 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),
&ImportRequest::import_from(&module.to_string(), replacement_function),
call.func.start(),
checker.semantic(),
)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/flake8_async/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub(crate) use blocking_open_call::*;
pub(crate) use blocking_process_invocation::*;
pub(crate) use blocking_sleep::*;
pub(crate) use cancel_scope_no_checkpoint::*;
pub(crate) use sleep_forever_call::*;
pub(crate) use long_sleep_not_forever::*;
pub(crate) use sync_call::*;

mod async_busy_wait;
Expand All @@ -17,5 +17,5 @@ mod blocking_open_call;
mod blocking_process_invocation;
mod blocking_sleep;
mod cancel_scope_no_checkpoint;
mod sleep_forever_call;
mod long_sleep_not_forever;
mod sync_call;
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,6 @@ ASYNC116.py:57:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usu
56 57 | # catch from import
57 |- await sleep(86401) # error: 116, "async"
58 |+ await sleep_forever() # error: 116, "async"
58 59 |
59 60 |
60 61 | async def import_anyio():
Loading

0 comments on commit 880c31d

Please sign in to comment.