Move 'collator' from three arguments to a single "options" argument.

Support/test default behavior when case/diacriticSensitivity isn't specified. Remove 'collator','resolve-locale' from feature non-constant list.
mapbox · Apr 7, 2018 · 2e107ba · 2e107ba
1 parent be70589
commit 2e107ba
Show file tree

Hide file tree

Showing 21 changed files with 208 additions and 58 deletions.
diff --git a/docs/components/expression-metadata.js b/docs/components/expression-metadata.js
@@ -204,17 +204,7 @@ const types = {
     }],
     collator: [{
         type: 'collator',
-        parameters: [
-            'caseSensitive: boolean',
-            'diacriticSensitive: boolean',
-        ]
-    }, {
-        type: 'collator',
-        parameters: [
-            'caseSensitive: boolean',
-            'diacriticSensitive: boolean',
-            'locale: string'
-        ]
+        parameters: [ '{ "caseSensitive": boolean, "diacriticSensitive": boolean, "locale": string }' ]
     }]
 };
 

diff --git a/src/style-spec/expression/definitions/collator.js b/src/style-spec/expression/definitions/collator.js
@@ -61,13 +61,13 @@ export class CollatorInstantiation {
     }
 
     serialize() {
-        const serialized = ["collator"];
-        serialized.push(this.sensitivity === 'variant' || this.sensitivity === 'case');
-        serialized.push(this.sensitivity === 'variant' || this.sensitivity === 'accent');
+        const options = {};
+        options['caseSensitive'] = this.sensitivity === 'variant' || this.sensitivity === 'case';
+        options['diacriticSensitive'] = this.sensitivity === 'variant' || this.sensitivity === 'accent';
         if (this.locale) {
-            serialized.push(this.locale);
+            options['locale'] = this.locale;
         }
-        return serialized;
+        return ["collator", options];
     }
 }
 
