Skip to content

Commit

Permalink
Fix stmt_with unstable formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Mar 8, 2024
1 parent 23778ac commit a42c365
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext():
pass

if True:
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext() as c:
pass

with Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Document(aaaaa, bbbbbbbbbb, ddddddddddddd):
pass

# Regression test for https://github.com/astral-sh/ruff/issues/10267
with (
open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
)
):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,8 @@
with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c):
pass


with ( # outer comment
CtxManager1(),
CtxManager2(),
):
pass
20 changes: 19 additions & 1 deletion crates/ruff_python_formatter/src/other/with_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ pub enum WithItemLayout {
/// This layout is used independent of the target version.
SingleParenthesizedContextManager,

/// A with item that is the `with`s only context manager and it has no `target`.
///
/// ```python
/// with a + b:
/// ...
/// ```
///
/// In this case, use [`maybe_parenthesize_expression`] to get the same formatting as when
/// formatting any other statement with a clause header.
///
/// This layout is only used for Python 3.9+.
///
/// Be careful that [`Self::SingleParenthesizedContextManager`] and this layout are compatible because
/// removing optional parentheses or adding parentheses will make the formatter pick the opposite layout
/// the second time the file gets formatted.
SingleWithoutTarget,

/// This layout is used when the target python version doesn't support parenthesized context managers and
/// it's either a single, unparenthesized with item or multiple items.
///
Expand Down Expand Up @@ -106,7 +123,8 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
}
}

WithItemLayout::SingleParenthesizedContextManager => {
WithItemLayout::SingleParenthesizedContextManager
| WithItemLayout::SingleWithoutTarget => {
write!(
f,
[maybe_parenthesize_expression(
Expand Down
33 changes: 30 additions & 3 deletions crates/ruff_python_formatter/src/statement/stmt_with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ impl FormatNodeRule<StmtWith> for FormatStmtWith {
optional_parentheses(&single.format()).fmt(f)
}

WithItemsLayout::SingleWithoutTarget(single) => single
.format()
.with_options(WithItemLayout::SingleWithoutTarget)
.fmt(f),

WithItemsLayout::SingleParenthesizedContextManager(single) => single
.format()
.with_options(WithItemLayout::SingleParenthesizedContextManager)
Expand Down Expand Up @@ -150,15 +155,35 @@ enum WithItemsLayout<'a> {
///
/// In this case, prefer keeping the parentheses around the context expression instead of parenthesizing the entire
/// with item.
///
/// Ensure that this layout is compatible with [`Self::SingleWithoutTarget`] because removing the parentheses
/// results in the formatter taking that layout when formatting the file again
SingleParenthesizedContextManager(&'a WithItem),

/// The with statement's only item has no target.
///
/// ```python
/// with a + b:
/// ...
/// ```
///
/// In this case, use [`maybe_parenthesize_expression`] to format the context expression
/// to get the exact same formatting as when formatting an expression in any other clause header.
///
/// Only used for Python 3.9+
///
/// Be careful that [`Self::SingleParenthesizedContextManager`] and this layout are compatible because
/// adding parentheses around a [`WithItem`] will result in the context expression being parenthesized in
/// the next formatting pass.
SingleWithoutTarget(&'a WithItem),

/// It's a single with item with a target. Use the optional parentheses layout (see [`optional_parentheses`])
/// to mimic the `maybe_parenthesize_expression` behavior.
///
/// ```python
/// with (
/// a + b
/// ) as b:
/// a + b as b
/// ):
/// ...
/// ```
///
Expand Down Expand Up @@ -275,7 +300,9 @@ impl<'a> WithItemsLayout<'a> {

Ok(match with.items.as_slice() {
[single] => {
if can_omit_optional_parentheses(&single.context_expr, context) {
if single.optional_vars.is_none() {
Self::SingleWithoutTarget(single)
} else if can_omit_optional_parentheses(&single.context_expr, context) {
Self::SingleWithTarget(single)
} else {
Self::ParenthesizeIfExpands
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,20 @@ if True:
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext():
pass
if True:
with anyio.CancelScope(shield=True) if get_running_loop() else contextlib.nullcontext() as c:
pass
with Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Document(aaaaa, bbbbbbbbbb, ddddddddddddd):
pass
# Regression test for https://github.com/astral-sh/ruff/issues/10267
with (
open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
)
):
pass
```

## Outputs
Expand Down Expand Up @@ -661,11 +672,22 @@ if True:
) if get_running_loop() else contextlib.nullcontext():
pass
if True:
with anyio.CancelScope(
shield=True
) if get_running_loop() else contextlib.nullcontext() as c:
pass
with Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Document(
aaaaa, bbbbbbbbbb, ddddddddddddd
):
pass
# Regression test for https://github.com/astral-sh/ruff/issues/10267
with open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
):
pass
```


Expand Down Expand Up @@ -1052,13 +1074,23 @@ if True:
):
pass
if True:
with (
anyio.CancelScope(shield=True)
if get_running_loop()
else contextlib.nullcontext() as c
):
pass
with (
Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc),
Document(aaaaa, bbbbbbbbbb, ddddddddddddd),
):
pass
```


# Regression test for https://github.com/astral-sh/ruff/issues/10267
with open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
):
pass
```
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ if True:
with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c):
pass
with ( # outer comment
CtxManager1(),
CtxManager2(),
):
pass
```

## Outputs
Expand Down Expand Up @@ -217,7 +221,10 @@ with (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c
):
pass
```


with ( # outer comment
CtxManager1(),
CtxManager2(),
):
pass
```

0 comments on commit a42c365

Please sign in to comment.