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 a069277
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,45 +13,40 @@
pass
# trailing


with (
a # a
, # comma
b # c
): # colon
a # a
, # comma
b # c
): # colon
pass


with (
a # a
as # as
# own line
b # b
, # comma
c # c
): # colon
a # a
as # as
# own line
b # b
, # comma
c # c
): # colon
pass # body
# body trailing own

with (
a # a
as # as
# own line
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # b
a # a
as # as
# own line
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # b
): pass


with (a,): # magic trailing comma
with (a, ): # magic trailing comma
pass


with (a): # should remove brackets
pass

with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
pass


# currently unparsable by black: https://github.com/psf/black/issues/3678
with (name_2 for name_0 in name_4):
pass
Expand Down Expand Up @@ -87,21 +82,21 @@
): pass

with (
a # trailing same line comment
a # trailing same line comment
# trailing own line comment
as b
): pass

with (a # trailing same line comment
with (a # trailing same line comment
# trailing own line comment
) as b: pass

with (
(a
# trailing own line comment
# trailing own line comment
)
as # trailing as same line comment
b # trailing b same line comment
as # trailing as same line comment
b # trailing b same line comment
): pass

with (
Expand Down Expand Up @@ -304,6 +299,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 @@ -6,7 +6,6 @@
):
pass


# Black avoids parenthesizing the with because it can make all with items fit by just breaking
# around parentheses. We don't implement this optimisation because it makes it difficult to see where
# the different context managers start and end.
Expand Down Expand Up @@ -37,7 +36,6 @@
):
pass


# Black avoids parentheses here because it can make the entire with
# header fit without requiring parentheses to do so.
# We don't implement this optimisation because it very difficult to see where
Expand Down Expand Up @@ -66,7 +64,6 @@
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
pass


# Black parenthesizes this binary expression but also preserves the parentheses of the first with-item.
# It does so because it prefers splitting already parenthesized context managers, even if it leads to more parentheses
# like in this case.
Expand All @@ -85,4 +82,8 @@
with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c):
pass


with ( # outer comment
CtxManager1(),
CtxManager2(),
):
pass
20 changes: 13 additions & 7 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,20 @@ if True:
#[test]
fn quick_test() {
let source = r#"
def main() -> None:
if True:
some_very_long_variable_name_abcdefghijk = Foo()
some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
== "This is a very long string abcdefghijk"
]
with (
open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
)
):
pass
with open(
"/etc/hosts" # This is an incredibly long comment that has been replaced for sanitization
):
pass
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
pass
"#;
let source_type = PySourceType::Python;
let (tokens, comment_ranges) = tokens_and_ranges(source, source_type).unwrap();
Expand Down
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
22 changes: 21 additions & 1 deletion crates/ruff_python_formatter/src/statement/stmt_with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ impl FormatNodeRule<StmtWith> for FormatStmtWith {
.with_options(WithItemLayout::SingleParenthesizedContextManager)
.fmt(f),

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

WithItemsLayout::ParenthesizeIfExpands => {
parenthesize_if_expands(&format_with(|f| {
let mut joiner = f.join_comma_separated(
Expand Down Expand Up @@ -165,6 +170,19 @@ enum WithItemsLayout<'a> {
/// Only used for Python 3.9+
SingleWithTarget(&'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+
SingleWithoutTarget(&'a WithItem),

/// The target python version doesn't support parenthesized context managers because it is Python 3.8 or older.
///
/// In this case, never add parentheses and join the with items with spaces.
Expand Down Expand Up @@ -275,7 +293,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

0 comments on commit a069277

Please sign in to comment.