Skip to content

Commit

Permalink
Minor review followups
Browse files Browse the repository at this point in the history
  • Loading branch information
Lauren Budorick committed Apr 16, 2018
1 parent dc5decc commit caba650
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 29 deletions.
4 changes: 2 additions & 2 deletions build/generate-flow-typed-style-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
Expand Down
8 changes: 6 additions & 2 deletions build/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
};

Expand Down Expand Up @@ -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}`);
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/data/program_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/style-spec/expression/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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')]);
}

Expand All @@ -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')]);
}

Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/function/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/function/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
12 changes: 7 additions & 5 deletions src/style-spec/util/properties.js
Original file line number Diff line number Diff line change
@@ -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;
}
14 changes: 7 additions & 7 deletions src/style-spec/validate/validate_function.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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'));
}
}
Expand Down Expand Up @@ -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)];
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/validate/validate_property.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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` +
Expand Down

0 comments on commit caba650

Please sign in to comment.