diff --git a/common/src/format.rs b/common/src/format.rs index 981b703609..08aa879545 100644 --- a/common/src/format.rs +++ b/common/src/format.rs @@ -223,7 +223,7 @@ fn parse_fill_and_align(text: &str) -> (Option, Option, &str) } } -fn parse_number(text: &str) -> Result<(Option, &str), &'static str> { +fn parse_number(text: &str) -> Result<(Option, &str), FormatSpecError> { let num_digits: usize = get_num_digits(text); if num_digits == 0 { return Ok((None, text)); @@ -232,7 +232,7 @@ fn parse_number(text: &str) -> Result<(Option, &str), &'static str> { Ok((Some(num), &text[num_digits..])) } else { // NOTE: this condition is different from CPython - Err("Too many decimal digits in format string") + Err(FormatSpecError::DecimalDigitsTooMany) } } @@ -252,14 +252,14 @@ fn parse_zero(text: &str) -> (bool, &str) { } } -fn parse_precision(text: &str) -> Result<(Option, &str), &'static str> { +fn parse_precision(text: &str) -> Result<(Option, &str), FormatSpecError> { let mut chars = text.chars(); Ok(match chars.next() { Some('.') => { let (size, remaining) = parse_number(chars.as_str())?; if let Some(size) = size { if size > i32::MAX as usize { - return Err("Precision too big"); + return Err(FormatSpecError::PrecisionTooBig); } (Some(size), remaining) } else { @@ -271,7 +271,7 @@ fn parse_precision(text: &str) -> Result<(Option, &str), &'static str> { } impl FormatSpec { - pub fn parse(text: &str) -> Result { + pub fn parse(text: &str) -> Result { // get_integer in CPython let (preconversor, text) = FormatPreconversor::parse(text); let (mut fill, mut align, text) = parse_fill_and_align(text); @@ -283,7 +283,7 @@ impl FormatSpec { let (precision, text) = parse_precision(text)?; let (format_type, text) = FormatType::parse(text); if !text.is_empty() { - return Err("Invalid format specifier".to_owned()); + return Err(FormatSpecError::InvalidFormatSpecifier); } if zero && fill.is_none() { @@ -359,7 +359,7 @@ impl FormatSpec { magnitude_str } - fn validate_format(&self, default_format_type: FormatType) -> Result<(), String> { + fn validate_format(&self, default_format_type: FormatType) -> Result<(), FormatSpecError> { let format_type = self.format_type.as_ref().unwrap_or(&default_format_type); match (&self.grouping_option, format_type) { ( @@ -373,14 +373,14 @@ impl FormatSpec { | FormatType::Number, ) => { let ch = char::from(format_type); - Err(format!("Cannot specify ',' with '{ch}'.")) + Err(FormatSpecError::UnspecifiedFormat(',', ch)) } ( Some(FormatGrouping::Underscore), FormatType::String | FormatType::Character | FormatType::Number, ) => { let ch = char::from(format_type); - Err(format!("Cannot specify '_' with '{ch}'.")) + Err(FormatSpecError::UnspecifiedFormat('_', ch)) } _ => Ok(()), } @@ -422,11 +422,11 @@ impl FormatSpec { } } - pub fn format_float(&self, num: f64) -> Result { + pub fn format_float(&self, num: f64) -> Result { self.validate_format(FormatType::FixedPointLower)?; let precision = self.precision.unwrap_or(6); let magnitude = num.abs(); - let raw_magnitude_str: Result = match self.format_type { + let raw_magnitude_str: Result = match self.format_type { Some(FormatType::FixedPointUpper) => Ok(float_ops::format_fixed( precision, magnitude, @@ -445,13 +445,9 @@ impl FormatSpec { | Some(FormatType::String) | Some(FormatType::Character) => { let ch = char::from(self.format_type.as_ref().unwrap()); - Err(format!( - "Unknown format code '{ch}' for object of type 'float'", - )) - } - Some(FormatType::Number) => { - Err("Format code 'n' for object of type 'float' not implemented yet".to_owned()) + Err(FormatSpecError::UnknownFormatCode(ch, "float")) } + Some(FormatType::Number) => Err(FormatSpecError::NotImplemented('n', "float")), Some(FormatType::GeneralFormatUpper) => { let precision = if precision == 0 { 1 } else { precision }; Ok(float_ops::format_general( @@ -524,14 +520,14 @@ impl FormatSpec { } #[inline] - fn format_int_radix(&self, magnitude: BigInt, radix: u32) -> Result { + fn format_int_radix(&self, magnitude: BigInt, radix: u32) -> Result { match self.precision { - Some(_) => Err("Precision not allowed in integer format specifier".to_owned()), + Some(_) => Err(FormatSpecError::PrecisionNotAllowed), None => Ok(magnitude.to_str_radix(radix)), } } - pub fn format_int(&self, num: &BigInt) -> Result { + pub fn format_int(&self, num: &BigInt) -> Result { self.validate_format(FormatType::Decimal)?; let magnitude = num.abs(); let prefix = if self.alternate_form { @@ -545,13 +541,13 @@ impl FormatSpec { } else { "" }; - let raw_magnitude_str: Result = match self.format_type { + let raw_magnitude_str: Result = match self.format_type { Some(FormatType::Binary) => self.format_int_radix(magnitude, 2), Some(FormatType::Decimal) => self.format_int_radix(magnitude, 10), Some(FormatType::Octal) => self.format_int_radix(magnitude, 8), Some(FormatType::HexLower) => self.format_int_radix(magnitude, 16), Some(FormatType::HexUpper) => match self.precision { - Some(_) => Err("Precision not allowed in integer format specifier".to_owned()), + Some(_) => Err(FormatSpecError::PrecisionNotAllowed), None => { let mut result = magnitude.to_str_radix(16); result.make_ascii_uppercase(); @@ -559,20 +555,13 @@ impl FormatSpec { } }, Some(FormatType::Number) => self.format_int_radix(magnitude, 10), - Some(FormatType::String) => { - Err("Unknown format code 's' for object of type 'int'".to_owned()) - } + Some(FormatType::String) => Err(FormatSpecError::UnknownFormatCode('s', "int")), Some(FormatType::Character) => match (self.sign, self.alternate_form) { - (Some(_), _) => { - Err("Sign not allowed with integer format specifier 'c'".to_owned()) - } - (_, true) => Err( - "Alternate form (#) not allowed with integer format specifier 'c'".to_owned(), - ), + (Some(_), _) => Err(FormatSpecError::NotAllowed("Sign")), + (_, true) => Err(FormatSpecError::NotAllowed("Alternate form (#)")), (_, _) => match num.to_u32() { Some(n) if n <= 0x10ffff => Ok(std::char::from_u32(n).unwrap().to_string()), - // TODO: raise OverflowError - Some(_) | None => Err("%c arg not in range(0x110000)".to_owned()), + Some(_) | None => Err(FormatSpecError::CodeNotInRange), }, }, Some(FormatType::GeneralFormatUpper) @@ -583,7 +572,7 @@ impl FormatSpec { | Some(FormatType::ExponentLower) | Some(FormatType::Percentage) => match num.to_f64() { Some(float) => return self.format_float(float), - _ => Err("Unable to convert int to float".to_owned()), + _ => Err(FormatSpecError::UnableToConvert), }, None => self.format_int_radix(magnitude, 10), }; @@ -605,7 +594,7 @@ impl FormatSpec { ) } - pub fn format_string(&self, s: &BorrowedStr) -> Result { + pub fn format_string(&self, s: &BorrowedStr) -> Result { self.validate_format(FormatType::String)?; match self.format_type { Some(FormatType::String) | None => self @@ -618,9 +607,7 @@ impl FormatSpec { }), _ => { let ch = char::from(self.format_type.as_ref().unwrap()); - Err(format!( - "Unknown format code '{ch}' for object of type 'str'", - )) + Err(FormatSpecError::UnknownFormatCode(ch, "str")) } } } @@ -630,7 +617,7 @@ impl FormatSpec { magnitude_str: &BorrowedStr, sign_str: &str, default_align: FormatAlign, - ) -> Result { + ) -> Result { let align = self.align.unwrap_or(default_align); let num_chars = magnitude_str.char_len(); @@ -670,6 +657,20 @@ impl FormatSpec { } } +#[derive(Debug, PartialEq)] +pub enum FormatSpecError { + DecimalDigitsTooMany, + PrecisionTooBig, + InvalidFormatSpecifier, + UnspecifiedFormat(char, char), + UnknownFormatCode(char, &'static str), + PrecisionNotAllowed, + NotAllowed(&'static str), + UnableToConvert, + CodeNotInRange, + NotImplemented(char, &'static str), +} + #[derive(Debug, PartialEq)] pub enum FormatParseError { UnmatchedBracket, @@ -683,7 +684,7 @@ pub enum FormatParseError { } impl FromStr for FormatSpec { - type Err = String; + type Err = FormatSpecError; fn from_str(s: &str) -> Result { FormatSpec::parse(s) } @@ -1104,31 +1105,31 @@ mod tests { fn test_format_invalid_specification() { assert_eq!( FormatSpec::parse("%3"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse(".2fa"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse("ds"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse("x+"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse("b4"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse("o!"), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); assert_eq!( FormatSpec::parse("d "), - Err("Invalid format specifier".to_owned()) + Err(FormatSpecError::InvalidFormatSpecifier) ); } diff --git a/extra_tests/snippets/builtin_format.py b/extra_tests/snippets/builtin_format.py index dca2071042..2ee7a9b862 100644 --- a/extra_tests/snippets/builtin_format.py +++ b/extra_tests/snippets/builtin_format.py @@ -67,10 +67,10 @@ def test_zero_padding(): assert f"{1234:.3g}" == "1.23e+03" assert f"{1234567:.6G}" == "1.23457E+06" assert f'{"🐍":4}' == "🐍 " - assert_raises(ValueError, "{:,o}".format, 1, _msg="ValueError: Cannot specify ',' with 'o'.") 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'.") +assert_raises(OverflowError, "{:c}".format, 0x110000, _msg="OverflowError: %c arg not in range(0x110000)") diff --git a/vm/src/builtins/float.rs b/vm/src/builtins/float.rs index 91f0221ce0..76862aacaa 100644 --- a/vm/src/builtins/float.rs +++ b/vm/src/builtins/float.rs @@ -6,7 +6,7 @@ use crate::{ class::PyClassImpl, common::format::FormatSpec, common::{float_ops, hash}, - convert::{ToPyObject, ToPyResult}, + convert::{IntoPyException, ToPyObject, ToPyResult}, function::{ ArgBytesLike, OptionalArg, OptionalOption, PyArithmeticValue::{self, *}, @@ -191,7 +191,7 @@ impl PyFloat { fn format(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult { FormatSpec::parse(spec.as_str()) .and_then(|format_spec| format_spec.format_float(self.value)) - .map_err(|msg| vm.new_value_error(msg)) + .map_err(|err| err.into_pyexception(vm)) } #[pystaticmethod(magic)] diff --git a/vm/src/builtins/int.rs b/vm/src/builtins/int.rs index 224f8c5b74..d7fd2f8de0 100644 --- a/vm/src/builtins/int.rs +++ b/vm/src/builtins/int.rs @@ -5,7 +5,7 @@ use crate::{ class::PyClassImpl, common::format::FormatSpec, common::hash, - convert::{ToPyObject, ToPyResult}, + convert::{IntoPyException, ToPyObject, ToPyResult}, function::{ ArgByteOrder, ArgIntoBool, OptionalArg, OptionalOption, PyArithmeticValue, PyComparisonValue, @@ -570,7 +570,7 @@ impl PyInt { fn format(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult { FormatSpec::parse(spec.as_str()) .and_then(|format_spec| format_spec.format_int(&self.value)) - .map_err(|msg| vm.new_value_error(msg)) + .map_err(|err| err.into_pyexception(vm)) } #[pymethod(magic)] diff --git a/vm/src/builtins/str.rs b/vm/src/builtins/str.rs index b57d9590f9..bf0d17276a 100644 --- a/vm/src/builtins/str.rs +++ b/vm/src/builtins/str.rs @@ -11,7 +11,7 @@ use crate::{ format::{FormatSpec, FormatString, FromTemplate}, str::{BorrowedStr, PyStrKind, PyStrKindData}, }, - convert::{ToPyException, ToPyObject}, + convert::{IntoPyException, ToPyException, ToPyObject}, format::{format, format_map}, function::{ArgIterable, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue}, intern::PyInterned, @@ -732,7 +732,7 @@ impl PyStr { fn format_str(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult { FormatSpec::parse(spec.as_str()) .and_then(|format_spec| format_spec.format_string(self.borrow())) - .map_err(|msg| vm.new_value_error(msg)) + .map_err(|err| err.into_pyexception(vm)) } /// Return a titlecased version of the string where words start with an diff --git a/vm/src/format.rs b/vm/src/format.rs index d839540c3e..8b51fa0aed 100644 --- a/vm/src/format.rs +++ b/vm/src/format.rs @@ -1,12 +1,54 @@ use crate::{ builtins::{PyBaseExceptionRef, PyStrRef}, common::format::*, - convert::ToPyException, + convert::{IntoPyException, ToPyException}, function::FuncArgs, stdlib::builtins, AsObject, PyObject, PyObjectRef, PyResult, VirtualMachine, }; +impl IntoPyException for FormatSpecError { + fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef { + match self { + FormatSpecError::DecimalDigitsTooMany => { + vm.new_value_error("Too many decimal digits in format string".to_owned()) + } + FormatSpecError::PrecisionTooBig => vm.new_value_error("Precision too big".to_owned()), + FormatSpecError::InvalidFormatSpecifier => { + vm.new_value_error("Invalid format specifier".to_owned()) + } + FormatSpecError::UnspecifiedFormat(c1, c2) => { + let msg = format!("Cannot specify '{}' with '{}'.", c1, c2); + vm.new_value_error(msg) + } + FormatSpecError::UnknownFormatCode(c, s) => { + let msg = format!("Unknown format code '{}' for object of type '{}'", c, s); + vm.new_value_error(msg) + } + FormatSpecError::PrecisionNotAllowed => { + vm.new_value_error("Precision not allowed in integer format specifier".to_owned()) + } + FormatSpecError::NotAllowed(s) => { + let msg = format!("{} not allowed with integer format specifier 'c'", s); + vm.new_value_error(msg) + } + FormatSpecError::UnableToConvert => { + vm.new_value_error("Unable to convert int to float".to_owned()) + } + FormatSpecError::CodeNotInRange => { + vm.new_overflow_error("%c arg not in range(0x110000)".to_owned()) + } + FormatSpecError::NotImplemented(c, s) => { + let msg = format!( + "Format code '{}' for object of type '{}' not implemented yet", + c, s + ); + vm.new_value_error(msg) + } + } + } +} + impl ToPyException for FormatParseError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { match self {