Use registry instead of cascading conditionals

5 files changed, 71 insertions, 80 deletions

diff --git a/include/mbgl/style/expression/is_expression.hpp b/include/mbgl/style/expression/is_expression.hpp
index 8eeb0434e4..77c489619c 100644
--- a/include/mbgl/style/expression/is_expression.hpp
+++ b/include/mbgl/style/expression/is_expression.hpp
@@ -1,8 +1,6 @@
 #pragma once
 
 #include <mbgl/style/expression/expression.hpp>
-#include <mbgl/style/expression/compound_expression.hpp>
-#include <unordered_set>
 
 namespace mbgl {
 namespace style {
diff --git a/include/mbgl/style/expression/parsing_context.hpp b/include/mbgl/style/expression/parsing_context.hpp
index 1ede3e50db..5127a476b1 100644
--- a/include/mbgl/style/expression/parsing_context.hpp
+++ b/include/mbgl/style/expression/parsing_context.hpp
@@ -137,6 +137,10 @@ private:
     std::shared_ptr<std::vector<ParsingError>> errors;
 };
 
+using ParseFunction = ParseResult (*)(const conversion::Convertible&, ParsingContext&);
+using ExpressionRegistry = std::unordered_map<std::string, ParseFunction>;
+const ExpressionRegistry& getExpressionRegistry();
+
 } // namespace expression
 } // namespace style
 } // namespace mbgl
diff --git a/src/mbgl/style/expression/is_expression.cpp b/src/mbgl/style/expression/is_expression.cpp
index 0ed3ac594e..77212f6a1e 100644
--- a/src/mbgl/style/expression/is_expression.cpp
+++ b/src/mbgl/style/expression/is_expression.cpp
@@ -1,6 +1,11 @@
 #include <mbgl/style/expression/is_expression.hpp>
+#include <mbgl/style/expression/compound_expression.hpp>
+#include <mbgl/style/expression/parsing_context.hpp>
+
 #include <mbgl/style/conversion.hpp>
 
