From 30be3909436fbcddba675a114986e621997846a2 Mon Sep 17 00:00:00 2001 From: Luan Taraschi <130802253+luantaraschi@users.noreply.github.com> Date: Sat, 22 Aug 2026 00:43:38 -0300 Subject: [PATCH] Reject the string format flags CPython refuses (#8571) A sign, a space or an alternate form parsed fine on a string spec and then had no effect: >>> format('ab', '+') 'ab' >>> f"{'ab':#5}" 'ab ' CPython raises ValueError for all three. format_string already refused z and an explicit '=' alignment with the same family of messages, so the three checks go next to those, in the order CPython reports them: sign, then z, then the alternate form, then the alignment. Assisted-by: Claude Code:claude-opus-5 --- crates/common/src/format.rs | 57 ++++++++++++++++++++++++++ crates/vm/src/format.rs | 4 ++ extra_tests/snippets/builtin_format.py | 24 +++++++++++ 3 files changed, 85 insertions(+) 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: