From 4100b9f0080f14ddeffd43d9587d496633b17fe2 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 8 Mar 2024 11:43:33 +0100 Subject: [PATCH] Fix stmt_with unstable formatting --- .../test/fixtures/ruff/statement/with.py | 11 +++ .../test/fixtures/ruff/statement/with_39.py | 6 +- .../src/other/with_item.rs | 20 +++- .../src/statement/stmt_with.rs | 33 ++++++- .../snapshots/format@statement__with.py.snap | 95 +++++++++++-------- .../format@statement__with_39.py.snap | 21 ++-- 6 files changed, 132 insertions(+), 54 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py index b222747733fe96..5b92ed3f7f766c 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py @@ -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 diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with_39.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with_39.py index e1b2da74e4ea01..0e6e384fa3b092 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with_39.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with_39.py @@ -85,4 +85,8 @@ with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c): pass - +with ( # outer comment + CtxManager1(), + CtxManager2(), +): + pass diff --git a/crates/ruff_python_formatter/src/other/with_item.rs b/crates/ruff_python_formatter/src/other/with_item.rs index 38088e3b78cda6..fdd4cb54c855d7 100644 --- a/crates/ruff_python_formatter/src/other/with_item.rs +++ b/crates/ruff_python_formatter/src/other/with_item.rs @@ -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. /// @@ -106,7 +123,8 @@ impl FormatNodeRule for FormatWithItem { } } - WithItemLayout::SingleParenthesizedContextManager => { + WithItemLayout::SingleParenthesizedContextManager + | WithItemLayout::SingleWithoutTarget => { write!( f, [maybe_parenthesize_expression( diff --git a/crates/ruff_python_formatter/src/statement/stmt_with.rs b/crates/ruff_python_formatter/src/statement/stmt_with.rs index 696e7a0e0acbe6..00d2fe31c4602f 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_with.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_with.rs @@ -73,6 +73,11 @@ impl FormatNodeRule 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) @@ -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 + /// ): /// ... /// ``` /// @@ -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 diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap index fb2a0b081c264b..da5ab43f4d9434 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap @@ -19,45 +19,40 @@ with ( 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 @@ -93,21 +88,21 @@ with ( ): 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 ( @@ -310,9 +305,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 @@ -347,14 +353,12 @@ with ( pass # trailing - with ( a, # a # comma b, # c ): # colon pass - with ( a as ( # a # as # own line @@ -373,20 +377,17 @@ with ( ): pass - 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 @@ -661,11 +662,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 ``` @@ -703,14 +715,12 @@ with ( pass # trailing - with ( a, # a # comma b, # c ): # colon pass - with ( a as ( # a # as # own line @@ -729,13 +739,11 @@ with ( ): pass - with ( a, ): # magic trailing comma pass - with a: # should remove brackets pass @@ -744,7 +752,6 @@ with ( ): pass - # currently unparsable by black: https://github.com/psf/black/issues/3678 with (name_2 for name_0 in name_4): pass @@ -1052,13 +1059,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 +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__with_39.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__with_39.py.snap index 5319f46100e147..53a842524dc983 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__with_39.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__with_39.py.snap @@ -12,7 +12,6 @@ if True: ): 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. @@ -43,7 +42,6 @@ if True: ): 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 @@ -72,7 +70,6 @@ with xxxxxxxx.some_kind_of_method( 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. @@ -91,7 +88,11 @@ if True: with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c): pass - +with ( # outer comment + CtxManager1(), + CtxManager2(), +): + pass ``` ## Outputs @@ -119,7 +120,6 @@ if True: ): 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. @@ -156,7 +156,6 @@ if True: ): 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 @@ -193,7 +192,6 @@ with ( ): 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. @@ -217,7 +215,10 @@ with ( aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c ): pass -``` - - +with ( # outer comment + CtxManager1(), + CtxManager2(), +): + pass +```