From b81eb42671cd24ad1bea42cc059db4e7aaf3c906 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Mon, 13 Nov 2017 10:49:55 -0800
Subject: [PATCH] Stricter validation of filters that mix old and new syntax

---
 src/style-spec/validate/validate_filter.js    | 29 +++++++++++--------
 .../style-spec/fixture/filters.input.json     |  7 +++++
 .../style-spec/fixture/filters.output.json    |  4 +++
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/style-spec/validate/validate_filter.js b/src/style-spec/validate/validate_filter.js
index ddc4a5dc6cf..4ad4ddce6ac 100644
--- 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
index 460f4301239..6e9dfe43d5f 100644
--- 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
index 54968d1b120..ed3900a35a7 100644
--- 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
   }
 ]
\ No newline at end of file