diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO109.py b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO109.py new file mode 100644 index 0000000000000..670e0486b2a14 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_trio/TRIO109.py @@ -0,0 +1,10 @@ +async def func(): + ... + + +async def func(timeout): + ... + + +async def func(timeout=10): + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 6cf80b7f870dd..0ad8dc16e5777 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -356,6 +356,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { flake8_builtins::rules::builtin_variable_shadowing(checker, name, name.range()); } } + if checker.enabled(Rule::TrioAsyncFunctionWithTimeout) { + flake8_trio::rules::async_function_with_timeout(checker, function_def); + } #[cfg(feature = "unreachable-code")] if checker.enabled(Rule::UnreachableCode) { checker diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 36c0caf53b59b..7b307a7da68ac 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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, "109") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioAsyncFunctionWithTimeout), (Flake8Trio, "110") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioUnneededSleep), (Flake8Trio, "115") => (RuleGroup::Preview, rules::flake8_trio::rules::TrioZeroSleepCall), diff --git a/crates/ruff_linter/src/rules/flake8_trio/mod.rs b/crates/ruff_linter/src/rules/flake8_trio/mod.rs index 20724ac7e6c9c..2e0bb7911ecba 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/mod.rs @@ -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::TrioAsyncFunctionWithTimeout, Path::new("TRIO109.py"))] #[test_case(Rule::TrioUnneededSleep, Path::new("TRIO110.py"))] #[test_case(Rule::TrioZeroSleepCall, Path::new("TRIO115.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff_linter/src/rules/flake8_trio/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_trio/rules/async_function_with_timeout.rs new file mode 100644 index 0000000000000..f32421fbd8c39 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/async_function_with_timeout.rs @@ -0,0 +1,53 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `async` functions with a `timeout` argument. +/// +/// ## Why is this bad? +/// Rather than implementing asynchronous timeout behavior manually, prefer +/// trio's built-in timeout functionality, available as `trio.fail_after`, +/// `trio.move_on_after`, `trio.fail_at`, and `trio.move_on_at`. +/// +/// ## Example +/// ```python +/// async def func(): +/// await long_running_task(timeout=2) +/// ``` +/// +/// Use instead: +/// ```python +/// async def func(): +/// with trio.fail_after(2): +/// await long_running_task() +/// ``` +#[violation] +pub struct TrioAsyncFunctionWithTimeout; + +impl Violation for TrioAsyncFunctionWithTimeout { + #[derive_message_formats] + fn message(&self) -> String { + format!("Prefer `trio.fail_after` and `trio.move_on_after` over manual `async` timeout behavior") + } +} + +/// TRIO109 +pub(crate) fn async_function_with_timeout( + checker: &mut Checker, + function_def: &ast::StmtFunctionDef, +) { + if !function_def.is_async { + return; + } + let Some(timeout) = function_def.parameters.find("timeout") else { + return; + }; + checker.diagnostics.push(Diagnostic::new( + TrioAsyncFunctionWithTimeout, + timeout.range(), + )); +} 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 267b80231f8cc..3126b10224b93 100644 --- a/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_trio/rules/mod.rs @@ -1,8 +1,10 @@ +pub(crate) use async_function_with_timeout::*; pub(crate) use sync_call::*; pub(crate) use timeout_without_await::*; pub(crate) use unneeded_sleep::*; pub(crate) use zero_sleep_call::*; +mod async_function_with_timeout; mod sync_call; mod timeout_without_await; mod unneeded_sleep; diff --git a/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO109_TRIO109.py.snap b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO109_TRIO109.py.snap new file mode 100644 index 0000000000000..07df19550bee3 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_trio/snapshots/ruff_linter__rules__flake8_trio__tests__TRIO109_TRIO109.py.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff_linter/src/rules/flake8_trio/mod.rs +--- +TRIO109.py:5:16: TRIO109 Prefer `trio.fail_after` and `trio.move_on_after` over manual `async` timeout behavior + | +5 | async def func(timeout): + | ^^^^^^^ TRIO109 +6 | ... + | + +TRIO109.py:9:16: TRIO109 Prefer `trio.fail_after` and `trio.move_on_after` over manual `async` timeout behavior + | + 9 | async def func(timeout=10): + | ^^^^^^^^^^ TRIO109 +10 | ... + | + + diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index c45bb251e62e3..0af30770e4f32 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -2281,6 +2281,15 @@ pub struct Parameters { } impl Parameters { + /// Returns the [`ParameterWithDefault`] with the given name, or `None` if no such [`ParameterWithDefault`] exists. + pub fn find(&self, name: &str) -> Option<&ParameterWithDefault> { + self.posonlyargs + .iter() + .chain(&self.args) + .chain(&self.kwonlyargs) + .find(|arg| arg.parameter.name.as_str() == name) + } + /// Returns `true` if a parameter with the given name included in this [`Parameters`]. pub fn includes(&self, name: &str) -> bool { if self diff --git a/ruff.schema.json b/ruff.schema.json index 34cfe384aa67d..36f7f31eaab77 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3475,6 +3475,7 @@ "TRIO10", "TRIO100", "TRIO105", + "TRIO109", "TRIO11", "TRIO110", "TRIO115",