diff --git a/Lib/test/test_format.py b/Lib/test/test_format.py index 9ad3eb8269..abacda2acc 100644 --- a/Lib/test/test_format.py +++ b/Lib/test/test_format.py @@ -284,8 +284,6 @@ class FormatTest(unittest.TestCase): test_exc_common('%x', 3.14, TypeError, "%x format: an integer is required, not float") - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_str_format(self): testformat("%r", "\u0378", "'\\u0378'") # non printable testformat("%a", "\u0378", "'\\u0378'") # non printable @@ -319,8 +317,6 @@ class FormatTest(unittest.TestCase): else: raise TestFailed('"%*d"%(maxsize, -127) should fail') - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_bytes_and_bytearray_format(self): # %c will insert a single byte, either from an int in range(256), or # from a bytes argument of length 1, not from a str. diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 473d3c65cf..c166b1fd81 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -3,9 +3,10 @@ use crate::common::float_ops; use crate::{ - builtins::{try_f64_to_bigint, tuple, PyBytes, PyFloat, PyInt, PyStr}, + builtins::{try_f64_to_bigint, tuple, PyByteArray, PyBytes, PyFloat, PyInt, PyStr}, function::ArgIntoFloat, protocol::PyBuffer, + stdlib::builtins, ItemProtocol, PyObjectRef, PyResult, TryFromBorrowedObject, TryFromObject, TypeProtocol, VirtualMachine, }; @@ -177,7 +178,7 @@ impl CFormatSpec { Some(CFormatQuantity::Amount(width)) => cmp::max(width, num_chars), _ => num_chars, }; - let fill_chars_needed = width - num_chars; + let fill_chars_needed = width.saturating_sub(num_chars); let fill_string = CFormatSpec::compute_fill_string(fill_char, fill_chars_needed); if !fill_string.is_empty() { @@ -359,10 +360,11 @@ impl CFormatSpec { fn bytes_format(&self, vm: &VirtualMachine, obj: PyObjectRef) -> PyResult> { match &self.format_type { CFormatType::String(preconversor) => match preconversor { + // Unlike strings, %r and %a are identical for bytes: the behaviour corresponds to + // %a for strings (not %r) CFormatPreconversor::Repr | CFormatPreconversor::Ascii => { - let s = obj.repr(vm)?; - let s = self.format_string(s.as_str().to_owned()); - Ok(s.into_bytes()) + let b = builtins::ascii(obj, vm)?.into(); + Ok(b) } CFormatPreconversor::Str | CFormatPreconversor::Bytes => { if let Ok(buffer) = PyBuffer::try_from_borrowed_object(vm, &obj) { @@ -432,43 +434,66 @@ impl CFormatSpec { } }, CFormatType::Float(_) => { - let value = ArgIntoFloat::try_from_object(vm, obj)?.to_f64(); + let type_name = obj.class().name().to_string(); + let value = ArgIntoFloat::try_from_object(vm, obj) + .map_err(|e| { + if e.isinstance(&vm.ctx.exceptions.type_error) { + // formatfloat in bytesobject.c generates its own specific exception + // text in this case, mirror it here. + vm.new_type_error(format!("float argument required, not {}", type_name)) + } else { + e + } + })? + .to_f64(); Ok(self.format_float(value).into_bytes()) } CFormatType::Character => { if let Some(i) = obj.payload::() { let ch = i - .as_bigint() - .to_u32() - .and_then(std::char::from_u32) - .ok_or_else(|| { - vm.new_overflow_error("%c arg not in range(0x110000)".to_owned()) - })?; + .try_to_primitive::(vm) + .map_err(|_| vm.new_overflow_error("%c arg not in range(256)".to_owned()))? + as char; return Ok(self.format_char(ch).into_bytes()); } - if let Some(s) = obj.payload::() { - if let Ok(ch) = s.as_str().chars().exactly_one() { - return Ok(self.format_char(ch).into_bytes()); + if let Some(b) = obj.payload::() { + if b.len() == 1 { + return Ok(self.format_char(b.as_bytes()[0] as char).into_bytes()); + } + } else if let Some(ba) = obj.payload::() { + let buf = ba.borrow_buf(); + if buf.len() == 1 { + return Ok(self.format_char(buf[0] as char).into_bytes()); } } - Err(vm.new_type_error("%c requires int or char".to_owned())) + Err(vm.new_type_error( + "%c requires an integer in range(256) or a single byte".to_owned(), + )) } } } - fn string_format(&self, vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + fn string_format( + &self, + vm: &VirtualMachine, + obj: PyObjectRef, + idx: &usize, + ) -> PyResult { match &self.format_type { CFormatType::String(preconversor) => { let result = match preconversor { - CFormatPreconversor::Str => obj.str(vm)?, - CFormatPreconversor::Repr | CFormatPreconversor::Ascii => obj.repr(vm)?, + CFormatPreconversor::Ascii => builtins::ascii(obj, vm)?.into(), + CFormatPreconversor::Str => obj.str(vm)?.as_str().to_owned(), + CFormatPreconversor::Repr => obj.repr(vm)?.as_str().to_owned(), CFormatPreconversor::Bytes => { - return Err(vm.new_value_error( - "unsupported format character 'b' (0x62)".to_owned(), - )); + // idx is the position of the %, we want the position of the b + return Err(vm.new_value_error(format!( + "unsupported format character 'b' (0x62) at index {}", + idx + 1 + ))); } }; - Ok(self.format_string(result.as_str().to_owned())) + Ok(self.format_string(result)) } CFormatType::Number(number_type) => match number_type { CNumberType::Decimal => match_class!(match &obj { @@ -563,7 +588,7 @@ fn try_update_quantity_from_tuple<'a, I: Iterator>( Some(CFormatQuantity::FromValuesTuple) => match elements.next() { Some(width_obj) => { if let Some(i) = width_obj.payload::() { - let i = i.try_to_primitive::(vm)? as usize; + let i = i.try_to_primitive::(vm)?.abs() as usize; *q = Some(CFormatQuantity::Amount(i)); Ok(()) } else { @@ -656,7 +681,8 @@ impl CFormatBytes { let is_mapping = values_obj.class().has_attr("__getitem__") && !values_obj.isinstance(&vm.ctx.types.tuple_type) - && !values_obj.isinstance(&vm.ctx.types.str_type); + && !values_obj.isinstance(&vm.ctx.types.bytes_type) + && !values_obj.isinstance(&vm.ctx.types.bytearray_type); if num_specifiers == 0 { // literal only @@ -674,7 +700,7 @@ impl CFormatBytes { Ok(result) } else { Err(vm.new_type_error( - "not all arguments converted during string formatting".to_owned(), + "not all arguments converted during bytes formatting".to_owned(), )) }; } @@ -730,8 +756,7 @@ impl CFormatBytes { // check that all arguments were converted if value_iter.next().is_some() && !is_mapping { - Err(vm - .new_type_error("not all arguments converted during string formatting".to_owned())) + Err(vm.new_type_error("not all arguments converted during bytes formatting".to_owned())) } else { Ok(result) } @@ -834,7 +859,7 @@ impl CFormatString { if mapping_required { // dict return if is_mapping { - for (_, part) in &self.parts { + for (idx, part) in &self.parts { match part { CFormatPart::Literal(literal) => result.push_str(literal), CFormatPart::Spec(spec) => { @@ -842,7 +867,7 @@ impl CFormatString { Some(key) => values_obj.get_item(key, vm)?, None => unreachable!(), }; - let part_result = spec.string_format(vm, value)?; + let part_result = spec.string_format(vm, value, idx)?; result.push_str(&part_result); } } @@ -861,7 +886,7 @@ impl CFormatString { }; let mut value_iter = values.iter(); - for (_, part) in &mut self.parts { + for (idx, part) in &mut self.parts { match part { CFormatPart::Literal(literal) => result.push_str(literal), CFormatPart::Spec(spec) => { @@ -874,7 +899,7 @@ impl CFormatString { vm.new_type_error("not enough arguments for format string".to_owned()) ), }?; - let part_result = spec.string_format(vm, value)?; + let part_result = spec.string_format(vm, value, idx)?; result.push_str(&part_result); } } @@ -917,7 +942,7 @@ where break; } } - return Ok(Some(CFormatQuantity::Amount(num as usize))); + return Ok(Some(CFormatQuantity::Amount(num.abs() as usize))); } } Ok(None)