Address review comments:

- CollatorInstantiation -> Collator (and try to keep Intl.Collator in its own scope)
- Collator holds onto its Intl.Collator now so it can be used across multiple compares
- Add Collator to values.js
- Fix parse indexes
- Add/tidy comments

src/style-spec/expression/definitions/collator.js

@@ -7,20 +7,23 @@ import type EvaluationContext from '../evaluation_context';
 import type ParsingContext from '../parsing_context';
 import type { Type } from '../types';
 
+// Flow type declarations for Intl cribbed from
+// https://github.com/facebook/flow/issues/1270
+
 declare var Intl: {
-  Collator: Class<Collator>
+  Collator: Class<Intl$Collator>
 }
 
-declare class Collator {
+declare class Intl$Collator {
   constructor (
     locales?: string | string[],
     options?: CollatorOptions
-  ): Collator;
+  ): Intl$Collator;
 
   static (
     locales?: string | string[],
     options?: CollatorOptions
-  ): Collator;
+  ): Intl$Collator;
 
   compare (a: string, b: string): number;
 
@@ -36,9 +39,10 @@ type CollatorOptions = {
   caseFirst?: 'upper' | 'lower' | 'false'
 }
 
-export class CollatorInstantiation {
+export class Collator {
     locale: string | null;
     sensitivity: 'base' | 'accent' | 'case' | 'variant';
+    collator: Intl$Collator;
 
     constructor(caseSensitive: boolean, diacriticSensitive: boolean, locale: string | null) {
         if (caseSensitive)
@@ -47,15 +51,17 @@ export class CollatorInstantiation {
             this.sensitivity = diacriticSensitive ? 'accent' : 'base';
 
         this.locale = locale;
+        this.collator = new Intl.Collator(this.locale ? this.locale : [],
+            { sensitivity: this.sensitivity, usage: 'search' });
     }
 
     compare(lhs: string, rhs: string): number {
-        return new Intl.Collator(this.locale ? this.locale : [],
-                                 { sensitivity: this.sensitivity, usage: 'search' })
-            .compare(lhs, rhs);
+        return this.collator.compare(lhs, rhs);
     }
 
     resolvedLocale(): string {
+        // We create a Collator without "usage: search" because we don't want
+        // the search options encoded in our result (e.g. "en-u-co-search")
         return new Intl.Collator(this.locale ? this.locale : [])
             .resolvedOptions().locale;
     }
@@ -97,20 +103,20 @@ export class CollatorExpression implements Expression {
         if (!caseSensitive) return null;
 
         const diacriticSensitive = context.parse(
-            options['diacriticSensitive'] === undefined ? false : options['diacriticSensitive'], 2, BooleanType);
+            options['diacriticSensitive'] === undefined ? false : options['diacriticSensitive'], 1, BooleanType);
         if (!diacriticSensitive) return null;
 
         let locale = null;
         if (options['locale']) {
-            locale = context.parse(options['locale'], 3, StringType);
+            locale = context.parse(options['locale'], 1, StringType);
             if (!locale) return null;
         }
 
         return new CollatorExpression(caseSensitive, diacriticSensitive, locale);
     }
 
     evaluate(ctx: EvaluationContext) {
-        return new CollatorInstantiation(this.caseSensitive.evaluate(ctx), this.diacriticSensitive.evaluate(ctx), this.locale ? this.locale.evaluate(ctx) : null);
+        return new Collator(this.caseSensitive.evaluate(ctx), this.diacriticSensitive.evaluate(ctx), this.locale ? this.locale.evaluate(ctx) : null);
     }
 
     eachChild(fn: (Expression) => void) {

src/style-spec/expression/definitions/literal.js

@@ -1,9 +1,7 @@
 // @flow
 
 import assert from 'assert';
-import { isValue, typeOf } from '../values';
-import Color from '../../util/color';
-import { CollatorInstantiation } from '../definitions/collator';
+import { isValue, typeOf, Color, Collator } from '../values';
 
 import type { Type } from '../types';
 import type { Value }  from '../values';
@@ -58,8 +56,13 @@ class Literal implements Expression {
         if (this.type.kind === 'array' || this.type.kind === 'object') {
             return ["literal", this.value];
         } else if (this.value instanceof Color) {
+            // Constant-folding can generate Literal expressions that you
+            // couldn't actually generate with a "literal" expression,
+            // so we have to implement an equivalent serialization here
             return ["rgba"].concat(this.value.toArray());
-        } else if (this.value instanceof CollatorInstantiation) {
+        } else if (this.value instanceof Collator) {
+            // Same as Color above: literal serialization delegated to
+            // Collator (not CollatorExpression)
             return this.value.serialize();
         } else {
             assert(this.value === null ||

src/style-spec/expression/values.js

@@ -3,7 +3,8 @@
 import assert from 'assert';
 
 import Color from '../util/color';
-import { NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, array } from './types';
+import { Collator } from './definitions/collator';
+import { NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, array } from './types';
 
 import type { Type } from './types';
 
@@ -26,7 +27,7 @@ export function validateRGBA(r: mixed, g: mixed, b: mixed, a?: mixed): ?string {
     return null;
 }
 
-export type Value = null | string | boolean | number | Color | $ReadOnlyArray<Value> | { +[string]: Value }
+export type Value = null | string | boolean | number | Color | Collator | $ReadOnlyArray<Value> | { +[string]: Value }
 
 export function isValue(mixed: mixed): boolean {
     if (mixed === null) {
@@ -39,6 +40,8 @@ export function isValue(mixed: mixed): boolean {
         return true;
     } else if (mixed instanceof Color) {
         return true;
+    } else if (mixed instanceof Collator) {
+        return true;
     } else if (Array.isArray(mixed)) {
         for (const item of mixed) {
             if (!isValue(item)) {
@@ -69,6 +72,8 @@ export function typeOf(value: Value): Type {
         return NumberType;
     } else if (value instanceof Color) {
         return ColorType;
+    } else if (value instanceof Collator) {
+        return CollatorType;
     } else if (Array.isArray(value)) {
         const length = value.length;
         let itemType: ?Type;
@@ -92,4 +97,4 @@ export function typeOf(value: Value): Type {
     }
 }
 
-export {Color};
+export { Color, Collator };

src/style-spec/reference/v8.json

@@ -2194,7 +2194,7 @@
         }
       },
       "collator": {
-        "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.",
+        "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": {