8000 fixup! feat(core): add ModuleWithProviders generic type migration · angular/angular@dd4b96a · GitHub
[go: up one dir, main page]
Skip to content

Commit dd4b96a

Browse files
CaerusKaruCaerusKaru
committed
fixup! feat(core): add ModuleWithProviders generic type migration
1 parent 9672579 commit dd4b96a

File tree

4 files changed

+53
-58
lines changed

4 files changed

+53
-58
lines changed

‎packages/core/schematics/migrations/module-with-providers/index.ts

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host'
1515
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
1616

1717
import {Collector} from './collector';
18-
import {ModuleWithProvidersTransform} from './transform';
18+
import {AnalysisFailure, ModuleWithProvidersTransform} from './transform';
1919

2020

2121

@@ -59,36 +59,28 @@ function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, baseP
5959

6060
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
6161
const typeChecker = program.getTypeChecker();
62-
const moduleCollector = new Collector(typeChecker);
62+
const collector = new Collector(typeChecker);
6363
const sourceFiles = program.getSourceFiles().filter(
6464
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
6565

6666
// Analyze source files by detecting all modules.
67-
sourceFiles.forEach(sourceFile => moduleCollector.visitNode(sourceFile));
67+
sourceFiles.forEach(sourceFile => collector.visitNode(sourceFile));
6868

69-
const {resolvedModules, resolvedNonGenerics} = moduleCollector;
69+
const {resolvedModules, resolvedNonGenerics} = collector;
7070
const transformer = new ModuleWithProvidersTransform(typeChecker, getUpdateRecorder);
7171
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();
7272

73-
resolvedModules.forEach(module => {
74-
transformer.migrateModule(module).forEach(({message, node}) => {
75-
const nodeSourceFile = node.getSourceFile();
76-
const relativeFilePath = relative(basePath, nodeSourceFile.fileName);
77-
const {line, character} =
78-
ts.getLineAndCharacterOfPosition(node.getSourceFile(), node.getStart());
79-
failures.push(`${relativeFilePath}@${line + 1}:${character + 1}: ${message}`);
80-
});
81-
});
82-
83-
resolvedNonGenerics.forEach(type => {
84-
transformer.migrateType(type).forEach(({message, node}) => {
85-
const nodeSourceFile = node.getSourceFile();
86-
const relativeFilePath = relative(basePath, nodeSourceFile.fileName);
87-
const {line, character} =
88-
ts.getLineAndCharacterOfPosition(node.getSourceFile(), node.getStart());
89-
failures.push(`${relativeFilePath}@${line + 1}:${character + 1}: ${message}`);
90-
});
91-
});
73+
[...resolvedModules.reduce(
74+
(failures, m) => failures.concat(transformer.migrateModule(m)), [] as AnalysisFailure[]),
75+
...resolvedNonGenerics.reduce(
76+
(failures, t) => failures.concat(transformer.migrateType(t)), [] as AnalysisFailure[])]
77+
.forEach(({message, node}) => {
78+
const nodeSourceFile = node.getSourceFile();
79+
const relativeFilePath = relative(basePath, nodeSourceFile.fileName);
80+
const {line, character} =
81+
ts.getLineAndCharacterOfPosition(node.getSourceFile(), node.getStart());
82+
failures.push(`${relativeFilePath}@${line + 1}:${character + 1}: ${message}`);
83+
});
9284

9385
// Walk through each update recorder and commit the update. We need to commit the
9486
// updates in batches per source file as there can be only one recorder per source

‎packages/core/schematics/migrations/module-with-providers/transform.ts

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,47 +27,49 @@ export class ModuleWithProvidersTransform {
2727
private partialEvaluator: PartialEvaluator =
2828
new PartialEvaluator(new TypeScriptReflectionHost(this.typeChecker), this.typeChecker);
2929

30-
/** Set of methods which were already checked or migrated. */
31-
private visitedMethods = new Set<ts.MethodDeclaration>();
32-
3330
constructor(
3431
private typeChecker: ts.TypeChecker,
3532
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {}
3633

3734
/** Migrates a given NgModule by walking through the referenced providers and static methods. */
3835
migrateModule(module: ResolvedNgModule): AnalysisFailure[] {
39-
return module.staticMethodsWithoutType.map(this._resolveStaticMethod.bind(this))
36+
return module.staticMethodsWithoutType.map(this._migrateStaticNgModuleMethod.bind(this))
4037
.filter(v => v) as AnalysisFailure[];
4138
}
4239

4340
/** Migrates a ModuleWithProviders type definition that has no explicit generic type */
44-
migrateType(type: ts.TypeReferenceNode) {
41+
migrateType(type: ts.TypeReferenceNode): AnalysisFailure[] {
4542
const parent = type.parent;
4643
let moduleText: string|undefined;
4744
if ((ts.isFunctionDeclaration(parent) || ts.isMethodDeclaration(parent)) && parent.body) {
4845
const returnStatement = parent.body.statements.find(ts.isReturnStatement);
49-
moduleText = this._getReturnStatementNgModuleType(returnStatement);
46+
47+
// No return type found, exit
48+
if (!returnStatement || !returnStatement.expression) {
49+
return [{node: parent, message: `Return type is not statically analyzable.`}];
50+
}
51+
52+
moduleText = this._getNgModuleTypeOfExpression(returnStatement.expression);
5053
} else if (ts.isPropertyDeclaration(parent) || ts.isVariableDeclaration(parent)) {
5154
if (!parent.initializer) {
5255
addTodoToNode(type, TODO_COMMENT);
5356
this._updateNode(type, type);
5457
return [{node: parent, message: `Unable to determine type for declaration.`}];
5558
}
5659

57-
const evaluatedExpr = this.partialEvaluator.evaluate(parent.initializer);
58-
moduleText = this._visitObjectLiteralExpressionResolvedValue(evaluatedExpr);
60+
moduleText = this._getNgModuleTypeOfExpression(parent.initializer);
5961
}
6062

6163
if (moduleText) {
62-
this.migrateTypeReferenceNode(type, moduleText);
64+
this._addGenericToTypeReference(type, moduleText);
6365
return [];
6466
}
6567

6668
return [{node: parent, message: `Type is not statically analyzable.`}];
6769
}
6870

6971
/** Add a given generic to a type reference node */
70-
migrateTypeReferenceNode(node: ts.TypeReferenceNode, typeName: string) {
72+
private _addGenericToTypeReference(node: ts.TypeReferenceNode, typeName: string) {
7173
const newGenericExpr = createModuleWithProvidersType(typeName, node);
7274
this._updateNode(node, newGenericExpr);
7375
}
@@ -76,12 +78,7 @@ export class ModuleWithProvidersTransform {
7678
* Migrates a given static method if its ModuleWithProviders does not provide
7779
* a generic type.
7880
*/
79-
migrateStaticMethod(method: ts.MethodDeclaration, typeName: string) {
80-
if (this.visitedMethods.has(method)) {
81-
return;
82-
}
83-
this.visitedMethods.add(method);
84-
81+
private _updateStaticMethodType(method: ts.MethodDeclaration, typeName: string) {
8582
const newGenericExpr =
8683
createModuleWithProvidersType(typeName, method.type as ts.TypeReferenceNode);
8784
const newMethodDecl = ts.updateMethod(
@@ -100,33 +97,38 @@ export class ModuleWithProvidersTransform {
10097
return ngModule && (value.size === 1 || (providers && value.size === 2));
10198
}
10299

103-
/** Determine the generic type of a suspected ModuleWithProviders return type */
104-
private _resolveStaticMethod(node: ts.MethodDeclaration): AnalysisFailure|null {
105-
const returnStatement: ts.ReturnStatement|undefined = node.body &&
100+
/** Determine the generic type of a suspected ModuleWithProviders return type and add it
101+
* explicitly */
102+
private _migrateStaticNgModuleMethod(node: ts.MethodDeclaration): AnalysisFailure|null {
103+
const returnStatement = node.body &&
106104
node.body.statements.find(n => ts.isReturnStatement(n)) as ts.ReturnStatement | undefined;
107-
const moduleText = this._getReturnStatementNgModuleType(returnStatement);
105+
106+
// No return type found, exit
107+
if (!returnStatement || !returnStatement.expression) {
108+
return {node: node, message: `Return type is not statically analyzable.`};
109+
}
110+
111+
const moduleText = this._getNgModuleTypeOfExpression(returnStatement.expression);
108112

109113
if (moduleText) {
110-
this.migrateStaticMethod(node, moduleText);
114+
this._updateStaticMethodType(node, moduleText);
111115
return null;
112116
}
113117

114118
return {node: node, message: `Method type is not statically analyzable.`};
115119
}
116120

117-
/** Evaluate and return the ngModule type from a method or function's return statement */
118-
private _getReturnStatementNgModuleType(returnStatement?: ts.ReturnStatement): string|undefined {
119-
// No return type found, exit
120-
if (!returnStatement || !returnStatement.expression) {
121-
return;
122-
}
123-
124-
const evaluatedExpr = this.partialEvaluator.evaluate(returnStatement.expression);
125-
return this._visitObjectLiteralExpressionResolvedValue(evaluatedExpr);
121+
/** Evaluate and return the ngModule type from an expression */
122+
private _getNgModuleTypeOfExpression(expr: ts.Expression): string|undefined {
123+
const evaluatedExpr = this.partialEvaluator.evaluate(expr);
124+
return this._getTypeOfResolvedValue(evaluatedExpr);
126125
}
127126

128-
/** Visits a given object literal expression to determine the ngModule type. */
129-
private _visitObjectLiteralExpressionResolvedValue(value: ResolvedValue): string|undefined {
127+
/**
128+
* Visits a given object literal expression to determine the ngModule type. If the expression
129+
* cannot be resolved, add a TODO to alert the user.
130+
*/
131+
private _getTypeOfResolvedValue(value: ResolvedValue): string|undefined {
130132
if (value instanceof Map && this.isModuleWithProvidersType(value)) {
131133
const mapValue = value.get('ngModule') !;
132134
if (mapValue instanceof Reference && ts.isClassDeclaration(mapValue.node) &&

‎packages/core/schematics/migrations/module-with-providers/util.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ export function createModuleWithProvidersType(
1818
typeNode, typeNode.typeName, ts.createNodeArray([typeReferenceNode]));
1919
}
2020

21-
/** Determine whether a node is a ModuleWithProviders type reference node with a generic type */
21+
/** Determine whether a node is a ModuleWithProviders type reference node without a generic type */
2222
export function isModuleWithProvidersNotGeneric(
2323
typeChecker: ts.TypeChecker, node: ts.Node): node is ts.TypeReferenceNode {
2424
if (!ts.isTypeReferenceNode(node) || !ts.isIdentifier(node.typeName)) {
2525
return false;
2626
}
2727

2828
const imp = getImportOfIdentifier(typeChecker, node.typeName);
29-
return !!imp && imp.name === 'ModuleWithProviders' && imp.importModule === '@angular/core';
29+
return !!imp && imp.name === 'ModuleWithProviders' && imp.importModule === '@angular/core' &&
30+
!node.typeArguments;
3031
}

‎packages/core/schematics/test/module_with_providers_migration_spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ describe('ModuleWithProviders migration', () => {
191191
192192
export const myModuleWithProviders: ModuleWithProviders = {ngModule: BaseModule};
193193
194-
export function mwpFunction: ModuleWithProviders {
194+
export function mwpFunction(): ModuleWithProviders {
195195
return myModuleWithProviders;
196196
}
197197

0 commit comments

Comments
 (0)
0