Fix review comments.

CMakeLists.txt

@@ -565,6 +565,7 @@ add_library(
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/get_covering_stops.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/image_expression.cpp
+    ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/in.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/interpolate.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_constant.cpp
     ${PROJECT_SOURCE_DIR}/src/mbgl/style/expression/is_expression.cpp

include/mbgl/style/expression/in.hpp

@@ -8,7 +8,7 @@ namespace mbgl {
 namespace style {
 namespace expression {
 
-class In : public Expression {
+class In final : public Expression {
 public:
     In(std::unique_ptr<Expression> needle_, std::unique_ptr<Expression> haystack_)
         : Expression(Kind::In, type::Boolean), needle(std::move(needle_)), haystack(std::move(haystack_)) {}
@@ -26,16 +26,13 @@ class In : public Expression {
         return false;
     }
 
-    std::vector<optional<Value>> possibleOutputs() const override { return {nullopt}; }
+    std::vector<optional<Value>> possibleOutputs() const override { return {{true}, {false}}; }
 
     std::string getOperator() const override { return "in"; }
 
 private:
     std::unique_ptr<Expression> needle;
     std::unique_ptr<Expression> haystack;
-    static bool isComparableType(type::Type type);
-    static bool isComparableRuntimeValue(type::Type type);
-    static bool isSearchableRuntimeValue(type::Type type);
 };
 
 } // namespace expression

src/mbgl/style/expression/in.cpp

@@ -8,15 +8,30 @@ namespace mbgl {
 namespace style {
 namespace expression {
 
+namespace {
+bool isComparableType(type::Type type) {
+    return type == type::Boolean || type == type::String || type == type::Number || type == type::Null ||
+        type == type::Value;
+}
+
+bool isComparableRuntimeValue(type::Type type) {
+    return type == type::Boolean || type == type::String || type == type::Number || type == type::Null;
+}
+
+bool isSearchableRuntimeValue(type::Type type) {
+    return type == type::String || type.is<type::Array>() || type == type::Null;
+}
+}
+
 EvaluationResult In::evaluate(const EvaluationContext& params) const {
     const EvaluationResult evaluatedNeedle = needle->evaluate(params);
-    const EvaluationResult evaluatedHeystack = haystack->evaluate(params);
-
     if (!evaluatedNeedle) {
         return evaluatedNeedle.error();
     }
-    if (!evaluatedHeystack) {
-        return evaluatedHeystack.error();
+
+    const EvaluationResult evaluatedHaystack = haystack->evaluate(params);
+    if (!evaluatedHaystack) {
+        return evaluatedHaystack.error();
     }
 
     type::Type evaluatedNeedleType = typeOf(*evaluatedNeedle);
@@ -25,18 +40,18 @@ EvaluationResult In::evaluate(const EvaluationContext& params) const {
                                toString(evaluatedNeedleType) + " instead."};
     }
 
-    type::Type evaluatedHeystackType = typeOf(*evaluatedHeystack);
-    if (!isSearchableRuntimeValue(evaluatedHeystackType)) {
+    type::Type evaluatedHaystackType = typeOf(*evaluatedHaystack);
+    if (!isSearchableRuntimeValue(evaluatedHaystackType)) {
         return EvaluationError{"Expected second argument to be of type array or string, but found " +
-                               toString(evaluatedHeystackType) + " instead."};
+                               toString(evaluatedHaystackType) + " instead."};
     }
 
-    if (evaluatedNeedleType == type::Null || evaluatedHeystackType == type::Null) {
+    if (evaluatedNeedleType == type::Null || evaluatedHaystackType == type::Null) {
         return EvaluationResult(false);
     }
 
-    if (evaluatedHeystackType == type::String) {
-        const auto heystackString = evaluatedHeystack->get<std::string>();
+    if (evaluatedHaystackType == type::String) {
+        const auto haystackString = evaluatedHaystack->get<std::string>();
         std::string needleValue = "";
         if (evaluatedNeedleType == type::Boolean) {
             needleValue = evaluatedNeedle->get<bool>() ? "true" : " false";
@@ -47,20 +62,20 @@ EvaluationResult In::evaluate(const EvaluationContext& params) const {
             needleValue.erase(needleValue.find_last_not_of('0') + 1, std::string::npos);
             needleValue.erase(needleValue.find_last_not_of('.') + 1, std::string::npos);
         }
-        return EvaluationResult(heystackString.find(needleValue) != std::string::npos);
+        return EvaluationResult(haystackString.find(needleValue) != std::string::npos);
     } else {
-        const auto heystackArray = evaluatedHeystack->get<std::vector<Value>>();
+        const auto haystackArray = evaluatedHaystack->get<std::vector<Value>>();
 
         bool result = false;
         if (evaluatedNeedleType == type::Boolean) {
             auto needleValue = evaluatedNeedle->get<bool>();
-            result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end();
+            result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end();
         } else if (evaluatedNeedleType == type::String) {
             auto needleValue = evaluatedNeedle->get<std::string>();
-            result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end();
+            result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end();
         } else if (evaluatedNeedleType == type::Number) {
             auto needleValue = evaluatedNeedle->get<double>();
-            result = find(heystackArray.begin(), heystackArray.end(), needleValue) != heystackArray.end();
+            result = find(haystackArray.begin(), haystackArray.end(), needleValue) != haystackArray.end();
         }
         return EvaluationResult(result);
     }
@@ -98,19 +113,6 @@ ParseResult In::parse(const Convertible& value, ParsingContext& ctx) {
     return ParseResult(std::make_unique<In>(std::move(*needle), std::move(*haystack)));
 }
 
-bool In::isComparableType(type::Type type) {
-    return type == type::Boolean || type == type::String || type == type::Number || type == type::Null ||
-           type == type::Value;
-}
-
-bool In::isComparableRuntimeValue(type::Type type) {
-    return type == type::Boolean || type == type::String || type == type::Number || type == type::Null;
-}
-
-bool In::isSearchableRuntimeValue(type::Type type) {
-    return type == type::String || type.is<type::Array>() || type == type::Null;
-}
-
 } // namespace expression
 } // namespace style
 } // namespace mbgl