Skip to content

fix(mf2/icu-messageformat-1): Unique names for selectors sharing an argument - #477

Open
NUAAwqy wants to merge 1 commit into
messageformat:mainfrom
NUAAwqy:fix/mf1-to-message-duplicate-declaration
Open

NUAAwqy wants to merge 1 commit into
messageformat:mainfrom
NUAAwqy:fix/mf1-to-message-duplicate-declaration

Conversation

@NUAAwqy

@NUAAwqy NUAAwqy commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Addresses #472.

What's broken

An MF1 argument may legally drive selectors of different types in different arms. findSelectArgs() merges selectors only when arg, type and pluralOffset all match (equalSelectArgs), which is right for selection — one entry per selector — but argToInputDeclaration() then named every entry after the argument, and MF2 forbids declaring a variable twice, so mf1ToMessageData emitted .input {$count …} twice and the model died:

MessageDataModelError: duplicate-declaration

Reproduced shapes (all now in the test table): {type, select, …} + {count, selectordinal, …} from one argument; two plurals of the same argument with different offset:; plural arms containing a nested select/selectordinal on the same or another argument; and a group whose natural rename suffix collides with a real MF1 argument (count_1 is an actual argument, so the group must skip to count_2/count_3).

The fix

In mf2/icu-messageformat-1/src/mf1-to-message-data.ts:

  1. selectorNames(args) counts how many entries share each arg. A group of one keeps the bare name, so every message that does not hit the bug produces a byte-identical model. In a larger group each entry gets arg_1, arg_2, …, skipping any suffix already used by another argument.
  2. Renamed entries are declared .local $count_1 = {$count :number select=ordinal} over the external argument instead of .input, so .match never repeats a variable.
  3. The octothorpe of a plural/selectordinal resolves to names[idx] — the declaration its selector was chosen by — rather than to token.arg, which keeps # at value − offset for offset plurals.

The shape suggested on the issue (keep one selector as .input, chain the rest as .local off it) was measured and rejected: a base carrying select=ordinal or offset breaks the .local chained off it, and groups need not contain an option-free member at all ({n, plural, offset:1 …} {n, plural, offset:2 …} has none). Declaring every member .local of the external argument is one code path and mirrors MF1, where each selector formats the raw argument independently. If a base+locals shape is preferred, the change is local to argToDeclaration/selectorNames.

Two limits, stated plainly:

  • MF2 resolves every lifted selector on every format() call, so an argument used both as select and as selectordinal must be numeric; MF1 evaluates only the taken arm, so an exotic message can newly report bad-operand/bad-selector at format time. The test table uses numeric values accordingly.
  • A pre-existing divergence between MF1's negative-offset plural rules and Intl.PluralRules is untouched; single-selector shapes are byte-identical before and after this fix.

Validation

  • Six shapes in a new Multiple selectors per argument table plus two structural tests in mf1.test.ts. Red first: Tests 23 failed on the pristine tree, all duplicate-declaration; with the fix, all green.
  • Expectations are not hand-written: every exp value was taken from the MF1 reference implementation (@messageformat/core, new MF1('en').compile(src)(params)) via a differential harness that compares, per parameter set, MF1 output, the MF2 model built from the data object, and the model re-parsed from its own stringifyMessage() source — 26/26 parameter sets identical across the six shapes.
  • Whole-repo A/B: pristine 614f28f → Tests 2792 passed | 4 skipped; with the fix → Tests 2820 passed | 4 skipped, Test Files 37 passed (37) — exactly the +28 new cases, and no existing expectation changed, which is the mechanical proof that single-selector messages are untouched.
  • Mutation checks, five mutants all killed: # keeping token.arg (wrong offset values), bare-arg names (back to duplicate-declaration), all-.input declarations, dropping the collision skip, and a .local reading name instead of arg.arg.
  • Gates, all rc=0: npm test, npm run build, npm run lint, npx prettier --check .

Prepared and tested with AI assistance; no human review is claimed.

…rgument

An MF1 argument may drive several selectors of different types, e.g. a
selectordinal in one arm of a select and a plural in another. Each of
them was lifted into an .input declaration named after the argument, so
the resulting data model was rejected with a duplicate-declaration error.

Selectors that share an argument now get unique names and are declared
as .local expressions of the external variable, and the octothorpe of a
plural or selectordinal resolves to the declaration it was selected by
rather than to the argument name. Messages where every argument drives
at most one selector are unchanged.

Addresses messageformat#472

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant