Skip to content

Commit

Permalink
For string-valued properties, do coercion rather than assertion
Browse files Browse the repository at this point in the history
  • Loading branch information
jfirebaugh committed Sep 13, 2018
1 parent f42af5b commit 4256281
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 45 deletions.
14 changes: 11 additions & 3 deletions docs/components/expression-metadata.js
Expand Up @@ -65,14 +65,22 @@ const types = {
type: 'object',
parameters: ['value', { repeat: [ 'fallback: value' ] }]
}],
'to-number': [{
type: 'number',
parameters: ['value', { repeat: [ 'fallback: value' ] }]
'to-boolean': [{
type: 'boolean',
parameters: ['value']
}],
'to-color': [{
type: 'color',
parameters: ['value', { repeat: [ 'fallback: value' ] }]
}],
'to-number': [{
type: 'number',
parameters: ['value', { repeat: [ 'fallback: value' ] }]
}],
'to-string': [{
type: 'string',
parameters: ['value']
}],
at: [{
type: 'ItemType',
parameters: ['number', 'array']
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/expression/definitions/coalesce.js
Expand Up @@ -31,7 +31,7 @@ class Coalesce implements Expression {
const parsedArgs = [];

for (const arg of args.slice(1)) {
const parsed = context.parse(arg, 1 + parsedArgs.length, outputType, undefined, {omitTypeAnnotations: true});
const parsed = context.parse(arg, 1 + parsedArgs.length, outputType, undefined, {typeAnnotation: 'omit'});
if (!parsed) return null;
outputType = outputType || parsed.type;
parsedArgs.push(parsed);
Expand Down
19 changes: 14 additions & 5 deletions src/style-spec/expression/definitions/coercion.js
Expand Up @@ -2,8 +2,8 @@

import assert from 'assert';

import { ColorType, ValueType, NumberType } from '../types';
import { Color, validateRGBA } from '../values';
import {BooleanType, ColorType, NumberType, StringType, ValueType} from '../types';
import {Color, toString as valueToString, validateRGBA} from '../values';
import RuntimeError from '../runtime_error';

import type { Expression } from '../expression';
Expand All @@ -14,8 +14,10 @@ import type { Type } from '../types';
import { Formatted, FormattedSection } from './formatted';

const types = {
'to-boolean': BooleanType,
'to-color': ColorType,
'to-number': NumberType,
'to-color': ColorType
'to-string': StringType
};

/**
Expand All @@ -41,6 +43,9 @@ class Coercion implements Expression {
const name: string = (args[0]: any);
assert(types[name], name);

if ((name === 'to-boolean' || name === 'to-string') && args.length !== 2)
return context.error(`Expected one argument.`);

const type = types[name];

const parsed = [];
Expand All @@ -54,7 +59,9 @@ class Coercion implements Expression {
}

evaluate(ctx: EvaluationContext) {
if (this.type.kind === 'color') {
if (this.type.kind === 'boolean') {
return Boolean(this.args[0].evaluate(ctx));
} else if (this.type.kind === 'color') {
let input;
let error;
for (const arg of this.args) {
Expand Down Expand Up @@ -86,7 +93,7 @@ class Coercion implements Expression {
}
}
throw new RuntimeError(`Could not parse formatted text from value '${typeof input === 'string' ? input : JSON.stringify(input)}'`);
} else {
} else if (this.type.kind === 'number') {
let value = null;
for (const arg of this.args) {
value = arg.evaluate(ctx);
Expand All @@ -96,6 +103,8 @@ class Coercion implements Expression {
return num;
}
throw new RuntimeError(`Could not convert ${JSON.stringify(value)} to number.`);
} else {
return valueToString(this.args[0].evaluate(ctx));
}
}

Expand Down
12 changes: 2 additions & 10 deletions src/style-spec/expression/definitions/index.js
Expand Up @@ -69,8 +69,10 @@ const expressions: ExpressionRegistry = {
'object': Assertion,
'step': Step,
'string': Assertion,
'to-boolean': Coercion,
'to-color': Coercion,
'to-number': Coercion,
'to-string': Coercion,
'var': Var
};

Expand Down Expand Up @@ -121,16 +123,6 @@ CompoundExpression.register(expressions, {
[ValueType],
(ctx, [v]) => typeToString(typeOf(v.evaluate(ctx)))
],
'to-string': [
StringType,
[ValueType],
(ctx, [v]) => valueToString(v.evaluate(ctx))
],
'to-boolean': [
BooleanType,
[ValueType],
(ctx, [v]) => Boolean(v.evaluate(ctx))
],
'to-rgba': [
array(NumberType, 4),
[ColorType],
Expand Down
6 changes: 5 additions & 1 deletion src/style-spec/expression/index.js
Expand Up @@ -116,7 +116,11 @@ export function isExpression(expression: mixed) {
*/
export function createExpression(expression: mixed, propertySpec: StylePropertySpecification): Result<StyleExpression, Array<ParsingError>> {
const parser = new ParsingContext(definitions, [], getExpectedType(propertySpec));
const parsed = parser.parse(expression);

// For string-valued properties, coerce to string at the top level rather than asserting.
const parsed = parser.parse(expression, undefined, undefined, undefined,
propertySpec.type === 'string' ? {typeAnnotation: 'coerce'} : undefined);

if (!parsed) {
assert(parser.errors.length > 0);
return error(parser.errors);
Expand Down
42 changes: 23 additions & 19 deletions src/style-spec/expression/parsing_context.js
Expand Up @@ -61,20 +61,29 @@ class ParsingContext {
index?: number,
expectedType?: ?Type,
bindings?: Array<[string, Expression]>,
options: {omitTypeAnnotations?: boolean} = {}
options: {typeAnnotation?: 'assert' | 'coerce' | 'omit'} = {}
): ?Expression {
if (index) {
return this.concat(index, expectedType, bindings)._parse(expr, options);
}
return this._parse(expr, options);
}

_parse(expr: mixed, options: {omitTypeAnnotations?: boolean}): ?Expression {

_parse(expr: mixed, options: {typeAnnotation?: 'assert' | 'coerce' | 'omit'}): ?Expression {
if (expr === null || typeof expr === 'string' || typeof expr === 'boolean' || typeof expr === 'number') {
expr = ['literal', expr];
}

function annotate(parsed, type, typeAnnotation: 'assert' | 'coerce' | 'omit') {
if (typeAnnotation === 'assert') {
return new Assertion(type, [parsed]);
} else if (typeAnnotation === 'coerce') {
return new Coercion(type, [parsed]);
} else {
return parsed;
}
}

if (Array.isArray(expr)) {
if (expr.length === 0) {
return this.error(`Expected an array with at least one element. If you wanted a literal array, use ["literal", []].`);
Expand All @@ -95,24 +104,19 @@ class ParsingContext {
const expected = this.expectedType;
const actual = parsed.type;

// When we expect a number, string, boolean, or array but
// have a Value, we can wrap it in a refining assertion.
// When we expect a Color but have a String or Value, we
// can wrap it in "to-color" coercion.
// 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.
//
if ((expected.kind === 'string' || expected.kind === 'number' || expected.kind === 'boolean' || expected.kind === 'object' || expected.kind === 'array') && actual.kind === 'value') {
if (!options.omitTypeAnnotations) {
parsed = new Assertion(expected, [parsed]);
}
} else if (expected.kind === 'color' && (actual.kind === 'value' || actual.kind === 'string')) {
if (!options.omitTypeAnnotations) {
parsed = new Coercion(expected, [parsed]);
}
} else if (expected.kind === 'formatted' && (actual.kind === 'value' || actual.kind === 'string')) {
if (!options.omitTypeAnnotations) {
parsed = new Coercion(expected, [parsed]);
}
} else if (this.checkSubtype(this.expectedType, parsed.type)) {
parsed = annotate(parsed, expected, options.typeAnnotation || 'assert');
} else if ((expected.kind === 'color' || expected.kind === 'formatted') && (actual.kind === 'value' || actual.kind === 'string')) {
parsed = annotate(parsed, expected, options.typeAnnotation || 'coerce');
} else if (this.checkSubtype(expected, actual)) {
return null;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/style-spec/function/convert.js
Expand Up @@ -36,7 +36,9 @@ function convertIdentityFunction(parameters, propertySpec): Array<mixed> {
const get = ['get', parameters.property];

if (parameters.default === undefined) {
return get;
// By default, expressions for string-valued properties get coerced. To preserve
// legacy function semantics, insert an explicit assertion instead.
return propertySpec.type === 'string' ? ['string', get] : get;
} else if (propertySpec.type === 'enum') {
return [
'match',
Expand Down
Expand Up @@ -17,11 +17,9 @@
"outputs": [
"one",
"two",
{
"error": "Expected value to be of type string, but found number instead."
},
{"error": "Expected value to be of type string, but found null instead."}
"5",
""
],
"serialized": ["string", ["coalesce", ["get", "a"], ["get", "b"]]]
"serialized": ["to-string", ["coalesce", ["get", "a"], ["get", "b"]]]
}
}
10 changes: 10 additions & 0 deletions test/integration/expression-tests/to-boolean/2-ary/test.json
@@ -0,0 +1,10 @@
{
"expression": ["to-boolean", ["get", "x"], ["get", "y"]],
"inputs": [[{}, {}]],
"expected": {
"compiled": {
"result": "error",
"errors": [{"key": "", "error": "Expected one argument."}]
}
}
}
10 changes: 10 additions & 0 deletions test/integration/expression-tests/to-string/2-ary/test.json
@@ -0,0 +1,10 @@
{
"expression": ["to-string", ["get", "x"], ["get", "y"]],
"inputs": [[{}, {}]],
"expected": {
"compiled": {
"result": "error",
"errors": [{"key": "", "error": "Expected one argument."}]
}
}
}
24 changes: 24 additions & 0 deletions test/integration/expression-tests/to-string/implicit/test.json
@@ -0,0 +1,24 @@
{
"expression": ["get", "p"],
"propertySpec": {
"type": "string"
},
"inputs": [
[{}, {"properties": {}}],
[{}, {"properties": {"p": 0}}],
[{}, {"properties": {"p": "a"}}]
],
"expected": {
"compiled": {
"result": "success",
"isFeatureConstant": false,
"isZoomConstant": true,
"type": "string"
},
"outputs": ["", "0", "a"],
"serialized": [
"to-string",
["get", "p"]
]
}
}

0 comments on commit 4256281

Please sign in to comment.