[core] For string-valued properties, do coercion rather than assertion

This commit is contained in:
John Firebaugh
2018-09-13 15:03:19 -07:00
committed by Chris Loer
parent 89b075f592
commit 2e93a9ddcf
9 changed files with 89 additions and 51 deletions

View File

@@ -52,6 +52,25 @@ public:
} // namespace detail
/*
Controls the annotation behavior of the parser when encountering an expression
whose type is not a subtype of the expected type. The default behavior, used
when optional<TypeAnnotationOption> is a nullopt, is as follows:
When we expect a number, string, boolean, or array but have a value, wrap it in an assertion.
When we expect a color or formatted string, but have a string or value, wrap it in a coercion.
Otherwise, we do static type-checking.
These behaviors are overridable for:
* The "coalesce" operator, which needs to omit type annotations.
* String-valued properties (e.g. `text-field`), where coercion is more convenient than assertion.
*/
enum class TypeAnnotationOption {
coerce,
assert,
omit
};
class ParsingContext {
public:
ParsingContext() : errors(std::make_shared<std::vector<ParsingError>>()) {}
@@ -70,24 +89,18 @@ public:
const std::vector<ParsingError>& getErrors() const { return *errors; }
const std::string getCombinedErrors() const;
enum TypeAnnotationOption {
includeTypeAnnotations,
omitTypeAnnotations
};
/*
Parse the given style-spec JSON value as an expression.
*/
ParseResult parseExpression(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
optional<TypeAnnotationOption> = {});
/*
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);
ParseResult parseLayerPropertyExpression(const mbgl::style::conversion::Convertible& value);
/*
Parse a child expression. For use by individual Expression::parse() methods.
@@ -95,7 +108,7 @@ public:
ParseResult parse(const mbgl::style::conversion::Convertible&,
std::size_t,
optional<type::Type> = {},
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
optional<TypeAnnotationOption> = {});
/*
Parse a child expression. For use by individual Expression::parse() methods.
@@ -156,7 +169,7 @@ private:
appropriate ParseXxxx::parse(const V&, ParsingContext) method.
*/
ParseResult parse(const mbgl::style::conversion::Convertible& value,
TypeAnnotationOption typeAnnotationOption = includeTypeAnnotations);
optional<TypeAnnotationOption> = {});
std::string key;
optional<type::Type> expected;

View File

@@ -0,0 +1,3 @@
{
"core": "Arguments to the `concat` expression operator are now automatically converted to string values as if via `to-string`, as are values for the `text-field`, `icon-image`, and `*-pattern` properties. Previously, such values were required to be strings, and if they were not, an error occurred."
}

View File

