Don't "flatten" nested if expressions when formatting (#6996)

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/if.py

@@ -39,3 +39,72 @@ d1 = [
     ("b") else # 2
     ("c")
 ]
+
+e1 = (
+    a
+    if True # 1
+    else b
+    if False # 2
+    else c
+)
+
+
+# Flattening nested if-expressions.
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        (NamedValuesListIterable
+        if named
+        else FlatValuesListIterable)
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (FlatValuesListIterable
+        if flat
+        else ValuesListIterable)
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable(1,)
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable
+        if flat
+        else ValuesListIterable)
+    )

crates/ruff_python_formatter/src/expression/expr_if_exp.rs

@@ -1,16 +1,46 @@
-use ruff_formatter::{format_args, write};
+use ruff_formatter::{write, FormatRuleWithOptions};
 use ruff_python_ast::node::AnyNodeRef;
-use ruff_python_ast::ExprIfExp;
+use ruff_python_ast::{Expr, ExprIfExp};
 
 use crate::comments::leading_comments;
 use crate::expression::parentheses::{
-    in_parentheses_only_group, in_parentheses_only_soft_line_break_or_space, NeedsParentheses,
+    in_parentheses_only_group, in_parentheses_only_soft_line_break_or_space,
-    OptionalParentheses,
+    is_expression_parenthesized, NeedsParentheses, OptionalParentheses,
 };
 use crate::prelude::*;
 
+#[derive(Default, Copy, Clone)]
+pub enum ExprIfExpLayout {
+    #[default]
+    Default,
+
+    /// The [`ExprIfExp`] is nested inside another [`ExprIfExp`], so it should not be given a new
+    /// group. For example, avoid grouping the `else` clause in:
+    /// ```python
+    /// clone._iterable_class = (
+    ///     NamedValuesListIterable
+    ///     if named
+    ///     else FlatValuesListIterable
+    ///     if flat
+    ///     else ValuesListIterable
+    /// )
+    /// ```
+    Nested,
+}
+
 #[derive(Default)]
-pub struct FormatExprIfExp;
+pub struct FormatExprIfExp {
+    layout: ExprIfExpLayout,
+}
+
+impl FormatRuleWithOptions<ExprIfExp, PyFormatContext<'_>> for FormatExprIfExp {
+    type Options = ExprIfExpLayout;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.layout = options;
+        self
+    }
+}
 
 impl FormatNodeRule<ExprIfExp> for FormatExprIfExp {
     fn fmt_fields(&self, item: &ExprIfExp, f: &mut PyFormatter) -> FormatResult<()> {
@@ -22,25 +52,33 @@ impl FormatNodeRule<ExprIfExp> for FormatExprIfExp {
         } = item;
         let comments = f.context().comments().clone();
 
-        // We place `if test` and `else orelse` on a single line, so the `test` and `orelse` leading
+        let inner = format_with(|f: &mut PyFormatter| {
-        // comments go on the line before the `if` or `else` instead of directly ahead `test` or
+            // We place `if test` and `else orelse` on a single line, so the `test` and `orelse` leading
-        // `orelse`
+            // comments go on the line before the `if` or `else` instead of directly ahead `test` or
-        write!(
+            // `orelse`
-            f,
+            write!(
-            [in_parentheses_only_group(&format_args![
+                f,
-                body.format(),
+                [
-                in_parentheses_only_soft_line_break_or_space(),
+                    body.format(),
-                leading_comments(comments.leading(test.as_ref())),
+                    in_parentheses_only_soft_line_break_or_space(),
-                text("if"),
+                    leading_comments(comments.leading(test.as_ref())),
-                space(),
+                    text("if"),
-                test.format(),
+                    space(),
-                in_parentheses_only_soft_line_break_or_space(),
+                    test.format(),
-                leading_comments(comments.leading(orelse.as_ref())),
+                    in_parentheses_only_soft_line_break_or_space(),
-                text("else"),
+                    leading_comments(comments.leading(orelse.as_ref())),
-                space(),
+                    text("else"),
-                orelse.format()
+                    space(),
-            ])]
+                ]
-        )
+            )?;
+
+            FormatOrElse { orelse }.fmt(f)
+        });
+
+        match self.layout {
+            ExprIfExpLayout::Default => in_parentheses_only_group(&inner).fmt(f),
+            ExprIfExpLayout::Nested => inner.fmt(f),
+        }
     }
 }
 
@@ -53,3 +91,21 @@ impl NeedsParentheses for ExprIfExp {
         OptionalParentheses::Multiline
     }
 }
+
+#[derive(Debug)]
+struct FormatOrElse<'a> {
+    orelse: &'a Expr,
+}
+
+impl Format<PyFormatContext<'_>> for FormatOrElse<'_> {
+    fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
+        match self.orelse {
+            Expr::IfExp(expr)
+                if !is_expression_parenthesized(expr.into(), f.context().source()) =>
+            {
+                write!(f, [expr.format().with_options(ExprIfExpLayout::Nested)])
+            }
+            _ => write!(f, [in_parentheses_only_group(&self.orelse.format())]),
+        }
+    }
+}

crates/ruff_python_formatter/tests/snapshots/black_compatibility@conditional_expression.py.snap

@@ -136,6 +136,15 @@ def something():
      for some_boolean_variable in some_iterable
  )
  
+@@ -86,5 +78,7 @@
+     clone._iterable_class = (
+         NamedValuesListIterable
+         if named
+-        else FlatValuesListIterable if flat else ValuesListIterable
++        else FlatValuesListIterable
++        if flat
++        else ValuesListIterable
+     )
 ```
 
 ## Ruff Output
@@ -221,7 +230,9 @@ def something():
     clone._iterable_class = (
         NamedValuesListIterable
         if named
-        else FlatValuesListIterable if flat else ValuesListIterable
+        else FlatValuesListIterable
+        if flat
+        else ValuesListIterable
     )
 ```
 

crates/ruff_python_formatter/tests/snapshots/format@expression__if.py.snap

@@ -45,6 +45,75 @@ d1 = [
     ("b") else # 2
     ("c")
 ]
+
+e1 = (
+    a
+    if True # 1
+    else b
+    if False # 2
+    else c
+)
+
+
+# Flattening nested if-expressions.
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        (NamedValuesListIterable
+        if named
+        else FlatValuesListIterable)
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (FlatValuesListIterable
+        if flat
+        else ValuesListIterable)
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable(1,)
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable + FlatValuesListIterable
+        if flat
+        else ValuesListIterable)
+    )
 ```
 
 ## Output
@@ -96,6 +165,81 @@ d1 = [
     # 2
     else ("c")
 ]
+
+e1 = (
+    a
+    if True  # 1
+    else b
+    if False  # 2
+    else c
+)
+
+
+# Flattening nested if-expressions.
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        (NamedValuesListIterable if named else FlatValuesListIterable)
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (FlatValuesListIterable if flat else ValuesListIterable)
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable(
+            1,
+        )
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else FlatValuesListIterable
+        + FlatValuesListIterable
+        + FlatValuesListIterable
+        + FlatValuesListIterable
+        if flat
+        else ValuesListIterable
+    )
+
+
+def something():
+    clone._iterable_class = (
+        NamedValuesListIterable
+        if named
+        else (
+            FlatValuesListIterable
+            + FlatValuesListIterable
+            + FlatValuesListIterable
+            + FlatValuesListIterable
+            if flat
+            else ValuesListIterable
+        )
+    )
 ```