diff --git a/crates/common/src/format.rs b/crates/common/src/format.rs index 2c545de07..7a5e3ac0f 100644 --- a/crates/common/src/format.rs +++ b/crates/common/src/format.rs @@ -1112,9 +1112,19 @@ impl FormatSpec { self.validate_format(FormatType::String)?; match self.format_type { Some(FormatType::String) | None => { + // CPython rejects these four in this order: sign, z, #, then '='. + if let Some(sign) = self.sign { + return Err(FormatSpecError::StringSpecNotAllowed(match sign { + FormatSign::MinusOrSpace => "Space", + FormatSign::Plus | FormatSign::Minus => "Sign", + })); + } if self.no_neg_0 { return Err(FormatSpecError::NegativeZeroCoercionNotAllowed("string")); } + if self.alternate_form { + return Err(FormatSpecError::StringSpecNotAllowed("Alternate form (#)")); + } if self.align == Some(FormatAlign::AfterSign) && self.align_specified { return Err(FormatSpecError::StringAlignmentFlag); } @@ -1363,6 +1373,7 @@ pub enum FormatSpecError { AlignmentFlag, NegativeZeroCoercionNotAllowed(&'static str), StringAlignmentFlag, + StringSpecNotAllowed(&'static str), NotImplemented(char, &'static str), } @@ -1845,6 +1856,52 @@ mod tests { ); } + #[test] + fn format_string_rejects_sign_space_and_alternate_form() { + let value = "result".to_owned(); + let cases = [ + ("+", "Sign"), + ("-", "Sign"), + ("+8s", "Sign"), + (" ", "Space"), + (" 8s", "Space"), + ("#", "Alternate form (#)"), + ("#8s", "Alternate form (#)"), + ]; + + for (text, flag) in cases { + let spec = FormatSpec::parse(text).unwrap(); + assert_eq!( + spec.format_string(&value), + Err(FormatSpecError::StringSpecNotAllowed(flag)), + "{text}" + ); + } + } + + #[test] + fn format_string_reports_the_flag_cpython_reports_first() { + let value = "result".to_owned(); + // Sign beats z, z beats the alternate form, and the alternate form + // beats an explicit '=' alignment. + let cases = [ + ("+z#5", FormatSpecError::StringSpecNotAllowed("Sign")), + ( + "x=z#5", + FormatSpecError::NegativeZeroCoercionNotAllowed("string"), + ), + ( + "x=#5", + FormatSpecError::StringSpecNotAllowed("Alternate form (#)"), + ), + ]; + + for (text, expected) in cases { + let spec = FormatSpec::parse(text).unwrap(); + assert_eq!(spec.format_string(&value), Err(expected), "{text}"); + } + } + #[test] fn format_complex_rejects_zero_padding_before_after_sign_alignment() { for text in [ diff --git a/crates/vm/src/format.rs b/crates/vm/src/format.rs index 4099cbd9b..75f5c32f0 100644 --- a/crates/vm/src/format.rs +++ b/crates/vm/src/format.rs @@ -86,6 +86,10 @@ impl IntoPyException for FormatSpecError { Self::StringAlignmentFlag => { vm.new_value_error("'=' alignment not allowed in string format specifier") } + Self::StringSpecNotAllowed(s) => { + let msg = format!("{s} not allowed in string format specifier"); + vm.new_value_error(msg) + } Self::NotImplemented(c, s) => { let msg = format!("Format code '{c}' for object of type '{s}' not implemented yet"); vm.new_value_error(msg) diff --git a/extra_tests/snippets/builtin_format.py b/extra_tests/snippets/builtin_format.py index d92ddefe8..6b7403da1 100644 --- a/extra_tests/snippets/builtin_format.py +++ b/extra_tests/snippets/builtin_format.py @@ -32,6 +32,30 @@ except ValueError as error: else: raise AssertionError("expected ValueError for '=8s' string format specifier") +# regression: a sign, a space or an alternate form used to be accepted and dropped. +for spec, flag in [ + ("+", "Sign"), + ("-", "Sign"), + ("+5", "Sign"), + (" ", "Space"), + (" 5", "Space"), + ("#", "Alternate form (#)"), + ("#5", "Alternate form (#)"), + ("+.2", "Sign"), +]: + try: + format("result", spec) + except ValueError as error: + expected = f"{flag} not allowed in string format specifier" + if str(error) != expected: + raise AssertionError( + f"{spec!r}: unexpected error message: {error}" + ) from error + else: + raise AssertionError( + f"expected ValueError for {spec!r} string format specifier" + ) + # regression: unknown conversion specifiers used to be silently ignored instead of raising. # The ValueError case itself is covered by test_str, but here we're testing the error message. try: