From 90af397d9b42058902b828ac1aeaea19bc30ba07 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 13 Sep 2018 20:36:35 +0200 Subject: [PATCH] refactor(schematics): constructor check rule does not check all diagnostics (#13103) * Currently if the node of a diagnostic couldn't be resolved, or the `className` is not part of the upgrade data, we accidentally `return` instead of `continuing` the `for of` loop that checks every determined diagnostic. * Adds missing upgrade data for a V7 merged breaking change: https://github.com/angular/material2/pull/8286 * Renames the `test-cases/v5` directory to `test-cases/v6` because the test-cases are running against version target: `v6`. * Adds test cases for V7 constructor checks data; test cases for property name change from #8286 --- .../update/material/data/property-names.ts | 15 +++++++ .../constructorSignatureCheckRule.ts | 4 +- .../misc/constructor-checks.spec.ts | 16 +++++--- .../misc/constructor-checks_input.ts | 40 ++++++++++++++++--- ...st-cases.spec.ts => v6-test-cases.spec.ts} | 16 ++++---- .../attribute-selectors_expected_output.ts | 0 .../{v5 => v6}/attribute-selectors_input.ts | 0 .../{v5 => v6}/class-names_expected_output.ts | 0 .../{v5 => v6}/class-names_input.ts | 0 .../{v5 => v6}/css-names_expected_output.ts | 0 .../test-cases/{v5 => v6}/css-names_input.ts | 0 .../element-selectors_expected_output.ts | 0 .../{v5 => v6}/element-selectors_input.ts | 0 .../{v5 => v6}/input-names_expected_output.ts | 0 .../{v5 => v6}/input-names_input.ts | 0 .../output-names_expected_output.ts | 0 .../{v5 => v6}/output-names_input.ts | 0 .../property-names_expected_output.ts | 0 .../{v5 => v6}/property-names_input.ts | 0 .../update/test-cases/v7-test-cases.spec.ts | 38 ++++++++++++++++++ .../v7/property-names_expected_output.ts | 18 +++++++++ .../test-cases/v7/property-names_input.ts | 18 +++++++++ 22 files changed, 144 insertions(+), 21 deletions(-) rename src/lib/schematics/update/test-cases/{v5-test-cases.spec.ts => v6-test-cases.spec.ts} (88%) rename src/lib/schematics/update/test-cases/{v5 => v6}/attribute-selectors_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/attribute-selectors_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/class-names_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/class-names_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/css-names_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/css-names_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/element-selectors_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/element-selectors_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/input-names_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/input-names_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/output-names_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/output-names_input.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/property-names_expected_output.ts (100%) rename src/lib/schematics/update/test-cases/{v5 => v6}/property-names_input.ts (100%) create mode 100644 src/lib/schematics/update/test-cases/v7-test-cases.spec.ts create mode 100644 src/lib/schematics/update/test-cases/v7/property-names_expected_output.ts create mode 100644 src/lib/schematics/update/test-cases/v7/property-names_input.ts diff --git a/src/lib/schematics/update/material/data/property-names.ts b/src/lib/schematics/update/material/data/property-names.ts index 901a8bf4dbb..789beaa7313 100644 --- a/src/lib/schematics/update/material/data/property-names.ts +++ b/src/lib/schematics/update/material/data/property-names.ts @@ -22,6 +22,21 @@ export interface MaterialPropertyNameData { } export const propertyNames: VersionChanges = { + [TargetVersion.V7]: [ + { + pr: 'https://github.com/angular/material2/pull/8286', + changes: [ + { + replace: 'onChange', + replaceWith: 'changed', + whitelist: { + classes: ['SelectionModel'] + } + } + ] + } + ], + [TargetVersion.V6]: [ { pr: 'https://github.com/angular/material2/pull/10161', diff --git a/src/lib/schematics/update/rules/signature-check/constructorSignatureCheckRule.ts b/src/lib/schematics/update/rules/signature-check/constructorSignatureCheckRule.ts index 1c830b8b2a8..5667b79e24a 100644 --- a/src/lib/schematics/update/rules/signature-check/constructorSignatureCheckRule.ts +++ b/src/lib/schematics/update/rules/signature-check/constructorSignatureCheckRule.ts @@ -53,7 +53,7 @@ function visitSourceFile(context: WalkContext, program: ts.Program) { const node = findConstructorNode(diagnostic, sourceFile); if (!node) { - return; + continue; } const classType = program.getTypeChecker().getTypeAtLocation(node.expression); @@ -63,7 +63,7 @@ function visitSourceFile(context: WalkContext, program: ts.Program) { // TODO(devversion): Consider handling pass-through classes better. // TODO(devversion): e.g. `export class CustomCalendar extends MatCalendar {}` if (!constructorChecks.includes(className)) { - return; + continue; } const classSignatures = classType.getConstructSignatures() diff --git a/src/lib/schematics/update/test-cases/misc/constructor-checks.spec.ts b/src/lib/schematics/update/test-cases/misc/constructor-checks.spec.ts index 9cd1cdb7df6..88214f95eb9 100644 --- a/src/lib/schematics/update/test-cases/misc/constructor-checks.spec.ts +++ b/src/lib/schematics/update/test-cases/misc/constructor-checks.spec.ts @@ -16,6 +16,13 @@ describe('v5 constructor checks', () => { await runPostScheduledTasks(runner, 'tslint-fix').toPromise(); + expect(output).toMatch(/\[22.*Found "NativeDateAdapter"/, + 'Expected the constructor checks to report if an argument is not assignable.'); + expect(output).not.toMatch(/\[26.*Found "NativeDateAdapter".*/, + 'Expected the constructor to not report if an argument is assignable.'); + + expect(output).not.toMatch(/Found "NonMaterialClass".*: new NonMaterialClass\(\)/); + expect(output).toMatch(/Found "NativeDateAdapter".*super.*: super\(string, Platform\)/); expect(output).toMatch(/Found "NativeDateAdapter".*: new \w+\(string, Platform\)/); @@ -31,12 +38,11 @@ describe('v5 constructor checks', () => { expect(output).toMatch(/Found "MatCalendar".*super.*: super\(any, any, any, any\)/); expect(output).toMatch(/Found "MatCalendar".*: new \w+\(any, any, any, any\)/); - expect(output).toMatch(/\[97.*Found "NativeDateAdapter"/, - 'Expected the constructor checks to report if an argument is not assignable.'); - expect(output).not.toMatch(/\[99.*Found "NativeDateAdapter".*/, - 'Expected the constructor to not report if an argument is assignable.'); + expect(output).toMatch(/Found "MatDrawerContent".*super.*: super\((any, ){4}any\)/); + expect(output).toMatch(/Found "MatDrawerContent".*: new \w+\((any, ){4}any\)/); - expect(output).not.toMatch(/Found "NonMaterialClass".*: new NonMaterialClass\(\)/); + expect(output).toMatch(/Found "MatSidenavContent".*super.*: super\((any, ){4}any\)/); + expect(output).toMatch(/Found "MatSidenavContent".*: new \w+\((any, ){4}any\)/); }); }); diff --git a/src/lib/schematics/update/test-cases/misc/constructor-checks_input.ts b/src/lib/schematics/update/test-cases/misc/constructor-checks_input.ts index 2e089f9957b..6cf71ef731c 100644 --- a/src/lib/schematics/update/test-cases/misc/constructor-checks_input.ts +++ b/src/lib/schematics/update/test-cases/misc/constructor-checks_input.ts @@ -17,6 +17,14 @@ class MatAutocomplete { constructor(_changeDetector: any, _elementRef: any, _defaults: string[]) {} } +class NonMaterialClass {} + +const _t1 = new NativeDateAdapter('b', 'invalid-argument'); + +const _t2 = new NativeDateAdapter('a', {IOS: true}); + +const _t3 = new NonMaterialClass('invalid-argument'); + class MatTooltip { constructor( private _overlay: any, @@ -40,10 +48,20 @@ class MatCalendar { constructor(_intl: any, _adapter: any, _formats: any, _changeDetector: any) {} } -class NonMaterialClass {} +class MatDrawerContent { + constructor (_cd: any, + _container: any, + _elementRef: any, + _scrollDispatcher: any, + _ngZone: any) {} +} -class DelegateClass { - constructor(_adapter: NativeDateAdapter) {} +class MatSidenavContent { + constructor (_cd: any, + _container: any, + _elementRef: any, + _scrollDispatcher: any, + _ngZone: any) {} } /* Actual test case using the previously defined definitions. */ @@ -94,8 +112,18 @@ class E extends MatCalendar { const _E = new MatCalendar({}, {}, {}, {}, {}, {}, {}); -const _F = new NativeDateAdapter('b', 'invalid-argument'); +class F extends MatDrawerContent { + constructor(changeDetectorRef: any, container: any) { + super(changeDetectorRef, container); + } +} -const _G = new NativeDateAdapter('a', {IOS: true}); +const _F = new MatDrawerContent({}, 'container'); -const _H = new NonMaterialClass('invalid-argument'); +class G extends MatSidenavContent { + constructor(changeDetectorRef: any, container: any) { + super(changeDetectorRef, container); + } +} + +const _G = new MatSidenavContent({}, 'container'); diff --git a/src/lib/schematics/update/test-cases/v5-test-cases.spec.ts b/src/lib/schematics/update/test-cases/v6-test-cases.spec.ts similarity index 88% rename from src/lib/schematics/update/test-cases/v5-test-cases.spec.ts rename to src/lib/schematics/update/test-cases/v6-test-cases.spec.ts index 26c1a0becbf..145a6ec0b11 100644 --- a/src/lib/schematics/update/test-cases/v5-test-cases.spec.ts +++ b/src/lib/schematics/update/test-cases/v6-test-cases.spec.ts @@ -3,20 +3,20 @@ import {runPostScheduledTasks} from '../../test-setup/post-scheduled-tasks'; import {migrationCollection} from '../../test-setup/test-app'; import {createTestAppWithTestCase, readFileContent, resolveBazelDataFile} from './index.spec'; -describe('v5 test cases', () => { +describe('v6 upgrade test cases', () => { /** * Name of test cases that will be used to verify that update schematics properly update * a developers application. */ const testCases = [ - 'v5/attribute-selectors', - 'v5/class-names', - 'v5/css-names', - 'v5/element-selectors', - 'v5/input-names', - 'v5/output-names', - 'v5/property-names', + 'v6/attribute-selectors', + 'v6/class-names', + 'v6/css-names', + 'v6/element-selectors', + 'v6/input-names', + 'v6/output-names', + 'v6/property-names', ]; // Iterates through every test case directory and generates a jasmine test block that will diff --git a/src/lib/schematics/update/test-cases/v5/attribute-selectors_expected_output.ts b/src/lib/schematics/update/test-cases/v6/attribute-selectors_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/attribute-selectors_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/attribute-selectors_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/attribute-selectors_input.ts b/src/lib/schematics/update/test-cases/v6/attribute-selectors_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/attribute-selectors_input.ts rename to src/lib/schematics/update/test-cases/v6/attribute-selectors_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/class-names_expected_output.ts b/src/lib/schematics/update/test-cases/v6/class-names_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/class-names_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/class-names_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/class-names_input.ts b/src/lib/schematics/update/test-cases/v6/class-names_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/class-names_input.ts rename to src/lib/schematics/update/test-cases/v6/class-names_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/css-names_expected_output.ts b/src/lib/schematics/update/test-cases/v6/css-names_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/css-names_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/css-names_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/css-names_input.ts b/src/lib/schematics/update/test-cases/v6/css-names_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/css-names_input.ts rename to src/lib/schematics/update/test-cases/v6/css-names_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/element-selectors_expected_output.ts b/src/lib/schematics/update/test-cases/v6/element-selectors_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/element-selectors_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/element-selectors_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/element-selectors_input.ts b/src/lib/schematics/update/test-cases/v6/element-selectors_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/element-selectors_input.ts rename to src/lib/schematics/update/test-cases/v6/element-selectors_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/input-names_expected_output.ts b/src/lib/schematics/update/test-cases/v6/input-names_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/input-names_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/input-names_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/input-names_input.ts b/src/lib/schematics/update/test-cases/v6/input-names_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/input-names_input.ts rename to src/lib/schematics/update/test-cases/v6/input-names_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/output-names_expected_output.ts b/src/lib/schematics/update/test-cases/v6/output-names_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/output-names_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/output-names_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/output-names_input.ts b/src/lib/schematics/update/test-cases/v6/output-names_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/output-names_input.ts rename to src/lib/schematics/update/test-cases/v6/output-names_input.ts diff --git a/src/lib/schematics/update/test-cases/v5/property-names_expected_output.ts b/src/lib/schematics/update/test-cases/v6/property-names_expected_output.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/property-names_expected_output.ts rename to src/lib/schematics/update/test-cases/v6/property-names_expected_output.ts diff --git a/src/lib/schematics/update/test-cases/v5/property-names_input.ts b/src/lib/schematics/update/test-cases/v6/property-names_input.ts similarity index 100% rename from src/lib/schematics/update/test-cases/v5/property-names_input.ts rename to src/lib/schematics/update/test-cases/v6/property-names_input.ts diff --git a/src/lib/schematics/update/test-cases/v7-test-cases.spec.ts b/src/lib/schematics/update/test-cases/v7-test-cases.spec.ts new file mode 100644 index 00000000000..4310bfa1025 --- /dev/null +++ b/src/lib/schematics/update/test-cases/v7-test-cases.spec.ts @@ -0,0 +1,38 @@ +import {SchematicTestRunner} from '@angular-devkit/schematics/testing'; +import {runPostScheduledTasks} from '../../test-setup/post-scheduled-tasks'; +import {migrationCollection} from '../../test-setup/test-app'; +import {createTestAppWithTestCase, readFileContent, resolveBazelDataFile} from './index.spec'; + +describe('v7 upgrade test cases', () => { + + /** + * Name of test cases that will be used to verify that update schematics properly update + * a developers application. + */ + const testCases = [ + 'v7/property-names' + ]; + + // Iterates through every test case directory and generates a jasmine test block that will + // verify that the update schematics properly update the test input to the expected output. + testCases.forEach(testCaseName => { + const inputPath = resolveBazelDataFile(`${testCaseName}_input.ts`); + const expectedOutputPath = resolveBazelDataFile(`${testCaseName}_expected_output.ts`); + + it(`should apply update schematics to test case: ${testCaseName}`, () => { + const runner = new SchematicTestRunner('schematics', migrationCollection); + + runner.runSchematic('migration-02', {}, createTestAppWithTestCase(inputPath)); + + // Run the scheduled TSLint fix task from the update schematic. This task is responsible for + // identifying outdated code parts and performs the fixes. Since tasks won't run automatically + // within a `SchematicTestRunner`, we manually need to run the scheduled task. + return runPostScheduledTasks(runner, 'tslint-fix').toPromise().then(() => { + expect(readFileContent('projects/material/src/main.ts')) + .toBe(readFileContent(expectedOutputPath)); + }); + }); + }); +}); + + diff --git a/src/lib/schematics/update/test-cases/v7/property-names_expected_output.ts b/src/lib/schematics/update/test-cases/v7/property-names_expected_output.ts new file mode 100644 index 00000000000..e4869c6daf4 --- /dev/null +++ b/src/lib/schematics/update/test-cases/v7/property-names_expected_output.ts @@ -0,0 +1,18 @@ +import {EventEmitter, OnInit} from '@angular/core'; + +class SelectionModel { + onChange = new EventEmitter(); +} + +/* Actual test case using the previously defined definitions. */ + +class A implements OnInit { + self = {me: this}; + + constructor(private a: SelectionModel) {} + + ngOnInit() { + this.a.changed.subscribe(() => console.log('Changed')); + this.self.me.a.changed.subscribe(() => console.log('Changed 2')); + } +} diff --git a/src/lib/schematics/update/test-cases/v7/property-names_input.ts b/src/lib/schematics/update/test-cases/v7/property-names_input.ts new file mode 100644 index 00000000000..0cc059d4052 --- /dev/null +++ b/src/lib/schematics/update/test-cases/v7/property-names_input.ts @@ -0,0 +1,18 @@ +import {EventEmitter, OnInit} from '@angular/core'; + +class SelectionModel { + onChange = new EventEmitter(); +} + +/* Actual test case using the previously defined definitions. */ + +class A implements OnInit { + self = {me: this}; + + constructor(private a: SelectionModel) {} + + ngOnInit() { + this.a.onChange.subscribe(() => console.log('Changed')); + this.self.me.a.onChange.subscribe(() => console.log('Changed 2')); + } +}