Skip to content

Commit

Permalink
Avoid unused async when context manager includes TaskGroup (#12605)
Browse files Browse the repository at this point in the history
## Summary

Closes #12354.
  • Loading branch information
charliermarsh authored Aug 1, 2024
1 parent 7e6b190 commit d774a3b
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 22 deletions.
29 changes: 17 additions & 12 deletions crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC100.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,61 @@


async def func():
with trio.fail_after():
async with trio.fail_after():
...


async def func():
with trio.fail_at():
async with trio.fail_at():
await ...


async def func():
with trio.move_on_after():
async with trio.move_on_after():
...


async def func():
with trio.move_at():
async with trio.move_at():
await ...


async def func():
with trio.move_at():
async with trio.open_nursery() as nursery:
async with trio.move_at():
async with trio.open_nursery():
...


async def func():
with anyio.move_on_after():
async with anyio.move_on_after(delay=0.2):
...


async def func():
with anyio.fail_after():
async with anyio.fail_after():
...


async def func():
with anyio.CancelScope():
async with anyio.CancelScope():
...


async def func():
with anyio.CancelScope():
async with anyio.CancelScope():
...


async def func():
with asyncio.timeout():
async with asyncio.timeout(delay=0.2):
...


async def func():
with asyncio.timeout_at():
async with asyncio.timeout_at(when=0.2):
...


async def func():
async with asyncio.timeout(delay=0.2), asyncio.TaskGroup():
...
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,31 @@ pub(crate) fn cancel_scope_no_checkpoint(
return;
}

// If the body contains an `await` statement, the context manager is used correctly.
let mut visitor = AwaitVisitor::default();
visitor.visit_body(&with_stmt.body);
if visitor.seen_await {
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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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.
|
6 | async def func():
7 | with trio.fail_after():
7 | async with trio.fail_after():
| _____^
8 | | ...
| |___________^ ASYNC100
Expand All @@ -13,7 +13,7 @@ ASYNC100.py:7:5: ASYNC100 A `with trio.fail_after(...):` context does not contai
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.
|
16 | async def func():
17 | with trio.move_on_after():
17 | async with trio.move_on_after():
| _____^
18 | | ...
| |___________^ ASYNC100
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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.
|
6 | async def func():
7 | with trio.fail_after():
7 | async with trio.fail_after():
| _____^
8 | | ...
| |___________^ ASYNC100
Expand All @@ -13,7 +13,7 @@ ASYNC100.py:7:5: ASYNC100 A `with trio.fail_after(...):` context does not contai
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.
|
16 | async def func():
17 | with trio.move_on_after():
17 | async with trio.move_on_after():
| _____^
18 | | ...
| |___________^ ASYNC100
Expand All @@ -22,7 +22,7 @@ ASYNC100.py:17:5: ASYNC100 A `with trio.move_on_after(...):` context does not co
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.
|
32 | async def func():
33 | with anyio.move_on_after():
33 | async with anyio.move_on_after(delay=0.2):
| _____^
34 | | ...
| |___________^ ASYNC100
Expand All @@ -31,7 +31,7 @@ ASYNC100.py:33:5: ASYNC100 A `with anyio.move_on_after(...):` context does not c
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.
|
37 | async def func():
38 | with anyio.fail_after():
38 | async with anyio.fail_after():
| _____^
39 | | ...
| |___________^ ASYNC100
Expand All @@ -40,7 +40,7 @@ ASYNC100.py:38:5: ASYNC100 A `with anyio.fail_after(...):` context does not cont
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.
|
42 | async def func():
43 | with anyio.CancelScope():
43 | async with anyio.CancelScope():
| _____^
44 | | ...
| |___________^ ASYNC100
Expand All @@ -49,7 +49,7 @@ ASYNC100.py:43:5: ASYNC100 A `with anyio.CancelScope(...):` context does not con
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.
|
47 | async def func():
48 | with anyio.CancelScope():
48 | async with anyio.CancelScope():
| _____^
49 | | ...
| |___________^ ASYNC100
Expand All @@ -58,7 +58,7 @@ ASYNC100.py:48:5: ASYNC100 A `with anyio.CancelScope(...):` context does not con
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.
|
52 | async def func():
53 | with asyncio.timeout():
53 | async with asyncio.timeout(delay=0.2):
| _____^
54 | | ...
| |___________^ ASYNC100
Expand All @@ -67,7 +67,7 @@ ASYNC100.py:53:5: ASYNC100 A `with asyncio.timeout(...):` context does not conta
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.
|
57 | async def func():
58 | with asyncio.timeout_at():
58 | async with asyncio.timeout_at(when=0.2):
| _____^
59 | | ...
| |___________^ ASYNC100
Expand Down

0 comments on commit d774a3b

Please sign in to comment.