From 64022f51d4fbbd93683cf188e9c3a986a345a73d Mon Sep 17 00:00:00 2001 From: JoostK Date: Mon, 6 Apr 2020 22:33:38 +0200 Subject: [PATCH] fix(compiler): resolve enum values in binary operations (#36461) During static evaluation of expressions, the partial evaluator may come across a binary + operator for which it needs to evaluate its operands. Any of these operands may be a reference to an enum member, in which case the enum member's value needs to be used as literal value, not the enum member reference itself. This commit fixes the behavior by resolving an `EnumValue` when used as a literal value. Fixes #35584 Resolves FW-1951 PR Close #36461 --- .../partial_evaluator/src/interpreter.ts | 32 ++++++++++--------- .../partial_evaluator/test/evaluator_spec.ts | 31 ++++++++++++++---- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index 83f32f14e2..ab12a398fb 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -203,19 +203,13 @@ export class StaticInterpreter { const pieces: string[] = [node.head.text]; for (let i = 0; i < node.templateSpans.length; i++) { const span = node.templateSpans[i]; - let value = this.visit(span.expression, context); - if (value instanceof EnumValue) { - value = value.resolved; - } - if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean' || - value == null) { - pieces.push(`${value}`); - } else if (value instanceof DynamicValue) { + const value = literal( + this.visit(span.expression, context), + () => DynamicValue.fromDynamicString(span.expression)); + if (value instanceof DynamicValue) { return DynamicValue.fromDynamicInput(node, value); - } else { - return DynamicValue.fromDynamicInput(node, DynamicValue.fromDynamicString(span.expression)); } - pieces.push(span.literal.text); + pieces.push(`${value}`, span.literal.text); } return pieces.join(''); } @@ -520,8 +514,12 @@ export class StaticInterpreter { const opRecord = BINARY_OPERATORS.get(tokenKind)!; let lhs: ResolvedValue, rhs: ResolvedValue; if (opRecord.literal) { - lhs = literal(this.visitExpression(node.left, context), node.left); - rhs = literal(this.visitExpression(node.right, context), node.right); + lhs = literal( + this.visitExpression(node.left, context), + value => DynamicValue.fromInvalidExpressionType(node.left, value)); + rhs = literal( + this.visitExpression(node.right, context), + value => DynamicValue.fromInvalidExpressionType(node.right, value)); } else { lhs = this.visitExpression(node.left, context); rhs = this.visitExpression(node.right, context); @@ -585,12 +583,16 @@ function isFunctionOrMethodReference(ref: Reference): ts.isFunctionExpression(ref.node); } -function literal(value: ResolvedValue, node: ts.Node): any { +function literal( + value: ResolvedValue, reject: (value: ResolvedValue) => ResolvedValue): ResolvedValue { + if (value instanceof EnumValue) { + value = value.resolved; + } if (value instanceof DynamicValue || value === null || value === undefined || typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { return value; } - return DynamicValue.fromInvalidExpressionType(node, value); + return reject(value); } function isVariableDeclarationDeclared(node: ts.VariableDeclaration): boolean { diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts index 8d2d773cbe..7c3dcd3ae0 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts @@ -448,6 +448,31 @@ runInEachFileSystem(() => { expect(evaluate('const a = 2, b = 4;', '`1${a}3${b}5`')).toEqual('12345'); }); + it('template expressions should resolve enums', () => { + expect(evaluate('enum Test { VALUE = "test" };', '`a.${Test.VALUE}.b`')).toBe('a.test.b'); + }); + + it('string concatenation should resolve enums', () => { + expect(evaluate('enum Test { VALUE = "test" };', '"a." + Test.VALUE + ".b"')) + .toBe('a.test.b'); + }); + + it('should resolve non-literals as dynamic string', () => { + const value = evaluate(`const a: any = [];`, '`a.${a}.b`'); + + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('`a.${a}.b`'); + + if (!value.isFromDynamicInput()) { + return fail('Should originate from dynamic input'); + } else if (!value.reason.isFromDynamicString()) { + return fail('Should refer to a dynamic string part'); + } + expect(value.reason.node.getText()).toEqual('a'); + }); + it('enum resolution works', () => { const result = evaluate( ` @@ -512,12 +537,6 @@ runInEachFileSystem(() => { expect(value.node).toBe(prop.initializer); }); - it('should resolve enums in template expressions', () => { - const value = - evaluate(`enum Test { VALUE = 'test', } const value = \`a.\${Test.VALUE}.b\`;`, 'value'); - expect(value).toBe('a.test.b'); - }); - it('should not attach identifiers to FFR-resolved values', () => { const value = evaluate( `