From cc103991bbbd00389e31199743c27f6f0ae70951 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Fri, 30 Jun 2017 12:24:04 -0400 Subject: [PATCH] Fix array handling following OO-ification --- src/style-spec/function/convert.js | 2 +- src/style-spec/function/definitions/index.js | 36 ++-- src/style-spec/function/evaluation_context.js | 10 +- src/style-spec/function/expression.js | 154 +++++++++++------- .../to_string/basic/test.json | 2 +- .../expression-tests/typeof/basic/test.json | 31 +++- 6 files changed, 155 insertions(+), 80 deletions(-) diff --git a/src/style-spec/function/convert.js b/src/style-spec/function/convert.js index ef772372b5b..acdf410075f 100644 --- a/src/style-spec/function/convert.js +++ b/src/style-spec/function/convert.js @@ -46,7 +46,7 @@ function annotateValue(value, spec) { if (spec.type === 'color') { return ['parse_color', ['string', value]]; } else if (spec.type === 'array' && typeof spec.length === 'number') { - return ['array', spec.value, value]; + return ['array', spec.value, spec.length, value]; } else if (spec.type === 'array') { return ['array', spec.value, value]; } else { diff --git a/src/style-spec/function/definitions/index.js b/src/style-spec/function/definitions/index.js index ca79bdcc401..7f6a55a456a 100644 --- a/src/style-spec/function/definitions/index.js +++ b/src/style-spec/function/definitions/index.js @@ -16,7 +16,7 @@ const { nargs } = require('../types'); -const { ParsingError, LambdaExpression } = require('../expression'); +const { parseExpression, ParsingError, LambdaExpression } = require('../expression'); const MatchExpression = require('./match'); const CurveExpression = require('./curve'); @@ -43,7 +43,6 @@ const expressions: { [string]: Class } = { 'object': defineAssertion('object', ObjectType), 'array': class extends LambdaExpression { static getName() { return 'array'; } - static getType() { return lambda(array(ValueType), StringType, ValueType); } static parse(args, context) { const types : {[string]:Type} = { string: StringType, @@ -51,21 +50,32 @@ const expressions: { [string]: Class } = { boolean: BooleanType }; - if (args.length === 2 && typeof args[0] === 'string') { - const itemType = types[args[0]]; - if (!itemType) - throw new ParsingError(`${context.key}.1`, `Unsupported array item type ${args[0]}`); - const parsed = super.parse(args, context); - parsed.type = lambda(array(itemType), StringType, ValueType); - return parsed; - } else { - args = ['Value'].concat(args); - return super.parse(args, context); + if (args.length === 0) + throw new ParsingError(context.key, 'Expected at least one argument to "array"'); + + const value = parseExpression(args[args.length - 1], context); + + let itemType = ValueType; + let N = undefined; + if (args.length > 1) { + if (typeof args[0] !== 'string' || !types[args[0]]) + throw new ParsingError(`${context.key}.1`, `The item type argument to "array" must be one of ${Object.keys(types).join(', ')}`); + itemType = types[args[0]]; + } + if (args.length > 2) { + if (typeof args[1] !== 'number') + throw new ParsingError(`${context.key}.2`, 'The length argument to "array" must be a number literal.'); + N = args[1]; } + return new this( + context.key, + lambda(array(itemType, N), ValueType), + [value] + ); } compile(args) { - return { js: `this.as(${args[1].js}, ${JSON.stringify(this.type.result.name)})` }; + return { js: `this.as(${args[args.length - 1].js}, ${JSON.stringify(this.type.result.name)})` }; } }, diff --git a/src/style-spec/function/evaluation_context.js b/src/style-spec/function/evaluation_context.js index 85bdba413b9..592a4478f66 100644 --- a/src/style-spec/function/evaluation_context.js +++ b/src/style-spec/function/evaluation_context.js @@ -1,6 +1,7 @@ const parseColor = require('../util/parse_color'); const interpolate = require('../util/interpolate'); const interpolationFactor = require('./interpolation_factor'); +const {ArrayLiteral} = require('./expression'); class RuntimeError extends Error { constructor(message) { @@ -97,13 +98,8 @@ module.exports = () => ({ array: function(type, items) { if (!type) { - let itemtype; - for (const item of items) { - if (!itemtype) itemtype = typeof item; - else if (itemtype !== typeof item) itemtype = 'Value'; - } - type = (!itemtype || itemtype === 'Value') ? - 'Array' : `Array<${titlecase(itemtype)}>`; + const t = ArrayLiteral.inferArrayType(items); + type = t.name; } return {type, items}; }, diff --git a/src/style-spec/function/expression.js b/src/style-spec/function/expression.js index 7c1fc52f0b6..195303ea49d 100644 --- a/src/style-spec/function/expression.js +++ b/src/style-spec/function/expression.js @@ -23,8 +23,6 @@ export type CompiledExpression = {| function?: Function |} -export type LiteralValue = null | string | number | boolean | {} | Array - const primitiveTypes = { string: StringType, number: NumberType, @@ -78,72 +76,107 @@ class BaseExpression { } class LiteralExpression extends BaseExpression { - type: PrimitiveType | ArrayType; - value: LiteralValue; - constructor(key: *, type: PrimitiveType | ArrayType, value: LiteralValue) { + value: any; + constructor(key: *, type: PrimitiveType | ArrayType, value: any) { super(key, type); this.value = value; } + compile() { throw new Error('Unimplemented'); } +} + +class PrimitiveLiteral extends LiteralExpression { + type: PrimitiveType; + value: null | string | number | boolean; + constructor( + key: *, + type: PrimitiveType, + value: null | string | number | boolean + ) { + super(key, type, value); + } + static parse(value: any, context: ParsingContext) { - const type = typeof value; - if ( - type === 'string' || - type === 'number' || - type === 'boolean' - ) { - return new this(context.key, primitiveTypes[type], value); - } + const type = value === null ? NullType : primitiveTypes[typeof value]; + return new this(context.key, type, value); + } - if (Array.isArray(value)) { - let itemType; - // infer the array's item type - for (const item of value) { - const t = primitiveTypes[typeof item]; - if (t && !itemType) { - itemType = t; - } else if (t && itemType === t) { - continue; - } else { - itemType = ValueType; - break; - } - } + compile() { + return {js: JSON.stringify(this.value)}; + } - const type = array(itemType || ValueType, value.length); - return new this( - context.key, - type, - value - ); - } else if (value && typeof value === 'object') { - return new this(context.key, ObjectType, value); - } else { - throw new ParsingError(context.key, `Expected an array or object, but found ${typeof value} instead`); + serialize(_: boolean) { + return this.value; + } +} + +class ArrayLiteral extends LiteralExpression { + type: ArrayType; + value: Array; + constructor(key: *, type: ArrayType, value: Array) { + super(key, type, value); + } + + static inferArrayType(value: Array) { + let itemType; + // infer the array's item type + for (const item of value) { + const t = primitiveTypes[typeof item]; + if (t && !itemType) { + itemType = t; + } else if (t && itemType === t) { + continue; + } else { + itemType = ValueType; + break; + } } + + return array(itemType || ValueType, value.length); + } + + static parse(value: Array, context: ParsingContext) { + return new this( + context.key, + this.inferArrayType(value), + value + ); } compile() { - let wrapped; - if (Array.isArray(this.value)) { - wrapped = { - type: this.type.name, - items: this.value - }; - } else if (this.value && typeof this.value === 'object') { - wrapped = { - type: this.type.name, - value: this.value - }; - } + const wrapped = { + type: this.type.name, + items: this.value + }; return { - js: wrapped ? `(${JSON.stringify(wrapped)})` : JSON.stringify(this.value) + js: `(${JSON.stringify(wrapped)})` }; } - serialize(_: boolean) { - return this.value; + serialize() { return ['literal', this.value]; } +} + +class ObjectLiteral extends LiteralExpression { + value: {}; + constructor(key: *, value: {}) { + super(key, ObjectType, value); + } + + static parse(value: {}, context: ParsingContext) { + return new this(context.key, value); } + + compile() { + const wrapped = { + type: this.type.name, + value: this.value + }; + return { + js: `(${JSON.stringify(wrapped)})` + }; + } + + serialize() { return ['literal', this.value]; } } class LambdaExpression extends BaseExpression { @@ -184,10 +217,10 @@ class LambdaExpression extends BaseExpression { function parseExpression(expr: mixed, context: ParsingContext) : Expression { const key = context.key; if (expr === null || typeof expr === 'undefined') - return new LiteralExpression(key, NullType, null); + return PrimitiveLiteral.parse(expr, context); if (primitiveTypes[typeof expr]) - return LiteralExpression.parse(expr, context); + return PrimitiveLiteral.parse(expr, context); if (!Array.isArray(expr)) { throw new ParsingError(key, `Expected an array, but found ${typeof expr} instead.`); @@ -201,7 +234,15 @@ function parseExpression(expr: mixed, context: ParsingContext) : Expression { if (op === 'literal') { if (expr.length !== 2) throw new ParsingError(key, `'literal' expression requires exactly one argument, but found ${expr.length - 1} instead.`); - return LiteralExpression.parse(expr[1], context.concat(1, 'literal')); + const argcontext = context.concat(1, 'literal'); + if (Array.isArray(expr[1])) { + return ArrayLiteral.parse(expr[1], argcontext); + } + if (expr[1] && typeof expr[1] === 'object') { + return ObjectLiteral.parse(expr[1], argcontext); + } + + throw new ParsingError(argcontext.key, `Expected argument to 'literal' to be an array or object, but found ${typeof expr[1]} instead.`); } const Expr = context.definitions[op]; @@ -217,5 +258,6 @@ module.exports = { ParsingError, parseExpression, LiteralExpression, - LambdaExpression + LambdaExpression, + ArrayLiteral }; diff --git a/test/integration/expression-tests/to_string/basic/test.json b/test/integration/expression-tests/to_string/basic/test.json index 92ac06fb1c5..696bbd2c4db 100644 --- a/test/integration/expression-tests/to_string/basic/test.json +++ b/test/integration/expression-tests/to_string/basic/test.json @@ -65,7 +65,7 @@ "false", "null", { - "error": "ExpressionEvaluationError: Expected a primitive value in [\"string\", ...], but found Array instead." + "error": "ExpressionEvaluationError: Expected a primitive value in [\"string\", ...], but found Array instead." }, { "error": "ExpressionEvaluationError: Expected a primitive value in [\"string\", ...], but found Object instead." diff --git a/test/integration/expression-tests/typeof/basic/test.json b/test/integration/expression-tests/typeof/basic/test.json index 15d3ba48c5b..dbdd37dfb79 100644 --- a/test/integration/expression-tests/typeof/basic/test.json +++ b/test/integration/expression-tests/typeof/basic/test.json @@ -43,7 +43,31 @@ {}, { "properties": { - "x": [] + "x": [1, 2, 3] + } + } + ], + [ + {}, + { + "properties": { + "x": ["a", "b", "c"] + } + } + ], + [ + {}, + { + "properties": { + "x": [true, false] + } + } + ], + [ + {}, + { + "properties": { + "x": [1, false] } } ], @@ -68,7 +92,10 @@ "String", "Number", "Boolean", - "Array", + "Array", + "Array", + "Array", + "Array", "Object" ] }