diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC100.py b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC100.py index 8371d2e2a5..0ccdf30a04 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC100.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC100.py @@ -1,51 +1,63 @@ import anyio import asyncio import trio +from contextlib import nullcontext async def func(): - async with trio.fail_after(): + with trio.fail_after(): ... async def func(): - async with trio.fail_at(): + with trio.fail_at(): await ... async def func(): - async with trio.move_on_after(): + with trio.move_on_after(): ... async def func(): - async with trio.move_at(): + with trio.move_at(): await ... async def func(): - async with trio.move_at(): + with trio.move_at(): async with trio.open_nursery(): ... async def func(): - async with anyio.move_on_after(delay=0.2): + with trio.move_at(): + async for x in ...: + ... + + +async def func(): + with anyio.move_on_after(delay=0.2): ... async def func(): - async with anyio.fail_after(): + with anyio.fail_after(): ... async def func(): - async with anyio.CancelScope(): + with anyio.CancelScope(): ... async def func(): - async with anyio.CancelScope(): + with anyio.CancelScope(), nullcontext(): + ... + + +async def func(): + with nullcontext(), anyio.CancelScope(): ... @@ -62,3 +74,18 @@ async def func(): async def func(): async with asyncio.timeout(delay=0.2), asyncio.TaskGroup(): ... + + +async def func(): + async with asyncio.timeout(delay=0.2), asyncio.TaskGroup(), asyncio.timeout(delay=0.2): + ... + + +async def func(): + async with asyncio.timeout(delay=0.2), asyncio.TaskGroup(), asyncio.timeout(delay=0.2), asyncio.TaskGroup(): + ... + + +async def func(): + async with asyncio.timeout(delay=0.2), asyncio.timeout(delay=0.2): + ... diff --git a/crates/ruff_linter/src/rules/flake8_async/rules/cancel_scope_no_checkpoint.rs b/crates/ruff_linter/src/rules/flake8_async/rules/cancel_scope_no_checkpoint.rs index 7064318e7f..c5b5deaf8a 100644 --- a/crates/ruff_linter/src/rules/flake8_async/rules/cancel_scope_no_checkpoint.rs +++ b/crates/ruff_linter/src/rules/flake8_async/rules/cancel_scope_no_checkpoint.rs @@ -13,9 +13,9 @@ use crate::settings::types::PreviewMode; /// /// ## Why is this bad? /// Some asynchronous context managers, such as `asyncio.timeout` and -/// `trio.move_on_after`, have no effect unless they contain an `await` -/// statement. The use of such context managers without an `await` statement is -/// likely a mistake. +/// `trio.move_on_after`, have no effect unless they contain a checkpoint. +/// The use of such context managers without an `await`, `async with` or +/// `async for` statement is likely a mistake. /// /// ## Example /// ```python @@ -55,17 +55,29 @@ pub(crate) fn cancel_scope_no_checkpoint( with_stmt: &StmtWith, with_items: &[WithItem], ) { - let Some(method_name) = with_items.iter().find_map(|item| { - let call = item.context_expr.as_call_expr()?; - let qualified_name = checker - .semantic() - .resolve_qualified_name(call.func.as_ref())?; - MethodName::try_from(&qualified_name) - }) else { + let Some((with_item_pos, method_name)) = with_items + .iter() + .enumerate() + .filter_map(|(pos, item)| { + let call = item.context_expr.as_call_expr()?; + let qualified_name = checker + .semantic() + .resolve_qualified_name(call.func.as_ref())?; + let method_name = MethodName::try_from(&qualified_name)?; + if method_name.is_timeout_context() { + Some((pos, method_name)) + } else { + None + } + }) + .last() + else { return; }; - if !method_name.is_timeout_context() { + // If this is an `async with` and the timeout has items after it, then the + // further items are checkpoints. + if with_stmt.is_async && with_item_pos < with_items.len() - 1 { return; } @@ -76,24 +88,6 @@ pub(crate) fn cancel_scope_no_checkpoint( return; } - // If there's an `asyncio.TaskGroup()` context manager alongside the timeout, it's fine, as in: - // ```python - // async with asyncio.timeout(2.0), asyncio.TaskGroup(): - // ... - // ``` - if with_items.iter().any(|item| { - item.context_expr.as_call_expr().is_some_and(|call| { - checker - .semantic() - .resolve_qualified_name(call.func.as_ref()) - .is_some_and(|qualified_name| { - matches!(qualified_name.segments(), ["asyncio", "TaskGroup"]) - }) - }) - }) { - return; - } - if matches!(checker.settings.preview, PreviewMode::Disabled) { if matches!(method_name.module(), AsyncModule::Trio) { checker.diagnostics.push(Diagnostic::new( diff --git a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap index 22f7c8a1eb..86f0972a0b 100644 --- a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap +++ b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC100_ASYNC100.py.snap @@ -1,20 +1,20 @@ --- source: crates/ruff_linter/src/rules/flake8_async/mod.rs --- -ASYNC100.py:7:5: ASYNC100 A `with trio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:8:5: ASYNC100 A `with trio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -6 | async def func(): -7 | async with trio.fail_after(): +7 | async def func(): +8 | with trio.fail_after(): | _____^ -8 | | ... +9 | | ... | |___________^ ASYNC100 | -ASYNC100.py:17:5: ASYNC100 A `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:18:5: ASYNC100 A `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -16 | async def func(): -17 | async with trio.move_on_after(): +17 | async def func(): +18 | with trio.move_on_after(): | _____^ -18 | | ... +19 | | ... | |___________^ ASYNC100 | diff --git a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC100_ASYNC100.py.snap b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC100_ASYNC100.py.snap index bf704040e6..0eca205a5b 100644 --- a/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC100_ASYNC100.py.snap +++ b/crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__preview__ASYNC100_ASYNC100.py.snap @@ -1,74 +1,101 @@ --- source: crates/ruff_linter/src/rules/flake8_async/mod.rs --- -ASYNC100.py:7:5: ASYNC100 A `with trio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:8:5: ASYNC100 A `with trio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -6 | async def func(): -7 | async with trio.fail_after(): +7 | async def func(): +8 | with trio.fail_after(): | _____^ -8 | | ... +9 | | ... | |___________^ ASYNC100 | -ASYNC100.py:17:5: ASYNC100 A `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:18:5: ASYNC100 A `with trio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -16 | async def func(): -17 | async with trio.move_on_after(): +17 | async def func(): +18 | with trio.move_on_after(): | _____^ -18 | | ... +19 | | ... | |___________^ ASYNC100 | -ASYNC100.py:33:5: ASYNC100 A `with anyio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:40:5: ASYNC100 A `with anyio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -32 | async def func(): -33 | async with anyio.move_on_after(delay=0.2): +39 | async def func(): +40 | with anyio.move_on_after(delay=0.2): | _____^ -34 | | ... +41 | | ... | |___________^ ASYNC100 | -ASYNC100.py:38:5: ASYNC100 A `with anyio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:45:5: ASYNC100 A `with anyio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -37 | async def func(): -38 | async with anyio.fail_after(): +44 | async def func(): +45 | with anyio.fail_after(): | _____^ -39 | | ... +46 | | ... | |___________^ ASYNC100 | -ASYNC100.py:43:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:50:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -42 | async def func(): -43 | async with anyio.CancelScope(): +49 | async def func(): +50 | with anyio.CancelScope(): | _____^ -44 | | ... +51 | | ... | |___________^ ASYNC100 | -ASYNC100.py:48:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:55:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -47 | async def func(): -48 | async with anyio.CancelScope(): +54 | async def func(): +55 | with anyio.CancelScope(), nullcontext(): | _____^ -49 | | ... +56 | | ... | |___________^ ASYNC100 | -ASYNC100.py:53:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:60:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -52 | async def func(): -53 | async with asyncio.timeout(delay=0.2): +59 | async def func(): +60 | with nullcontext(), anyio.CancelScope(): | _____^ -54 | | ... +61 | | ... | |___________^ ASYNC100 | -ASYNC100.py:58:5: ASYNC100 A `with asyncio.timeout_at(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. +ASYNC100.py:65:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. | -57 | async def func(): -58 | async with asyncio.timeout_at(when=0.2): +64 | async def func(): +65 | async with asyncio.timeout(delay=0.2): | _____^ -59 | | ... +66 | | ... + | |___________^ ASYNC100 + | + +ASYNC100.py:70:5: ASYNC100 A `with asyncio.timeout_at(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. + | +69 | async def func(): +70 | async with asyncio.timeout_at(when=0.2): + | _____^ +71 | | ... + | |___________^ ASYNC100 + | + +ASYNC100.py:80:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. + | +79 | async def func(): +80 | async with asyncio.timeout(delay=0.2), asyncio.TaskGroup(), asyncio.timeout(delay=0.2): + | _____^ +81 | | ... + | |___________^ ASYNC100 + | + +ASYNC100.py:90:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. + | +89 | async def func(): +90 | async with asyncio.timeout(delay=0.2), asyncio.timeout(delay=0.2): + | _____^ +91 | | ... | |___________^ ASYNC100 |