Skip to content

Commit

Permalink
Disallow interpolation for variable-length numeric arrays (#5519)
Browse files Browse the repository at this point in the history
* Disallow interpolation for variable-length numeric arrays

* Add separate unit test for piecewise-constant check

This fix causes the 'curve' parsing validation to preempt
createExpression()'s check for piecewise-constant zoom function using
'step' interpolation, because `line-dasharray`'s style spec type,
`array<number>`, already isn't interpolatable.

Adding this unit test to make sure we always enforce step curves, even
for a theoretical style property whose base type is interpolatable.
  • Loading branch information
anandthakker authored Oct 25, 2017
1 parent 4c7d0d7 commit 3eced30
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/style-spec/expression/definitions/curve.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ class Curve implements Expression {
if (interpolation.name !== 'step' &&
outputType.kind !== 'number' &&
outputType.kind !== 'color' &&
!(outputType.kind === 'array' && outputType.itemType.kind === 'number')) {
!(
outputType.kind === 'array' &&
outputType.itemType.kind === 'number' &&
typeof outputType.N === 'number'
)
) {
return context.error(`Type ${toString(outputType)} is not interpolatable, and thus cannot be used as a ${interpolation.name} curve's output type.`);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"expression": [
"curve",
["exponential", 2],
["number", ["get", "x"]],
1,
["array", "number", ["get", "array"]],
3,
["array", "number", ["get", "array_two"]]
],
"inputs": [],
"expected": {
"compiled": {
"result": "error",
"errors": [
{
"key": "",
"error": "Type array<number> is not interpolatable, and thus cannot be used as a exponential curve's output type."
}
]
}
}
}
21 changes: 21 additions & 0 deletions test/unit/style-spec/expression.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

const test = require('mapbox-gl-js-test').test;
const {createExpression} = require('../../../src/style-spec/expression');

test('createExpression', (t) => {
test('require piecewise-constant zoom curves to use "step" interpolation', (t) => {
const expression = createExpression([
'curve', ['linear'], ['zoom'], 0, 0, 10, 10
], {
type: 'number',
function: 'piecewise-constant'
});
t.equal(expression.result, 'error');
t.equal(expression.errors.length, 1);
t.equal(expression.errors[0].message, 'interpolation type must be "step" for this property');
t.end();
});

t.end();
});
2 changes: 1 addition & 1 deletion test/unit/style-spec/fixture/functions.output.json
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
"line": 934
},
{
"message": "layers[52].paint.line-dasharray: interpolation type must be \"step\" for this property",
"message": "layers[52].paint.line-dasharray: Type array<number> is not interpolatable, and thus cannot be used as a linear curve's output type.",
"line": 943
}
]

0 comments on commit 3eced30

Please sign in to comment.