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: #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

Author: devversion

src/lib/schematics/update/material/data/property-names.ts

@@ -22,6 +22,21 @@ export interface MaterialPropertyNameData {
 }
 
 export const propertyNames: VersionChanges<MaterialPropertyNameData> = {
+  [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',

src/lib/schematics/update/rules/signature-check/constructorSignatureCheckRule.ts

@@ -53,7 +53,7 @@ function visitSourceFile(context: WalkContext<null>, 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<null>, 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()

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\)/);
   });
 });
 

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 _F = new MatDrawerContent({}, 'container');
 
-const _G = new NativeDateAdapter('a', {IOS: true});
+class G extends MatSidenavContent {
+  constructor(changeDetectorRef: any, container: any) {
+    super(changeDetectorRef, container);
+  }
+}
 
-const _H = new NonMaterialClass('invalid-argument');
+const _G = new MatSidenavContent({}, 'container');

src/lib/schematics/update/test-cases/v5-test-cases.spec.ts renamed to 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

src/lib/schematics/update/test-cases/v5/attribute-selectors_expected_output.ts renamed to src/lib/schematics/update/test-cases/v6/attribute-selectors_expected_output.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));
+      });
+    });
+  });
+});
+
+

src/lib/schematics/update/test-cases/v5/attribute-selectors_input.ts renamed to src/lib/schematics/update/test-cases/v6/attribute-selectors_input.ts

@@ -0,0 +1,18 @@
+import {EventEmitter, OnInit} from '@angular/core';
+
+class SelectionModel {
+  onChange = new EventEmitter<void>();
+}
+
+/* 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'));
+  }
+}

src/lib/schematics/update/test-cases/v5/class-names_expected_output.ts renamed to src/lib/schematics/update/test-cases/v6/class-names_expected_output.ts

@@ -0,0 +1,18 @@
+import {EventEmitter, OnInit} from '@angular/core';
+
+class SelectionModel {
+  onChange = new EventEmitter<void>();
+}
+
+/* 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'));
+  }
+}