From 3799f43c71b4c9ef0eaeaa41990d21544ecb545a Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Sat, 23 Sep 2017 11:30:51 -0700 Subject: [PATCH] fix(compiler): add parens around binary / ternary expressions This fixes cases where binary / ternary operators are nested (e.g. a nested ternary operator). --- .../src/transformers/node_emitter.ts | 14 +++---- .../test/transformers/node_emitter_spec.ts | 39 +++++++++++-------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index e115c14e7f..6b32c40cc2 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -310,13 +310,13 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { return this.record(expr, this._visitIdentifier(expr.value)); } - visitConditionalExpr(expr: ConditionalExpr): RecordedNode { - // TODO {chuckj}: Review use of ! on flaseCase. Should it be non-nullable? + visitConditionalExpr(expr: ConditionalExpr): RecordedNode { + // TODO {chuckj}: Review use of ! on falseCase. Should it be non-nullable? return this.record( expr, - ts.createConditional( + ts.createParen(ts.createConditional( expr.condition.visitExpression(this, null), expr.trueCase.visitExpression(this, null), - expr.falseCase !.visitExpression(this, null))); + expr.falseCase !.visitExpression(this, null)))); } visitNotExpr(expr: NotExpr): RecordedNode { @@ -345,7 +345,7 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { /* type */ undefined, this._visitStatements(expr.statements))); } - visitBinaryOperatorExpr(expr: BinaryOperatorExpr): RecordedNode { + visitBinaryOperatorExpr(expr: BinaryOperatorExpr): RecordedNode { let binaryOperator: ts.BinaryOperator; switch (expr.operator) { case BinaryOperator.And: @@ -397,9 +397,9 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { throw new Error(`Unknown operator: ${expr.operator}`); } return this.record( - expr, ts.createBinary( + expr, ts.createParen(ts.createBinary( expr.lhs.visitExpression(this, null), binaryOperator, - expr.rhs.visitExpression(this, null))); + expr.rhs.visitExpression(this, null)))); } visitReadPropExpr(expr: ReadPropExpr): RecordedNode { diff --git a/packages/compiler-cli/test/transformers/node_emitter_spec.ts b/packages/compiler-cli/test/transformers/node_emitter_spec.ts index 050f2621ca..3082e750ed 100644 --- a/packages/compiler-cli/test/transformers/node_emitter_spec.ts +++ b/packages/compiler-cli/test/transformers/node_emitter_spec.ts @@ -179,7 +179,7 @@ describe('TypeScriptNodeEmitter', () => { it('should support blank literals', () => { expect(emitStmt(o.literal(null).toStmt())).toEqual('null;'); expect(emitStmt(o.literal(undefined).toStmt())).toEqual('undefined;'); - expect(emitStmt(o.variable('a', null).isBlank().toStmt())).toEqual('a == null;'); + expect(emitStmt(o.variable('a', null).isBlank().toStmt())).toEqual('(a == null);'); }); it('should support external identifiers', () => { @@ -195,23 +195,28 @@ describe('TypeScriptNodeEmitter', () => { expect(emitStmt(o.not(someVar).toStmt())).toEqual('!someVar;'); expect(emitStmt(o.assertNotNull(someVar).toStmt())).toEqual('someVar;'); expect(emitStmt(someVar.conditional(o.variable('trueCase'), o.variable('falseCase')).toStmt())) - .toEqual('someVar ? trueCase : falseCase;'); + .toEqual('(someVar ? trueCase : falseCase);'); + expect(emitStmt(someVar.conditional(o.variable('trueCase'), o.variable('falseCase')) + .conditional(o.variable('trueCase'), o.variable('falseCase')) + .toStmt())) + .toEqual('((someVar ? trueCase : falseCase) ? trueCase : falseCase);'); - expect(emitStmt(lhs.equals(rhs).toStmt())).toEqual('lhs == rhs;'); - expect(emitStmt(lhs.notEquals(rhs).toStmt())).toEqual('lhs != rhs;'); - expect(emitStmt(lhs.identical(rhs).toStmt())).toEqual('lhs === rhs;'); - expect(emitStmt(lhs.notIdentical(rhs).toStmt())).toEqual('lhs !== rhs;'); - expect(emitStmt(lhs.minus(rhs).toStmt())).toEqual('lhs - rhs;'); - expect(emitStmt(lhs.plus(rhs).toStmt())).toEqual('lhs + rhs;'); - expect(emitStmt(lhs.divide(rhs).toStmt())).toEqual('lhs / rhs;'); - expect(emitStmt(lhs.multiply(rhs).toStmt())).toEqual('lhs * rhs;'); - expect(emitStmt(lhs.modulo(rhs).toStmt())).toEqual('lhs % rhs;'); - expect(emitStmt(lhs.and(rhs).toStmt())).toEqual('lhs && rhs;'); - expect(emitStmt(lhs.or(rhs).toStmt())).toEqual('lhs || rhs;'); - expect(emitStmt(lhs.lower(rhs).toStmt())).toEqual('lhs < rhs;'); - expect(emitStmt(lhs.lowerEquals(rhs).toStmt())).toEqual('lhs <= rhs;'); - expect(emitStmt(lhs.bigger(rhs).toStmt())).toEqual('lhs > rhs;'); - expect(emitStmt(lhs.biggerEquals(rhs).toStmt())).toEqual('lhs >= rhs;'); + expect(emitStmt(lhs.equals(rhs).toStmt())).toEqual('(lhs == rhs);'); + expect(emitStmt(lhs.notEquals(rhs).toStmt())).toEqual('(lhs != rhs);'); + expect(emitStmt(lhs.identical(rhs).toStmt())).toEqual('(lhs === rhs);'); + expect(emitStmt(lhs.notIdentical(rhs).toStmt())).toEqual('(lhs !== rhs);'); + expect(emitStmt(lhs.minus(rhs).toStmt())).toEqual('(lhs - rhs);'); + expect(emitStmt(lhs.plus(rhs).toStmt())).toEqual('(lhs + rhs);'); + expect(emitStmt(lhs.divide(rhs).toStmt())).toEqual('(lhs / rhs);'); + expect(emitStmt(lhs.multiply(rhs).toStmt())).toEqual('(lhs * rhs);'); + expect(emitStmt(lhs.plus(rhs).multiply(rhs).toStmt())).toEqual('((lhs + rhs) * rhs);'); + expect(emitStmt(lhs.modulo(rhs).toStmt())).toEqual('(lhs % rhs);'); + expect(emitStmt(lhs.and(rhs).toStmt())).toEqual('(lhs && rhs);'); + expect(emitStmt(lhs.or(rhs).toStmt())).toEqual('(lhs || rhs);'); + expect(emitStmt(lhs.lower(rhs).toStmt())).toEqual('(lhs < rhs);'); + expect(emitStmt(lhs.lowerEquals(rhs).toStmt())).toEqual('(lhs <= rhs);'); + expect(emitStmt(lhs.bigger(rhs).toStmt())).toEqual('(lhs > rhs);'); + expect(emitStmt(lhs.biggerEquals(rhs).toStmt())).toEqual('(lhs >= rhs);'); }); it('should support function expressions', () => {