honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(ivy): add backwards compatibility config to template type-checking (#29698) 

View Engine's implementation of naive template type-checking is less
advanced than the current Ivy implementation. As a result, Ivy catches lots
of typing bugs which VE does not. As a result, it's necessary to tone down
the Ivy template type-checker in the default case.

This commit introduces a mechanism for doing that, by passing a config to
the template type-checking engine. Through this configuration, particular
checks can be loosened or disabled entirely.

Testing strategy: TCB tests included.

PR Close #29698
This commit is contained in:
Alex Rickabaugh 2019-03-22 11:16:55 -07:00 committed by Ben Lesh
parent cd1277cfb7
commit 182e2c7449
7 changed files with 168 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
packages/compiler-cli/src/ngtsc/program.ts
View File

@ -31,7 +31,7 @@ import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator,
import {ivySwitchTransform} from './switch';
import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform';
import {aliasTransformFactory} from './transform/src/alias';
import {TypeCheckContext, TypeCheckProgramHost} from './typecheck';
import {TypeCheckContext, TypeCheckProgramHost, TypeCheckingConfig} from './typecheck';
import {normalizeSeparators} from './util/src/path';
import {getRootDirs, isDtsPath} from './util/src/typescript';
@ -191,7 +191,12 @@ export class NgtscProgram implements api.Program {
const compilation = this.ensureAnalyzed();
const diagnostics = [...compilation.diagnostics];
if (!!this.options.fullTemplateTypeCheck) {
const ctx = new TypeCheckContext(this.refEmitter !);
const config: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
};
const ctx = new TypeCheckContext(config, this.refEmitter !);
compilation.typeCheck(ctx);
diagnostics.push(...this.compileTypeCheckProgram(ctx));
}

22
packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
View File

@ -54,3 +54,25 @@ export interface TypeCtorMetadata {
*/
fields: {inputs: string[]; outputs: string[]; queries: string[];};
}
export interface TypeCheckingConfig {
/**
* Whether to check the left-hand side type of binding operations.
*
* For example, if this is `false` then the expression `[input]="expr"` will have `expr` type-
* checked, but not the assignment of the resulting type to the `input` property of whichever
* directive or component is receiving the binding. If set to `true`, both sides of the assignment
* are checked.
*/
checkTypeOfBindings: boolean;
/**
* Whether to narrow the types of template contexts.
*/
applyTemplateContextGuards: boolean;
/**
* Whether to descend into template bodies and check any bindings there.
*/
checkTemplateBodies: boolean;
}

12
packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
View File

@ -13,7 +13,7 @@ import {NoopImportRewriter, ReferenceEmitter} from '../../imports';
import {ClassDeclaration} from '../../reflection';
import {ImportManager} from '../../translator';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCtorMetadata} from './api';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from './api';
import {generateTypeCheckBlock} from './type_check_block';
import {generateTypeCtor} from './type_constructor';
@ -27,7 +27,7 @@ import {generateTypeCtor} from './type_constructor';
* checking code.
*/
export class TypeCheckContext {
constructor(private refEmitter: ReferenceEmitter) {}
constructor(private config: TypeCheckingConfig, private refEmitter: ReferenceEmitter) {}
/**
* A `Map` of `ts.SourceFile`s that the context has seen to the operations (additions of methods
@ -141,7 +141,7 @@ export class TypeCheckContext {
this.opMap.set(sf, []);
}
const ops = this.opMap.get(sf) !;
ops.push(new TcbOp(node, tcbMeta));
ops.push(new TcbOp(node, tcbMeta, this.config));
}
}
@ -171,8 +171,8 @@ interface Op {
*/
class TcbOp implements Op {
constructor(
readonly node: ClassDeclaration<ts.ClassDeclaration>, readonly meta: TypeCheckBlockMetadata) {
}
readonly node: ClassDeclaration<ts.ClassDeclaration>, readonly meta: TypeCheckBlockMetadata,
readonly config: TypeCheckingConfig) {}
/**
* Type check blocks are inserted immediately after the end of the component class.
@ -181,7 +181,7 @@ class TcbOp implements Op {
execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer):
string {
const tcb = generateTypeCheckBlock(this.node, this.meta, im, refEmitter);
const tcb = generateTypeCheckBlock(this.node, this.meta, this.config, im, refEmitter);
return printer.printNode(ts.EmitHint.Unspecified, tcb, sf);
}
}

43
packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts
View File

@ -9,6 +9,7 @@
import {AST, ASTWithSource, Binary, Conditional, Interpolation, KeyedRead, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PropertyRead} from '@angular/compiler';
import * as ts from 'typescript';
import {TypeCheckingConfig} from './api';
const BINARY_OPS = new Map<string, ts.SyntaxKind>([
['+', ts.SyntaxKind.PlusToken],
['-', ts.SyntaxKind.MinusToken],
@ -34,7 +35,8 @@ const BINARY_OPS = new Map<string, ts.SyntaxKind>([
* AST.
*/
export function astToTypescript(
ast: AST, maybeResolve: (ast: AST) => ts.Expression | null): ts.Expression {
ast: AST, maybeResolve: (ast: AST) => ts.Expression | null,
config: TypeCheckingConfig): ts.Expression {
const resolved = maybeResolve(ast);
if (resolved !== null) {
return resolved;
@ -42,17 +44,17 @@ export function astToTypescript(
// Branch based on the type of expression being processed.
if (ast instanceof ASTWithSource) {
// Fall through to the underlying AST.
return astToTypescript(ast.ast, maybeResolve);
return astToTypescript(ast.ast, maybeResolve, config);
} else if (ast instanceof PropertyRead) {
// This is a normal property read - convert the receiver to an expression and emit the correct
// TypeScript expression to read the property.
const receiver = astToTypescript(ast.receiver, maybeResolve);
const receiver = astToTypescript(ast.receiver, maybeResolve, config);
return ts.createPropertyAccess(receiver, ast.name);
} else if (ast instanceof Interpolation) {
return astArrayToExpression(ast.expressions, maybeResolve);
return astArrayToExpression(ast.expressions, maybeResolve, config);
} else if (ast instanceof Binary) {
const lhs = astToTypescript(ast.left, maybeResolve);
const rhs = astToTypescript(ast.right, maybeResolve);
const lhs = astToTypescript(ast.left, maybeResolve, config);
const rhs = astToTypescript(ast.right, maybeResolve, config);
const op = BINARY_OPS.get(ast.operation);
if (op === undefined) {
throw new Error(`Unsupported Binary.operation: ${ast.operation}`);
@ -67,30 +69,30 @@ export function astToTypescript(
return ts.createLiteral(ast.value);
}
} else if (ast instanceof MethodCall) {
const receiver = astToTypescript(ast.receiver, maybeResolve);
const receiver = astToTypescript(ast.receiver, maybeResolve, config);
const method = ts.createPropertyAccess(receiver, ast.name);
const args = ast.args.map(expr => astToTypescript(expr, maybeResolve));
const args = ast.args.map(expr => astToTypescript(expr, maybeResolve, config));
return ts.createCall(method, undefined, args);
} else if (ast instanceof Conditional) {
const condExpr = astToTypescript(ast.condition, maybeResolve);
const trueExpr = astToTypescript(ast.trueExp, maybeResolve);
const falseExpr = astToTypescript(ast.falseExp, maybeResolve);
const condExpr = astToTypescript(ast.condition, maybeResolve, config);
const trueExpr = astToTypescript(ast.trueExp, maybeResolve, config);
const falseExpr = astToTypescript(ast.falseExp, maybeResolve, config);
return ts.createParen(ts.createConditional(condExpr, trueExpr, falseExpr));
} else if (ast instanceof LiteralArray) {
const elements = ast.expressions.map(expr => astToTypescript(expr, maybeResolve));
const elements = ast.expressions.map(expr => astToTypescript(expr, maybeResolve, config));
return ts.createArrayLiteral(elements);
} else if (ast instanceof LiteralMap) {
const properties = ast.keys.map(({key}, idx) => {
const value = astToTypescript(ast.values[idx], maybeResolve);
const value = astToTypescript(ast.values[idx], maybeResolve, config);
return ts.createPropertyAssignment(ts.createStringLiteral(key), value);
});
return ts.createObjectLiteral(properties, true);
} else if (ast instanceof KeyedRead) {
const receiver = astToTypescript(ast.obj, maybeResolve);
const key = astToTypescript(ast.key, maybeResolve);
const receiver = astToTypescript(ast.obj, maybeResolve, config);
const key = astToTypescript(ast.key, maybeResolve, config);
return ts.createElementAccess(receiver, key);
} else if (ast instanceof NonNullAssert) {
const expr = astToTypescript(ast.expression, maybeResolve);
const expr = astToTypescript(ast.expression, maybeResolve, config);
return ts.createNonNullExpression(expr);
} else {
throw new Error(`Unknown node type: ${Object.getPrototypeOf(ast).constructor}`);
@ -102,13 +104,14 @@ export function astToTypescript(
* and separating them with commas.
*/
function astArrayToExpression(
astArray: AST[], maybeResolve: (ast: AST) => ts.Expression | null): ts.Expression {
astArray: AST[], maybeResolve: (ast: AST) => ts.Expression | null,
config: TypeCheckingConfig): ts.Expression {
// Reduce the `asts` array into a `ts.Expression`. Multiple expressions are combined into a
// `ts.BinaryExpression` with a comma separator. First make a copy of the input array, as
// it will be modified during the reduction.
const asts = astArray.slice();
return asts.reduce(
(lhs, ast) =>
ts.createBinary(lhs, ts.SyntaxKind.CommaToken, astToTypescript(ast, maybeResolve)),
astToTypescript(asts.pop() !, maybeResolve));
(lhs, ast) => ts.createBinary(
lhs, ts.SyntaxKind.CommaToken, astToTypescript(ast, maybeResolve, config)),
astToTypescript(asts.pop() !, maybeResolve, config));
}

41
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View File

@ -13,7 +13,7 @@ import {NOOP_DEFAULT_IMPORT_RECORDER, Reference, ReferenceEmitter} from '../../i
import {ClassDeclaration} from '../../reflection';
import {ImportManager, translateExpression, translateType} from '../../translator';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCheckingConfig} from './api';
import {astToTypescript} from './expression';
/**
@ -29,8 +29,10 @@ import {astToTypescript} from './expression';
*/
export function generateTypeCheckBlock(
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCheckBlockMetadata,
importManager: ImportManager, refEmitter: ReferenceEmitter): ts.FunctionDeclaration {
const tcb = new Context(meta.boundTarget, node.getSourceFile(), importManager, refEmitter);
config: TypeCheckingConfig, importManager: ImportManager,
refEmitter: ReferenceEmitter): ts.FunctionDeclaration {
const tcb =
new Context(config, meta.boundTarget, node.getSourceFile(), importManager, refEmitter);
const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template !);
return ts.createFunctionDeclaration(
@ -187,7 +189,7 @@ class TcbTemplateBodyOp extends TcbOp {
// The second kind of guard is a template context guard. This guard narrows the template
// rendering context variable `ctx`.
if (dir.hasNgTemplateContextGuard) {
if (dir.hasNgTemplateContextGuard && this.tcb.config.applyTemplateContextGuards) {
const ctx = this.scope.resolve(this.template);
const guardInvoke = tsCallMethod(dirId, 'ngTemplateContextGuard', [dirInstId, ctx]);
directiveGuards.push(guardInvoke);
@ -288,7 +290,13 @@ class TcbUnclaimedInputsOp extends TcbOp {
continue;
}
const expr = tcbExpression(binding.value, this.tcb, this.scope);
let expr = tcbExpression(binding.value, this.tcb, this.scope);
// If checking the type of bindings is disabled, cast the resulting expression to 'any' before
// the assignment.
if (!this.tcb.config.checkTypeOfBindings) {
expr = tsCastToAny(expr);
}
if (binding.type === BindingType.Property) {
if (binding.name !== 'style' && binding.name !== 'class') {
@ -331,6 +339,7 @@ class Context {
private nextId = 1;
constructor(
readonly config: TypeCheckingConfig,
readonly boundTarget: BoundTarget<TypeCheckableDirectiveMeta>,
private sourceFile: ts.SourceFile, private importManager: ImportManager,
private refEmitter: ReferenceEmitter) {}
@ -605,9 +614,11 @@ class Scope {
} else if (node instanceof TmplAstTemplate) {
// Template children are rendered in a child scope.
this.appendDirectivesAndInputsOfNode(node);
const ctxIndex = this.opQueue.push(new TcbTemplateContextOp(this.tcb, this)) - 1;
this.templateCtxOpMap.set(node, ctxIndex);
this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node));
if (this.tcb.config.checkTemplateBodies) {
const ctxIndex = this.opQueue.push(new TcbTemplateContextOp(this.tcb, this)) - 1;
this.templateCtxOpMap.set(node, ctxIndex);
this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node));
}
} else if (node instanceof TmplAstBoundText) {
this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, node));
}
@ -681,7 +692,7 @@ function tcbExpression(ast: AST, tcb: Context, scope: Scope): ts.Expression {
// `astToTypescript` actually does the conversion. A special resolver `tcbResolve` is passed which
// interprets specific expression nodes that interact with the `ImplicitReceiver`. These nodes
// actually refer to identifiers within the current scope.
return astToTypescript(ast, (ast) => tcbResolve(ast, tcb, scope));
return astToTypescript(ast, (ast) => tcbResolve(ast, tcb, scope), tcb.config);
}
/**
@ -695,7 +706,12 @@ function tcbCallTypeCtor(
// Construct an array of `ts.PropertyAssignment`s for each input of the directive that has a
// matching binding.
const members = bindings.map(b => ts.createPropertyAssignment(b.field, b.expression));
const members = bindings.map(({field, expression}) => {
if (!tcb.config.checkTypeOfBindings) {
expression = tsCastToAny(expression);
}
return ts.createPropertyAssignment(field, expression);
});
// Call the `ngTypeCtor` method on the directive class, with an object literal argument created
// from the matched inputs.
@ -874,3 +890,8 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null {
return null;
}
}
function tsCastToAny(expr: ts.Expression): ts.Expression {
return ts.createParen(
ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)));
}

75
packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
View File

@ -12,7 +12,7 @@ import * as ts from 'typescript';
import {ImportMode, Reference, ReferenceEmitStrategy, ReferenceEmitter} from '../../imports';
import {ClassDeclaration, isNamedClassDeclaration} from '../../reflection';
import {ImportManager} from '../../translator';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from '../src/api';
import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../src/api';
import {generateTypeCheckBlock} from '../src/type_check_block';
@ -74,6 +74,66 @@ describe('type check blocks', () => {
expect(block).not.toContain('.class = ');
expect(block).not.toContain('.style = ');
});
describe('config', () => {
const DIRECTIVES: TestDirective[] = [{
name: 'Dir',
selector: '[dir]',
exportAs: ['dir'],
inputs: {'dirInput': 'dirInput'},
hasNgTemplateContextGuard: true,
}];
const BASE_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
};
describe('config.applyTemplateContextGuards', () => {
const TEMPLATE = `<div *dir></div>`;
const GUARD_APPLIED = 'if (i0.Dir.ngTemplateContextGuard(';
it('should apply template context guards when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(GUARD_APPLIED);
});
it('should not apply template context guards when disabled', () => {
const DISABLED_CONFIG = {...BASE_CONFIG, applyTemplateContextGuards: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).not.toContain(GUARD_APPLIED);
});
});
describe('config.checkTemplateBodies', () => {
const TEMPLATE = `<ng-template>{{a}}</ng-template>`;
it('should descend into template bodies when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('ctx.a;');
});
it('should not descend into template bodies when disabled', () => {
const DISABLED_CONFIG = {...BASE_CONFIG, checkTemplateBodies: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).not.toContain('ctx.a;');
});
});
describe('config.checkTypeOfBindings', () => {
const TEMPLATE = `<div dir [dirInput]="a" [nonDirInput]="a"></div>`;
it('should check types of bindings when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('i0.Dir.ngTypeCtor({ dirInput: ctx.a })');
expect(block).toContain('.nonDirInput = ctx.a;');
});
it('should not check types of bindings when disabled', () => {
const DISABLED_CONFIG = {...BASE_CONFIG, checkTypeOfBindings: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('i0.Dir.ngTypeCtor({ dirInput: (ctx.a as any) })');
expect(block).toContain('.nonDirInput = (ctx.a as any);');
});
});
});
});
it('should generate a circular directive reference correctly', () => {
@ -128,7 +188,8 @@ type TestDirective =
Partial<Pick<TypeCheckableDirectiveMeta, Exclude<keyof TypeCheckableDirectiveMeta, 'ref'>>>&
{selector: string, name: string};
function tcb(template: string, directives: TestDirective[] = []): string {
function tcb(
template: string, directives: TestDirective[] = [], config?: TypeCheckingConfig): string {
const classes = ['Test', ...directives.map(dir => dir.name)];
const code = classes.map(name => `class ${name} {}`).join('\n');
@ -161,9 +222,15 @@ function tcb(template: string, directives: TestDirective[] = []): string {
fnName: 'Test_TCB',
};
config = config || {
applyTemplateContextGuards: true,
checkTypeOfBindings: true,
checkTemplateBodies: true,
};
const im = new ImportManager(undefined, 'i');
const tcb =
generateTypeCheckBlock(clazz, meta, im, new ReferenceEmitter([new FakeReferenceStrategy()]));
const tcb = generateTypeCheckBlock(
clazz, meta, config, im, new ReferenceEmitter([new FakeReferenceStrategy()]));
const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tcb, sf);
return res.replace(/\s+/g, ' ');

9
packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts
View File

@ -13,6 +13,7 @@ import {LogicalFileSystem} from '../../path';
import {isNamedClassDeclaration} from '../../reflection';
import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript';
import {getRootDirs} from '../../util/src/typescript';
import {TypeCheckingConfig} from '../src/api';
import {TypeCheckContext} from '../src/context';
import {TypeCheckProgramHost} from '../src/host';
@ -24,6 +25,12 @@ const LIB_D_TS = {
type NonNullable<T> = T extends null | undefined ? never : T;`
};
const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkTemplateBodies: true,
checkTypeOfBindings: true,
};
describe('ngtsc typechecking', () => {
describe('ctors', () => {
it('compiles a basic type constructor', () => {
@ -47,7 +54,7 @@ TestClass.ngTypeCtor({value: 'test'});
new AbsoluteModuleStrategy(program, checker, options, host),
new LogicalProjectStrategy(checker, logicalFs),
]);
const ctx = new TypeCheckContext(emitter);
const ctx = new TypeCheckContext(ALL_ENABLED_CONFIG, emitter);
const TestClass = getDeclaration(program, 'main.ts', 'TestClass', isNamedClassDeclaration);
ctx.addTypeCtor(program.getSourceFile('main.ts') !, TestClass, {
fnName: 'ngTypeCtor',