From 2ed0d513b097908e1f3588a9e83f90bb926f088d Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 19 Oct 2019 21:27:11 +0200 Subject: [PATCH] feat(ivy): strictness flags for template type checking The template type checking abilities of the Ivy compiler are far more advanced than the level of template type checking that was previously done for Angular templates. Up until now, a single compiler option called "fullTemplateTypeCheck" was available to configure the level of template type checking. However, now that more advanced type checking is being done, new errors may surface that were previously not reported, in which case it may not be feasible to fix all new errors at once. Having only a single option to disable a large number of template type checking capabilities does not allow for incrementally addressing newly reported types of errors. As a solution, this commit introduces some new compiler options to be able to enable/disable certain kinds of template type checks on a fine-grained basis. --- packages/compiler-cli/src/ngtsc/program.ts | 27 ++ packages/compiler-cli/src/transformers/api.ts | 94 +++++ .../test/ngtsc/template_typecheck_spec.ts | 346 ++++++++++++++++++ 3 files changed, 467 insertions(+) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 780fc1a93adc4c..cd5093a2e942bc 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -453,6 +453,33 @@ export class NgtscProgram implements api.Program { }; } + // Apply explicitly configured strictness flags on top of the default configuration + // based on "fullTemplateTypeCheck". + if (this.options.strictInputTypes !== undefined) { + typeCheckingConfig.checkTypeOfInputBindings = this.options.strictInputTypes; + } + if (this.options.strictNullInputTypes !== undefined) { + typeCheckingConfig.strictNullInputBindings = this.options.strictNullInputTypes; + } + if (this.options.strictOutputEventTypes !== undefined) { + typeCheckingConfig.checkTypeOfOutputEvents = this.options.strictOutputEventTypes; + } + if (this.options.strictAnimationEventTypes !== undefined) { + typeCheckingConfig.checkTypeOfAnimationEvents = this.options.strictAnimationEventTypes; + } + if (this.options.strictDomEventTypes !== undefined) { + typeCheckingConfig.checkTypeOfDomEvents = this.options.strictDomEventTypes; + } + if (this.options.strictSafeNavigationTypes !== undefined) { + typeCheckingConfig.strictSafeNavigationTypes = this.options.strictSafeNavigationTypes; + } + if (this.options.strictLocalRefTypes !== undefined) { + typeCheckingConfig.checkTypeOfReferences = this.options.strictLocalRefTypes; + } + if (this.options.strictAttributeTypes !== undefined) { + typeCheckingConfig.checkTypeOfAttributes = this.options.strictAttributeTypes; + } + // Execute the typeCheck phase of each decorator in the program. const prepSpan = this.perfRecorder.start('typeCheckPrep'); const ctx = new TypeCheckContext(typeCheckingConfig, this.refEmitter !, this.typeCheckFilePath); diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts index 4df350040a96fc..a7aa422b4bf40e 100644 --- a/packages/compiler-cli/src/transformers/api.ts +++ b/packages/compiler-cli/src/transformers/api.ts @@ -112,6 +112,100 @@ export interface CompilerOptions extends ts.CompilerOptions { // This will be true be default in Angular 6. fullTemplateTypeCheck?: boolean; + /** + * Whether to check the type of a binding to a directive/component input against the type of the + * field on the directive/component. + * + * 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. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. + */ + strictInputTypes?: boolean; + + /** + * Whether to use strict null types for input bindings for directives. + * + * If this is `true`, applications that are compiled with TypeScript's `strictNullChecks` enabled + * will produce type errors for bindings which can evaluate to `undefined` or `null` where the + * inputs's type does not include `undefined` or `null` in its type. If set to `false`, all + * binding expressions are wrapped in a non-null assertion operator to effectively disable strict + * null checks. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. Note that if + * `strictInputTypes` is set to `false`, this flag has no effect. + */ + strictNullInputTypes?: boolean; + + /** + * Whether to check text attributes that happen to be consumed by a directive or component. + * + * For example, in a template containing `` the `disabled` attribute ends + * up being consumed as an input with type `boolean` by the `matInput` directive. At runtime the + * input will be set to the attribute's string value, which is the empty string for attributes + * without a value, so with this flag set to `true` an error would be reported. If set to `false`, + * text attributes will never report an error. + * + * Note that if `strictInputTypes` is set to `false`, this flag has no effect. + */ + strictAttributeTypes?: 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`. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. + */ + strictSafeNavigationTypes?: boolean; + + /** + * Whether to infer the type of local references. + * + * If this is `true`, the type of `#ref` variables in the template will be determined by the + * referenced entity (either a directive or a DOM element). If set to `false`, the type of `ref` + * will be `any`. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. + */ + strictLocalRefTypes?: boolean; + + /** + * Whether to infer the type of the `$event` variable in event bindings for directive outputs. + * + * If this is `true`, the type of `$event` will be inferred based on the generic type of + * `EventEmitter`/`Subject` of the output. If set to `false`, the `$event` variable will be of + * type `any`. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. + */ + strictOutputEventTypes?: boolean; + + /** + * Whether to infer the type of the `$event` variable in event bindings for animations. + * + * If this is `true`, the type of `$event` will be `AnimationEvent` from `@angular/animations`. + * If set to `false`, the `$event` variable will be of type `any`. + * + * Defaults to `true` if "fullTemplateTypeCheck" is set, `false` otherwise. + */ + strictAnimationEventTypes?: boolean; + + /** + * Whether to infer the type of the `$event` variable in event bindings to DOM events. + * + * If this is `true`, the type of `$event` will be inferred based on TypeScript's + * `HTMLElementEventMap`, with a fallback to the native `Event` type. If set to `false`, the + * `$event` variable will be of type `any`. + * + * Defaults to `false`. + */ + strictDomEventTypes?: boolean; + // Whether to use the CompilerHost's fileNameToModuleName utility (if available) to generate // import module specifiers. This is false by default, and exists to support running ngtsc // within Google. This option is internal and is used by the ng_module.bzl rule to switch diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 2791395b1b4051..467667fd538b34 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -58,6 +58,11 @@ export declare class NgIf { export declare class CommonModule { static ɵmod: i0.ɵɵNgModuleDefWithMeta; } +`); + env.write('node_modules/@angular/animations/index.d.ts', ` +export declare class AnimationEvent { + element: any; +} `); }); @@ -142,6 +147,347 @@ export declare class CommonModule { expect(diags[2].messageText).toEqual(`Property 'focused' does not exist on type 'TestCmp'.`); }); + describe('strictInputTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp {} + + @Directive({selector: '[dir]'}) + class TestDir { + @Input() foo: string; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + }); + + it('should check expressions and their type when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toEqual(`Type 'number' is not assignable to type 'string'.`); + expect(diags[1].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + + it('should check expressions but not their type when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + + describe('strictNullInputTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + nullable: string | null | undefined; + } + + @Directive({selector: '[dir]'}) + class TestDir { + @Input() foo: string; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + }); + + it('should check expressions and their nullability when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictNullInputTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText) + .toEqual(`Type 'string | null | undefined' is not assignable to type 'string'.`); + expect(diags[1].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + + it('should check expressions but not their nullability when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictNullInputTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + + describe('strictSafeNavigationTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + user?: {name: string}; + } + + @Directive({selector: '[dir]'}) + class TestDir { + @Input() foo: string; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + }); + + it('should infer result type for safe navigation expressions when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictSafeNavigationTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText) + .toEqual(`Type 'string | undefined' is not assignable to type 'string'.`); + expect(diags[1].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + + it('should not infer result type for safe navigation expressions when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictSafeNavigationTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + + describe('strictOutputEventTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, Directive, EventEmitter, NgModule, Output} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + update(data: string) {} + } + + @Directive({selector: '[dir]'}) + class TestDir { + @Output() update = new EventEmitter(); + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + }); + + it('should expressions and infer type of $event when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + expect(diags[1].messageText) + .toEqual(`Argument of type 'number' is not assignable to parameter of type 'string'.`); + }); + + it('should check expressions but not infer type of $event when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + + describe('strictAnimationEventTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + update(data: string) {} + } + + @NgModule({ + declarations: [TestCmp], + }) + class Module {} + `); + }); + + it('should check expressions and let $event be of type AnimationEvent when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictAnimationEventTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + expect(diags[1].messageText) + .toEqual( + `Argument of type 'AnimationEvent' is not assignable to parameter of type 'string'.`); + }); + + it('should check expressions and let $event be of type any when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictAnimationEventTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + + describe('strictLocalRefTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'test', + template: '{{ref.does_not_exist}}', + }) + class TestCmp {} + + @NgModule({ + declarations: [TestCmp], + }) + class Module {} + `); + }); + + it('should infer the type of references when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictLocalRefTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'does_not_exist' does not exist on type 'HTMLInputElement'.`); + }); + + it('should let the type of reference be any when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictLocalRefTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + }); + + describe('strictAttributeTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + @Component({ + selector: 'test', + template: '', + }) + class TestCmp {} + + @Directive({selector: '[dir]'}) + class TestDir { + @Input() disabled: boolean; + @Input() cols: number; + } + + @NgModule({ + declarations: [TestCmp, TestDir], + }) + class Module {} + `); + }); + + it('should produce an error for text attributes when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictAttributeTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText).toEqual(`Type 'string' is not assignable to type 'boolean'.`); + expect(diags[1].messageText).toEqual(`Type 'string' is not assignable to type 'number'.`); + }); + + it('should not produce an error for text attributes when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictAttributeTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + }); + + describe('strictDomEventTypes', () => { + beforeEach(() => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp { + update(data: string) {} + } + + @NgModule({ + declarations: [TestCmp], + }) + class Module {} + `); + }); + + it('should check expressions and infer type of $event when enabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictDomEventTypes: true}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + expect(diags[1].messageText) + .toEqual( + `Argument of type 'FocusEvent' is not assignable to parameter of type 'string'.`); + }); + + it('should check expressions but not infer type of $event when disabled', () => { + env.tsconfig({fullTemplateTypeCheck: true, strictDomEventTypes: false}); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText) + .toEqual(`Property 'invalid' does not exist on type 'TestCmp'.`); + }); + }); + it('should check basic usage of NgIf', () => { env.write('test.ts', ` import {CommonModule} from '@angular/common';