Refactor to reuse parse_spec implementation

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

@@ -11,7 +11,9 @@ pub(crate) fn error_to_string(err: &FormatParseError) -> String {
         FormatParseError::InvalidCharacterAfterRightBracket => {
             "Only '.' or '[' may follow ']' in format field specifier"
         }
-        FormatParseError::InvalidFormatSpecifier => "Max string recursion exceeded",
+        FormatParseError::PlaceholderRecursionExceeded => {
+            "Max format placeholder recursion exceeded"
+        }
         FormatParseError::MissingStartBracket => "Single '}' encountered in format string",
         FormatParseError::MissingRightBracket => "Expected '}' before end of string",
         FormatParseError::UnmatchedBracket => "Single '{' encountered in format string",

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F521_F521.py.snap

@@ -28,7 +28,7 @@ F521.py:3:1: F521 `.format` call has invalid format string: Expected '}' before
 5 | "{:{:{}}}".format(1, 2, 3)
   |
 
-F521.py:5:1: F521 `.format` call has invalid format string: Max string recursion exceeded
+F521.py:5:1: F521 `.format` call has invalid format string: Max format placeholder recursion exceeded
   |
 3 | "{foo[}".format(foo=1)
 4 | # too much string recursion (placeholder-in-placeholder)

crates/ruff_python_literal/src/format.rs

@@ -207,7 +207,7 @@ impl FormatParse for FormatType {
 /// ```
 ///
 /// However, replacements can only be singly nested (preserving our sanity).
-/// A [`FormatSpecError::ReplacementNotAllowed`] will be raised while parsing in this case.
+/// A [`FormatSpecError::PlaceholderRecursionExceeded`] will be raised while parsing in this case.
 /// ```python
 /// "hello {name:{fmt:{not_allowed}}}".format(name="test", fmt="<20")  # Syntax error
 /// ```
@@ -239,6 +239,13 @@ pub struct FormatSpec {
     replacements: Vec<FormatPart>,
 }
 
+#[derive(Debug, PartialEq, Default)]
+pub enum AllowPlaceholderNesting {
+    #[default]
+    Yes,
+    No,
+}
+
 fn get_num_digits(text: &str) -> usize {
     for (index, character) in text.char_indices() {
         if !character.is_ascii_digit() {
@@ -314,65 +321,42 @@ fn parse_precision(text: &str) -> Result<(Option<usize>, &str), FormatSpecError>
 }
 
 /// Parses a format part within a format spec
-fn parse_nested_format_part<'a>(
+fn parse_nested_placeholder<'a>(
     parts: &mut Vec<FormatPart>,
     text: &'a str,
 ) -> Result<&'a str, FormatSpecError> {
-    let mut end_bracket_pos = None;
-    let mut left = String::new();
-
-    if text.is_empty() {
-        return Ok(text);
-    }
-
-    for (idx, c) in text.char_indices() {
-        if idx == 0 {
-            if c != '{' {
-                return Ok(text);
-            }
-        } else if c == '{' {
-            return Err(FormatSpecError::ReplacementNotAllowed);
-        } else if c == '}' {
-            end_bracket_pos = Some(idx);
-            break;
-        } else {
-            left.push(c);
+    match FormatString::parse_spec(text, AllowPlaceholderNesting::No) {
+        // Not a nested replacement, OK
+        Err(FormatParseError::MissingStartBracket) => Ok(text),
+        Err(err) => Err(FormatSpecError::InvalidPlaceholder(err)),
+        Ok((format_part, text)) => {
+            parts.push(format_part);
+            Ok(text)
         }
     }
-    if let Some(pos) = end_bracket_pos {
-        let (_, right) = text.split_at(pos);
-        let format_part = FormatString::parse_part_in_brackets(&left)
-            .map_err(FormatSpecError::InvalidReplacement)?;
-        parts.push(format_part);
-        Ok(&right[1..])
-    } else {
-        Err(FormatSpecError::InvalidReplacement(
-            FormatParseError::MissingRightBracket,
-        ))
-    }
 }
 
 impl FormatSpec {
     pub fn parse(text: &str) -> Result<Self, FormatSpecError> {
         let mut replacements = vec![];
         // get_integer in CPython
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (conversion, text) = FormatConversion::parse(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (mut fill, mut align, text) = parse_fill_and_align(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (sign, text) = FormatSign::parse(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (alternate_form, text) = parse_alternate_form(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (zero, text) = parse_zero(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (width, text) = parse_number(text)?;
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (grouping_option, text) = FormatGrouping::parse(text);
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
         let (precision, text) = parse_precision(text)?;
-        let text = parse_nested_format_part(&mut replacements, text)?;
+        let text = parse_nested_placeholder(&mut replacements, text)?;
 
         let (format_type, _text) = if text.is_empty() {
             (None, text)
@@ -831,8 +815,8 @@ pub enum FormatSpecError {
     PrecisionTooBig,
     InvalidFormatSpecifier,
     InvalidFormatType,
-    InvalidReplacement(FormatParseError),
-    ReplacementNotAllowed,
+    InvalidPlaceholder(FormatParseError),
+    PlaceholderRecursionExceeded,
     UnspecifiedFormat(char, char),
     UnknownFormatCode(char, &'static str),
     PrecisionNotAllowed,
@@ -847,7 +831,7 @@ pub enum FormatParseError {
     UnmatchedBracket,
     MissingStartBracket,
     UnescapedStartBracketInLiteral,
-    InvalidFormatSpecifier,
+    PlaceholderRecursionExceeded,
     UnknownConversion,
     EmptyAttribute,
     MissingRightBracket,
@@ -866,8 +850,8 @@ impl std::fmt::Display for FormatParseError {
             Self::UnescapedStartBracketInLiteral => {
                 std::write!(fmt, "unescaped start bracket in literal")
             }
-            Self::InvalidFormatSpecifier => {
-                std::write!(fmt, "invalid format specifier")
+            Self::PlaceholderRecursionExceeded => {
+                std::write!(fmt, "multiply nested replacement not allowed")
             }
             Self::UnknownConversion => {
                 std::write!(fmt, "unknown conversion")
@@ -1067,20 +1051,23 @@ impl FormatString {
         })
     }
 
-    fn parse_spec(text: &str) -> Result<(FormatPart, &str), FormatParseError> {
+    fn parse_spec(
+        text: &str,
+        allow_nesting: AllowPlaceholderNesting,
+    ) -> Result<(FormatPart, &str), FormatParseError> {
+        let Some(text) = text.strip_prefix('{') else {
+            return Err(FormatParseError::MissingStartBracket);
+        };
+
         let mut nested = false;
-        let mut end_bracket_pos = None;
         let mut left = String::new();
 
-        // There may be one layer nesting brackets in spec
         for (idx, c) in text.char_indices() {
-            if idx == 0 {
-                if c != '{' {
-                    return Err(FormatParseError::MissingStartBracket);
-                }
-            } else if c == '{' {
-                if nested {
-                    return Err(FormatParseError::InvalidFormatSpecifier);
+            println!("Parsing {:?} {:?}", c, nested);
+            if c == '{' {
+                // There may be one layer nesting brackets in spec
+                if nested || allow_nesting == AllowPlaceholderNesting::No {
+                    return Err(FormatParseError::PlaceholderRecursionExceeded);
                 }
                 nested = true;
                 left.push(c);
@@ -1091,19 +1078,14 @@ impl FormatString {
                     left.push(c);
                     continue;
                 }
-                end_bracket_pos = Some(idx);
-                break;
+                let (_, right) = text.split_at(idx);
+                let format_part = FormatString::parse_part_in_brackets(&left)?;
+                return Ok((format_part, &right[1..]));
             } else {
                 left.push(c);
             }
         }
-        if let Some(pos) = end_bracket_pos {
-            let (_, right) = text.split_at(pos);
-            let format_part = FormatString::parse_part_in_brackets(&left)?;
-            Ok((format_part, &right[1..]))
-        } else {
-            Err(FormatParseError::UnmatchedBracket)
-        }
+        Err(FormatParseError::UnmatchedBracket)
     }
 }
 
@@ -1122,7 +1104,7 @@ impl<'a> FromTemplate<'a> for FormatString {
             // Try to parse both literals and bracketed format parts until we
             // run out of text
             cur_text = FormatString::parse_literal(cur_text)
-                .or_else(|_| FormatString::parse_spec(cur_text))
+                .or_else(|_| FormatString::parse_spec(cur_text, AllowPlaceholderNesting::Yes))
                 .map(|(part, new_text)| {
                     parts.push(part);
                     new_text
@@ -1482,14 +1464,14 @@ mod tests {
         );
         assert_eq!(
             FormatSpec::parse("{"),
-            Err(FormatSpecError::InvalidReplacement(
-                FormatParseError::MissingRightBracket
+            Err(FormatSpecError::InvalidPlaceholder(
+                FormatParseError::UnmatchedBracket
             ))
         );
         assert_eq!(
             FormatSpec::parse("{x"),
-            Err(FormatSpecError::InvalidReplacement(
-                FormatParseError::MissingRightBracket
+            Err(FormatSpecError::InvalidPlaceholder(
+                FormatParseError::UnmatchedBracket
             ))
         );
         assert_eq!(
@@ -1498,7 +1480,9 @@ mod tests {
         );
         assert_eq!(
             FormatSpec::parse("{{x}}"),
-            Err(FormatSpecError::ReplacementNotAllowed)
+            Err(FormatSpecError::InvalidPlaceholder(
+                FormatParseError::PlaceholderRecursionExceeded
+            ))
         );
         assert_eq!(
             FormatSpec::parse("d "),