+#include <unordered_set>
+
 namespace mbgl {
 namespace style {
 namespace expression {
@@ -8,33 +13,13 @@ namespace expression {
 using namespace mbgl::style::conversion;
 
 bool isExpression(const Convertible& value) {
-    static std::unordered_set<std::string> specialForms = {
-        "all",
-        "any",
-        "array",
-        "at",
-        "boolean",
-        "case",
-        "coalesce",
-        "color",
-        "curve",
-        "let",
-        "literal",
-        "match",
-        "number",
-        "string",
-        "to-boolean",
-        "to-color",
-        "to-number",
-        "to-string",
-        "var",
-    };
+    const ExpressionRegistry& registry = getExpressionRegistry();
 
     if (!isArray(value) || arrayLength(value) == 0) return false;
     optional<std::string> name = toString(arrayMember(value, 0));
     if (!name) return false;
     
-    return (specialForms.find(*name) != specialForms.end()) ||
+    return (registry.find(*name) != registry.end()) ||
         (CompoundExpressionRegistry::definitions.find(*name) != CompoundExpressionRegistry::definitions.end());
 }
 
diff --git a/src/mbgl/style/expression/literal.cpp b/src/mbgl/style/expression/literal.cpp
index 85664f0399..fc11878bea 100644
--- a/src/mbgl/style/expression/literal.cpp
+++ b/src/mbgl/style/expression/literal.cpp
@@ -49,6 +49,8 @@ optional<Value> parseValue(const Convertible& value, ParsingContext& ctx) {
     }
     
     optional<mbgl::Value> v = toValue(value);
+    // since value represents a JSON value, if it's not undefined, object, or
+    // array, it must be convertible to mbgl::Value
     assert(v);
     
     return v->match(
@@ -62,28 +64,42 @@ optional<Value> parseValue(const Convertible& value, ParsingContext& ctx) {
 }
 
 ParseResult Literal::parse(const Convertible& value, ParsingContext& ctx) {
-    const optional<Value> parsedValue = parseValue(value, ctx);
-
-    if (!parsedValue) {
+    if (isObject(value)) {
+        ctx.error(R"(Bare objects invalid. Use ["literal", {...}] instead.)");
         return ParseResult();
-    }
-
-    // special case: infer the item type if possible for zero-length arrays
-    if (
-        ctx.getExpected() &&
-        ctx.getExpected()->template is<type::Array>() &&
-        parsedValue->template is<std::vector<Value>>()
-    ) {
-        auto type = typeOf(*parsedValue).template get<type::Array>();
-        auto expected = ctx.getExpected()->template get<type::Array>();
+    } else if (isArray(value)) {
+        // object or array value, quoted with ["literal", value]
+        if (arrayLength(value) != 2) {
+            ctx.error("'literal' expression requires exactly one argument, but found " + std::to_string(arrayLength(value) - 1) + " instead.");
+            return ParseResult();
+        }
+        const optional<Value> parsedValue = parseValue(arrayMember(value, 1), ctx);
+        if (!parsedValue) {
+            return ParseResult();
+        }
+        
+        // special case: infer the item type if possible for zero-length arrays
         if (
-            type.N && (*type.N == 0) &&
-            (!expected.N || (*expected.N == 0))
+            ctx.getExpected() &&
+            ctx.getExpected()->template is<type::Array>() &&
+            parsedValue->template is<std::vector<Value>>()
         ) {
-            return ParseResult(std::make_unique<Literal>(expected, parsedValue->template get<std::vector<Value>>()));
+            auto type = typeOf(*parsedValue).template get<type::Array>();
+            auto expected = ctx.getExpected()->template get<type::Array>();
+            if (
+                type.N && (*type.N == 0) &&
+                (!expected.N || (*expected.N == 0))
+            ) {
+                return ParseResult(std::make_unique<Literal>(expected, parsedValue->template get<std::vector<Value>>()));
+            }
         }
+        
+        return ParseResult(std::make_unique<Literal>(*parsedValue));
+    } else {
+        // bare primitive value (string, number, boolean, null)
+        const optional<Value> parsedValue = parseValue(value, ctx);
+        return ParseResult(std::make_unique<Literal>(*parsedValue));
     }
-    return ParseResult(std::make_unique<Literal>(*parsedValue));
 }
 
 } // namespace expression
diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp
index 2f855a1ad9..18a88b8199 100644
--- a/src/mbgl/style/expression/parsing_context.cpp
+++ b/src/mbgl/style/expression/parsing_context.cpp
@@ -68,7 +68,29 @@ ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_,
     return child.parse(value);
 }
 
-using namespace mbgl::style::conversion;
+const ExpressionRegistry& getExpressionRegistry() {
+    static ExpressionRegistry registry {{
+        {"all", All::parse},
+        {"any", Any::parse},
+        {"array", ArrayAssertion::parse},
+        {"at", At::parse},
+        {"boolean", Assertion::parse},
+        {"case", Case::parse},
+        {"coalesce", Coalesce::parse},
+        {"curve", parseCurve},
+        {"let", Let::parse},
+        {"literal", Literal::parse},
+        {"match", parseMatch},
+        {"number", Assertion::parse},
+        {"object", Assertion::parse},
+        {"string", Assertion::parse},
+        {"to-color", Coercion::parse},
+        {"to-number", Coercion::parse},
+        {"var", Var::parse}
+    }};
+    return registry;
+}
+
 ParseResult ParsingContext::parse(const Convertible& value)
 {
     ParseResult parsed;
@@ -90,48 +112,14 @@ ParseResult ParsingContext::parse(const Convertible& value)
             return ParseResult();
         }
         
-        if (*op == "literal") {
-            if (length != 2) {
-                error(
-                    "'literal' expression requires exactly one argument, but found " + std::to_string(length - 1) + " instead."
-                );
-                return ParseResult();
-            }
-            
-            parsed = Literal::parse(arrayMember(value, 1), *this);
-        } else if (*op == "match") {
-            parsed = parseMatch(value, *this);
-        } else if (*op == "curve") {
-            parsed = parseCurve(value, *this);
-        } else if (*op == "coalesce") {
-            parsed = Coalesce::parse(value, *this);
-        } else if (*op == "case") {
-            parsed = Case::parse(value, *this);
-        } else if (*op == "array") {
-            parsed = ArrayAssertion::parse(value, *this);
-        } else if (*op == "let") {
-            parsed = Let::parse(value, *this);
-        } else if (*op == "var") {
-            parsed = Var::parse(value, *this);
-        } else if (*op == "at") {
-            parsed = At::parse(value, *this);
-        } else if (*op == "string" || *op == "number" || *op == "boolean" || *op == "object") {
-            parsed = Assertion::parse(value, *this);
-        } else if (*op == "to-color" || *op == "to-number") {
-            parsed = Coercion::parse(value, *this);
-        } else if (*op == "any") {
-            parsed = Any::parse(value, *this);
-        } else if (*op == "all") {
-            parsed = All::parse(value, *this);
+        const ExpressionRegistry& registry = getExpressionRegistry();
+        auto parseFunction = registry.find(*op);
+        if (parseFunction != registry.end()) {
+            parsed = parseFunction->second(value, *this);
         } else {
             parsed = parseCompoundExpression(*op, value, *this);
         }
     } else {
-        if (isObject(value)) {
-            error(R"(Bare objects invalid. Use ["literal", {...}] instead.)");
-            return ParseResult();
-        }
-        
         parsed = Literal::parse(value, *this);
     }