refactor(Parser): cleanup

modules/change_detection/src/parser/ast.js

@@ -326,6 +326,9 @@ export class FunctionCall extends AST {
 
   eval(context) {
     var obj = this.target.eval(context);
+    if (! (obj instanceof Function)) {
+      throw new BaseException(`${obj} is not a function`);
+    }
     return FunctionWrapper.apply(obj, evalList(context, this.args));
   }
 

modules/change_detection/src/parser/closure_map.es6

@@ -10,6 +10,9 @@ export class ClosureMap {
   }
 
   fn(name:string) {
-    return new Function('o', 'args', 'return o.' + name + '.apply(o, args);');
+    var method = `o.${name}`;
+    return new Function('o', 'args',
+      `if (!${method}) throw new Error('"${name}" is undefined');` +
+      `return ${method}.apply(o, args);`);
   }
 }

modules/change_detection/src/parser/parser.js

@@ -115,7 +115,7 @@ class _ParseAST {
   expectIdentifierOrKeywordOrString():string {
     var n = this.next;
     if (!n.isIdentifier() && !n.isKeyword() && !n.isString()) {
-      this.error(`Unexpected token ${n}, expected identifier, or keyword, or string`)
+      this.error(`Unexpected token ${n}, expected identifier, keyword, or string`)
     }
     this.advance();
     return n.toString();
@@ -127,10 +127,13 @@ class _ParseAST {
       var expr = this.parseFormatter();
       ListWrapper.push(exprs, expr);
 
-      while (this.optionalCharacter($SEMICOLON)) {
+      if (this.optionalCharacter($SEMICOLON)) {
         if (! this.parseAction) {
           this.error("Binding expression cannot contain chained expression");
         }
+        while (this.optionalCharacter($SEMICOLON)){} //read all semicolons
+      } else if (this.index < this.tokens.length) {
+        this.error(`Unexpected token '${this.next}'`);
       }
     }
     return exprs.length == 1 ? exprs[0] : new Chain(exprs);
@@ -163,6 +166,10 @@ class _ParseAST {
         this.error(`Expression ${expression} is not assignable`);
       }
 
+      if (!this.parseAction) {
+        this.error("Binding expression cannot contain assignments");
+      }
+
       this.expectOperator('=');
       result = new Assignment(result, this.parseConditional());
     }
@@ -303,7 +310,12 @@ class _ParseAST {
   }
 
   parsePrimary() {
-    if (this.next.isKeywordNull() || this.next.isKeywordUndefined()) {
+    if (this.optionalCharacter($LPAREN)) {
+      var result = this.parseFormatter();
+      this.expectCharacter($RPAREN);
+      return result;
+
+    } else if (this.next.isKeywordNull() || this.next.isKeywordUndefined()) {
       this.advance();
       return new LiteralPrimitive(null);
 

modules/change_detection/test/parser/parser_spec.js

@@ -1,4 +1,4 @@
-import {ddescribe, describe, it, iit, expect, beforeEach} from 'test_lib/test_lib';
+import {ddescribe, describe, it, xit, iit, expect, beforeEach} from 'test_lib/test_lib';
 import {BaseException, isBlank} from 'facade/lang';
 import {MapWrapper} from 'facade/collection';
 import {Parser} from 'change_detection/parser/parser';
@@ -22,12 +22,6 @@ class TestData {
   }
 }
 
-class ContextWithErrors {
-  get boo() {
-    throw new BaseException("boo to you");
-  }
-}
-
 export function main() {
   function td(a = 0, b = 0, fnReturnValue = "constant") {
     return new TestData(a, b, fnReturnValue);
@@ -56,19 +50,20 @@ export function main() {
 
   describe("parser", () => {
     describe("parseAction", () => {
-      it("should parse field access", () => {
+      describe("basic expressions", () => {
-        expectEval("a", td(999)).toEqual(999);
-        expectEval("a.a", td(td(999))).toEqual(999);
-      });
-
-      it('should throw when accessing a field on null', () => {
-        expectEvalError("a.a.a").toThrowError();
-      });
-
         it('should parse numerical expressions', () => {
           expectEval("1").toEqual(1);
         });
 
+        it('should parse strings', () => {
+          expectEval("'1'").toEqual('1');
+          expectEval('"1"').toEqual('1');
+        });
+
+        it('should parse null', () => {
+          expectEval("null").toBe(null);
+        });
+
         it('should parse unary - expressions', () => {
           expectEval("-1").toEqual(-1);
           expectEval("+1").toEqual(1);
@@ -76,6 +71,7 @@ export function main() {
 
         it('should parse unary ! expressions', () => {
           expectEval("!true").toEqual(!true);
+          expectEval("!!true").toEqual(!!true);
         });
 
         it('should parse multiplicative expressions', () => {
@@ -108,14 +104,16 @@ export function main() {
           expectEval("false||false").toEqual(false || false);
         });
 
-      it('should parse ternary/conditional expressions', () => {
+        it('should short-circuit AND operator', () => {
-        expectEval("7==3+4?10:20").toEqual(10);
+          expectEval('false && a()', td(() => {throw "BOOM"})).toBe(false);
-        expectEval("false?10:20").toEqual(20);
         });
 
-      it('should throw on incorrect ternary operator syntax', () => {
+        it('should short-circuit OR operator', () => {
-        expectEvalError("true?1").
+          expectEval('true || a()', td(() => {throw "BOOM"})).toBe(true);
-          toThrowError(new RegExp('Parser Error: Conditional expression true\\?1 requires all 3 expressions'));
+        });
+
+        it('should evaluate grouped expressions', () => {
+          expectEval("(1+2)*3").toEqual((1+2)*3);
         });
 
         it('should auto convert ints to strings', () => {
@@ -126,7 +124,7 @@ export function main() {
           expectEval("'str ' + 4 + 4").toEqual("str 44");
         });
 
-      it('should eval binary operators with null as null', () => {
+        it('should throw when one of the operands is null', () => {
           expectEvalError("null < 0").toThrowError();
           expectEvalError("null * 3").toThrowError();
           expectEvalError("null + 6").toThrowError();
@@ -136,65 +134,9 @@ export function main() {
           expectEvalError("null + null").toThrowError();
           expectEvalError("null - null").toThrowError();
         });
-
-      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'));
-      });
-
-      it('should pass exceptions', () => {
-        expect(() => {
-          createParser().parseAction('boo').eval(new ContextWithErrors());
-        }).toThrowError('boo to you');
-      });
-
-      it('should evaluate assignments', () => {
-        var context = td();
-        expectEval("a=12", context).toEqual(12);
-        expect(context.a).toEqual(12);
-
-        context = td(td(td()));
-        expectEval("a.a.a=123;", context).toEqual(123);
-        expect(context.a.a.a).toEqual(123);
-
-        context = td();
-        expectEval("a=123; b=234", context).toEqual(234);
-        expect(context.a).toEqual(123);
-        expect(context.b).toEqual(234);
-
-        context = td([100]);
-        expectEval('a[0] = 200', context).toEqual(200);
-        expect(context.a[0]).toEqual(200);
-
-        context = td(MapWrapper.createFromPairs([["key", 100]]));
-        expectEval('a["key"] = 200', context).toEqual(200);
-        expect(MapWrapper.get(context.a, "key")).toEqual(200);
-
-        context = td([MapWrapper.createFromPairs([["key", 100]])]);
-        expectEval('a[0]["key"] = 200', context).toEqual(200);
-        expect(MapWrapper.get(context.a[0], "key")).toEqual(200);
-      });
-
-      it('should throw on bad assignment', () => {
-        expectEvalError("5=4").toThrowError(new RegExp("Expression 5 is not assignable"));
-      });
-
-      it("should evaluate method calls", () => {
-        expectEval("fn()", td(0,0, "constant")).toEqual("constant");
-        expectEval("add(1,2)").toEqual(3);
-        expectEval("a.add(1,2)", td(td())).toEqual(3);
-        expectEval("fn().add(1,2)", td(0,0,td())).toEqual(3);
-      });
-
-      it("should evaluate function calls", () => {
-        expectEval("fn()(1,2)", td(0, 0, (a,b) => a + b)).toEqual(3);
       });
 
+      describe("literals", () => {
         it('should evaluate array', () => {
           expectEval("[1][0]").toEqual(1);
           expectEval("[[1]][0][0]").toEqual(1);
@@ -203,10 +145,6 @@ export function main() {
           expectEval("[1, 2].length").toEqual(2);
         });
 
-      it("should error when unfinished exception", () => {
-        expectEvalError('a[0').toThrowError(new RegExp("Missing expected ]"));
-      });
-
         it('should evaluate map', () => {
           expectEval("{}").toEqual(MapWrapper.create());
           expectEval("{a:'b'}['a']").toEqual('b');
@@ -217,23 +155,178 @@ export function main() {
           expectEval("{\"a\":'b'}['invalid']").not.toBeDefined();
         });
 
+        it('should only allow identifier, string, or keyword as map key', () => {
+          expectEvalError('{(:0}').toThrowError(new RegExp('expected identifier, keyword, or string'));
+          expectEvalError('{1234:0}').toThrowError(new RegExp('expected identifier, keyword, or string'));
+        });
+      });
+
+      describe("member access", () => {
+        it("should parse field access", () => {
+          expectEval("a", td(999)).toEqual(999);
+          expectEval("a.a", td(td(999))).toEqual(999);
+        });
+
+        it('should throw when accessing a field on null', () => {
+          expectEvalError("a.a.a").toThrowError();
+        });
+
+        it('should only allow identifier or keyword as member names', () => {
+          expectEvalError('x.(').toThrowError(new RegExp('identifier or keyword'));
+          expectEvalError('x. 1234').toThrowError(new RegExp('identifier or keyword'));
+          expectEvalError('x."foo"').toThrowError(new RegExp('identifier or keyword'));
+        });
+      });
+
+      describe("method calls", () => {
+        it("should evaluate method calls", () => {
+          expectEval("fn()", td(0, 0, "constant")).toEqual("constant");
+          expectEval("add(1,2)").toEqual(3);
+          expectEval("a.add(1,2)", td(td())).toEqual(3);
+          expectEval("fn().add(1,2)", td(0, 0, td())).toEqual(3);
+        });
+
+        it('should throw when no method', () => {
+          expectEvalError("blah()").toThrowError();
+        });
+      });
+
+      describe("functional calls", () => {
+        it("should evaluate function calls", () => {
+          expectEval("fn()(1,2)", td(0, 0, (a, b) => a + b)).toEqual(3);
+        });
+
+        it('should throw on non-function function calls', () => {
+          expectEvalError("4()").toThrowError(new RegExp('4 is not a function'));
+        });
+
+        it('should parse functions for object indices', () => {
+          expectEval('a[b()]()', td([()=>6], () => 0)).toEqual(6);
+        });
+      });
+
+      describe("conditional", () => {
+        it('should parse ternary/conditional expressions', () => {
+          expectEval("7==3+4?10:20").toEqual(10);
+          expectEval("false?10:20").toEqual(20);
+        });
+
+        it('should throw on incorrect ternary operator syntax', () => {
+          expectEvalError("true?1").
+            toThrowError(new RegExp('Parser Error: Conditional expression true\\?1 requires all 3 expressions'));
+        });
+      });
+
+      describe("assignment", () => {
+        it("should support field assignments", () => {
+          var context = td();
+          expectEval("a=12", context).toEqual(12);
+          expect(context.a).toEqual(12);
+        });
+
+        it("should support nested field assignments", () => {
+          var context = td(td(td()));
+          expectEval("a.a.a=123;", context).toEqual(123);
+          expect(context.a.a.a).toEqual(123);
+        });
+
+        it("should support multiple assignments", () => {
+          var context = td();
+          expectEval("a=123; b=234", context).toEqual(234);
+          expect(context.a).toEqual(123);
+          expect(context.b).toEqual(234);
+        });
+
+        it("should support array updates", () => {
+          var context = td([100]);
+          expectEval('a[0] = 200', context).toEqual(200);
+          expect(context.a[0]).toEqual(200);
+        });
+
+        it("should support map updates", () => {
+          var context = td(MapWrapper.createFromPairs([["key", 100]]));
+          expectEval('a["key"] = 200', context).toEqual(200);
+          expect(MapWrapper.get(context.a, "key")).toEqual(200);
+        });
+        
+        it("should support array/map updates", () => {
+          var context = td([MapWrapper.createFromPairs([["key", 100]])]);
+          expectEval('a[0]["key"] = 200', context).toEqual(200);
+          expect(MapWrapper.get(context.a[0], "key")).toEqual(200);
+        });
+
+        it('should allow assignment after array dereference', () => {
+          var context = td([td()]);
+          expectEval('a[0].a = 200', context).toEqual(200);
+          expect(context.a[0].a).toEqual(200);
+        });
+
+        it('should throw on bad assignment', () => {
+          expectEvalError("5=4").toThrowError(new RegExp("Expression 5 is not assignable"));
+        });
+      });
+
+      describe("general error handling", () => {
+        it("should throw on an unexpected token", () => {
+          expectEvalError("[1,2] trac")
+            .toThrowError(new RegExp('Unexpected token \'trac\''));
+        });
+
+        it('should throw a reasonable error for unconsumed tokens', () => {
+          expectEvalError(")").toThrowError(new RegExp("Unexpected token \\) at column 1 in \\[\\)\\]"));
+        });
+
+        it('should throw on missing expected token', () => {
+          expectEvalError("a(b").toThrowError(new RegExp("Missing expected \\) at the end of the expression \\[a\\(b\\]"));
+        });
+      });
+
+      it("should error when using formatters", () => {
+        expectEvalError('x|blah').toThrowError(new RegExp('Cannot have a formatter'));
+      });
+
+      it('should pass exceptions', () => {
+        expect(() => {
+          createParser().parseAction('a()').eval(td(() => {throw new BaseException("boo to you")}));
+        }).toThrowError('boo to you');
+      });
+
+      describe("multiple statements", () => {
+        it("should return the last non-blank value", () => {
+          expectEval("a=1;b=3;a+b").toEqual(4);
+          expectEval("1;;").toEqual(1);
+        });
+      });
+    });
+
     describe("parseBinding", () => {
-        it("should parse formatters", function () {
+      describe("formatters", () => {
+        it("should parse formatters", () => {
           var exp = parseBinding("'Foo'|uppercase");
           expect(exp).toBeAnInstanceOf(Formatter);
           expect(exp.name).toEqual("uppercase");
         });
 
-        it("should parse formatters with args", function () {
+        it("should parse formatters with args", () => {
           var exp = parseBinding("1|increment:2");
           expect(exp).toBeAnInstanceOf(Formatter);
           expect(exp.name).toEqual("increment");
           expect(exp.args[0]).toBeAnInstanceOf(LiteralPrimitive);
         });
 
+        it('should only allow identifier or keyword as formatter names', () => {
+          expect(() => parseBinding('"Foo"|(')).toThrowError(new RegExp('identifier or keyword'));
+          expect(() => parseBinding('"Foo"|1234')).toThrowError(new RegExp('identifier or keyword'));
+          expect(() => parseBinding('"Foo"|"uppercase"')).toThrowError(new RegExp('identifier or keyword'));
+        });
+      });
+
       it('should throw on chain expressions', () => {
         expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression"));
       });
+
+      it('should throw on assignmnt', () => {
+        expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression"));
       });
     });
   });