diff --git a/flow-typed/visitor.js b/flow-typed/visitor.js deleted file mode 100644 index 6f278ab7a76..00000000000 --- a/flow-typed/visitor.js +++ /dev/null @@ -1,3 +0,0 @@ -declare type Visitor = { - visit: (T) => void -}; diff --git a/src/style-spec/function/compile.js b/src/style-spec/function/compile.js index 34340cda698..353782186ee 100644 --- a/src/style-spec/function/compile.js +++ b/src/style-spec/function/compile.js @@ -8,9 +8,12 @@ const { CompilationContext } = require('./expression'); const parseExpression = require('./parse_expression'); -const { CompoundExpression } = require('./compound_expression'); const definitions = require('./definitions'); const evaluationContext = require('./evaluation_context'); +const { + isFeatureConstant, + isZoomConstant +} = require('./is_constant'); import type { Type } from './types.js'; import type { Expression, ParsingError } from './expression.js'; @@ -78,34 +81,3 @@ function compileExpression( }; } -function isFeatureConstant(e: Expression) { - let result = true; - e.accept({ - visit: (expression) => { - if (expression instanceof CompoundExpression) { - if (expression.name === 'get') { - result = result && (expression.args.length > 1); - } else if (expression.name === 'has') { - result = result && (expression.args.length > 1); - } else { - result = result && !( - expression.name === 'properties' || - expression.name === 'geometry-type' || - expression.name === 'id' - ); - } - } - } - }); - return result; -} - -function isZoomConstant(e: Expression) { - let result = true; - e.accept({ - visit: (expression) => { - if (expression.name === 'zoom') result = false; - } - }); - return result; -} diff --git a/src/style-spec/function/compound_expression.js b/src/style-spec/function/compound_expression.js index a1d48238fc2..027bc0ebf13 100644 --- a/src/style-spec/function/compound_expression.js +++ b/src/style-spec/function/compound_expression.js @@ -41,9 +41,8 @@ class CompoundExpression implements Expression { return [ name ].concat(args); } - accept(visitor: Visitor) { - visitor.visit(this); - this.args.forEach(a => a.accept(visitor)); + eachChild(fn: (Expression) => void) { + this.args.forEach(fn); } static parse(args: Array, context: ParsingContext): ?Expression { diff --git a/src/style-spec/function/definitions/array.js b/src/style-spec/function/definitions/array.js index 469307203bb..3c18c6ba08b 100644 --- a/src/style-spec/function/definitions/array.js +++ b/src/style-spec/function/definitions/array.js @@ -78,9 +78,8 @@ class ArrayAssertion implements Expression { } } - accept(visitor: Visitor) { - visitor.visit(this); - this.input.accept(visitor); + eachChild(fn: (Expression) => void) { + fn(this.input); } } diff --git a/src/style-spec/function/definitions/assertion.js b/src/style-spec/function/definitions/assertion.js index 623e4689b2a..da2565224c3 100644 --- a/src/style-spec/function/definitions/assertion.js +++ b/src/style-spec/function/definitions/assertion.js @@ -23,12 +23,12 @@ const types = { class Assertion implements Expression { key: string; type: Type; - inputs: Array; + args: Array; - constructor(key: string, type: Type, inputs: Array) { + constructor(key: string, type: Type, args: Array) { this.key = key; this.type = type; - this.inputs = inputs; + this.args = args; } static parse(args: Array, context: ParsingContext): ?Expression { @@ -40,33 +40,32 @@ class Assertion implements Expression { const type = types[name]; - const inputs = []; + const parsed = []; for (let i = 1; i < args.length; i++) { const input = parseExpression(args[i], context.concat(i, ValueType)); if (!input) return null; - inputs.push(input); + parsed.push(input); } - return new Assertion(context.key, type, inputs); + return new Assertion(context.key, type, parsed); } compile(ctx: CompilationContext) { const jsType = JSON.stringify(this.type.kind.toLowerCase()); - const inputs = []; - for (const input of this.inputs) { - inputs.push(ctx.addExpression(input.compile(ctx))); + const args = []; + for (const input of this.args) { + args.push(ctx.addExpression(input.compile(ctx))); } - const inputsVar = ctx.addVariable(`[${inputs.join(',')}]`); + const inputsVar = ctx.addVariable(`[${args.join(',')}]`); return `$this.asJSType(${jsType}, ${inputsVar})`; } serialize() { - return [ this.type.kind.toLowerCase() ].concat(this.inputs.map(i => i.serialize())); + return [ this.type.kind.toLowerCase() ].concat(this.args.map(i => i.serialize())); } - accept(visitor: Visitor) { - visitor.visit(this); - this.inputs.forEach(input => input.accept(visitor)); + eachChild(fn: (Expression) => void) { + this.args.forEach(fn); } } diff --git a/src/style-spec/function/definitions/at.js b/src/style-spec/function/definitions/at.js index a17005a2d7b..3d72df6d291 100644 --- a/src/style-spec/function/definitions/at.js +++ b/src/style-spec/function/definitions/at.js @@ -44,10 +44,9 @@ class At implements Expression { return [ 'at', this.index.serialize(), this.input.serialize() ]; } - accept(visitor: Visitor) { - visitor.visit(this); - this.index.accept(visitor); - this.input.accept(visitor); + eachChild(fn: (Expression) => void) { + fn(this.index); + fn(this.input); } } diff --git a/src/style-spec/function/definitions/case.js b/src/style-spec/function/definitions/case.js index 94303ae7dc8..72eef6dce30 100644 --- a/src/style-spec/function/definitions/case.js +++ b/src/style-spec/function/definitions/case.js @@ -73,13 +73,12 @@ class Case implements Expression { return result; } - accept(visitor: Visitor) { - visitor.visit(this); + eachChild(fn: (Expression) => void) { for (const [test, expression] of this.branches) { - test.accept(visitor); - expression.accept(visitor); + fn(test); + fn(expression); } - this.otherwise.accept(visitor); + fn(this.otherwise); } } diff --git a/src/style-spec/function/definitions/coalesce.js b/src/style-spec/function/definitions/coalesce.js index d18a4382bc7..bb2041997dd 100644 --- a/src/style-spec/function/definitions/coalesce.js +++ b/src/style-spec/function/definitions/coalesce.js @@ -50,11 +50,8 @@ class Coalesce implements Expression { return ['coalesce'].concat(this.args.map(a => a.serialize())); } - accept(visitor: Visitor) { - visitor.visit(this); - for (const arg of this.args) { - arg.accept(visitor); - } + eachChild(fn: (Expression) => void) { + this.args.forEach(fn); } } diff --git a/src/style-spec/function/definitions/coercion.js b/src/style-spec/function/definitions/coercion.js index 3b37fd4b865..5e09b2c0350 100644 --- a/src/style-spec/function/definitions/coercion.js +++ b/src/style-spec/function/definitions/coercion.js @@ -26,12 +26,12 @@ const types = { class Coercion implements Expression { key: string; type: Type; - inputs: Array; + args: Array; - constructor(key: string, type: Type, inputs: Array) { + constructor(key: string, type: Type, args: Array) { this.key = key; this.type = type; - this.inputs = inputs; + this.args = args; } static parse(args: Array, context: ParsingContext): ?Expression { @@ -43,32 +43,31 @@ class Coercion implements Expression { const type = types[name]; - const inputs = []; + const parsed = []; for (let i = 1; i < args.length; i++) { const input = parseExpression(args[i], context.concat(i, ValueType)); if (!input) return null; - inputs.push(input); + parsed.push(input); } - return new Coercion(context.key, type, inputs); + return new Coercion(context.key, type, parsed); } compile(ctx: CompilationContext) { - const inputs = []; - for (const input of this.inputs) { - inputs.push(ctx.addExpression(input.compile(ctx))); + const args = []; + for (const input of this.args) { + args.push(ctx.addExpression(input.compile(ctx))); } - const inputsVar = ctx.addVariable(`[${inputs.join(',')}]`); + const inputsVar = ctx.addVariable(`[${args.join(',')}]`); return `$this.to${this.type.kind}(${inputsVar})`; } serialize() { - return [ `to-${this.type.kind.toLowerCase()}` ].concat(this.inputs.map(i => i.serialize())); + return [ `to-${this.type.kind.toLowerCase()}` ].concat(this.args.map(i => i.serialize())); } - accept(visitor: Visitor) { - visitor.visit(this); - this.inputs.forEach(input => input.accept(visitor)); + eachChild(fn: (Expression) => void) { + this.args.forEach(fn); } } diff --git a/src/style-spec/function/definitions/contains.js b/src/style-spec/function/definitions/contains.js index 0977b9c37f3..b0a30b1de2e 100644 --- a/src/style-spec/function/definitions/contains.js +++ b/src/style-spec/function/definitions/contains.js @@ -50,10 +50,9 @@ class Contains implements Expression { return [ 'contains', this.value.serialize(), this.array.serialize() ]; } - accept(visitor: Visitor) { - visitor.visit(this); - this.array.accept(visitor); - this.value.accept(visitor); + eachChild(fn: (Expression) => void) { + fn(this.array); + fn(this.value); } } diff --git a/src/style-spec/function/definitions/curve.js b/src/style-spec/function/definitions/curve.js index e5e967735be..7d7ed778375 100644 --- a/src/style-spec/function/definitions/curve.js +++ b/src/style-spec/function/definitions/curve.js @@ -113,8 +113,8 @@ class Curve implements Expression { const label = rest[i]; const value = rest[i + 1]; - const labelKey = isStep ? i + 4 : i + 3; - const valueKey = isStep ? i + 5 : i + 4; + const labelKey = isStep ? i + 2 : i + 3; + const valueKey = isStep ? i + 3 : i + 4; if (typeof label !== 'number') { return context.error('Input/output pairs for "curve" expressions must be defined using literal numeric values (not computed expressions) for the input values.', labelKey); @@ -176,11 +176,10 @@ class Curve implements Expression { return result; } - accept(visitor: Visitor) { - visitor.visit(this); - this.input.accept(visitor); + eachChild(fn: (Expression) => void) { + fn(this.input); for (const [ , expression] of this.stops) { - expression.accept(visitor); + fn(expression); } } } diff --git a/src/style-spec/function/definitions/let.js b/src/style-spec/function/definitions/let.js index 96ff8a9317f..bec01888de0 100644 --- a/src/style-spec/function/definitions/let.js +++ b/src/style-spec/function/definitions/let.js @@ -33,12 +33,11 @@ class Let implements Expression { return serialized; } - accept(visitor: Visitor) { - visitor.visit(this); + eachChild(fn: (Expression) => void) { for (const binding of this.bindings) { - binding[1].accept(visitor); + fn(binding[1]); } - this.result.accept(visitor); + fn(this.result); } static parse(args: Array, context: ParsingContext) { diff --git a/src/style-spec/function/definitions/literal.js b/src/style-spec/function/definitions/literal.js index 8f458908f31..277da4edc61 100644 --- a/src/style-spec/function/definitions/literal.js +++ b/src/style-spec/function/definitions/literal.js @@ -4,7 +4,7 @@ const { Color, isValue, typeOf } = require('../values'); import type { Type } from '../types'; import type { Value } from '../values'; -import type { Expression, ParsingContext } from '../expression'; +import type { Expression, ParsingContext, CompilationContext } from '../expression'; const u2028 = /\u2028/g; const u2029 = /\u2029/g; @@ -45,9 +45,19 @@ class Literal implements Expression { return new Literal(context.key, type, value); } - compile() { - const value = Literal.compile(this.value); - return typeof this.value === 'object' ? `(${value})` : value; + compile(ctx: CompilationContext) { + let value; + if (this.type.kind === 'Color') { + value = `(new $this.Color(${(this.value: any).join(', ')}))`; + } else { + value = Literal.compile(this.value); + } + + if (typeof this.value === 'object' && this.value !== null) { + return ctx.addVariable(value); + } else { + return value; + } } static compile(value: Value) { @@ -72,7 +82,7 @@ class Literal implements Expression { } } - accept(visitor: Visitor) { visitor.visit(this); } + eachChild() {} } module.exports = Literal; diff --git a/src/style-spec/function/definitions/match.js b/src/style-spec/function/definitions/match.js index 4e58f059e09..0e982991a6e 100644 --- a/src/style-spec/function/definitions/match.js +++ b/src/style-spec/function/definitions/match.js @@ -135,13 +135,10 @@ class Match implements Expression { return result; } - accept(visitor: Visitor) { - visitor.visit(this); - this.input.accept(visitor); - for (const output of this.outputs) { - output.accept(visitor); - } - this.otherwise.accept(visitor); + eachChild(fn: (Expression) => void) { + fn(this.input); + this.outputs.forEach(fn); + fn(this.otherwise); } } diff --git a/src/style-spec/function/definitions/var.js b/src/style-spec/function/definitions/var.js index ea44d5d1a6e..ec77b36a097 100644 --- a/src/style-spec/function/definitions/var.js +++ b/src/style-spec/function/definitions/var.js @@ -35,7 +35,7 @@ class Var implements Expression { return [this.name]; } - accept(visitor: Visitor) { visitor.visit(this); } + eachChild() {} } diff --git a/src/style-spec/function/evaluation_context.js b/src/style-spec/function/evaluation_context.js index d0b77731029..67f4253b07d 100644 --- a/src/style-spec/function/evaluation_context.js +++ b/src/style-spec/function/evaluation_context.js @@ -62,6 +62,7 @@ function ensure(condition: any, message: string) { module.exports = () => ({ types: types, + Color: Color, // used for compiling color literals ensure: ensure, error: (msg: string) => ensure(false, msg), diff --git a/src/style-spec/function/expression.js b/src/style-spec/function/expression.js index 79b7f83a4ea..91e0ec90904 100644 --- a/src/style-spec/function/expression.js +++ b/src/style-spec/function/expression.js @@ -14,7 +14,7 @@ export interface Expression { compile(CompilationContext): string; // eslint-disable-line no-use-before-define serialize(): any; - accept(Visitor): void; + eachChild(fn: Expression => void): void; } class ParsingError extends Error { diff --git a/src/style-spec/function/is_constant.js b/src/style-spec/function/is_constant.js new file mode 100644 index 00000000000..c7dca51472c --- /dev/null +++ b/src/style-spec/function/is_constant.js @@ -0,0 +1,41 @@ +// @flow + +const { CompoundExpression } = require('./compound_expression'); + +import type { Expression } from './expression.js'; + +function isFeatureConstant(e: Expression) { + if (e instanceof CompoundExpression) { + if (e.name === 'get' && e.args.length === 1) { + return false; + } else if (e.name === 'has' && e.args.length === 1) { + return false; + } else if ( + e.name === 'properties' || + e.name === 'geometry-type' || + e.name === 'id' + ) { + return false; + } + } + + let result = true; + e.eachChild(arg => { + if (result && !isFeatureConstant(arg)) { result = false; } + }); + return result; +} + +function isZoomConstant(e: Expression) { + if (e instanceof CompoundExpression && e.name === 'zoom') { return false; } + let result = true; + e.eachChild((arg) => { + if (result && !isZoomConstant(arg)) { result = false; } + }); + return result; +} + +module.exports = { + isFeatureConstant, + isZoomConstant, +}; diff --git a/src/style-spec/function/parse_expression.js b/src/style-spec/function/parse_expression.js index bac93ef2fd0..35d19743a9f 100644 --- a/src/style-spec/function/parse_expression.js +++ b/src/style-spec/function/parse_expression.js @@ -1,5 +1,7 @@ // @flow +const Literal = require('./definitions/literal'); +const {CompilationContext} = require('./expression'); import type {ParsingContext, Expression} from './expression'; /** @@ -55,6 +57,22 @@ function parseExpression(expr: mixed, context: ParsingContext): ?Expression { } } + // If an expression's arguments are all literals, we can evaluate + // it immediately and replace it with a literal value in the + // parsed/compiled result. + if (!(parsed instanceof Literal) && isConstant(parsed)) { + const cc = new CompilationContext(); + const ec = require('./evaluation_context')(); + const compiled = cc.compileToFunction(parsed, ec); + try { + const value = compiled({}, {}); + parsed = new Literal(parsed.key, parsed.type, value); + } catch (e) { + context.error(e.message); + return null; + } + } + return parsed; } @@ -68,4 +86,27 @@ function parseExpression(expr: mixed, context: ParsingContext): ?Expression { } } +function isConstant(expression: Expression) { + // requires within function body to workaround circular dependency + const {CompoundExpression} = require('./compound_expression'); + const {isZoomConstant, isFeatureConstant} = require('./is_constant'); + const Var = require('./definitions/var'); + + if (expression instanceof Var) { + return false; + } else if (expression instanceof CompoundExpression && expression.name === 'error') { + return false; + } + + let literalArgs = true; + expression.eachChild(arg => { + if (!(arg instanceof Literal)) { literalArgs = false; } + }); + if (!literalArgs) { + return false; + } + + return isZoomConstant(expression) && isFeatureConstant(expression); +} + module.exports = parseExpression; diff --git a/test/integration/expression-tests/constant-folding/evaluation-error/test.json b/test/integration/expression-tests/constant-folding/evaluation-error/test.json new file mode 100644 index 00000000000..287d52c8395 --- /dev/null +++ b/test/integration/expression-tests/constant-folding/evaluation-error/test.json @@ -0,0 +1,28 @@ +{ + "expectExpressionType": {"kind": "Color"}, + "expression": [ + "curve", + ["step"], + ["get", "x"], + "black", + 0, + "invalid", + 10, + "blue" + ], + "inputs": [ + [{}, {"properties": {"x": -1}}], + [{}, {"properties": {"x": 0}}], + [{}, {"properties": {"x": 5}}], + [{}, {"properties": {"x": 10}}], + [{}, {"properties": {"x": 11}}] + ], + "expected": { + "compiled": { + "result": "error", + "errors": [ + {"key": "[5]", "error": "Could not parse color from value 'invalid'"} + ] + } + } +} diff --git a/test/integration/expression-tests/constant-folding/to-color/test.json b/test/integration/expression-tests/constant-folding/to-color/test.json new file mode 100644 index 00000000000..ae5cb8b13f7 --- /dev/null +++ b/test/integration/expression-tests/constant-folding/to-color/test.json @@ -0,0 +1,14 @@ +{ + "expectExpressionType": null, + "expression": ["to-color", "red"], + "inputs": [[{}, {}]], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": true, + "isZoomConstant": true, + "type": "Color" + }, + "outputs": [[1, 0, 0, 1]] + } +} diff --git a/test/integration/expression-tests/get/from-literal--missing/test.json b/test/integration/expression-tests/get/from-literal--missing/test.json index 3226f4c398d..46adce39357 100644 --- a/test/integration/expression-tests/get/from-literal--missing/test.json +++ b/test/integration/expression-tests/get/from-literal--missing/test.json @@ -4,13 +4,13 @@ "inputs": [[{}, {}]], "expected": { "compiled": { - "result": "success", - "isFeatureConstant": true, - "isZoomConstant": true, - "type": "Number" - }, - "outputs": [ - {"error": "Expected value to be of type Number, but found Null instead."} - ] + "result": "error", + "errors": [ + { + "key": "", + "error": "Expected value to be of type Number, but found Null instead." + } + ] + } } } diff --git a/test/integration/lib/expression.js b/test/integration/lib/expression.js index cff81de7660..c6f5e36202f 100644 --- a/test/integration/lib/expression.js +++ b/test/integration/lib/expression.js @@ -5,7 +5,21 @@ const path = require('path'); const harness = require('./harness'); const diff = require('diff'); const fs = require('fs'); -const stringify = require('json-stringify-pretty-compact'); +const compactStringify = require('json-stringify-pretty-compact'); + +// we have to handle this edge case here because we have test fixtures for this +// edge case, and we don't want UPDATE=1 to mess with them +function stringify(v) { + let s = compactStringify(v); + // http://timelessrepo.com/json-isnt-a-javascript-subset + if (s.indexOf('\u2028') >= 0) { + s = s.replace(/\u2028/g, '\\u2028'); + } + if (s.indexOf('\u2029') >= 0) { + s = s.replace(/\u2029/g, '\\u2029'); + } + return s; +} let linter; try {