From 0fd95a507e3706f76025d074a9ca8dec84c912c7 Mon Sep 17 00:00:00 2001 From: Anand Thakker Date: Sat, 1 Jul 2017 21:44:14 -0400 Subject: [PATCH] Add 'let' expressions --- src/style-spec/function/compile.js | 5 +- src/style-spec/function/definitions/index.js | 6 +- src/style-spec/function/expression.js | 174 ++++++++++++++++-- src/style-spec/function/index.js | 10 +- src/style-spec/function/type_check.js | 50 ++++- test/expression.test.js | 2 +- .../expression-tests/let/basic/test.json | 30 +++ .../expression-tests/let/nested/test.json | 25 +++ .../let/property-function/test.json | 23 +++ .../expression-tests/let/reserved/test.json | 18 ++ .../expression-tests/let/shadow/test.json | 25 +++ .../expression-tests/let/unbound/test.json | 26 +++ .../expression-tests/let/zoom/test.json | 34 ++++ 13 files changed, 395 insertions(+), 33 deletions(-) create mode 100644 test/integration/expression-tests/let/basic/test.json create mode 100644 test/integration/expression-tests/let/nested/test.json create mode 100644 test/integration/expression-tests/let/property-function/test.json create mode 100644 test/integration/expression-tests/let/reserved/test.json create mode 100644 test/integration/expression-tests/let/shadow/test.json create mode 100644 test/integration/expression-tests/let/unbound/test.json create mode 100644 test/integration/expression-tests/let/zoom/test.json diff --git a/src/style-spec/function/compile.js b/src/style-spec/function/compile.js index 9cababecf8e..84595b7544f 100644 --- a/src/style-spec/function/compile.js +++ b/src/style-spec/function/compile.js @@ -25,7 +25,6 @@ type CompiledExpression = {| function: Function, isFeatureConstant: boolean, isZoomConstant: boolean, - type: Type, expression: Expression |} @@ -80,14 +79,12 @@ mapProperties = mapProperties || {}; var props = feature && feature.properties || {}; return this.unwrap(${compiled}) `); - const type = checked.expression instanceof LiteralExpression ? - checked.expression.type : checked.expression.type.result; + return { result: 'success', function: fn.bind(evaluationContext()), isFeatureConstant: isFeatureConstant(checked.expression), isZoomConstant: isZoomConstant(checked.expression), - type, expression: checked.expression }; } diff --git a/src/style-spec/function/definitions/index.js b/src/style-spec/function/definitions/index.js index 1b2c19546ee..1c2e91a3798 100644 --- a/src/style-spec/function/definitions/index.js +++ b/src/style-spec/function/definitions/index.js @@ -185,10 +185,10 @@ const expressions: { [string]: Class } = { const ancestors = context.ancestors.join(':'); // zoom expressions may only appear like: // ['curve', interp, ['zoom'], ...] - // or ['coalesce', ['curve', interp, ['zoom'], ...], ... ] + // or ['let', ..., ['coalesce', ['curve', interp, ['zoom'], ...], ... ] ] if ( - !/^(1.)?2/.test(context.key) || - !/(coalesce:)?curve/.test(ancestors) + !/2$/.test(context.key) || + !/^(let\.result:|coalesce:)*curve$/.test(ancestors) ) { throw new ParsingError( context.key, diff --git a/src/style-spec/function/expression.js b/src/style-spec/function/expression.js index 301c2c36965..b491383c9fa 100644 --- a/src/style-spec/function/expression.js +++ b/src/style-spec/function/expression.js @@ -4,7 +4,8 @@ const { NullType, StringType, NumberType, - BooleanType + BooleanType, + typename } = require('./types'); const {Color, isValue, typeOf} = require('./values'); @@ -13,7 +14,7 @@ import type { Value } from './values'; import type { Type, LambdaType } from './types'; import type { ExpressionName } from './expression_name'; -export type Expression = LambdaExpression | LiteralExpression; // eslint-disable-line no-use-before-define +export type Expression = LambdaExpression | LiteralExpression | LetExpression | Reference; // eslint-disable-line no-use-before-define export type CompileError = {| error: string, @@ -28,24 +29,59 @@ class ParsingError extends Error { } } +class Scope { + parent: ?Scope; + bindings: {[string]: Expression}; + constructor(parent?: Scope, bindings: {[string]: Expression} = {}) { + this.parent = parent; + this.bindings = bindings; + } + + concat(bindings: {[string]: Expression}) { + return new Scope(this, bindings); + } + + get(name: string): Expression { + if (this.bindings[name]) { return this.bindings[name]; } + if (this.parent) { return this.parent.get(name); } + throw new Error(`${name} not found in scope.`); + } + + has(name: string): boolean { + if (this.bindings[name]) return true; + return this.parent ? this.parent.has(name) : false; + } + + toString(depth:number = 0): string { + const spaces = []; + while (--depth > 0) { spaces.push(' '); } + return `${spaces.join('')}${depth > 0 ? '[parent]' : ''}${ + Object.keys(this.bindings) + .map(b => spaces.concat([b, ': ', this.bindings[b].type.name].join(''))) + .join('\n') + }`; + } +} + class ParsingContext { key: string; path: Array; ancestors: Array; definitions: {[string]: Class}; - constructor(definitions: *, path: * = [], ancestors: * = []) { + scope: Scope; + constructor(definitions: *, path: Array = [], ancestors: * = [], scope: Scope = new Scope()) { this.definitions = definitions; this.path = path; this.key = path.join('.'); this.ancestors = ancestors; + this.scope = scope; } - concat(index: number, expressionName: ?string) { - return new ParsingContext( - this.definitions, - this.path.concat(index), - expressionName ? this.ancestors.concat(expressionName) : this.ancestors - ); + concat(index?: number, expressionName?: string, bindings?: {[string]: Expression}) { + const path = typeof index === 'number' ? this.path.concat(index) : this.path; + const ancestors = expressionName ? this.ancestors.concat(expressionName) : this.ancestors; + const scope = bindings ? this.scope.concat(bindings) : this.scope; + return new ParsingContext(this.definitions, path, ancestors, scope); } } @@ -57,6 +93,10 @@ class BaseExpression { (this: any).type = type; } + getResultType() { + return this.type.kind === 'lambda' ? this.type.result : this.type; + } + compile(): string | Array { throw new Error('Unimplemented'); } @@ -168,6 +208,103 @@ class LambdaExpression extends BaseExpression { } } +class Reference extends BaseExpression { + name: string; + constructor(key: string, name: string, type: Type) { + super(key, type); + if (!/^[a-zA-Z_]+[a-zA-Z_0-9]*$/.test(name)) + throw new ParsingError(key, `Invalid identifier ${name}.`); + this.name = name; + } + + compile() { return this.name; } + + serialize(_: boolean) { + return [this.name]; + } +} + +class LetExpression extends BaseExpression { + names: Array; + scope: Scope; + result: Expression; + constructor(key: string, names: Array, scope: Scope, result: Expression) { + super(key, result.type); + this.names = names; + this.scope = scope; + this.result = result; + } + + compile() { + const names = []; + const values = []; + const errors = []; + for (const name in this.scope.bindings) { + names.push(name); + const value = this.scope.bindings[name].compile(); + if (Array.isArray(value)) { + errors.push.apply(errors, value); + } else { + values.push(value); + } + } + + const result = this.result.compile(); + if (Array.isArray(result)) { + errors.push.apply(errors, result); + return errors; + } + + if (errors.length > 0) return errors; + + return `(function (${names.join(', ')}) { + return ${result}; + }.bind(this))(${values.join(', ')})`; + } + + serialize(withTypes: boolean) { + const serialized = ['let']; + for (const name of this.names) { + serialized.push(name, this.scope.get(name).serialize(withTypes)); + } + serialized.push(this.result.serialize(withTypes)); + return serialized; + } + + visit(fn: (BaseExpression) => void): void { + fn(this); + for (const name in this.scope.bindings) { + this.scope.get(name).visit(fn); + } + this.result.visit(fn); + } + + static parse(args: Array, context: ParsingContext) { + if (args.length < 3) + throw new ParsingError(context.key, `Expected at least 3 arguments, but found ${args.length} instead.`); + + const bindings: {[string]: Expression} = {}; + const names = []; + for (let i = 0; i < args.length - 1; i += 2) { + const name = args[i]; + const key = context.path.concat(i + 1).join('.'); + if (typeof name !== 'string') + throw new ParsingError(key, `Expected string, but found ${typeof name} instead`); + + if (context.definitions[name]) + throw new ParsingError(key, `"${name}" is reserved, so it cannot not be used as a "let" binding.`); + + const value = parseExpression(args[i + 1], context.concat(i + 2, 'let.binding')); + + bindings[name] = value; + names.push(name); + } + const resultContext = context.concat(args.length, 'let.result', bindings); + const result = parseExpression(args[args.length - 1], resultContext); + return new this(context.key, names, resultContext.scope, result); + } +} + function parseExpression(expr: mixed, context: ParsingContext) : Expression { const key = context.key; @@ -189,18 +326,18 @@ function parseExpression(expr: mixed, context: ParsingContext) : Expression { const op = expr[0]; if (typeof op !== 'string') { throw new ParsingError(`${key}.0`, `Expression name must be a string, but found ${typeof op} instead. If you wanted a literal array, use ["literal", [...]].`); - } - - if (op === 'literal') { + } else if (op === 'literal') { return LiteralExpression.parse(expr.slice(1), context); + } else if (op === 'let') { + return LetExpression.parse(expr.slice(1), context); + } else if (context.scope.has(op)) { + return new Reference(context.key, op, typename('T')); } const Expr = context.definitions[op]; - if (!Expr) { - throw new ParsingError(`${key}.0`, `Unknown expression "${op}". If you wanted a literal array, use ["literal", [...]].`); - } + if (Expr) return Expr.parse(expr.slice(1), context); - return Expr.parse(expr.slice(1), context); + throw new ParsingError(`${key}.0`, `Unknown expression "${op}". If you wanted a literal array, use ["literal", [...]].`); } else if (typeof expr === 'object') { throw new ParsingError(key, `Bare objects invalid. Use ["literal", {...}] instead.`); } else { @@ -209,9 +346,12 @@ function parseExpression(expr: mixed, context: ParsingContext) : Expression { } module.exports = { + Scope, ParsingContext, ParsingError, parseExpression, LiteralExpression, - LambdaExpression + LambdaExpression, + LetExpression, + Reference }; diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index 772f5de3a0f..9a33e4598b8 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -4,6 +4,8 @@ const compileExpression = require('./compile'); const convert = require('./convert'); const {ColorType, StringType, NumberType, ValueType, array} = require('./types'); const CurveExpression = require('./definitions/curve'); +const CoalesceExpression = require('./definitions')['coalesce']; +const {LetExpression} = require('./expression'); function createFunction(parameters, propertySpec) { let expr; @@ -33,7 +35,13 @@ function createFunction(parameters, propertySpec) { // our prepopulate-and-interpolate approach to paint properties // that are zoom-and-property dependent. let curve = compiled.expression; - if (!(curve instanceof CurveExpression)) { curve = curve.args[0]; } + while (!(curve instanceof CurveExpression)) { + if (curve instanceof CoalesceExpression) { + curve = curve.args[0]; + } else if (curve instanceof LetExpression) { + curve = curve.result; + } + } const curveArgs = [].concat(curve.args); const serialized = curve.serialize(); const interpolation = serialized[1]; diff --git a/src/style-spec/function/type_check.js b/src/style-spec/function/type_check.js index 7eb07c0eb16..a5e5f64d702 100644 --- a/src/style-spec/function/type_check.js +++ b/src/style-spec/function/type_check.js @@ -19,20 +19,53 @@ export type TypecheckResult = {| const assert = require('assert'); const extend = require('../util/extend'); - -const { NullType, lambda, array, variant, nargs } = require('./types'); - -const { LiteralExpression } = require('./expression'); +const { + NullType, + lambda, + array, + variant, + nargs, + typename +} = require('./types'); +const { + LetExpression, + Scope, + Reference, + LiteralExpression +} = require('./expression'); module.exports = typeCheckExpression; // typecheck the given expression and return a new TypedExpression // tree with all generics resolved -function typeCheckExpression(expected: Type, e: Expression) : TypecheckResult { +function typeCheckExpression(expected: Type, e: Expression, scope: Scope = new Scope()) : TypecheckResult { if (e instanceof LiteralExpression) { const error = match(expected, e.type); if (error) return { result: 'error', errors: [{ key: e.key, error }] }; return {result: 'success', expression: e}; + } else if (e instanceof Reference) { + const referee = scope.get(e.name); + const error = match(expected, referee.type); + if (error) return { result: 'error', errors: [{key: e.key, error }] }; + return { + result: 'success', + expression: new Reference(e.key, e.name, referee.type) + }; + } else if (e instanceof LetExpression) { + const bindings = {}; + for (const name in e.scope.bindings) { + const value = e.scope.bindings[name]; + const checkedValue = typeCheckExpression(typename('T'), value, scope); + if (checkedValue.result === 'error') return checkedValue; + bindings[name] = checkedValue.expression; + } + const nextScope = scope.concat(bindings); + const checkedResult = typeCheckExpression(expected, e.result, nextScope); + if (checkedResult.result === 'error') return checkedResult; + return { + result: 'success', + expression: new LetExpression(e.key, e.names, nextScope, checkedResult.expression) + }; } else { // e is a lambda expression, so check its result type against the // expected type and recursively typecheck its arguments @@ -88,7 +121,10 @@ function typeCheckExpression(expected: Type, e: Expression) : TypecheckResult { // - collect typename mappings when ^ succeeds or type errors when it fails for (let i = 0; i < argValues.length; i++) { const param = expandedParams[i]; - const arg = argValues[i]; + let arg = argValues[i]; + if (arg instanceof Reference) { + arg = scope.get(arg.name); + } const error = match( resolveTypenamesIfPossible(param, typenames), arg.type, @@ -115,7 +151,7 @@ function typeCheckExpression(expected: Type, e: Expression) : TypecheckResult { const t = expandedParams[i]; const arg = argValues[i]; const expected = resolveTypenamesIfPossible(t, typenames); - const checked = typeCheckExpression(expected, arg); + const checked = typeCheckExpression(expected, arg, scope); if (checked.result === 'error') { errors.push.apply(errors, checked.errors); } else if (errors.length === 0) { diff --git a/test/expression.test.js b/test/expression.test.js index 1f3ce8de6c9..43ab93a36d6 100644 --- a/test/expression.test.js +++ b/test/expression.test.js @@ -17,7 +17,7 @@ expressionSuite.run('js', {tests: tests}, (fixture) => { const testResult = { compileResult: util.pick(compiled, ['result', 'js', 'isFeatureConstant', 'isZoomConstant', 'errors']) }; - if (compiled.result === 'success') testResult.compileResult.type = compiled.type.name; + if (compiled.result === 'success') testResult.compileResult.type = compiled.expression.getResultType().name; if (compiled.result === 'success' && fixture.evaluate) { const evaluateResults = []; diff --git a/test/integration/expression-tests/let/basic/test.json b/test/integration/expression-tests/let/basic/test.json new file mode 100644 index 00000000000..faeecbe71f8 --- /dev/null +++ b/test/integration/expression-tests/let/basic/test.json @@ -0,0 +1,30 @@ +{ + "expression": [ + "let", + "a", 1, + "b", 2, + [ + "+", + [ + "+", + ["a"], + ["b"] + ], + ["a"] + ] + ], + "evaluate": [ + [{}, {}] + ], + "expected": { + "compileResult": { + "isFeatureConstant": true, + "isZoomConstant": true, + "result": "success", + "type": "Number" + }, + "evaluateResults": [ + 4 + ] + } +} diff --git a/test/integration/expression-tests/let/nested/test.json b/test/integration/expression-tests/let/nested/test.json new file mode 100644 index 00000000000..2ac09e2838e --- /dev/null +++ b/test/integration/expression-tests/let/nested/test.json @@ -0,0 +1,25 @@ +{ + "expression": [ + "let", + "a", 1, + [ + "let", + "b", ["+", 1, ["a"]], + ["+", ["a"], ["b"]] + ] + ], + "evaluate": [ + [{}, {}] + ], + "expected": { + "compileResult": { + "isFeatureConstant": true, + "isZoomConstant": true, + "result": "success", + "type": "Number" + }, + "evaluateResults": [ + 3 + ] + } +} diff --git a/test/integration/expression-tests/let/property-function/test.json b/test/integration/expression-tests/let/property-function/test.json new file mode 100644 index 00000000000..d88d16c060c --- /dev/null +++ b/test/integration/expression-tests/let/property-function/test.json @@ -0,0 +1,23 @@ +{ + "expression": [ + "let", + "a", ["get", "x"], + [ + "+", 1, ["number", ["a"]] + ] + ], + "evaluate": [ + [{}, {"properties": {"x": 5}}] + ], + "expected": { + "compileResult": { + "isFeatureConstant": false, + "isZoomConstant": true, + "result": "success", + "type": "Number" + }, + "evaluateResults": [ + 6 + ] + } +} diff --git a/test/integration/expression-tests/let/reserved/test.json b/test/integration/expression-tests/let/reserved/test.json new file mode 100644 index 00000000000..b6f96f7019d --- /dev/null +++ b/test/integration/expression-tests/let/reserved/test.json @@ -0,0 +1,18 @@ +{ + "expression": [ + "let", + "zoom", 1, + ["zoom"] + ], + "expected": { + "compileResult": { + "result": "error", + "errors": [ + { + "error": "\"zoom\" is reserved, so it cannot not be used as a \"let\" binding.", + "key": "1" + } + ] + } + } +} diff --git a/test/integration/expression-tests/let/shadow/test.json b/test/integration/expression-tests/let/shadow/test.json new file mode 100644 index 00000000000..069532b593b --- /dev/null +++ b/test/integration/expression-tests/let/shadow/test.json @@ -0,0 +1,25 @@ +{ + "expression": [ + "let", + "a", 1, + [ + "let", + "a", 2, + ["a"] + ] + ], + "evaluate": [ + [{}, {}] + ], + "expected": { + "compileResult": { + "isFeatureConstant": true, + "isZoomConstant": true, + "result": "success", + "type": "Number" + }, + "evaluateResults": [ + 2 + ] + } +} diff --git a/test/integration/expression-tests/let/unbound/test.json b/test/integration/expression-tests/let/unbound/test.json new file mode 100644 index 00000000000..9c175b7d67a --- /dev/null +++ b/test/integration/expression-tests/let/unbound/test.json @@ -0,0 +1,26 @@ +{ + "expression": [ + "let", + "a", 1, + [ + "+", + [ + "+", + ["a"], + ["b"] + ], + ["a"] + ] + ], + "expected": { + "compileResult": { + "result": "error", + "errors": [ + { + "error": "Unknown expression \"b\". If you wanted a literal array, use [\"literal\", [...]].", + "key": "3.1.2.0" + } + ] + } + } +} diff --git a/test/integration/expression-tests/let/zoom/test.json b/test/integration/expression-tests/let/zoom/test.json new file mode 100644 index 00000000000..e7291f55d5b --- /dev/null +++ b/test/integration/expression-tests/let/zoom/test.json @@ -0,0 +1,34 @@ +{ + "expression": [ + "let", + "z0_value", 10, + "z20_value", 30, + [ + "curve", + [ + "linear" + ], + [ + "zoom" + ], + 0, + ["z0_value"], + 20, + ["z20_value"] + ] + ], + "evaluate": [ + [{"zoom": 10}, {}] + ], + "expected": { + "compileResult": { + "isFeatureConstant": true, + "isZoomConstant": false, + "result": "success", + "type": "Number" + }, + "evaluateResults": [ + 20 + ] + } +}