From f9745327e61a98aec906e249a6077412397c1e5f Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 4 Jun 2015 19:06:09 +0200 Subject: [PATCH] fix(Parser): Parse pipes in arguments fixes #1680 --- .../src/change_detection/parser/parser.ts | 69 ++++++++----------- .../change_detection/parser/parser_spec.ts | 62 +++++++++-------- .../test/change_detection/parser/unparser.ts | 4 +- .../change_detection/parser/unparser_spec.ts | 4 +- 4 files changed, 67 insertions(+), 72 deletions(-) diff --git a/modules/angular2/src/change_detection/parser/parser.ts b/modules/angular2/src/change_detection/parser/parser.ts index b34a73015c..ec35bb8c94 100644 --- a/modules/angular2/src/change_detection/parser/parser.ts +++ b/modules/angular2/src/change_detection/parser/parser.ts @@ -216,10 +216,21 @@ class _ParseAST { parsePipe() { var result = this.parseExpression(); if (this.optionalOperator("|")) { - return this.parseInlinedPipe(result); - } else { - return result; + if (this.parseAction) { + this.error("Cannot have a pipe in an action expression"); + } + + do { + var name = this.expectIdentifierOrKeyword(); + var args = []; + while (this.optionalCharacter($COLON)) { + ListWrapper.push(args, this.parsePipe()); + } + result = new Pipe(result, name, args, true); + } while (this.optionalOperator("|")); } + + return result; } parseExpression() { @@ -249,13 +260,13 @@ class _ParseAST { var result = this.parseLogicalOr(); if (this.optionalOperator('?')) { - var yes = this.parseExpression(); + var yes = this.parsePipe(); if (!this.optionalCharacter($COLON)) { var end = this.inputIndex; var expression = this.input.substring(start, end); this.error(`Conditional expression ${expression} requires all 3 expressions`); } - var no = this.parseExpression(); + var no = this.parsePipe(); return new Conditional(result, yes, no); } else { return result; @@ -368,7 +379,7 @@ class _ParseAST { result = this.parseAccessMemberOrMethodCall(result, true); } else if (this.optionalCharacter($LBRACKET)) { - var key = this.parseExpression(); + var key = this.parsePipe(); this.expectCharacter($RBRACKET); result = new KeyedAccess(result, key); @@ -385,9 +396,9 @@ class _ParseAST { parsePrimary() { if (this.optionalCharacter($LPAREN)) { - var result = this.parsePipe(); + let result = this.parsePipe(); this.expectCharacter($RPAREN); - return result; + return result } else if (this.next.isKeywordNull() || this.next.isKeywordUndefined()) { this.advance(); @@ -434,7 +445,7 @@ class _ParseAST { var result = []; if (!this.next.isCharacter(terminator)) { do { - ListWrapper.push(result, this.parseExpression()); + ListWrapper.push(result, this.parsePipe()); } while (this.optionalCharacter($COMMA)); } return result; @@ -449,7 +460,7 @@ class _ParseAST { var key = this.expectIdentifierOrKeywordOrString(); ListWrapper.push(keys, key); this.expectCharacter($COLON); - ListWrapper.push(values, this.parseExpression()); + ListWrapper.push(values, this.parsePipe()); } while (this.optionalCharacter($COMMA)); this.expectCharacter($RBRACE); } @@ -457,50 +468,28 @@ class _ParseAST { } parseAccessMemberOrMethodCall(receiver, isSafe: boolean = false): AST { - var id = this.expectIdentifierOrKeyword(); + let id = this.expectIdentifierOrKeyword(); if (this.optionalCharacter($LPAREN)) { - var args = this.parseCallArguments(); + let args = this.parseCallArguments(); this.expectCharacter($RPAREN); - var fn = this.reflector.method(id); + let fn = this.reflector.method(id); return isSafe ? new SafeMethodCall(receiver, id, fn, args) : new MethodCall(receiver, id, fn, args); } else { - var getter = this.reflector.getter(id); - var setter = this.reflector.setter(id); - var am = isSafe ? new SafeAccessMember(receiver, id, getter, setter) : - new AccessMember(receiver, id, getter, setter); - - if (this.optionalOperator("|")) { - return this.parseInlinedPipe(am); - } else { - return am; - } + let getter = this.reflector.getter(id); + let setter = this.reflector.setter(id); + return isSafe ? new SafeAccessMember(receiver, id, getter, setter) : + new AccessMember(receiver, id, getter, setter); } } - parseInlinedPipe(result) { - do { - if (this.parseAction) { - this.error("Cannot have a pipe in an action expression"); - } - var name = this.expectIdentifierOrKeyword(); - var args = ListWrapper.create(); - while (this.optionalCharacter($COLON)) { - ListWrapper.push(args, this.parseExpression()); - } - result = new Pipe(result, name, args, true); - } while (this.optionalOperator("|")); - - return result; - } - parseCallArguments() { if (this.next.isCharacter($RPAREN)) return []; var positionals = []; do { - ListWrapper.push(positionals, this.parseExpression()); + ListWrapper.push(positionals, this.parsePipe()); } while (this.optionalCharacter($COMMA)); return positionals; } diff --git a/modules/angular2/test/change_detection/parser/parser_spec.ts b/modules/angular2/test/change_detection/parser/parser_spec.ts index 7136291909..fcfd15ffe1 100644 --- a/modules/angular2/test/change_detection/parser/parser_spec.ts +++ b/modules/angular2/test/change_detection/parser/parser_spec.ts @@ -3,9 +3,10 @@ import {BaseException, isBlank, isPresent} from 'angular2/src/facade/lang'; import {reflector} from 'angular2/src/reflection/reflection'; import {MapWrapper, ListWrapper} from 'angular2/src/facade/collection'; import {Parser} from 'angular2/src/change_detection/parser/parser'; +import {Unparser} from './unparser'; import {Lexer} from 'angular2/src/change_detection/parser/lexer'; import {Locals} from 'angular2/src/change_detection/parser/locals'; -import {Pipe, LiteralPrimitive, AccessMember} from 'angular2/src/change_detection/parser/ast'; +import {Pipe, LiteralPrimitive} from 'angular2/src/change_detection/parser/ast'; class TestData { constructor(public a?: any, public b?: any, public fnReturnValue?: any) {} @@ -383,34 +384,20 @@ export function main() { describe("parseBinding", () => { describe("pipes", () => { it("should parse pipes", () => { - var exp = parseBinding("'Foo'|uppercase").ast; - expect(exp).toBeAnInstanceOf(Pipe); - expect(exp.name).toEqual("uppercase"); + var originalExp = '"Foo" | uppercase'; + var ast = parseBinding(originalExp).ast; + expect(ast).toBeAnInstanceOf(Pipe); + expect(new Unparser().unparse(ast)).toEqual(`(${originalExp})`); }); it("should parse pipes in the middle of a binding", () => { - var exp = parseBinding("user|a|b.name").ast; - - expect(exp.name).toEqual("name"); - expect(exp.receiver).toBeAnInstanceOf(Pipe); - expect(exp.receiver.name).toEqual("b"); - - expect(exp.receiver.exp).toBeAnInstanceOf(Pipe); - expect(exp.receiver.exp.name).toEqual("a"); + var ast = parseBinding('(user | a | b).name').ast; + expect(new Unparser().unparse(ast)).toEqual('((user | a) | b).name'); }); it("should parse pipes with args", () => { - var exp = parseBinding("(1|a:2)|b:3").ast; - - expect(exp).toBeAnInstanceOf(Pipe); - expect(exp.name).toEqual("b"); - expect(exp.args[0]).toBeAnInstanceOf(LiteralPrimitive); - - expect(exp.exp).toBeAnInstanceOf(Pipe); - expect(exp.exp.name).toEqual("a"); - expect(exp.exp.args[0]).toBeAnInstanceOf(LiteralPrimitive); - - expect(exp.exp.exp).toBeAnInstanceOf(LiteralPrimitive); + var ast = parseBinding("(1|a:2)|b:3").ast; + expect(new Unparser().unparse(ast)).toEqual('((1 | a:2) | b:3)'); }); it('should only allow identifier or keyword as formatter names', () => { @@ -421,6 +408,25 @@ export function main() { .toThrowError(new RegExp('identifier or keyword')); }); + it('should parse pipes', () => { + let unparser = new Unparser(); + let exps = [ + ['a(b | c)', 'a((b | c))'], + ['a.b(c.d(e) | f)', 'a.b((c.d(e) | f))'], + ['[1, 2, 3] | a', '([1, 2, 3] | a)'], + ['{a: 1} | b', '({a: 1} | b)'], + ['a[b] | c', '(a[b] | c)'], + ['a?.b | c', '(a?.b | c)'], + ['true | a', '(true | a)'], + ['a | b:c | d', '(a | b:(c | d))'], + ['(a | b:c) | d', '((a | b:c) | d)'] + ]; + + ListWrapper.forEach(exps, e => { + var ast = parseBinding(e[0]).ast; + expect(unparser.unparse(ast)).toEqual(e[1]); + }); + }); }); it('should store the source in the result', @@ -433,7 +439,7 @@ export function main() { expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression")); }); - it('should throw on assignmnt', () => { + it('should throw on assignment', () => { expect(() => parseBinding("1;2")).toThrowError(new RegExp("contain chained expression")); }); }); @@ -584,11 +590,9 @@ export function main() { }); it('should parse prefix/suffix with multiple interpolation', () => { - var ast = parseInterpolation('before{{a}}middle{{b}}after').ast; - expect(ast.strings).toEqual(['before', 'middle', 'after']); - expect(ast.expressions.length).toEqual(2); - expect(ast.expressions[0].name).toEqual('a'); - expect(ast.expressions[1].name).toEqual('b'); + var originalExp = 'before {{ a }} middle {{ b }} after'; + var ast = parseInterpolation(originalExp).ast; + expect(new Unparser().unparse(ast)).toEqual(originalExp); }); }); diff --git a/modules/angular2/test/change_detection/parser/unparser.ts b/modules/angular2/test/change_detection/parser/unparser.ts index 96fab27906..4b65ee1277 100644 --- a/modules/angular2/test/change_detection/parser/unparser.ts +++ b/modules/angular2/test/change_detection/parser/unparser.ts @@ -68,12 +68,14 @@ export class Unparser implements AstVisitor { } visitPipe(ast: Pipe) { + this._expression += '('; this._visit(ast.exp); this._expression += ` | ${ast.name}`; ast.args.forEach(arg => { this._expression += ':'; this._visit(arg); - }) + }); + this._expression += ')'; } visitFunctionCall(ast: FunctionCall) { diff --git a/modules/angular2/test/change_detection/parser/unparser_spec.ts b/modules/angular2/test/change_detection/parser/unparser_spec.ts index 7ff995364a..3f55eb299b 100644 --- a/modules/angular2/test/change_detection/parser/unparser_spec.ts +++ b/modules/angular2/test/change_detection/parser/unparser_spec.ts @@ -64,7 +64,7 @@ export function main() { it('should support Conditional', () => { check('a ? b : c', Conditional); }); it('should support Pipe', () => { - var originalExp = 'a | b'; + var originalExp = '(a | b)'; var ast = parseBinding(originalExp).ast; expect(ast).toBeAnInstanceOf(Pipe); expect(unparser.unparse(ast)).toEqual(originalExp); @@ -94,7 +94,7 @@ export function main() { it('should support SafeMethodCall', () => { check('a?.b(c, d)', SafeMethodCall); }); it('should support complex expression', () => { - var originalExp = 'a + 3 * fn([c + d | e.f], {a: 3})[g].h && i'; + var originalExp = 'a + 3 * fn([(c + d | e).f], {a: 3})[g].h && i'; var ast = parseBinding(originalExp).ast; expect(unparser.unparse(ast)).toEqual(originalExp); });