From 341058cd62472c6f2430db18950345de34f324cb Mon Sep 17 00:00:00 2001
From: Alexander Shalamov <alexander.shalamov@mapbox.com>
Date: Thu, 28 Mar 2019 00:44:32 +0200
Subject: [PATCH] [core] Traverse expression tree when checking for property
 overrides

Before this change, symbol layer was only checking whether top level
'text-field' layout property expression is FormatExpression and if it
has paint property overrides. This change takes into account that
'text-field' might have nested expressions, thus, requires traversal
over child expressions.

Fixes: #14254
---
 src/mbgl/style/expression/value.cpp         |  5 ++
 src/mbgl/style/layers/symbol_layer_impl.hpp | 62 ++++++++++++++++-----
 test/style/style_layer.test.cpp             | 12 ++++
 3 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/src/mbgl/style/expression/value.cpp b/src/mbgl/style/expression/value.cpp
index 436ed83ecd4..c2c21053368 100644
--- a/src/mbgl/style/expression/value.cpp
+++ b/src/mbgl/style/expression/value.cpp
@@ -166,6 +166,11 @@ mbgl::Value ValueConverter<mbgl::Value>::fromExpressionValue(const Value& value)
                     }
                     options.emplace("text-font", std::vector<mbgl::Value>{ std::string("literal"), fontStack });
                 }
+
+                if (section.textColor) {
+                    options.emplace("text-color", fromExpressionValue(*section.textColor));
+                }
+
                 serialized.push_back(options);
             }
             return serialized;
diff --git a/src/mbgl/style/layers/symbol_layer_impl.hpp b/src/mbgl/style/layers/symbol_layer_impl.hpp
index f937fccaa83..9b63e0e8d6d 100644
--- a/src/mbgl/style/layers/symbol_layer_impl.hpp
+++ b/src/mbgl/style/layers/symbol_layer_impl.hpp
@@ -3,9 +3,11 @@
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
+#include <mbgl/style/expression/literal.hpp>
 #include <mbgl/style/expression/format_expression.hpp>
 #include <mbgl/style/expression/formatted.hpp>
 #include <mbgl/style/expression/format_section_override.hpp>
+#include <mbgl/style/expression/value.hpp>
 
 namespace mbgl {
 namespace style {
@@ -56,25 +58,57 @@ struct FormatSectionOverrides<TypeList<PaintProperty...>> {
 
     template<typename Property, typename FormattedProperty>
     static bool hasOverride(const FormattedProperty& formatted) {
+
+        const auto checkLiteral = [] (const TextField::Type& literal) {
+            for (const auto& section : literal.sections) {
+                 if (Property::hasOverride(section)) {
+                     return true;
+                 }
+            }
+            return false;
+        };
+
         return formatted.match(
-                [] (const TextField::Type& t) {
-                    for (const auto& section : t.sections) {
-                        if (Property::hasOverride(section)) {
-                            return true;
-                        }
-                    }
-                    return false;
+                [&checkLiteral] (const TextField::Type& literal) {
+                    return checkLiteral(literal);
                 },
-                [] (const PropertyExpression<TextField::Type>& t) {
-                    if (t.getExpression().getKind() == expression::Kind::FormatExpression) {
-                        const auto* e = static_cast<const expression::FormatExpression*>(&t.getExpression());
-                        for (const auto& section : e->getSections()) {
-                            if (Property::hasOverride(section)) {
-                                return true;
+                [&checkLiteral] (const PropertyExpression<TextField::Type>& property) {
+                    bool expressionHasOverrides = false;
+                    const auto checkExpression = [&](const expression::Expression& e) {
+                        if (expressionHasOverrides) {
+                            return;
+                        }
+
+                        if (e.getKind() == expression::Kind::Literal &&
+                            e.getType() == expression::type::Formatted) {
+                            const auto* literalExpr = static_cast<const expression::Literal*>(&e);
+                            const auto formattedValue = expression::fromExpressionValue<expression::Formatted>(literalExpr->getValue());
+                            if (formattedValue && checkLiteral(*formattedValue)) {
+                                expressionHasOverrides = true;
+                            }
+                            return;
+                        }
+
+                        if (e.getKind() == expression::Kind::FormatExpression) {
+                            const auto* formatExpr = static_cast<const expression::FormatExpression*>(&e);
+                            for (const auto& section : formatExpr->getSections()) {
+                                if (Property::hasOverride(section)) {
+                                    expressionHasOverrides = true;
+                                    break;
+                                }
                             }
                         }
+                    };
+
+                    // Check root property expression and return early.
+                    checkExpression(property.getExpression());
+                    if (expressionHasOverrides) {
+                        return true;
                     }
-                    return false;
+
+                    // Traverse thru children and check whether any of them have overrides.
+                    property.getExpression().eachChild(checkExpression);
+                    return expressionHasOverrides;
                 },
                 [] (const auto&) {
                     return false;
diff --git a/test/style/style_layer.test.cpp b/test/style/style_layer.test.cpp
index e58a5fe5d00..7598d888e99 100644
--- a/test/style/style_layer.test.cpp
+++ b/test/style/style_layer.test.cpp
@@ -1,4 +1,5 @@
 #include <mbgl/style/expression/dsl.hpp>
+#include <mbgl/style/expression/match.hpp>
 #include <mbgl/style/expression/format_expression.hpp>
 #include <mbgl/style/style_impl.hpp>
 #include <mbgl/style/layers/background_layer.hpp>
@@ -330,6 +331,17 @@ void testHasOverrides(LayoutType& layout) {
     PropertyExpression<Formatted> propExprOverride(std::move(formatExprOverride));
     layout.template get<TextField>() = PropertyValueType<Formatted>(std::move(propExprOverride));
     EXPECT_TRUE(MockOverrides::hasOverrides(layout.template get<TextField>()));
+
+    // Nested expressions, overridden text-color.
+    auto formattedExpr1 = format("first paragraph");
+    std::vector<FormatExpressionSection> sections{ { literal("second paragraph"), nullopt, nullopt, toColor(literal("blue")) } };
+    auto formattedExpr2 = std::make_unique<FormatExpression>(std::move(sections));
+    std::unordered_map<std::string, std::shared_ptr<Expression>> branches{ { "1st", std::move(formattedExpr1) },
+                                                                           { "2nd", std::move(formattedExpr2) } };
+    auto match = std::make_unique<Match<std::string>>(type::Formatted, literal("input"), std::move(branches), format("otherwise"));
+    PropertyExpression<Formatted> nestedPropExpr(std::move(match));
+    layout.template get<TextField>() = PropertyValueType<Formatted>(std::move(nestedPropExpr));
+    EXPECT_TRUE(MockOverrides::hasOverrides(layout.template get<TextField>()));
 }
 
 } // namespace