Skip to content

Commit 22a5a83

Browse files
yt2byt2b
authored
PyInt.format only raises ValueError (RustPython#4428)
1 parent 604aa42 commit 22a5a83

File tree

6 files changed

+98
-55
lines changed

6 files changed

+98
-55
lines changed

‎common/src/format.rs

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ fn parse_fill_and_align(text: &str) -> (Option<char>, Option<FormatAlign>, &str)
223223
}
224224
}
225225

226-
fn parse_number(text: &str) -> Result<(Option<usize>, &str), &'static str> {
226+
fn parse_number(text: &str) -> Result<(Option<usize>, &str), FormatSpecError> {
227227
let num_digits: usize = get_num_digits(text);
228228
if num_digits == 0 {
229229
return Ok((None, text));
@@ -232,7 +232,7 @@ fn parse_number(text: &str) -> Result<(Option<usize>, &str), &'static str> {
232232
Ok((Some(num), &text[num_digits..]))
233233
} else {
234234
// NOTE: this condition is different from CPython
235-
Err("Too many decimal digits in format string")
235+
Err(FormatSpecError::DecimalDigitsTooMany)
236236
}
237237
}
238238

@@ -252,14 +252,14 @@ fn parse_zero(text: &str) -> (bool, &str) {
252252
}
253253
}
254254

