Skip to content

Validate star width and precision arguments in translations - #1402

Merged
akx merged 1 commit into
python-babel:masterfrom
ice-pring:fix/check-format-star-arguments
Oct 26, 2026
Merged

akx merged 1 commit into
python-babel:masterfrom
ice-pring:fix/check-format-star-arguments

Conversation

@ice-pring

Copy link
Copy Markdown
Contributor

python_format accepts translations that add, remove, or move a * width/precision argument, even when formatting the translated string with the original arguments raises TypeError. For example:

python_format(None, Message('%*s', '%s'))  # currently accepted
'%*s' % (4, 'word')                     # 'word'
'%s' % (4, 'word')                      # TypeError

Each dynamic width or precision consumes an integer argument before the converted value, as described in the Python formatting specification. Include those arguments in the existing positional compatibility checks using the integer conversion category. Numeric widths/precisions and escaped percent signs keep their existing behavior.

Add ten incompatible examples that first verify the source formats successfully and the translation fails at runtime; all ten fail the checker assertion before this fix. Seven compatible controls cover width/precision combinations, numeric conversion compatibility, negative widths, named placeholders, and escaped percent signs. A catalog-level test also checks an incompatible plural translation.

Validation on Windows, against e2a1791, with imported CLDR 48.2:

  • CPython 3.14.5 and 3.12.14: complete pytest/coverage runs each passed 7,847 tests, with 9 skips and 2 existing expected failures.
  • Checker tests plus its module doctests: 62 passed.
  • Configured pre-commit hooks for both changed files, source Ruff lint, and git diff --check passed.

The complete runs used a workspace --basetemp path without spaces. With the default temporary directory containing a space, an existing test_compendium_with_comment assertion fails because PO comment wrapping splits the pathname. The same isolated failure was verified with the checker module restored from upstream; no test was deselected for the successful runs. One existing pytest parametrization deprecation warning remains. Other interpreter/platform combinations were not run.

@sunxiuguo

Copy link
Copy Markdown

Thanks for covering the missing star arguments. There is one newly accepted incompatible pair when stars reuse the d category:

args = (3.5, "x")
"%d%s" % args  # "3x"
"%*s" % args   # TypeError: * wants int
python_format(None, Message("%d%s", "%*s"))  # accepted at e06b245e

The base (e2a17919) rejects that pair as unbalanced; %.*s has the same issue. %d accepts floats, whereas dynamic width/precision requires an integer. Could star arguments retain a distinct compatibility category rather than reuse d, with regression coverage for these pairs?

AI assistance was used to reproduce and review this; the commands were actually executed on CPython 3.12.14 using the base/head checker modules with Babel 2.17.0 dependencies. This was not a full repository test run.

@akx akx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Starargs are pretty rare, but why not.

@codspeed

codspeed Bot commented Oct 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 88 untouched benchmarks


Comparing ice-pring:fix/check-format-star-arguments (e06b245) with master (e2a1791)

Open in CodSpeed

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.39%. Comparing base (e2a1791) to head (e06b245).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1402      +/-   ##
==========================================
+ Coverage   93.37%   93.39%   +0.02%     
==========================================
  Files          27       27              
  Lines        4922     4924       +2     
==========================================
+ Hits         4596     4599       +3     
+ Misses        326      325       -1     
Flag Coverage Δ
macos-14-3.10 92.48% <100.00%> (+0.02%) ⬆️
macos-14-3.11 92.42% <100.00%> (+0.02%) ⬆️
macos-14-3.12 92.62% <100.00%> (+0.02%) ⬆️
macos-14-3.13 92.62% <100.00%> (+0.02%) ⬆️
macos-14-3.14 92.60% <100.00%> (+0.02%) ⬆️
macos-14-3.15 92.60% <100.00%> (+0.02%) ⬆️
macos-14-3.8 92.35% <100.00%> (+0.02%) ⬆️
macos-14-3.9 92.42% <100.00%> (+0.02%) ⬆️
macos-14-pypy3.10 92.48% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.10 92.50% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.11 92.44% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.12 92.64% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.13 92.64% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.14 92.62% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.15 92.62% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.8 92.37% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-3.9 92.44% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-pypy3.10 92.50% <100.00%> (+0.02%) ⬆️
windows-2022-3.10 92.49% <100.00%> (+0.02%) ⬆️
windows-2022-3.11 92.43% <100.00%> (+0.02%) ⬆️
windows-2022-3.12 92.64% <100.00%> (+0.02%) ⬆️
windows-2022-3.13 92.64% <100.00%> (+0.02%) ⬆️
windows-2022-3.14 92.62% <100.00%> (+0.02%) ⬆️
windows-2022-3.15 92.62% <100.00%> (+0.02%) ⬆️
windows-2022-3.8 92.47% <100.00%> (+0.02%) ⬆️
windows-2022-3.9 92.43% <100.00%> (+0.02%) ⬆️
windows-2022-pypy3.10 92.49% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@akx
akx merged commit 2b16fdc into python-babel:master Oct 26, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants