Add more test cases; check format types that occur after placeholders

crates/ruff/resources/test/fixtures/pylint/bad_string_format_character.py

@@ -17,7 +17,9 @@
 "{:*^30s}".format("centered") # OK
 "{:{s}}".format("hello", s="s")  # OK (nested replacement value not checked)
 "{:{s:y}}".format("hello", s="s")  # [bad-format-character] (nested replacement format spec checked)
-"{0:.{prec}g}".format(1.23, prec=15)  # OK (cannot validate after nested replacement)
+"{0:.{prec}g}".format(1.23, prec=15)  # OK
+"{0:.{foo}x{bar}y{foobar}g}".format(...)  # OK (all nested replacements are consumed without considering in between chars)
+"{0:.{foo}{bar}{foobar}y}".format(...)  # [bad-format-character] (check value after replacements)
 
 ## f-strings
 

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE1300_bad_string_format_character.py.snap

@@ -57,7 +57,18 @@ bad_string_format_character.py:19:1: PLE1300 Unsupported format character 'y'
 18 | "{:{s}}".format("hello", s="s")  # OK (nested replacement value not checked)
 19 | "{:{s:y}}".format("hello", s="s")  # [bad-format-character] (nested replacement format spec checked)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLE1300
-20 | "{0:.{prec}g}".format(1.23, prec=15)  # OK (cannot validate after nested replacement)
+20 | "{0:.{prec}g}".format(1.23, prec=15)  # OK
+21 | "{0:.{foo}x{bar}y{foobar}g}".format(...)  # OK (all nested replacements are consumed without considering in between chars)
+   |
+
+bad_string_format_character.py:22:1: PLE1300 Unsupported format character 'y'
+   |
+20 | "{0:.{prec}g}".format(1.23, prec=15)  # OK
+21 | "{0:.{foo}x{bar}y{foobar}g}".format(...)  # OK (all nested replacements are consumed without considering in between chars)
+22 | "{0:.{foo}{bar}{foobar}y}".format(...)  # [bad-format-character] (check value after replacements)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLE1300
+23 | 
+24 | ## f-strings
    |
 
 

crates/ruff_python_literal/src/format.rs

@@ -318,7 +318,7 @@ fn parse_precision(text: &str) -> Result<(Option<usize>, &str), FormatSpecError>
     })
 }
 
-/// Parses a format part within a format spec
+/// Parses a placeholder within a format spec
 fn parse_nested_placeholder<'a>(
     placeholders: &mut Vec<FormatPart>,
     text: &'a str,
@@ -334,17 +334,22 @@ fn parse_nested_placeholder<'a>(
     }
 }
 
+/// Parse and consume all placeholders in a format spec
 ///
-fn consume_remaining_placeholders<'a>(
+/// This will also consume any intermediate characters such as `x` and `y` in
+/// ```
+/// "x{foo}y{bar}z"
+/// ```
+fn consume_all_placeholders<'a>(
     placeholders: &mut Vec<FormatPart>,
     text: &'a str,
 ) -> Result<&'a str, FormatSpecError> {
     let mut chars = text.chars();
+    let mut text = text;
     let mut placeholder_count = placeholders.len();
 
     while chars.clone().contains(&'{') {
-        dbg!(&chars, placeholder_count);
+        text = parse_nested_placeholder(placeholders, chars.as_str())?;
-        let text = parse_nested_placeholder(placeholders, chars.as_str())?;
         chars = text.chars();
         // If we did not parse a placeholder, consume a character
         if placeholder_count == placeholders.len() {
@@ -353,14 +358,11 @@ fn consume_remaining_placeholders<'a>(
             placeholder_count = placeholders.len();
         }
     }
-
+    Ok(text)
-    Ok(chars.as_str())
 }
 
 impl FormatSpec {
     pub fn parse(text: &str) -> Result<Self, FormatSpecError> {
-        println!();
-        dbg!(text);
         let mut replacements = vec![];
         let text = parse_nested_placeholder(&mut replacements, text)?;
         let (conversion, text) = FormatConversion::parse(text);
@@ -378,7 +380,7 @@ impl FormatSpec {
         let (grouping_option, text) = FormatGrouping::parse(text);
         let text = parse_nested_placeholder(&mut replacements, text)?;
         let (precision, text) = parse_precision(text)?;
-        let text = consume_remaining_placeholders(&mut replacements, text)?;
+        let text = consume_all_placeholders(&mut replacements, text)?;
 
         let (format_type, _text) = if text.is_empty() {
             (None, text)