Skip to content

Commit

Permalink
Refactor some internal function signatures
Browse files Browse the repository at this point in the history
- Reverse arguments to Definition#compile()
- Refactor compile, parse to take expression definitions as argument
  • Loading branch information
Anand Thakker committed Jun 22, 2017
1 parent 0c24e22 commit 8275bb2
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 41 deletions.
12 changes: 9 additions & 3 deletions src/style-spec/function/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const evaluationContext = require('./evaluation_context');
import type { ExpressionName } from './expression_name.js';
import type { Definition } from './expressions.js';
export type CompiledExpression = {|
result: 'success',
js: string,
Expand Down Expand Up @@ -62,8 +64,12 @@ const evaluationContext = require('./evaluation_context');
*
* @private
*/
function compileExpression(expr: mixed, expectedType?: Type) {
const parsed = parseExpression(expr);
function compileExpression(
definitions: {[string]: Definition},
expr: mixed,
expectedType?: Type
) {
const parsed = parseExpression(definitions, expr);
if (parsed.error) {
return {
result: 'error',
Expand Down Expand Up @@ -134,7 +140,7 @@ function compile(expected: Type | null, e: TypedExpression) /*: CompiledExpressi
let isZoomConstant = compiledArgs.reduce((memo, arg) => memo && arg.isZoomConstant, true);

const definition = expressions[e.name];
const compiled = definition.compile(e, compiledArgs);
const compiled = definition.compile(compiledArgs, e);
if (compiled.errors) {
return {
result: 'error',
Expand Down
54 changes: 27 additions & 27 deletions src/style-spec/function/expressions.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ const {
import type { ExpressionName } from './expression_name.js';
type Definition = {
export type Definition = {
name: ExpressionName,
type: LambdaType,
compile: (expr: TypedLambdaExpression, args: Array<CompiledExpression>) => ({ js?: string, errors?: Array<string>, isFeatureConstant?: boolean, isZoomConstant?: boolean })
compile: (args: Array<CompiledExpression>, expr: TypedLambdaExpression) => ({ js?: string, errors?: Array<string>, isFeatureConstant?: boolean, isZoomConstant?: boolean })
}
*/

Expand All @@ -44,27 +44,27 @@ const expressions: { [string]: Definition } = {
'string': {
name: 'string',
type: lambda(StringType, ValueType),
compile: (_, args) => ({ js: `String(${args[0].js})` })
compile: args => ({ js: `String(${args[0].js})` })
},
'number': {
name: 'string',
type: lambda(NumberType, ValueType),
compile: (_, args) => ({js: `Number(${args[0].js})`})
compile: args => ({js: `Number(${args[0].js})`})
},
'boolean': {
name: 'boolean',
type: lambda(BooleanType, ValueType),
compile: (_, args) => ({js: `Boolean(${args[0].js})`})
compile: args => ({js: `Boolean(${args[0].js})`})
},
'json_array': {
name: 'json_array',
type: lambda(vector(ValueType), ValueType),
compile: (_, args) => ({js: `this.as(${args[0].js}, 'Vector<Value>')`})
compile: args => ({js: `this.as(${args[0].js}, 'Vector<Value>')`})
},
'object': {
name: 'object',
type: lambda(ObjectType, ValueType),
compile: (_, args) => ({js: `this.as(${args[0].js}, 'Object')`})
compile: args => ({js: `this.as(${args[0].js}, 'Object')`})
},
'color': {
name: 'color',
Expand All @@ -74,30 +74,30 @@ const expressions: { [string]: Definition } = {
'array': {
name: 'array',
type: lambda(anyArray(typename('T')), nargs(Infinity, typename('T'))),
compile: (e, args) => ({js: `this.array(${JSON.stringify(e.type.result.name)}, [${args.map(a => a.js).join(', ')}])`})
compile: (args, e) => ({js: `this.array(${JSON.stringify(e.type.result.name)}, [${args.map(a => a.js).join(', ')}])`})
},
'vector': {
name: 'vector',
type: lambda(vector(typename('T')), nargs(Infinity, typename('T'))),
compile: (e, args) => ({js: `this.vector(${JSON.stringify(e.type.result.name)}, [${args.map(a => a.js).join(', ')}])`})
compile: (args, e) => ({js: `this.vector(${JSON.stringify(e.type.result.name)}, [${args.map(a => a.js).join(', ')}])`})
},
'color_to_array': {
name: 'color_to_array',
type: lambda(array(NumberType, 4), ColorType),
compile: (_, args) => ({js: `this.array('Array<Number, 4>', ${args[0].js}.value)`})
compile: args => ({js: `this.array('Array<Number, 4>', ${args[0].js}.value)`})
},
'get': {
name: 'get',
type: lambda(ValueType, StringType, nargs(1, ObjectType)),
compile: (_, args) => ({
compile: args => ({
js: `this.get(${args.length > 1 ? args[1].js : 'props'}, ${args[0].js}, ${args.length > 1 ? 'undefined' : '"feature.properties"'})`,
isFeatureConstant: args.length > 1 && args[1].isFeatureConstant
})
},
'has': {
name: 'has',
type: lambda(BooleanType, StringType, nargs(1, ObjectType)),
compile: (_, args) => ({
compile: args => ({
js: `this.has(${args.length > 1 ? args[1].js : 'props'}, ${args[0].js}, ${args.length > 1 ? 'undefined' : '"feature.properties"'})`,
isFeatureConstant: args.length > 1 && args[1].isFeatureConstant
})
Expand All @@ -122,7 +122,7 @@ const expressions: { [string]: Definition } = {
vector(typename('T')),
StringType
)),
compile: (e, args) => {
compile: args => {
let t = args[0].type;
if (t.kind === 'lambda') { t = t.result; }
assert(t.kind === 'vector' || t.kind === 'primitive');
Expand Down Expand Up @@ -170,7 +170,7 @@ const expressions: { [string]: Definition } = {
'^': {
name: '^',
type: lambda(NumberType, NumberType, NumberType),
compile: (_, args) => ({js: `Math.pow(${args[0].js}, ${args[1].js})`})
compile: args => ({js: `Math.pow(${args[0].js}, ${args[1].js})`})
},
'log10': defineMathFunction('log10', 1),
'ln': defineMathFunction('ln', 1, 'log'),
Expand All @@ -192,22 +192,22 @@ const expressions: { [string]: Definition } = {
'!': {
name: '!',
type: lambda(BooleanType, BooleanType),
compile: (_, args) => ({js: `!(${args[0].js})`})
compile: args => ({js: `!(${args[0].js})`})
},
'upcase': {
name: 'upcase',
type: lambda(StringType, StringType),
compile: (_, args) => ({js: `(${args[0].js}).toUpperCase()`})
compile: args => ({js: `(${args[0].js}).toUpperCase()`})
},
'downcase': {
name: 'downcase',
type: lambda(StringType, StringType),
compile: (_, args) => ({js: `(${args[0].js}).toLowerCase()`})
compile: args => ({js: `(${args[0].js}).toLowerCase()`})
},
'concat': {
name: 'concat',
type: lambda(StringType, nargs(Infinity, ValueType)),
compile: (_, args) => ({js: `[${args.map(a => a.js).join(', ')}].join('')`})
compile: args => ({js: `[${args.map(a => a.js).join(', ')}].join('')`})
},
'rgb': {
name: 'rgb',
Expand All @@ -222,7 +222,7 @@ const expressions: { [string]: Definition } = {
'case': {
name: 'case',
type: lambda(typename('T'), nargs(Infinity, BooleanType, typename('T')), typename('T')),
compile: (_, args) => {
compile: args => {
args = [].concat(args);
const result = [];
while (args.length > 1) {
Expand All @@ -239,7 +239,7 @@ const expressions: { [string]: Definition } = {
// note that, since they're pulled out during parsing, the input
// values of type T aren't reflected in the signature here
type: lambda(typename('T'), typename('U'), nargs(Infinity, typename('T'))),
compile: (e, args) => {
compile: (args, e) => {
if (!e.matchInputs) { throw new Error('Missing match input values'); }
const inputs = e.matchInputs;
if (args.length !== inputs.length + 2) {
Expand Down Expand Up @@ -278,15 +278,15 @@ const expressions: { [string]: Definition } = {
'coalesce': {
name: 'coalesce',
type: lambda(typename('T'), nargs(Infinity, typename('T'))),
compile: (_, args) => ({
compile: args => ({
js: `this.coalesce(${args.map(a => `() => ${a.js}`).join(', ')})`
})
},

'curve': {
name: 'curve',
type: lambda(typename('T'), InterpolationType, NumberType, nargs(Infinity, NumberType, typename('T'))),
compile: (_, args) => {
compile: args => {
const interpolation = args[0].expression;
if (interpolation.literal) { throw new Error('Invalid interpolation type'); } // enforced by type checking

Expand Down Expand Up @@ -386,7 +386,7 @@ function defineMathFunction(name: ExpressionName, arity: number, mathName?: stri
return {
name: name,
type: lambda(NumberType, ...args),
compile: (_, args) => ({ js: `Math.${key}(${args.map(a => a.js).join(', ')})` })
compile: args => ({ js: `Math.${key}(${args.map(a => a.js).join(', ')})` })
};
}

Expand All @@ -395,7 +395,7 @@ function defineBinaryMathOp(name, isAssociative) {
return {
name: name,
type: lambda(NumberType, ...args),
compile: (_, args) => ({ js: `${args.map(a => a.js).join(name)}` })
compile: args => ({ js: `${args.map(a => a.js).join(name)}` })
};
}

Expand All @@ -405,20 +405,20 @@ function defineComparisonOp(name) {
return {
name: name,
type: lambda(BooleanType, typename('T'), typename('T')),
compile: (_, args) => ({ js: `${args[0].js} ${op} ${args[1].js}` })
compile: args => ({ js: `${args[0].js} ${op} ${args[1].js}` })
};
}

function defineBooleanOp(op) {
return {
name: op,
type: lambda(BooleanType, nargs(Infinity, BooleanType)),
compile: (_, args) => ({ js: `${args.map(a => a.js).join(op)}` })
compile: args => ({ js: `${args.map(a => a.js).join(op)}` })
};
}

function fromContext(name) {
return (_, args) => {
return args => {
const argvalues = args.map(a => a.js).join(', ');
return { js: `this.${name}(${argvalues})` };
};
Expand Down
3 changes: 2 additions & 1 deletion src/style-spec/function/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const expressions = require('./expressions');
const compileExpression = require('./compile');
const convert = require('./convert');
const {ColorType, StringType, NumberType, ValueType, array, vector} = require('./types');
Expand All @@ -20,7 +21,7 @@ function createFunction(parameters, propertySpec) {
}

const expectedType = getExpectedType(propertySpec);
const compiled = compileExpression(expr, expectedType);
const compiled = compileExpression(expressions, expr, expectedType);
if (compiled.result === 'success') {
const f = function (zoom, properties) {
const val = compiled.function({zoom}, {properties});
Expand Down
22 changes: 13 additions & 9 deletions src/style-spec/function/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ const {
array
} = require('./types');

const expressions = require('./expressions');

/*::
import type { TypeError, TypedExpression } from './type_check.js';
import type { ExpressionName } from './expression_name.js';
import type { Definition } from './expressions.js';
export type ParseError = {|
error: string,
key: string
Expand All @@ -35,7 +35,11 @@ module.exports = parseExpression;
*
* @private
*/
function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpression | ParseError */ {
function parseExpression(
definitions: {[string]: Definition},
expr: mixed,
path: Array<number> = []
) /*: TypedExpression | ParseError */ {
const key = path.join('.');
if (expr === null || typeof expr === 'undefined') return {
literal: true,
Expand Down Expand Up @@ -80,7 +84,7 @@ function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpress
};
}

const definition = expressions[op];
const definition = definitions[op];
if (!definition) {
return {
key,
Expand All @@ -95,7 +99,7 @@ function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpress
error: `Expected at least 2 arguments, but found only ${expr.length - 1}.`
};

const inputExpression = parseExpression(expr[1], path.concat(1));
const inputExpression = parseExpression(definitions, expr[1], path.concat(1));
if (inputExpression.error) return inputExpression;

// parse input/output pairs.
Expand All @@ -112,7 +116,7 @@ function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpress

const parsedInputGroup = [];
for (let j = 0; j < inputGroup.length; j++) {
const parsedValue = parseExpression(inputGroup[j], path.concat(i, j));
const parsedValue = parseExpression(definitions, inputGroup[j], path.concat(i, j));
if (parsedValue.error) return parsedValue;
if (!parsedValue.literal) return {
key: `${key}.${i}.${j}`,
Expand All @@ -122,12 +126,12 @@ function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpress
}
matchInputs.push(parsedInputGroup);

const output = parseExpression(expr[i + 1], path.concat(i));
const output = parseExpression(definitions, expr[i + 1], path.concat(i));
if (output.error) return output;
outputExpressions.push(output);
}

const otherwise = parseExpression(expr[expr.length - 1], path.concat(expr.length - 1));
const otherwise = parseExpression(definitions, expr[expr.length - 1], path.concat(expr.length - 1));
if (otherwise.error) return otherwise;
outputExpressions.push(otherwise);

Expand All @@ -143,7 +147,7 @@ function parseExpression(expr: mixed, path: Array<number> = []) /*: TypedExpress

const args = [];
for (const arg of expr.slice(1)) {
const parsedArg = parseExpression(arg, path.concat(1 + args.length));
const parsedArg = parseExpression(definitions, arg, path.concat(1 + args.length));
if (parsedArg.error) return parsedArg;
args.push(parsedArg);
}
Expand Down
3 changes: 2 additions & 1 deletion test/expression.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require('flow-remove-types/register');
const util = require('../src/util/util');
const expressionSuite = require('./integration').expression;
const expressions = require('../src/style-spec/function/expressions');
const compileExpression = require('../src/style-spec/function/compile');

let tests;
Expand All @@ -12,7 +13,7 @@ if (process.argv[1] === __filename && process.argv.length > 2) {
}

expressionSuite.run('js', {tests: tests}, (fixture) => {
const compiled = compileExpression(fixture.expression);
const compiled = compileExpression(expressions, fixture.expression);

const testResult = {
compileResult: util.pick(compiled, ['result', 'js', 'isFeatureConstant', 'isZoomConstant', 'errors'])
Expand Down

0 comments on commit 8275bb2

Please sign in to comment.