fix(mf2): Treat duplicate option identifiers as a data model error

eemeli · eemeli · commit d821d05990ae · 2024-01-02T01:00:20.000+02:00
diff --git a/packages/mf2-messageformat/src/__fixtures/test-messages.json b/packages/mf2-messageformat/src/__fixtures/test-messages.json
@@ -330,6 +330,8 @@
     "src": "no-equal {|42| :number minimumFractionDigits 2}",
     "syntaxError": true
   },
+  { "src": "bad {:placeholder option=x option=x}", "syntaxError": true },
+  { "src": "bad {:placeholder ns:option=x ns:option=y}", "syntaxError": true },
   { "src": "bad {:placeholder option=}", "syntaxError": true },
   { "src": "bad {:placeholder option value}", "syntaxError": true },
   { "src": "bad {:placeholder option:value}", "syntaxError": true },
@@ -341,7 +343,12 @@
   { "src": "bad {:placeholder :option=x}", "syntaxError": true },
   { "src": "bad {:placeholder option::x=y}", "syntaxError": true },
   { "src": "bad {$placeholder option}", "syntaxError": true },
+  { "src": "no {placeholder end", "syntaxError": true },
   { "src": "no {$placeholder end", "syntaxError": true },
+  { "src": "no {:placeholder end", "syntaxError": true },
+  { "src": "no {|placeholder| end", "syntaxError": true },
+  { "src": "no {|literal} end", "syntaxError": true },
+  { "src": "no {|literal or placeholder end", "syntaxError": true },
   { "src": ".match {} * {{foo}}", "syntaxError": true },
   { "src": ".match {|foo|} {|bar|} ** {{foo}}", "syntaxError": true },
   { "src": ".match * {{foo}}", "syntaxError": true },
diff --git a/packages/mf2-messageformat/src/cst-parser/expression.ts b/packages/mf2-messageformat/src/cst-parser/expression.ts
@@ -127,6 +127,13 @@ function parseFunctionRefOrMarkup(
     pos += ws;
     const opt = parseOption(ctx, pos);
     if (opt.end === pos) break; // error
+    if (options.some(prev => sameIdentifier(prev.name, opt.name))) {
+      ctx.onError(
+        'duplicate-option',
+        opt.name[0].start,
+        opt.name.at(-1)?.end ?? -1
+      );
+    }
     options.push(opt);
     pos = opt.end;
   }
@@ -182,6 +189,11 @@ function parseIdentifier(
   }
 }
 
+const sameIdentifier = (a: CST.Identifier, b: CST.Identifier) =>
+  a.length === b.length &&
+  a[0].value === b[0].value &&
+  a[2]?.value === b[2]?.value;
+
 function parseReservedAnnotation(
   ctx: ParseContext,
   start: number
diff --git a/packages/mf2-messageformat/src/errors.ts b/packages/mf2-messageformat/src/errors.ts
@@ -19,6 +19,7 @@ export class MessageSyntaxError extends MessageError {
     | 'bad-input-expression'
     | 'bad-selector'
     | 'duplicate-declaration'
+    | 'duplicate-option'
     | 'extra-content'
     | 'forward-reference'
     | 'key-mismatch'
@@ -49,6 +50,7 @@ export class MessageSyntaxError extends MessageError {
         message = `Syntax parse error: Missing ${expected} at ${start}`;
         break;
       case 'duplicate-declaration':
+      case 'duplicate-option':
       case 'forward-reference':
       case 'key-mismatch':
       case 'missing-fallback':
@@ -73,6 +75,7 @@ export class MissingSyntaxError extends MessageSyntaxError {
 export class MessageDataModelError extends MessageSyntaxError {
   declare type:
     | 'duplicate-declaration'
+    | 'duplicate-option'
     | 'forward-reference'
     | 'key-mismatch'
     | 'missing-fallback';
