Fix Filter.AnyExpression and Filter.AllExpression tests to correctly use expression syntax, and port Fiter.EqualsNull test to match gl-jsupstream/expression-filters-2

2 files changed, 21 insertions, 14 deletions

diff --git a/src/mbgl/style/expression/compound_expression.cpp b/src/mbgl/style/expression/compound_expression.cpp
index 689f8cdf34..3bd8a836df 100644
--- a/src/mbgl/style/expression/compound_expression.cpp
+++ b/src/mbgl/style/expression/compound_expression.cpp
@@ -224,10 +224,10 @@ Value featureIdAsExpressionValue(EvaluationContext params) {
     });
 };
 
-Value featurePropertyAsExpressionValue(EvaluationContext params, const std::string& key) {
+optional<Value> featurePropertyAsExpressionValue(EvaluationContext params, const std::string& key) {
     assert(params.feature);
     auto property = params.feature->getValue(key);
-    return property ? toExpressionValue(*property) : Null;
+    return property ? toExpressionValue(*property) : optional<Value>();
 };
 
 optional<std::string> featureTypeAsString(FeatureType type) {
@@ -507,7 +507,8 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
 
     // Legacy Filters
     define("filter-==", [](const EvaluationContext& params, const std::string& key, const Value &lhs) -> Result<bool> {
-        return lhs == featurePropertyAsExpressionValue(params, key);
+        const auto rhs = featurePropertyAsExpressionValue(params, key);
+        return rhs ? lhs == *rhs : false;
     });
 
     define("filter-id-==", [](const EvaluationContext& params, const Value &lhs) -> Result<bool> {
@@ -624,7 +625,7 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
         if (varargs.size() < 2) return false;
         assert(varargs[0].is<std::string>());
         auto value = featurePropertyAsExpressionValue(params, varargs[0].get<std::string>());
-        return std::find(varargs.begin() + 1, varargs.end(), value) != varargs.end();
+        return value ? std::find(varargs.begin() + 1, varargs.end(), *value) != varargs.end() : false;
     });
 
     return definitions;
diff --git a/test/style/filter.test.cpp b/test/style/filter.test.cpp
index e770f707c8..c59a73eab1 100644
--- a/test/style/filter.test.cpp
+++ b/test/style/filter.test.cpp
@@ -34,9 +34,16 @@ void invalidFilter(const char * json) {
     EXPECT_NE(error.message, "");
 }
 
-TEST(Filter, EqualsInvalid) {
-    invalidFilter("[\"==\", \"foo\", null]");
-    invalidFilter("[\"==\", \"foo\", [1, 2]]");
+TEST(Filter, EqualsNull) {
+    auto f = R"(["==", "foo", null])";
+    ASSERT_TRUE(filter(f, {{ "foo", mapbox::geometry::null_value }}));
+
+    ASSERT_FALSE(filter(f, {{ "foo", int64_t(0) }}));
+    ASSERT_FALSE(filter(f, {{ "foo", int64_t(1) }}));
+    ASSERT_FALSE(filter(f, {{ "foo", std::string("0") }}));
+    ASSERT_FALSE(filter(f, {{ "foo", true }}));
+    ASSERT_FALSE(filter(f, {{ "foo", false }}));
+    ASSERT_FALSE(filter(f, {{ }}));
 }
 TEST(Filter, EqualsString) {
     auto f = R"(["==", "foo", "bar"])";
@@ -95,9 +102,9 @@ TEST(Filter, Any) {
 
 TEST(Filter, AnyExpression) {
     ASSERT_FALSE(filter("[\"any\"]"));
-    ASSERT_TRUE(filter("[\"any\", true]"));
-    ASSERT_TRUE(filter("[\"any\",true, false]"));
-    ASSERT_TRUE(filter("[\"any\", true, true]"));
+    ASSERT_TRUE(filter("[\"any\", [\"==\", [\"get\", \"foo\"], 1]]", {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_FALSE(filter("[\"any\", [\"==\", [\"get\", \"foo\"], 0]]", {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_TRUE(filter("[\"any\", [\"==\", [\"get\", \"foo\"], 0], [\"==\", [\"get\", \"foo\"], 1]]", {{ std::string("foo"), int64_t(1) }}));
 }
 
 TEST(Filter, All) {
@@ -108,10 +115,9 @@ TEST(Filter, All) {
 }
 
 TEST(Filter, AllExpression) {
-    ASSERT_TRUE(filter("[\"all\"]"));
-    ASSERT_TRUE(filter("[\"all\", true]"));
-    ASSERT_FALSE(filter("[\"all\",true, false]"));
-    ASSERT_TRUE(filter("[\"any\", true, true]"));
+    ASSERT_TRUE(filter("[\"all\", [\"==\", [\"get\", \"foo\"], 1]]", {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_FALSE(filter("[\"all\", [\"==\", [\"get\", \"foo\"], 0]]", {{ std::string("foo"), int64_t(1) }}));
+    ASSERT_FALSE(filter("[\"all\", [\"==\", [\"get\", \"foo\"], 0], [\"==\", [\"get\", \"foo\"], 1]]", {{ std::string("foo"), int64_t(1) }}));
 }
 
 TEST(Filter, None) {