Add separator validation (RustPython#5904)

* Add separator validation

* Remove @unittest.expectedFailure

Author: yt2b

Lib/test/test_format.py

@@ -502,29 +502,21 @@ def test_g_format_has_no_trailing_zeros(self):
         self.assertEqual(format(12300050.0, ".6g"), "1.23e+07")
         self.assertEqual(format(12300050.0, "#.6g"), "1.23000e+07")
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_with_two_commas_in_format_specifier(self):
         error_msg = re.escape("Cannot specify ',' with ','.")
         with self.assertRaisesRegex(ValueError, error_msg):
             '{:,,}'.format(1)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_with_two_underscore_in_format_specifier(self):
         error_msg = re.escape("Cannot specify '_' with '_'.")
         with self.assertRaisesRegex(ValueError, error_msg):
             '{:__}'.format(1)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     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):
             '{:,_}'.format(1)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     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):

Lib/test/test_fstring.py

@@ -1838,29 +1838,21 @@ def test_invalid_syntax_error_message(self):
         ):
             compile("f'{a $ b}'", "?", "exec")
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_with_two_commas_in_format_specifier(self):
         error_msg = re.escape("Cannot specify ',' with ','.")
         with self.assertRaisesRegex(ValueError, error_msg):
             f"{1:,,}"
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_with_two_underscore_in_format_specifier(self):
         error_msg = re.escape("Cannot specify '_' with '_'.")
         with self.assertRaisesRegex(ValueError, error_msg):
             f"{1:__}"
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     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):
             f"{1:,_}"
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     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):

Lib/test/test_long.py

@@ -625,8 +625,6 @@ def __lt__(self, other):
                     eq(x > y, Rcmp > 0)
                     eq(x >= y, Rcmp >= 0)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test__format__(self):
         self.assertEqual(format(123456789, 'd'), '123456789')
         self.assertEqual(format(123456789, 'd'), '123456789')

common/src/format.rs

@@ -127,6 +127,15 @@ impl FormatParse for FormatGrouping {
     }
 }
 
+impl From<&FormatGrouping> for char {
+    fn from(fg: &FormatGrouping) -> Self {
+        match fg {
+            FormatGrouping::Comma => ',',
+            FormatGrouping::Underscore => '_',
+        }
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub enum FormatType {
     String,
@@ -281,6 +290,7 @@ impl FormatSpec {
     pub fn parse(text: impl AsRef<Wtf8>) -> Result<Self, FormatSpecError> {
         Self::_parse(text.as_ref())
     }
+
     fn _parse(text: &Wtf8) -> Result<Self, FormatSpecError> {
         // get_integer in CPython
         let (conversion, text) = FormatConversion::parse(text);
@@ -290,6 +300,9 @@ impl FormatSpec {
         let (zero, text) = parse_zero(text);
         let (width, text) = parse_number(text)?;
         let (grouping_option, text) = FormatGrouping::parse(text);
+        if let Some(grouping) = &grouping_option {
+            Self::validate_separator(grouping, text)?;
+        }
         let (precision, text) = parse_precision(text)?;
         let (format_type, text) = FormatType::parse(text);
         if !text.is_empty() {
@@ -314,6 +327,20 @@ impl FormatSpec {
         })
     }
 
+    fn validate_separator(grouping: &FormatGrouping, text: &Wtf8) -> Result<(), FormatSpecError> {
+        let mut chars = text.code_points().peekable();
+        match chars.peek().and_then(|cp| CodePoint::to_char(*cp)) {
+            Some(c) if c == ',' || c == '_' => {
+                if c == char::from(grouping) {
+                    Err(FormatSpecError::UnspecifiedFormat(c, c))
+                } else {
+                    Err(FormatSpecError::ExclusiveFormat(',', '_'))
+                }
+            }
+            _ => Ok(()),
+        }
+    }
+
     fn compute_fill_string(fill_char: CodePoint, fill_chars_needed: i32) -> Wtf8Buf {
         (0..fill_chars_needed).map(|_| fill_char).collect()
     }
@@ -406,10 +433,7 @@ impl FormatSpec {
     fn add_magnitude_separators(&self, magnitude_str: String, prefix: &str) -> String {
         match &self.grouping_option {
             Some(fg) => {
-                let sep = match fg {
-                    FormatGrouping::Comma => ',',
-                    FormatGrouping::Underscore => '_',
-                };
+                let sep = char::from(fg);
                 let inter = self.get_separator_interval().try_into().unwrap();
                 let magnitude_len = magnitude_str.len();
                 let width = self.width.unwrap_or(magnitude_len) as i32 - prefix.len() as i32;
@@ -720,10 +744,7 @@ impl FormatSpec {
         }?;
         match &self.grouping_option {
             Some(fg) => {
-                let sep = match fg {
-                    FormatGrouping::Comma => ',',
-                    FormatGrouping::Underscore => '_',
-                };
+                let sep = char::from(fg);
                 let inter = self.get_separator_interval().try_into().unwrap();
                 let len = magnitude_str.len() as i32;
                 let separated_magnitude =
@@ -818,6 +839,7 @@ pub enum FormatSpecError {
     PrecisionTooBig,
     InvalidFormatSpecifier,
     UnspecifiedFormat(char, char),
+    ExclusiveFormat(char, char),
     UnknownFormatCode(char, &'static str),
     PrecisionNotAllowed,
     NotAllowed(&'static str),

vm/src/format.rs

@@ -21,6 +21,10 @@ impl IntoPyException for FormatSpecError {
                 let msg = format!("Cannot specify '{c1}' with '{c2}'.");
                 vm.new_value_error(msg)
             }
+            Self::ExclusiveFormat(c1, c2) => {
+                let msg = format!("Cannot specify both '{c1}' and '{c2}'.");
+                vm.new_value_error(msg)
+            }
             Self::UnknownFormatCode(c, s) => {
                 let msg = format!("Unknown format code '{c}' for object of type '{s}'");
                 vm.new_value_error(msg)