Preparatory expression refactors (#5407)

* Add heatmap-density expression (prepares for #5253) * Accept options for context in which expression is used Prepares for #5193 * Unify creatExpression and createExpressionWithErrorHandling Closes #5409 * declaration => property
mapbox · Oct 5, 2017 · 2d359bb · 2d359bb
1 parent dff1fa4
commit 2d359bb
Show file tree

Hide file tree

Showing 19 changed files with 199 additions and 112 deletions.
diff --git a/bench/benchmarks/expressions.js b/bench/benchmarks/expressions.js
@@ -7,7 +7,7 @@ const createFunction = require('../../src/style-spec/function');
 const convertFunction = require('../../src/style-spec/function/convert');
 const {
     isExpression,
-    createExpressionWithErrorHandling,
+    createExpression,
     getExpectedType,
     getDefaultValue
 } = require('../../src/style-spec/expression');
@@ -40,7 +40,14 @@ class ExpressionBenchmark extends Benchmark {
                     const expressionData = function(rawValue, propertySpec: StylePropertySpecification) {
                         const rawExpression = convertFunction(rawValue, propertySpec);
                         const compiledFunction = createFunction(rawValue, propertySpec);
-                        const compiledExpression = createExpressionWithErrorHandling(rawExpression, getExpectedType(propertySpec), getDefaultValue(propertySpec));
+                        const compiledExpression = createExpression(rawExpression, {
+                            context: 'property',
+                            expectedType: getExpectedType(propertySpec),
+                            defaultValue: getDefaultValue(propertySpec)
+                        });
+                        if (compiledExpression.result !== 'success') {
+                            throw new Error(compiledExpression.errors.map(err => `${err.key}: ${err.message}`).join(', '));
+                        }
                         return {
                             propertySpec,
                             rawValue,
@@ -93,7 +100,11 @@ class FunctionConvert extends ExpressionBenchmark {
 class ExpressionCreate extends ExpressionBenchmark {
     bench() {
         for (const {rawExpression, propertySpec} of this.data) {
-            createExpressionWithErrorHandling(rawExpression, getExpectedType(propertySpec), getDefaultValue(propertySpec));
+            createExpression(rawExpression, {
+                context: 'property',
+                expectedType: getExpectedType(propertySpec),
+                defaultValue: getDefaultValue(propertySpec)
+            });
         }
     }
 }

diff --git a/src/style-spec/expression/definitions/index.js b/src/style-spec/expression/definitions/index.js
@@ -185,6 +185,11 @@ CompoundExpression.register(expressions, {
         [],
         (ctx) => ctx.globals.zoom
     ],
+    'heatmap-density': [
+        NumberType,
+        [],
+        (ctx) => ctx.globals.heatmapDensity || 0
+    ],
     '+': [
         NumberType,
         varargs(NumberType),

diff --git a/src/style-spec/expression/evaluation_context.js b/src/style-spec/expression/evaluation_context.js
@@ -5,13 +5,13 @@ const Scope = require('./scope');
 const parseColor = require('../util/parse_color');
 const {Color} = require('./values');
 
-import type { Feature } from './index';
+import type { Feature, GlobalProperties } from './index';
 import type { Expression } from './expression';
 
 const geometryTypes = ['Unknown', 'Point', 'LineString', 'Polygon'];
 
 class EvaluationContext {
-    globals: {zoom: number};
+    globals: GlobalProperties;
     feature: ?Feature;
 
     scope: Scope;

diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
@@ -11,7 +11,6 @@ const Let = require('./definitions/let');
 const definitions = require('./definitions');
 const isConstant = require('./is_constant');
 const {unwrap} = require('./values');
-const extend = require('../util/extend');
 
 import type {Type} from './types';
 import type {Value} from './values';
@@ -24,32 +23,69 @@ export type Feature = {
     +properties: {[string]: any}
 };
 
+export type GlobalProperties = {
+    zoom: number,
+    heatmapDensity?: number
+};
+
+export type StyleExpressionContext = 'property' | 'filter';
+
+export type StyleExpressionOptions = {
+    context: StyleExpressionContext,
+    expectedType: Type | null,
+    defaultValue?: Value | null
+}
+
 export type StyleExpressionErrors = {
     result: 'error',
     errors: Array<ParsingError>
 };
 
-export type StyleExpression = {
+type ZoomConstantExpression = {
     result: 'success',
+    context: StyleExpressionContext,
     isZoomConstant: true,
     isFeatureConstant: boolean,
-    evaluate: (globals: {zoom: number}, feature?: Feature) => any,
+    evaluate: (globals: GlobalProperties, feature?: Feature) => any,
     // parsed: Expression
-} | {
+};
+
+export type StyleDeclarationExpression = ZoomConstantExpression | {
     result: 'success',
+    context: 'property',
     isZoomConstant: false,
     isFeatureConstant: boolean,
-    evaluate: (globals: {zoom: number}, feature?: Feature) => any,
+    evaluate: (globals: GlobalProperties, feature?: Feature) => any,
     // parsed: Expression,
     interpolation: InterpolationType,
     zoomStops: Array<number>
 };
 
+export type StyleFilterExpression = ZoomConstantExpression | {
+    result: 'success',
+    context: 'filter',
+    isZoomConstant: false,
+    isFeatureConstant: boolean,
+    evaluate: (GlobalProperties, feature?: Feature) => any,
+    // parsed: Expression,
+};
+
+export type StyleExpression = StyleDeclarationExpression | StyleFilterExpression;
+
 type StylePropertyValue = null | string | number | Array<string> | Array<number>;
 type FunctionParameters = DataDrivenPropertyValueSpecification<StylePropertyValue>
 
-function createExpression(expression: mixed, expectedType: Type | null): StyleExpressionErrors | StyleExpression {
-    const parser = new ParsingContext(definitions, [], expectedType);
+/**
+ * Parse and typecheck the given style spec JSON expression.  If
+ * options.defaultValue is provided, then the resulting StyleExpression's
+ * `evaluate()` method will handle errors by logging a warning (once per
+ * message) and returning the default value.  Otherwise, it will throw
+ * evaluation errors.
+ *
+ * @private
+ */
+function createExpression(expression: mixed, options: StyleExpressionOptions): StyleExpressionErrors | StyleExpression {
+    const parser = new ParsingContext(definitions, [], options.expectedType);
     const parsed = parser.parse(expression);
     if (!parsed) {
         assert(parser.errors.length > 0);
@@ -60,19 +96,55 @@ function createExpression(expression: mixed, expectedType: Type | null): StyleEx
     }
 
     const evaluator = new EvaluationContext();
-    function evaluate(globals, feature) {
-        evaluator.globals = globals;
-        evaluator.feature = feature;
-        return parsed.evaluate(evaluator);
+
+    let evaluate;
+    if (options.defaultValue === undefined) {
+        evaluate = function (globals, feature) {
+            evaluator.globals = globals;
+            evaluator.feature = feature;
+            return parsed.evaluate(evaluator);
+        };
+    } else {
+        const warningHistory: {[key: string]: boolean} = {};
+        const defaultValue = options.defaultValue;
+        evaluate = function (globals, feature) {
+            evaluator.globals = globals;
+            evaluator.feature = feature;
+            try {
+                const val = parsed.evaluate(evaluator);
+                if (val === null || val === undefined) {
+                    return unwrap(defaultValue);
+                }
+                return unwrap(val);
+            } catch (e) {
+                if (!warningHistory[e.message]) {
+                    warningHistory[e.message] = true;
+                    if (typeof console !== 'undefined') {
+                        console.warn(e.message);
+                    }
+                }
+                return unwrap(defaultValue);
+            }
+        };
     }
 
     const isFeatureConstant = isConstant.isFeatureConstant(parsed);
-    const isZoomConstant = isConstant.isZoomConstant(parsed);
+    const isZoomConstant = isConstant.isGlobalPropertyConstant(parsed, ['zoom']);
 
     if (isZoomConstant) {
         return {
             result: 'success',
-            isZoomConstant,
+            context: options.context,
+            isZoomConstant: true,
+            isFeatureConstant,
+            evaluate,
+            parsed
+        };
+    } else if (options.context === 'filter') {
+        return {
+            result: 'success',
+            context: 'filter',
+            isZoomConstant: false,
             isFeatureConstant,
             evaluate,
             parsed
@@ -94,6 +166,7 @@ function createExpression(expression: mixed, expectedType: Type | null): StyleEx
 
     return {
         result: 'success',
+        context: 'property',
         isZoomConstant: false,
         isFeatureConstant,
         evaluate,
@@ -107,40 +180,7 @@ function createExpression(expression: mixed, expectedType: Type | null): StyleEx
     };
 }
 
-function createExpressionWithErrorHandling(expression: mixed, expectedType: Type | null, defaultValue: Value | null): StyleExpression {
-    expression = createExpression(expression, expectedType);
-
-    if (expression.result !== 'success') {
-        // this should have been caught in validation
-        throw new Error(expression.errors.map(err => `${err.key}: ${err.message}`).join(', '));
-    }
-
-    const evaluate = expression.evaluate;
-    const warningHistory: {[key: string]: boolean} = {};
-
-    return extend({}, expression, {
-        evaluate(globals, feature) {
-            try {
-                const val = evaluate(globals, feature);
-                if (val === null || val === undefined) {
-                    return unwrap(defaultValue);
-                }
-                return unwrap(val);
-            } catch (e) {
-                if (!warningHistory[e.message]) {
-                    warningHistory[e.message] = true;
-                    if (typeof console !== 'undefined') {
-                        console.warn(e.message);
-                    }
-                }
-                return unwrap(defaultValue);
-            }
-        }
-    });
-}
-
-module.exports = createExpression;
-module.exports.createExpressionWithErrorHandling = createExpressionWithErrorHandling;
+module.exports.createExpression = createExpression;
 module.exports.isExpression = isExpression;
 module.exports.getExpectedType = getExpectedType;
 module.exports.getDefaultValue = getDefaultValue;

diff --git a/src/style-spec/expression/is_constant.js b/src/style-spec/expression/is_constant.js
@@ -26,16 +26,16 @@ function isFeatureConstant(e: Expression) {
     return result;
 }
 
-function isZoomConstant(e: Expression) {
-    if (e instanceof CompoundExpression && e.name === 'zoom') { return false; }
+function isGlobalPropertyConstant(e: Expression, properties: Array<string>) {
+    if (e instanceof CompoundExpression && properties.indexOf(e.name) >= 0) { return false; }
     let result = true;
     e.eachChild((arg) => {
-        if (result && !isZoomConstant(arg)) { result = false; }
+        if (result && !isGlobalPropertyConstant(arg, properties)) { result = false; }
     });
     return result;
 }
 
 module.exports = {
     isFeatureConstant,
-    isZoomConstant,
+    isGlobalPropertyConstant,
 };
diff --git a/src/style-spec/expression/parsing_context.js b/src/style-spec/expression/parsing_context.js
@@ -163,7 +163,7 @@ module.exports = ParsingContext;
 function isConstant(expression: Expression) {
     // requires within function body to workaround circular dependency
     const {CompoundExpression} = require('./compound_expression');
-    const {isZoomConstant, isFeatureConstant} = require('./is_constant');
+    const {isGlobalPropertyConstant, isFeatureConstant} = require('./is_constant');
     const Var = require('./definitions/var');
 
     if (expression instanceof Var) {
@@ -180,5 +180,6 @@ function isConstant(expression: Expression) {
         return false;
     }
 
-    return isZoomConstant(expression) && isFeatureConstant(expression);
+    return isFeatureConstant(expression) &&
+        isGlobalPropertyConstant(expression, ['zoom', 'heatmap-density']);
 }
diff --git a/src/style-spec/function/convert.js b/src/style-spec/function/convert.js
@@ -3,7 +3,7 @@ const extend = require('../util/extend');
 
 module.exports = convertFunction;
 
-function convertFunction(parameters, propertySpec) {
+function convertFunction(parameters, propertySpec, name) {
     let expression;
 
     parameters = extend({}, parameters);
@@ -30,7 +30,10 @@ function convertFunction(parameters, propertySpec) {
             throw new Error('Unimplemented');
         }
 
-        if (zoomAndFeatureDependent) {
+        if (name === 'heatmap-color') {
+            assert(zoomDependent);
+            expression = convertZoomFunction(parameters, propertySpec, stops, ['heatmap-density']);
+        } else if (zoomAndFeatureDependent) {
             expression = convertZoomAndPropertyFunction(parameters, propertySpec, stops, defaultExpression);
         } else if (zoomDependent) {
             expression = convertZoomFunction(parameters, propertySpec, stops);
@@ -177,16 +180,16 @@ function convertPropertyFunction(parameters, propertySpec, stops, defaultExpress
     return expression;
 }
 
-function convertZoomFunction(parameters, propertySpec, stops) {
+function convertZoomFunction(parameters, propertySpec, stops, input = ['zoom']) {
     const type = getFunctionType(parameters, propertySpec);
     let expression;
     let isStep = false;
     if (type === 'interval') {
-        expression = ['curve', ['step'], ['zoom']];
+        expression = ['curve', ['step'], input];
         isStep = true;
     } else if (type === 'exponential') {
         const base = parameters.base !== undefined ? parameters.base : 1;
-        expression = ['curve', ['exponential', base], ['zoom']];
+        expression = ['curve', ['exponential', base], input];
     } else {
         throw new Error(`Unknown zoom function type "${type}"`);
     }

diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js
@@ -9,7 +9,7 @@ function identityFunction(x) {
     return x;
 }
 
-function createFunction(parameters, propertySpec) {
+function createFunction(parameters, propertySpec, name) {
     const isColor = propertySpec.type === 'color';
     const zoomAndFeatureDependent = parameters.stops && typeof parameters.stops[0][0] === 'object';
     const featureDependent = zoomAndFeatureDependent || parameters.property !== undefined;
@@ -118,16 +118,20 @@ function createFunction(parameters, propertySpec) {
             }
         };
     } else if (zoomDependent) {
+        let evaluate;
+        if (name === 'heatmap-color') {
+            evaluate = ({heatmapDensity}) => outputFunction(innerFun(parameters, propertySpec, heatmapDensity, hashedStops, categoricalKeyType));
+        } else {
+            evaluate = ({zoom}) => outputFunction(innerFun(parameters, propertySpec, zoom, hashedStops, categoricalKeyType));
+        }
         return {
             isFeatureConstant: true,
             isZoomConstant: false,
             interpolation: type === 'exponential' ?
                 {name: 'exponential', base: parameters.base !== undefined ? parameters.base : 1} :
                 {name: 'step'},
             zoomStops: parameters.stops.map(s => s[0]),
-            evaluate({zoom}) {
-                return outputFunction(innerFun(parameters, propertySpec, zoom, hashedStops, categoricalKeyType));
-            }
+            evaluate
         };
     } else {
         return {

diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js
@@ -1,14 +1,17 @@
 
 const ValidationError = require('../error/validation_error');
-const createExpression = require('../expression');
+const {createExpression} = require('../expression');
 const {getExpectedType, getDefaultValue} = require('../expression');
 const unbundle = require('../util/unbundle_jsonlint');
 
 module.exports = function validateExpression(options) {
     const expression = createExpression(
         deepUnbundle(options.value.expression),
-        getExpectedType(options.valueSpec),
-        getDefaultValue(options.valueSpec));
+        {
+            context: options.expressionContext,
+            expectedType: getExpectedType(options.valueSpec),
+            defaultValue: getDefaultValue(options.valueSpec)
+        });
 
     if (expression.result === 'success') {
         return [];