Skip to content

Commit 51af471

Browse files
devversionjelbourn
authored andcommitted
refactor(schematics): remove type checker for identifier switch rule (#12454)
Removes type checking from the `switchIdentifiersRule` because we don't want to depend on types for older versions when running the schematics. The rule still maintains the explicit checks that ensure that only identifiers from Material or the CDK are being updated,
1 parent 73f3f94 commit 51af471

File tree

3 files changed

+52
-85
lines changed

3 files changed

+52
-85
lines changed

src/lib/schematics/update/material/typescript-specifiers.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ export const materialModuleSpecifier = '@angular/material';
1515
/** Name of the Angular CDK module specifier. */
1616
export const cdkModuleSpecifier = '@angular/cdk';
1717

18-
/** Whether the specified node is part of an Angular Material import declaration. */
18+
/** Whether the specified node is part of an Angular Material or CDK import declaration. */
1919
export function isMaterialImportDeclaration(node: ts.Node) {
2020
return isMaterialDeclaration(getImportDeclaration(node));
2121
}
2222

23-
/** Whether the specified node is part of an Angular Material export declaration. */
23+
/** Whether the specified node is part of an Angular Material or CDK import declaration. */
2424
export function isMaterialExportDeclaration(node: ts.Node) {
25-
return getExportDeclaration(getImportDeclaration(node));
25+
return isMaterialDeclaration(getExportDeclaration(node));
2626
}
2727

2828
/** Whether the declaration is part of Angular Material. */

src/lib/schematics/update/rules/switchIdentifiersRule.ts

Lines changed: 48 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -7,136 +7,103 @@
77
*/
88

99
import {green, red} from 'chalk';
10-
import {relative} from 'path';
11-
import {ProgramAwareRuleWalker, RuleFailure, Rules} from 'tslint';
10+
import {RuleFailure, Rules, RuleWalker} from 'tslint';
1211
import * as ts from 'typescript';
1312
import {classNames} from '../material/data/class-names';
1413
import {
1514
isMaterialExportDeclaration,
1615
isMaterialImportDeclaration,
1716
} from '../material/typescript-specifiers';
18-
import {getOriginalSymbolFromNode} from '../typescript/identifiers';
1917
import {
2018
isExportSpecifierNode,
2119
isImportSpecifierNode,
22-
isNamespaceImportNode
20+
isNamespaceImportNode,
2321
} from '../typescript/imports';
2422

2523
/**
2624
* Rule that walks through every identifier that is part of Angular Material and replaces the
2725
* outdated name with the new one.
2826
*/
29-
export class Rule extends Rules.TypedRule {
30-
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
31-
return this.applyWithWalker(
32-
new SwitchIdentifiersWalker(sourceFile, this.getOptions(), program));
27+
export class Rule extends Rules.AbstractRule {
28+
29+
apply(sourceFile: ts.SourceFile): RuleFailure[] {
30+
return this.applyWithWalker(new SwitchIdentifiersWalker(sourceFile, this.getOptions()));
3331
}
3432
}
3533

36-
export class SwitchIdentifiersWalker extends ProgramAwareRuleWalker {
37-
constructor(sf, opt, prog) {
38-
super(sf, opt, prog);
39-
}
34+
export class SwitchIdentifiersWalker extends RuleWalker {
4035

41-
/** List of Angular Material declarations inside of the current source file. */
42-
materialDeclarations: ts.Declaration[] = [];
36+
/**
37+
* List of identifier names that have been imported from `@angular/material` or `@angular/cdk`
38+
* in the current source file and therefore can be considered trusted.
39+
*/
40+
trustedIdentifiers: Set<string> = new Set();
4341

44-
/** List of Angular Material namespace declarations in the current source file. */
45-
materialNamespaceDeclarations: ts.Declaration[] = [];
42+
/** List of namespaces that have been imported from `@angular/material` or `@angular/cdk`. */
43+
trustedNamespaces: Set<string> = new Set();
4644

4745
/** Method that is called for every identifier inside of the specified project. */
4846
visitIdentifier(identifier: ts.Identifier) {
49-
// Store Angular Material namespace identifiers in a list of declarations.
50-
// Namespace identifiers can be: `import * as md from '@angular/material';`
51-
this._storeNamespaceImports(identifier);
52-
5347
// For identifiers that aren't listed in the className data, the whole check can be
5448
// skipped safely.
5549
if (!classNames.some(data => data.replace === identifier.text)) {
5650
return;
5751
}
5852

59-
const symbol = getOriginalSymbolFromNode(identifier, this.getTypeChecker());
53+
// For namespace imports that are referring to Angular Material or the CDK, we store the
54+
// namespace name in order to be able to safely find identifiers that don't belong to the
55+
// developer's application.
56+
if (isNamespaceImportNode(identifier) && isMaterialImportDeclaration(identifier)) {
57+
this.trustedNamespaces.add(identifier.text);
6058

61-
// If the symbol is not defined or could not be resolved, just skip the following identifier
62-
// checks.
63-
if (!symbol || !symbol.name || symbol.name === 'unknown') {
64-
console.error(`Could not resolve symbol for identifier "${identifier.text}" ` +
65-
`in file ${this._getRelativeFileName()}`);
66-
return;
59+
return this._createFailureWithReplacement(identifier);
6760
}
6861

69-
// For export declarations that are referring to Angular Material, the identifier should be
70-
// switched to the new name.
62+
// For export declarations that are referring to Angular Material or the CDK, the identifier
63+
// can be immediately updated to the new name.
7164
if (isExportSpecifierNode(identifier) && isMaterialExportDeclaration(identifier)) {
72-
return this.createIdentifierFailure(identifier, symbol);
65+
return this._createFailureWithReplacement(identifier);
7366
}
7467

75-
// For import declarations that are referring to Angular Material, the value declarations
76-
// should be stored so that other identifiers in the file can be compared.
68+
// For import declarations that are referring to Angular Material or the CDK, the name of
69+
// the import identifiers. This allows us to identify identifiers that belong to Material and
70+
// the CDK, and we won't accidentally touch a developer's identifier.
7771
if (isImportSpecifierNode(identifier) && isMaterialImportDeclaration(identifier)) {
78-
this.materialDeclarations.push(symbol.valueDeclaration);
72+
this.trustedIdentifiers.add(identifier.text);
7973

80-
// For identifiers that are not part of an import or export, the list of Material declarations
81-
// should be checked to ensure that only identifiers of Angular Material are updated.
82-
// Identifiers that are imported through an Angular Material namespace will be updated.
83-
} else if (this.materialDeclarations.indexOf(symbol.valueDeclaration) === -1 &&
84-
!this._isIdentifierFromNamespace(identifier)) {
85-
return;
74+
return this._createFailureWithReplacement(identifier);
8675
}
8776

88-
return this.createIdentifierFailure(identifier, symbol);
77+
// In case the identifier is part of a property access expression, we need to verify that the
78+
// property access originates from a namespace that has been imported from Material or the CDK.
79+
if (ts.isPropertyAccessExpression(identifier.parent)) {
80+
const expression = identifier.parent.expression;
81+
82+
if (ts.isIdentifier(expression) && this.trustedNamespaces.has(expression.text)) {
83+
return this._createFailureWithReplacement(identifier);
84+
}
85+
} else if (this.trustedIdentifiers.has(identifier.text)) {
86+
return this._createFailureWithReplacement(identifier);
87+
}
8988
}
9089

9190
/** Creates a failure and replacement for the specified identifier. */
92-
private createIdentifierFailure(identifier: ts.Identifier, symbol: ts.Symbol) {
93-
let classData = classNames.find(
94-
data => data.replace === symbol.name || data.replace === identifier.text);
91+
private _createFailureWithReplacement(identifier: ts.Identifier) {
92+
const classData = classNames.find(data => data.replace === identifier.text);
9593

9694
if (!classData) {
97-
console.error(`Could not find updated name for identifier "${identifier.getText()}" in ` +
98-
` in file ${this._getRelativeFileName()}.`);
95+
console.error(`Could not find updated name for identifier "${identifier.text}" in ` +
96+
` in file ${this.getSourceFile().fileName}.`);
9997
return;
10098
}
10199

102100
const replacement = this.createReplacement(
103-
identifier.getStart(), identifier.getWidth(), classData.replaceWith);
101+
identifier.getStart(), identifier.getWidth(), classData.replaceWith);
104102

105103
this.addFailureAtNode(
106-
identifier,
107-
`Found deprecated identifier "${red(classData.replace)}" which has been renamed to` +
108-
` "${green(classData.replaceWith)}"`,
109-
replacement);
110-
}
111-
112-
/** Checks namespace imports from Angular Material and stores them in a list. */
113-
private _storeNamespaceImports(identifier: ts.Identifier) {
114-
// In some situations, developers will import Angular Material completely using a namespace
115-
// import. This is not recommended, but should be still handled in the migration tool.
116-
if (isNamespaceImportNode(identifier) && isMaterialImportDeclaration(identifier)) {
117-
const symbol = getOriginalSymbolFromNode(identifier, this.getTypeChecker());
118-
119-
if (symbol) {
120-
return this.materialNamespaceDeclarations.push(symbol.valueDeclaration);
121-
}
122-
}
123-
}
124-
125-
/** Checks whether the given identifier is part of the Material namespace. */
126-
private _isIdentifierFromNamespace(identifier: ts.Identifier) {
127-
if (identifier.parent && identifier.parent.kind !== ts.SyntaxKind.PropertyAccessExpression) {
128-
return;
129-
}
130-
131-
const propertyExpression = identifier.parent as ts.PropertyAccessExpression;
132-
const expressionSymbol = getOriginalSymbolFromNode(propertyExpression.expression,
133-
this.getTypeChecker());
134-
135-
return this.materialNamespaceDeclarations.indexOf(expressionSymbol.valueDeclaration) !== -1;
136-
}
137-
138-
/** Returns the current source file path relative to the root directory of the project. */
139-
private _getRelativeFileName(): string {
140-
return relative(this.getProgram().getCurrentDirectory(), this.getSourceFile().fileName);
104+
identifier,
105+
`Found deprecated identifier "${red(classData.replace)}" which has been renamed to` +
106+
` "${green(classData.replaceWith)}"`,
107+
replacement);
141108
}
142109
}

tools/gulp/tasks/material-release.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ task('schematics:build', tsBuildTask(join(schematicsDir, 'tsconfig.json')));
5656
* Task that will build the material package. Special treatment for this package includes:
5757
* - Copying all prebuilt themes into the package
5858
* - Bundling theming scss into a single theming file
59-
* - Copying schematics code into the packatge
59+
* - Copying schematics code into the package
6060
*/
6161
task('material:prepare-release', sequenceTask(
6262
['material:build', 'schematics:build'],

0 commit comments

Comments
 (0)