feat(ivy): logical not and safe navigation operation handling in TCBs (#29698)

This commit adds support in the template type-checking engine for handling
the logical not operation and the safe navigation operation.

Safe navigation in particular is tricky, as the View Engine implementation
has a rather inconvenient flaw. View Engine checks a safe navigation
operation `a?.b` as:

```typescript
(a != null ? a!.b : null as any)
```

The type of this expression is always 'any', as the false branch of the
ternary has type 'any'. Thus, using null-safe navigation throws away the
type of the result, and breaks type-checking for the rest of the expression.

A flag is introduced in the type-checking configuration to allow Ivy to
mimic this behavior when needed.

Testing strategy: TCB tests included.

PR Close #29698
This commit is contained in:
Alex Rickabaugh 2019-04-02 13:03:42 -07:00 committed by Ben Lesh
parent 182e2c7449
commit f4c536ae36
5 changed files with 63 additions and 1 deletions

View File

@ -195,6 +195,7 @@ export class NgtscProgram implements api.Program {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
strictSafeNavigationTypes: true,
};
const ctx = new TypeCheckContext(config, this.refEmitter !);
compilation.typeCheck(ctx);

View File

@ -7,6 +7,7 @@
*/
import {BoundTarget, DirectiveMeta} from '@angular/compiler';
import * as ts from 'typescript';
import {Reference} from '../../imports';
import {ClassDeclaration} from '../../reflection';
@ -71,6 +72,15 @@ export interface TypeCheckingConfig {
*/
applyTemplateContextGuards: boolean;
/**
* Whether to use a strict type for null-safe navigation operations.
*
* If this is `false`, then the return type of `a?.b` or `a?()` will be `any`. If set to `true`,
* then the return type of `a?.b` for example will be the same as the type of the ternary
* expression `a != null ? a.b : a`.
*/
strictSafeNavigationTypes: boolean;
/**
* Whether to descend into template bodies and check any bindings there.
*/

View File

@ -6,10 +6,15 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AST, ASTWithSource, Binary, Conditional, Interpolation, KeyedRead, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PropertyRead} from '@angular/compiler';
import {AST, ASTWithSource, Binary, Conditional, Interpolation, KeyedRead, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, SafeMethodCall, SafePropertyRead} from '@angular/compiler';
import * as ts from 'typescript';
import {TypeCheckingConfig} from './api';
const NULL_AS_ANY =
ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
const UNDEFINED = ts.createIdentifier('undefined');
const BINARY_OPS = new Map<string, ts.SyntaxKind>([
['+', ts.SyntaxKind.PlusToken],
['-', ts.SyntaxKind.MinusToken],
@ -94,6 +99,26 @@ export function astToTypescript(
} else if (ast instanceof NonNullAssert) {
const expr = astToTypescript(ast.expression, maybeResolve, config);
return ts.createNonNullExpression(expr);
} else if (ast instanceof PrefixNot) {
return ts.createLogicalNot(astToTypescript(ast.expression, maybeResolve, config));
} else if (ast instanceof SafePropertyRead) {
// A safe property expression a?.b takes the form `(a != null ? a!.b : whenNull)`, where
// whenNull is either of type 'any' or or 'undefined' depending on strictness. The non-null
// assertion is necessary because in practice 'a' may be a method call expression, which won't
// have a narrowed type when repeated in the ternary true branch.
const receiver = astToTypescript(ast.receiver, maybeResolve, config);
const expr = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
const whenNull = config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY;
return safeTernary(receiver, expr, whenNull);
} else if (ast instanceof SafeMethodCall) {
const receiver = astToTypescript(ast.receiver, maybeResolve, config);
// See the comment in SafePropertyRead above for an explanation of the need for the non-null
// assertion here.
const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name);
const args = ast.args.map(expr => astToTypescript(expr, maybeResolve, config));
const expr = ts.createCall(method, undefined, args);
const whenNull = config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY;
return safeTernary(receiver, expr, whenNull);
} else {
throw new Error(`Unknown node type: ${Object.getPrototypeOf(ast).constructor}`);
}
@ -115,3 +140,10 @@ function astArrayToExpression(
lhs, ts.SyntaxKind.CommaToken, astToTypescript(ast, maybeResolve, config)),
astToTypescript(asts.pop() !, maybeResolve, config));
}
function safeTernary(
lhs: ts.Expression, whenNotNull: ts.Expression, whenNull: ts.Expression): ts.Expression {
const notNullComp = ts.createBinary(lhs, ts.SyntaxKind.ExclamationEqualsToken, ts.createNull());
const ternary = ts.createConditional(notNullComp, whenNotNull, whenNull);
return ts.createParen(ternary);
}

View File

@ -87,6 +87,7 @@ describe('type check blocks', () => {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
strictSafeNavigationTypes: true,
};
describe('config.applyTemplateContextGuards', () => {
@ -133,6 +134,22 @@ describe('type check blocks', () => {
expect(block).toContain('.nonDirInput = (ctx.a as any);');
});
});
describe('config.strictSafeNavigationTypes', () => {
const TEMPLATE = `{{a?.b}} {{a?.method()}}`;
it('should use undefined for safe navigation operations when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('(ctx.a != null ? ctx.a!.method() : undefined)');
expect(block).toContain('(ctx.a != null ? ctx.a!.b : undefined)');
});
it('should use an \'any\' type for safe navigation operations when disabled', () => {
const DISABLED_CONFIG = {...BASE_CONFIG, strictSafeNavigationTypes: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('(ctx.a != null ? ctx.a!.method() : null as any)');
expect(block).toContain('(ctx.a != null ? ctx.a!.b : null as any)');
});
});
});
});
@ -226,6 +243,7 @@ function tcb(
applyTemplateContextGuards: true,
checkTypeOfBindings: true,
checkTemplateBodies: true,
strictSafeNavigationTypes: true,
};
const im = new ImportManager(undefined, 'i');

View File

@ -29,6 +29,7 @@ const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
strictSafeNavigationTypes: true,
};
describe('ngtsc typechecking', () => {