diff --git a/Lib/test/test_float.py b/Lib/test/test_float.py index a514111c1..609a7164f 100644 --- a/Lib/test/test_float.py +++ b/Lib/test/test_float.py @@ -725,7 +725,6 @@ class IEEEFormatTestCase(unittest.TestCase): class FormatTestCase(unittest.TestCase): - @unittest.expectedFailure # TODO: RUSTPYTHON; ValueError: Invalid format specifier def test_format(self): # these should be rewritten to use both format(x, spec) and # x.__format__(spec) diff --git a/Lib/test/test_format.py b/Lib/test/test_format.py index 6868c8717..f6452341e 100644 --- a/Lib/test/test_format.py +++ b/Lib/test/test_format.py @@ -515,7 +515,6 @@ class FormatTest(unittest.TestCase): with self.assertRaisesRegex(ValueError, error_msg): '{:__}'.format(1) - @unittest.expectedFailure # TODO: RUSTPYTHON; Wrong error message def test_with_a_commas_and_an_underscore_in_format_specifier(self): error_msg = re.escape("Cannot specify both ',' and '_'.") with self.assertRaisesRegex(ValueError, error_msg): @@ -523,7 +522,6 @@ class FormatTest(unittest.TestCase): with self.assertRaisesRegex(ValueError, error_msg): '{:.,_f}'.format(1.1) - @unittest.expectedFailure # TODO: RUSTPYTHON; Wrong error message def test_with_an_underscore_and_a_comma_in_format_specifier(self): error_msg = re.escape("Cannot specify both ',' and '_'.") with self.assertRaisesRegex(ValueError, error_msg): diff --git a/crates/common/src/format.rs b/crates/common/src/format.rs index 1f43335b7..4218dca7b 100644 --- a/crates/common/src/format.rs +++ b/crates/common/src/format.rs @@ -232,6 +232,7 @@ pub struct FormatSpec { width: Option, grouping_option: Option, precision: Option, + frac_grouping_option: Option, format_type: Option, } @@ -291,22 +292,49 @@ fn parse_zero(text: &Wtf8) -> (bool, &Wtf8) { } } -fn parse_precision(text: &Wtf8) -> Result<(Option, &Wtf8), FormatSpecError> { +fn parse_char(text: &Wtf8, expected: char) -> (bool, &Wtf8) { let mut chars = text.code_points(); - Ok(match chars.next().and_then(CodePoint::to_char) { - Some('.') => { - let (size, remaining) = parse_number(chars.as_wtf8())?; - if let Some(size) = size { - if size > i32::MAX as usize { - return Err(FormatSpecError::PrecisionTooBig); - } - (Some(size), remaining) - } else { - (None, text) - } + if chars.next().and_then(CodePoint::to_char) == Some(expected) { + (true, chars.as_wtf8()) + } else { + (false, text) + } +} + +fn parse_precision( + text: &Wtf8, +) -> Result<(Option, Option, &Wtf8), FormatSpecError> { + let (dot, text) = parse_char(text, '.'); + if !dot { + return Ok((None, None, text)); + } + let (precision, text) = parse_number(text)?; + if let Some(precision) = precision + && precision > i32::MAX as usize + { + return Err(FormatSpecError::PrecisionTooBig); + } + let mut frac_grouping = None; + let (comma, text) = parse_char(text, ','); + if comma { + frac_grouping = Some(FormatGrouping::Comma); + } + let (underscore, text) = parse_char(text, '_'); + if underscore { + if frac_grouping.is_some() { + return Err(FormatSpecError::ExclusiveFormat(',', '_')); } - _ => (None, text), - }) + frac_grouping = Some(FormatGrouping::Underscore); + } + let (trailing_comma, _) = parse_char(text, ','); + if trailing_comma && frac_grouping == Some(FormatGrouping::Underscore) { + return Err(FormatSpecError::ExclusiveFormat(',', '_')); + } + // Not having a precision or underscore/comma after a dot is an error. + if precision.is_none() && frac_grouping.is_none() { + return Err(FormatSpecError::PrecisionMissing); + } + Ok((precision, frac_grouping, text)) } impl FormatSpec { @@ -331,7 +359,7 @@ impl FormatSpec { if let Some(grouping) = grouping_option { Self::validate_separator(grouping, text)?; } - let (precision, text) = parse_precision(text)?; + let (precision, frac_grouping_option, text) = parse_precision(text)?; let (format_type, text) = FormatType::parse(text); if !text.is_empty() { return Err(FormatSpecError::InvalidFormatSpecifier); @@ -351,6 +379,7 @@ impl FormatSpec { width, grouping_option, precision, + frac_grouping_option, format_type, }) } @@ -463,7 +492,14 @@ impl FormatSpec { Err(FormatSpecError::UnspecifiedFormat('_', ch)) } _ => Ok(()), + }?; + if let Some(grouping) = self.frac_grouping_option + && matches!(format_type, FormatType::Number(_)) + { + let ch = char::from(format_type); + return Err(FormatSpecError::UnspecifiedFormat(char::from(grouping), ch)); } + Ok(()) } const fn get_separator_interval(&self) -> usize { @@ -491,7 +527,9 @@ impl FormatSpec { let disp_digit_cnt = if self.fill == Some('0'.into()) && self.align == Some(FormatAlign::AfterSign) { - let width = self.width.unwrap_or(magnitude_len) as i32 - prefix.len() as i32; + let width = self.width.unwrap_or(magnitude_len) as i32 + - prefix.len() as i32 + - self.frac_separator_count(&magnitude_str) as i32; cmp::max(width, magnitude_len as i32) } else { magnitude_len as i32 @@ -502,6 +540,41 @@ impl FormatSpec { } } + fn frac_digit_span(&self, magnitude_str: &str) -> Option<(FormatGrouping, usize, usize)> { + let grouping = self.frac_grouping_option?; + let start = magnitude_str.find('.')? + 1; + let end = magnitude_str[start..] + .bytes() + .position(|b| !b.is_ascii_digit()) + .map_or(magnitude_str.len(), |offset| start + offset); + (start < end).then_some((grouping, start, end)) + } + + fn frac_separator_count(&self, magnitude_str: &str) -> usize { + match self.frac_digit_span(magnitude_str) { + Some((_, start, end)) => (end - start - 1) / self.get_separator_interval(), + None => 0, + } + } + + fn add_frac_separators(&self, magnitude_str: String) -> String { + let Some((grouping, start, end)) = self.frac_digit_span(&magnitude_str) else { + return magnitude_str; + }; + let inter = self.get_separator_interval(); + let sep = char::from(grouping); + let mut result = magnitude_str[..start].to_string(); + let mut frac = &magnitude_str[start..end]; + while frac.len() > inter { + result.push_str(&frac[..inter]); + result.push(sep); + frac = &frac[inter..]; + } + result.push_str(frac); + result.push_str(&magnitude_str[end..]); + result + } + /// Returns true if this format spec uses the locale-aware 'n' format type. #[must_use] pub fn has_locale_format(&self) -> bool { @@ -664,6 +737,7 @@ impl FormatSpec { num: &Complex64, locale: &LocaleInfo, ) -> Result { + self.validate_format(FormatType::FixedPoint(Case::Lower))?; // Reuse format_complex_re_im with 'g' type to get the base formatted parts, // then apply locale grouping. This matches CPython's format_complex_internal: // 'n' → 'g', add_parens=0, skip_re=0. @@ -850,6 +924,7 @@ impl FormatSpec { } }; let magnitude_str = self.add_magnitude_separators(raw_magnitude_str?, sign_str); + let magnitude_str = self.add_frac_separators(magnitude_str); Ok( self.format_sign_and_align( &AsciiStr::new(&magnitude_str), @@ -1074,17 +1149,16 @@ impl FormatSpec { }, }, }?; - match &self.grouping_option { + let magnitude_str = match &self.grouping_option { Some(fg) => { let sep = char::from(fg); let inter = self.get_separator_interval().try_into().unwrap(); let len = magnitude_str.len() as i32; - let separated_magnitude = - Self::add_magnitude_separators_for_char(magnitude_str, inter, sep, len); - Ok(separated_magnitude) + Self::add_magnitude_separators_for_char(magnitude_str, inter, sep, len) } - None => Ok(magnitude_str), - } + None => magnitude_str, + }; + Ok(self.add_frac_separators(magnitude_str)) } fn format_sign_and_align( @@ -1175,6 +1249,7 @@ impl Deref for AsciiStr<'_> { pub enum FormatSpecError { DecimalDigitsTooMany, PrecisionTooBig, + PrecisionMissing, InvalidFormatSpecifier, UnspecifiedFormat(char, char), ExclusiveFormat(char, char), @@ -1539,6 +1614,7 @@ mod tests { width: Some(33), grouping_option: None, precision: None, + frac_grouping_option: None, format_type: None, }); assert_eq!(FormatSpec::parse("33"), expected); @@ -1555,6 +1631,7 @@ mod tests { width: Some(33), grouping_option: None, precision: None, + frac_grouping_option: None, format_type: None, }); assert_eq!(FormatSpec::parse("<>33"), expected); @@ -1571,6 +1648,7 @@ mod tests { width: Some(23), grouping_option: Some(FormatGrouping::Comma), precision: Some(11), + frac_grouping_option: None, format_type: Some(FormatType::Binary), }); assert_eq!(FormatSpec::parse("<>-#23,.11b"), expected); @@ -1738,6 +1816,106 @@ mod tests { assert_eq!(fmt_float("06,%", f64::INFINITY), "00inf%"); } + #[test] + fn format_float_fractional_grouping() { + // Fraction digits group away from the decimal point, so the last group + // may be shorter than the interval. + assert_eq!(fmt_float(".6,f", 1234.56789), "1234.567,890"); + assert_eq!(fmt_float(".7,f", 1234.56789), "1234.567,890,0"); + assert_eq!(fmt_float(".4,f", 1.1), "1.100,0"); + assert_eq!(fmt_float(".3,f", 1.1), "1.100"); + assert_eq!(fmt_float(".6_f", 1234.56789), "1234.567_890"); + // Omitting the precision keeps the type's default. + assert_eq!(fmt_float(".,f", 1.1), "1.100,000"); + // The two parts are independent and may use different separators. + assert_eq!(fmt_float(",.6,f", 1234.56789), "1,234.567,890"); + assert_eq!(fmt_float(",.6_f", 1234.56789), "1,234.567_890"); + assert_eq!(fmt_float("_.6,f", 1234.56789), "1_234.567,890"); + } + + #[test] + fn format_float_fractional_grouping_never_touches_tail() { + // Only the digits between the point and any tail are groupable: the + // exponent and a trailing percent sign must stay intact. + assert_eq!(fmt_float(".6,e", 12345678900.0), "1.234,568e+10"); + assert_eq!(fmt_float(".6,E", 1234.5678), "1.234,568E+03"); + assert_eq!(fmt_float(".8,%", 1.2345e-05), "0.001,234,50%"); + // Values with no point have nothing to group. + assert_eq!(fmt_float(".6,f", f64::INFINITY), "inf"); + assert_eq!(fmt_float(".6,f", f64::NAN), "nan"); + assert_eq!(fmt_float(".0,f", 1234.56789), "1235"); + } + + #[test] + fn format_float_fractional_grouping_counts_toward_width() { + // Separators are inserted before padding, so they consume width. + assert_eq!(fmt_float("020.6,f", 1234.56789), "000000001234.567,890"); + assert_eq!(fmt_float("015.6,f", 1.5), "0000001.500,000"); + assert_eq!(fmt_float("<20.6,f", 1234.56789), "1234.567,890 "); + // Zero padding of the integer part must reserve room for them too. + assert_eq!(fmt_float("020,.6,f", 1234.56789), "0,000,001,234.567,890"); + assert_eq!(fmt_float("+020,.6_f", 1e-10), "+000,000,000.000_000"); + assert_eq!(fmt_float("= 015,.6,E", 1234.0), " 01.234,000E+03"); + assert_eq!(fmt_float("-015_._e", 1.1), "001.100_000e+00"); + } + + #[test] + fn format_parse_fractional_grouping_errors() { + // Mixing the two separators is rejected wherever it appears. + assert_eq!( + FormatSpec::parse(".,_f"), + Err(FormatSpecError::ExclusiveFormat(',', '_')) + ); + assert_eq!( + FormatSpec::parse("._,f"), + Err(FormatSpecError::ExclusiveFormat(',', '_')) + ); + // A repeated separator is left in the spec and rejected as a whole. + assert_eq!( + FormatSpec::parse(".,,f"), + Err(FormatSpecError::InvalidFormatSpecifier) + ); + assert_eq!( + FormatSpec::parse(".__f"), + Err(FormatSpecError::InvalidFormatSpecifier) + ); + // A dot needs either digits or a separator after it. + assert_eq!( + FormatSpec::parse("."), + Err(FormatSpecError::PrecisionMissing) + ); + assert_eq!( + FormatSpec::parse(".f"), + Err(FormatSpecError::PrecisionMissing) + ); + // 'n' draws its separators from the locale. + assert_eq!( + FormatSpec::parse(".6,n").unwrap().format_float(1234.5678), + Err(FormatSpecError::UnspecifiedFormat(',', 'n')) + ); + assert_eq!( + FormatSpec::parse("._n").unwrap().format_float(1234.5678), + Err(FormatSpecError::UnspecifiedFormat('_', 'n')) + ); + // The integer separator is reported first when both are present. + assert_eq!( + FormatSpec::parse("_.6,n").unwrap().format_float(1234.5678), + Err(FormatSpecError::UnspecifiedFormat('_', 'n')) + ); + // The complex locale path rewrites 'n' to 'g', so it must validate first. + let locale = LocaleInfo { + thousands_sep: ",".to_owned(), + decimal_point: ".".to_owned(), + grouping: vec![3, 0], + }; + assert_eq!( + FormatSpec::parse(".6,n") + .unwrap() + .format_complex_locale(&Complex64::new(1.0, 2.345678), &locale), + Err(FormatSpecError::UnspecifiedFormat(',', 'n')) + ); + } + #[test] fn format_float_empty_type_with_precision() { // Empty presentation type with a precision is repr-like: precision is diff --git a/crates/vm/src/format.rs b/crates/vm/src/format.rs index 7e18dd75d..2f4652dcc 100644 --- a/crates/vm/src/format.rs +++ b/crates/vm/src/format.rs @@ -48,6 +48,7 @@ impl IntoPyException for FormatSpecError { vm.new_value_error("Too many decimal digits in format string") } Self::PrecisionTooBig => vm.new_value_error("Precision too big"), + Self::PrecisionMissing => vm.new_value_error("Format specifier missing precision"), Self::InvalidFormatSpecifier => vm.new_value_error("Invalid format specifier"), Self::UnspecifiedFormat(c1, c2) => { let msg = format!("Cannot specify '{c1}' with '{c2}'.");