255-
fn parse_precision(text: &str) -> Result<(Option<usize>, &str), &'static str> {
255+
fn parse_precision(text: &str) -> Result<(Option<usize>, &str), FormatSpecError> {
256256
let mut chars = text.chars();
257257
Ok(match chars.next() {
258258
Some('.') => {
259259
let (size, remaining) = parse_number(chars.as_str())?;
260260
if let Some(size) = size {
261261
if size > i32::MAX as usize {
262-
return Err("Precision too big");
262+
return Err(FormatSpecError::PrecisionTooBig);
263263
}
264264
(Some(size), remaining)
265265
} else {
@@ -271,7 +271,7 @@ fn parse_precision(text: &str) -> Result<(Option<usize>, &str), &'static str> {
271271
}
272272

273273
impl FormatSpec {
274-
pub fn parse(text: &str) -> Result<Self, String> {
274+
pub fn parse(text: &str) -> Result<Self, FormatSpecError> {
275275
// get_integer in CPython
276276
let (preconversor, text) = FormatPreconversor::parse(text);
277277
let (mut fill, mut align, text) = parse_fill_and_align(text);
@@ -283,7 +283,7 @@ impl FormatSpec {
283283
let (precision, text) = parse_precision(text)?;
284284
let (format_type, text) = FormatType::parse(text);
285285
if !text.is_empty() {
286-
return Err("Invalid format specifier".to_owned());
286+
return Err(FormatSpecError::InvalidFormatSpecifier);
287287
}
288288

289289
if zero && fill.is_none() {
@@ -359,7 +359,7 @@ impl FormatSpec {
359359
magnitude_str
360360
}
361361

362-
fn validate_format(&self, default_format_type: FormatType) -> Result<(), String> {
362+
fn validate_format(&self, default_format_type: FormatType) -> Result<(), FormatSpecError> {
363363
let format_type = self.format_type.as_ref().unwrap_or(&default_format_type);
364364
match (&self.grouping_option, format_type) {
365365
(
@@ -373,14 +373,14 @@ impl FormatSpec {
373373
| FormatType::Number,
374374
) => {
375375
let ch = char::from(format_type);
376-
Err(format!("Cannot specify ',' with '{ch}'."))
376+
Err(FormatSpecError::UnspecifiedFormat(',', ch))
377377
}
378378
(
379379
Some(FormatGrouping::Underscore),
380380
FormatType::String | FormatType::Character | FormatType::Number,
381381
) => {
382382
let ch = char::from(format_type);
383-
Err(format!("Cannot specify '_' with '{ch}'."))
383+
Err(FormatSpecError::UnspecifiedFormat('_', ch))
384384
}
385385
_ => Ok(()),
386386
}
@@ -422,11 +422,11 @@ impl FormatSpec {
422422
}
423423
}
424424

425-
pub fn format_float(&self, num: f64) -> Result<String, String> {
425+
pub fn format_float(&self, num: f64) -> Result<String, FormatSpecError> {
426426
self.validate_format(FormatType::FixedPointLower)?;
427427
let precision = self.precision.unwrap_or(6);
428428
let magnitude = num.abs();
429-
let raw_magnitude_str: Result<String, String> = match self.format_type {
429+
let raw_magnitude_str: Result<String, FormatSpecError> = match self.format_type {
430430
Some(FormatType::FixedPointUpper) => Ok(float_ops::format_fixed(
431431
precision,
432432
magnitude,
@@ -445,13 +445,9 @@ impl FormatSpec {
445445
| Some(FormatType::String)
446446
| Some(FormatType::Character) => {
447447
let ch = char::from(self.format_type.as_ref().unwrap());
448-
Err(format!(
449-
"Unknown format code '{ch}' for object of type 'float'",
450-
))
451-
}
452-
Some(FormatType::Number) => {
453-
Err("Format code 'n' for object of type 'float' not implemented yet".to_owned())
448+
Err(FormatSpecError::UnknownFormatCode(ch, "float"))
454449
}
450+
Some(FormatType::Number) => Err(FormatSpecError::NotImplemented('n', "float")),
455451
Some(FormatType::GeneralFormatUpper) => {
456452
let precision = if precision == 0 { 1 } else { precision };
457453
Ok(float_ops::format_general(
@@ -524,14 +520,14 @@ impl FormatSpec {
524520
}
525521

526522
#[inline]
527-
fn format_int_radix(&self, magnitude: BigInt, radix: u32) -> Result<String, String> {
523+
fn format_int_radix(&self, magnitude: BigInt, radix: u32) -> Result<String, FormatSpecError> {
528524
match self.precision {
529-
Some(_) => Err("Precision not allowed in integer format specifier".to_owned()),
525+
Some(_) => Err(FormatSpecError::PrecisionNotAllowed),
530526
None => Ok(magnitude.to_str_radix(radix)),
531527
}
532528
}
533529

534-
pub fn format_int(&self, num: &BigInt) -> Result<String, String> {
530+
pub fn format_int(&self, num: &BigInt) -> Result<String, FormatSpecError> {
535531
self.validate_format(FormatType::Decimal)?;
536532
let magnitude = num.abs();
537533
let prefix = if self.alternate_form {
@@ -545,34 +541,27 @@ impl FormatSpec {
545541
} else {
546542
""
547543
};
548-
let raw_magnitude_str: Result<String, String> = match self.format_type {
544+
let raw_magnitude_str: Result<String, FormatSpecError> = match self.format_type {
549545
Some(FormatType::Binary) => self.format_int_radix(magnitude, 2),
550546
Some(FormatType::Decimal) => self.format_int_radix(magnitude, 10),
551547
Some(FormatType::Octal) => self.format_int_radix(magnitude, 8),
552548
Some(FormatType::HexLower) => self.format_int_radix(magnitude, 16),
553549
Some(FormatType::HexUpper) => match self.precision {
554-
Some(_) => Err("Precision not allowed in integer format specifier".to_owned()),
550+
Some(_) => Err(FormatSpecError::PrecisionNotAllowed),
555551
None => {
556552
let mut result = magnitude.to_str_radix(16);
557553
result.make_ascii_uppercase();
558554
Ok(result)
559555
}
560556
},
561557
Some(FormatType::Number) => self.format_int_radix(magnitude, 10),
562-
Some(FormatType::String) => {
563-
Err("Unknown format code 's' for object of type 'int'".to_owned())
564-
}
558+
Some(FormatType::String) => Err(FormatSpecError::UnknownFormatCode('s', "int")),
565559
Some(FormatType::Character) => match (self.sign, self.alternate_form) {
566-
(Some(_), _) => {
567-
Err("Sign not allowed with integer format specifier 'c'".to_owned())
568-
}
569-
(_, true) => Err(
570-
"Alternate form (#) not allowed with integer format specifier 'c'".to_owned(),
571-
),
560+
(Some(_), _) => Err(FormatSpecError::NotAllowed("Sign")),
561+
(_, true) => Err(FormatSpecError::NotAllowed("Alternate form (#)")),
572562
(_, _) => match num.to_u32() {
573563
Some(n) if n <= 0x10ffff => Ok(std::char::from_u32(n).unwrap().to_string()),
574-
// TODO: raise OverflowError
575-
Some(_) | None => Err("%c arg not in range(0x110000)".to_owned()),
564+
Some(_) | None => Err(FormatSpecError::CodeNotInRange),
576565
},
577566
},
578567
Some(FormatType::GeneralFormatUpper)
@@ -583,7 +572,7 @@ impl FormatSpec {
583572
| Some(FormatType::ExponentLower)
584573
| Some(FormatType::Percentage) => match num.to_f64() {
585574
Some(float) => return self.format_float(float),
586-
_ => Err("Unable to convert int to float".to_owned()),
575+
_ => Err(FormatSpecError::UnableToConvert),
587576
},
588577
None => self.format_int_radix(magnitude, 10),
589578
};
@@ -605,7 +594,7 @@ impl FormatSpec {
605594
)
606595
}
607596

608-
pub fn format_string(&self, s: &BorrowedStr) -> Result<String, String> {
597+
pub fn format_string(&self, s: &BorrowedStr) -> Result<String, FormatSpecError> {
609598
self.validate_format(FormatType::String)?;
610599
match self.format_type {
611600
Some(FormatType::String) | None => self
@@ -618,9 +607,7 @@ impl FormatSpec {
618607
}),
619608
_ => {
620609
let ch = char::from(self.format_type.as_ref().unwrap());
621-
Err(format!(
622-
"Unknown format code '{ch}' for object of type 'str'",
623-
))
610+
Err(FormatSpecError::UnknownFormatCode(ch, "str"))
624611
}
625612
}
626613
}
@@ -630,7 +617,7 @@ impl FormatSpec {
630617
magnitude_str: &BorrowedStr,
631618
sign_str: &str,
632619
default_align: FormatAlign,
633-
) -> Result<String, String> {
620+
) -> Result<String, FormatSpecError> {
634621
let align = self.align.unwrap_or(default_align);
635622

636623
let num_chars = magnitude_str.char_len();
@@ -670,6 +657,20 @@ impl FormatSpec {
670657
}
671658
}
672659

660+
#[derive(Debug, PartialEq)]
661+
pub enum FormatSpecError {
662+
DecimalDigitsTooMany,
663+
PrecisionTooBig,
664+
InvalidFormatSpecifier,
665+
UnspecifiedFormat(char, char),
666+
UnknownFormatCode(char, &'static str),
667+
PrecisionNotAllowed,
668+
NotAllowed(&'static str),
669+
UnableToConvert,
670+
CodeNotInRange,
671+
NotImplemented(char, &'static str),
672+
}
673+
673674
#[derive(Debug, PartialEq)]
674675
pub enum FormatParseError {
675676
UnmatchedBracket,
@@ -683,7 +684,7 @@ pub enum FormatParseError {
683684
}
684685

685686
impl FromStr for FormatSpec {
686-
type Err = String;
687+
type Err = FormatSpecError;
687688
fn from_str(s: &str) -> Result<Self, Self::Err> {
688689
FormatSpec::parse(s)
689690
}
@@ -1104,31 +1105,31 @@ mod tests {
11041105
fn test_format_invalid_specification() {
11051106
assert_eq!(
11061107
FormatSpec::parse("%3"),
1107-
Err("Invalid format specifier".to_owned())
1108+
Err(FormatSpecError::InvalidFormatSpecifier)
11081109
);
11091110
assert_eq!(
11101111
FormatSpec::parse(".2fa"),
1111-
Err("Invalid format specifier".to_owned())
1112+
Err(FormatSpecError::InvalidFormatSpecifier)
11121113
);
11131114
assert_eq!(
11141115
FormatSpec::parse("ds"),
1115-
Err("Invalid format specifier".to_owned())
1116+
Err(FormatSpecError::InvalidFormatSpecifier)
11161117
);
11171118
assert_eq!(
11181119
FormatSpec::parse("x+"),
1119-
Err("Invalid format specifier".to_owned())
1120+
Err(FormatSpecError::InvalidFormatSpecifier)
11201121
);
11211122
assert_eq!(
11221123
FormatSpec::parse("b4"),
1123-
Err("Invalid format specifier".to_owned())
1124+
Err(FormatSpecError::InvalidFormatSpecifier)
11241125
);
11251126
assert_eq!(
11261127
FormatSpec::parse("o!"),
1127-
Err("Invalid format specifier".to_owned())
1128+
Err(FormatSpecError::InvalidFormatSpecifier)
11281129
);
11291130
assert_eq!(
11301131
FormatSpec::parse("d "),
1131-
Err("Invalid format specifier".to_owned())
1132+
Err(FormatSpecError::InvalidFormatSpecifier)
11321133
);
11331134
}
11341135

‎extra_tests/snippets/builtin_format.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ def test_zero_padding():
6767
assert f"{1234:.3g}" == "1.23e+03"
6868
assert f"{1234567:.6G}" == "1.23457E+06"
6969
assert f'{"🐍":4}' == "🐍 "
70-
7170
assert_raises(ValueError, "{:,o}".format, 1, _msg="ValueError: Cannot specify ',' with 'o'.")
7271
assert_raises(ValueError, "{:_n}".format, 1, _msg="ValueError: Cannot specify '_' with 'n'.")
7372
assert_raises(ValueError, "{:,o}".format, 1.0, _msg="ValueError: Cannot specify ',' with 'o'.")
7473
assert_raises(ValueError, "{:_n}".format, 1.0, _msg="ValueError: Cannot specify '_' with 'n'.")
7574
assert_raises(ValueError, "{:,}".format, "abc", _msg="ValueError: Cannot specify ',' with 's'.")
7675
assert_raises(ValueError, "{:,x}".format, "abc", _msg="ValueError: Cannot specify ',' with 'x'.")
76+
assert_raises(OverflowError, "{:c}".format, 0x110000, _msg="OverflowError: %c arg not in range(0x110000)")

‎vm/src/builtins/float.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
class::PyClassImpl,
77
common::format::FormatSpec,
88
common::{float_ops, hash},
9-
convert::{ToPyObject, ToPyResult},
9+
convert::{IntoPyException, ToPyObject, ToPyResult},
1010
function::{
1111
ArgBytesLike, OptionalArg, OptionalOption,
1212
PyArithmeticValue::{self, *},
@@ -191,7 +191,7 @@ impl PyFloat {
191191
fn format(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult<String> {
192192
FormatSpec::parse(spec.as_str())
193193
.and_then(|format_spec| format_spec.format_float(self.value))
194-
.map_err(|msg| vm.new_value_error(msg))
194+
.map_err(|err| err.into_pyexception(vm))
195195
}
196196

197197
#[pystaticmethod(magic)]

‎vm/src/builtins/int.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
class::PyClassImpl,
66
common::format::FormatSpec,
77
common::hash,
8-
convert::{ToPyObject, ToPyResult},
8+
convert::{IntoPyException, ToPyObject, ToPyResult},
99
function::{
1010
ArgByteOrder, ArgIntoBool, OptionalArg, OptionalOption, PyArithmeticValue,
1111
PyComparisonValue,
@@ -570,7 +570,7 @@ impl PyInt {
570570
fn format(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult<String> {
571571
FormatSpec::parse(spec.as_str())
572572
.and_then(|format_spec| format_spec.format_int(&self.value))
573-
.map_err(|msg| vm.new_value_error(msg))
573+
.map_err(|err| err.into_pyexception(vm))
574574
}
575575

576576
#[pymethod(magic)]

‎vm/src/builtins/str.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
format::{FormatSpec, FormatString, FromTemplate},
1212
str::{BorrowedStr, PyStrKind, PyStrKindData},
1313
},
14-
convert::{ToPyException, ToPyObject},
14+
convert::{IntoPyException, ToPyException, ToPyObject},
1515
format::{format, format_map},
1616
function::{ArgIterable, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue},
1717
intern::PyInterned,
@@ -732,7 +732,7 @@ impl PyStr {
732732
fn format_str(&self, spec: PyStrRef, vm: &VirtualMachine) -> PyResult<String> {
733733
FormatSpec::parse(spec.as_str())
734734
.and_then(|format_spec| format_spec.format_string(self.borrow()))
735-
.map_err(|msg| vm.new_value_error(msg))
735+
.map_err(|err| err.into_pyexception(vm))
736736
}
737737

738738
/// Return a titlecased version of the string where words start with an

‎vm/src/format.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,54 @@
11
use crate::{
22
builtins::{PyBaseExceptionRef, PyStrRef},
33
common::format::*,
4-
convert::ToPyException,
4+
convert::{IntoPyException, ToPyException},
55
function::FuncArgs,
66
stdlib::builtins,
77
AsObject, PyObject, PyObjectRef, PyResult, VirtualMachine,
88
};
99

10+
impl IntoPyException for FormatSpecError {
11+
fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef {
12+
match self {
13+
FormatSpecError::DecimalDigitsTooMany => {
14+
vm.new_value_error("Too many decimal digits in format string".to_owned())
15+
}
16+
FormatSpecError::PrecisionTooBig => vm.new_value_error("Precision too big".to_owned()),
17+
FormatSpecError::InvalidFormatSpecifier => {
18+
vm.new_value_error("Invalid format specifier".to_owned())
19+
}
20+
FormatSpecError::UnspecifiedFormat(c1, c2) => {
21+
let msg = format!("Cannot specify '{}' with '{}'.", c1, c2);
22+
vm.new_value_error(msg)
23+
}
24+
FormatSpecError::UnknownFormatCode(c, s) => {
25+
let msg = format!("Unknown format code '{}' for object of type '{}'", c, s);
26+
vm.new_value_error(msg)
27+
}
28+
FormatSpecError::PrecisionNotAllowed => {
29+
vm.new_value_error("Precision not allowed in integer format specifier".to_owned())
30+
}
31+
FormatSpecError::NotAllowed(s) => {
32+
let msg = format!("{} not allowed with integer format specifier 'c'", s);
33+
vm.new_value_error(msg)
34+
}
35+
FormatSpecError::UnableToConvert => {
36+
vm.new_value_error("Unable to convert int to float".to_owned())
37+
}
38+
FormatSpecError::CodeNotInRange => {
39+
vm.new_overflow_error("%c arg not in range(0x110000)".to_owned())
40+
}
41+
FormatSpecError::NotImplemented(c, s) => {
42+
let msg = format!(
43+
"Format code '{}' for object of type '{}' not implemented yet",
44+
c, s
45+
);
46+
vm.new_value_error(msg)
47+
}
48+
}
49+
}
50+
}
51+
1052
impl ToPyException for FormatParseError {
1153
fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef {
1254
match self {

0 commit comments

Comments
 (0)