From af015982f59939307bd56fea3f2377cce8fb5202 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 19 Dec 2019 15:05:56 -0800 Subject: [PATCH] fix(ivy): wrap 'as any' casts in parentheses when needed (#34649) Previously, when generating template type-checking code, casts to 'any' were produced as `expr as any`, regardless of the expression. However, for certain expression types, this led to precedence issues with the cast. For example, `a !== b` is a `ts.BinaryExpression`, and wrapping it directly in the cast yields `a !== b as any`, which is semantically equivalent to `a !== (b as any)`. This is obviously not what is intended. Instead, this commit adds a list of expression types for which a "bare" wrapping is permitted. For other expressions, parentheses are added to ensure correct precedence: `(a !== b) as any` PR Close #34649 --- .../src/ngtsc/typecheck/src/ts_util.ts | 39 +++++++++++++++++++ .../typecheck/test/type_check_block_spec.ts | 13 ++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts index 8169a79bdc..a45e491d5c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -9,7 +9,46 @@ import * as ts from 'typescript'; import {ClassDeclaration} from '../../reflection'; +/** + * A `Set` of `ts.SyntaxKind`s of `ts.Expression` which are safe to wrap in a `ts.AsExpression` + * without needing to be wrapped in parentheses. + * + * For example, `foo.bar()` is a `ts.CallExpression`, and can be safely cast to `any` with + * `foo.bar() as any`. however, `foo !== bar` is a `ts.BinaryExpression`, and attempting to cast + * without the parentheses yields the expression `foo !== bar as any`. This is semantically + * equivalent to `foo !== (bar as any)`, which is not what was intended. Thus, + * `ts.BinaryExpression`s need to be wrapped in parentheses before casting. + */ +// +const SAFE_TO_CAST_WITHOUT_PARENS: Set = new Set([ + // Expressions which are already parenthesized can be cast without further wrapping. + ts.SyntaxKind.ParenthesizedExpression, + + // Expressions which form a single lexical unit leave no room for precedence issues with the cast. + ts.SyntaxKind.Identifier, + ts.SyntaxKind.CallExpression, + ts.SyntaxKind.NonNullExpression, + ts.SyntaxKind.ElementAccessExpression, + ts.SyntaxKind.PropertyAccessExpression, + ts.SyntaxKind.ArrayLiteralExpression, + ts.SyntaxKind.ObjectLiteralExpression, + + // The same goes for various literals. + ts.SyntaxKind.StringLiteral, + ts.SyntaxKind.NumericLiteral, + ts.SyntaxKind.TrueKeyword, + ts.SyntaxKind.FalseKeyword, + ts.SyntaxKind.NullKeyword, + ts.SyntaxKind.UndefinedKeyword, +]); + export function tsCastToAny(expr: ts.Expression): ts.Expression { + // Wrap `expr` in parentheses if needed (see `SAFE_TO_CAST_WITHOUT_PARENS` above). + if (!SAFE_TO_CAST_WITHOUT_PARENS.has(expr.kind)) { + expr = ts.createParen(expr); + } + + // The outer expression is always wrapped in parentheses. return ts.createParen( ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword))); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 50ec363108..50f9aa3e08 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -376,20 +376,31 @@ describe('type check blocks', () => { }); describe('config.checkTypeOfBindings', () => { - const TEMPLATE = `
`; it('should check types of bindings when enabled', () => { + const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": ((ctx).a) })'); expect(block).toContain('(ctx).b;'); }); + it('should not check types of bindings when disabled', () => { + const TEMPLATE = `
`; const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('Dir.ngTypeCtor({ "dirInput": (((ctx).a as any)) })'); expect(block).toContain('((ctx).b as any);'); }); + + it('should wrap the cast to any in parentheses when required', () => { + const TEMPLATE = `
`; + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; + const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); + expect(block).toContain( + 'Dir.ngTypeCtor({ "dirInput": (((((ctx).a) === ((ctx).b)) as any)) })'); + }); }); describe('config.checkTypeOfOutputEvents', () => {