Fix unnecessary space around power op in overlong f-string expressions (#14489)

crates/ruff_formatter/src/buffer.rs

@@ -1,10 +1,11 @@
 use super::{write, Arguments, FormatElement};
 use crate::format_element::Interned;
-use crate::prelude::LineMode;
+use crate::prelude::{LineMode, Tag};
 use crate::{FormatResult, FormatState};
 use rustc_hash::FxHashMap;
 use std::any::{Any, TypeId};
 use std::fmt::Debug;
+use std::num::NonZeroUsize;
 use std::ops::{Deref, DerefMut};
 
 /// A trait for writing or formatting into [`FormatElement`]-accepting buffers or streams.
@@ -294,10 +295,11 @@ where
     }
 }
 
-/// A Buffer that removes any soft line breaks.
+/// A Buffer that removes any soft line breaks or [`if_group_breaks`](crate::builders::if_group_breaks) elements.
 ///
 /// - Removes [`lines`](FormatElement::Line) with the mode [`Soft`](LineMode::Soft).
 /// - Replaces [`lines`](FormatElement::Line) with the mode [`Soft`](LineMode::SoftOrSpace) with a [`Space`](FormatElement::Space)
+/// - Removes [`if_group_breaks`](crate::builders::if_group_breaks) elements.
 ///
 /// # Examples
 ///
@@ -350,6 +352,8 @@ pub struct RemoveSoftLinesBuffer<'a, Context> {
     /// It's fine to not snapshot the cache. The worst that can happen is that it holds on interned elements
     /// that are now unused. But there's little harm in that and the cache is cleaned when dropping the buffer.
     interned_cache: FxHashMap<Interned, Interned>,
+
+    state: RemoveSoftLineBreaksState,
 }
 
 impl<'a, Context> RemoveSoftLinesBuffer<'a, Context> {
@@ -357,6 +361,7 @@ impl<'a, Context> RemoveSoftLinesBuffer<'a, Context> {
     pub fn new(inner: &'a mut dyn Buffer<Context = Context>) -> Self {
         Self {
             inner,
+            state: RemoveSoftLineBreaksState::default(),
             interned_cache: FxHashMap::default(),
         }
     }
@@ -375,6 +380,8 @@ fn clean_interned(
     if let Some(cleaned) = interned_cache.get(interned) {
         cleaned.clone()
     } else {
+        let mut state = RemoveSoftLineBreaksState::default();
+
         // Find the first soft line break element or interned element that must be changed
         let result = interned
             .iter()
@@ -382,8 +389,9 @@ fn clean_interned(
             .find_map(|(index, element)| match element {
                 FormatElement::Line(LineMode::Soft | LineMode::SoftOrSpace) => {
                     let mut cleaned = Vec::new();
-                    cleaned.extend_from_slice(&interned[..index]);
+                    let (before, after) = interned.split_at(index);
-                    Some((cleaned, &interned[index..]))
+                    cleaned.extend_from_slice(before);
+                    Some((cleaned, &after[1..]))
                 }
                 FormatElement::Interned(inner) => {
                     let cleaned_inner = clean_interned(inner, interned_cache);
@@ -398,19 +406,33 @@ fn clean_interned(
                     }
                 }
 
-                _ => None,
+                element => {
+                    if state.should_drop(element) {
+                        let mut cleaned = Vec::new();
+                        let (before, after) = interned.split_at(index);
+                        cleaned.extend_from_slice(before);
+                        Some((cleaned, &after[1..]))
+                    } else {
+                        None
+                    }
+                }
             });
 
         let result = match result {
             // Copy the whole interned buffer so that becomes possible to change the necessary elements.
             Some((mut cleaned, rest)) => {
                 for element in rest {
+                    if state.should_drop(element) {
+                        continue;
+                    }
+
                     let element = match element {
                         FormatElement::Line(LineMode::Soft) => continue,
                         FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space,
                         FormatElement::Interned(interned) => {
                             FormatElement::Interned(clean_interned(interned, interned_cache))
                         }
+
                         element => element.clone(),
                     };
                     cleaned.push(element);
@@ -431,12 +453,17 @@ impl<Context> Buffer for RemoveSoftLinesBuffer<'_, Context> {
     type Context = Context;
 
     fn write_element(&mut self, element: FormatElement) {
+        if self.state.should_drop(&element) {
+            return;
+        }
+
         let element = match element {
             FormatElement::Line(LineMode::Soft) => return,
             FormatElement::Line(LineMode::SoftOrSpace) => FormatElement::Space,
             FormatElement::Interned(interned) => {
                 FormatElement::Interned(self.clean_interned(&interned))
             }
+
             element => element,
         };
 
@@ -456,14 +483,77 @@ impl<Context> Buffer for RemoveSoftLinesBuffer<'_, Context> {
     }
 
     fn snapshot(&self) -> BufferSnapshot {
-        self.inner.snapshot()
+        BufferSnapshot::Any(Box::new(RemoveSoftLinebreaksSnapshot {
+            inner: self.inner.snapshot(),
+            state: self.state,
+        }))
     }
 
     fn restore_snapshot(&mut self, snapshot: BufferSnapshot) {
-        self.inner.restore_snapshot(snapshot);
+        let RemoveSoftLinebreaksSnapshot { inner, state } = snapshot.unwrap_any();
+        self.inner.restore_snapshot(inner);
+        self.state = state;
     }
 }
 
+#[derive(Copy, Clone, Debug, Default)]
+enum RemoveSoftLineBreaksState {
+    #[default]
+    Default,
+    InIfGroupBreaks {
+        conditional_content_level: NonZeroUsize,
+    },
+}
+
+impl RemoveSoftLineBreaksState {
+    fn should_drop(&mut self, element: &FormatElement) -> bool {
+        match self {
+            Self::Default => {
+                // Entered the start of an `if_group_breaks`
+                if let FormatElement::Tag(Tag::StartConditionalContent(condition)) = element {
+                    if condition.mode.is_expanded() {
+                        *self = Self::InIfGroupBreaks {
+                            conditional_content_level: NonZeroUsize::new(1).unwrap(),
+                        };
+                        return true;
+                    }
+                }
+
+                false
+            }
+            Self::InIfGroupBreaks {
+                conditional_content_level,
+            } => {
+                match element {
+                    // A nested `if_group_breaks` or `if_group_fits`
+                    FormatElement::Tag(Tag::StartConditionalContent(_)) => {
+                        *conditional_content_level = conditional_content_level.saturating_add(1);
+                    }
+                    // The end of an `if_group_breaks` or `if_group_fits`.
+                    FormatElement::Tag(Tag::EndConditionalContent) => {
+                        if let Some(level) = NonZeroUsize::new(conditional_content_level.get() - 1)
+                        {
+                            *conditional_content_level = level;
+                        } else {
+                            // Found the end tag of the initial `if_group_breaks`. Skip this element but retain
+                            // the elements coming after
+                            *self = RemoveSoftLineBreaksState::Default;
+                        }
+                    }
+                    _ => {}
+                }
+
+                true
+            }
+        }
+    }
+}
+
+struct RemoveSoftLinebreaksSnapshot {
+    inner: BufferSnapshot,
+    state: RemoveSoftLineBreaksState,
+}
+
 pub trait BufferExtensions: Buffer + Sized {
     /// Returns a new buffer that calls the passed inspector for every element that gets written to the output
     #[must_use]

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

@@ -346,3 +346,6 @@ _ = (
     f'This string uses double quotes in an expression {"it's a quote"}'
     f'This f-string does not use any quotes.'
 )
+
+# Regression test for https://github.com/astral-sh/ruff/issues/14487
+f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc"

crates/ruff_python_formatter/src/builders.rs

@@ -1,7 +1,7 @@
 use ruff_formatter::{write, Argument, Arguments};
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
-use crate::context::{FStringState, NodeLevel, WithNodeLevel};
+use crate::context::{NodeLevel, WithNodeLevel};
 use crate::other::commas::has_magic_trailing_comma;
 use crate::prelude::*;
 
@@ -206,16 +206,6 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
 
     pub(crate) fn finish(&mut self) -> FormatResult<()> {
         self.result.and_then(|()| {
-            // If the formatter is inside an f-string expression element, and the layout
-            // is flat, then we don't need to add a trailing comma.
-            if let FStringState::InsideExpressionElement(context) =
-                self.fmt.context().f_string_state()
-            {
-                if !context.can_contain_line_breaks() {
-                    return Ok(());
-                }
-            }
-
             if let Some(last_end) = self.entries.position() {
                 let magic_trailing_comma = has_magic_trailing_comma(
                     TextRange::new(last_end, self.sequence_end),

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

@@ -353,6 +353,9 @@ _ = (
     f'This string uses double quotes in an expression {"it's a quote"}'
     f'This f-string does not use any quotes.'
 )
+
+# Regression test for https://github.com/astral-sh/ruff/issues/14487
+f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc"
 ```
 
 ## Outputs
@@ -728,6 +731,9 @@ _ = (
     f"This string uses double quotes in an expression {"it's a quote"}"
     f"This f-string does not use any quotes."
 )
+
+# Regression test for https://github.com/astral-sh/ruff/issues/14487
+f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc"
 ```
 
 
@@ -1091,6 +1097,9 @@ _ = (
     f'This string uses double quotes in an expression {"it's a quote"}'
     f'This f-string does not use any quotes.'
 )
+
+# Regression test for https://github.com/astral-sh/ruff/issues/14487
+f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccccccccccccc"
 ```
 
 
@@ -1444,7 +1453,7 @@ _ = (
      # comment 27
      # comment 28
  } woah {x}"
-@@ -318,27 +330,27 @@
+@@ -318,29 +330,29 @@
          if indent2:
              foo = f"""hello world
  hello {
@@ -1490,4 +1499,6 @@ _ = (
 +    f"This string uses double quotes in an expression {"it's a quote"}"
 +    f"This f-string does not use any quotes."
  )
+ 
+ # Regression test for https://github.com/astral-sh/ruff/issues/14487
 ```