From 23664802504d6b4bfd6869fbe6a982ff21c22ab2 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 11 Feb 2020 10:50:29 +0100 Subject: [PATCH] refactor(core): move schematic import manager to shared utils (#35339) The import manager has been created for both the `missing-injectable` and `undecorated-classes-with-di` migration. Both initial PRs brought in the manager class, so the manager is duplicated in the schematics. In order to reduce this duplication, and to expose the manager to other schematics/migrations, we move it into the shared schematic utils. PR Close #35339 --- .../missing-injectable/transform.ts | 3 +- .../missing-injectable/update_recorder.ts | 5 +- .../decorator_rewrite/decorator_rewriter.ts | 3 +- .../import_rewrite_visitor.ts | 3 +- .../import_manager.ts | 254 ------------------ .../undecorated-classes-with-di/transform.ts | 2 +- .../update_recorder.ts | 5 +- .../import_manager.ts | 9 +- 8 files changed, 18 insertions(+), 266 deletions(-) delete mode 100644 packages/core/schematics/migrations/undecorated-classes-with-di/import_manager.ts rename packages/core/schematics/{migrations/missing-injectable => utils}/import_manager.ts (97%) diff --git a/packages/core/schematics/migrations/missing-injectable/transform.ts b/packages/core/schematics/migrations/missing-injectable/transform.ts index d0412974e0..c70e75b12d 100644 --- a/packages/core/schematics/migrations/missing-injectable/transform.ts +++ b/packages/core/schematics/migrations/missing-injectable/transform.ts @@ -11,14 +11,15 @@ import {DynamicValue, ResolvedValue} from '@angular/compiler-cli/src/ngtsc/parti import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection'; import * as ts from 'typescript'; +import {ImportManager} from '../../utils/import_manager'; import {getAngularDecorators} from '../../utils/ng_decorators'; import {ResolvedDirective, ResolvedNgModule} from './definition_collector'; -import {ImportManager} from './import_manager'; import {ProviderLiteral, ProvidersEvaluator} from './providers_evaluator'; import {UpdateRecorder} from './update_recorder'; + /** Name of decorators which imply that a given class does not need to be migrated. */ const NO_MIGRATE_DECORATORS = ['Injectable', 'Directive', 'Component', 'Pipe']; diff --git a/packages/core/schematics/migrations/missing-injectable/update_recorder.ts b/packages/core/schematics/migrations/missing-injectable/update_recorder.ts index 80fc670f15..0e2ad3ee0d 100644 --- a/packages/core/schematics/migrations/missing-injectable/update_recorder.ts +++ b/packages/core/schematics/migrations/missing-injectable/update_recorder.ts @@ -7,15 +7,14 @@ */ import * as ts from 'typescript'; +import {ImportManagerUpdateRecorder} from '../../utils/import_manager'; /** * Update recorder interface that is used to transform source files in a non-colliding * way. Also this indirection makes it possible to re-use logic for both TSLint rules * and CLI devkit schematic updates. */ -export interface UpdateRecorder { - addNewImport(start: number, importText: string): void; - updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void; +export interface UpdateRecorder extends ImportManagerUpdateRecorder { addClassDecorator(node: ts.ClassDeclaration, text: string, className: string): void; replaceDecorator(node: ts.Decorator, newText: string, className: string): void; updateObjectLiteral(node: ts.ObjectLiteralExpression, newText: string): void; diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/decorator_rewriter.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/decorator_rewriter.ts index f2b2c4cf32..fa34c27d63 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/decorator_rewriter.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/decorator_rewriter.ts @@ -10,12 +10,13 @@ import {AotCompiler} from '@angular/compiler'; import {PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator'; import * as ts from 'typescript'; +import {ImportManager} from '../../../utils/import_manager'; import {NgDecorator} from '../../../utils/ng_decorators'; import {unwrapExpression} from '../../../utils/typescript/functions'; -import {ImportManager} from '../import_manager'; import {ImportRewriteTransformerFactory, UnresolvedIdentifierError} from './import_rewrite_visitor'; + /** * Class that can be used to copy decorators to a new location. The rewriter ensures that * identifiers and imports are rewritten to work in the new file location. Fields in a diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/import_rewrite_visitor.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/import_rewrite_visitor.ts index 4176876158..3003e9cc27 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/import_rewrite_visitor.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-di/decorator_rewrite/import_rewrite_visitor.ts @@ -10,13 +10,14 @@ import {AotCompilerHost} from '@angular/compiler'; import {dirname, resolve} from 'path'; import * as ts from 'typescript'; +import {ImportManager} from '../../../utils/import_manager'; import {Import, getImportOfIdentifier} from '../../../utils/typescript/imports'; import {getValueSymbolOfDeclaration} from '../../../utils/typescript/symbol'; -import {ImportManager} from '../import_manager'; import {getPosixPath} from './path_format'; import {ResolvedExport, getExportSymbolsOfFile} from './source_file_exports'; + /** * Factory that creates a TypeScript transformer which ensures that * referenced identifiers are available at the target file location. diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/import_manager.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/import_manager.ts deleted file mode 100644 index 8bc46eca1f..0000000000 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/import_manager.ts +++ /dev/null @@ -1,254 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {dirname, resolve} from 'path'; -import * as ts from 'typescript'; -import {UpdateRecorder} from './update_recorder'; - -/** - * Import manager that can be used to add TypeScript imports to given source - * files. The manager ensures that multiple transformations are applied properly - * without shifted offsets and that similar existing import declarations are re-used. - */ -export class ImportManager { - /** Map of import declarations that need to be updated to include the given symbols. */ - private updatedImports = - new Map(); - /** Map of source-files and their previously used identifier names. */ - private usedIdentifierNames = new Map(); - /** - * Array of previously resolved symbol imports. Cache can be re-used to return - * the same identifier without checking the source-file again. - */ - private importCache: { - sourceFile: ts.SourceFile, - symbolName: string|null, - moduleName: string, - identifier: ts.Identifier - }[] = []; - - constructor( - private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder, - private printer: ts.Printer) {} - - /** - * Adds an import to the given source-file and returns the TypeScript - * identifier that can be used to access the newly imported symbol. - */ - addImportToSourceFile( - sourceFile: ts.SourceFile, symbolName: string|null, moduleName: string, - typeImport = false): ts.Expression { - const sourceDir = dirname(sourceFile.fileName); - let importStartIndex = 0; - let existingImport: ts.ImportDeclaration|null = null; - - // In case the given import has been already generated previously, we just return - // the previous generated identifier in order to avoid duplicate generated imports. - const cachedImport = this.importCache.find( - c => c.sourceFile === sourceFile && c.symbolName === symbolName && - c.moduleName === moduleName); - if (cachedImport) { - return cachedImport.identifier; - } - - // Walk through all source-file top-level statements and search for import declarations - // that already match the specified "moduleName" and can be updated to import the - // given symbol. If no matching import can be found, the last import in the source-file - // will be used as starting point for a new import that will be generated. - for (let i = sourceFile.statements.length - 1; i >= 0; i--) { - const statement = sourceFile.statements[i]; - - if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier) || - !statement.importClause) { - continue; - } - - if (importStartIndex === 0) { - importStartIndex = this._getEndPositionOfNode(statement); - } - - const moduleSpecifier = statement.moduleSpecifier.text; - - if (moduleSpecifier.startsWith('.') && - resolve(sourceDir, moduleSpecifier) !== resolve(sourceDir, moduleName) || - moduleSpecifier !== moduleName) { - continue; - } - - if (statement.importClause.namedBindings) { - const namedBindings = statement.importClause.namedBindings; - - // In case a "Type" symbol is imported, we can't use namespace imports - // because these only export symbols available at runtime (no types) - if (ts.isNamespaceImport(namedBindings) && !typeImport) { - return ts.createPropertyAccess( - ts.createIdentifier(namedBindings.name.text), - ts.createIdentifier(symbolName || 'default')); - } else if (ts.isNamedImports(namedBindings) && symbolName) { - const existingElement = namedBindings.elements.find( - e => - e.propertyName ? e.propertyName.text === symbolName : e.name.text === symbolName); - - if (existingElement) { - return ts.createIdentifier(existingElement.name.text); - } - - // In case the symbol could not be found in an existing import, we - // keep track of the import declaration as it can be updated to include - // the specified symbol name without having to create a new import. - existingImport = statement; - } - } else if (statement.importClause.name && !symbolName) { - return ts.createIdentifier(statement.importClause.name.text); - } - } - - if (existingImport) { - const propertyIdentifier = ts.createIdentifier(symbolName !); - const generatedUniqueIdentifier = this._getUniqueIdentifier(sourceFile, symbolName !); - const needsGeneratedUniqueName = generatedUniqueIdentifier.text !== symbolName; - const importName = needsGeneratedUniqueName ? generatedUniqueIdentifier : propertyIdentifier; - - // Since it can happen that multiple classes need to be imported within the - // specified source file and we want to add the identifiers to the existing - // import declaration, we need to keep track of the updated import declarations. - // We can't directly update the import declaration for each identifier as this - // would throw off the recorder offsets. We need to keep track of the new identifiers - // for the import and perform the import transformation as batches per source-file. - this.updatedImports.set( - existingImport, (this.updatedImports.get(existingImport) || []).concat({ - propertyName: needsGeneratedUniqueName ? propertyIdentifier : undefined, - importName: importName, - })); - - // Keep track of all updated imports so that we don't generate duplicate - // similar imports as these can't be statically analyzed in the source-file yet. - this.importCache.push({sourceFile, moduleName, symbolName, identifier: importName}); - - return importName; - } - - let identifier: ts.Identifier|null = null; - let newImport: ts.ImportDeclaration|null = null; - - if (symbolName) { - const propertyIdentifier = ts.createIdentifier(symbolName); - const generatedUniqueIdentifier = this._getUniqueIdentifier(sourceFile, symbolName); - const needsGeneratedUniqueName = generatedUniqueIdentifier.text !== symbolName; - identifier = needsGeneratedUniqueName ? generatedUniqueIdentifier : propertyIdentifier; - - newImport = ts.createImportDeclaration( - undefined, undefined, - ts.createImportClause( - undefined, - ts.createNamedImports([ts.createImportSpecifier( - needsGeneratedUniqueName ? propertyIdentifier : undefined, identifier)])), - ts.createStringLiteral(moduleName)); - } else { - identifier = this._getUniqueIdentifier(sourceFile, 'defaultExport'); - newImport = ts.createImportDeclaration( - undefined, undefined, ts.createImportClause(identifier, undefined), - ts.createStringLiteral(moduleName)); - } - - const newImportText = this.printer.printNode(ts.EmitHint.Unspecified, newImport, sourceFile); - // If the import is generated at the start of the source file, we want to add - // a new-line after the import. Otherwise if the import is generated after an - // existing import, we need to prepend a new-line so that the import is not on - // the same line as the existing import anchor. - this.getUpdateRecorder(sourceFile) - .addNewImport( - importStartIndex, importStartIndex === 0 ? `${newImportText}\n` : `\n${newImportText}`); - - // Keep track of all generated imports so that we don't generate duplicate - // similar imports as these can't be statically analyzed in the source-file yet. - this.importCache.push({sourceFile, symbolName, moduleName, identifier}); - - return identifier; - } - - /** - * Stores the collected import changes within the appropriate update recorders. The - * updated imports can only be updated *once* per source-file because previous updates - * could otherwise shift the source-file offsets. - */ - recordChanges() { - this.updatedImports.forEach((expressions, importDecl) => { - const sourceFile = importDecl.getSourceFile(); - const recorder = this.getUpdateRecorder(sourceFile); - const namedBindings = importDecl.importClause !.namedBindings as ts.NamedImports; - const newNamedBindings = ts.updateNamedImports( - namedBindings, - namedBindings.elements.concat(expressions.map( - ({propertyName, importName}) => ts.createImportSpecifier(propertyName, importName)))); - - const newNamedBindingsText = - this.printer.printNode(ts.EmitHint.Unspecified, newNamedBindings, sourceFile); - recorder.updateExistingImport(namedBindings, newNamedBindingsText); - }); - } - - /** Gets an unique identifier with a base name for the given source file. */ - private _getUniqueIdentifier(sourceFile: ts.SourceFile, baseName: string): ts.Identifier { - if (this.isUniqueIdentifierName(sourceFile, baseName)) { - this._recordUsedIdentifier(sourceFile, baseName); - return ts.createIdentifier(baseName); - } - - let name = null; - let counter = 1; - do { - name = `${baseName}_${counter++}`; - } while (!this.isUniqueIdentifierName(sourceFile, name)); - - this._recordUsedIdentifier(sourceFile, name !); - return ts.createIdentifier(name !); - } - - /** - * Checks whether the specified identifier name is used within the given - * source file. - */ - private isUniqueIdentifierName(sourceFile: ts.SourceFile, name: string) { - if (this.usedIdentifierNames.has(sourceFile) && - this.usedIdentifierNames.get(sourceFile) !.indexOf(name) !== -1) { - return false; - } - - // Walk through the source file and search for an identifier matching - // the given name. In that case, it's not guaranteed that this name - // is unique in the given declaration scope and we just return false. - const nodeQueue: ts.Node[] = [sourceFile]; - while (nodeQueue.length) { - const node = nodeQueue.shift() !; - if (ts.isIdentifier(node) && node.text === name) { - return false; - } - nodeQueue.push(...node.getChildren()); - } - return true; - } - - private _recordUsedIdentifier(sourceFile: ts.SourceFile, identifierName: string) { - this.usedIdentifierNames.set( - sourceFile, (this.usedIdentifierNames.get(sourceFile) || []).concat(identifierName)); - } - - /** - * Determines the full end of a given node. By default the end position of a node is - * before all trailing comments. This could mean that generated imports shift comments. - */ - private _getEndPositionOfNode(node: ts.Node) { - const nodeEndPos = node.getEnd(); - const commentRanges = ts.getTrailingCommentRanges(node.getSourceFile().text, nodeEndPos); - if (!commentRanges || !commentRanges.length) { - return nodeEndPos; - } - return commentRanges[commentRanges.length - 1] !.end; - } -} diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/transform.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/transform.ts index ac3d92e16d..5ec39b6e3e 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/transform.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-di/transform.ts @@ -11,6 +11,7 @@ import {PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluato import {ChangeDetectionStrategy, ViewEncapsulation} from '@angular/core'; import * as ts from 'typescript'; +import {ImportManager} from '../../utils/import_manager'; import {getAngularDecorators} from '../../utils/ng_decorators'; import {hasExplicitConstructor} from '../../utils/typescript/class_declaration'; import {findBaseClassDeclarations} from '../../utils/typescript/find_base_classes'; @@ -18,7 +19,6 @@ import {getImportOfIdentifier} from '../../utils/typescript/imports'; import {UnexpectedMetadataValueError, convertDirectiveMetadataToExpression} from './decorator_rewrite/convert_directive_metadata'; import {DecoratorRewriter} from './decorator_rewrite/decorator_rewriter'; -import {ImportManager} from './import_manager'; import {hasDirectiveDecorator, hasInjectableDecorator} from './ng_declaration_collector'; import {UpdateRecorder} from './update_recorder'; diff --git a/packages/core/schematics/migrations/undecorated-classes-with-di/update_recorder.ts b/packages/core/schematics/migrations/undecorated-classes-with-di/update_recorder.ts index baa341ecce..ed5ee2b2a9 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-di/update_recorder.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-di/update_recorder.ts @@ -8,15 +8,14 @@ */ import * as ts from 'typescript'; +import {ImportManagerUpdateRecorder} from '../../utils/import_manager'; /** * Update recorder interface that is used to transform source files in a non-colliding * way. Also this indirection makes it possible to re-use transformation logic with * different replacement tools (e.g. TSLint or CLI devkit). */ -export interface UpdateRecorder { - addNewImport(start: number, importText: string): void; - updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void; +export interface UpdateRecorder extends ImportManagerUpdateRecorder { addClassDecorator(node: ts.ClassDeclaration, text: string): void; addClassComment(node: ts.ClassDeclaration, text: string): void; commitUpdate(): void; diff --git a/packages/core/schematics/migrations/missing-injectable/import_manager.ts b/packages/core/schematics/utils/import_manager.ts similarity index 97% rename from packages/core/schematics/migrations/missing-injectable/import_manager.ts rename to packages/core/schematics/utils/import_manager.ts index 8bc46eca1f..12a9b6cbcc 100644 --- a/packages/core/schematics/migrations/missing-injectable/import_manager.ts +++ b/packages/core/schematics/utils/import_manager.ts @@ -8,7 +8,12 @@ import {dirname, resolve} from 'path'; import * as ts from 'typescript'; -import {UpdateRecorder} from './update_recorder'; + +/** Update recorder for managing imports. */ +export interface ImportManagerUpdateRecorder { + addNewImport(start: number, importText: string): void; + updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void; +} /** * Import manager that can be used to add TypeScript imports to given source @@ -33,7 +38,7 @@ export class ImportManager { }[] = []; constructor( - private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder, + private getUpdateRecorder: (sf: ts.SourceFile) => ImportManagerUpdateRecorder, private printer: ts.Printer) {} /**