From d37c94cfa592b33bc3a5baed6116b0360aa586f4 Mon Sep 17 00:00:00 2001 From: Lauren Budorick Date: Mon, 16 Apr 2018 14:17:53 -0700 Subject: [PATCH] Minor review followups --- build/generate-flow-typed-style-spec.js | 4 ++-- build/generate-style-code.js | 8 ++++++-- src/data/program_configuration.js | 4 ++-- src/style-spec/expression/index.js | 8 ++++---- src/style-spec/function/convert.js | 5 ++--- src/style-spec/function/index.js | 4 ++-- src/style-spec/util/properties.js | 12 +++++++----- src/style-spec/validate/validate_function.js | 14 +++++++------- src/style-spec/validate/validate_property.js | 4 ++-- 9 files changed, 34 insertions(+), 29 deletions(-) diff --git a/build/generate-flow-typed-style-spec.js b/build/generate-flow-typed-style-spec.js index 2386ad95ff0..d7a4070a801 100644 --- a/build/generate-flow-typed-style-spec.js +++ b/build/generate-flow-typed-style-spec.js @@ -41,9 +41,9 @@ function flowType(property) { } })(); - if (properties.isPropertyExpression(property)) { + if (properties.supportsPropertyExpression(property)) { return `DataDrivenPropertyValueSpecification<${baseType}>`; - } else if (properties.isZoomExpression(property)) { + } else if (properties.supportsZoomExpression(property)) { return `PropertyValueSpecification<${baseType}>`; } else if (property.expression) { return `ExpressionSpecification`; diff --git a/build/generate-style-code.js b/build/generate-style-code.js index 5ed24ddc429..8fa49c1bb21 100644 --- a/build/generate-style-code.js +++ b/build/generate-style-code.js @@ -43,8 +43,10 @@ global.propertyType = function (property) { case 'color-ramp': return `ColorRampProperty`; case 'data-constant': - default: + case 'constant': return `DataConstantProperty<${flowType(property)}>`; + default: + throw new Error(`unknown property-type "${property['property-type']}" for ${property.name}`); } }; @@ -97,8 +99,10 @@ global.propertyValue = function (property, type) { case 'color-ramp': return `new ColorRampProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`; case 'data-constant': - default: + case 'constant': return `new DataConstantProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`; + default: + throw new Error(`unknown property-type "${property['property-type']}" for ${property.name}`); } }; diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index 624896bdcbf..399cbeff2ca 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -2,7 +2,7 @@ import { packUint8ToFloat } from '../shaders/encode_attribute'; import Color from '../style-spec/util/color'; -import { isPropertyExpression } from '../style-spec/util/properties'; +import { supportsPropertyExpression } from '../style-spec/util/properties'; import { register } from '../util/web_worker_transfer'; import { PossiblyEvaluatedPropertyValue } from '../style/properties'; import { StructArrayLayout1f4, StructArrayLayout2f8, StructArrayLayout4f16 } from './array_types'; @@ -292,7 +292,7 @@ export default class ProgramConfiguration { for (const property in layer.paint._values) { if (!filterProperties(property)) continue; const value = layer.paint.get(property); - if (!(value instanceof PossiblyEvaluatedPropertyValue) || !isPropertyExpression(value.property.specification)) { + if (!(value instanceof PossiblyEvaluatedPropertyValue) || !supportsPropertyExpression(value.property.specification)) { continue; } const name = paintAttributeName(property, layer.type); diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index c2e293708fb..97a35a5683a 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -15,7 +15,7 @@ import definitions from './definitions'; import * as isConstant from './is_constant'; import RuntimeError from './runtime_error'; import { success, error } from '../util/result'; -import { isPropertyExpression, isZoomExpression, isInterpolated } from '../util/properties'; +import { supportsPropertyExpression, supportsZoomExpression, supportsInterpolation } from '../util/properties'; import type {Type} from './types'; import type {Value} from './values'; @@ -208,12 +208,12 @@ export function createPropertyExpression(expression: mixed, propertySpec: StyleP const parsed = expression.value.expression; const isFeatureConstant = isConstant.isFeatureConstant(parsed); - if (!isFeatureConstant && !isPropertyExpression(propertySpec)) { + if (!isFeatureConstant && !supportsPropertyExpression(propertySpec)) { return error([new ParsingError('', 'property expressions not supported')]); } const isZoomConstant = isConstant.isGlobalPropertyConstant(parsed, ['zoom']); - if (!isZoomConstant && !isZoomExpression(propertySpec)) { + if (!isZoomConstant && !supportsZoomExpression(propertySpec)) { return error([new ParsingError('', 'zoom expressions not supported')]); } @@ -222,7 +222,7 @@ export function createPropertyExpression(expression: mixed, propertySpec: StyleP return error([new ParsingError('', '"zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.')]); } else if (zoomCurve instanceof ParsingError) { return error([zoomCurve]); - } else if (zoomCurve instanceof Interpolate && !isInterpolated(propertySpec)) { + } else if (zoomCurve instanceof Interpolate && !supportsInterpolation(propertySpec)) { return error([new ParsingError('', '"interpolate" expressions cannot be used with this property')]); } diff --git a/src/style-spec/function/convert.js b/src/style-spec/function/convert.js index 15d4e7212bd..1fd944418f4 100644 --- a/src/style-spec/function/convert.js +++ b/src/style-spec/function/convert.js @@ -236,10 +236,9 @@ function appendStopPair(curve, input, output, isStep) { function getFunctionType(parameters, propertySpec) { if (parameters.type) { return parameters.type; - } else if (propertySpec.expression) { - return propertySpec.expression.interpolated ? 'exponential' : 'interval'; } else { - return 'exponential'; + assert(propertySpec.expression); + return (propertySpec.expression: any).interpolated ? 'exponential' : 'interval'; } } diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index e4cc24ce254..f6a3f884c82 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -5,7 +5,7 @@ import extend from '../util/extend'; import getType from '../util/get_type'; import * as interpolate from '../util/interpolate'; import Interpolate from '../expression/definitions/interpolate'; -import { isInterpolated } from '../util/properties'; +import { supportsInterpolation } from '../util/properties'; export function isFunction(value) { return typeof value === 'object' && value !== null && !Array.isArray(value); @@ -20,7 +20,7 @@ export function createFunction(parameters, propertySpec) { const zoomAndFeatureDependent = parameters.stops && typeof parameters.stops[0][0] === 'object'; const featureDependent = zoomAndFeatureDependent || parameters.property !== undefined; const zoomDependent = zoomAndFeatureDependent || !featureDependent; - const type = parameters.type || (isInterpolated(propertySpec) ? 'exponential' : 'interval'); + const type = parameters.type || (supportsInterpolation(propertySpec) ? 'exponential' : 'interval'); if (isColor) { parameters = extend({}, parameters); diff --git a/src/style-spec/util/properties.js b/src/style-spec/util/properties.js index 5db71d85624..dbb3ef91442 100644 --- a/src/style-spec/util/properties.js +++ b/src/style-spec/util/properties.js @@ -1,13 +1,15 @@ // @flow -export function isPropertyExpression(spec: Object): boolean { +import type {StylePropertySpecification} from '../style-spec'; + +export function supportsPropertyExpression(spec: StylePropertySpecification): boolean { return spec['property-type'] === 'data-driven' || spec['property-type'] === 'cross-faded-data-driven'; } -export function isZoomExpression(spec: Object): boolean { - return spec.expression && spec.expression.parameters.indexOf('zoom') > -1; +export function supportsZoomExpression(spec: StylePropertySpecification): boolean { + return !!spec.expression && spec.expression.parameters.indexOf('zoom') > -1; } -export function isInterpolated(spec: Object): boolean { - return spec.expression && spec.expression.interpolated; +export function supportsInterpolation(spec: StylePropertySpecification): boolean { + return !!spec.expression && spec.expression.interpolated; } diff --git a/src/style-spec/validate/validate_function.js b/src/style-spec/validate/validate_function.js index 9fe32dc087b..f34303a06dc 100644 --- a/src/style-spec/validate/validate_function.js +++ b/src/style-spec/validate/validate_function.js @@ -7,9 +7,9 @@ import validateArray from './validate_array'; import validateNumber from './validate_number'; import { unbundle } from '../util/unbundle_jsonlint'; import { - isPropertyExpression as acceptsPropertyExpression, - isZoomExpression as acceptsZoomExpression, - isInterpolated as acceptsInterpolated + supportsPropertyExpression, + supportsZoomExpression, + supportsInterpolation } from '../util/properties'; export default function validateFunction(options) { @@ -47,14 +47,14 @@ export default function validateFunction(options) { errors.push(new ValidationError(options.key, options.value, 'missing required property "stops"')); } - if (functionType === 'exponential' && options.valueSpec.expression && !acceptsInterpolated(options.valueSpec)) { + if (functionType === 'exponential' && options.valueSpec.expression && !supportsInterpolation(options.valueSpec)) { errors.push(new ValidationError(options.key, options.value, 'exponential functions not supported')); } if (options.styleSpec.$version >= 8) { - if (isPropertyFunction && !acceptsPropertyExpression(options.valueSpec)) { + if (isPropertyFunction && !supportsPropertyExpression(options.valueSpec)) { errors.push(new ValidationError(options.key, options.value, 'property functions not supported')); - } else if (isZoomFunction && !acceptsZoomExpression(options.valueSpec)) { + } else if (isZoomFunction && !supportsZoomExpression(options.valueSpec)) { errors.push(new ValidationError(options.key, options.value, 'zoom functions not supported')); } } @@ -165,7 +165,7 @@ export default function validateFunction(options) { if (type !== 'number' && functionType !== 'categorical') { let message = `number expected, ${type} found`; - if (acceptsPropertyExpression(functionValueSpec) && functionType === undefined) { + if (supportsPropertyExpression(functionValueSpec) && functionType === undefined) { message += '\nIf you intended to use a categorical function, specify `"type": "categorical"`.'; } return [new ValidationError(options.key, reportValue, message)]; diff --git a/src/style-spec/validate/validate_property.js b/src/style-spec/validate/validate_property.js index a23a96da0b1..2b9349d8e30 100644 --- a/src/style-spec/validate/validate_property.js +++ b/src/style-spec/validate/validate_property.js @@ -4,7 +4,7 @@ import ValidationError from '../error/validation_error'; import getType from '../util/get_type'; import { isFunction } from '../function'; import { unbundle, deepUnbundle } from '../util/unbundle_jsonlint'; -import { isPropertyExpression } from '../util/properties'; +import { supportsPropertyExpression } from '../util/properties'; export default function validateProperty(options, propertyType) { const key = options.key; @@ -33,7 +33,7 @@ export default function validateProperty(options, propertyType) { } let tokenMatch; - if (getType(value) === 'string' && isPropertyExpression(valueSpec) && !valueSpec.tokens && (tokenMatch = /^{([^}]+)}$/.exec(value))) { + if (getType(value) === 'string' && supportsPropertyExpression(valueSpec) && !valueSpec.tokens && (tokenMatch = /^{([^}]+)}$/.exec(value))) { return [new ValidationError( key, value, `"${propertyKey}" does not support interpolation syntax\n` +