Proper f string formatting 5

crates/ruff/src/checkers/ast/analyze/expression.rs

@@ -931,15 +931,15 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 pylint::rules::await_outside_async(checker, expr);
             }
         }
-        Expr::FString(ast::ExprFString { values, .. }) => {
+        Expr::FString(ast::ExprFString { parts, .. }) => {
             if checker.enabled(Rule::FStringMissingPlaceholders) {
-                pyflakes::rules::f_string_missing_placeholders(expr, values, checker);
+                pyflakes::rules::f_string_missing_placeholders(expr, parts, checker);
             }
             if checker.enabled(Rule::HardcodedSQLExpression) {
                 flake8_bandit::rules::hardcoded_sql_expression(checker, expr);
             }
             if checker.enabled(Rule::ExplicitFStringTypeConversion) {
-                ruff::rules::explicit_f_string_type_conversion(checker, expr, values);
+                ruff::rules::explicit_f_string_type_conversion(checker, expr, parts);
             }
         }
         Expr::BinOp(ast::ExprBinOp {

crates/ruff/src/checkers/ast/mod.rs

@@ -1283,17 +1283,6 @@ where
         self.semantic.flags = flags_snapshot;
     }
 
-    fn visit_format_spec(&mut self, format_spec: &'b Expr) {
-        match format_spec {
-            Expr::FString(ast::ExprFString { values, .. }) => {
-                for value in values {
-                    self.visit_expr(value);
-                }
-            }
-            _ => unreachable!("Unexpected expression for format_spec"),
-        }
-    }
-
     fn visit_parameters(&mut self, parameters: &'b Parameters) {
         // Step 1: Binding.
         // Bind, but intentionally avoid walking default expressions, as we handle them

crates/ruff/src/rules/flake8_comprehensions/fixes.rs

@@ -1004,7 +1004,7 @@ pub(crate) fn fix_unnecessary_map(
         // If the expression is embedded in an f-string, surround it with spaces to avoid
         // syntax errors.
         if matches!(object_type, ObjectType::Set | ObjectType::Dict) {
-            if parent.is_some_and(Expr::is_formatted_value_expr) {
+            if parent.is_some_and(Expr::is_f_string_expr) {
                 content = format!(" {content} ");
             }
         }

crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs

@@ -63,13 +63,22 @@ pub(super) fn is_empty_or_null_string(expr: &Expr) -> bool {
             ..
         }) => string.is_empty(),
         Expr::Constant(constant) if constant.value.is_none() => true,
-        Expr::FString(ast::ExprFString { values, .. }) => {
-            values.iter().all(is_empty_or_null_string)
+        Expr::FString(ast::ExprFString { parts, .. }) => {
+            parts.iter().all(is_empty_or_null_string_part)
         }
         _ => false,
     }
 }
 
+fn is_empty_or_null_string_part(part: &ast::FStringPart) -> bool {
+    match part {
+        ast::FStringPart::Literal(ast::PartialString { value, .. }) => value.is_empty(),
+        ast::FStringPart::FormattedValue(ast::FormattedValue { expression, .. }) => {
+            is_empty_or_null_string(expression)
+        }
+    }
+}
+
 pub(super) fn split_names(names: &str) -> Vec<&str> {
     // Match the following pytest code:
     //    [x.strip() for x in argnames.split(",") if x.strip()]

crates/ruff/src/rules/flynt/helpers.rs

@@ -2,25 +2,22 @@ use ruff_python_ast::{self as ast, Arguments, Constant, ConversionFlag, Expr};
 use ruff_text_size::TextRange;
 
 /// Wrap an expression in a `FormattedValue` with no special formatting.
-fn to_formatted_value_expr(inner: &Expr) -> Expr {
-    let node = ast::ExprFormattedValue {
-        value: Box::new(inner.clone()),
+fn to_formatted_value_expr(inner: &Expr) -> ast::FStringPart {
+    ast::FStringPart::FormattedValue(ast::FormattedValue {
+        expression: Box::new(inner.clone()),
         debug_text: None,
         conversion: ConversionFlag::None,
-        format_spec: None,
+        format_spec: vec![],
         range: TextRange::default(),
-    };
-    node.into()
+    })
 }
 
 /// Convert a string to a constant string expression.
-pub(super) fn to_constant_string(s: &str) -> Expr {
-    let node = ast::ExprConstant {
-        value: s.to_owned().into(),
-        kind: None,
+pub(super) fn to_constant_string(s: &str) -> ast::FStringPart {
+    ast::FStringPart::Literal(ast::PartialString {
+        value: s.to_owned(),
         range: TextRange::default(),
-    };
-    node.into()
+    })
 }
 
 /// Figure out if `expr` represents a "simple" call
@@ -52,15 +49,17 @@ fn is_simple_callee(func: &Expr) -> bool {
 }
 
 /// Convert an expression to a f-string element (if it looks like a good idea).
-pub(super) fn to_f_string_element(expr: &Expr) -> Option<Expr> {
+pub(super) fn to_fstring_part(expr: &Expr) -> Option<ast::FStringPart> {
     match expr {
         // These are directly handled by `unparse_f_string_element`:
         Expr::Constant(ast::ExprConstant {
-            value: Constant::Str(_),
+            value: Constant::Str(value),
+            range,
             ..
-        })
-        | Expr::FString(_)
-        | Expr::FormattedValue(_) => Some(expr.clone()),
+        }) => Some(ast::FStringPart::Literal(ast::PartialString {
+            value: value.to_string(),
+            range: *range,
+        })),
         // These should be pretty safe to wrap in a formatted value.
         Expr::Constant(ast::ExprConstant {
             value:

crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs

@@ -91,7 +91,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
         return Some(node.into());
     }
 
-    let mut fstring_elems = Vec::with_capacity(joinees.len() * 2);
+    let mut fstring_parts = Vec::with_capacity(joinees.len() * 2);
     let mut first = true;
 
     for expr in joinees {
@@ -101,13 +101,13 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
             return None;
         }
         if !std::mem::take(&mut first) {
-            fstring_elems.push(helpers::to_constant_string(joiner));
+            fstring_parts.push(helpers::to_constant_string(joiner));
         }
-        fstring_elems.push(helpers::to_f_string_element(expr)?);
+        fstring_parts.push(helpers::to_fstring_part(expr)?);
     }
 
     let node = ast::ExprFString {
-        values: fstring_elems,
+        parts: fstring_parts,
         implicit_concatenated: false,
         range: TextRange::default(),
     };

crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs

@@ -1,4 +1,4 @@
-use ruff_python_ast::{Expr, PySourceType, Ranged};
+use ruff_python_ast::{Expr, FStringPart, PySourceType, Ranged};
 use ruff_python_parser::{lexer, AsMode, StringKind, Tok};
 use ruff_text_size::{TextRange, TextSize};
 
@@ -80,10 +80,14 @@ fn find_useless_f_strings<'a>(
 }
 
 /// F541
-pub(crate) fn f_string_missing_placeholders(expr: &Expr, values: &[Expr], checker: &mut Checker) {
+pub(crate) fn f_string_missing_placeholders(
+    expr: &Expr,
+    values: &[FStringPart],
+    checker: &mut Checker,
+) {
     if !values
         .iter()
-        .any(|value| matches!(value, Expr::FormattedValue(_)))
+        .any(|value| matches!(value, FStringPart::FormattedValue(_)))
     {
         for (prefix_range, tok_range) in
             find_useless_f_strings(expr, checker.locator(), checker.source_type)

crates/ruff/src/rules/pylint/rules/assert_on_string_literal.rs

@@ -71,25 +71,21 @@ pub(crate) fn assert_on_string_literal(checker: &mut Checker, test: &Expr) {
             }
             _ => {}
         },
-        Expr::FString(ast::ExprFString { values, .. }) => {
+        Expr::FString(ast::ExprFString { parts, .. }) => {
             checker.diagnostics.push(Diagnostic::new(
                 AssertOnStringLiteral {
-                    kind: if values.iter().all(|value| match value {
-                        Expr::Constant(ast::ExprConstant { value, .. }) => match value {
-                            Constant::Str(value, ..) => value.is_empty(),
-                            Constant::Bytes(value) => value.is_empty(),
-                            _ => false,
-                        },
-                        _ => false,
+                    kind: if parts.iter().all(|part| match part {
+                        ast::FStringPart::Literal(ast::PartialString { value, .. }) => {
+                            value.is_empty()
+                        }
+                        ast::FStringPart::FormattedValue(_) => false,
                     }) {
                         Kind::Empty
-                    } else if values.iter().any(|value| match value {
-                        Expr::Constant(ast::ExprConstant { value, .. }) => match value {
-                            Constant::Str(value, ..) => !value.is_empty(),
-                            Constant::Bytes(value) => !value.is_empty(),
-                            _ => false,
-                        },
-                        _ => false,
+                    } else if parts.iter().any(|part| match part {
+                        ast::FStringPart::Literal(ast::PartialString { value, .. }) => {
+                            !value.is_empty()
+                        }
+                        ast::FStringPart::FormattedValue(_) => false,
                     }) {
                         Kind::NonEmpty
                     } else {

crates/ruff/src/rules/pyupgrade/rules/use_pep604_annotation.rs

@@ -159,7 +159,6 @@ fn is_allowed_value(expr: &Expr) -> bool {
         | Expr::GeneratorExp(_)
         | Expr::Compare(_)
         | Expr::Call(_)
-        | Expr::FormattedValue(_)
         | Expr::FString(_)
         | Expr::Constant(_)
         | Expr::Attribute(_)

crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs

@@ -55,21 +55,23 @@ impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
 pub(crate) fn explicit_f_string_type_conversion(
     checker: &mut Checker,
     expr: &Expr,
-    values: &[Expr],
+    values: &[ast::FStringPart],
 ) {
     for (index, formatted_value) in values
         .iter()
         .filter_map(|expr| {
-            if let Expr::FormattedValue(expr) = &expr {
+            if let ast::FStringPart::FormattedValue(expr) = &expr {
                 Some(expr)
             } else {
                 None
             }
         })
         .enumerate()
     {
-        let ast::ExprFormattedValue {
-            value, conversion, ..
+        let ast::FormattedValue {
+            expression,
+            conversion,
+            ..
         } = formatted_value;
 
         // Skip if there's already a conversion flag.
@@ -86,7 +88,7 @@ pub(crate) fn explicit_f_string_type_conversion(
                     range: _,
                 },
             ..
-        }) = value.as_ref()
+        }) = expression.as_ref()
         else {
             continue;
         };
@@ -121,7 +123,7 @@ pub(crate) fn explicit_f_string_type_conversion(
             continue;
         }
 
-        let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, value.range());
+        let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, expression.range());
         if checker.patch(diagnostic.kind.rule()) {
             diagnostic.try_set_fix(|| {
                 convert_call_to_conversion_flag(expr, index, checker.locator(), checker.stylist())

crates/ruff/src/rules/ruff/rules/unreachable.rs

@@ -625,7 +625,6 @@ impl<'stmt> BasicBlocksBuilder<'stmt> {
                         | Expr::Set(_)
                         | Expr::Compare(_)
                         | Expr::Call(_)
-                        | Expr::FormattedValue(_)
                         | Expr::FString(_)
                         | Expr::Constant(_)
                         | Expr::Attribute(_)

crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs

@@ -88,10 +88,12 @@ pub(crate) fn raise_vanilla_args(checker: &mut Checker, expr: &Expr) {
 /// some whitespace).
 fn contains_message(expr: &Expr) -> bool {
     match expr {
-        Expr::FString(ast::ExprFString { values, .. }) => {
-            for value in values {
-                if contains_message(value) {
-                    return true;
+        Expr::FString(ast::ExprFString { parts, .. }) => {
+            for part in parts {
+                if let ast::FStringPart::Literal(ast::PartialString { value, .. }) = part {
+                    if value.chars().any(char::is_whitespace) {
+                        return true;
+                    }
                 }
             }
         }

crates/ruff_python_ast/src/comparable.rs

@@ -517,6 +517,36 @@ impl<'a> From<&'a ast::ExceptHandler> for ComparableExceptHandler<'a> {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub enum ComparableFStringPart<'a> {
+    Literal(&'a str),
+    FormattedValue(FormattedValue<'a>),
+}
+
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct FormattedValue<'a> {
+    expression: ComparableExpr<'a>,
+    debug_text: Option<&'a ast::DebugText>,
+    conversion: ast::ConversionFlag,
+    format_spec: Vec<ComparableFStringPart<'a>>,
+}
+
+impl<'a> From<&'a ast::FStringPart> for ComparableFStringPart<'a> {
+    fn from(fstring_part: &'a ast::FStringPart) -> Self {
+        match fstring_part {
+            ast::FStringPart::Literal(ast::PartialString { value, .. }) => Self::Literal(value),
+            ast::FStringPart::FormattedValue(formatted_value) => {
+                Self::FormattedValue(FormattedValue {
+                    expression: (&formatted_value.expression).into(),
+                    debug_text: formatted_value.debug_text.as_ref(),
+                    conversion: formatted_value.conversion,
+                    format_spec: formatted_value.format_spec.iter().map(Into::into).collect(),
+                })
+            }
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct ComparableElifElseClause<'a> {
     test: Option<ComparableExpr<'a>>,
@@ -644,12 +674,12 @@ pub struct ExprFormattedValue<'a> {
     value: Box<ComparableExpr<'a>>,
     debug_text: Option<&'a ast::DebugText>,
     conversion: ast::ConversionFlag,
-    format_spec: Option<Box<ComparableExpr<'a>>>,
+    format_spec: Vec<ComparableFStringPart<'a>>,
 }
 
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct ExprFString<'a> {
-    values: Vec<ComparableExpr<'a>>,
+    parts: Vec<ComparableFStringPart<'a>>,
 }
 
 #[derive(Debug, PartialEq, Eq, Hash)]
@@ -885,24 +915,12 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
                 func: func.into(),
                 arguments: arguments.into(),
             }),
-            ast::Expr::FormattedValue(ast::ExprFormattedValue {
-                value,
-                conversion,
-                debug_text,
-                format_spec,
-                range: _,
-            }) => Self::FormattedValue(ExprFormattedValue {
-                value: value.into(),
-                conversion: *conversion,
-                debug_text: debug_text.as_ref(),
-                format_spec: format_spec.as_ref().map(Into::into),
-            }),
             ast::Expr::FString(ast::ExprFString {
-                values,
+                parts,
                 implicit_concatenated: _,
                 range: _,
             }) => Self::FString(ExprFString {
-                values: values.iter().map(Into::into).collect(),
+                parts: parts.iter().map(Into::into).collect(),
             }),
             ast::Expr::Constant(ast::ExprConstant {
                 value,