Merge pull request #4421 from yt2b/implement_string_format_validation

Implement string format validation

common/src/format.rs

@@ -522,7 +522,6 @@ impl FormatSpec {
             sign_str,
             FormatAlign::Right,
         )
-        .map_err(|msg| msg.to_owned())
     }
 
     #[inline]
@@ -605,10 +604,10 @@ impl FormatSpec {
             &sign_prefix,
             FormatAlign::Right,
         )
-        .map_err(|msg| msg.to_owned())
     }
 
-    pub fn format_string(&self, s: &BorrowedStr) -> Result<String, &'static str> {
+    pub fn format_string(&self, s: &BorrowedStr) -> Result<String, String> {
+        self.validate_format(FormatType::String)?;
         match self.format_type {
             Some(FormatType::String) | None => self
                 .format_sign_and_align(s, "", FormatAlign::Left)
@@ -618,7 +617,13 @@ impl FormatSpec {
                     }
                     value
                 }),
-            _ => Err("Unknown format code for object of type 'str'"),
+            _ => {
+                let ch = char::from(self.format_type.as_ref().unwrap());
+                Err(format!(
+                    "Unknown format code '{}' for object of type 'str'",
+                    ch
+                ))
+            }
         }
     }
 
@@ -627,7 +632,7 @@ impl FormatSpec {
         magnitude_str: &BorrowedStr,
         sign_str: &str,
         default_align: FormatAlign,
-    ) -> Result<String, &'static str> {
+    ) -> Result<String, String> {
         let align = self.align.unwrap_or(default_align);
 
         let num_chars = magnitude_str.char_len();

extra_tests/snippets/builtin_format.py

@@ -72,3 +72,5 @@ assert_raises(ValueError, "{:,o}".format, 1, _msg="ValueError: Cannot specify ',
 assert_raises(ValueError, "{:_n}".format, 1, _msg="ValueError: Cannot specify '_' with 'n'.")
 assert_raises(ValueError, "{:,o}".format, 1.0, _msg="ValueError: Cannot specify ',' with 'o'.")
 assert_raises(ValueError, "{:_n}".format, 1.0, _msg="ValueError: Cannot specify '_' with 'n'.")
+assert_raises(ValueError, "{:,}".format, "abc", _msg="ValueError: Cannot specify ',' with 's'.")
+assert_raises(ValueError, "{:,x}".format, "abc", _msg="ValueError: Cannot specify ',' with 'x'.")

vm/src/builtins/str.rs

@@ -730,9 +730,11 @@ impl PyStr {
 
     #[pymethod(name = "__format__")]
     fn format_str(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult<String> {
-        FormatSpec::parse(spec.as_str())
-            .and_then(|format_spec| format_spec.format_string(self.borrow()))
-            .map_err(|err| vm.new_value_error(err.to_string()))
+        let format_spec =
+            FormatSpec::parse(spec.as_str()).map_err(|err| vm.new_value_error(err.to_string()))?;
+        format_spec
+            .format_string(self.borrow())
+            .map_err(|msg| vm.new_value_error(msg))
     }
 
     /// Return a titlecased version of the string where words start with an