Navigation Menu

Skip to content

Commit

Permalink
feat(ivy): graceful evaluation of unknown or invalid expressions (#33453
Browse files Browse the repository at this point in the history
)

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

PR Close #33453
  • Loading branch information
JoostK authored and atscott committed Nov 1, 2019
1 parent 4d4b527 commit ce30888
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 25 deletions.
13 changes: 7 additions & 6 deletions packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts
Expand Up @@ -38,9 +38,10 @@ export const enum DynamicValueReason {
EXTERNAL_REFERENCE,

/**
* A type of `ts.Expression` that `StaticInterpreter` doesn't know how to evaluate.
* Syntax that `StaticInterpreter` doesn't know how to evaluate, for example a type of
* `ts.Expression` that is not supported.
*/
UNKNOWN_EXPRESSION_TYPE,
UNSUPPORTED_SYNTAX,

/**
* A declaration of a `ts.Identifier` could not be found.
Expand Down Expand Up @@ -80,8 +81,8 @@ export class DynamicValue<R = unknown> {
return new DynamicValue(node, ref, DynamicValueReason.EXTERNAL_REFERENCE);
}

static fromUnknownExpressionType(node: ts.Node): DynamicValue {
return new DynamicValue(node, undefined, DynamicValueReason.UNKNOWN_EXPRESSION_TYPE);
static fromUnsupportedSyntax(node: ts.Node): DynamicValue {
return new DynamicValue(node, undefined, DynamicValueReason.UNSUPPORTED_SYNTAX);
}

static fromUnknownIdentifier(node: ts.Identifier): DynamicValue {
Expand All @@ -108,8 +109,8 @@ export class DynamicValue<R = unknown> {
return this.code === DynamicValueReason.EXTERNAL_REFERENCE;
}

isFromUnknownExpressionType(this: DynamicValue<R>): this is DynamicValue {
return this.code === DynamicValueReason.UNKNOWN_EXPRESSION_TYPE;
isFromUnsupportedSyntax(this: DynamicValue<R>): this is DynamicValue {
return this.code === DynamicValueReason.UNSUPPORTED_SYNTAX;
}

isFromUnknownIdentifier(this: DynamicValue<R>): this is DynamicValue {
Expand Down
Expand Up @@ -138,7 +138,7 @@ export class StaticInterpreter {
} else if (this.host.isClass(node)) {
result = this.visitDeclaration(node, context);
} else {
return DynamicValue.fromUnknownExpressionType(node);
return DynamicValue.fromUnsupportedSyntax(node);
}
if (result instanceof DynamicValue && result.node !== node) {
return DynamicValue.fromDynamicInput(node, result);
Expand Down Expand Up @@ -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.fromInvalidExpressionType(property, spread));
}
spread.forEach((value, key) => map.set(key, value));
} else {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -360,10 +357,7 @@ export class StaticInterpreter {
return new ArrayConcatBuiltinFn(node, lhs);
}
if (typeof rhs !== 'number' || !Number.isInteger(rhs)) {
return DynamicValue.fromUnknown(node);
}
if (rhs < 0 || rhs >= lhs.length) {
throw new Error(`Index out of bounds: ${rhs} vs ${lhs.length}`);
return DynamicValue.fromInvalidExpressionType(node, rhs);
}
return lhs[rhs];
} else if (lhs instanceof Reference) {
Expand Down Expand Up @@ -498,7 +492,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.fromUnsupportedSyntax(node);
}

const op = UNARY_OPERATORS.get(operatorKind) !;
Expand All @@ -513,14 +507,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.fromUnsupportedSyntax(node);
}

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);
Expand Down Expand Up @@ -554,9 +548,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.fromInvalidExpressionType(node, spread)];
} else {
return spread;
}
Expand All @@ -582,12 +576,12 @@ function isFunctionOrMethodReference(ref: Reference<ts.Node>):
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 {
Expand Down
Expand Up @@ -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); });

Expand Down Expand Up @@ -182,6 +187,97 @@ 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');
expect(value.isFromUnsupportedSyntax()).toBe(true);
});

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');
expect(value.isFromUnsupportedSyntax()).toBe(true);
});

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.isFromInvalidExpressionType()) {
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.isFromInvalidExpressionType()) {
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)) {
Expand Down

0 comments on commit ce30888

Please sign in to comment.