@@ -85,17 +85,24 @@ export class CollatorExpression implements Expression {
     }
 
     static parse(args: Array<mixed>, context: ParsingContext): ?Expression {
-        if (args.length !== 3 && args.length !== 4)
-            return context.error(`Expected two or three arguments.`);
+        if (args.length !== 2)
+            return context.error(`Expected one argument.`);
 
-        const caseSensitive = context.parse(args[1], 1, BooleanType);
+        const options = (args[1]: any);
+        if (typeof options !== "object" || Array.isArray(options))
+            return context.error(`Collator options argument must be an object.`);
+
+        const caseSensitive = context.parse(
+            options['caseSensitive'] === undefined ? false : options['caseSensitive'], 1, BooleanType);
         if (!caseSensitive) return null;
-        const diacriticSensitive = context.parse(args[2], 2, BooleanType);
+
+        const diacriticSensitive = context.parse(
+            options['diacriticSensitive'] === undefined ? false : options['diacriticSensitive'], 2, BooleanType);
         if (!diacriticSensitive) return null;
 
         let locale = null;
-        if (args.length === 4) {
-            locale = context.parse(args[3], 3, StringType);
+        if (options['locale']) {
+            locale = context.parse(options['locale'], 3, StringType);
             if (!locale) return null;
         }
 
@@ -123,8 +130,12 @@ export class CollatorExpression implements Expression {
     }
 
     serialize() {
-        const serialized = ["collator"];
-        this.eachChild(child => { serialized.push(child.serialize()); });
-        return serialized;
+        const options = {};
+        options['caseSensitive'] = this.caseSensitive;
+        options['diacriticSensitive'] = this.diacriticSensitive;
+        if (this.locale) {
+            options['locale'] = this.locale;
+        }
+        return ["collator", options];
     }
 }
diff --git a/src/style-spec/expression/definitions/index.js b/src/style-spec/expression/definitions/index.js
@@ -526,7 +526,6 @@ CompoundExpression.register(expressions, {
         (ctx, args) => args.map(arg => arg.evaluate(ctx)).join('')
     ],
     'resolved-locale': [
-        // Must be added to non-featureConstant list in parsing_context.js
         StringType,
         [CollatorType],
         (ctx, [collator]) => collator.evaluate(ctx).resolvedLocale()

diff --git a/src/style-spec/expression/parsing_context.js b/src/style-spec/expression/parsing_context.js
@@ -201,5 +201,5 @@ function isConstant(expression: Expression) {
     }
 
     return isFeatureConstant(expression) &&
-        isGlobalPropertyConstant(expression, ['zoom', 'heatmap-density', 'resolved-locale', 'collator']);
+        isGlobalPropertyConstant(expression, ['zoom', 'heatmap-density']);
 }
diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json
@@ -2194,7 +2194,7 @@
         }
       },
       "collator": {
-        "doc": "Returns a `collator` for use in locale dependent comparison operations. The first two arguments control case and diacritic sensitivity, and the optional third argument specifies the IETF language tag of the locale to use. If no locale is provided, the default locale is used.",
+        "doc": "Returns a `collator` for use in locale dependent comparison operations. The `caseSensitive` and `diacriticSensitive` options default to `false`. The `locale` argument specifies the IETF language tag of the locale to use. If none is provided, the default locale is used. If the requested locale is not available, the `collator` will use a system-defined fallback locale. Use `resolved-locale` to test the results of locale fallback behavior.",
         "group": "Types",
         "sdk-support": {
           "basic functionality": {

diff --git a/test/integration/expression-tests/collator/accent-equals-de/test.json b/test/integration/expression-tests/collator/accent-equals-de/test.json
@@ -1,12 +1,25 @@
 {
   "expression": [
     "case",
-    ["==", ["resolved-locale", ["collator", true, false, "de"]], "de"],
+    [
+      "==",
+      [
+        "resolved-locale",
+        [
+          "collator",
+          {"caseSensitive": true, "diacriticSensitive": false, "locale": "de"}
+        ]
+      ],
+      "de"
+    ],
     [
       "==",
       ["string", ["get", "lhs"]],
       ["get", "rhs"],
-      ["collator", true, false, "de"]
+      [
+        "collator",
+        {"caseSensitive": true, "diacriticSensitive": false, "locale": "de"}
+      ]
     ],
     ["case", ["==", ["get", "rhs"], "ue"], true, false]
   ],
@@ -24,12 +37,15 @@
     "outputs": [true, false],
     "serialized": [
       "case",
-      ["==", ["resolved-locale", ["collator", true, false, "de"]], "de"],
+      false,
       [
         "==",
         ["string", ["get", "lhs"]],
         ["get", "rhs"],
-        ["collator", true, false, "de"]
+        [
+          "collator",
+          {"caseSensitive": true, "diacriticSensitive": false, "locale": "de"}
+        ]
       ],
       ["case", ["==", ["get", "rhs"], "ue"], true, false]
     ]

diff --git a/test/integration/expression-tests/collator/accent-lt-en/test.json b/test/integration/expression-tests/collator/accent-lt-en/test.json
@@ -3,7 +3,10 @@
     "<",
     ["string", ["get", "lhs"]],
     ["get", "rhs"],
-    ["collator", true, false, "en"]
+    [
+      "collator",
+      {"caseSensitive": true, "diacriticSensitive": false, "locale": "en"}
+    ]
   ],
   "inputs": [
     [{}, {"properties": {"lhs": "a", "rhs": "ä"}}],
@@ -22,7 +25,10 @@
       "<",
       ["string", ["get", "lhs"]],
       ["string", ["get", "rhs"]],
-      ["collator", true, false, "en"]
+      [
+        "collator",
+        {"caseSensitive": true, "diacriticSensitive": false, "locale": "en"}
+      ]
     ]
   }
 }
diff --git a/test/integration/expression-tests/collator/accent-not-equals-en/test.json b/test/integration/expression-tests/collator/accent-not-equals-en/test.json
@@ -5,7 +5,10 @@
       "!=",
       ["string", ["get", "lhs"]],
       ["get", "rhs"],
-      ["collator", true, false, "en"]
+      [
+        "collator",
+        {"caseSensitive": true, "diacriticSensitive": false, "locale": "en"}
+      ]
     ]
   ],
   "inputs": [
@@ -27,7 +30,10 @@
         "!=",
         ["string", ["get", "lhs"]],
         ["get", "rhs"],
-        ["collator", true, false, "en"]
+        [
+          "collator",
+          {"caseSensitive": true, "diacriticSensitive": false, "locale": "en"}
+        ]
       ]
     ]
   }

diff --git a/test/integration/expression-tests/collator/base-default-locale/test.json b/test/integration/expression-tests/collator/base-default-locale/test.json
@@ -3,7 +3,7 @@
     "==",
     ["string", ["get", "lhs"]],
     ["get", "rhs"],
-    ["collator", false, false]
+    ["collator", {"caseSensitive": false, "diacriticSensitive": false}]
   ],
   "inputs": [
     [{}, {"properties": {"lhs": "a", "rhs": "a"}}],
@@ -22,7 +22,7 @@
       "==",
       ["string", ["get", "lhs"]],
       ["get", "rhs"],
-      ["collator", false, false]
+      ["collator", {"caseSensitive": false, "diacriticSensitive": false}]
     ]
   }
 }
diff --git a/test/integration/expression-tests/collator/base-equals-en/test.json b/test/integration/expression-tests/collator/base-equals-en/test.json
@@ -3,7 +3,10 @@
     "==",
     ["string", ["get", "lhs"]],
     ["get", "rhs"],
-    ["collator", false, false, "en"]
+    [
+      "collator",
+      {"caseSensitive": false, "diacriticSensitive": false, "locale": "en"}
+    ]
   ],
   "inputs": [
     [{}, {"properties": {"lhs": "a", "rhs": "ä"}}],
@@ -22,7 +25,10 @@
       "==",
       ["string", ["get", "lhs"]],
       ["get", "rhs"],
-      ["collator", false, false, "en"]
+      [
+        "collator",
+        {"caseSensitive": false, "diacriticSensitive": false, "locale": "en"}
+      ]
     ]
   }
 }
diff --git a/test/integration/expression-tests/collator/base-gt-en/test.json b/test/integration/expression-tests/collator/base-gt-en/test.json
@@ -3,7 +3,10 @@
     ">",
     ["string", ["get", "lhs"]],
     ["get", "rhs"],
-    ["collator", false, false, "en"]
+    [
+      "collator",
+      {"caseSensitive": false, "diacriticSensitive": false, "locale": "en"}
+    ]
   ],
   "inputs": [
     [{}, {"properties": {"lhs": "a", "rhs": "ä"}}],
@@ -22,7 +25,10 @@
       ">",
       ["string", ["get", "lhs"]],
       ["string", ["get", "rhs"]],
-      ["collator", false, false, "en"]
+      [
+        "collator",
+        {"caseSensitive": false, "diacriticSensitive": false, "locale": "en"}
+      ]
     ]
   }
 }
diff --git a/test/integration/expression-tests/collator/case-lteq-en/test.json b/test/integration/expression-tests/collator/case-lteq-en/test.json
@@ -3,7 +3,10 @@
     "<=",
     ["string", ["get", "lhs"]],
     ["get", "rhs"],
-    ["collator", false, true, "en"]
+    [
+      "collator",
+      {"caseSensitive": false, "diacriticSensitive": true, "locale": "en"}
+    ]
   ],
   "inputs": [
     [{}, {"properties": {"lhs": "ä", "rhs": "a"}}],
@@ -23,7 +26,10 @@
       "<=",
       ["string", ["get", "lhs"]],
       ["string", ["get", "rhs"]],
-      ["collator", false, true, "en"]
+      [
+        "collator",
+        {"caseSensitive": false, "diacriticSensitive": true, "locale": "en"}
+      ]
     ]
   }
 }