@@ -52,7 +52,7 @@ std::unique_ptr<Expression> convertTokenStringToExpression(const std::string& so
if (pos != end) {
for (brace++; brace != end && tokenReservedChars.find(*brace) == std::string::npos; brace++);
if (brace != end && *brace == '}') {
inputs.push_back(toString(get(literal(std::string(pos + 1, brace)))));
inputs.push_back(get(literal(std::string(pos + 1, brace))));
pos = brace + 1;
} else {
inputs.push_back(literal(std::string(pos, brace)));
@@ -65,7 +65,7 @@ std::unique_ptr<Expression> convertTokenStringToExpression(const std::string& so
case 0:
return literal(source);
case 1:
return std::move(inputs[0]);
return toString(std::move(inputs[0]));
default:
return concat(std::move(inputs));
}

View File

@@ -57,7 +57,7 @@ ParseResult Coalesce::parse(const Convertible& value, ParsingContext& ctx) {
Coalesce::Args args;
args.reserve(length - 1);
for (std::size_t i = 1; i < length; i++) {
auto parsed = ctx.parse(arrayMember(value, i), i, outputType, ParsingContext::omitTypeAnnotations);
auto parsed = ctx.parse(arrayMember(value, i), i, outputType, TypeAnnotationOption::omit);
if (!parsed) {
return parsed;
}

View File

@@ -8,6 +8,16 @@ namespace mbgl {
namespace style {
namespace expression {
EvaluationResult toBoolean(const Value& v) {
return v.match(
[&] (double f) { return static_cast<bool>(f); },
[&] (const std::string& s) { return s.length() > 0; },
[&] (bool b) { return b; },
[&] (const NullValue&) { return false; },
[&] (const auto&) { return true; }
);
}
EvaluationResult toNumber(const Value& v) {
optional<double> result = v.match(
[](NullValue) -> optional<double> { return 0.0; },
@@ -78,10 +88,14 @@ Coercion::Coercion(type::Type type_, std::vector<std::unique_ptr<Expression>> in
{
assert(!inputs.empty());
type::Type t = getType();
if (t.is<type::NumberType>()) {
coerceSingleValue = toNumber;
if (t.is<type::BooleanType>()) {
coerceSingleValue = toBoolean;
} else if (t.is<type::ColorType>()) {
coerceSingleValue = toColor;
} else if (t.is<type::NumberType>()) {
coerceSingleValue = toNumber;
} else if (t.is<type::StringType>()) {
coerceSingleValue = [] (const Value& v) -> EvaluationResult { return toString(v); };
} else {
assert(false);
}
@@ -89,16 +103,20 @@ Coercion::Coercion(type::Type type_, std::vector<std::unique_ptr<Expression>> in
std::string Coercion::getOperator() const {
return getType().match(
[](const type::NumberType&) { return "to-number"; },
[](const type::BooleanType&) { return "to-boolean"; },
[](const type::ColorType&) { return "to-color"; },
[](const type::NumberType&) { return "to-number"; },
[](const type::StringType&) { return "to-string"; },
[](const auto&) { assert(false); return ""; });
}
using namespace mbgl::style::conversion;
ParseResult Coercion::parse(const Convertible& value, ParsingContext& ctx) {
static std::unordered_map<std::string, type::Type> types {
{"to-boolean", type::Boolean},
{"to-color", type::Color},
{"to-number", type::Number},
{"to-color", type::Color}
{"to-string", type::String}
};
std::size_t length = arrayLength(value);
@@ -110,7 +128,12 @@ ParseResult Coercion::parse(const Convertible& value, ParsingContext& ctx) {
auto it = types.find(*toString(arrayMember(value, 0)));
assert(it != types.end());
if ((it->second == type::Boolean || it->second == type::String) && length != 2) {
ctx.error("Expected one argument.");
return ParseResult();
}
std::vector<std::unique_ptr<Expression>> parsed;
parsed.reserve(length - 1);
for (std::size_t i = 1; i < length; i++) {

View File

@@ -309,19 +309,6 @@ std::unordered_map<std::string, CompoundExpressionRegistry::Definition> initiali
define("typeof", [](const Value& v) -> Result<std::string> { return toString(typeOf(v)); });
define("to-string", [](const Value& value) -> Result<std::string> {
return toString(value);
});
define("to-boolean", [](const Value& v) -> Result<bool> {
return v.match(
[&] (double f) { return static_cast<bool>(f); },
[&] (const std::string& s) { return s.length() > 0; },
[&] (bool b) { return b; },
[&] (const NullValue&) { return false; },
[&] (const auto&) { return true; }
);
});
define("to-rgba", [](const Color& color) -> Result<std::array<double, 4>> {
return color.toArray();
});

View File

@@ -74,7 +74,7 @@ std::unique_ptr<Expression> toColor(std::unique_ptr<Expression> value) {
}
std::unique_ptr<Expression> toString(std::unique_ptr<Expression> value) {
return compound("to-string", std::move(value));
return std::make_unique<Coercion>(type::String, vec(std::move(value)));
}
std::unique_ptr<Expression> get(const char* value) {

View File

@@ -21,6 +21,7 @@
#include <mbgl/style/expression/step.hpp>
#include <mbgl/style/expression/find_zoom_curve.hpp>
#include <mbgl/style/expression/dsl.hpp>
#include <mbgl/style/conversion/get_json_type.hpp>
#include <mbgl/style/conversion_impl.hpp>
@@ -76,7 +77,7 @@ using namespace mbgl::style::conversion;
ParseResult ParsingContext::parse(const Convertible& value,
std::size_t index_,
optional<type::Type> expected_,
TypeAnnotationOption typeAnnotationOption) {
optional<TypeAnnotationOption> typeAnnotationOption) {
ParsingContext child(key + "[" + util::toString(index_) + "]",
errors,
std::move(expected_),
@@ -118,14 +119,16 @@ const ExpressionRegistry& getExpressionRegistry() {
{"object", Assertion::parse},
{"step", Step::parse},
{"string", Assertion::parse},
{"to-boolean", Coercion::parse},
{"to-color", Coercion::parse},
{"to-number", Coercion::parse},
{"to-string", Coercion::parse},
{"var", Var::parse}
}};
return registry;
}
ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
ParseResult ParsingContext::parse(const Convertible& value, optional<TypeAnnotationOption> typeAnnotationOption) {
ParseResult parsed;
if (isArray(value)) {
@@ -161,22 +164,27 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption
return parsed;
}
auto array = [&](std::unique_ptr<Expression> expression) {
std::vector<std::unique_ptr<Expression>> args;
args.push_back(std::move(expression));
return args;
auto annotate = [] (std::unique_ptr<Expression> expression, type::Type type, TypeAnnotationOption typeAnnotation) -> std::unique_ptr<Expression> {
switch (typeAnnotation) {
case TypeAnnotationOption::assert:
return std::make_unique<Assertion>(type, dsl::vec(std::move(expression)));
case TypeAnnotationOption::coerce:
return std::make_unique<Coercion>(type, dsl::vec(std::move(expression)));
case TypeAnnotationOption::omit:
return expression;
}
// Not reachable, but placate GCC.
assert(false);
return expression;
};
if (expected) {
const type::Type actual = (*parsed)->getType();
if ((*expected == type::String || *expected == type::Number || *expected == type::Boolean || *expected == type::Object || expected->is<type::Array>()) && actual == type::Value) {
if (typeAnnotationOption == includeTypeAnnotations) {
parsed = { std::make_unique<Assertion>(*expected, array(std::move(*parsed))) };
}
parsed = { annotate(std::move(*parsed), *expected, typeAnnotationOption.value_or(TypeAnnotationOption::assert)) };
} else if (*expected == type::Color && (actual == type::Value || actual == type::String)) {
if (typeAnnotationOption == includeTypeAnnotations) {
parsed = { std::make_unique<Coercion>(*expected, array(std::move(*parsed))) };
}
parsed = { annotate(std::move(*parsed), *expected, typeAnnotationOption.value_or(TypeAnnotationOption::coerce)) };
} else {
checkType((*parsed)->getType());
if (errors->size() > 0) {
@@ -212,11 +220,15 @@ ParseResult ParsingContext::parse(const Convertible& value, TypeAnnotationOption
return parsed;
}
ParseResult ParsingContext::parseExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
ParseResult ParsingContext::parseExpression(const Convertible& value, optional<TypeAnnotationOption> typeAnnotationOption) {
return parse(value, typeAnnotationOption);
}
ParseResult ParsingContext::parseLayerPropertyExpression(const Convertible& value, TypeAnnotationOption typeAnnotationOption) {
ParseResult ParsingContext::parseLayerPropertyExpression(const Convertible& value) {
optional<TypeAnnotationOption> typeAnnotationOption;
if (expected && *expected == type::String) {
typeAnnotationOption = TypeAnnotationOption::coerce;
}
ParseResult parsed = parse(value, typeAnnotationOption);
if (parsed && !isZoomConstant(**parsed)) {
optional<variant<const Interpolate*, const Step*, ParsingError>> zoomCurve = findZoomCurve(parsed->get());

View File

@@ -86,9 +86,9 @@ TEST(StyleConversion, TokenStrings) {
using namespace mbgl::style::expression::dsl;
ASSERT_EQ(*convertTokenStringToExpression("{token}"), *toString(get(literal("token"))));
ASSERT_EQ(*convertTokenStringToExpression("token {token}"), *concat(vec(literal("token "), toString(get(literal("token"))))));
ASSERT_EQ(*convertTokenStringToExpression("{token} token"), *concat(vec(toString(get(literal("token"))), literal(" token"))));
ASSERT_EQ(*convertTokenStringToExpression("{token} {token}"), *concat(vec(toString(get(literal("token"))), literal(" "), toString(get(literal("token"))))));
ASSERT_EQ(*convertTokenStringToExpression("{token} {token"), *concat(vec(toString(get(literal("token"))), literal(" "), literal("{token"))));
ASSERT_EQ(*convertTokenStringToExpression("{token {token}"), *concat(vec(literal("{token "), toString(get(literal("token"))))));
ASSERT_EQ(*convertTokenStringToExpression("token {token}"), *concat(vec(literal("token "), get(literal("token")))));
ASSERT_EQ(*convertTokenStringToExpression("{token} token"), *concat(vec(get(literal("token")), literal(" token"))));
ASSERT_EQ(*convertTokenStringToExpression("{token} {token}"), *concat(vec(get(literal("token")), literal(" "), get(literal("token")))));
ASSERT_EQ(*convertTokenStringToExpression("{token} {token"), *concat(vec(get(literal("token")), literal(" "), literal("{token"))));
ASSERT_EQ(*convertTokenStringToExpression("{token {token}"), *concat(vec(literal("{token "), get(literal("token")))));
}