Skip to content

Commit

Permalink
Omit select context managers from SIM117 (#8606)
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
  • Loading branch information
maltevesper committed Nov 21, 2023
1 parent 213d315 commit 6fb0bf9
Show file tree
Hide file tree
Showing 3 changed files with 95 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 surpress combination, if with_item is alreayd combined with another item
async with asyncio.timeout(1), A():
async with B():
pass
43 changes: 43 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 @@ -16,6 +17,12 @@ use super::fix_with;
/// Checks for the unnecessary nesting of multiple consecutive context
/// managers.
///
/// The following context managers are exempt if used standalone:
///
/// - `anyio`.{`CancelScope`, `fail_after`, `move_on_after`}
/// - `asyncio`.{`timeout`, `timeout_at`}
/// - `trio`.{`fail_after`, `fail_at`, `move_on_after`, `move_on_at`}
///
/// ## Why is this bad?
/// In Python 3, a single `with` block can include multiple context
/// managers.
Expand Down Expand Up @@ -73,6 +80,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
///
/// async with asyncio.timeout(1): # timeout should stand out
/// with resource1(), resource2():
/// ...
fn explicit_with_items(checker: &mut Checker, with_items: &[WithItem]) -> bool {
match with_items {
[with_item] => match &with_item.context_expr {
Expr::Call(expr_call) => checker
.semantic()
.resolve_call_path(&expr_call.func)
.is_some_and(|call_path| {
matches!(
call_path.as_slice(),
["asyncio", "timeout"]
| ["asyncio", "timeout_at"]
| ["anyio", "CancelScope"]
| ["anyio", "fail_after"]
| ["anyio", "move_on_after"]
| ["trio", "fail_after"]
| ["trio", "fail_at"]
| ["trio", "move_on_after"]
| ["trio", "move_on_at"]
)
}),
_ => false,
},
_ => false,
}
}

/// SIM117
pub(crate) fn multiple_with_statements(
checker: &mut Checker,
Expand Down Expand Up @@ -111,6 +150,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 surpress combination, if with_item is alreayd combined with another item
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 surpress combination, if with_item is alreayd combined with another item
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 6fb0bf9

Please sign in to comment.