[core] Improve typing for !=, == expressions

cmake/core-files.cmake

@@ -410,6 +410,7 @@ set(MBGL_CORE_FILES
     include/mbgl/style/expression/coalesce.hpp
     include/mbgl/style/expression/coercion.hpp
     include/mbgl/style/expression/compound_expression.hpp
+    include/mbgl/style/expression/equals.hpp
     include/mbgl/style/expression/expression.hpp
     include/mbgl/style/expression/find_zoom_curve.hpp
     include/mbgl/style/expression/get_covering_stops.hpp
@@ -432,6 +433,7 @@ set(MBGL_CORE_FILES
     src/mbgl/style/expression/coalesce.cpp
     src/mbgl/style/expression/coercion.cpp
     src/mbgl/style/expression/compound_expression.cpp
+    src/mbgl/style/expression/equals.cpp
     src/mbgl/style/expression/find_zoom_curve.cpp
     src/mbgl/style/expression/get_covering_stops.cpp
     src/mbgl/style/expression/interpolate.cpp

include/mbgl/style/expression/equals.hpp

@@ -0,0 +1,31 @@
+#pragma once
+
+#include <mbgl/style/expression/expression.hpp>
+#include <mbgl/style/expression/parsing_context.hpp>
+#include <mbgl/style/conversion.hpp>
+
+#include <memory>
+
+namespace mbgl {
+namespace style {
+namespace expression {
+
+class Equals : public Expression {
+public:
+    Equals(std::unique_ptr<Expression> lhs, std::unique_ptr<Expression> rhs, bool negate);
+
+    static ParseResult parse(const mbgl::style::conversion::Convertible&, ParsingContext&);
+
+    void eachChild(const std::function<void(const Expression&)>& visit) const override;
+    bool operator==(const Expression&) const override;
+    EvaluationResult evaluate(const EvaluationContext&) const override;
+
+private:
+    std::unique_ptr<Expression> lhs;
+    std::unique_ptr<Expression> rhs;
+    bool negate;
+};
+
+} // namespace expression
+} // namespace style
+} // namespace mbgl

platform/node/test/ignores.json

@@ -3,19 +3,6 @@
   "expression-tests/array/implicit-2": "https://github.com/mapbox/mapbox-gl-native/issues/10533",
   "expression-tests/array/implicit-3": "https://github.com/mapbox/mapbox-gl-native/issues/10533",
   "expression-tests/coalesce/inference": "https://github.com/mapbox/mapbox-gl-native/issues/10588",
-  "expression-tests/equal/array": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/color": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/null-lhs": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/null-rhs": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/object": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/not_equal/mismatch": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/not_equal/number": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/not_equal/string": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
-  "expression-tests/not_equal/value": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
   "expression-tests/interpolate/linear-color": "https://github.com/mapbox/mapbox-gl-native/issues/10604",
   "expression-tests/to-rgba/alpha": "https://github.com/mapbox/mapbox-gl-native/issues/10604",
   "expression-tests/to-rgba/basic": "https://github.com/mapbox/mapbox-gl-native/issues/10604",
@@ -74,7 +61,6 @@
   "render-tests/regressions/mapbox-gl-js#5370": "skip - https://github.com/mapbox/mapbox-gl-native/pull/9439",
   "render-tests/regressions/mapbox-gl-js#5599": "https://github.com/mapbox/mapbox-gl-native/issues/10399",
   "render-tests/regressions/mapbox-gl-js#5740": "https://github.com/mapbox/mapbox-gl-native/issues/10619",
-  "render-tests/regressions/mapbox-gl-js#5947": "https://github.com/mapbox/mapbox-gl-native/issues/10678",
   "render-tests/regressions/mapbox-gl-native#7357": "https://github.com/mapbox/mapbox-gl-native/issues/7357",
   "render-tests/runtime-styling/image-add-sdf": "https://github.com/mapbox/mapbox-gl-native/issues/9847",
   "render-tests/runtime-styling/paint-property-fill-flat-to-extrude": "https://github.com/mapbox/mapbox-gl-native/issues/6745",

src/mbgl/style/expression/compound_expression.cpp

@@ -175,12 +175,6 @@ struct Signature<Lambda, std::enable_if_t<std::is_class<Lambda>::value>>
 
 using Definition = CompoundExpressionRegistry::Definition;
 
-template <typename T>
-Result<bool> equal(const T& lhs, const T& rhs) { return lhs == rhs; }
-
-template <typename T>
-Result<bool> notEqual(const T& lhs, const T& rhs) { return lhs != rhs; }
-
 template <typename Fn>
 static std::unique_ptr<detail::SignatureBase> makeSignature(Fn evaluateFunction) {
     return std::make_unique<detail::Signature<Fn>>(evaluateFunction);
@@ -376,16 +370,6 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
         return result;
     });
 
-    define("==", equal<double>);
-    define("==", equal<const std::string&>);
-    define("==", equal<bool>);
-    define("==", equal<NullValue>);
-
-    define("!=", notEqual<double>);
-    define("!=", notEqual<const std::string&>);
-    define("!=", notEqual<bool>);
-    define("!=", notEqual<NullValue>);
-
     define(">", [](double lhs, double rhs) -> Result<bool> { return lhs > rhs; });
     define(">", [](const std::string& lhs, const std::string& rhs) -> Result<bool> { return lhs > rhs; });
     define(">=", [](double lhs, double rhs) -> Result<bool> { return lhs >= rhs; });

src/mbgl/style/expression/equals.cpp

@@ -0,0 +1,83 @@
+#include <mbgl/style/expression/equals.hpp>
+
+namespace mbgl {
+namespace style {
+namespace expression {
+
+Equals::Equals(std::unique_ptr<Expression> lhs_, std::unique_ptr<Expression> rhs_, bool negate_)
+    : Expression(type::Boolean),
+      lhs(std::move(lhs_)),
+      rhs(std::move(rhs_)),
+      negate(negate_) {
+}
+
+EvaluationResult Equals::evaluate(const EvaluationContext& params) const {
+    EvaluationResult lhsResult = lhs->evaluate(params);
+    if (!lhsResult) return lhsResult;
+
+    EvaluationResult rhsResult = rhs->evaluate(params);
+    if (!rhsResult) return lhsResult;
+
+    bool result = *lhsResult == *rhsResult;
+    if (negate) {
+        result = !result;
+    }
+    return result;
+}
+
+void Equals::eachChild(const std::function<void(const Expression&)>& visit) const {
+    visit(*lhs);
+    visit(*rhs);
+}
+
+bool Equals::operator==(const Expression& e) const {
+    if (auto eq = dynamic_cast<const Equals*>(&e)) {
+        return eq->negate == negate && *eq->lhs == *lhs && *eq->rhs == *rhs;
+    }
+    return false;
+}
+
+static bool isComparableType(const type::Type& type) {
+    return type == type::String ||
+        type == type::Number ||
+        type == type::Boolean ||
+        type == type::Null;
+}
+
+using namespace mbgl::style::conversion;
+ParseResult Equals::parse(const Convertible& value, ParsingContext& ctx) {
+    std::size_t length = arrayLength(value);
+
+    if (length != 3) {
+        ctx.error("Expected two arguments.");
+        return ParseResult();
+    }
+
+    bool negate = toString(arrayMember(value, 0)) == std::string("!=");
+
+    ParseResult lhs = ctx.parse(arrayMember(value, 1), 1, {type::Value});
+    if (!lhs) return ParseResult();
+
+    ParseResult rhs = ctx.parse(arrayMember(value, 2), 2, {type::Value});
+    if (!rhs) return ParseResult();
+
+    type::Type lhsType = (*lhs)->getType();
+    type::Type rhsType = (*rhs)->getType();
+
+    if (!isComparableType(lhsType) && !isComparableType(rhsType)) {
+        ctx.error("Expected at least one argument to be a string, number, boolean, or null, but found (" +
+            toString(lhsType) + ", " + toString(rhsType) + ") instead.");
+        return ParseResult();
+    }
+
+    if (lhsType != rhsType && lhsType != type::Value && rhsType != type::Value) {
+        ctx.error("Cannot compare " + toString(lhsType) + " and " + toString(rhsType) + ".");
+        return ParseResult();
+    }
+
+    return ParseResult(std::make_unique<Equals>(std::move(*lhs), std::move(*rhs), negate));
+}
+
+} // namespace expression
+} // namespace style
+} // namespace mbgl

src/mbgl/style/expression/parsing_context.cpp

@@ -13,6 +13,7 @@
 #include <mbgl/style/expression/coalesce.hpp>
 #include <mbgl/style/expression/coercion.hpp>
 #include <mbgl/style/expression/compound_expression.hpp>
+#include <mbgl/style/expression/equals.hpp>
 #include <mbgl/style/expression/interpolate.hpp>
 #include <mbgl/style/expression/let.hpp>
 #include <mbgl/style/expression/literal.hpp>
@@ -75,6 +76,8 @@ ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_,
 
 const ExpressionRegistry& getExpressionRegistry() {
     static ExpressionRegistry registry {{
+        {"==", Equals::parse},
+        {"!=", Equals::parse},
         {"all", All::parse},
         {"any", Any::parse},
         {"array", ArrayAssertion::parse},