[core] Rewrite style parsing logic for reuse in node bindings

include/mbgl/style/conversion.hpp

@@ -0,0 +1,263 @@
+#pragma once
+
+#include <mbgl/style/types.hpp>
+#include <mbgl/style/property_value.hpp>
+#include <mbgl/style/transition_options.hpp>
+
+#include <mbgl/util/variant.hpp>
+#include <mbgl/util/optional.hpp>
+#include <mbgl/util/color.hpp>
+#include <mbgl/util/enum.hpp>
+
+#include <array>
+#include <string>
+#include <vector>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+/*
+   This namespace defines a generic conversion from a templated type `V` representing a JSON object conforming
+   to a style property from the Mapbox Style Specification, to `PropertyValue<T>`:
+
+       template <class T, class V>
+       Result<PropertyValue<T>> convertPropertyValue(const V& value);
+
+   This is used concretely for conversions from RapidJSON types in mbgl core, and from v8 types in
+   the node bindings.
+
+   The requirements are that the following are legal expressions for a value `v` of type `const V&`:
+
+      * `isArray(v)` -- returns a boolean indicating whether `v` represents a JSON array
+      * `arrayLength(v)` -- called only if `isArray(v)`; returns a size_t length
+      * `arrayMember(v)` -- called only if `isArray(v)`; returns `V` or `V&`
+
+      * `isObject(v)` -- returns a boolean indicating whether `v` represents a JSON object
+      * `objectMember(v, name)` -- called only if `isObject(v)`; `name` is `const char *`; return value:
+         * is true when evaluated in a boolean context iff the named member exists
+         * is convertable to a `V` or `V&` when dereferenced
+
+      * `toBool(v)` -- returns `optional<bool>`, absence indicating `v` is not a JSON boolean
+      * `toNumber(v)` -- returns `optional<float>`, absence indicating `v` is not a JSON number
+      * `toString(v)` -- returns `optional<std::string>`, absence indicating `v` is not a JSON string
+
+   If for any reason the conversion fails, the result of `convertPropertyValue` will be the `Error` variant,
+   which includes explanatory text.
+*/
+
+struct Error { const char * message; };
+
+template <class V>
+using Result = variant<V, Error>;
+
+template <class V, class T, class Enable = void>
+struct ConstantConverter {};
+
+template <class V>
+struct ConstantConverter<V, bool> {
+    Result<bool> operator()(const V& value) const {
+        optional<bool> converted = toBool(value);
+        if (!converted) {
+            return Error { "value must be a boolean" };
+        }
+        return *converted;
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, float> {
+    Result<float> operator()(const V& value) const {
+        optional<float> converted = toNumber(value);
+        if (!converted) {
+            return Error { "value must be a number" };
+        }
+        return *converted;
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, std::string> {
+    Result<std::string> operator()(const V& value) const {
+        optional<std::string> converted = toString(value);
+        if (!converted) {
+            return Error { "value must be a string" };
+        }
+        return *converted;
+    }
+};
+
+template <class V, class T>
+struct ConstantConverter<V, T, typename std::enable_if_t<std::is_enum<T>::value>> {
+    Result<T> operator()(const V& value) const {
+        optional<std::string> string = toString(value);
+        if (!string) {
+            return Error { "value must be a string" };
+        }
+
+        const auto result = Enum<T>::toEnum(*string);
+        if (!result) {
+            return Error { "value must be a valid enumeration value" };
+        }
+
+        return *result;
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, Color> {
+    Result<Color> operator()(const V& value) const {
+        optional<std::string> string = toString(value);
+        if (!string) {
+            return Error { "value must be a string" };
+        }
+
+        optional<Color> color = Color::parse(*string);
+        if (!color) {
+            return Error { "value must be a valid color" };
+        }
+
+        return *color;
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, std::array<float, 2>> {
+    Result<std::array<float, 2>> operator()(const V& value) const {
+        if (!isArray(value) || arrayLength(value) != 2) {
+            return Error { "value must be an array of two numbers" };
+        }
+
+        optional<float> first = toNumber(arrayMember(value, 0));
+        optional<float> second = toNumber(arrayMember(value, 1));
+        if (!first || !second) {
+            return Error { "value must be an array of two numbers" };
+        }
+
+        return std::array<float, 2> {{ *first, *second }};
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, std::array<float, 4>> {
+    Result<std::array<float, 4>> operator()(const V& value) const {
+        if (!isArray(value) || arrayLength(value) != 4) {
+            return Error { "value must be an array of four numbers" };
+        }
+
+        optional<float> first = toNumber(arrayMember(value, 0));
+        optional<float> second = toNumber(arrayMember(value, 1));
+        optional<float> third = toNumber(arrayMember(value, 2));
+        optional<float> fourth = toNumber(arrayMember(value, 3));
+        if (!first || !second) {
+            return Error { "value must be an array of four numbers" };
+        }
+
+        return std::array<float, 4> {{ *first, *second, *third, *fourth }};
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, std::vector<float>> {
+    Result<std::vector<float>> operator()(const V& value) const {
+        if (!isArray(value)) {
+            return Error { "value must be an array" };
+        }
+
+        std::vector<float> result;
+        result.reserve(arrayLength(value));
+
+        for (std::size_t i = 0; i < arrayLength(value); ++i) {
+            optional<float> number = toNumber(arrayMember(value, i));
+            if (!number) {
+                return Error { "value must be an array of numbers" };
+            }
+            result.push_back(*number);
+        }
+
+        return result;
+    }
+};
+
+template <class V>
+struct ConstantConverter<V, std::vector<std::string>> {
+    Result<std::vector<std::string>> operator()(const V& value) const {
+        if (!isArray(value)) {
+            return Error { "value must be an array" };
+        }
+
+        std::vector<std::string> result;
+        result.reserve(arrayLength(value));
+
+        for (std::size_t i = 0; i < arrayLength(value); ++i) {
+            optional<std::string> string = toString(arrayMember(value, i));
+            if (!string) {
+                return Error { "value must be an array of strings" };
+            }
+            result.push_back(*string);
+        }
+
+        return result;
+    }
+};
+
+template <class T, class V>
+Result<PropertyValue<T>> convertPropertyValue(const V& value) {
+    if (!isObject(value)) {
+        Result<T> constant = ConstantConverter<V, T>()(value);
+        if (constant.template is<Error>()) {
+            return constant.template get<Error>();
+        }
+        return constant.template get<T>();
+    }
+
+    auto stopsValue = objectMember(value, "stops");
+    if (!stopsValue) {
+        return Error { "function value must specify stops" };
+    }
+
+    if (!isArray(*stopsValue)) {
+        return Error { "function stops must be an array" };
+    }
+
+    std::vector<std::pair<float, T>> stops;
+    for (std::size_t i = 0; i < arrayLength(*stopsValue); ++i) {
+        const auto& stopValue = arrayMember(*stopsValue, i);
+
+        if (!isArray(stopValue)) {
+            return Error { "function stop must be an array" };
+        }
+
+        if (arrayLength(stopValue) != 2) {
+            return Error { "function stop must have two elements" };
+        }
+
+        optional<float> z = toNumber(arrayMember(stopValue, 0));
+        if (!z) {
+            return Error { "function stop zoom level must be a number" };
+        }
+
+        Result<T> v = ConstantConverter<V, T>()(arrayMember(stopValue, 1));
+        if (v.template is<Error>()) {
+            return v.template get<Error>();
+        }
+
+        stops.emplace_back(*z, v.template get<T>());
+    }
+
+    auto baseValue = objectMember(value, "base");
+    if (!baseValue) {
+        return Function<T>(stops, 1.0f);
+    }
+
+    optional<float> base = toNumber(*baseValue);
+    if (!base) {
+        return Error { "function base must be a number"};
+    }
+
+    return Function<T>(stops, *base);
+}
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl

src/mbgl/style/property_parsing.cpp

@@ -1,136 +1,8 @@
 #include <mbgl/style/property_parsing.hpp>
 
-#include <mbgl/platform/log.hpp>
-
 namespace mbgl {
 namespace style {
 
-template <>
-optional<bool> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsBool()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a boolean", name);
-        return {};
-    }
-
-    return value.GetBool();
-}
-
-template <>
-optional<float> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsNumber()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a number, or a number function", name);
-        return {};
-    }
-
-    return value.GetDouble();
-}
-
-template <>
-optional<std::string> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsString()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a string", name);
-        return {};
-    }
-
-    return std::string { value.GetString(), value.GetStringLength() };
-}
-
-template <>
-optional<Color> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsString()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a string", name);
-        return {};
-    }
-
-    optional<Color> result = Color::parse({ value.GetString(), value.GetStringLength() });
-    if (!result) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a valid color", name);
-        return {};
-    }
-
-    return result;
-}
-
-template <>
-optional<std::array<float, 2>> parseConstant(const char* name, const JSValue& value) {
-    if (value.IsArray() && value.Size() == 2 &&
-            value[rapidjson::SizeType(0)].IsNumber() &&
-            value[rapidjson::SizeType(1)].IsNumber()) {
-
-        float first = value[rapidjson::SizeType(0)].GetDouble();
-        float second = value[rapidjson::SizeType(1)].GetDouble();
-        return { {{ first, second }} };
-    } else {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be an array of two numbers", name);
-        return {};
-    }
-}
-
-template <>
-optional<std::array<float, 4>> parseConstant(const char* name, const JSValue& value) {
-    if (value.IsArray() && value.Size() == 4 &&
-            value[rapidjson::SizeType(0)].IsNumber() &&
-            value[rapidjson::SizeType(1)].IsNumber() &&
-            value[rapidjson::SizeType(2)].IsNumber() &&
-            value[rapidjson::SizeType(3)].IsNumber()) {
-
-        float first = value[rapidjson::SizeType(0)].GetDouble();
-        float second = value[rapidjson::SizeType(1)].GetDouble();
-        float third = value[rapidjson::SizeType(2)].GetDouble();
-        float fourth = value[rapidjson::SizeType(3)].GetDouble();
-        return { {{ first, second, third, fourth }} };
-    } else {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be an array of four numbers", name);
-        return {};
-    }
-}
-
-template <>
-optional<std::vector<float>> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsArray()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be an array of numbers", name);
-        return {};
-    }
-
-    std::vector<float> result;
-
-    for (rapidjson::SizeType i = 0; i < value.Size(); ++i) {
-        const JSValue& part = value[i];
-
-        if (!part.IsNumber()) {
-            Log::Warning(Event::ParseStyle, "value of '%s' must be an array of numbers", name);
-            return {};
-        }
-
-        result.push_back(part.GetDouble());
-    }
-
-    return result;
-}
-
-template <>
-optional<std::vector<std::string>> parseConstant(const char* name, const JSValue& value) {
-    if (!value.IsArray()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be an array of strings", name);
-        return {};
-    }
-
-    std::vector<std::string> result;
-
-    for (rapidjson::SizeType i = 0; i < value.Size(); ++i) {
-        const JSValue& part = value[i];
-
-        if (!part.IsString()) {
-            Log::Warning(Event::ParseStyle, "value of '%s' must be an array of strings", name);
-            return {};
-        }
-
-        result.push_back({ part.GetString(), part.GetStringLength() });
-    }
-
-    return result;
-}
-
 optional<TransitionOptions> parseTransitionOptions(const char *, const JSValue& value) {
     if (!value.IsObject()) {
         return {};

src/mbgl/style/property_parsing.hpp

@@ -1,117 +1,23 @@
 #pragma once
 
-#include <mbgl/style/types.hpp>
 #include <mbgl/style/property_value.hpp>
 #include <mbgl/style/transition_options.hpp>
-
-#include <mbgl/util/rapidjson.hpp>
-#include <mbgl/util/optional.hpp>
-#include <mbgl/util/color.hpp>
-#include <mbgl/util/enum.hpp>
+#include <mbgl/style/rapidjson_conversion.hpp>
+#include <mbgl/style/conversion.hpp>
 
 #include <mbgl/platform/log.hpp>
 
-#include <string>
-#include <array>
-#include <vector>
-
 namespace mbgl {
 namespace style {
 
-template <typename T, typename = std::enable_if_t<!std::is_enum<T>::value>>
-optional<T> parseConstant(const char* name, const JSValue&);
-
-template <> optional<bool> parseConstant(const char*, const JSValue&);
-template <> optional<float> parseConstant(const char*, const JSValue&);
-template <> optional<std::string> parseConstant(const char*, const JSValue&);
-template <> optional<Color> parseConstant(const char*, const JSValue&);
-template <> optional<std::array<float, 2>> parseConstant(const char*, const JSValue&);
-template <> optional<std::vector<float>> parseConstant(const char*, const JSValue&);
-template <> optional<std::vector<std::string>> parseConstant(const char*, const JSValue&);
-
-template <typename T>
-optional<T> parseConstant(const char* name, const JSValue& value,
-                          typename std::enable_if_t<std::is_enum<T>::value, void*> = nullptr) {
-    if (!value.IsString()) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a string", name);
-        return {};
-    }
-
-    const auto result = Enum<T>::toEnum({ value.GetString(), value.GetStringLength() });
-    if (!result) {
-        Log::Warning(Event::ParseStyle, "value of '%s' must be a valid enumeration value", name);
-    }
-
-    return result;
-}
-
 template <typename T>
 PropertyValue<T> parseProperty(const char* name, const JSValue& value) {
-    if (!value.IsObject()) {
-        auto constant = parseConstant<T>(name, value);
-
-        if (!constant) {
-            return {};
-        }
-
-        return *constant;
-    }
-
-    if (!value.HasMember("stops")) {
-        Log::Warning(Event::ParseStyle, "function must specify a function type");
+    conversion::Result<PropertyValue<T>> result = conversion::convertPropertyValue<T>(value);
+    if (result.template is<conversion::Error>()) {
+        Log::Warning(Event::ParseStyle, "%s: %s", name, result.template get<conversion::Error>().message);
         return {};
     }
-
-    float base = 1.0f;
-
-    if (value.HasMember("base")) {
-        const JSValue& value_base = value["base"];
-
-        if (!value_base.IsNumber()) {
-            Log::Warning(Event::ParseStyle, "base must be numeric");
-            return {};
-        }
-
-        base = value_base.GetDouble();
-    }
-
-    const JSValue& stopsValue = value["stops"];
-
-    if (!stopsValue.IsArray()) {
-        Log::Warning(Event::ParseStyle, "stops function must specify a stops array");
-        return {};
-    }
-
-    std::vector<std::pair<float, T>> stops;
-
-    for (rapidjson::SizeType i = 0; i < stopsValue.Size(); ++i) {
-        const JSValue& stop = stopsValue[i];
-
-        if (!stop.IsArray()) {
-            Log::Warning(Event::ParseStyle, "function argument must be a numeric value");
-            return {};
-        }
-
-        if (stop.Size() != 2) {
-            Log::Warning(Event::ParseStyle, "stop must have zoom level and value specification");
-            return {};
-        }
-
-        const JSValue& z = stop[rapidjson::SizeType(0)];
-        if (!z.IsNumber()) {
-            Log::Warning(Event::ParseStyle, "zoom level in stop must be a number");
-            return {};
-        }
-
-        optional<T> v = parseConstant<T>(name, stop[rapidjson::SizeType(1)]);
-        if (!v) {
-            return {};
-        }
-
-        stops.emplace_back(z.GetDouble(), *v);
-    }
-
-    return Function<T>(stops, base);
+    return result.template get<PropertyValue<T>>();
 }
 
 optional<TransitionOptions> parseTransitionOptions(const char * name, const JSValue&);

src/mbgl/style/rapidjson_conversion.hpp

@@ -0,0 +1,55 @@
+#pragma once
+
+#include <mbgl/util/rapidjson.hpp>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+inline bool isArray(const JSValue& value) {
+    return value.IsArray();
+}
+
+inline std::size_t arrayLength(const JSValue& value) {
+    return value.Size();
+}
+
+inline const JSValue& arrayMember(const JSValue& value, std::size_t i) {
+    return value[rapidjson::SizeType(i)];
+}
+
+inline bool isObject(const JSValue& value) {
+    return value.IsObject();
+}
+
+inline const JSValue* objectMember(const JSValue& value, const char * name) {
+    if (!value.HasMember(name)) {
+        return nullptr;
+    }
+    return &value[name];
+}
+
+inline optional<bool> toBool(const JSValue& value) {
+    if (!value.IsBool()) {
+        return {};
+    }
+    return value.GetBool();
+}
+
+inline optional<float> toNumber(const JSValue& value) {
+    if (!value.IsNumber()) {
+        return {};
+    }
+    return value.GetDouble();
+}
+
+inline optional<std::string> toString(const JSValue& value) {
+    if (!value.IsString()) {
+        return {};
+    }
+    return {{ value.GetString(), value.GetStringLength() }};
+}
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl

test/fixtures/style_parser/circle-blur.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'circle-blur' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "circle-blur: value must be a number"]
         ]
     }
 }

test/fixtures/style_parser/circle-color.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'circle-color' must be a string"]
+            [1, "WARNING", "ParseStyle", "circle-color: value must be a string"]
         ]
     }
 }

