Skip to content

Commit 1c988bb

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 1c988bb

File tree

3 files changed

+46
-3
lines changed

3 files changed

+46
-3
lines changed

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

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,20 @@ 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 constructor comes from the inherited class and
66+
// check if the inherited is added to the constructor checks upgrade data.
67+
// e.g. `class CustomCalendar extends MatCalendar {}`.
68+
const signatureClassNames = classType.getConstructSignatures()
69+
.map(signature => getClassDeclarationOfSignature(signature))
70+
.map(declaration => declaration && declaration.name ? declaration.name.text : null)
71+
.filter(Boolean);
72+
73+
// Besides checking the signature class names, we need to check the actual class name because
74+
// there can be classes without an explicit constructor.
75+
if (!constructorChecks.includes(className) &&
76+
!signatureClassNames.some(name => constructorChecks.includes(name!))) {
6677
continue;
6778
}
6879

@@ -89,6 +100,25 @@ function getParameterTypesFromSignature(signature: ts.Signature, program: ts.Pro
89100
.map(typeNode => program.getTypeChecker().getTypeFromTypeNode(typeNode!));
90101
}
91102

103+
/** Determines the class declaration of the given construct signature. */
104+
function getClassDeclarationOfSignature(signature: ts.Signature): ts.ClassDeclaration | null {
105+
let node: ts.Node = signature.getDeclaration();
106+
107+
// Handle signatures which don't have an actual declaration. This happens if a class
108+
// does not have an explicitly written constructor.
109+
if (!node) {
110+
return null;
111+
}
112+
113+
while (!ts.isSourceFile(node = node.parent)) {
114+
if (ts.isClassDeclaration(node)) {
115+
return node;
116+
}
117+
}
118+
119+
return null;
120+
}
121+
92122
/**
93123
* Walks through each node of a source file in order to find a new-expression node or super-call
94124
* expression node that is captured by the specified diagnostic.

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)