From f407f502374f1da504ca7aa949cf8597c93346c6 Mon Sep 17 00:00:00 2001 From: Lucas Wojciechowski Date: Thu, 19 Apr 2018 17:02:24 -0700 Subject: Misc code cleanup --- include/mbgl/style/filter.hpp | 2 +- .../xcshareddata/IDEWorkspaceChecks.plist | 8 - src/mbgl/style/conversion/filter.cpp | 176 ++++++++++----------- src/mbgl/style/filter.cpp | 5 +- test/style/conversion/stringify.test.cpp | 6 + 5 files changed, 89 insertions(+), 108 deletions(-) delete mode 100644 platform/macos/macos.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist diff --git a/include/mbgl/style/filter.hpp b/include/mbgl/style/filter.hpp index b932e06f51..5034b33bd8 100644 --- a/include/mbgl/style/filter.hpp +++ b/include/mbgl/style/filter.hpp @@ -20,7 +20,7 @@ public: friend bool operator==(const Filter& lhs, const Filter& rhs) { if (!lhs.expression || !rhs.expression) { - return ((bool) lhs.expression) == ((bool) rhs.expression); + return lhs.expression == rhs.expression; } else { return *(lhs.expression) == *(rhs.expression); } diff --git a/platform/macos/macos.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist b/platform/macos/macos.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist deleted file mode 100644 index 18d981003d..0000000000 --- a/platform/macos/macos.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist +++ /dev/null @@ -1,8 +0,0 @@ - - - - - IDEDidComputeMac32BitWarning - - - diff --git a/src/mbgl/style/conversion/filter.cpp b/src/mbgl/style/conversion/filter.cpp index 7b0540dca2..501556175a 100644 --- a/src/mbgl/style/conversion/filter.cpp +++ b/src/mbgl/style/conversion/filter.cpp @@ -10,13 +10,31 @@ namespace mbgl { namespace style { namespace conversion { -// TODO factor this out -using namespace mbgl::style::expression; +static bool isExpression(const Convertible& filter); + std::unique_ptr convertLegacyFilter(const Convertible& values, Error& error); -using GeometryValue = mapbox::geometry::value; +optional Converter::operator()(const Convertible& value, Error& error) const { + + optional> expression; + + if (isExpression(value)) { + expression::ParsingContext parsingContext(expression::type::Boolean); + expression::ParseResult parseResult = parsingContext.parseExpression(value); + if (!parseResult) { + error = { parsingContext.getCombinedErrors() }; + return {}; + } + expression = std::move(*parseResult); + } else { + expression = convertLegacyFilter(value, error); + } + + if (!expression) return {}; + else return { Filter { std::move(*expression) } }; +} // This is a port from https://github.com/mapbox/mapbox-gl-js/blob/master/src/style-spec/feature_filter/index.js -static bool isExpressionFilter(const Convertible& filter) { +static bool isExpression(const Convertible& filter) { if (!isArray(filter) || arrayLength(filter) == 0) { return false; } @@ -40,7 +58,7 @@ static bool isExpressionFilter(const Convertible& filter) { } else if (*op == "any" || *op == "all") { for (std::size_t i = 1; i < arrayLength(filter); i++) { Convertible f = arrayMember(filter, i); - if (!isExpressionFilter(f) && !toBool(f)) { + if (!isExpression(f) && !toBool(f)) { return false; } } @@ -50,38 +68,19 @@ static bool isExpressionFilter(const Convertible& filter) { return true; } } - -static bool isLegacyFilter(const Convertible& filter) { - return !isExpressionFilter(filter); -} - -std::unique_ptr createLiteralExpression(const Convertible& value) { - // TODO handle null case - return std::make_unique(toExpressionValue(*toValue(value))); -} - -std::vector> createLiteralExpressionArray(const Convertible &input, std::size_t startIndex = 0) { - std::vector> output; - for (std::size_t i = startIndex; i < arrayLength(input); i++) { - output.push_back(createLiteralExpression(arrayMember(input, i))); - } - return output; -} - -std::unique_ptr createExpression(std::string op, std::vector> args, Error& error) { +std::unique_ptr createExpression(std::string op, std::vector> args, Error& error) { if (op == "any") { - return std::make_unique(std::move(args)); + return std::make_unique(std::move(args)); } else if (op == "all") { - return std::make_unique(std::move(args)); + return std::make_unique(std::move(args)); } else { - ParsingContext context; - ParseResult parseResult = createCompoundExpression(op, std::move(args), context); - - if (!parseResult || context.getErrors().size() > 0) { - error = { context.getErrors()[0].message }; + expression::ParsingContext parsingContext(expression::type::Boolean); + expression::ParseResult parseResult = createCompoundExpression(op, std::move(args), parsingContext); + if (!parseResult) { + error = { parsingContext.getCombinedErrors() }; return {}; } else { return std::move(*parseResult); @@ -89,40 +88,27 @@ std::unique_ptr createExpression(std::string op, std::vector convertLegacyFilter(const Convertible& values, Error& error); +std::unique_ptr createExpression(std::string op, std::unique_ptr expression, Error& error) { + std::vector> args; + args.push_back(std::move(expression)); + return createExpression(op, std::move(args), error); +} + +std::unique_ptr convertLiteral(const Convertible& convertible) { + optional value = toValue(convertible); + expression::Value expressionValue = value ? expression::toExpressionValue(*value) : expression::Null; + return std::make_unique(expressionValue); +} -std::vector> createLegacyFilter2Array(const Convertible &input, Error& error, std::size_t startIndex = 0) { - std::vector> output; +std::vector> convertLiteralArray(const Convertible &input, std::size_t startIndex = 0) { + std::vector> output; for (std::size_t i = startIndex; i < arrayLength(input); i++) { - output.push_back(convertLegacyFilter(arrayMember(input, i), error)); + output.push_back(convertLiteral(arrayMember(input, i))); } return output; } -optional Converter::operator()(const Convertible& value, Error& error) const { - - optional> expression; - - if (isLegacyFilter(value)) { - expression = convertLegacyFilter(value, error); - } else { - - expression::ParsingContext ctx(expression::type::Boolean); - expression::ParseResult parseResult = ctx.parseExpression(value); - if (!parseResult) { - error = { ctx.getCombinedErrors() }; - return {}; - } - - expression = std::move(*parseResult); - } - - if (!expression) return {}; - - return { Filter { std::move(*expression) } }; -} - - std::unique_ptr convertComparisonOp(const Convertible& values, Error& error, optional opOverride = {}) { +std::unique_ptr convertLegacyComparisonFilter(const Convertible& values, Error& error, optional opOverride = {}) { optional op = opOverride ? opOverride : toString(arrayMember(values, 0)); optional property = toString(arrayMember(values, 1)); @@ -130,83 +116,81 @@ optional Converter::operator()(const Convertible& value, Error& error = { "filter property must be a string" }; return {}; } else if (*property == "$type") { - return createExpression("filter-type-" + *op, createLiteralExpressionArray(values, 2), error); + return createExpression("filter-type-" + *op, convertLiteralArray(values, 2), error); } else if (*property == "$id") { - return createExpression("filter-id-" + *op, createLiteralExpressionArray(values, 2), error); + return createExpression("filter-id-" + *op, convertLiteralArray(values, 2), error); } else { - return createExpression("filter-" + *op, createLiteralExpressionArray(values, 1), error); + return createExpression("filter-" + *op, convertLiteralArray(values, 1), error); } } -std::unique_ptr convertHasOp(std::string property, Error& error) { +std::unique_ptr convertLegacyHasFilter(std::string property, Error& error) { if (property == "$type") { - return std::make_unique(true); + return std::make_unique(true); } else if (property == "$id") { - return createExpression("filter-has-id", {}, error); + return createExpression("filter-has-id", std::vector>(), error); } else { - std::vector> args; - args.push_back(std::make_unique(property)); - return createExpression("filter-has", std::move(args), error); + return createExpression("filter-has", std::make_unique(property), error); } } - -std::unique_ptr convertNegation(std::unique_ptr expression, Error& error) { - std::vector> args; - args.push_back(std::move(expression)); - return createExpression("!", std::move(args), error); -} -std::unique_ptr convertInOp(const Convertible& values, Error& error) { +std::unique_ptr convertLegacyInFilter(const Convertible& values, Error& error) { optional property = toString(arrayMember(values, 1)); if (!property) { error = { "filter property must be a string" }; return {}; } else if (arrayLength(values) == 0) { - return std::make_unique(false); + return std::make_unique(false); } else if (*property == "$type") { - return createExpression("filter-type-in", createLiteralExpressionArray(values, 2), error); + return createExpression("filter-type-in", convertLiteralArray(values, 2), error); } else if (*property == "id") { - return createExpression("filter-id-in", createLiteralExpressionArray(values, 2), error); + return createExpression("filter-id-in", convertLiteralArray(values, 2), error); } else { - return createExpression("filter-in", createLiteralExpressionArray(values, 1), error); + return createExpression("filter-in", convertLiteralArray(values, 1), error); } } + +std::vector> convertLegacyFilterArray(const Convertible &input, Error& error, std::size_t startIndex = 0) { + std::vector> output; + for (std::size_t i = startIndex; i < arrayLength(input); i++) { + output.push_back(convertLegacyFilter(arrayMember(input, i), error)); + } + return output; +} -std::unique_ptr convertLegacyFilter(const Convertible& values, Error& error) { - +std::unique_ptr convertLegacyFilter(const Convertible& values, Error& error) { if (isUndefined(values)) { - return std::make_unique(true); + return std::make_unique(true); } optional op = toString(arrayMember(values, 0)); - + if (!op) { error = { "filter operator must be a string" }; return {}; } else if (arrayLength(values) <= 1) { - return std::make_unique(*op != "any"); + return std::make_unique(*op != "any"); } else { return ( *op == "==" || *op == "<" || *op == ">" || *op == "<=" || - *op == ">=" ? convertComparisonOp(values, error) : - *op == "!=" ? convertNegation(convertComparisonOp(values, error, {"=="}), error) : - *op == "any" ? createExpression("any", createLegacyFilter2Array(values, error, 1), error) : - *op == "all" ? createExpression("all", createLegacyFilter2Array(values, error, 1), error) : - *op == "none" ? convertNegation(createExpression("any", createLegacyFilter2Array(values, error, 1), error), error) : - *op == "in" ? convertInOp(values, error) : - *op == "!in" ? convertNegation(convertInOp(values, error), error) : - *op == "has" ? convertHasOp(*toString(arrayMember(values, 1)), error) : - *op == "!has" ? convertNegation(convertHasOp(*toString(arrayMember(values, 1)), error), error) : - std::make_unique(true) + *op == ">=" ? convertLegacyComparisonFilter(values, error) : + *op == "!=" ? createExpression("!", convertLegacyComparisonFilter(values, error, {"=="}), error) : + *op == "any" ? createExpression("any", convertLegacyFilterArray(values, error, 1), error) : + *op == "all" ? createExpression("all", convertLegacyFilterArray(values, error, 1), error) : + *op == "none" ? createExpression("!", createExpression("any", convertLegacyFilterArray(values, error, 1), error), error) : + *op == "in" ? convertLegacyInFilter(values, error) : + *op == "!in" ? createExpression("!", convertLegacyInFilter(values, error), error) : + *op == "has" ? convertLegacyHasFilter(*toString(arrayMember(values, 1)), error) : + *op == "!has" ? createExpression("!", convertLegacyHasFilter(*toString(arrayMember(values, 1)), error), error) : + std::make_unique(true) ); } } - + } // namespace conversion } // namespace style } // namespace mbgl - diff --git a/src/mbgl/style/filter.cpp b/src/mbgl/style/filter.cpp index 0da7229e3c..a1c0902898 100644 --- a/src/mbgl/style/filter.cpp +++ b/src/mbgl/style/filter.cpp @@ -6,16 +6,15 @@ namespace style { bool Filter::operator()(const expression::EvaluationContext &context) const { - // TODO What is our fallback behvaiour? if (!this->expression) return true; const expression::EvaluationResult result = this->expression->evaluate(context); if (result) { const optional typed = expression::fromExpressionValue(*result); return typed ? *typed : false; + } else { + return true; } - return false; - } } // namespace style diff --git a/test/style/conversion/stringify.test.cpp b/test/style/conversion/stringify.test.cpp index b8f6872f77..65a665ac90 100644 --- a/test/style/conversion/stringify.test.cpp +++ b/test/style/conversion/stringify.test.cpp @@ -74,6 +74,12 @@ TEST(Stringify, Value) { ASSERT_EQ(stringify(Value(1.2)), "1.2"); } +// TODO write new test +//TEST(Stringify, Filter) { +// ASSERT_EQ(stringify(NullFilter()), "null"); +// ASSERT_EQ(stringify(EqualsFilter { "a", 1.0 }), "[\"==\",\"a\",1.0]"); +//} + TEST(Stringify, CameraFunction) { ASSERT_EQ(stringify(CameraFunction(ExponentialStops { {{0, 1}}, 1 })), "[\"interpolate\",[\"linear\"],[\"zoom\"],0.0,1.0]"); -- cgit v1.2.1