test/fixtures/style_parser/circle-opacity.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'circle-opacity' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "circle-opacity: value must be a number"]
         ]
     }
 }

test/fixtures/style_parser/circle-radius.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'circle-radius' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "circle-radius: value must be a number"]
         ]
     }
 }

test/fixtures/style_parser/function-numeric.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "function argument must be a numeric value"]
+            [1, "WARNING", "ParseStyle", "line-width: function stop must be an array"]
         ]
     }
 }

test/fixtures/style_parser/function-type.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "function must specify a function type"]
+            [1, "WARNING", "ParseStyle", "line-width: function value must specify stops"]
         ]
     }
 }

test/fixtures/style_parser/line-opacity.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'line-opacity' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "line-opacity: value must be a number"]
         ]
     }
 }

test/fixtures/style_parser/line-translate.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'line-translate' must be an array of two numbers"]
+            [1, "WARNING", "ParseStyle", "line-translate: value must be an array of two numbers"]
         ]
     }
 }

test/fixtures/style_parser/line-width.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'line-width' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "line-width: value must be a number"]
         ]
     }
 }

test/fixtures/style_parser/stop-zoom-value.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "stop must have zoom level and value specification"]
+            [1, "WARNING", "ParseStyle", "fill-opacity: function stop must have two elements"]
         ]
     }
 }

test/fixtures/style_parser/stops-array.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "stops function must specify a stops array"]
+            [1, "WARNING", "ParseStyle", "line-width: function stops must be an array"]
         ]
     }
 }

test/fixtures/style_parser/text-size.info.json

@@ -1,7 +1,7 @@
 {
     "default": {
         "log": [
-            [1, "WARNING", "ParseStyle", "value of 'text-size' must be a number, or a number function"]
+            [1, "WARNING", "ParseStyle", "text-size: value must be a number"]
         ]
     }
 }