Evaluate expressions in array members (#518)

mapbox · Apr 17, 2023 · 7b029c0 · 7b029c0
1 parent 22bfe9a
commit 7b029c0
Show file tree

Hide file tree

Showing 16 changed files with 93 additions and 59 deletions.
diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js
@@ -2,8 +2,8 @@
 
 import assert from 'assert';
 
-import {BooleanType, ColorType, NumberType, StringType, ValueType} from '../types.js';
-import {Color, toString as valueToString, validateRGBA} from '../values.js';
+import {BooleanType, ColorType, NumberType, StringType, ValueType, array, NullType} from '../types.js';
+import {Color, isValue, toString as valueToString, typeOf, validateRGBA} from '../values.js';
 import RuntimeError from '../runtime_error.js';
 import Formatted from '../types/formatted.js';
 import FormatExpression from '../definitions/format.js';
@@ -13,7 +13,8 @@ import ResolvedImage from '../types/resolved_image.js';
 import type {Expression, SerializedExpression} from '../expression.js';
 import type ParsingContext from '../parsing_context.js';
 import type EvaluationContext from '../evaluation_context.js';
-import type {Type} from '../types.js';
+import type {Type, ArrayType} from '../types.js';
+import getType from '../../util/get_type.js';
 
 const types = {
     'to-boolean': BooleanType,
@@ -30,7 +31,7 @@ const types = {
  * @private
  */
 class Coercion implements Expression {
-    type: Type;
+    type: Type | ArrayType;
     args: Array<Expression>;
 
     constructor(type: Type, args: Array<Expression>) {
@@ -43,24 +44,60 @@ class Coercion implements Expression {
             return context.error(`Expected at least one argument.`);
 
         const name: string = (args[0]: any);
-        assert(types[name], name);
+        const parsed = [];
+        let type: Type | ArrayType = NullType;
+        if (name === 'to-array') {
+            if (!Array.isArray(args[1])) {
+                return null;
+            }
+            const arrayLength = args[1].length;
+            if (context.expectedType) {
+                if (context.expectedType.kind === 'array') {
+                    type = array(context.expectedType.itemType, arrayLength);
+                } else {
+                    return context.error(`Expected ${context.expectedType.kind} but found array.`);
+                }
+            } else if (arrayLength > 0 && isValue(args[1][0])) {
+                const value = (args[1][0]: any);
+                type = array(typeOf(value), arrayLength);
+            } else {
+                return null;
+            }
+            for (let i = 0; i < arrayLength; i++) {
+                // $FlowIgnore
+                const member = args[1][i];
+                let parsedMember;
+                if (getType(member) === 'array') {
+                    parsedMember = context.parse(member, undefined, type.itemType);
+                } else {
+                    const memberType = getType(member);
+                    if (memberType !== type.itemType.kind) {
+                        return context.error(`Expected ${type.itemType.kind} but found ${memberType}.`);
+                    }
+                    parsedMember = context.registry['literal'].parse(['literal', member === undefined ? null : member], context);
+                }
+                if (!parsedMember) return null;
+                parsed.push(parsedMember);
+            }
+        } else {
+            assert(types[name], name);
 
-        if ((name === 'to-boolean' || name === 'to-string') && args.length !== 2)
-            return context.error(`Expected one argument.`);
+            if ((name === 'to-boolean' || name === 'to-string') && args.length !== 2)
+                return context.error(`Expected one argument.`);
 
-        const type = types[name];
+            type = types[name];
 
-        const parsed = [];
-        for (let i = 1; i < args.length; i++) {
-            const input = context.parse(args[i], i, ValueType);
-            if (!input) return null;
-            parsed.push(input);
+            for (let i = 1; i < args.length; i++) {
+                const input = context.parse(args[i], i, ValueType);
+                if (!input) return null;
+                parsed.push(input);
+            }
         }
 
         return new Coercion(type, parsed);
     }
 
-    evaluate(ctx: EvaluationContext): null | boolean | number | string | Color | Formatted | ResolvedImage {
+    evaluate(ctx: EvaluationContext): any {
         if (this.type.kind === 'boolean') {
             return Boolean(this.args[0].evaluate(ctx));
         } else if (this.type.kind === 'color') {
@@ -102,6 +139,8 @@ class Coercion implements Expression {
             return Formatted.fromString(valueToString(this.args[0].evaluate(ctx)));
         } else if (this.type.kind === 'resolvedImage') {
             return ResolvedImage.fromString(valueToString(this.args[0].evaluate(ctx)));
+        } else if (this.type.kind === 'array') {
+            return this.args.map(arg => { return arg.evaluate(ctx); });
         } else {
             return valueToString(this.args[0].evaluate(ctx));
         }
@@ -124,7 +163,7 @@ class Coercion implements Expression {
             return new ImageExpression(this.args[0]).serialize();
         }
 
-        const serialized = [`to-${this.type.kind}`];
+        const serialized: Array<mixed> = this.type.kind === 'array' ? [] : [`to-${this.type.kind}`];
         this.eachChild(child => { serialized.push(child.serialize()); });
         return serialized;
     }

diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
@@ -319,7 +319,7 @@ export function normalizePropertyExpression<T>(value: PropertyValueSpecification
     if (isFunction(value)) {
         return (new StylePropertyFunction(value, specification): any);
 
-    } else if (isExpression(value)) {
+    } else if (isExpression(value) || (Array.isArray(value) && value.length > 0)) {
         const expression = createPropertyExpression(value, specification);
         if (expression.result === 'error') {
             // this should have been caught in validation

diff --git a/src/style-spec/expression/parsing_context.js b/src/style-spec/expression/parsing_context.js
@@ -62,7 +62,7 @@ class ParsingContext {
         bindings?: Array<[string, Expression]>,
         options: {typeAnnotation?: 'assert' | 'coerce' | 'omit'} = {}
     ): ?Expression {
-        if (index) {
+        if (index || expectedType) {
             return this.concat(index, expectedType, bindings)._parse(expr, options);
         }
         return this._parse(expr, options);
@@ -88,13 +88,7 @@ class ParsingContext {
                 return this.error(`Expected an array with at least one element. If you wanted a literal array, use ["literal", []].`);
             }
 
-            const op = expr[0];
-            if (typeof op !== 'string') {
-                this.error(`Expression name must be a string, but found ${typeof op} instead. If you wanted a literal array, use ["literal", [...]].`, 0);
-                return null;
-            }
-
-            const Expr = this.registry[op];
+            const Expr = typeof expr[0] === 'string' ? this.registry[expr[0]] : undefined;
             if (Expr) {
                 let parsed = Expr.parse(expr, this);
                 if (!parsed) return null;
@@ -137,7 +131,8 @@ class ParsingContext {
                 return parsed;
             }
 
-            return this.error(`Unknown expression "${op}". If you wanted a literal array, use ["literal", [...]].`, 0);
+            // Try to parse as array
+            return Coercion.parse(['to-array', expr], this);
         } else if (typeof expr === 'undefined') {
             return this.error(`'undefined' value invalid. Use null instead.`);
         } else if (typeof expr === 'object') {
@@ -155,7 +150,7 @@ class ParsingContext {
      * parsing, is copied by reference rather than cloned.
      * @private
      */
-    concat(index: number, expectedType?: ?Type, bindings?: Array<[string, Expression]>): ParsingContext {
+    concat(index: ?number, expectedType?: ?Type, bindings?: Array<[string, Expression]>): ParsingContext {
         const path = typeof index === 'number' ? this.path.concat(index) : this.path;
         const scope = bindings ? this.scope.concat(bindings) : this.scope;
         return new ParsingContext(

diff --git a/src/style-spec/expression/types.js b/src/style-spec/expression/types.js
@@ -26,7 +26,8 @@ export type Type =
     ErrorTypeT |
     CollatorTypeT |
     FormattedTypeT |
-    ResolvedImageTypeT
+    ResolvedImageTypeT |
+    ArrayType;
 
 export type ArrayType = {
     kind: 'array',

diff --git a/src/style-spec/validate/validate.js b/src/style-spec/validate/validate.js
@@ -28,6 +28,7 @@ import validateProjection from './validate_projection.js';
 import type {StyleReference} from '../reference/latest.js';
 import type {StyleSpecification} from '../types.js';
 import type ValidationError from '../error/validation_error.js';
+import getType from '../util/get_type.js';
 
 const VALIDATORS = {
     '*'() {
@@ -70,7 +71,7 @@ export type ValidationOptions = {
     styleSpec: StyleReference;
 }
 
-export default function validate(options: ValidationOptions): Array<ValidationError> {
+export default function validate(options: ValidationOptions, arrayAsExpression: boolean = false): Array<ValidationError> {
     const value = options.value;
     const valueSpec = options.valueSpec;
     const styleSpec = options.styleSpec;
@@ -80,7 +81,13 @@ export default function validate(options: ValidationOptions): Array<ValidationEr
     } else if (valueSpec.expression && isExpression(deepUnbundle(value))) {
         return validateExpression(options);
     } else if (valueSpec.type && VALIDATORS[valueSpec.type]) {
-        return VALIDATORS[valueSpec.type](options);
+        const valid = VALIDATORS[valueSpec.type](options);
+        if (arrayAsExpression === true && valid.length > 0 && getType(options.value) === "array") {
+            // Try to validate as an expression
+            return validateExpression(options);
+        } else {
+            return valid;
+        }
     } else {
         const valid = validateObject(extend({}, options, {
             valueSpec: valueSpec.type ? styleSpec[valueSpec.type] : valueSpec

diff --git a/src/style-spec/validate/validate_array.js b/src/style-spec/validate/validate_array.js
@@ -56,7 +56,7 @@ export default function validateArray(options: Options): Array<ValidationError>
             style,
             styleSpec,
             key: `${key}[${i}]`
-        }));
+        }, true));
     }
     return errors;
 }
diff --git a/test/integration/expression-tests/parse/non-string/test.json b/test/integration/expression-tests/parse/non-string/test.json
@@ -1,14 +1,14 @@
 {
   "expression": [1, 2],
+  "inputs": [[{}, {}]],
   "expected": {
     "compiled": {
-      "result": "error",
-      "errors": [
-        {
-          "key": "[0]",
-          "error": "Expression name must be a string, but found number instead. If you wanted a literal array, use [\"literal\", [...]]."
-        }
-      ]
-    }
+      "result": "success",
+      "isFeatureConstant": true,
+      "isZoomConstant": true,
+      "type": "array<number, 2>"
+    },
+    "outputs": [[1, 2]],
+    "serialized": [1, 2]
   }
-}
+}
diff --git a/test/integration/expression-tests/parse/unknown-expression/test.json b/test/integration/expression-tests/parse/unknown-expression/test.json
@@ -5,8 +5,8 @@
       "result": "error",
       "errors": [
         {
-          "key": "[1][4][0]",
-          "error": "Unknown expression \"FAKE-EXPRESSION\". If you wanted a literal array, use [\"literal\", [...]]."
+          "key": "[1][4]",
+          "error": "Expected number but found array."
         }
       ]
     }

diff --git a/test/unit/style-spec/fixture/filters.output-api-supported.json b/test/unit/style-spec/fixture/filters.output-api-supported.json
@@ -8,7 +8,7 @@
     "line": 22
   },
   {
-    "message": "layers[2].filter[0]: Unknown expression \"=\". If you wanted a literal array, use [\"literal\", [...]].",
+    "message": "layers[2].filter: Expected boolean but found array.",
     "line": 29
   },
   {

diff --git a/test/unit/style-spec/fixture/filters.output.json b/test/unit/style-spec/fixture/filters.output.json
@@ -8,7 +8,7 @@
     "line": 22
   },
   {
-    "message": "layers[2].filter[0]: Unknown expression \"=\". If you wanted a literal array, use [\"literal\", [...]].",
+    "message": "layers[2].filter: Expected boolean but found array.",
     "line": 29
   },
   {

diff --git a/test/unit/style-spec/fixture/sources.output-api-supported.json b/test/unit/style-spec/fixture/sources.output-api-supported.json
@@ -20,11 +20,11 @@
     "line": 34
   },
   {
-    "message": "sources.video-wrong-coordinates.coordinates[2][1]: number expected, string found",
+    "message": "sources.video-wrong-coordinates.coordinates[2]: Expected number but found string.",
     "line": 34
   },
   {
-    "message": "sources.video-wrong-coordinates.coordinates[3]: array length 2 expected, length 0 found",
+    "message": "sources.video-wrong-coordinates.coordinates[3]: Expected an array with at least one element. If you wanted a literal array, use [\"literal\", []].",
     "line": 34
   },
   {

diff --git a/test/unit/style-spec/fixture/sources.output.json b/test/unit/style-spec/fixture/sources.output.json
@@ -20,11 +20,11 @@
     "line": 34
   },
   {
-    "message": "sources.video-wrong-coordinates.coordinates[2][1]: number expected, string found",
+    "message": "sources.video-wrong-coordinates.coordinates[2]: Expected number but found string.",
     "line": 34
   },
   {
-    "message": "sources.video-wrong-coordinates.coordinates[3]: array length 2 expected, length 0 found",
+    "message": "sources.video-wrong-coordinates.coordinates[3]: Expected an array with at least one element. If you wanted a literal array, use [\"literal\", []].",
     "line": 34
   },
   {

diff --git a/test/unit/style-spec/fixture/text-field-format.output-api-supported.json b/test/unit/style-spec/fixture/text-field-format.output-api-supported.json
@@ -11,10 +11,6 @@
     "message": "layers[6].layout.text-field[1]: Expected number but found string instead.",
     "line": 99
   },
-  {
-    "message": "layers[7].layout.text-field[1][0]: Unknown expression \"Helvetica\". If you wanted a literal array, use [\"literal\", [...]].",
-    "line": 111
-  },
   {
     "message": "layers[10].layout.text-field[1]: Expected array<string> but found number instead.",
     "line": 147

diff --git a/test/unit/style-spec/fixture/text-field-format.output.json b/test/unit/style-spec/fixture/text-field-format.output.json
@@ -11,10 +11,6 @@
     "message": "layers[6].layout.text-field[1]: Expected number but found string instead.",
     "line": 99
   },
-  {
-    "message": "layers[7].layout.text-field[1][0]: Unknown expression \"Helvetica\". If you wanted a literal array, use [\"literal\", [...]].",
-    "line": 111
-  },
   {
     "message": "layers[10].layout.text-field[1]: Expected array<string> but found number instead.",
     "line": 147

diff --git a/test/unit/style/light.test.js b/test/unit/style/light.test.js
@@ -84,13 +84,13 @@ test('Light#setLight', (t) => {
         const light = new Light({});
 
         const lightSpy = t.spy(light, '_validate');
-        light.setLight({color: [999]}, {validate: false});
+        light.setLight({color: 999}, {validate: false});
         light.updateTransitions({transition: false}, {});
         light.recalculate({zoom: 16, now: 10});
 
         t.ok(lightSpy.calledOnce);
         t.deepEqual(lightSpy.args[0][2], {validate: false});
-        t.deepEqual(light.properties.get('color'), [999]);
+        t.deepEqual(light.properties.get('color'), 999);
         t.end();
     });
     t.end();

diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js
@@ -902,12 +902,12 @@ test('Map', (t) => {
                 const fog = new Fog({});
                 const fogSpy = t.spy(fog, '_validate');
 
-                fog.set({color: [444]}, {validate: false});
+                fog.set({color: 444}, {validate: false});
                 fog.updateTransitions({transition: false}, {});
                 fog.recalculate({zoom: 16, now: 10});
 
                 t.ok(fogSpy.calledOnce);
-                t.deepEqual(fog.properties.get('color'), [444]);
+                t.deepEqual(fog.properties.get('color'), 444);
                 t.end();
             });
             t.end();