Enforce restrictions on selector expression per spec, fixes #416

This uplifts the tests from the spec PR, projectfluent/fluent#280 and implements the selector validation to follow what `abstract.js` does in the spec. In @fluent/bundle, the invalid selectors are just accepted. Might be worth revisiting.
projectfluent · Aug 5, 2019 · 4312437 · 4312437
1 parent 7cec8b9
commit 4312437
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 10 deletions.
diff --git a/fluent-bundle/test/fixtures_reference/select_expressions.json b/fluent-bundle/test/fixtures_reference/select_expressions.json
@@ -122,6 +122,66 @@
             ],
             "attributes": {}
         },
+        {
+            "id": "invalid-selector-select-expression",
+            "value": [
+                {
+                    "type": "select",
+                    "selector": {
+                        "type": "num",
+                        "value": 3,
+                        "precision": 0
+                    },
+                    "variants": [
+                        {
+                            "key": {
+                                "type": "str",
+                                "value": "key"
+                            },
+                            "value": "default"
+                        }
+                    ],
+                    "star": 0
+                }
+            ],
+            "attributes": {}
+        },
+        {
+            "id": "invalid-selector-nested-expression",
+            "value": [
+                {
+                    "type": "select",
+                    "selector": {
+                        "type": "select",
+                        "selector": {
+                            "type": "var",
+                            "name": "sel"
+                        },
+                        "variants": [
+                            {
+                                "key": {
+                                    "type": "str",
+                                    "value": "key"
+                                },
+                                "value": "value"
+                            }
+                        ],
+                        "star": 0
+                    },
+                    "variants": [
+                        {
+                            "key": {
+                                "type": "str",
+                                "value": "key"
+                            },
+                            "value": "default"
+                        }
+                    ],
+                    "star": 0
+                }
+            ],
+            "attributes": {}
+        },
         {
             "id": "empty-variant",
             "value": [

diff --git a/fluent-syntax/src/errors.js b/fluent-syntax/src/errors.js
@@ -76,6 +76,8 @@ function getErrorMessage(code, args) {
       return "Unbalanced closing brace in TextElement.";
     case "E0028":
       return "Expected an inline expression";
+    case "E0029":
+      return "Expected simple expression as selector";
     default:
       return code;
   }

diff --git a/fluent-syntax/src/parser.js b/fluent-syntax/src/parser.js
@@ -607,16 +607,26 @@ class FluentParser {
         return selector;
       }
 
-      if (selector.type === "MessageReference") {
-        if (selector.attribute === null) {
-          throw new ParseError("E0016");
-        } else {
-          throw new ParseError("E0018");
-        }
-      }
-
-      if (selector.type === "TermReference" && selector.attribute === null) {
-        throw new ParseError("E0017");
+      // Validate selector expression according to
+      // abstract.js in the Fluent specification
+      switch (selector.type) {
+        case "MessageReference":
+          if (selector.attribute === null) {
+            throw new ParseError("E0016");
+          } else {
+            throw new ParseError("E0018");
+          }
+        case "TermReference":
+          if (selector.attribute === null) {
+            throw new ParseError("E0017");
+          }
+        case "StringLiteral":
+        case "NumberLiteral":
+        case "VariableReference":
+        case "FunctionReference":
+          break;
+        default:
+          throw new ParseError("E0029");
       }
 
       ps.next();

diff --git a/fluent-syntax/test/fixtures_reference/select_expressions.ftl b/fluent-syntax/test/fixtures_reference/select_expressions.ftl
@@ -21,6 +21,20 @@ invalid-selector-term-variant =
        *[key] value
     }
 
+# ERROR Nested expressions are not valid selectors
+invalid-selector-select-expression =
+    { { 3 } ->
+        *[key] default
+    }
+
+# ERROR Select expressions are not valid selectors
+invalid-selector-nested-expression =
+    { { $sel ->
+        *[key] value
+        } ->
+        *[key] default
+    }
+
 empty-variant =
     { $sel ->
        *[key] {""}

diff --git a/fluent-syntax/test/fixtures_reference/select_expressions.json b/fluent-syntax/test/fixtures_reference/select_expressions.json
@@ -145,6 +145,24 @@
             "annotations": [],
             "content": "invalid-selector-term-variant =\n    { -term(case: \"nominative\") ->\n       *[key] value\n    }\n\n"
         },
+        {
+            "type": "Comment",
+            "content": "ERROR Nested expressions are not valid selectors"
+        },
+        {
+            "type": "Junk",
+            "annotations": [],
+            "content": "invalid-selector-select-expression =\n    { { 3 } ->\n        *[key] default\n    }\n\n"
+        },
+        {
+            "type": "Comment",
+            "content": "ERROR Select expressions are not valid selectors"
+        },
+        {
+            "type": "Junk",
+            "annotations": [],
+            "content": "invalid-selector-nested-expression =\n    { { $sel ->\n        *[key] value\n        } ->\n        *[key] default\n    }\n\n"
+        },
         {
             "type": "Message",
             "id": {