Per review:

* Split Position conversion into its own file
* Add light transition parsing + tests

6 files changed, 92 insertions, 24 deletions

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index 678f940f48..387c718ee6 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -315,6 +315,7 @@ set(MBGL_CORE_FILES
     include/mbgl/style/conversion/layer.hpp
     include/mbgl/style/conversion/light.hpp
     include/mbgl/style/conversion/make_property_setters.hpp
+    include/mbgl/style/conversion/position.hpp
     include/mbgl/style/conversion/property_setter.hpp
     include/mbgl/style/conversion/property_value.hpp
     include/mbgl/style/conversion/source.hpp
diff --git a/include/mbgl/style/conversion/constant.hpp b/include/mbgl/style/conversion/constant.hpp
index f6af073edb..07c0a35fae 100644
--- a/include/mbgl/style/conversion/constant.hpp
+++ b/include/mbgl/style/conversion/constant.hpp
@@ -5,7 +5,6 @@
 #include <mbgl/util/color.hpp>
 #include <mbgl/util/enum.hpp>
 #include <mbgl/util/string.hpp>
-#include <mbgl/util/position.hpp>
 
 #include <array>
 #include <string>
@@ -94,26 +93,6 @@ struct Converter<Color> {
     }
 };
 
-template <>
-struct Converter<Position> {
-    template <class V>
-    optional<Position> operator()(const V& value, Error& error) const {
-        optional<std::array<float, 3>> spherical = convert<std::array<float, 3>>(value, error);
-
-        if (!spherical) {
-            return {};
-        }
-
-        optional<Position> converted = Position(*spherical);
-        if (!converted) {
-            error = { "value must be a valid position" };
-            return {};
-        }
-
-        return *converted;
-    }
-};
-
 template <size_t N>
 struct Converter<std::array<float, N>> {
     template <class V>
diff --git a/include/mbgl/style/conversion/light.hpp b/include/mbgl/style/conversion/light.hpp
index fdd62b539a..f85c5b694e 100644
--- a/include/mbgl/style/conversion/light.hpp
+++ b/include/mbgl/style/conversion/light.hpp
@@ -2,7 +2,7 @@
 
 #include <mbgl/style/light.hpp>
 #include <mbgl/style/conversion.hpp>
-#include <mbgl/style/conversion/constant.hpp>
+#include <mbgl/style/conversion/position.hpp>
 #include <mbgl/style/conversion/property_value.hpp>
 #include <mbgl/style/conversion/transition_options.hpp>
 
@@ -34,6 +34,17 @@ public:
             }
         }
 
+        const auto anchorTransition = objectMember(value, "anchor-transition");
+        if (anchorTransition) {
+            optional<TransitionOptions> transition =
+                convert<TransitionOptions>(*anchorTransition, error);
+            if (transition) {
+                light.setTransition<LightAnchor>(*transition);
+            } else {
+                return {};
+            }
+        }
+
         const auto color = objectMember(value, "color");
         if (color) {
             optional<PropertyValue<Color>> convertedColor =
@@ -46,6 +57,17 @@ public:
             }
         }
 
+        const auto colorTransition = objectMember(value, "color-transition");
+        if (colorTransition) {
+            optional<TransitionOptions> transition =
+                convert<TransitionOptions>(*colorTransition, error);
+            if (transition) {
+                light.setTransition<LightColor>(*transition);
+            } else {
+                return {};
+            }
+        }
+
         const auto position = objectMember(value, "position");
         if (position) {
             optional<PropertyValue<Position>> convertedPosition =
@@ -58,6 +80,17 @@ public:
             }
         }
 
+        const auto positionTransition = objectMember(value, "position-transition");
+        if (positionTransition) {
+            optional<TransitionOptions> transition =
+                convert<TransitionOptions>(*positionTransition, error);
+            if (transition) {
+                light.setTransition<LightPosition>(*transition);
+            } else {
+                return {};
+            }
+        }
+
         const auto intensity = objectMember(value, "intensity");
         if (intensity) {
             optional<PropertyValue<float>> convertedIntensity =
@@ -70,8 +103,18 @@ public:
             }
         }
 
+        const auto intensityTransition = objectMember(value, "intensity-transition");
+        if (intensityTransition) {
+            optional<TransitionOptions> transition =
+                convert<TransitionOptions>(*intensityTransition, error);
+            if (transition) {
+                light.setTransition<LightIntensity>(*transition);
+            } else {
+                return {};
+            }
+        }
         return { light };
-    }
+    };
 };
 
 } // namespace conversion
diff --git a/include/mbgl/style/conversion/position.hpp b/include/mbgl/style/conversion/position.hpp
new file mode 100644
index 0000000000..4570dd912a
--- /dev/null
+++ b/include/mbgl/style/conversion/position.hpp
@@ -0,0 +1,29 @@
+#pragma once
+
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/util/optional.hpp>
+#include <mbgl/util/position.hpp>
+
+#include <array>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <>
+struct Converter<Position> {
+    template <class V>
+    optional<Position> operator()(const V& value, Error& error) const {
+        optional<std::array<float, 3>> spherical = convert<std::array<float, 3>>(value, error);
+
+        if (!spherical) {
+            return {};
+        }
+
+        return Position(*spherical);
+    }
+};
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/include/mbgl/style/light.hpp b/include/mbgl/style/light.hpp
index 1fc5e7ac67..d740a9c9e2 100644
--- a/include/mbgl/style/light.hpp
+++ b/include/mbgl/style/light.hpp
@@ -175,6 +175,11 @@ public:
     }
 
     template <class P>
+    TransitionOptions getTransition() {
+        return cascading.template get<P>().getTransition();
+    }
+
+    template <class P>
     void setTransition(const TransitionOptions& value) {
         cascading.template get<P>().setTransition(value);
     }
@@ -213,7 +218,7 @@ struct LightAnchor : LightProperty<LightAnchorType> {
 };
 struct LightPosition : LightProperty<Position> {
     static Position defaultValue() {
-        return Position{{{ 1.15, 210, 30 }}};
+        return Position{ { { 1.15, 210, 30 } } };
     }
 };
 struct LightColor : LightProperty<Color> {
diff --git a/test/style/conversion/light.test.cpp b/test/style/conversion/light.test.cpp
index bb71544e8f..cff7a6b5bc 100644
--- a/test/style/conversion/light.test.cpp
+++ b/test/style/conversion/light.test.cpp
@@ -6,6 +6,7 @@
 #include <mbgl/style/conversion/light.hpp>
 #include <mbgl/util/rapidjson.hpp>
 #include <mbgl/util/color.hpp>
+#include <mbgl/util/chrono.hpp>
 #include <mbgl/util/position.hpp>
 
 #include <array>
@@ -52,6 +53,16 @@ TEST(StyleConversion, Light) {
     }
 
     {
+        auto light = parseLight("{\"color\":\"blue\",\"intensity\":0.3,\"color-transition\":{\"duration\":1000}}");
+        ASSERT_TRUE((bool) light);
+
+        ASSERT_FALSE(light->get<LightColor>().isUndefined());
+        ASSERT_TRUE(light->get<LightColor>().isConstant());
+        ASSERT_FALSE(light->get<LightColor>().isCameraFunction());
+        ASSERT_EQ(light->getTransition<LightColor>().duration, mbgl::Duration(mbgl::Milliseconds(1000)));
+    }
+
+    {
         auto light = parseLight("{\"intensity\":false}");
 
         ASSERT_FALSE((bool) light);