From 90fd1a92272c063b868819396c5035729fc8a2ad Mon Sep 17 00:00:00 2001 From: vsavkin Date: Thu, 6 Nov 2014 12:00:09 -0800 Subject: [PATCH] refactor(Parser): cleanup --- modules/change_detection/src/parser/ast.js | 3 + .../src/parser/closure_map.es6 | 5 +- modules/change_detection/src/parser/parser.js | 18 +- .../test/parser/parser_spec.js | 381 +++++++++++------- 4 files changed, 259 insertions(+), 148 deletions(-) diff --git a/modules/change_detection/src/parser/ast.js b/modules/change_detection/src/parser/ast.js index dac56d4e49..e342381af4 100644 --- a/modules/change_detection/src/parser/ast.js +++ b/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)); } diff --git a/modules/change_detection/src/parser/closure_map.es6 b/modules/change_detection/src/parser/closure_map.es6 index b77b76d303..f2ab3ccbae 100644 --- a/modules/change_detection/src/parser/closure_map.es6 +++ b/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);`); } } diff --git a/modules/change_detection/src/parser/parser.js b/modules/change_detection/src/parser/parser.js index 1e111677db..776b87c8ed 100644 --- a/modules/change_detection/src/parser/parser.js +++ b/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); diff --git a/modules/change_detection/test/parser/parser_spec.js b/modules/change_detection/test/parser/parser_spec.js index 3e4ee33955..b1e7102371 100644 --- a/modules/change_detection/test/parser/parser_spec.js +++ b/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,91 +50,235 @@ export function main() { describe("parser", () => { describe("parseAction", () => { - it("should parse field access", () => { - expectEval("a", td(999)).toEqual(999); - expectEval("a.a", td(td(999))).toEqual(999); + describe("basic expressions", () => { + 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); + }); + + it('should parse unary ! expressions', () => { + expectEval("!true").toEqual(!true); + expectEval("!!true").toEqual(!!true); + }); + + it('should parse multiplicative expressions', () => { + expectEval("3*4/2%5").toEqual(3 * 4 / 2 % 5); + }); + + it('should parse additive expressions', () => { + 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<=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); + }); + + it('should parse logicalAND expressions', () => { + expectEval("true&&true").toEqual(true && true); + expectEval("true&&false").toEqual(true && false); + }); + + it('should parse logicalOR expressions', () => { + expectEval("false||true").toEqual(false || true); + expectEval("false||false").toEqual(false || false); + }); + + it('should short-circuit AND operator', () => { + expectEval('false && a()', td(() => {throw "BOOM"})).toBe(false); + }); + + it('should short-circuit OR operator', () => { + expectEval('true || a()', td(() => {throw "BOOM"})).toBe(true); + }); + + it('should evaluate grouped expressions', () => { + expectEval("(1+2)*3").toEqual((1+2)*3); + }); + + it('should auto convert ints to strings', () => { + expectEval("'str ' + 4").toEqual("str 4"); + expectEval("4 + ' str'").toEqual("4 str"); + expectEval("4 + 4").toEqual(8); + expectEval("4 + 4 + ' str'").toEqual("8 str"); + expectEval("'str ' + 4 + 4").toEqual("str 44"); + }); + + it('should throw when one of the operands is null', () => { + expectEvalError("null < 0").toThrowError(); + expectEvalError("null * 3").toThrowError(); + expectEvalError("null + 6").toThrowError(); + expectEvalError("5 + null").toThrowError(); + expectEvalError("null - 4").toThrowError(); + expectEvalError("3 - null").toThrowError(); + expectEvalError("null + null").toThrowError(); + expectEvalError("null - null").toThrowError(); + }); }); - it('should throw when accessing a field on null', () => { - expectEvalError("a.a.a").toThrowError(); + describe("literals", () => { + it('should evaluate array', () => { + expectEval("[1][0]").toEqual(1); + expectEval("[[1]][0][0]").toEqual(1); + expectEval("[]").toEqual([]); + expectEval("[].length").toEqual(0); + expectEval("[1, 2].length").toEqual(2); + }); + + it('should evaluate map', () => { + expectEval("{}").toEqual(MapWrapper.create()); + expectEval("{a:'b'}['a']").toEqual('b'); + expectEval("{'a':'b'}['a']").toEqual('b'); + expectEval("{\"a\":'b'}['a']").toEqual('b'); + expectEval("{\"a\":'b'}['a']").toEqual("b"); + expectEval("{}['a']").not.toBeDefined(); + 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')); + }); }); - it('should parse numerical expressions', () => { - expectEval("1").toEqual(1); + 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')); + }); }); - it('should parse unary - expressions', () => { - expectEval("-1").toEqual(-1); - expectEval("+1").toEqual(1); + 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(); + }); }); - it('should parse unary ! expressions', () => { - expectEval("!true").toEqual(!true); + 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); + }); }); - it('should parse multiplicative expressions', () => { - expectEval("3*4/2%5").toEqual(3 * 4 / 2 % 5); + 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')); + }); }); - it('should parse additive expressions', () => { - expectEval("3+6-2").toEqual(3 + 6 - 2); + 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")); + }); }); - it('should parse relational expressions', () => { - expectEval("2<3").toEqual(2 < 3); - expectEval("2>3").toEqual(2 > 3); - expectEval("2<=2").toEqual(2 <= 2); - expectEval("2>=2").toEqual(2 >= 2); - }); + describe("general error handling", () => { + it("should throw on an unexpected token", () => { + expectEvalError("[1,2] trac") + .toThrowError(new RegExp('Unexpected token \'trac\'')); + }); - it('should parse equality expressions', () => { - expectEval("2==3").toEqual(2 == 3); - expectEval("2!=3").toEqual(2 != 3); - }); + it('should throw a reasonable error for unconsumed tokens', () => { + expectEvalError(")").toThrowError(new RegExp("Unexpected token \\) at column 1 in \\[\\)\\]")); + }); - it('should parse logicalAND expressions', () => { - expectEval("true&&true").toEqual(true && true); - expectEval("true&&false").toEqual(true && false); - }); - - it('should parse logicalOR expressions', () => { - expectEval("false||true").toEqual(false || true); - expectEval("false||false").toEqual(false || false); - }); - - 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')); - }); - - it('should auto convert ints to strings', () => { - expectEval("'str ' + 4").toEqual("str 4"); - expectEval("4 + ' str'").toEqual("4 str"); - expectEval("4 + 4").toEqual(8); - expectEval("4 + 4 + ' str'").toEqual("8 str"); - expectEval("'str ' + 4 + 4").toEqual("str 44"); - }); - - it('should eval binary operators with null as null', () => { - expectEvalError("null < 0").toThrowError(); - expectEvalError("null * 3").toThrowError(); - expectEvalError("null + 6").toThrowError(); - expectEvalError("5 + null").toThrowError(); - expectEvalError("null - 4").toThrowError(); - expectEvalError("3 - null").toThrowError(); - 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 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", () => { @@ -149,92 +287,47 @@ export function main() { it('should pass exceptions', () => { expect(() => { - createParser().parseAction('boo').eval(new ContextWithErrors()); + createParser().parseAction('a()').eval(td(() => {throw new BaseException("boo to you")})); }).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); + describe("multiple statements", () => { + it("should return the last non-blank value", () => { + expectEval("a=1;b=3;a+b").toEqual(4); + expectEval("1;;").toEqual(1); + }); }); + }); - 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); - }); - - it('should evaluate array', () => { - expectEval("[1][0]").toEqual(1); - expectEval("[[1]][0][0]").toEqual(1); - expectEval("[]").toEqual([]); - expectEval("[].length").toEqual(0); - 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'); - expectEval("{'a':'b'}['a']").toEqual('b'); - expectEval("{\"a\":'b'}['a']").toEqual('b'); - expectEval("{\"a\":'b'}['a']").toEqual("b"); - expectEval("{}['a']").not.toBeDefined(); - expectEval("{\"a\":'b'}['invalid']").not.toBeDefined(); - }); - - describe("parseBinding", () => { - it("should parse formatters", function () { + describe("parseBinding", () => { + 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 throw on chain expressions', () => { - expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression")); + 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")); + }); }); }); }