Reject the string format flags CPython refuses - #8571
Conversation
A sign, a space or an alternate form parsed fine on a string spec and
then had no effect:
>>> format('ab', '+')
'ab'
>>> f"{'ab':RustPython#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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughString formatting now rejects unsupported sign and alternate-form flags for strings. The formatter reports errors in CPython precedence order, the VM maps them to ChangesString format validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change makes string formatting reject flags that CPython rejects, with tests and checks reported passing; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
A sign, a space or an alternate form in a string format spec is accepted and then dropped:
format_stringincrates/common/src/format.rsalready turns downzand an explicit=alignment for strings, with the wording CPython uses. These three were never checked, so the spec parses, the flag means nothing to a string, and the caller gets plain padding where CPython refuses the spec outright.The three checks sit next to the two that were already there. Their order is the one CPython reports in, which I read off these, under CPython 3.14:
+z#5zand#x=z#5zcomes before#x=#5#comes before the=alignmentFormatSpecErrorgains one variant that carries the flag name, so the three share a message built the same way asStringAlignmentFlag.Tests
I ran the string spec shapes side by side under CPython 3.14 and this build: the four flags, fill and align pairs, width, precision, grouping, the format codes, and specs where two errors compete. Two differences are left afterwards, neither on this path.
Invalid format specifieris still missing the spec and the type, which #8477 covers. Andformat('ab', '+,5')reports the unknown format code here while CPython reports the grouping.extra_tests/snippets/builtin_format.py: eight specs checked by message, next to the=8scase that was already there. It passes under CPython 3.14 too.crates/common/src/format.rs, one per flag and one for the order in the tablecargo run --release -- -m test test_str test_format test_fstring: SUCCESS, 138, 18 and 90 testscargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi: no failurescargo fmt --check,ruff format --check,ruff check --select I, and clippy with the flags CI uses: cleantest_str.test_formatasserts all three, but its marker records an unrelated reason to stay an expected failure ('{0.}'.format()raises ValueError instead of IndexError), so the snippet carries the coverage instead.AI assistance
Claude Code (claude-opus-5) helped with the comparison against CPython, the change and this description. I read the final diff and ran the checks above on Linux.
Summary by CodeRabbit
Bug Fixes
#) flags.Tests
ValueErrormessages.