Variant keys can now be numbers, identifiers or quoted text

projectfluent · May 21, 2018 · f4a11ad · f4a11ad
1 parent 73170f0
commit f4a11ad
Show file tree

Hide file tree

Showing 9 changed files with 326 additions and 45 deletions.
diff --git a/spec/CHANGELOG.md b/spec/CHANGELOG.md
@@ -37,6 +37,17 @@
     These issues have been fixed and the new test suite will help ensure the
     correctness of the grammar in the future.
 
+  - Variant keys can now be numbers, identifiers or quoted text.
+
+    Previously, variant keys could either be numbers (`NumberExpressions`) or
+    text (`VariantNames`). Text keys allowed inline whitespace; the whitespace
+    at the extremes was trimmed. Special characters like `{` or `[` where
+    forbidden, but no espace sequences were allowed.
+
+    To fix this, variant keys can now be numbers (as before), identifiers
+    (`Identifier`) or quoted text (`StringExpressions`). The `VariantName`
+    AST node has been removed.
+
 ## 0.5.0 (January 31, 2018)
 
   - Added terms. (#62, #85)

diff --git a/spec/fluent.ebnf b/spec/fluent.ebnf
@@ -60,8 +60,7 @@ VariantList                ::= variant_list break_indent
 variant_list               ::= Variant* DefaultVariant Variant*
 Variant                    ::= break_indent VariantKey inline_space? Pattern
 DefaultVariant             ::= break_indent "*" VariantKey inline_space? Pattern
-VariantKey                 ::= "[" inline_space? (NumberExpression | VariantName) inline_space? "]"
-VariantName                ::= word (inline_space word)*
+VariantKey                 ::= "[" inline_space? (NumberExpression | StringExpression | Identifier) inline_space? "]"
 
 /* Identifiers */
 Identifier                 ::= identifier
@@ -73,7 +72,6 @@ Function                   ::= [A-Z] [A-Z_?-]*
 identifier                 ::= [a-zA-Z] [a-zA-Z0-9_-]*
 comment_line               ::= (line_end)
                              | ("\u0020" /.*/ line_end)
-word                       ::= (regular_char - backslash - "}" - "{" - "]" - "[")+
 
 /* Characters */
 backslash                  ::= "\\"

diff --git a/syntax/ast.mjs b/syntax/ast.mjs
@@ -191,13 +191,6 @@ export class Identifier extends SyntaxNode {
     }
 }
 
-export class VariantName extends Identifier {
-    constructor(name) {
-        super(name);
-        this.type = "VariantName";
-    }
-}
-
 export class BaseComment extends Entry {
     constructor(content) {
         super();

diff --git a/syntax/grammar.mjs b/syntax/grammar.mjs
@@ -307,24 +307,13 @@ let VariantKey = defer(() =>
         char("["),
         maybe(inline_space),
         either(
-            // Meh. It's not really an expression.
             NumberExpression,
-            VariantName),
+            StringExpression,
+            Identifier),
         maybe(inline_space),
         char("]"))
     .map(element_at(2)));
 
-let VariantName = defer(() =>
-    sequence(
-        word,
-        repeat(
-            sequence(
-                inline_space,
-                word)))
-    .map(flatten(2))
-    .map(join)
-    .map(into(FTL.VariantName)));
-
 /* ----------- */
 /* Identifiers */
 
@@ -375,17 +364,6 @@ let comment_line = defer(() =>
     .map(keep_abstract)
     .map(join));
 
-let word = defer(() =>
-    repeat1(
-        and(
-            not(char("[")),
-            not(char("]")),
-            not(char("{")),
-            not(char("}")),
-            not(backslash),
-            regular_char))
-    .map(join));
-
 /* ---------- */
 /* Characters */
 

diff --git a/test/fixtures/leading_dots.json b/test/fixtures/leading_dots.json
@@ -366,7 +366,7 @@
                                 {
                                     "type": "Variant",
                                     "key": {
-                                        "type": "VariantName",
+                                        "type": "Identifier",
                                         "name": "one"
                                     },
                                     "value": {
@@ -383,7 +383,7 @@
                                 {
                                     "type": "Variant",
                                     "key": {
-                                        "type": "VariantName",
+                                        "type": "Identifier",
                                         "name": "other"
                                     },
                                     "value": {

diff --git a/test/fixtures/member_expressions.json b/test/fixtures/member_expressions.json
@@ -20,7 +20,7 @@
                                 "name": "-term"
                             },
                             "key": {
-                                "type": "VariantName",
+                                "type": "Identifier",
                                 "name": "case"
                             }
                         }

diff --git a/test/fixtures/select_expressions.ftl b/test/fixtures/select_expressions.ftl
@@ -6,7 +6,7 @@ new-messages =
 
 valid-selector =
     { -term.case ->
-       *[    many     words    ] value
+       *[key] value
     }
 
 invalid-selector =
@@ -23,3 +23,49 @@ empty-variant =
     { 1 ->
        *[one] {""}
     }
+
+
+## Variant keys
+
+valid-variant-key-identifier-simple =
+    { 1 ->
+       *[key] value
+    }
+
+valid-variant-key-identifier-padded =
+    { 1 ->
+       *[    key    ] value
+    }
+
+# ERROR
+invalid-variant-key-identifier-with-space-inside =
+    { 1 ->
+       *[many     words] value
+    }
+
+# ERROR
+invalid-variant-key-identifier-non-latin =
+    { 1 ->
+       *[ĸəʎ] value
+    }
+
+# ERROR
+invalid-variant-key-number =
+    { 1 ->
+       *[15 ducks] value
+    }
+
+valid-variant-key-string-with-whitespace =
+    { 1 ->
+       *["   many     words   "] value
+    }
+
+valid-variant-key-string-non-latin =
+    { 1 ->
+       *["keʎ"] value
+    }
+
+valid-variant-key-string-start-with-digits =
+    { 1 ->
+       *["15 ducks"] value
+    }