Skip to content

Commit

Permalink
Use dedicated function
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 7, 2023
1 parent 24a3e07 commit aa92c35
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 38 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@

async def foo():
async def func():
...

async def foo(timeout):

async def func(timeout):
...

async def foo(timeout=10):

async def func(timeout=10):
...
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
if checker.enabled(Rule::TrioAsyncFunctionWithTimeout) {
flake8_trio::rules::async_function_with_timeout(checker, parameters, *is_async);
flake8_trio::rules::async_function_with_timeout(checker, function_def);
}
#[cfg(feature = "unreachable-code")]
if checker.enabled(Rule::UnreachableCode) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use crate::checkers::ast::Checker;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Parameters;
use ruff_python_ast as ast;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for asynchronous functions that have a timeout argument.
/// Checks for `async` functions with a `timeout` argument.
///
/// ## Why is this bad?
/// It is preferable to use the special functions that Trio
/// provides to handle timeouts rather than implement them manually.
/// 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
Expand All @@ -28,27 +31,23 @@ pub struct TrioAsyncFunctionWithTimeout;
impl Violation for TrioAsyncFunctionWithTimeout {
#[derive_message_formats]
fn message(&self) -> String {
format!("Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead")
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,
parameters: &Parameters,
is_async: bool,
function_def: &ast::StmtFunctionDef,
) {
if !is_async {
if !function_def.is_async {
return;
}

if let Some(timeout_argument) = parameters
.args
.iter()
.find(|argument| argument.parameter.name.eq("timeout"))
{
checker.diagnostics.push(Diagnostic::new(
TrioAsyncFunctionWithTimeout,
timeout_argument.range,
));
}
let Some(timeout) = function_def.parameters.find("timeout") else {
return;
};
checker.diagnostics.push(Diagnostic::new(
TrioAsyncFunctionWithTimeout,
timeout.range(),
));
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
---
source: crates/ruff_linter/src/rules/flake8_trio/mod.rs
---
TRIO109.py:5:15: TRIO109 Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
TRIO109.py:5:16: TRIO109 Prefer `trio.fail_after` and `trio.move_on_after` over manual `async` timeout behavior
|
3 | ...
4 |
5 | async def foo(timeout):
| ^^^^^^^ TRIO109
5 | async def func(timeout):
| ^^^^^^^ TRIO109
6 | ...
|

TRIO109.py:8:15: TRIO109 Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
|
6 | ...
7 |
8 | async def foo(timeout=10):
| ^^^^^^^^^^ TRIO109
9 | ...
|
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 | ...
|


9 changes: 9 additions & 0 deletions crates/ruff_python_ast/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit aa92c35

Please sign in to comment.