Skip to content

Commit

Permalink
refactor(schematics): constructor check rule does not check all diagn…
Browse files Browse the repository at this point in the history
…ostics

* 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: angular#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 angular#8286
  • Loading branch information
devversion committed Sep 13, 2018
1 parent 18d0fa8 commit 04edb81
Show file tree
Hide file tree
Showing 22 changed files with 144 additions and 21 deletions.
15 changes: 15 additions & 0 deletions src/lib/schematics/update/material/data/property-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\)/);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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. */
Expand Down Expand Up @@ -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');
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions src/lib/schematics/update/test-cases/v7-test-cases.spec.ts
Original file line number Diff line number Diff line change
@@ -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));
});
});
});
});


Original file line number Diff line number Diff line change
@@ -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'));
}
}
18 changes: 18 additions & 0 deletions src/lib/schematics/update/test-cases/v7/property-names_input.ts
Original file line number Diff line number Diff line change
@@ -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'));
}
}

0 comments on commit 04edb81

Please sign in to comment.