feat(parser): split parse into parseBinding and parseAction

modules/change_detection/src/parser/ast.js

@@ -2,7 +2,8 @@ import {FIELD, toBool, autoConvertAdd, isBlank, FunctionWrapper, BaseException}
 import {List, ListWrapper} from "facade/collection";
 
 export class AST {
-  eval(context, formatters) {
+  eval(context) {
+    throw new BaseException("Not supported");
   }
 
   visit(visitor) {
@@ -10,7 +11,7 @@ export class AST {
 }
 
 export class ImplicitReceiver extends AST {
-  eval(context, formatters) {
+  eval(context) {
     return context;
   }
 
@@ -20,17 +21,17 @@ export class ImplicitReceiver extends AST {
 }
 
 export class Conditional extends AST {
-  constructor(condition:AST, yes:AST, no:AST){
+  constructor(condition:AST, trueExp:AST, falseExp:AST){
     this.condition = condition;
-    this.yes = yes;
+    this.trueExp = trueExp;
-    this.no = no;
+    this.falseExp = falseExp;
   }
 
-  eval(context, formatters) {
+  eval(context) {
-    if(this.condition.eval(context, formatters)) {
+    if(this.condition.eval(context)) {
-      return this.yes.eval(context, formatters);
+      return this.trueExp.eval(context);
     } else {
-      return this.no.eval(context, formatters);
+      return this.falseExp.eval(context);
     }
   }
 }
@@ -42,8 +43,8 @@ export class FieldRead extends AST {
     this.getter = getter;
   }
 
-  eval(context, formatters) {
+  eval(context) {
-    return this.getter(this.receiver.eval(context, formatters));
+    return this.getter(this.receiver.eval(context));
   }
 
   visit(visitor) {
@@ -59,15 +60,6 @@ export class Formatter extends AST {
     this.allArgs = ListWrapper.concat([exp], args);
   }
 
-  eval(context, formatters) {
-    var formatter = formatters[this.name];
-    if (isBlank(formatter)) {
-      throw new BaseException(`No formatter '${this.name}' found!`);
-    }
-    var evaledArgs = evalList(context, this.allArgs, formatters);
-    return FunctionWrapper.apply(formatter, evaledArgs);
-  }
-
   visit(visitor) {
     visitor.visitFormatter(this);
   }
@@ -78,7 +70,7 @@ export class LiteralPrimitive extends AST {
   constructor(value) {
     this.value = value;
   }
-  eval(context, formatters) {
+  eval(context) {
     return this.value;
   }
   visit(visitor) {
@@ -100,16 +92,18 @@ export class Binary extends AST {
     visitor.visitBinary(this);
   }
 
-  eval(context, formatters) {
+  eval(context) {
-    var left = this.left.eval(context, formatters);
+    var left = this.left.eval(context);
     switch (this.operation) {
-      case '&&': return toBool(left) && toBool(this.right.eval(context, formatters));
+      case '&&': return toBool(left) && toBool(this.right.eval(context));
-      case '||': return toBool(left) || toBool(this.right.eval(context, formatters));
+      case '||': return toBool(left) || toBool(this.right.eval(context));
     }
-    var right = this.right.eval(context, formatters);
+    var right = this.right.eval(context);
 
     // Null check for the operations.
-    if (left == null || right == null) return null;
+    if (left == null || right == null) {
+      throw new BaseException("One of the operands is null");
+    }
 
     switch (this.operation) {
       case '+'  : return autoConvertAdd(left, right);
@@ -139,8 +133,8 @@ export class PrefixNot extends AST {
     this.expression = expression;
   }
   visit(visitor) { visitor.visitPrefixNot(this); }
-  eval(context, formatters) {
+  eval(context) {
-    return !toBool(this.expression.eval(context, formatters));
+    return !toBool(this.expression.eval(context));
   }
 }
 
@@ -155,11 +149,11 @@ export class AstVisitor {
 }
 
 var _evalListCache = [[],[0],[0,0],[0,0,0],[0,0,0,0],[0,0,0,0,0]];
-function evalList(context, exps:List, formatters){
+function evalList(context, exps:List){
   var length = exps.length;
   var result = _evalListCache[length];
   for (var i = 0; i < length; i++) {
-    result[i] = exps[i].eval(context, formatters);
+    result[i] = exps[i].eval(context);
   }
   return result;
 }

modules/change_detection/src/parser/parser.js

@@ -24,9 +24,14 @@ export class Parser {
     this._closureMap = closureMap;
   }
 
-  parse(input:string):AST {
+  parseAction(input:string):AST {
     var tokens = this._lexer.tokenize(input);
-    return new _ParseAST(input, tokens, this._closureMap).parseChain();
+    return new _ParseAST(input, tokens, this._closureMap, true).parseChain();
+  }
+
+  parseBinding(input:string):AST {
+    var tokens = this._lexer.tokenize(input);
+    return new _ParseAST(input, tokens, this._closureMap, false).parseChain();
   }
 }
 
@@ -34,12 +39,14 @@ class _ParseAST {
   @FIELD('final input:String')
   @FIELD('final tokens:List<Token>')
   @FIELD('final closureMap:ClosureMap')
+  @FIELD('final parseAction:boolean')
   @FIELD('index:int')
-  constructor(input:string, tokens:List, closureMap:ClosureMap) {
+  constructor(input:string, tokens:List, closureMap:ClosureMap, parseAction:boolean) {
     this.input = input;
     this.tokens = tokens;
     this.index = 0;
     this.closureMap = closureMap;
+    this.parseAction = parseAction;
   }
 
   peek(offset:int):Token {
@@ -79,17 +86,14 @@ class _ParseAST {
 
   parseChain():AST {
     var exprs = [];
-    var isChain = false;
     while (this.index < this.tokens.length) {
       var expr = this.parseFormatter();
       ListWrapper.push(exprs, expr);
 
       while (this.optionalCharacter($SEMICOLON)) {
-        isChain = true;
+        if (! this.parseAction) {
-      }
+          this.error("Binding expression cannot contain chained expression");
-
+        }
-      if (isChain && expr instanceof Formatter) {
-        this.error('Cannot have a formatter in a chain');
       }
     }
     return ListWrapper.first(exprs);
@@ -98,6 +102,9 @@ class _ParseAST {
   parseFormatter() {
     var result = this.parseExpression();
     while (this.optionalOperator("|")) {
+      if (this.parseAction) {
+        this.error("Cannot have a formatter in an action expression");
+      }
       var name = this.parseIdentifier();
       var args = ListWrapper.create();
       while (this.optionalCharacter($COLON)) {

modules/change_detection/test/parser/parser_spec.js

@@ -1,7 +1,8 @@
 import {ddescribe, describe, it, iit, expect, beforeEach} from 'test_lib/test_lib';
-import {BaseException} from 'facade/lang';
+import {BaseException, isBlank} from 'facade/lang';
 import {Parser} from 'change_detection/parser/parser';
 import {Lexer} from 'change_detection/parser/lexer';
+import {Formatter, LiteralPrimitive} from 'change_detection/parser/ast';
 import {ClosureMap} from 'change_detection/parser/closure_map';
 
 class TestData {
@@ -9,6 +10,14 @@ class TestData {
     this.a = a;
     this.b = b;
   }
+
+  constant() {
+    return "constant";
+  }
+
+  add(a, b) {
+    return a + b;
+  }
 }
 
 class ContextWithErrors {
@@ -22,100 +31,75 @@ export function main() {
     return new TestData(a, b);
   }
 
-  var context = td();
-  var formatters;
-
   function createParser() {
     return new Parser(new Lexer(), new ClosureMap());
   }
 
-  function _eval(text) {
+  function parseAction(text) {
-    return new Parser(new Lexer(), new ClosureMap()).parse(text)
+    return createParser().parseAction(text);
-      .eval(context, formatters);
   }
 
-  function expectEval(text) {
+  function parseBinding(text) {
-    return expect(_eval(text));
+    return createParser().parseBinding(text);
+  }
+
+  function expectEval(text, passedInContext = null) {
+    var c = isBlank(passedInContext) ? td() : passedInContext;
+    return expect(parseAction(text).eval(c));
   }
 
   function expectEvalError(text) {
-    return expect(() => _eval(text));
+    return expect(() => parseAction(text).eval(td()));
   }
 
   describe("parser", () => {
-    describe("field access", () => {
+    describe("parseAction", () => {
-      var parser;
+      it("should parse field access", () => {
-
+        expectEval("a", td(999)).toEqual(999);
-      beforeEach(() => {
+        expectEval("a.a", td(td(999))).toEqual(999);
-        parser = createParser();
       });
 
-      it("should parse field access",() => {
-        var exp = parser.parse("a");
-        var context = td(999);
-        expect(exp.eval(context, null)).toEqual(999);
-      });
-
-      it("should parse nested field access",() => {
-        var exp = parser.parse("a.a");
-        var context = td(td(999));
-        expect(exp.eval(context, null)).toEqual(999);
-      });
-    });
-
-    describe('expressions', () => {
-
       it('should parse numerical expressions', () => {
         expectEval("1").toEqual(1);
       });
 
-
       it('should parse unary - expressions', () => {
         expectEval("-1").toEqual(-1);
         expectEval("+1").toEqual(1);
       });
 
-
       it('should parse unary ! expressions', () => {
         expectEval("!true").toEqual(!true);
       });
 
-
       it('should parse multiplicative expressions', () => {
-        expectEval("3*4/2%5").toEqual(3*4/2%5);
+        expectEval("3*4/2%5").toEqual(3 * 4 / 2 % 5);
-        // TODO(rado): This exists only in Dart, figure out whether to support it.
-        // expectEval("3*4~/2%5")).toEqual(3*4~/2%5);
       });
 
-
       it('should parse additive expressions', () => {
-        expectEval("3+6-2").toEqual(3+6-2);
+        expectEval("3+6-2").toEqual(3 + 6 - 2);
       });
 
-
       it('should parse relational expressions', () => {
-        expectEval("2<3").toEqual(2<3);
+        expectEval("2<3").toEqual(2 < 3);
-        expectEval("2>3").toEqual(2>3);
+        expectEval("2>3").toEqual(2 > 3);
-        expectEval("2<=2").toEqual(2<=2);
+        expectEval("2<=2").toEqual(2 <= 2);
-        expectEval("2>=2").toEqual(2>=2);
+        expectEval("2>=2").toEqual(2 >= 2);
       });
 
-
       it('should parse equality expressions', () => {
-        expectEval("2==3").toEqual(2==3);
+        expectEval("2==3").toEqual(2 == 3);
-        expectEval("2!=3").toEqual(2!=3);
+        expectEval("2!=3").toEqual(2 != 3);
       });
 
-
       it('should parse logicalAND expressions', () => {
-        expectEval("true&&true").toEqual(true&&true);
+        expectEval("true&&true").toEqual(true && true);
-        expectEval("true&&false").toEqual(true&&false);
+        expectEval("true&&false").toEqual(true && false);
       });
 
-
       it('should parse logicalOR expressions', () => {
-        expectEval("false||true").toEqual(false||true);
+        expectEval("false||true").toEqual(false || true);
-        expectEval("false||false").toEqual(false||false);
+        expectEval("false||false").toEqual(false || false);
       });
 
       it('should parse ternary/conditional expressions', () => {
@@ -132,68 +116,61 @@ export function main() {
       });
 
       it('should behave gracefully with a null scope', () => {
-        var exp = createParser().parse("null");
+        var exp = createParser().parseAction("null");
-        expect(exp.eval(null, null)).toEqual(null);
+        expect(exp.eval(null)).toEqual(null);
       });
 
       it('should eval binary operators with null as null', () => {
-        expectEval("null < 0").toBeNull();
+        expectEvalError("null < 0").toThrowError();
-        expectEval("null * 3").toBeNull();
+        expectEvalError("null * 3").toThrowError();
-        expectEval("null + 6").toBeNull();
+        expectEvalError("null + 6").toThrowError();
-        expectEval("5 + null").toBeNull();
+        expectEvalError("5 + null").toThrowError();
-        expectEval("null - 4").toBeNull();
+        expectEvalError("null - 4").toThrowError();
-        expectEval("3 - null").toBeNull();
+        expectEvalError("3 - null").toThrowError();
-        expectEval("null + null").toBeNull();
+        expectEvalError("null + null").toThrowError();
-        expectEval("null - null").toBeNull();
+        expectEvalError("null - null").toThrowError();
-      });
-    });
-
-    describe('formatters', () => {
-      beforeEach(() => {
-        formatters = {
-          "uppercase": (s) => s.toUpperCase(),
-          "lowercase": (s) => s.toLowerCase(),
-          "increment": (a,b) => a + b
-        }
       });
 
-      it('should call a formatter', () => {
+      describe("error handling", () => {
-        expectEval("'Foo'|uppercase").toEqual("FOO");
+        it('should throw on incorrect ternary operator syntax', () => {
-        expectEval("'fOo'|uppercase|lowercase").toEqual("foo");
+          expectEvalError("true?1").
-      });
-
-      it('should call a formatter with arguments', () => {
-        expectEval("1|increment:2").toEqual(3);
-      });
-
-      it('should throw when invalid formatter', () => {
-        expectEvalError("1|nonexistent").toThrowError('No formatter \'nonexistent\' found!');
-      });;
-
-      it('should not allow formatters in a chain', () => {
-        expectEvalError("1;'World'|hello").
-            toThrowError(new RegExp('Cannot have a formatter in a chain'));
-        expectEvalError("'World'|hello;1").
-            toThrowError(new RegExp('Cannot have a formatter in a chain'));
-      });
-    });
-    
-    describe("error handling", () => {
-      it('should throw on incorrect ternary operator syntax', () => {
-        expectEvalError("true?1").
             toThrowError(new RegExp('Parser Error: Conditional expression true\\?1 requires all 3 expressions'));
+        });
+
+        it('should pass exceptions', () => {
+          expect(() => {
+            createParser().parseAction('boo').eval(new ContextWithErrors());
+          }).toThrowError('boo to you');
+        });
+
+        it('should only allow identifier or keyword as member names', () => {
+          expect(() => parseAction("x.(")).toThrowError(new RegExp('identifier or keyword'));
+          expect(() => parseAction('x. 1234')).toThrowError(new RegExp('identifier or keyword'));
+          expect(() => parseAction('x."foo"')).toThrowError(new RegExp('identifier or keyword'));
+        });
+
+        it("should error when using formatters", () => {
+          expectEvalError('x|blah').toThrowError(new RegExp('Cannot have a formatter'));
+        });
+      });
+    });
+
+    describe("parseBinding", () => {
+      it("should parse formatters", function () {
+        var exp = parseBinding("'Foo'|uppercase");
+        expect(exp).toBeAnInstanceOf(Formatter);
+        expect(exp.name).toEqual("uppercase");
       });
 
-      it('should pass exceptions', () => {
+      it("should parse formatters with args", function () {
-        expect(() => {
+        var exp = parseBinding("1|increment:2");
-          createParser().parse('boo').eval(new ContextWithErrors(), null);
+        expect(exp).toBeAnInstanceOf(Formatter);
-        }).toThrowError('boo to you');
+        expect(exp.name).toEqual("increment");
+        expect(exp.args[0]).toBeAnInstanceOf(LiteralPrimitive);
       });
 
-      it('should only allow identifier or keyword as member names', () => {
+      it('should throw on chain expressions', () => {
-        expectEvalError('x.(').toThrowError(new RegExp('identifier or keyword'));
+        expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression"));
-        expectEvalError('x. 1234').toThrowError(new RegExp('identifier or keyword'));
-        expectEvalError('x."foo"').toThrowError(new RegExp('identifier or keyword'));
       });
     });
   });