Skip to content

Commit

Permalink
Require a numeric domain for exponential and interval functions
Browse files Browse the repository at this point in the history
Fixes #587
  • Loading branch information
jfirebaugh committed Nov 30, 2016
1 parent b7bd8d1 commit e2e7c95
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 18 deletions.
4 changes: 2 additions & 2 deletions docs/_generate/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,9 @@ <h3 class='space-bottom1'><a href='#types-function' title='link to function'>Fun
<dt><var>identity</var></dt>
<dd>functions return their input as their output.</dd>
<dt><var>exponential</var></dt>
<dd>functions generate an output by interpolating between stops just less than and just greater than the function input. This is the default for numeric domain, <span class='icon smooth-ramp quiet micro space-right indivne' title='continuous'></span> range functions.</dd>
<dd>functions generate an output by interpolating between stops just less than and just greater than the function input. The domain must be numeric. This is the default for numeric domain, <span class='icon smooth-ramp quiet micro space-right indivne' title='continuous'></span> range functions.</dd>
<dt><var>interval</var></dt>
<dd>functions return the output value of the stop just less than the function input. This is the default for numeric domain, <span class='icon step-ramp quiet micro space-right indivne' title='discrete'></span> range functions.</dd>
<dd>functions return the output value of the stop just less than the function input. The domain must be numeric. This is the default for numeric domain, <span class='icon step-ramp quiet micro space-right indivne' title='discrete'></span> range functions.</dd>
<dt><var>categorical</var></dt>
<dd>functions return the output value of the stop equal to the function input. This is the default for string domain functions.</dd>
</dl>
Expand Down
43 changes: 29 additions & 14 deletions lib/validate/validate_function.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ var unbundle = require('../util/unbundle_jsonlint');

module.exports = function validateFunction(options) {
var functionValueSpec = options.valueSpec;
var functionValue = options.value;
var functionType = unbundle(options.value.type);
var stopKeyType;

var isPropertyFunction = options.value.property !== undefined;
var isZoomFunction = options.value.property === undefined;
var isPropertyFunction = options.value.property !== undefined;
var isZoomAndPropertyFunction =
getType(options.value.stops) === 'array' &&
getType(options.value.stops[0]) === 'array' &&
getType(options.value.stops[0][0]) === 'object';

var errors = validateObject({
key: options.key,
Expand All @@ -25,7 +29,7 @@ module.exports = function validateFunction(options) {
objectElementValidators: { stops: validateFunctionStops }
});

if (unbundle(options.value.type) !== 'identity' && !options.value.stops) {
if (functionType !== 'identity' && !options.value.stops) {
errors.push(new ValidationError(options.key, options.value, 'missing required property "stops"'));
}

Expand All @@ -40,7 +44,7 @@ module.exports = function validateFunction(options) {
return errors;

function validateFunctionStops(options) {
if (unbundle(functionValue.type) === 'identity') {
if (functionType === 'identity') {
return [new ValidationError(options.key, options.value, 'identity function may not have a "stops" property')];
}

Expand Down Expand Up @@ -76,13 +80,10 @@ module.exports = function validateFunction(options) {
return [new ValidationError(key, value, 'array length %d expected, length %d found', 2, value.length)];
}

var type = getType(value[0]);
if (!stopKeyType) stopKeyType = type;
if (type !== stopKeyType) {
return [new ValidationError(key, value, '%s stop key type must match previous stop key type %s', type, stopKeyType)];
}

if (type === 'object') {
if (isZoomAndPropertyFunction) {
if (getType(value[0]) !== 'object') {
return [new ValidationError(key, value, 'object expected, %s found', getType(value[0]))];
}
if (value[0].zoom === undefined) {
return [new ValidationError(key, value, 'object stop key must have zoom')];
}
Expand Down Expand Up @@ -131,12 +132,26 @@ module.exports = function validateFunction(options) {
}

function validateValue(options) {
var errors = [];
var type = getType(options.value);

if (!stopKeyType) {
stopKeyType = type;
if (!functionType && type === 'string') {
functionType = 'categorical';
}
} else if (type !== stopKeyType) {
return [new ValidationError(options.key, options.value, '%s stop domain type must match previous stop domain type %s', type, stopKeyType)];
}

if (type !== 'number' && type !== 'string' && type !== 'array') {
errors.push(new ValidationError(options.key, options.value, 'property value must be a number, string or array'));
return [new ValidationError(options.key, options.value, 'property value must be a number, string or array')];
}
return errors;

if (type !== 'number' && functionType !== 'categorical') {
return [new ValidationError(options.key, options.value, 'number expected, %s found', type)];
}

return [];
}

};
108 changes: 107 additions & 1 deletion test/fixture/functions.input.json
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@
[
{
"zoom": "1",
"value": "asdf"
"value": 0
},
true
]
Expand Down Expand Up @@ -384,6 +384,112 @@
"stops": []
}
}
},
{
"id": "valid implicitly categorical property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"stops": [
[
"good",
"green"
]
]
}
}
},
{
"id": "valid implicitly categorical zoom-and-property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"stops": [
[
{"zoom": 0, "value": "good"},
"green"
]
]
}
}
},
{
"id": "invalid domain type for exponential property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"type": "exponential",
"stops": [
[
"good",
"green"
]
]
}
}
},
{
"id": "invalid domain type for exponential zoom-and-property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"type": "exponential",
"stops": [
[
{"zoom": 0, "value": "good"},
"green"
]
]
}
}
},
{
"id": "invalid domain type for interval property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"type": "interval",
"stops": [
[
"good",
"green"
]
]
}
}
},
{
"id": "invalid domain type for interval zoom-and-property function",
"type": "fill",
"source": "source",
"source-layer": "layer",
"paint": {
"fill-color": {
"property": "mapbox",
"type": "interval",
"stops": [
[
{"zoom": 0, "value": "good"},
"green"
]
]
}
}
}
]
}
18 changes: 17 additions & 1 deletion test/fixture/functions.output.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"line": 204
},
{
"message": "layers[12].paint.fill-antialias.stops[1]: number stop key type must match previous stop key type object",
"message": "layers[12].paint.fill-antialias.stops[1]: object expected, number found",
"line": 227
},
{
Expand Down Expand Up @@ -82,5 +82,21 @@
{
"message": "layers[21].paint.background-color.stops: identity function may not have a \"stops\" property",
"line": 384
},
{
"message": "layers[24].paint.fill-color.stops[0][0]: number expected, string found",
"line": 433
},
{
"message": "layers[25].paint.fill-color.stops[0][0].value: number expected, string found",
"line": 451
},
{
"message": "layers[26].paint.fill-color.stops[0][0]: number expected, string found",
"line": 469
},
{
"message": "layers[27].paint.fill-color.stops[0][0].value: number expected, string found",
"line": 487
}
]

0 comments on commit e2e7c95

Please sign in to comment.