From 1863d5097802a835487198f2e9b5085d9cae1bc4 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 26 Nov 2014 09:44:31 -0800 Subject: [PATCH] feat(parser): adds support for variable bindings --- modules/change_detection/src/parser/ast.js | 23 ++++++- .../parser/context_with_variable_bindings.js | 20 +++++++ modules/change_detection/src/record.js | 3 + modules/change_detection/src/record_range.js | 60 ++++++++++++++----- .../test/change_detector_spec.js | 29 +++++++++ .../test/parser/parser_spec.js | 37 +++++++++++- .../test/record_range_spec.js | 2 +- 7 files changed, 155 insertions(+), 19 deletions(-) create mode 100644 modules/change_detection/src/parser/context_with_variable_bindings.js diff --git a/modules/change_detection/src/parser/ast.js b/modules/change_detection/src/parser/ast.js index 645899becc..a0d3cc0dd9 100644 --- a/modules/change_detection/src/parser/ast.js +++ b/modules/change_detection/src/parser/ast.js @@ -1,5 +1,6 @@ import {FIELD, autoConvertAdd, isBlank, isPresent, FunctionWrapper, BaseException} from "facade/lang"; import {List, Map, ListWrapper, MapWrapper} from "facade/collection"; +import {ContextWithVariableBindings} from "./context_with_variable_bindings"; export class AST { eval(context) { @@ -97,7 +98,16 @@ export class AccessMember extends AST { } eval(context) { - return this.getter(this.receiver.eval(context)); + var evaluatedContext = this.receiver.eval(context); + + while (evaluatedContext instanceof ContextWithVariableBindings) { + if (evaluatedContext.hasBinding(this.name)) { + return evaluatedContext.get(this.name); + } + evaluatedContext = evaluatedContext.parent; + } + + return this.getter(evaluatedContext); } get isAssignable() { @@ -105,7 +115,16 @@ export class AccessMember extends AST { } assign(context, value) { - return this.setter(this.receiver.eval(context), value); + var evaluatedContext = this.receiver.eval(context); + + while (evaluatedContext instanceof ContextWithVariableBindings) { + if (evaluatedContext.hasBinding(this.name)) { + throw new BaseException(`Cannot reassign a variable binding ${this.name}`) + } + evaluatedContext = evaluatedContext.parent; + } + + return this.setter(evaluatedContext, value); } visit(visitor, args) { diff --git a/modules/change_detection/src/parser/context_with_variable_bindings.js b/modules/change_detection/src/parser/context_with_variable_bindings.js new file mode 100644 index 0000000000..f390aa42d2 --- /dev/null +++ b/modules/change_detection/src/parser/context_with_variable_bindings.js @@ -0,0 +1,20 @@ +import {MapWrapper} from 'facade/collection'; + +export class ContextWithVariableBindings { + parent:any; + /// varBindings are read-only. updating/adding keys is not supported. + varBindings:Map; + + constructor(parent:any, varBindings:Map) { + this.parent = parent; + this.varBindings = varBindings; + } + + hasBinding(name:string):boolean { + return MapWrapper.contains(this.varBindings, name); + } + + get(name:string) { + return MapWrapper.get(this.varBindings, name); + } +} \ No newline at end of file diff --git a/modules/change_detection/src/record.js b/modules/change_detection/src/record.js index 682df92e08..788e66ecd5 100644 --- a/modules/change_detection/src/record.js +++ b/modules/change_detection/src/record.js @@ -30,6 +30,7 @@ export class ProtoRecord { context:any; funcOrValue:any; arity:int; + name:string; dest; next:ProtoRecord; @@ -39,12 +40,14 @@ export class ProtoRecord { mode:int, funcOrValue, arity:int, + name:string, dest) { this.recordRange = recordRange; this._mode = mode; this.funcOrValue = funcOrValue; this.arity = arity; + this.name = name; this.dest = dest; this.next = null; diff --git a/modules/change_detection/src/record_range.js b/modules/change_detection/src/record_range.js index d8aaa50ffd..d265b08230 100644 --- a/modules/change_detection/src/record_range.js +++ b/modules/change_detection/src/record_range.js @@ -15,7 +15,7 @@ import {List, Map, ListWrapper, MapWrapper} from 'facade/collection'; import {AST, AccessMember, ImplicitReceiver, AstVisitor, LiteralPrimitive, Binary, Formatter, MethodCall, FunctionCall, PrefixNot, Conditional, LiteralArray, LiteralMap, KeyedAccess, Chain, Assignment} from './parser/ast'; - +import {ContextWithVariableBindings} from './parser/context_with_variable_bindings'; export class ProtoRecordRange { headRecord:ProtoRecord; @@ -304,11 +304,36 @@ export class RecordRange { for (var record:Record = this.headRecord; record != null; record = record.next) { + if (record.isImplicitReceiver) { - record.updateContext(context); + this._setContextForRecord(context, record); } } } + + _setContextForRecord(context, record:Record) { + var proto = record.protoRecord; + + while (context instanceof ContextWithVariableBindings) { + if (context.hasBinding(proto.name)) { + this._setVarBindingGetter(context, record, proto); + return; + } + context = context.parent; + } + + this._setRegularGetter(context, record, proto); + } + + _setVarBindingGetter(context, record:Record, proto:ProtoRecord) { + record.funcOrValue = _mapGetter(proto.name); + record.updateContext(context.varBindings); + } + + _setRegularGetter(context, record:Record, proto:ProtoRecord) { + record.funcOrValue = proto.funcOrValue; + record.updateContext(context); + } } function _link(a:Record, b:Record) { @@ -353,25 +378,25 @@ class ProtoRecordCreator { } visitLiteralPrimitive(ast:LiteralPrimitive, dest) { - this.add(this.construct(RECORD_TYPE_CONST, ast.value, 0, dest)); + this.add(this.construct(RECORD_TYPE_CONST, ast.value, 0, null, dest)); } visitBinary(ast:Binary, dest) { var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, - _operationToFunction(ast.operation), 2, dest); + _operationToFunction(ast.operation), 2, null, dest); ast.left.visit(this, new Destination(record, 0)); ast.right.visit(this, new Destination(record, 1)); this.add(record); } visitPrefixNot(ast:PrefixNot, dest) { - var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _operation_negate, 1, dest); + var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _operation_negate, 1, null, dest); ast.expression.visit(this, new Destination(record, 0)); this.add(record); } visitAccessMember(ast:AccessMember, dest) { - var record = this.construct(RECORD_TYPE_PROPERTY, ast.getter, 0, dest); + var record = this.construct(RECORD_TYPE_PROPERTY, ast.getter, 0, ast.name, dest); if (ast.receiver instanceof ImplicitReceiver) { record.setIsImplicitReceiver(); } else { @@ -381,7 +406,7 @@ class ProtoRecordCreator { } visitFormatter(ast:Formatter, dest) { - var record = this.construct(RECORD_TYPE_INVOKE_FORMATTER, ast.name, ast.allArgs.length, dest); + var record = this.construct(RECORD_TYPE_INVOKE_FORMATTER, ast.name, ast.allArgs.length, null, dest); for (var i = 0; i < ast.allArgs.length; ++i) { ast.allArgs[i].visit(this, new Destination(record, i)); } @@ -389,7 +414,7 @@ class ProtoRecordCreator { } visitMethodCall(ast:MethodCall, dest) { - var record = this.construct(RECORD_TYPE_INVOKE_METHOD, ast.fn, ast.args.length, dest); + var record = this.construct(RECORD_TYPE_INVOKE_METHOD, ast.fn, ast.args.length, null, dest); for (var i = 0; i < ast.args.length; ++i) { ast.args[i].visit(this, new Destination(record, i)); } @@ -402,7 +427,7 @@ class ProtoRecordCreator { } visitFunctionCall(ast:FunctionCall, dest) { - var record = this.construct(RECORD_TYPE_INVOKE_CLOSURE, null, ast.args.length, dest); + var record = this.construct(RECORD_TYPE_INVOKE_CLOSURE, null, ast.args.length, null, dest); ast.target.visit(this, new Destination(record, null)); for (var i = 0; i < ast.args.length; ++i) { ast.args[i].visit(this, new Destination(record, i)); @@ -411,7 +436,7 @@ class ProtoRecordCreator { } visitConditional(ast:Conditional, dest) { - var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _cond, 3, dest); + var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _cond, 3, null, dest); ast.condition.visit(this, new Destination(record, 0)); ast.trueExp.visit(this, new Destination(record, 1)); ast.falseExp.visit(this, new Destination(record, 2)); @@ -422,7 +447,7 @@ class ProtoRecordCreator { visitLiteralArray(ast:LiteralArray, dest) { var length = ast.expressions.length; - var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _arrayFn(length), length, dest); + var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _arrayFn(length), length, null, dest); for (var i = 0; i < length; ++i) { ast.expressions[i].visit(this, new Destination(record, i)); } @@ -431,7 +456,7 @@ class ProtoRecordCreator { visitLiteralMap(ast:LiteralMap, dest) { var length = ast.values.length; - var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _mapFn(ast.keys, length), length, dest); + var record = this.construct(RECORD_TYPE_INVOKE_PURE_FUNCTION, _mapFn(ast.keys, length), length, null, dest); for (var i = 0; i < length; ++i) { ast.values[i].visit(this, new Destination(record, i)); } @@ -448,8 +473,8 @@ class ProtoRecordCreator { ast.visit(this, memento); } - construct(recordType, funcOrValue, arity, dest) { - return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity, dest); + construct(recordType, funcOrValue, arity, name, dest) { + return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity, name, dest); } add(protoRecord:ProtoRecord) { @@ -540,3 +565,10 @@ function _mapFn(keys:List, length:int) { default: throw new BaseException(`Does not support literal maps with more than 9 elements`); } } + +//TODO: cache the getters +function _mapGetter(key) { + return function(map) { + return MapWrapper.get(map, key); + } +} \ No newline at end of file diff --git a/modules/change_detection/test/change_detector_spec.js b/modules/change_detection/test/change_detector_spec.js index e91c09788e..ecc42cf78b 100644 --- a/modules/change_detection/test/change_detector_spec.js +++ b/modules/change_detection/test/change_detector_spec.js @@ -2,6 +2,7 @@ import {ddescribe, describe, it, iit, xit, expect} from 'test_lib/test_lib'; import {isPresent} from 'facade/lang'; import {List, ListWrapper, MapWrapper} from 'facade/collection'; +import {ContextWithVariableBindings} from 'change_detection/parser/context_with_variable_bindings'; import {Parser} from 'change_detection/parser/parser'; import {Lexer} from 'change_detection/parser/lexer'; @@ -183,6 +184,34 @@ export function main() { expect(counter).toEqual(2); }); }); + + describe("ContextWithVariableBindings", () => { + it('should read a field from ContextWithVariableBindings', () => { + var locals = new ContextWithVariableBindings(null, + MapWrapper.createFromPairs([["key", "value"]])); + + expect(executeWatch('key', 'key', locals)) + .toEqual(['key=value']); + }); + + it('should handle nested ContextWithVariableBindings', () => { + var nested = new ContextWithVariableBindings(null, + MapWrapper.createFromPairs([["key", "value"]])); + var locals = new ContextWithVariableBindings(nested, MapWrapper.create()); + + expect(executeWatch('key', 'key', locals)) + .toEqual(['key=value']); + }); + + it("should fall back to a regular field read when ContextWithVariableBindings " + + "does not have the requested field", () => { + var locals = new ContextWithVariableBindings(new Person("Jim"), + MapWrapper.createFromPairs([["key", "value"]])); + + expect(executeWatch('name', 'name', locals)) + .toEqual(['name=Jim']); + }); + }); }); }); } diff --git a/modules/change_detection/test/parser/parser_spec.js b/modules/change_detection/test/parser/parser_spec.js index 38ed12e4a1..a4a1a555f2 100644 --- a/modules/change_detection/test/parser/parser_spec.js +++ b/modules/change_detection/test/parser/parser_spec.js @@ -4,6 +4,7 @@ import {reflector} from 'reflection/reflection'; import {MapWrapper, ListWrapper} from 'facade/collection'; import {Parser} from 'change_detection/parser/parser'; import {Lexer} from 'change_detection/parser/lexer'; +import {ContextWithVariableBindings} from 'change_detection/parser/context_with_variable_bindings'; import {Formatter, LiteralPrimitive} from 'change_detection/parser/ast'; class TestData { @@ -51,8 +52,9 @@ export function main() { return expect(parseAction(text).eval(c)); } - function expectEvalError(text) { - return expect(() => parseAction(text).eval(td())); + function expectEvalError(text, passedInContext = null) { + var c = isBlank(passedInContext) ? td() : passedInContext; + return expect(() => parseAction(text).eval(c)); } function evalAsts(asts, passedInContext = null) { @@ -196,6 +198,25 @@ export function main() { expectEvalError('x. 1234').toThrowError(new RegExp('identifier or keyword')); expectEvalError('x."foo"').toThrowError(new RegExp('identifier or keyword')); }); + + it("should read a field from ContextWithVariableBindings", () => { + var locals = new ContextWithVariableBindings(null, + MapWrapper.createFromPairs([["key", "value"]])); + expectEval("key", locals).toEqual("value"); + }); + + it("should handle nested ContextWithVariableBindings", () => { + var nested = new ContextWithVariableBindings(null, + MapWrapper.createFromPairs([["key", "value"]])); + var locals = new ContextWithVariableBindings(nested, MapWrapper.create()); + expectEval("key", locals).toEqual("value"); + }); + + it("should fall back to a regular field read when ContextWithVariableBindings "+ + "does not have the requested field", () => { + var locals = new ContextWithVariableBindings(td(999), MapWrapper.create()); + expectEval("a", locals).toEqual(999); + }); }); describe("method calls", () => { @@ -284,6 +305,18 @@ export function main() { it('should throw on bad assignment', () => { expectEvalError("5=4").toThrowError(new RegExp("Expression 5 is not assignable")); }); + + it('should reassign when no variable binding with the given name', () => { + var context = td(); + var locals = new ContextWithVariableBindings(context, MapWrapper.create()); + expectEval('a = 200', locals).toEqual(200); + expect(context.a).toEqual(200); + }); + + it('should throw when reassigning a variable binding', () => { + var locals = new ContextWithVariableBindings(null, MapWrapper.createFromPairs([["key", "value"]])); + expectEvalError('key = 200', locals).toThrowError(new RegExp("Cannot reassign a variable binding")); + }); }); describe("general error handling", () => { diff --git a/modules/change_detection/test/record_range_spec.js b/modules/change_detection/test/record_range_spec.js index e48418709c..446cef8a1a 100644 --- a/modules/change_detection/test/record_range_spec.js +++ b/modules/change_detection/test/record_range_spec.js @@ -46,7 +46,7 @@ export function main() { } function createRecord(rr) { - return new Record(rr, new ProtoRecord(null, 0, null, null, null), null); + return new Record(rr, new ProtoRecord(null, 0, null, null, null, null), null); } describe('record range', () => {