From 28358b6395985b76f49420875a74433ee0a81df5 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 19 Sep 2019 17:04:02 -0700 Subject: [PATCH] fix(language-service): Turn on strict mode for test project (#32783) This is the last part in refactoring of the test project. This PR turns on strict mode for typechecking and fixed tests that fail under this mode. PR Close #32783 --- .../language-service/test/completions_spec.ts | 7 ++-- .../language-service/test/diagnostics_spec.ts | 39 ++++++++++++------- .../test/project/app/app.component.ts | 4 +- .../test/project/app/expression-cases.ts | 2 +- .../test/project/app/ng-for-cases.ts | 4 +- .../test/project/app/parsing-cases.ts | 18 ++++----- .../test/project/tsconfig.json | 11 ++++++ packages/language-service/test/test_utils.ts | 29 +++++++------- 8 files changed, 70 insertions(+), 44 deletions(-) create mode 100644 packages/language-service/test/project/tsconfig.json diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 70d205fe68..b1163d93a3 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -152,10 +152,9 @@ describe('completions', () => { }); it('should respect paths configuration', () => { - mockHost.overrideOptions(options => { - options.baseUrl = '/app'; - options.paths = {'bar/*': ['foo/bar/*']}; - return options; + mockHost.overrideOptions({ + baseUrl: '/app', + paths: {'bar/*': ['foo/bar/*']}, }); mockHost.addScript('/app/foo/bar/shared.ts', ` export interface Node { diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index b017e7284a..d0ffac41cf 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -191,7 +191,7 @@ describe('diagnostics', () => { expect(() => ngLS.getDiagnostics(fileName)).not.toThrow(); }); - it('should not report an error for sub-types of string', () => { + it('should not report an error for sub-types of string in non-strict mode', () => { const fileName = '/app/app.component.ts'; mockHost.override(fileName, ` import { Component } from '@angular/core'; @@ -202,13 +202,16 @@ describe('diagnostics', () => { export class AppComponent { something: 'foo' | 'bar'; }`); + mockHost.overrideOptions({ + strict: false, // TODO: this test fails in strict mode + }); const tsDiags = tsLS.getSemanticDiagnostics(fileName); expect(tsDiags).toEqual([]); const ngDiags = ngLS.getDiagnostics(fileName); expect(ngDiags).toEqual([]); }); - it('should not report an error for sub-types of number', () => { + it('should not report an error for sub-types of number in non-strict mode', () => { const fileName = '/app/app.component.ts'; mockHost.override(fileName, ` import { Component } from '@angular/core'; @@ -219,6 +222,9 @@ describe('diagnostics', () => { export class AppComponent { something: 123 | 456; }`); + mockHost.overrideOptions({ + strict: false, // TODO: This test fails in strict mode + }); const tsDiags = tsLS.getSemanticDiagnostics(fileName); expect(tsDiags).toEqual([]); const ngDiags = ngLS.getDiagnostics(fileName); @@ -233,7 +239,7 @@ describe('diagnostics', () => { @Component({ template: '
' }) - export class MyComponent { + export class AppComponent { onClick() { } }`); const diagnostics = ngLS.getDiagnostics(fileName) !; @@ -245,7 +251,7 @@ describe('diagnostics', () => { }); // #13412 - it('should not report an error for using undefined', () => { + it('should not report an error for using undefined under non-strict mode', () => { const fileName = '/app/app.component.ts'; mockHost.override(fileName, ` import { Component } from '@angular/core'; @@ -256,6 +262,9 @@ describe('diagnostics', () => { export class AppComponent { something = 'foo'; }`); + mockHost.overrideOptions({ + strict: false, // TODO: This test fails in strict mode + }); const tsDiags = tsLS.getSemanticDiagnostics(fileName); expect(tsDiags).toEqual([]); const ngDiags = ngLS.getDiagnostics(fileName); @@ -342,7 +351,10 @@ describe('diagnostics', () => { }) export class AppComponent {}`); const tsDiags = tsLS.getSemanticDiagnostics(fileName); - expect(tsDiags).toEqual([]); + expect(tsDiags.length).toBe(1); + const msgText = ts.flattenDiagnosticMessageText(tsDiags[0].messageText, '\n'); + expect(msgText).toBe( + `Type 'null[]' is not assignable to type 'Provider[]'.\n Type 'null' is not assignable to type 'Provider'.`); const ngDiags = ngLS.getDiagnostics(fileName); expect(ngDiags.length).toBe(1); const {messageText, start, length} = ngDiags[0]; @@ -391,7 +403,7 @@ describe('diagnostics', () => { \` }) export class AppComponent { - fieldCount: number; + fieldCount: number = 0; }`); const tsDiags = tsLS.getSemanticDiagnostics(fileName); expect(tsDiags).toEqual([]); @@ -401,9 +413,8 @@ describe('diagnostics', () => { // Issue #15885 it('should be able to remove null and undefined from a type', () => { - mockHost.overrideOptions(options => { - options.strictNullChecks = true; - return options; + mockHost.overrideOptions({ + strictNullChecks: true, }); const fileName = '/app/app.component.ts'; mockHost.override(fileName, ` @@ -441,9 +452,8 @@ describe('diagnostics', () => { onSubmit(form: NgForm) {} }`); mockHost.addScript('/other/files/app/server.ts', 'export class Server {}'); - mockHost.overrideOptions(options => { - options.baseUrl = '/other/files'; - return options; + mockHost.overrideOptions({ + baseUrl: '/other/files', }); const tsDiags = tsLS.getSemanticDiagnostics(fileName); expect(tsDiags).toEqual([]); @@ -568,10 +578,13 @@ describe('diagnostics', () => { it('should not report errors for valid styleUrls', () => { const fileName = '/app/app.component.ts'; mockHost.override(fileName, ` + import {Component} from '@angular/core'; + @Component({ + template: '
', styleUrls: ['./test.css', './test.css'], }) - export class MyComponent {}`); + export class AppComponent {}`); const diagnostics = ngLS.getDiagnostics(fileName) !; expect(diagnostics.length).toBe(0); diff --git a/packages/language-service/test/project/app/app.component.ts b/packages/language-service/test/project/app/app.component.ts index 36a97d5055..d6cbb73f70 100644 --- a/packages/language-service/test/project/app/app.component.ts +++ b/packages/language-service/test/project/app/app.component.ts @@ -8,7 +8,7 @@ import {Component} from '@angular/core'; -export class Hero { +export interface Hero { id: number; name: string; } @@ -30,5 +30,5 @@ export class Hero { export class AppComponent { title = 'Tour of Heroes'; hero: Hero = {id: 1, name: 'Windstorm'}; - private internal: string; + private internal: string = 'internal'; } diff --git a/packages/language-service/test/project/app/expression-cases.ts b/packages/language-service/test/project/app/expression-cases.ts index 7dd2cacf36..84a78f2abb 100644 --- a/packages/language-service/test/project/app/expression-cases.ts +++ b/packages/language-service/test/project/app/expression-cases.ts @@ -44,5 +44,5 @@ export class ExpectNumericType { template: '{{ (name | lowercase).~{string-pipe}substring }}', }) export class LowercasePipe { - name: string; + name: string = 'name'; } diff --git a/packages/language-service/test/project/app/ng-for-cases.ts b/packages/language-service/test/project/app/ng-for-cases.ts index 1515c2ab2f..806daf5730 100644 --- a/packages/language-service/test/project/app/ng-for-cases.ts +++ b/packages/language-service/test/project/app/ng-for-cases.ts @@ -29,7 +29,7 @@ export class UnknownPeople { `, }) export class UnknownEven { - people: Person[]; + people: Person[] = []; } @Component({ @@ -39,5 +39,5 @@ export class UnknownEven { `, }) export class UnknownTrackBy { - people: Person[]; + people: Person[] = []; } diff --git a/packages/language-service/test/project/app/parsing-cases.ts b/packages/language-service/test/project/app/parsing-cases.ts index 3fcf215fcd..cd9fa37a4d 100644 --- a/packages/language-service/test/project/app/parsing-cases.ts +++ b/packages/language-service/test/project/app/parsing-cases.ts @@ -49,21 +49,21 @@ export class NoValueAttribute { template: '

', }) export class AttributeBinding { - test: string; + test: string = 'test'; } @Component({ template: '

', }) export class PropertyBinding { - test: string; + test: string = 'test'; } @Component({ template: '

', }) export class EventBinding { - test: string; + test: string = 'test'; modelChanged() {} } @@ -72,23 +72,23 @@ export class EventBinding { template: '

', }) export class TwoWayBinding { - test: string; + test: string = 'test'; } @Directive({ selector: '[string-model]', }) export class StringModel { - @Input() model: string; - @Output() modelChanged: EventEmitter; + @Input() model: string = 'model'; + @Output() modelChanged: EventEmitter = new EventEmitter(); } @Directive({ selector: '[number-model]', }) export class NumberModel { - @Input('inputAlias') model: number; - @Output('outputAlias') modelChanged: EventEmitter; + @Input('inputAlias') model: number = 0; + @Output('outputAlias') modelChanged: EventEmitter = new EventEmitter(); } interface Person { @@ -122,7 +122,7 @@ export class ForLetIEqual { `, }) export class ForUsingComponent { - people: Person[]; + people: Person[] = []; } @Component({ diff --git a/packages/language-service/test/project/tsconfig.json b/packages/language-service/test/project/tsconfig.json new file mode 100644 index 0000000000..0e793bb214 --- /dev/null +++ b/packages/language-service/test/project/tsconfig.json @@ -0,0 +1,11 @@ +{ + "//00": "This file is used for IDE only, actual compilation options is in MockTypescriptHost in test_utils.ts", + "compilerOptions": { + "strict": true, + "experimentalDecorators": true, + "baseUrl": "../../../..", + "paths": { + "@angular/*": ["packages/*"] + } + } +} diff --git a/packages/language-service/test/test_utils.ts b/packages/language-service/test/test_utils.ts index 8f87737fd9..4bf5074054 100644 --- a/packages/language-service/test/test_utils.ts +++ b/packages/language-service/test/test_utils.ts @@ -79,6 +79,17 @@ function loadTourOfHeroes(): ReadonlyMap { } const TOH = loadTourOfHeroes(); +const COMPILER_OPTIONS: Readonly = { + target: ts.ScriptTarget.ES5, + module: ts.ModuleKind.CommonJS, + moduleResolution: ts.ModuleResolutionKind.NodeJs, + emitDecoratorMetadata: true, + experimentalDecorators: true, + removeComments: false, + noImplicitAny: false, + lib: ['lib.es2015.d.ts', 'lib.dom.d.ts'], + strict: true, +}; export class MockTypescriptHost implements ts.LanguageServiceHost { private angularPath?: string; @@ -98,16 +109,7 @@ export class MockTypescriptHost implements ts.LanguageServiceHost { const support = setup(); this.nodeModulesPath = path.posix.join(support.basePath, 'node_modules'); this.angularPath = path.posix.join(this.nodeModulesPath, '@angular'); - this.options = { - target: ts.ScriptTarget.ES5, - module: ts.ModuleKind.CommonJS, - moduleResolution: ts.ModuleResolutionKind.NodeJs, - emitDecoratorMetadata: true, - experimentalDecorators: true, - removeComments: false, - noImplicitAny: false, - lib: ['lib.es2015.d.ts', 'lib.dom.d.ts'], - }; + this.options = COMPILER_OPTIONS; } override(fileName: string, content: string) { @@ -133,12 +135,12 @@ export class MockTypescriptHost implements ts.LanguageServiceHost { forgetAngular() { this.angularPath = undefined; } - overrideOptions(cb: (options: ts.CompilerOptions) => ts.CompilerOptions) { - this.options = cb((Object as any).assign({}, this.options)); + overrideOptions(options: Partial) { + this.options = {...this.options, ...options}; this.projectVersion++; } - getCompilationSettings(): ts.CompilerOptions { return this.options; } + getCompilationSettings(): ts.CompilerOptions { return {...this.options}; } getProjectVersion(): string { return this.projectVersion.toString(); } @@ -199,6 +201,7 @@ export class MockTypescriptHost implements ts.LanguageServiceHost { reset() { this.overrides.clear(); this.overrideDirectory.clear(); + this.options = COMPILER_OPTIONS; } private getRawFileContent(fileName: string): string|undefined {