Disallow feature-state expressions in Layout properties (#6600)

Also fix style-spec validation error messages and style-spec documentation.
mapbox · May 3, 2018 · 872c372 · 872c372
1 parent c9c4f61
commit 872c372
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 7 deletions.
diff --git a/docs/pages/style-spec.js b/docs/pages/style-spec.js
@@ -1130,9 +1130,10 @@ export default class extends React.Component {
                                 <a href="#expressions-get"><code>get</code></a>,
                                 <a href="#expressions-has"><code>has</code></a>,
                                 <a href="#expressions-id"><code>id</code></a>,
-                                <a href="#expressions-geometry-type"><code>geometry-type</code></a>, or
-                                <a href="#expressions-properties"><code>properties</code></a>. Data expressions allow a
-                                feature's properties to determine its appearance. They can be used to differentiate
+                                <a href="#expressions-geometry-type"><code>geometry-type</code></a>,
+                                <a href="#expressions-properties"><code>properties</code></a>, or
+                                <a href="#expressions-feature-state"><code>feature-state</code></a>. Data expressions allow a
+                                feature's properties or state to determine its appearance. They can be used to differentiate
                                 features within the same layer and to create data visualizations.</p>
 
                             <div className='col12 space-bottom'>
@@ -1159,7 +1160,9 @@ export default class extends React.Component {
                                 <a href="#layer-filter"><code>filter</code></a> property, and as values for most paint
                                 and layout properties. However, some paint and layout properties do not yet support data
                                 expressions. The level of support is indicated by the "data-driven styling" row of the
-                                "SDK Support" table for each property.</p>
+                                "SDK Support" table for each property. Data expressions with the
+                                <a href="#expressions-feature-state"><code>feature-state</code></a> operator are allowed
+                                only on paint properties.</p>
 
                             <h3>Camera expressions</h3>
                             <p>A <a id="camera-expression" className="anchor"></a><em>camera expression</em> is any

diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
@@ -219,7 +219,7 @@ export function createPropertyExpression(expression: mixed, propertySpec: StyleP
 
     const isFeatureConstant = isConstant.isFeatureConstant(parsed);
     if (!isFeatureConstant && !propertySpec['property-function']) {
-        return error([new ParsingError('', 'property expressions not supported')]);
+        return error([new ParsingError('', 'data expressions not supported')]);
     }
 
     const isZoomConstant = isConstant.isGlobalPropertyConstant(parsed, ['zoom']);

diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js
@@ -4,6 +4,7 @@ import ValidationError from '../error/validation_error';
 
 import { createExpression, createPropertyExpression } from '../expression';
 import { deepUnbundle } from '../util/unbundle_jsonlint';
+import { isStateConstant } from '../expression/is_constant';
 
 export default function validateExpression(options: any) {
     const expression = (options.expressionContext === 'property' ? createPropertyExpression : createExpression)(deepUnbundle(options.value), options.valueSpec);
@@ -18,5 +19,9 @@ export default function validateExpression(options: any) {
         return [new ValidationError(options.key, options.value, 'Invalid data expression for "text-font". Output values must be contained as literals within the expression.')];
     }
 
+    if (options.expressionContext === 'property' && options.propertyType === 'layout' &&
+        (!isStateConstant((expression.value: any)._styleExpression.expression))) {
+        return [new ValidationError(options.key, options.value, '"feature-state" data expressions are not supported with layout properties.')];
+    }
     return [];
 }
diff --git a/src/style-spec/validate/validate_property.js b/src/style-spec/validate/validate_property.js
@@ -57,6 +57,7 @@ export default function validateProperty(options, propertyType) {
         style: style,
         styleSpec: styleSpec,
         expressionContext: 'property',
+        propertyType: propertyType,
         propertyKey
     }));
 }
diff --git a/test/unit/style-spec/fixture/functions.input.json b/test/unit/style-spec/fixture/functions.input.json
@@ -959,6 +959,14 @@
       "paint": {
         "heatmap-color": { "stops": [[0, "red"], [1, "blue"]] }
       }
-    }
+    }, {
+        "id": "invalid expression - feature-state is not supported with layout properties",
+        "type": "line",
+        "source": "source",
+        "source-layer": "layer",
+        "layout": {
+          "line-join": [ "coalesce", ["feature-state", "myState"], "bevel"]
+        }
+      }
   ]
 }
diff --git a/test/unit/style-spec/fixture/functions.output.json b/test/unit/style-spec/fixture/functions.output.json
@@ -180,7 +180,7 @@
     "line": 925
   },
   {
-    "message": "layers[51].paint.fill-extrusion-opacity: property expressions not supported",
+    "message": "layers[51].paint.fill-extrusion-opacity: data expressions not supported",
     "line": 934
   },
   {
@@ -194,5 +194,9 @@
   {
     "message": "layers[54].paint.heatmap-color: zoom functions not supported",
     "line": 960
+  },
+  {                                                                                                                     
+    "message": "layers[55].layout.line-join: \"feature-state\" data expressions are not supported with layout properties.",
+    "line": 968
   }
 ]