Skip to content

Commit

Permalink
[flake8-simplify] Omit select context managers from SIM117 (#8801)
Browse files Browse the repository at this point in the history
Semantically it makes sense to put certain contextmanagers into separate
with statements. Currently asyncio.timeout and its relatives in anyio
and trio are exempt from SIM117.

Closes #8606

## Summary

Exempt asyncio.timeout and related functions from SIM117 (Collapse with
statements where possible).
See #8606 for more.

## Test Plan

Extended the insta tests.
  • Loading branch information
maltevesper authored Nov 21, 2023
1 parent bf729e7 commit 6fb6478
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,32 @@ def method1(self) -> T:
f" something { my_dict["key"] } something else "

f"foo {f"bar {x}"} baz"

# Allow cascading for some statements.
import anyio
import asyncio
import trio

async with asyncio.timeout(1):
async with A() as a:
pass

async with A():
async with asyncio.timeout(1):
pass

async with asyncio.timeout(1):
async with asyncio.timeout_at(1):
async with anyio.CancelScope():
async with anyio.fail_after(1):
async with anyio.move_on_after(1):
async with trio.fail_after(1):
async with trio.fail_at(1):
async with trio.move_on_after(1):
async with trio.move_on_at(1):
pass

# Do not supress combination, if a context manager is already combined with another.
async with asyncio.timeout(1), A():
async with B():
pass
44 changes: 44 additions & 0 deletions crates/ruff_linter/src/rules/flake8_simplify/rules/ast_with.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ast::Expr;
use log::error;

use ruff_diagnostics::{Diagnostic, Fix};
Expand All @@ -24,6 +25,13 @@ use super::fix_with;
/// will minimize the indentation depth of the code, making it more
/// readable.
///
/// The following context managers are exempt when used as standalone
/// statements:
///
/// - `anyio`.{`CancelScope`, `fail_after`, `move_on_after`}
/// - `asyncio`.{`timeout`, `timeout_at`}
/// - `trio`.{`fail_after`, `fail_at`, `move_on_after`, `move_on_at`}
///
/// ## Example
/// ```python
/// with A() as a:
Expand Down Expand Up @@ -73,6 +81,38 @@ fn next_with(body: &[Stmt]) -> Option<(bool, &[WithItem], &[Stmt])> {
Some((*is_async, items, body))
}

/// Check if `with_items` contains a single item which should not necessarily be
/// grouped with other items.
///
/// For example:
/// ```python
/// async with asyncio.timeout(1):
/// with resource1(), resource2():
/// ...
/// ```
fn explicit_with_items(checker: &mut Checker, with_items: &[WithItem]) -> bool {
let [with_item] = with_items else {
return false;
};
let Expr::Call(expr_call) = &with_item.context_expr else {
return false;
};
checker
.semantic()
.resolve_call_path(&expr_call.func)
.is_some_and(|call_path| {
matches!(
call_path.as_slice(),
["asyncio", "timeout" | "timeout_at"]
| ["anyio", "CancelScope" | "fail_after" | "move_on_after"]
| [
"trio",
"fail_after" | "fail_at" | "move_on_after" | "move_on_at"
]
)
})
}

/// SIM117
pub(crate) fn multiple_with_statements(
checker: &mut Checker,
Expand Down Expand Up @@ -111,6 +151,10 @@ pub(crate) fn multiple_with_statements(
return;
}

if explicit_with_items(checker, &with_stmt.items) || explicit_with_items(checker, items) {
return;
}

let Some(colon) = items.last().and_then(|item| {
SimpleTokenizer::starts_at(item.end(), checker.locator().contents())
.skip_trivia()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,5 +316,28 @@ SIM117.py:126:1: SIM117 [*] Use a single `with` statement with multiple contexts
135 134 |
136 |- f"foo {f"bar {x}"} baz"
135 |+ f"foo {f"bar {x}"} baz"
137 136 |
138 137 | # Allow cascading for some statements.
139 138 | import anyio

SIM117.py:163:1: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements
|
162 | # Do not supress combination, if a context manager is already combined with another.
163 | / async with asyncio.timeout(1), A():
164 | | async with B():
| |___________________^ SIM117
165 | pass
|
= help: Combine `with` statements

Unsafe fix
160 160 | pass
161 161 |
162 162 | # Do not supress combination, if a context manager is already combined with another.
163 |-async with asyncio.timeout(1), A():
164 |- async with B():
165 |- pass
163 |+async with asyncio.timeout(1), A(), B():
164 |+ pass


0 comments on commit 6fb6478

Please sign in to comment.