From 6efc212759d67eb0be7c933a89b97724ae6dad9f Mon Sep 17 00:00:00 2001 From: James Braza Date: Wed, 28 Aug 2024 22:13:12 -0700 Subject: [PATCH 1/7] Documenting how to use ASYNC109 a bit more --- .../rules/async_function_with_timeout.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index 4ceedafb5b435..99df880cfa222 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -11,10 +11,18 @@ use crate::settings::types::PythonVersion; /// ## What it does /// Checks for `async` functions with a `timeout` argument. /// +/// This rule is only applied for Python 3.11 and newer, +/// as `asyncio.timeout` was added in Python 3.11. +/// /// ## Why is this bad? /// Rather than implementing asynchronous timeout behavior manually, prefer -/// built-in timeout functionality, such as `asyncio.timeout`, `trio.fail_after`, -/// or `anyio.move_on_after`, among others. +/// built-in timeout functionality, such as `asyncio.timeout`, +/// `trio.fail_after`, or `anyio.move_on_after`, among others. +/// +/// If you are abstracting `asyncio` within a function +/// that internally utilizes `asyncio.timeout`, +/// this rule can be disabled, or use a more specific timeout name +/// such as `your_behavior_timeout`. /// /// ## Example /// From 6ef284c1cd1abef595260d2f19628530fbc24103 Mon Sep 17 00:00:00 2001 From: James Braza Date: Sat, 31 Aug 2024 23:08:45 -0700 Subject: [PATCH 2/7] Specified Python 3.11+ specifically for 'asyncio' --- .../flake8_async/rules/async_function_with_timeout.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index 99df880cfa222..18b008e3b2998 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -9,10 +9,11 @@ use crate::rules::flake8_async::helpers::AsyncModule; use crate::settings::types::PythonVersion; /// ## What it does -/// Checks for `async` functions with a `timeout` argument. +/// Checks for `async` functions (from `anyio`, `asyncio`, or `trio`) +/// with a `timeout` argument. /// -/// This rule is only applied for Python 3.11 and newer, -/// as `asyncio.timeout` was added in Python 3.11. +/// Note `asyncio.timeout` was added in Python 3.11, +/// so this rule is only applicable to `asyncio` functions in Python 3.11+. /// /// ## Why is this bad? /// Rather than implementing asynchronous timeout behavior manually, prefer From 74c4b606093b099d706f4b075a76aa22c393c7c3 Mon Sep 17 00:00:00 2001 From: James Braza Date: Sat, 31 Aug 2024 23:17:39 -0700 Subject: [PATCH 3/7] Changed the wording on sidestepping false positives --- .../flake8_async/rules/async_function_with_timeout.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index 18b008e3b2998..d34467aabbbb2 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -20,10 +20,10 @@ use crate::settings::types::PythonVersion; /// built-in timeout functionality, such as `asyncio.timeout`, /// `trio.fail_after`, or `anyio.move_on_after`, among others. /// -/// If you are abstracting `asyncio` within a function -/// that internally utilizes `asyncio.timeout`, -/// this rule can be disabled, or use a more specific timeout name -/// such as `your_behavior_timeout`. +/// For functions that wrap `asyncio.timeout`, +/// false positives from this rule can be sidestepped +/// by using a more verbose parameter name +/// describing the behavior wrapped with `asyncio.timeout`. /// /// ## Example /// From e0b00c5bceb466be21db8869d94eaa8d83ad0a87 Mon Sep 17 00:00:00 2001 From: James Braza Date: Sat, 31 Aug 2024 23:26:22 -0700 Subject: [PATCH 4/7] Documented structured concurrency a bit --- .../rules/flake8_async/rules/async_function_with_timeout.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index d34467aabbbb2..ea327853cb2ed 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -20,6 +20,12 @@ use crate::settings::types::PythonVersion; /// built-in timeout functionality, such as `asyncio.timeout`, /// `trio.fail_after`, or `anyio.move_on_after`, among others. /// +/// This rule is highly opinionated to enforce a design pattern +/// called "structured concurrency" that allows for +/// `async` functions to be oblivious to timeouts, +/// instead letting callers to handle the logic with a context manager +/// ([relevant blog](https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/#timeouts-and-cancellation). +/// /// For functions that wrap `asyncio.timeout`, /// false positives from this rule can be sidestepped /// by using a more verbose parameter name From a07bbb243d7e1996ef3c7b5b4627ba2e703beb29 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 1 Sep 2024 11:05:51 +0100 Subject: [PATCH 5/7] nitpicks --- .../rules/async_function_with_timeout.rs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index ea327853cb2ed..0f3251625d554 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -9,11 +9,7 @@ use crate::rules::flake8_async::helpers::AsyncModule; use crate::settings::types::PythonVersion; /// ## What it does -/// Checks for `async` functions (from `anyio`, `asyncio`, or `trio`) -/// with a `timeout` argument. -/// -/// Note `asyncio.timeout` was added in Python 3.11, -/// so this rule is only applicable to `asyncio` functions in Python 3.11+. +/// Checks for `async` function definitions with a `timeout` parameter. /// /// ## Why is this bad? /// Rather than implementing asynchronous timeout behavior manually, prefer @@ -21,15 +17,24 @@ use crate::settings::types::PythonVersion; /// `trio.fail_after`, or `anyio.move_on_after`, among others. /// /// This rule is highly opinionated to enforce a design pattern -/// called "structured concurrency" that allows for +/// called ["structured concurrency"] that allows for /// `async` functions to be oblivious to timeouts, -/// instead letting callers to handle the logic with a context manager -/// ([relevant blog](https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/#timeouts-and-cancellation). +/// instead letting callers to handle the logic with a context manager. +/// +/// ## Details /// -/// For functions that wrap `asyncio.timeout`, -/// false positives from this rule can be sidestepped -/// by using a more verbose parameter name -/// describing the behavior wrapped with `asyncio.timeout`. +/// This rule attempts to detect which async framework your code is using +/// by analysing the imports in the file it's checking. If it sees an +/// `anyio` import in your code, it will assume `anyio` is your framework +/// of choice; if it sees a `trio` import, it will assume `trio`; if it +/// sees neither, it will assume `asyncio`. `asyncio.timeout` was added +/// in Python 3.11, so if `asyncio` is detected as the framework being used, +/// this rule will be ignored when your configured [`target-version`] is set +/// to less than Python 3.11. +/// +/// For functions that wrap `asyncio.timeout`, `trio.fail_after` or +/// `anyio.move_on_after`, false positives from this rule can be avoided +/// by using a different parameter name. /// /// ## Example /// @@ -56,6 +61,8 @@ use crate::settings::types::PythonVersion; /// - [`asyncio` timeouts](https://docs.python.org/3/library/asyncio-task.html#timeouts) /// - [`anyio` timeouts](https://anyio.readthedocs.io/en/stable/cancellation.html) /// - [`trio` timeouts](https://trio.readthedocs.io/en/stable/reference-core.html#cancellation-and-timeouts) +/// +/// ["structured concurrency"]: https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/#timeouts-and-cancellation #[violation] pub struct AsyncFunctionWithTimeout { module: AsyncModule, From 82847180eaa786916ae46aed6a8fa0def39ebade Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 1 Sep 2024 11:06:29 +0100 Subject: [PATCH 6/7] Update async_function_with_timeout.rs --- .../src/rules/flake8_async/rules/async_function_with_timeout.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index 0f3251625d554..49cd56e83ace0 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -9,7 +9,7 @@ use crate::rules::flake8_async::helpers::AsyncModule; use crate::settings::types::PythonVersion; /// ## What it does -/// Checks for `async` function definitions with a `timeout` parameter. +/// Checks for `async` function definitions with `timeout` parameters. /// /// ## Why is this bad? /// Rather than implementing asynchronous timeout behavior manually, prefer From 7f61ec7096b3c5971c793c6c56a0936fa4b6b96f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 1 Sep 2024 11:12:00 +0100 Subject: [PATCH 7/7] Update async_function_with_timeout.rs --- .../rules/flake8_async/rules/async_function_with_timeout.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs index 49cd56e83ace0..943cc26288d6c 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/async_function_with_timeout.rs @@ -13,8 +13,8 @@ use crate::settings::types::PythonVersion; /// /// ## Why is this bad? /// Rather than implementing asynchronous timeout behavior manually, prefer -/// built-in timeout functionality, such as `asyncio.timeout`, -/// `trio.fail_after`, or `anyio.move_on_after`, among others. +/// built-in timeout functionality, such as `asyncio.timeout`, `trio.fail_after`, +/// or `anyio.move_on_after`, among others. /// /// This rule is highly opinionated to enforce a design pattern /// called ["structured concurrency"] that allows for