From 6f95a630e6ffbf95cb462448208e5d6bdbc87af9 Mon Sep 17 00:00:00 2001 From: JoostK Date: Mon, 28 Oct 2019 21:11:35 +0100 Subject: [PATCH] feat(ivy): graceful evaluation of unknown or invalid expressions During static evaluation of expressions within ngtsc, it may occur that certain expressions or just parts thereof cannot be statically interpreted for some reason. The static interpreter keeps track of the failure reason and the code path that was evaluated by means of `DynamicValue`, which will allow descriptive errors. In some situations however, the static interpreter would throw an exception instead, resulting in a crash of the compilation. Not only does this cause non-descriptive errors, more importantly does it prevent the evaluated result from being partial, i.e. parts of the result can be dynamic if their value does not have to be statically available to the compiler. This commit refactors the static interpreter to never throw errors for certain expressions that it cannot evaluate. Resolves FW-1582 --- .../ngtsc/partial_evaluator/src/dynamic.ts | 27 +++++ .../partial_evaluator/src/interpreter.ts | 29 +++-- .../partial_evaluator/test/evaluator_spec.ts | 102 ++++++++++++++++++ 3 files changed, 142 insertions(+), 16 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts index 85880d8e74beb1..b3bfa5c17a8c2e 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts @@ -42,6 +42,11 @@ export const enum DynamicValueReason { */ UNKNOWN_EXPRESSION_TYPE, + /** + * An operator was used that could not be statically evaluated. + */ + UNKNOWN_OPERATOR, + /** * A declaration of a `ts.Identifier` could not be found. */ @@ -54,6 +59,12 @@ export const enum DynamicValueReason { */ INVALID_EXPRESSION_TYPE, + /** + * A spread element did not evaluate to an array or object, so could therefore not be statically + * evaluated. + */ + INVALID_SPREAD_ELEMENT, + /** * A value could not be determined statically for any reason other the above. */ @@ -84,6 +95,10 @@ export class DynamicValue { return new DynamicValue(node, undefined, DynamicValueReason.UNKNOWN_EXPRESSION_TYPE); } + static fromUnknownOperator(node: ts.Node, operator: ts.SyntaxKind): DynamicValue { + return new DynamicValue(node, operator, DynamicValueReason.UNKNOWN_OPERATOR); + } + static fromUnknownIdentifier(node: ts.Identifier): DynamicValue { return new DynamicValue(node, undefined, DynamicValueReason.UNKNOWN_IDENTIFIER); } @@ -92,6 +107,10 @@ export class DynamicValue { return new DynamicValue(node, value, DynamicValueReason.INVALID_EXPRESSION_TYPE); } + static fromInvalidSpreadElement(node: ts.Node, value: unknown): DynamicValue { + return new DynamicValue(node, value, DynamicValueReason.INVALID_SPREAD_ELEMENT); + } + static fromUnknown(node: ts.Node): DynamicValue { return new DynamicValue(node, undefined, DynamicValueReason.UNKNOWN); } @@ -112,6 +131,10 @@ export class DynamicValue { return this.code === DynamicValueReason.UNKNOWN_EXPRESSION_TYPE; } + isFromUnknownOperator(this: DynamicValue): this is DynamicValue { + return this.code === DynamicValueReason.UNKNOWN_OPERATOR; + } + isFromUnknownIdentifier(this: DynamicValue): this is DynamicValue { return this.code === DynamicValueReason.UNKNOWN_IDENTIFIER; } @@ -120,6 +143,10 @@ export class DynamicValue { return this.code === DynamicValueReason.INVALID_EXPRESSION_TYPE; } + isFromInvalidSpreadElement(this: DynamicValue): this is DynamicValue { + return this.code === DynamicValueReason.INVALID_SPREAD_ELEMENT; + } + isFromUnknown(this: DynamicValue): this is DynamicValue { return this.code === DynamicValueReason.UNKNOWN; } 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 87544a61be492a..c9660d13404af5 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -184,7 +184,8 @@ export class StaticInterpreter { if (spread instanceof DynamicValue) { return DynamicValue.fromDynamicInput(node, spread); } else if (!(spread instanceof Map)) { - throw new Error(`Unexpected value in spread assignment: ${spread}`); + return DynamicValue.fromDynamicInput( + node, DynamicValue.fromInvalidSpreadElement(property, spread)); } spread.forEach((value, key) => map.set(key, value)); } else { @@ -292,9 +293,6 @@ export class StaticInterpreter { private visitElementAccessExpression(node: ts.ElementAccessExpression, context: Context): ResolvedValue { const lhs = this.visitExpression(node.expression, context); - if (node.argumentExpression === undefined) { - throw new Error(`Expected argument in ElementAccessExpression`); - } if (lhs instanceof DynamicValue) { return DynamicValue.fromDynamicInput(node, lhs); } @@ -303,8 +301,7 @@ export class StaticInterpreter { return DynamicValue.fromDynamicInput(node, rhs); } if (typeof rhs !== 'string' && typeof rhs !== 'number') { - throw new Error( - `ElementAccessExpression index should be string or number, got ${typeof rhs}: ${rhs}`); + return DynamicValue.fromInvalidExpressionType(node, rhs); } return this.accessHelper(node, lhs, rhs, context); @@ -360,10 +357,10 @@ export class StaticInterpreter { return new ArrayConcatBuiltinFn(node, lhs); } if (typeof rhs !== 'number' || !Number.isInteger(rhs)) { - return DynamicValue.fromUnknown(node); + return DynamicValue.fromInvalidExpressionType(node, rhs); } if (rhs < 0 || rhs >= lhs.length) { - throw new Error(`Index out of bounds: ${rhs} vs ${lhs.length}`); + return undefined; } return lhs[rhs]; } else if (lhs instanceof Reference) { @@ -498,7 +495,7 @@ export class StaticInterpreter { ResolvedValue { const operatorKind = node.operator; if (!UNARY_OPERATORS.has(operatorKind)) { - throw new Error(`Unsupported prefix unary operator: ${ts.SyntaxKind[operatorKind]}`); + return DynamicValue.fromUnknownOperator(node, operatorKind); } const op = UNARY_OPERATORS.get(operatorKind) !; @@ -513,14 +510,14 @@ export class StaticInterpreter { private visitBinaryExpression(node: ts.BinaryExpression, context: Context): ResolvedValue { const tokenKind = node.operatorToken.kind; if (!BINARY_OPERATORS.has(tokenKind)) { - throw new Error(`Unsupported binary operator: ${ts.SyntaxKind[tokenKind]}`); + return DynamicValue.fromUnknownOperator(node, tokenKind); } const opRecord = BINARY_OPERATORS.get(tokenKind) !; let lhs: ResolvedValue, rhs: ResolvedValue; if (opRecord.literal) { - lhs = literal(this.visitExpression(node.left, context)); - rhs = literal(this.visitExpression(node.right, context)); + lhs = literal(this.visitExpression(node.left, context), node.left); + rhs = literal(this.visitExpression(node.right, context), node.right); } else { lhs = this.visitExpression(node.left, context); rhs = this.visitExpression(node.right, context); @@ -554,9 +551,9 @@ export class StaticInterpreter { private visitSpreadElement(node: ts.SpreadElement, context: Context): ResolvedValueArray { const spread = this.visitExpression(node.expression, context); if (spread instanceof DynamicValue) { - return [DynamicValue.fromDynamicInput(node.expression, spread)]; + return [DynamicValue.fromDynamicInput(node, spread)]; } else if (!Array.isArray(spread)) { - throw new Error(`Unexpected value in spread expression: ${spread}`); + return [DynamicValue.fromInvalidSpreadElement(node, spread)]; } else { return spread; } @@ -582,12 +579,12 @@ function isFunctionOrMethodReference(ref: Reference): ts.isFunctionExpression(ref.node); } -function literal(value: ResolvedValue): any { +function literal(value: ResolvedValue, node: ts.Node): any { if (value instanceof DynamicValue || value === null || value === undefined || typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { return value; } - throw new Error(`Value ${value} is not literal and cannot be used in this context.`); + return DynamicValue.fromInvalidExpressionType(node, 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 022b345648cd73..32a9601c93df3f 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 @@ -151,6 +151,11 @@ runInEachFileSystem(() => { it('array access works', () => { expect(evaluate(`const a = [1, 2, 3];`, 'a[1] + a[0]')).toEqual(3); }); + it('array access out of bounds is `undefined`', () => { + expect(evaluate(`const a = [1, 2, 3];`, 'a[-1]')).toEqual(undefined); + expect(evaluate(`const a = [1, 2, 3];`, 'a[3]')).toEqual(undefined); + }); + it('array `length` property access works', () => { expect(evaluate(`const a = [1, 2, 3];`, 'a[\'length\'] + 1')).toEqual(4); }); @@ -182,6 +187,103 @@ runInEachFileSystem(() => { it('supports null', () => { expect(evaluate('const a = null;', 'a')).toEqual(null); }); + it('resolves unknown binary operators as dynamic value', () => { + const value = evaluate('declare const window: any;', '"location" in window'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('"location" in window'); + if (!value.isFromUnknownOperator()) { + return fail('Should have an unknown operator as reason'); + } + expect(value.reason).toEqual(ts.SyntaxKind.InKeyword); + }); + + it('resolves unknown unary operators as dynamic value', () => { + const value = evaluate('let index = 0;', '++index'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('++index'); + if (!value.isFromUnknownOperator()) { + return fail('Should have an unknown operator as reason'); + } + expect(value.reason).toEqual(ts.SyntaxKind.PlusPlusToken); + }); + + it('resolves invalid element accesses as dynamic value', () => { + const value = evaluate('const a = {}; const index: any = true;', 'a[index]'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('a[index]'); + if (!value.isFromInvalidExpressionType()) { + return fail('Should have an invalid expression type as reason'); + } + expect(value.reason).toEqual(true); + }); + + it('resolves invalid array accesses as dynamic value', () => { + const value = evaluate('const a = []; const index = 1.5;', 'a[index]'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('a[index]'); + if (!value.isFromInvalidExpressionType()) { + return fail('Should have an invalid expression type as reason'); + } + expect(value.reason).toEqual(1.5); + }); + + it('resolves binary operator on non-literals as dynamic value', () => { + const value = evaluate('const a: any = []; const b: any = [];', 'a + b'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('a + b'); + if (!(value.reason instanceof DynamicValue)) { + return fail(`Should have a DynamicValue as reason`); + } + if (!value.reason.isFromInvalidExpressionType()) { + return fail('Should have an invalid expression type as reason'); + } + expect(value.reason.node.getText()).toEqual('a'); + expect(value.reason.reason).toEqual([]); + }); + + it('resolves invalid spreads in array literals as dynamic value', () => { + const array = evaluate('const a: any = true;', '[1, ...a]'); + if (!Array.isArray(array)) { + return fail(`Should have resolved to an array`); + } + expect(array[0]).toBe(1); + const value = array[1]; + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('...a'); + if (!value.isFromInvalidSpreadElement()) { + return fail('Should have an invalid spread element as reason'); + } + expect(value.reason).toEqual(true); + }); + + it('resolves invalid spreads in object literals as dynamic value', () => { + const value = evaluate('const a: any = true;', '{b: true, ...a}'); + if (!(value instanceof DynamicValue)) { + return fail(`Should have resolved to a DynamicValue`); + } + expect(value.node.getText()).toEqual('{b: true, ...a}'); + if (!value.isFromDynamicInput()) { + return fail('Should have a dynamic input as reason'); + } + expect(value.reason.node.getText()).toEqual('...a'); + if (!value.reason.isFromInvalidSpreadElement()) { + return fail('Should have an invalid spread element as reason'); + } + expect(value.reason.reason).toEqual(true); + }); + it('resolves access from external variable declarations as dynamic value', () => { const value = evaluate('declare const window: any;', 'window.location'); if (!(value instanceof DynamicValue)) {