Skip to content

Commit 6d31d55

Browse files
eemelieemeli
committed
feat(mf2-fluent): Add default-true detectNumberSelection option
1 parent 3de3609 commit 6d31d55

File tree

5 files changed

+150
-52
lines changed

5 files changed

+150
-52
lines changed

‎packages/mf2-fluent/src/fluent-to-message.ts‎

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,38 +18,71 @@ interface SelectArg {
1818
keys: (string | number | typeof CATCHALL)[];
1919
}
2020

21-
function asSelectArg(sel: Fluent.SelectExpression): SelectArg {
22-
let defaultName = '';
23-
const keys = sel.variants.map(v => {
24-
const name = v.key.type === 'Identifier' ? v.key.name : v.key.parse().value;
25-
if (v.default) {
26-
defaultName = String(name);
27-
return CATCHALL;
28-
} else {
29-
return name;
30-
}
31-
});
32-
return { selector: sel.selector, defaultName, keys };
33-
}
34-
3521
function findSelectArgs(pattern: Fluent.Pattern): SelectArg[] {
3622
const args: SelectArg[] = [];
3723
const add = (arg: SelectArg) => {
3824
const prev = args.find(a => deepEqual(a.selector, arg.selector));
3925
if (prev) for (const key of arg.keys) prev.keys.push(key);
4026
else args.push(arg);
4127
};
28+
4229
for (const el of pattern.elements) {
4330
if (el.type === 'Placeable' && el.expression.type === 'SelectExpression') {
44-
add(asSelectArg(el.expression));
45-
for (const v of el.expression.variants) {
31+
const { selector, variants } = el.expression;
32+
let defaultName = '';
33+
const keys = variants.map(v => {
34+
const name =
35+
v.key.type === 'Identifier' ? v.key.name : v.key.parse().value;
36+
if (v.default) {
37+
defaultName = String(name);
38+
return CATCHALL;
39+
} else {
40+
return name;
41+
}
42+
});
43+
add({ selector, defaultName, keys });
44+
for (const v of variants) {
4645
for (const arg of findSelectArgs(v.value)) add(arg);
4746
}
4847
}
4948
}
5049
return args;
5150
}
5251

52+
function asSelectExpression(
53+
{ selector, defaultName, keys }: SelectArg,
54+
detectNumberSelection: boolean = true
55+
): Expression {
56+
switch (selector.type) {
57+
case 'StringLiteral':
58+
return {
59+
type: 'expression',
60+
arg: asValue(selector),
61+
annotation: { type: 'function', name: 'string' }
62+
};
63+
case 'VariableReference': {
64+
let name = detectNumberSelection ? 'number' : 'string';
65+
if (name === 'number') {
66+
for (const key of [...keys, defaultName]) {
67+
if (
68+
typeof key === 'string' &&
69+
!['zero', 'one', 'two', 'few', 'many', 'other'].includes(key)
70+
) {
71+
name = 'string';
72+
break;
73+
}
74+
}
75+
}
76+
return {
77+
type: 'expression',
78+
arg: asValue(selector),
79+
annotation: { type: 'function', name }
80+
};
81+
}
82+
}
83+
return asExpression(selector);
84+
}
85+
5386
function asValue(exp: Fluent.InlineExpression): Literal | VariableRef {
5487
switch (exp.type) {
5588
case 'NumberLiteral':
@@ -72,8 +105,9 @@ function asExpression(exp: Fluent.Expression): Expression {
72105
annotation: { type: 'function', name: 'number' }
73106
};
74107
case 'StringLiteral':
75-
case 'VariableReference':
108+
case 'VariableReference': {
76109
return { type: 'expression', arg: asValue(exp) };
110+
}
77111
case 'FunctionReference': {
78112
const annotation: FunctionAnnotation = {
79113
type: 'function',
@@ -171,9 +205,11 @@ function asFluentSelect(
171205
* {@link messageformat#Message} data object.
172206
*
173207
* @beta
208+
* @param options.detectNumberSelection - Set `false` to disable number selector detection based on keys.
174209
*/
175210
export function fluentToMessage(
176-
ast: Fluent.Pattern
211+
ast: Fluent.Pattern,
212+
options?: { detectNumberSelection?: boolean }
177213
): PatternMessage | SelectMessage {
178214
const args = findSelectArgs(ast);
179215
if (args.length === 0) {
@@ -261,7 +297,9 @@ export function fluentToMessage(
261297
return {
262298
type: 'select',
263299
declarations: [],
264-
selectors: args.map(arg => asExpression(arg.selector)),
300+
selectors: args.map(arg =>
301+
asSelectExpression(arg, options?.detectNumberSelection)
302+
),
265303
variants
266304
};
267305
}

‎packages/mf2-fluent/src/fluent-to-resource.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,21 @@ import { getFluentFunctions } from './functions';
1818
* or in the shape output by {@link fluentToResourceData} as `data`.
1919
* @param locales - The locale code or codes to use for all of the resource's messages.
2020
* @param options - The MessageFormat constructor options to use for all of the resource's messages.
21+
* @param options.detectNumberSelection - Set `false` to disable number selector detection based on keys.
2122
*/
2223
export function fluentToResource(
2324
source: string | Fluent.Resource | FluentMessageResourceData,
2425
locales?: string | string[],
25-
options?: MessageFormatOptions
26+
options?: MessageFormatOptions & { detectNumberSelection?: boolean }
2627
): FluentMessageResource {
2728
const res: FluentMessageResource = new Map();
2829

29-
const functions = Object.assign(getFluentFunctions(res), options?.functions);
30-
const opt = { ...options, functions };
30+
const { detectNumberSelection, ...opt } = options ?? {};
31+
opt.functions = Object.assign(getFluentFunctions(res), options?.functions);
3132

3233
const data =
3334
typeof source === 'string' || source instanceof Fluent.Resource
34-
? fluentToResourceData(source).data
35+
? fluentToResourceData(source, { detectNumberSelection }).data
3536
: source;
3637
for (const [id, group] of data) {
3738
let rg = res.get(id);
@@ -55,10 +56,14 @@ export function fluentToResource(
5556
* @param source - A Fluent resource,
5657
* as the string contents of an FTL file or
5758
* as a {@link https://projectfluent.org/fluent.js/syntax/classes/resource.html | Fluent.Resource}
59+
* @param options.detectNumberSelection - Set `false` to disable number selector detection based on keys.
5860
* @returns An object containing the messages as `data` and any resource-level
5961
* `comments` of the resource.
6062
*/
61-
export function fluentToResourceData(source: string | Fluent.Resource): {
63+
export function fluentToResourceData(
64+
source: string | Fluent.Resource,
65+
options?: { detectNumberSelection?: boolean }
66+
): {
6267
data: FluentMessageResourceData;
6368
comments: string;
6469
} {
@@ -76,7 +81,7 @@ export function fluentToResourceData(source: string | Fluent.Resource): {
7681
const id = msg.type === 'Term' ? `-${msg.id.name}` : msg.id.name;
7782
const group: Map<string, Message> = new Map();
7883
if (msg.value) {
79-
const entry = fluentToMessage(msg.value);
84+
const entry = fluentToMessage(msg.value, options);
8085
if (msg.comment) entry.comment = msg.comment.content;
8186
if (groupComment) {
8287
entry.comment = entry.comment
@@ -86,7 +91,7 @@ export function fluentToResourceData(source: string | Fluent.Resource): {
8691
group.set('', entry);
8792
}
8893
for (const attr of msg.attributes) {
89-
group.set(attr.id.name, fluentToMessage(attr.value));
94+
group.set(attr.id.name, fluentToMessage(attr.value, options));
9095
}
9196
data.set(id, group);
9297
break;

‎packages/mf2-fluent/src/fluent.test.ts‎

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ const testCases: Record<string, TestCase> = {
9494
msg: 'num-fraction-bad',
9595
scope: { arg: 1234 },
9696
exp: '{$arg}',
97-
errors: ['oops']
97+
errors: ['bad-option']
9898
},
9999
{ msg: 'num-style', scope: { arg: 1234 }, exp: '123,400%' },
100100
{ msg: 'num-currency', scope: { arg: 1234 }, exp: '€1,234.00' },
@@ -163,23 +163,23 @@ const testCases: Record<string, TestCase> = {
163163
}
164164
`,
165165
tests: [
166-
{ msg: 'ref-attr', exp: 'It', errors: ['$style', 'not-selectable'] },
166+
{ msg: 'ref-attr', exp: 'It', errors: ['$style'] },
167167
{
168168
msg: 'ref-attr',
169169
scope: { style: 'chicago' },
170170
exp: 'It',
171-
errors: ['$style', 'not-selectable']
171+
errors: ['$style']
172172
},
173173
{
174174
msg: 'call-attr-no-args',
175175
exp: 'It',
176-
errors: ['$style', 'not-selectable']
176+
errors: ['$style']
177177
},
178178
{
179179
msg: 'call-attr-no-args',
180180
scope: { style: 'chicago' },
181181
exp: 'It',
182-
errors: ['$style', 'not-selectable']
182+
errors: ['$style']
183183
},
184184
{ msg: 'call-attr-with-expected-arg', exp: 'She' },
185185
{
@@ -190,13 +190,13 @@ const testCases: Record<string, TestCase> = {
190190
{
191191
msg: 'call-attr-with-other-arg',
192192
exp: 'It',
193-
errors: ['$style', 'not-selectable']
193+
errors: ['$style']
194194
},
195195
{
196196
msg: 'call-attr-with-other-arg',
197197
scope: { style: 'chicago' },
198198
exp: 'It',
199-
errors: ['$style', 'not-selectable']
199+
errors: ['$style']
200200
}
201201
]
202202
},
@@ -254,7 +254,7 @@ const testCases: Record<string, TestCase> = {
254254
msg: 'select',
255255
scope: {},
256256
exp: 'B',
257-
errors: ['$selector', 'not-selectable']
257+
errors: ['$selector']
258258
},
259259
{ msg: 'select', scope: { selector: 'a' }, exp: 'A' },
260260
{ msg: 'select', scope: { selector: 'b' }, exp: 'B' },
@@ -263,7 +263,7 @@ const testCases: Record<string, TestCase> = {
263263
msg: 'number',
264264
scope: {},
265265
exp: 'B',
266-
errors: ['$selector', 'not-selectable']
266+
errors: ['$selector']
267267
},
268268
{ msg: 'number', scope: { selector: 0 }, exp: 'A' },
269269
{ msg: 'number', scope: { selector: 1 }, exp: 'B' },
@@ -272,18 +272,23 @@ const testCases: Record<string, TestCase> = {
272272
msg: 'plural',
273273
scope: {},
274274
exp: 'B',
275-
errors: ['$selector', 'not-selectable']
275+
errors: ['$selector', 'bad-input', 'not-selectable']
276276
},
277277
{ msg: 'plural', scope: { selector: 1 }, exp: 'A' },
278278
{ msg: 'plural', scope: { selector: 2 }, exp: 'B' },
279-
{ msg: 'plural', scope: { selector: 'one' }, exp: 'A' },
279+
{
280+
msg: 'plural',
281+
scope: { selector: 'one' },
282+
exp: 'B',
283+
errors: ['bad-input', 'not-selectable']
284+
},
280285
{
281286
msg: 'default',
282287
scope: {},
283288
exp: 'D',
284-
errors: ['$selector', 'not-selectable']
289+
errors: ['$selector']
285290
},
286-
{ msg: 'default', scope: { selector: 1 }, exp: 'A' },
291+
{ msg: 'default', scope: { selector: 1 }, exp: 'D' },
287292
{ msg: 'default', scope: { selector: 2 }, exp: 'D' },
288293
{ msg: 'default', scope: { selector: 'one' }, exp: 'A' }
289294
]
@@ -335,7 +340,14 @@ for (const [title, { locale = 'en', src, tests }] of Object.entries(
335340
for (const [attr, mf] of group) {
336341
const { functions } = mf.resolvedOptions();
337342
// eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain
338-
validate(data.get(id)?.get(attr ?? '')!, functions);
343+
const req = validate(data.get(id)?.get(attr ?? '')!, type => {
344+
throw new Error(`Validation failed: ${type}`);
345+
});
346+
for (const fn of req.functions) {
347+
if (typeof functions[fn] !== 'function') {
348+
throw new Error(`Unknown message function: ${fn}`);
349+
}
350+
}
339351
}
340352
}
341353
});
@@ -352,17 +364,22 @@ for (const [title, { locale = 'en', src, tests }] of Object.entries(
352364
const test_ = only ? test.only : test;
353365
test_(name, () => {
354366
const onError = jest.fn();
355-
const str = res
356-
.get(msg)
357-
?.get(attr ?? '')
358-
?.format(scope, onError);
367+
const mf = res.get(msg)!.get(attr ?? '');
368+
const str = mf!.format(scope, onError);
359369
if (exp instanceof RegExp) expect(str).toMatch(exp);
360370
else expect(str).toBe(exp);
361371
if (errors?.length) {
372+
//console.dir(mf?.resolvedOptions().message,{depth:null});
373+
//console.dir(onError.mock.calls, { depth: null });
362374
expect(onError).toHaveBeenCalledTimes(errors.length);
363375
for (let i = 0; i < errors.length; ++i) {
364376
const [err] = onError.mock.calls[i];
365-
expect(err.message).toMatch(errors[i]);
377+
const expErr = errors[i];
378+
if (typeof expErr === 'string' && !expErr.startsWith('$')) {
379+
expect(err.type).toBe(expErr);
380+
} else {
381+
expect(err.message).toMatch(expErr);
382+
}
366383
}
367384
} else {
368385
expect(onError).not.toHaveBeenCalled();
@@ -374,6 +391,20 @@ for (const [title, { locale = 'en', src, tests }] of Object.entries(
374391
const template = Fluent.parse(src, { withSpans: false });
375392
const res = resourceToFluent(data, template);
376393

394+
class FixResult extends Fluent.Transformer {
395+
// When converting to MF2, number wrappers are added
396+
visitSelectExpression(node: Fluent.SelectExpression) {
397+
if (
398+
node.selector.type === 'FunctionReference' &&
399+
node.selector.id.name === 'NUMBER'
400+
) {
401+
node.selector = node.selector.arguments.positional[0];
402+
}
403+
return this.genericVisit(node);
404+
}
405+
}
406+
new FixResult().visit(res);
407+
377408
class FixExpected extends Fluent.Transformer {
378409
// MF2 uses first-match selection, so default variants will be last
379410
visitSelectExpression(node: Fluent.SelectExpression) {
@@ -456,7 +487,7 @@ describe('formatToParts', () => {
456487
const onError = jest.fn();
457488
const sel = res.get('sel')?.get('')?.formatToParts(undefined, onError);
458489
expect(sel).toEqual([{ type: 'literal', value: 'B' }]);
459-
expect(onError).toHaveBeenCalledTimes(2);
490+
expect(onError).toHaveBeenCalledTimes(1);
460491
});
461492
});
462493

@@ -612,7 +643,9 @@ describe('formatToParts', () => {
612643
const onError = jest.fn();
613644
const msg = res.get('gender')?.get('')?.formatToParts(undefined, onError);
614645
expect(msg).toEqual([{ type: 'literal', value: 'N' }]);
615-
expect(onError).toHaveBeenCalledTimes(2);
646+
expect(onError.mock.calls.map(args => args[0].type)).toEqual([
647+
'unresolved-var'
648+
]);
616649
});
617650

618651
test('plural with match', () => {
@@ -631,8 +664,16 @@ describe('formatToParts', () => {
631664
});
632665

633666
test('plural with non-plural input', () => {
634-
const msg = res.get('plural')?.get('')?.formatToParts({ num: 'NaN' });
667+
const onError = jest.fn();
668+
const msg = res
669+
.get('plural')
670+
?.get('')
671+
?.formatToParts({ num: 'NaN' }, onError);
635672
expect(msg).toEqual([{ type: 'literal', value: 'Other' }]);
673+
expect(onError.mock.calls.map(args => args[0].type)).toEqual([
674+
'bad-input',
675+
'not-selectable'
676+
]);
636677
});
637678
});
638679
});
@@ -709,12 +750,12 @@ describe('fluentToResourceData', () => {
709750
const res = resourceToFluent(data);
710751
expect(Fluent.serialize(res, {})).toBe(source`
711752
single =
712-
{ $num ->
753+
{ NUMBER($num) ->
713754
[0] One Zero selector.
714755
*[other] One Other selector.
715756
}
716757
multi =
717-
{ $num ->
758+
{ NUMBER($num) ->
718759
[0]
719760
{ $gender ->
720761
[feminine] Combine Zero multiple F selectors.

0 commit comments

Comments
 (0)