format ExprJoinedStr (#5932)

Author: davidszotten

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

@@ -0,0 +1,7 @@
+(
+    f'{one}'
+    f'{two}'
+)
+
+
+rf"Not-so-tricky \"quote"

crates/ruff_python_formatter/src/expression/expr_constant.rs

@@ -7,7 +7,9 @@ use ruff_python_ast::str::is_implicit_concatenation;
 
 use crate::expression::number::{FormatComplex, FormatFloat, FormatInt};
 use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
-use crate::expression::string::{FormatString, StringLayout, StringPrefix, StringQuotes};
+use crate::expression::string::{
+    AnyString, FormatString, StringLayout, StringPrefix, StringQuotes,
+};
 use crate::prelude::*;
 use crate::FormatNodeRule;
 
@@ -56,7 +58,9 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
                     ExprConstantLayout::Default => StringLayout::Default,
                     ExprConstantLayout::String(layout) => layout,
                 };
-                FormatString::new(item).with_layout(string_layout).fmt(f)
+                FormatString::new(&AnyString::Constant(item))
+                    .with_layout(string_layout)
+                    .fmt(f)
             }
         }
     }

crates/ruff_python_formatter/src/expression/expr_joined_str.rs

@@ -1,21 +1,18 @@
+use super::string::{AnyString, FormatString};
 use crate::context::PyFormatContext;
 use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
-use crate::{not_yet_implemented_custom_text, FormatNodeRule, PyFormatter};
-use ruff_formatter::{write, Buffer, FormatResult};
+use crate::prelude::*;
+use crate::{FormatNodeRule, PyFormatter};
+use ruff_formatter::FormatResult;
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::ExprJoinedStr;
 
 #[derive(Default)]
 pub struct FormatExprJoinedStr;
 
 impl FormatNodeRule<ExprJoinedStr> for FormatExprJoinedStr {
-    fn fmt_fields(&self, _item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
-        write!(
-            f,
-            [not_yet_implemented_custom_text(
-                r#"f"NOT_YET_IMPLEMENTED_ExprJoinedStr""#
-            )]
-        )
+    fn fmt_fields(&self, item: &ExprJoinedStr, f: &mut PyFormatter) -> FormatResult<()> {
+        FormatString::new(&AnyString::JoinedStr(item)).fmt(f)
     }
 }
 

crates/ruff_python_formatter/src/expression/string.rs

@@ -1,9 +1,11 @@
 use std::borrow::Cow;
 
 use bitflags::bitflags;
-use ruff_python_ast::{ExprConstant, Ranged};
+use ruff_python_ast::node::AnyNodeRef;
+use ruff_python_ast::{self as ast, ExprConstant, ExprJoinedStr, Ranged};
 use ruff_python_parser::lexer::{lex_starts_at, LexicalError, LexicalErrorType};
 use ruff_python_parser::{Mode, Tok};
+use ruff_source_file::Locator;
 use ruff_text_size::{TextLen, TextRange, TextSize};
 
 use ruff_formatter::{format_args, write, FormatError};
@@ -13,11 +15,62 @@ use crate::comments::{leading_comments, trailing_comments};
 use crate::expression::parentheses::{
     in_parentheses_only_group, in_parentheses_only_soft_line_break_or_space,
 };
+use crate::expression::Expr;
 use crate::prelude::*;
 use crate::QuoteStyle;
 
+#[derive(Copy, Clone)]
+enum Quoting {
+    CanChange,
+    Preserve,
+}
+
+pub(super) enum AnyString<'a> {
+    Constant(&'a ExprConstant),
+    JoinedStr(&'a ExprJoinedStr),
+}
+
+impl<'a> AnyString<'a> {
+    fn quoting(&self, locator: &Locator) -> Quoting {
+        match self {
+            Self::Constant(_) => Quoting::CanChange,
+            Self::JoinedStr(joined_str) => {
+                if joined_str.values.iter().any(|value| match value {
+                    Expr::FormattedValue(ast::ExprFormattedValue { range, .. }) => {
+                        let string_content = locator.slice(*range);
+                        string_content.contains(['"', '\''])
+                    }
+                    _ => false,
+                }) {
+                    Quoting::Preserve
+                } else {
+                    Quoting::CanChange
+                }
+            }
+        }
+    }
+}
+
+impl Ranged for AnyString<'_> {
+    fn range(&self) -> TextRange {
+        match self {
+            Self::Constant(expr) => expr.range(),
+            Self::JoinedStr(expr) => expr.range(),
+        }
+    }
+}
+
+impl<'a> From<&AnyString<'a>> for AnyNodeRef<'a> {
+    fn from(value: &AnyString<'a>) -> Self {
+        match value {
+            AnyString::Constant(expr) => AnyNodeRef::ExprConstant(expr),
+            AnyString::JoinedStr(expr) => AnyNodeRef::ExprJoinedStr(expr),
+        }
+    }
+}
+
 pub(super) struct FormatString<'a> {
-    constant: &'a ExprConstant,
+    string: &'a AnyString<'a>,
     layout: StringLayout,
 }
 
@@ -30,10 +83,12 @@ pub enum StringLayout {
 }
 
 impl<'a> FormatString<'a> {
-    pub(super) fn new(constant: &'a ExprConstant) -> Self {
-        debug_assert!(constant.value.is_str() || constant.value.is_bytes());
+    pub(super) fn new(string: &'a AnyString) -> Self {
+        if let AnyString::Constant(constant) = string {
+            debug_assert!(constant.value.is_str() || constant.value.is_bytes());
+        }
         Self {
-            constant,
+            string,
             layout: StringLayout::Default,
         }
     }
@@ -48,40 +103,43 @@ impl<'a> Format<PyFormatContext<'_>> for FormatString<'a> {
     fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
         match self.layout {
             StringLayout::Default => {
-                let string_range = self.constant.range();
+                let string_range = self.string.range();
                 let string_content = f.context().locator().slice(string_range);
 
                 if is_implicit_concatenation(string_content) {
-                    in_parentheses_only_group(&FormatStringContinuation::new(self.constant)).fmt(f)
+                    in_parentheses_only_group(&FormatStringContinuation::new(self.string)).fmt(f)
                 } else {
-                    FormatStringPart::new(string_range).fmt(f)
+                    FormatStringPart::new(string_range, self.string.quoting(&f.context().locator()))
+                        .fmt(f)
                 }
             }
             StringLayout::ImplicitConcatenatedBinaryLeftSide => {
-                FormatStringContinuation::new(self.constant).fmt(f)
+                FormatStringContinuation::new(self.string).fmt(f)
             }
         }
     }
 }
 
 struct FormatStringContinuation<'a> {
-    constant: &'a ExprConstant,
+    string: &'a AnyString<'a>,
 }
 
 impl<'a> FormatStringContinuation<'a> {
-    fn new(constant: &'a ExprConstant) -> Self {
-        debug_assert!(constant.value.is_str() || constant.value.is_bytes());
-        Self { constant }
+    fn new(string: &'a AnyString<'a>) -> Self {
+        if let AnyString::Constant(constant) = string {
+            debug_assert!(constant.value.is_str() || constant.value.is_bytes());
+        }
+        Self { string }
     }
 }
 
 impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {
     fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
         let comments = f.context().comments().clone();
         let locator = f.context().locator();
-        let mut dangling_comments = comments.dangling_comments(self.constant);
+        let mut dangling_comments = comments.dangling_comments(self.string);
 
-        let string_range = self.constant.range();
+        let string_range = self.string.range();
         let string_content = locator.slice(string_range);
 
         // The AST parses implicit concatenation as a single string.
@@ -155,7 +213,7 @@ impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {
                     joiner.entry(&format_args![
                         line_suffix_boundary(),
                         leading_comments(leading_part_comments),
-                        FormatStringPart::new(token_range),
+                        FormatStringPart::new(token_range, self.string.quoting(&locator)),
                         trailing_comments(trailing_part_comments)
                     ]);
 
@@ -178,11 +236,15 @@ impl Format<PyFormatContext<'_>> for FormatStringContinuation<'_> {
 
 struct FormatStringPart {
     part_range: TextRange,
+    quoting: Quoting,
 }
 
 impl FormatStringPart {
-    const fn new(range: TextRange) -> Self {
-        Self { part_range: range }
+    const fn new(range: TextRange, quoting: Quoting) -> Self {
+        Self {
+            part_range: range,
+            quoting,
+        }
     }
 }
 
@@ -204,10 +266,15 @@ impl Format<PyFormatContext<'_>> for FormatStringPart {
 
         let raw_content = &string_content[relative_raw_content_range];
         let is_raw_string = prefix.is_raw_string();
-        let preferred_quotes = if is_raw_string {
-            preferred_quotes_raw(raw_content, quotes, f.options().quote_style())
-        } else {
-            preferred_quotes(raw_content, quotes, f.options().quote_style())
+        let preferred_quotes = match self.quoting {
+            Quoting::Preserve => quotes,
+            Quoting::CanChange => {
+                if is_raw_string {
+                    preferred_quotes_raw(raw_content, quotes, f.options().quote_style())
+                } else {
+                    preferred_quotes(raw_content, quotes, f.options().quote_style())
+                }
+            }
         };
 
         write!(f, [prefix, preferred_quotes])?;

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

@@ -44,7 +44,7 @@ class DebugVisitor(Visitor[T]):
 ```diff
 --- Black
 +++ Ruff
-@@ -3,24 +3,29 @@
+@@ -3,24 +3,24 @@
      tree_depth: int = 0
  
      def visit_default(self, node: LN) -> Iterator[T]:
@@ -53,30 +53,25 @@ class DebugVisitor(Visitor[T]):
          if isinstance(node, Node):
              _type = type_repr(node.type)
 -            out(f'{indent}{_type}', fg='yellow')
-+            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
++            out(f"{indent}{_type}", fg="yellow")
              self.tree_depth += 1
              for child in node.children:
                  yield from self.visit(child)
  
              self.tree_depth -= 1
 -            out(f'{indent}/{_type}', fg='yellow', bold=False)
-+            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
++            out(f"{indent}/{_type}", fg="yellow", bold=False)
          else:
              _type = token.tok_name.get(node.type, str(node.type))
 -            out(f'{indent}{_type}', fg='blue', nl=False)
-+            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
++            out(f"{indent}{_type}", fg="blue", nl=False)
              if node.prefix:
                  # We don't have to handle prefixes for `Node` objects since
                  # that delegates to the first child anyway.
 -                out(f' {node.prefix!r}', fg='green', bold=False, nl=False)
 -            out(f' {node.value!r}', fg='blue', bold=False)
-+                out(
-+                    f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
-+                    fg="green",
-+                    bold=False,
-+                    nl=False,
-+                )
-+            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
++                out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
++            out(f" {node.value!r}", fg="blue", bold=False)
  
      @classmethod
      def show(cls, code: str) -> None:
@@ -93,26 +88,21 @@ class DebugVisitor(Visitor[T]):
         indent = " " * (2 * self.tree_depth)
         if isinstance(node, Node):
             _type = type_repr(node.type)
-            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow")
+            out(f"{indent}{_type}", fg="yellow")
             self.tree_depth += 1
             for child in node.children:
                 yield from self.visit(child)
 
             self.tree_depth -= 1
-            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="yellow", bold=False)
+            out(f"{indent}/{_type}", fg="yellow", bold=False)
         else:
             _type = token.tok_name.get(node.type, str(node.type))
-            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", nl=False)
+            out(f"{indent}{_type}", fg="blue", nl=False)
             if node.prefix:
                 # We don't have to handle prefixes for `Node` objects since
                 # that delegates to the first child anyway.
-                out(
-                    f"NOT_YET_IMPLEMENTED_ExprJoinedStr",
-                    fg="green",
-                    bold=False,
-                    nl=False,
-                )
-            out(f"NOT_YET_IMPLEMENTED_ExprJoinedStr", fg="blue", bold=False)
+                out(f" {node.prefix!r}", fg="green", bold=False, nl=False)
+            out(f" {node.value!r}", fg="blue", bold=False)
 
     @classmethod
     def show(cls, code: str) -> None:

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

@@ -27,7 +27,7 @@ def do_not_touch_this_prefix3():
  
  def do_not_touch_this_prefix2():
 -    FR'There was a bug where docstring prefixes would be normalized even with -S.'
-+    f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
++    Rf"There was a bug where docstring prefixes would be normalized even with -S."
  
  
  def do_not_touch_this_prefix3():
@@ -43,7 +43,7 @@ def do_not_touch_this_prefix():
 
 
 def do_not_touch_this_prefix2():
-    f"NOT_YET_IMPLEMENTED_ExprJoinedStr"
+    Rf"There was a bug where docstring prefixes would be normalized even with -S."
 
 
 def do_not_touch_this_prefix3():