From 5612f3b0508d63770b218d581c465aae878f5573 Mon Sep 17 00:00:00 2001
From: Eemeli Aro <eemeli@mozilla.com>
Date: Mon, 12 Aug 2024 17:39:50 +0100
Subject: [PATCH] Add Duplicate Variant error (#853)

* Add Duplicate Variant error

* Apply suggestions from code review

Co-authored-by: Addison Phillips <addison@unicode.org>

* List, not set, of keys

---------

Co-authored-by: Addison Phillips <addison@unicode.org>
---
 spec/errors.md                    | 21 +++++++++++++++++++++
 spec/syntax.md                    |  5 ++++-
 test/schemas/v0/tests.schema.json |  1 +
 test/tests/data-model-errors.json | 16 ++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/spec/errors.md b/spec/errors.md
index 1fa78c8a2..7a6375ee9 100644
--- a/spec/errors.md
+++ b/spec/errors.md
@@ -178,6 +178,27 @@ appears on the left-hand side of more than one _option_ in the same _expression_
 > {{This is {$foo}}}
 > ```
 
+### Duplicate Variant
+
+A **_<dfn>Duplicate Variant</dfn>_** error occurs when the
+same list of _keys_ is used for more than one _variant_.
+
+> Examples of invalid messages resulting in a _Duplicate Variant_ error:
+>
+> ```
+> .match {$var :string}
+> * {{The first default}}
+> * {{The second default}}
+> ```
+>
+> ```
+> .match {$x :string} {$y :string}
+> *   foo   {{The first "foo" variant}}
+> bar *     {{The "bar" variant}}
+> *   |foo| {{The second "foo" variant}}
+> *   *     {{The default variant}}
+> ```
+
 ## Resolution Errors
 
 **_<dfn>Resolution Errors</dfn>_** occur when the runtime value of a part of a message
diff --git a/spec/syntax.md b/spec/syntax.md
index 0d5ec2657..42d742ef1 100644
--- a/spec/syntax.md
+++ b/spec/syntax.md
@@ -375,6 +375,9 @@ satisfied:
 - At least one _variant_ MUST exist whose _keys_ are all equal to the "catch-all" key `*`.
 - Each _selector_ MUST have an _annotation_,
   or contain a _variable_ that directly or indirectly references a _declaration_ with an _annotation_.
+- Each _variant_ MUST use a list of _keys_ that is unique from that
+  of all other _variants_ in the _message_.
+  _Literal_ _keys_ are compared by their contents, not their syntactical appearance.
 
 ```abnf
 matcher         = match-statement 1*([s] variant)
@@ -440,7 +443,7 @@ There MAY be any number of additional _selectors_.
 
 ### Variant
 
-A **_<dfn>variant</dfn>_** is a _quoted pattern_ associated with a set of _keys_ in a _matcher_.
+A **_<dfn>variant</dfn>_** is a _quoted pattern_ associated with a list of _keys_ in a _matcher_.
 Each _variant_ MUST begin with a sequence of _keys_,
 and terminate with a valid _quoted pattern_.
 The number of _keys_ in each _variant_ MUST match the number of _selectors_ in the _matcher_.
diff --git a/test/schemas/v0/tests.schema.json b/test/schemas/v0/tests.schema.json
index 9e23913ce..7b2056292 100644
--- a/test/schemas/v0/tests.schema.json
+++ b/test/schemas/v0/tests.schema.json
@@ -342,6 +342,7 @@
               "missing-selector-annotation",
               "duplicate-declaration",
               "duplicate-option-name",
+              "duplicate-variant",
               "unresolved-variable",
               "unknown-function",
               "unsupported-expression",
diff --git a/test/tests/data-model-errors.json b/test/tests/data-model-errors.json
index c07bbb366..86a674c43 100644
--- a/test/tests/data-model-errors.json
+++ b/test/tests/data-model-errors.json
@@ -164,6 +164,22 @@
           "type": "duplicate-option-name"
         }
       ]
+    },
+    {
+      "src": ".match {$var :string} * {{The first default}} * {{The second default}}",
+      "expErrors": [
+        {
+          "type": "duplicate-variant"
+        }
+      ]
+    },
+    {
+      "src": ".match {$x :string} {$y :string} * foo {{The first foo variant}} bar * {{The bar variant}} * |foo| {{The second foo variant}} * * {{The default variant}}",
+      "expErrors": [
+        {
+          "type": "duplicate-variant"
+        }
+      ]
     }
   ]
 }