Correctly detect expressions and legacy filters for the new in expres…

…sion
mapbox · Mar 4, 2020 · bd2ddc7 · bd2ddc7
1 parent 3f90780
commit bd2ddc7
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 1 deletion.
diff --git a/src/style-spec/feature_filter/index.js b/src/style-spec/feature_filter/index.js
@@ -21,7 +21,8 @@ function isExpressionFilter(filter: any) {
         return filter.length >= 2 && filter[1] !== '$id' && filter[1] !== '$type';
 
     case 'in':
-        return filter.length >= 3 && Array.isArray(filter[2]);
+        return filter.length >= 3 && (typeof filter[1] !== 'string' || Array.isArray(filter[2]));
+
     case '!in':
     case '!has':
     case 'none':

diff --git a/test/unit/style-spec/feature_filter.test.js b/test/unit/style-spec/feature_filter.test.js
@@ -1,5 +1,6 @@
 import {test} from '../../util/test';
 import createFilter from '../../../src/style-spec/feature_filter';
+import {isExpressionFilter} from '../../../src/style-spec/feature_filter';
 import convertFilter from '../../../src/style-spec/feature_filter/convert';
 
 test('filter', t => {
@@ -71,6 +72,36 @@ test('filter', t => {
     t.end();
 });
 
+test('legacy filter detection', t => {
+    t.test('definitely legacy filters', t => {
+        // Expressions with more than two arguments.
+        t.notOk(isExpressionFilter(["in", "color", "red", "blue"]));
+
+        // Expressions where the second argument is not a string or array.
+        t.notOk(isExpressionFilter(["in", "value", 42]));
+        t.notOk(isExpressionFilter(["in", "value", true]));
+        t.end();
+    });
+
+    t.test('ambiguous value', t => {
+        // Should err on the side of reporting as a legacy filter. Style authors can force filters
+        // by using a literal expression as the first argument.
+        t.notOk(isExpressionFilter(["in", "color", "red"]));
+        t.end();
+    });
+
+    t.test('definitely expressions', t => {
+        t.ok(isExpressionFilter(["in", ["get", "color"], "reddish"]));
+        t.ok(isExpressionFilter(["in", ["get", "color"], ["red", "blue"]]));
+        t.ok(isExpressionFilter(["in", 42, 42]));
+        t.ok(isExpressionFilter(["in", true, true]));
+        t.ok(isExpressionFilter(["in", "red", ["get", "colors"]]));
+        t.end();
+    })
+
+    t.end();
+});
+
 test('convert legacy filters to expressions', t => {
     t.beforeEach(done => {
         t.stub(console, 'warn');