Skip to content

Commit e6b2730

Browse files
committed
refactor(schematics): properly handle inherited constructors for signature checks
* Currently if someone extends an Angular Material class of which the signature has changed and the constructor is inherited, the constructor signature checks rule won't report the breaking change because the rule does not properly handle the inherited constructor.
1 parent f69f930 commit e6b2730

File tree

3 files changed

+45
-3
lines changed

3 files changed

+45
-3
lines changed

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,19 @@ function visitSourceFile(context: WalkContext<null>, program: ts.Program) {
6060
const className = classType.symbol && classType.symbol.name;
6161
const isNewExpression = ts.isNewExpression(node);
6262

63-
// TODO(devversion): Consider handling pass-through classes better.
64-
// TODO(devversion): e.g. `export class CustomCalendar extends MatCalendar {}`
65-
if (!constructorChecks.includes(className)) {
63+
// Determine the class names of the actual construct signatures because we cannot assume
64+
// that the diagnostic refers to a constructor of the actual expression. In case the constructor
65+
// is inherited, we need to detect that the owner-class of the constructor is added to the
66+
// constructor checks upgrade data. e.g. `class CustomCalendar extends MatCalendar {}`.
67+
const signatureClassNames = classType.getConstructSignatures()
68+
.map(signature => getClassDeclarationOfSignature(signature))
69+
.map(declaration => declaration && declaration.name ? declaration.name.text : null)
70+
.filter(Boolean);
71+
72+
// Besides checking the signature class names, we need to check the actual class name because
73+
// there can be classes without an explicit constructor.
74+
if (!constructorChecks.includes(className) &&
75+
!signatureClassNames.some(name => constructorChecks.includes(name!))) {
6676
continue;
6777
}
6878

@@ -116,3 +126,22 @@ function findConstructorNode(diagnostic: ts.Diagnostic, sourceFile: ts.SourceFil
116126

117127
return resolvedNode;
118128
}
129+
130+
/** Determines the class declaration of the specified construct signature. */
131+
function getClassDeclarationOfSignature(signature: ts.Signature): ts.ClassDeclaration | null {
132+
let node: ts.Node = signature.getDeclaration();
133+
134+
// Handle signatures which don't have an actual declaration. This happens if a class
135+
// does not have an explicitly written constructor.
136+
if (!node) {
137+
return null;
138+
}
139+
140+
while (!ts.isSourceFile(node = node.parent)) {
141+
if (ts.isClassDeclaration(node)) {
142+
return node;
143+
}
144+
}
145+
146+
return null;
147+
}

src/lib/schematics/update/test-cases/misc/constructor-checks.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ describe('v5 constructor checks', () => {
4343

4444
expect(output).toMatch(/Found "MatSidenavContent".*super.*: super\((any, ){4}any\)/);
4545
expect(output).toMatch(/Found "MatSidenavContent".*: new \w+\((any, ){4}any\)/);
46+
47+
expect(output).toMatch(/Found "ExtendedDateAdapter".*super.*: super\(string, Platform\)/);
48+
expect(output).toMatch(/Found "ExtendedDateAdapter".*: new \w+\(string, Platform\)/);
4649
});
4750
});
4851

src/lib/schematics/update/test-cases/misc/constructor-checks_input.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class MatSidenavContent {
6464
_ngZone: any) {}
6565
}
6666

67+
class ExtendedDateAdapter extends NativeDateAdapter {}
68+
6769
/* Actual test case using the previously defined definitions. */
6870

6971
class A extends NativeDateAdapter {
@@ -127,3 +129,11 @@ class G extends MatSidenavContent {
127129
}
128130

129131
const _G = new MatSidenavContent({}, 'container');
132+
133+
class H extends ExtendedDateAdapter {
134+
constructor() {
135+
super('myLocale');
136+
}
137+
}
138+
139+
const _H = new ExtendedDateAdapter('myLocale');

0 commit comments

Comments
 (0)