Fix unstable with-items formatting

Author: MichaReiser

crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py

@@ -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
@@ -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 (
@@ -304,6 +299,9 @@
     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

crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with_39.py

@@ -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.
@@ -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
@@ -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.
@@ -85,4 +82,8 @@
 with (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c):
     pass
 
-
+with (  # outer comment
+    CtxManager1(),
+    CtxManager2(),
+):
+    pass

crates/ruff_python_formatter/src/builders.rs

@@ -219,7 +219,6 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
             if let Some(last_end) = self.entries.position() {
                 let magic_trailing_comma = has_magic_trailing_comma(
                     TextRange::new(last_end, self.sequence_end),
-                    self.fmt.options(),
                     self.fmt.context(),
                 );
 

crates/ruff_python_formatter/src/lib.rs

@@ -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();

crates/ruff_python_formatter/src/other/arguments.rs

@@ -205,11 +205,7 @@ fn is_arguments_huggable(arguments: &Arguments, context: &PyFormatContext) -> bo
 
     // If the expression has a trailing comma, then we can't hug it.
     if options.magic_trailing_comma().is_respect()
-        && commas::has_magic_trailing_comma(
-            TextRange::new(arg.end(), arguments.end()),
-            options,
-            context,
-        )
+        && commas::has_magic_trailing_comma(TextRange::new(arg.end(), arguments.end()), context)
     {
         return false;
     }

crates/ruff_python_formatter/src/other/commas.rs

@@ -1,17 +1,14 @@
+use ruff_formatter::FormatContext;
 use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
 use ruff_text_size::TextRange;
 
 use crate::prelude::*;
-use crate::{MagicTrailingComma, PyFormatOptions};
+use crate::MagicTrailingComma;
 
 /// Returns `true` if the range ends with a magic trailing comma (and the magic trailing comma
 /// should be respected).
-pub(crate) fn has_magic_trailing_comma(
-    range: TextRange,
-    options: &PyFormatOptions,
-    context: &PyFormatContext,
-) -> bool {
-    match options.magic_trailing_comma() {
+pub(crate) fn has_magic_trailing_comma(range: TextRange, context: &PyFormatContext) -> bool {
+    match context.options().magic_trailing_comma() {
         MagicTrailingComma::Respect => {
             let first_token = SimpleTokenizer::new(context.source(), range)
                 .skip_trivia()

crates/ruff_python_formatter/src/other/with_item.rs

@@ -1,4 +1,4 @@
-use ruff_formatter::write;
+use ruff_formatter::{write, FormatRuleWithOptions};
 use ruff_python_ast::WithItem;
 
 use crate::comments::SourceComment;
@@ -8,8 +8,57 @@ use crate::expression::parentheses::{
 };
 use crate::prelude::*;
 
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
+pub enum WithItemLayout {
+    /// A with item that is the `with`s only context manager and its context expression is parenthesized.
+    ///
+    /// ```python
+    /// with (
+    ///     a + b
+    /// ) as b:
+    ///     ...
+    /// ```
+    ///
+    /// This layout is used independent of the target version.
+    SingleParenthesizedContextExpression,
+
+    /// This layout is used when the target python version doesn't support parenthesized context managers.
+    Python38OrOlder,
+
+    /// A with item where the `with` formatting adds parentheses around all context managers if necessary.
+    ///
+    /// ```python
+    /// with (
+    ///     a,
+    ///     b,
+    /// ): pass
+    /// ```
+    ///
+    /// This layout is generally used when the target version is Python 3.9 or newer but it is used
+    /// for Python 3.8 if the with item has a leading or trailing comment.
+    ///
+    /// ```python
+    /// with (
+    ///     # leading
+    ///     a
+    // ): ...
+    /// ```
+    #[default]
+    ParenthesizedContextManagers,
+}
+
 #[derive(Default)]
-pub struct FormatWithItem;
+pub struct FormatWithItem {
+    layout: WithItemLayout,
+}
+
+impl FormatRuleWithOptions<WithItem, PyFormatContext<'_>> for FormatWithItem {
+    type Options = WithItemLayout;
+
+    fn with_options(self, options: Self::Options) -> Self {
+        Self { layout: options }
+    }
+}
 
 impl FormatNodeRule<WithItem> for FormatWithItem {
     fn fmt_fields(&self, item: &WithItem, f: &mut PyFormatter) -> FormatResult<()> {
@@ -28,40 +77,52 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
             f.context().source(),
         );
 
-        // Remove the parentheses of the `with_items` if the with statement adds parentheses
-        if f.context().node_level().is_parenthesized() {
-            if is_parenthesized {
-                // ...except if the with item is parenthesized, then use this with item as a preferred breaking point
-                // or when it has comments, then parenthesize it to prevent comments from moving.
-                maybe_parenthesize_expression(
-                    context_expr,
-                    item,
-                    Parenthesize::IfBreaksOrIfRequired,
-                )
-                .fmt(f)?;
-            } else {
-                context_expr
-                    .format()
-                    .with_options(Parentheses::Never)
+        match self.layout {
+            // Remove the parentheses of the `with_items` if the with statement adds parentheses
+            WithItemLayout::ParenthesizedContextManagers => {
+                if is_parenthesized {
+                    // ...except if the with item is parenthesized, then use this with item as a preferred breaking point
+                    // or when it has comments, then parenthesize it to prevent comments from moving.
+                    maybe_parenthesize_expression(
+                        context_expr,
+                        item,
+                        Parenthesize::IfBreaksOrIfRequired,
+                    )
                     .fmt(f)?;
+                } else {
+                    context_expr
+                        .format()
+                        .with_options(Parentheses::Never)
+                        .fmt(f)?;
+                }
+            }
+
+            WithItemLayout::SingleParenthesizedContextExpression => {
+                write!(
+                    f,
+                    [maybe_parenthesize_expression(
+                        context_expr,
+                        item,
+                        Parenthesize::IfBreaks
+                    )]
+                )?;
+            }
+
+            WithItemLayout::Python38OrOlder => {
+                let parenthesize = if is_parenthesized {
+                    Parenthesize::IfBreaks
+                } else {
+                    Parenthesize::IfRequired
+                };
+                write!(
+                    f,
+                    [maybe_parenthesize_expression(
+                        context_expr,
+                        item,
+                        parenthesize
+                    )]
+                )?;
             }
-        } else {
-            // Prefer keeping parentheses for already parenthesized expressions over
-            // parenthesizing other nodes.
-            let parenthesize = if is_parenthesized {
-                Parenthesize::IfBreaks
-            } else {
-                Parenthesize::IfRequired
-            };
-
-            write!(
-                f,
-                [maybe_parenthesize_expression(
-                    context_expr,
-                    item,
-                    parenthesize
-                )]
-            )?;
         }
 
         if let Some(optional_vars) = optional_vars {