diff --git a/test/integration/expression-tests/collator/case-not-equals-en/test.json b/test/integration/expression-tests/collator/case-not-equals-en/test.json
@@ -5,7 +5,10 @@
       "!=",
       ["string", ["get", "lhs"]],
       ["get", "rhs"],
-      ["collator", false, true, "en"]
+      [
+        "collator",
+        {"caseSensitive": false, "diacriticSensitive": true, "locale": "en"}
+      ]
     ]
   ],
   "inputs": [
@@ -27,7 +30,10 @@
         "!=",
         ["string", ["get", "lhs"]],
         ["get", "rhs"],
-        ["collator", false, true, "en"]
+        [
+          "collator",
+          {"caseSensitive": false, "diacriticSensitive": true, "locale": "en"}
+        ]
       ]
     ]
   }

diff --git a/test/integration/expression-tests/collator/case-omitted-en/test.json b/test/integration/expression-tests/collator/case-omitted-en/test.json
@@ -0,0 +1,32 @@
+{
+  "expression": [
+    "==",
+    ["string", ["get", "lhs"]],
+    ["get", "rhs"],
+    ["collator", {"diacriticSensitive": true, "locale": "en"}]
+  ],
+  "inputs": [
+    [{}, {"properties": {"lhs": "ä", "rhs": "a"}}],
+    [{}, {"properties": {"lhs": "A", "rhs": "a"}}],
+    [{}, {"properties": {"lhs": "a", "rhs": "a"}}],
+    [{}, {"properties": {"lhs": "ä", "rhs": "b"}}]
+  ],
+  "expected": {
+    "compiled": {
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "outputs": [false, true, true, false],
+    "serialized": [
+      "==",
+      ["string", ["get", "lhs"]],
+      ["get", "rhs"],
+      [
+        "collator",
+        {"caseSensitive": false, "diacriticSensitive": true, "locale": "en"}
+      ]
+    ]
+  }
+}
diff --git a/test/integration/expression-tests/collator/comparison-number-error/test.json b/test/integration/expression-tests/collator/comparison-number-error/test.json
@@ -1,5 +1,10 @@
 {
-  "expression": ["<", 1, 2, ["collator", false, false]],
+  "expression": [
+    "<",
+    1,
+    2,
+    ["collator", {"caseSensitive": false, "diacriticSensitive": false}]
+  ],
   "expected": {
     "compiled": {
       "result": "error",

diff --git a/test/integration/expression-tests/collator/diacritic-omitted-en/test.json b/test/integration/expression-tests/collator/diacritic-omitted-en/test.json
@@ -0,0 +1,31 @@
+{
+  "expression": [
+    "<",
+    ["string", ["get", "lhs"]],
+    ["get", "rhs"],
+    ["collator", {"caseSensitive": ["==", 1, 1], "locale": "en"}]
+  ],
+  "inputs": [
+    [{}, {"properties": {"lhs": "a", "rhs": "ä"}}],
+    [{}, {"properties": {"lhs": "a", "rhs": "A"}}],
+    [{}, {"properties": {"lhs": "ä", "rhs": "b"}}]
+  ],
+  "expected": {
+    "compiled": {
+      "result": "success",
+      "isFeatureConstant": false,
+      "isZoomConstant": true,
+      "type": "boolean"
+    },
+    "outputs": [false, true, true],
+    "serialized": [
+      "<",
+      ["string", ["get", "lhs"]],
+      ["string", ["get", "rhs"]],
+      [
+        "collator",
+        {"caseSensitive": true, "diacriticSensitive": false, "locale": "en"}
+      ]
+    ]
+  }
+}