Stricter validation of filters that mix old and new syntax

mapbox · Nov 13, 2017 · b81eb42 · b81eb42
1 parent 94b75b6
commit b81eb42
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 12 deletions.
diff --git a/src/style-spec/validate/validate_filter.js b/src/style-spec/validate/validate_filter.js
@@ -8,23 +8,28 @@ const extend = require('../util/extend');
 const {isExpressionFilter} = require('../feature_filter');
 
 module.exports = function validateFilter(options) {
+    if (isExpressionFilter(unbundle.deep(options.value))) {
+        return validateExpression(extend({}, options, {
+            expressionContext: 'filter',
+            valueSpec: { value: 'boolean' }
+        }));
+    } else {
+        return validateNonExpressionFilter(options);
+    }
+};
+
+function validateNonExpressionFilter(options) {
     const value = options.value;
     const key = options.key;
-    const styleSpec = options.styleSpec;
-    let type;
-
-    let errors = [];
 
     if (getType(value) !== 'array') {
         return [new ValidationError(key, value, 'array expected, %s found', getType(value))];
     }
 
-    if (isExpressionFilter(unbundle.deep(value))) {
-        return validateExpression(extend({}, options, {
-            expressionContext: 'filter',
-            valueSpec: { value: 'boolean' }
-        }));
-    }
+    const styleSpec = options.styleSpec;
+    let type;
+
+    let errors = [];
 
     if (value.length < 1) {
         return [new ValidationError(key, value, 'filter array must have at least 1 element')];
@@ -81,7 +86,7 @@ module.exports = function validateFilter(options) {
     case 'all':
     case 'none':
         for (let i = 1; i < value.length; i++) {
-            errors = errors.concat(validateFilter({
+            errors = errors.concat(validateNonExpressionFilter({
                 key: `${key}[${i}]`,
                 value: value[i],
                 style: options.style,
@@ -103,4 +108,4 @@ module.exports = function validateFilter(options) {
     }
 
     return errors;
-};
+}
diff --git a/test/unit/style-spec/fixture/filters.input.json b/test/unit/style-spec/fixture/filters.input.json
@@ -143,6 +143,13 @@
         "has",
         {"mapbox": true}
       ]
+    },
+    {
+      "id": "mixing old filters and expressions",
+      "type": "line",
+      "source": "source",
+      "source-layer": "source-layer",
+      "filter": ["all", [">=", "Constructi", 1930], [">=", ["zoom"], 10]]
     }
   ]
 }
diff --git a/test/unit/style-spec/fixture/filters.output.json b/test/unit/style-spec/fixture/filters.output.json
@@ -50,5 +50,9 @@
   {
     "message": "layers[13].filter[1]: Bare objects invalid. Use [\"literal\", {...}] instead.",
     "line": 142
+  },
+  {
+    "message": "layers[14].filter[2][1]: string expected, array found",
+    "line": 152
   }
 ]