diff options
-rw-r--r-- | cmake/core-files.cmake | 1 | ||||
-rw-r--r-- | include/mbgl/style/conversion/data_driven_property_value.hpp | 14 | ||||
-rw-r--r-- | include/mbgl/style/conversion/expression.hpp | 39 | ||||
-rw-r--r-- | include/mbgl/style/conversion/heatmap_color_property_value.hpp | 7 | ||||
-rw-r--r-- | include/mbgl/style/conversion/property_value.hpp | 9 | ||||
-rw-r--r-- | include/mbgl/style/expression/parsing_context.hpp | 34 | ||||
-rw-r--r-- | platform/node/src/node_expression.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/style/conversion/filter.cpp | 7 | ||||
-rw-r--r-- | src/mbgl/style/expression/parsing_context.cpp | 27 | ||||
-rw-r--r-- | test/style/expression/expression.test.cpp | 2 | ||||
-rw-r--r-- | test/style/filter.test.cpp | 14 |
11 files changed, 88 insertions, 70 deletions
diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index dd39395e5a..0853097630 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -410,7 +410,6 @@ set(MBGL_CORE_FILES include/mbgl/style/conversion/coordinate.hpp include/mbgl/style/conversion/custom_geometry_source_options.hpp include/mbgl/style/conversion/data_driven_property_value.hpp - include/mbgl/style/conversion/expression.hpp include/mbgl/style/conversion/filter.hpp include/mbgl/style/conversion/function.hpp include/mbgl/style/conversion/geojson.hpp diff --git a/include/mbgl/style/conversion/data_driven_property_value.hpp b/include/mbgl/style/conversion/data_driven_property_value.hpp index b4297816df..07ed201c99 100644 --- a/include/mbgl/style/conversion/data_driven_property_value.hpp +++ b/include/mbgl/style/conversion/data_driven_property_value.hpp @@ -4,12 +4,12 @@ #include <mbgl/style/conversion.hpp> #include <mbgl/style/conversion/constant.hpp> #include <mbgl/style/conversion/function.hpp> -#include <mbgl/style/conversion/expression.hpp> #include <mbgl/style/expression/is_expression.hpp> #include <mbgl/style/expression/is_constant.hpp> #include <mbgl/style/expression/find_zoom_curve.hpp> #include <mbgl/style/expression/literal.hpp> #include <mbgl/style/expression/value.hpp> +#include <mbgl/style/expression/parsing_context.hpp> #include <unordered_set> @@ -22,15 +22,15 @@ template <class T> struct Converter<DataDrivenPropertyValue<T>> { optional<DataDrivenPropertyValue<T>> operator()(const Convertible& value, Error& error) const { + using namespace mbgl::style::expression; + if (isUndefined(value)) { return DataDrivenPropertyValue<T>(); - } else if (expression::isExpression(value)) { - optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>( - value, - error, - valueTypeToExpressionType<T>()); - + } else if (isExpression(value)) { + ParsingContext ctx(valueTypeToExpressionType<T>()); + ParseResult expression = ctx.parseLayerPropertyExpression(value); if (!expression) { + error = { ctx.getCombinedErrors() }; return {}; } diff --git a/include/mbgl/style/conversion/expression.hpp b/include/mbgl/style/conversion/expression.hpp deleted file mode 100644 index c5fcf906a7..0000000000 --- a/include/mbgl/style/conversion/expression.hpp +++ /dev/null @@ -1,39 +0,0 @@ -#pragma once - -#include <mbgl/style/expression/parsing_context.hpp> -#include <mbgl/style/expression/type.hpp> -#include <mbgl/style/conversion.hpp> - -#include <memory> - -namespace mbgl { -namespace style { -namespace conversion { - -using namespace mbgl::style::expression; - -template<> struct Converter<std::unique_ptr<Expression>> { - optional<std::unique_ptr<Expression>> operator()(const Convertible& value, Error& error, type::Type expected) const { - ParsingContext ctx(optional<type::Type> {expected}); - ParseResult parsed = ctx.parse(value); - if (parsed) { - return std::move(*parsed); - } - std::string combinedError; - for (const ParsingError& parsingError : ctx.getErrors()) { - if (combinedError.size() > 0) { - combinedError += "\n"; - } - if (parsingError.key.size() > 0) { - combinedError += parsingError.key + ": "; - } - combinedError += parsingError.message; - } - error = { combinedError }; - return {}; - }; -}; - -} // namespace conversion -} // namespace style -} // namespace mbgl diff --git a/include/mbgl/style/conversion/heatmap_color_property_value.hpp b/include/mbgl/style/conversion/heatmap_color_property_value.hpp index e3689c524c..a4710792d2 100644 --- a/include/mbgl/style/conversion/heatmap_color_property_value.hpp +++ b/include/mbgl/style/conversion/heatmap_color_property_value.hpp @@ -4,11 +4,11 @@ #include <mbgl/style/conversion.hpp> #include <mbgl/style/conversion/constant.hpp> #include <mbgl/style/conversion/function.hpp> -#include <mbgl/style/conversion/expression.hpp> #include <mbgl/style/expression/value.hpp> #include <mbgl/style/expression/is_constant.hpp> #include <mbgl/style/expression/is_expression.hpp> #include <mbgl/style/expression/find_zoom_curve.hpp> +#include <mbgl/style/expression/parsing_context.hpp> namespace mbgl { namespace style { @@ -17,11 +17,14 @@ namespace conversion { template <> struct Converter<HeatmapColorPropertyValue> { optional<HeatmapColorPropertyValue> operator()(const Convertible& value, Error& error) const { + using namespace mbgl::style::expression; if (isUndefined(value)) { return HeatmapColorPropertyValue(); } else if (isExpression(value)) { - optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, expression::type::Color); + ParsingContext ctx(type::Color); + ParseResult expression = ctx.parseLayerPropertyExpression(value); if (!expression) { + error = { ctx.getCombinedErrors() }; return {}; } assert(*expression); diff --git a/include/mbgl/style/conversion/property_value.hpp b/include/mbgl/style/conversion/property_value.hpp index 97117de2ec..3130661f61 100644 --- a/include/mbgl/style/conversion/property_value.hpp +++ b/include/mbgl/style/conversion/property_value.hpp @@ -4,11 +4,11 @@ #include <mbgl/style/conversion.hpp> #include <mbgl/style/conversion/constant.hpp> #include <mbgl/style/conversion/function.hpp> -#include <mbgl/style/conversion/expression.hpp> #include <mbgl/style/expression/value.hpp> #include <mbgl/style/expression/is_constant.hpp> #include <mbgl/style/expression/is_expression.hpp> #include <mbgl/style/expression/find_zoom_curve.hpp> +#include <mbgl/style/expression/parsing_context.hpp> namespace mbgl { namespace style { @@ -17,13 +17,18 @@ namespace conversion { template <class T> struct Converter<PropertyValue<T>> { optional<PropertyValue<T>> operator()(const Convertible& value, Error& error) const { + using namespace mbgl::style::expression; + if (isUndefined(value)) { return PropertyValue<T>(); } else if (isExpression(value)) { - optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, valueTypeToExpressionType<T>()); + ParsingContext ctx(valueTypeToExpressionType<T>()); + ParseResult expression = ctx.parseLayerPropertyExpression(value); if (!expression) { + error = { ctx.getCombinedErrors() }; return {}; } + if (isFeatureConstant(**expression)) { return { CameraFunction<T>(std::move(*expression)) }; } else { diff --git a/include/mbgl/style/expression/parsing_context.hpp b/include/mbgl/style/expression/parsing_context.hpp index f92a4c95ea..c19974a4f7 100644 --- a/include/mbgl/style/expression/parsing_context.hpp +++ b/include/mbgl/style/expression/parsing_context.hpp @@ -55,7 +55,7 @@ class ParsingContext { public: ParsingContext() : errors(std::make_shared<std::vector<ParsingError>>()) {} ParsingContext(std::string key_) : key(std::move(key_)), errors(std::make_shared<std::vector<ParsingError>>()) {} - explicit ParsingContext(optional<type::Type> expected_) + explicit ParsingContext(type::Type expected_) : expected(std::move(expected_)), errors(std::make_shared<std::vector<ParsingError>>()) {} @@ -67,6 +67,7 @@ public: std::string getKey() const { return key; } optional<type::Type> getExpected() const { return expected; } const std::vector<ParsingError>& getErrors() const { return *errors; } + const std::string getCombinedErrors() const; enum TypeAnnotationOption { includeTypeAnnotations, @@ -74,16 +75,21 @@ public: }; /* - Parse the given style-spec JSON value into an Expression object. - Specifically, this function is responsible for determining the expression - type (either Literal, or the one named in value[0]) and dispatching to the - appropriate ParseXxxx::parse(const V&, ParsingContext) method. + Parse the given style-spec JSON value as an expression. */ - ParseResult parse(const mbgl::style::conversion::Convertible& value, - TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations); + ParseResult parseExpression(const mbgl::style::conversion::Convertible& value, + TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations); /* - Parse a child expression. + Parse the given style-spec JSON value as an expression intended to be used + in a layout or paint property. This entails checking additional constraints + that exist in that context but not, e.g., for filters. + */ + ParseResult parseLayerPropertyExpression(const mbgl::style::conversion::Convertible& value, + TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations); + + /* + Parse a child expression. For use by individual Expression::parse() methods. */ ParseResult parse(const mbgl::style::conversion::Convertible&, std::size_t, @@ -91,7 +97,7 @@ public: TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations); /* - Parse a child expression. + Parse a child expression. For use by individual Expression::parse() methods. */ ParseResult parse(const mbgl::style::conversion::Convertible&, std::size_t index, @@ -141,6 +147,16 @@ private: errors(std::move(errors_)) {} + + /* + Parse the given style-spec JSON value into an Expression object. + Specifically, this function is responsible for determining the expression + type (either Literal, or the one named in value[0]) and dispatching to the + appropriate ParseXxxx::parse(const V&, ParsingContext) method. + */ + ParseResult parse(const mbgl::style::conversion::Convertible& value, + TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations); + std::string key; optional<type::Type> expected; std::shared_ptr<detail::Scope> scope; diff --git a/platform/node/src/node_expression.cpp b/platform/node/src/node_expression.cpp index 84515060a3..27866ccbed 100644 --- a/platform/node/src/node_expression.cpp +++ b/platform/node/src/node_expression.cpp @@ -75,8 +75,8 @@ void NodeExpression::Parse(const Nan::FunctionCallbackInfo<v8::Value>& info) { auto expr = info[0]; try { - ParsingContext ctx(expected); - ParseResult parsed = ctx.parse(mbgl::style::conversion::Convertible(expr)); + ParsingContext ctx = expected ? ParsingContext(*expected) : ParsingContext(); + ParseResult parsed = ctx.parseLayerPropertyExpression(mbgl::style::conversion::Convertible(expr)); if (parsed) { assert(ctx.getErrors().size() == 0); auto nodeExpr = new NodeExpression(std::move(*parsed)); diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp index fba149da12..3c941945fd 100644 --- a/src/mbgl/style/conversion/filter.cpp +++ b/src/mbgl/style/conversion/filter.cpp @@ -2,7 +2,7 @@ #include <mbgl/util/geometry.hpp> #include <mbgl/style/expression/expression.hpp> #include <mbgl/style/expression/type.hpp> -#include <mbgl/style/conversion/expression.hpp> +#include <mbgl/style/expression/parsing_context.hpp> namespace mbgl { namespace style { @@ -236,10 +236,13 @@ optional<Filter> convertCompoundFilter(const Convertible& value, Error& error) { } optional<Filter> convertExpressionFilter(const Convertible& value, Error& error) { - optional<std::unique_ptr<Expression>> expression = convert<std::unique_ptr<Expression>>(value, error, expression::type::Boolean); + expression::ParsingContext ctx(expression::type::Boolean); + expression::ParseResult expression = ctx.parseExpression(value); if (!expression) { + error = { ctx.getCombinedErrors() }; return {}; } + return { ExpressionFilter { std::move(*expression) } }; } diff --git a/src/mbgl/style/expression/parsing_context.cpp b/src/mbgl/style/expression/parsing_context.cpp index 713c0b5d27..b522aeff9a 100644 --- a/src/mbgl/style/expression/parsing_context.cpp +++ b/src/mbgl/style/expression/parsing_context.cpp @@ -206,8 +206,16 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption } } - // if this is the root expression, enforce constraints on the use ["zoom"]. - if (key.size() == 0 && !isZoomConstant(**parsed)) { + return parsed; +} + +ParseResult ParsingContext::parseExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) { + return parse(value, typeAnnotationOption); +} + +ParseResult ParsingContext::parseLayerPropertyExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) { + ParseResult parsed = parse(value, typeAnnotationOption); + if (parsed && !isZoomConstant(**parsed)) { optional<variant<const InterpolateBase*, const Step*, ParsingError>> zoomCurve = findZoomCurve(parsed->get()); if (!zoomCurve) { error(R"("zoom" expression may only be used as input to a top-level "step" or "interpolate" expression.)"); @@ -217,10 +225,23 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption return ParseResult(); } } - return parsed; } +const std::string ParsingContext::getCombinedErrors() const { + std::string combinedError; + for (const ParsingError& parsingError : *errors) { + if (combinedError.size() > 0) { + combinedError += "\n"; + } + if (parsingError.key.size() > 0) { + combinedError += parsingError.key + ": "; + } + combinedError += parsingError.message; + } + return combinedError; +} + optional<std::string> ParsingContext::checkType(const type::Type& t) { assert(expected); optional<std::string> err = type::checkSubtype(*expected, t); diff --git a/test/style/expression/expression.test.cpp b/test/style/expression/expression.test.cpp index 694569695c..fe5c261be1 100644 --- a/test/style/expression/expression.test.cpp +++ b/test/style/expression/expression.test.cpp @@ -49,7 +49,7 @@ TEST_P(ExpressionEqualityTest, ExpressionEquality) { assert(!document.HasParseError()); const JSValue* expression = &document; expression::ParsingContext ctx; - expression::ParseResult parsed = ctx.parse(conversion::Convertible(expression)); + expression::ParseResult parsed = ctx.parseExpression(conversion::Convertible(expression)); if (!parsed) { error_ = ctx.getErrors().size() > 0 ? ctx.getErrors()[0].message : "failed to parse"; }; diff --git a/test/style/filter.test.cpp b/test/style/filter.test.cpp index 6f261c43ec..49edcaef45 100644 --- a/test/style/filter.test.cpp +++ b/test/style/filter.test.cpp @@ -11,14 +11,19 @@ using namespace mbgl; using namespace mbgl::style; -bool filter(const char * json, const PropertyMap& featureProperties = {{}}, optional<FeatureIdentifier> featureId = {}, FeatureType featureType = FeatureType::Point, GeometryCollection featureGeometry = {}) { +bool filter(const char * json, + const PropertyMap& featureProperties = {{}}, + optional<FeatureIdentifier> featureId = {}, + FeatureType featureType = FeatureType::Point, + GeometryCollection featureGeometry = {}, + float zoom = 0.0f) { conversion::Error error; optional<Filter> filter = conversion::convertJSON<Filter>(json, error); EXPECT_TRUE(bool(filter)); EXPECT_EQ(error.message, ""); StubGeometryTileFeature feature { featureId, featureType, featureGeometry, featureProperties }; - expression::EvaluationContext context = { &feature }; + expression::EvaluationContext context = { zoom, &feature }; return (*filter)(context); } @@ -109,3 +114,8 @@ TEST(Filter, PropertyExpression) { ASSERT_TRUE(filter("[\"==\", [\"get\", \"two\"], 2]", {{"two", int64_t(2)}})); ASSERT_FALSE(filter("[\"==\", [\"get\", \"two\"], 4]", {{"two", int64_t(2)}})); } + +TEST(Filter, ZoomExpressionNested) { + ASSERT_TRUE(filter(R"(["==", ["get", "two"], ["zoom"]])", {{"two", int64_t(2)}}, {}, FeatureType::Point, {}, 2.0f)); + ASSERT_FALSE(filter(R"(["==", ["get", "two"], ["+", ["zoom"], 1]])", {{"two", int64_t(2)}}, {}, FeatureType::Point, {}, 2.0f)); +} |