diff --git a/crates/ruff/src/rules/pyflakes/format.rs b/crates/ruff/src/rules/pyflakes/format.rs index 101a51a6c52e9a..bdedd043980752 100644 --- a/crates/ruff/src/rules/pyflakes/format.rs +++ b/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", diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F521_F521.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F521_F521.py.snap index 59ae5d10a61b7b..7e5cb863c741c1 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F521_F521.py.snap +++ b/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) diff --git a/crates/ruff_python_literal/src/format.rs b/crates/ruff_python_literal/src/format.rs index fa7039e0e14169..96f724ad2c8504 100644 --- a/crates/ruff_python_literal/src/format.rs +++ b/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, } +#[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, &str), FormatSpecError> } /// Parses a format part within a format spec -fn parse_nested_format_part<'a>( +fn parse_nested_placeholder<'a>( parts: &mut Vec, 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 { 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 "),