diff --git a/packages/core/schematics/migrations/move-document/document_import_visitor.ts b/packages/core/schematics/migrations/move-document/document_import_visitor.ts index 9b5199c5abc5e..e9546e22aba23 100644 --- a/packages/core/schematics/migrations/move-document/document_import_visitor.ts +++ b/packages/core/schematics/migrations/move-document/document_import_visitor.ts @@ -8,22 +8,20 @@ import * as ts from 'typescript'; -const COMMON_IMPORT = '@angular/common'; -const PLATFORM_BROWSER_IMPORT = '@angular/platform-browser'; +export const COMMON_IMPORT = '@angular/common'; +export const PLATFORM_BROWSER_IMPORT = '@angular/platform-browser'; export const DOCUMENT_TOKEN_NAME = 'DOCUMENT'; -export interface Imports { +/** This contains the metadata necessary to move items from one import to another */ +export interface ResolvedDocumentImport { platformBrowserImport: ts.NamedImports|null; commonImport: ts.NamedImports|null; - replaceText: string|null; documentElement: ts.ImportSpecifier|null; } -/** - * Visitor that can be used to find a set of imports in a TypeScript file. - */ +/** Visitor that can be used to find a set of imports in a TypeScript file. */ export class DocumentImportVisitor { - importsMap: Map = new Map(); + importsMap: Map = new Map(); constructor(public typeChecker: ts.TypeChecker) {} @@ -49,7 +47,6 @@ export class DocumentImportVisitor { imports = { platformBrowserImport: null, commonImport: null, - replaceText: null, documentElement: null, }; } @@ -57,12 +54,13 @@ export class DocumentImportVisitor { if (moduleSpecifier.text === PLATFORM_BROWSER_IMPORT) { const documentElement = this.getDocumentElement(node); if (documentElement) { - imports.replaceText = this.getReplaceText(documentElement); imports.platformBrowserImport = node; imports.documentElement = documentElement; } } else if (moduleSpecifier.text === COMMON_IMPORT) { imports.commonImport = node; + } else { + return; } this.importsMap.set(sourceFile, imports); } @@ -71,10 +69,4 @@ export class DocumentImportVisitor { const elements = node.elements; return elements.find(el => (el.propertyName || el.name).escapedText === DOCUMENT_TOKEN_NAME); } - - private getReplaceText(documentElement: ts.ImportSpecifier): string { - return documentElement.propertyName ? - `${DOCUMENT_TOKEN_NAME} as ${documentElement.name.escapedText}` : - DOCUMENT_TOKEN_NAME; - } } diff --git a/packages/core/schematics/migrations/move-document/google3/BUILD.bazel b/packages/core/schematics/migrations/move-document/google3/BUILD.bazel deleted file mode 100644 index deaacc0d68211..0000000000000 --- a/packages/core/schematics/migrations/move-document/google3/BUILD.bazel +++ /dev/null @@ -1,12 +0,0 @@ -load("//tools:defaults.bzl", "ts_library") - -ts_library( - name = "google3", - srcs = glob(["**/*.ts"]), - tsconfig = "//packages/core/schematics:tsconfig.json", - visibility = ["//packages/core/schematics/test:__pkg__"], - deps = [ - "//packages/core/schematics/migrations/move-document", - "@npm//tslint", - ], -) diff --git a/packages/core/schematics/migrations/move-document/google3/moveDocumentRule.ts b/packages/core/schematics/migrations/move-document/google3/moveDocumentRule.ts deleted file mode 100644 index a2ab86f4eb4f9..0000000000000 --- a/packages/core/schematics/migrations/move-document/google3/moveDocumentRule.ts +++ /dev/null @@ -1,71 +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 {Replacement, RuleFailure, Rules} from 'tslint'; -import * as ts from 'typescript'; - -import {DOCUMENT_TOKEN_NAME, DocumentImportVisitor} from '../document_import_visitor'; -import {addToImport, removeFromImport} from '../move-import'; - - -/** - * Rule that moves the DOCUMENT InjectionToken from the deprecation source in - * angular/platform-browser - * to the new source in angular/common. The rule also provides TSLint automatic replacements that - * can - * be applied in order to automatically migrate to the new source. - */ -export class Rule extends Rules.TypedRule { - static FAILURE: string = - `DOCUMENT is no longer exported from @angular/platform-browser in v8. Please - import from @angular/common`; - - applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { - const visitor = new DocumentImportVisitor(program.getTypeChecker()); - const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !); - const failures: RuleFailure[] = []; - - // Analyze source files by detecting imports and declarations. - rootSourceFiles.forEach(sourceFile => visitor.visitNode(sourceFile)); - - const {importsMap} = visitor; - const imports = importsMap.get(sourceFile); - - // No DOCUMENT imports from platform-browser detected for the given source file. - if (!imports) { - return []; - } - - const {platformBrowserImport, commonImport, documentElement, replaceText} = imports; - - if (!documentElement || !replaceText || !platformBrowserImport) { - return []; - } - - // Replace the imports with the updated sources. - const platformBrowserDeclaration = platformBrowserImport.parent.parent; - const newPlatformBrowserText = removeFromImport(platformBrowserImport, DOCUMENT_TOKEN_NAME); - const newCommonText = - commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT; - const fixPlatformBrowser = new Replacement( - platformBrowserDeclaration.getStart(), platformBrowserDeclaration.getWidth(), - newPlatformBrowserText); - const fixCommon = new Replacement(platformBrowserDeclaration.end, 0, newCommonText); - const fixes: Replacement[] = fixPlatformBrowser.start > fixCommon.start ? - [fixCommon, fixPlatformBrowser] : - [fixPlatformBrowser, fixCommon]; - - failures.push(new RuleFailure( - sourceFile, documentElement.getStart(), documentElement.getWidth(), Rule.FAILURE, - this.ruleName, fixes)); - - return failures; - } -} - -const NEW_COMMON_TEXT = `import {DOCUMENT} from '@angular/common';`; diff --git a/packages/core/schematics/migrations/move-document/index.ts b/packages/core/schematics/migrations/move-document/index.ts index 2266ec5243844..a6fc607425c81 100644 --- a/packages/core/schematics/migrations/move-document/index.ts +++ b/packages/core/schematics/migrations/move-document/index.ts @@ -12,8 +12,8 @@ import * as ts from 'typescript'; import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; -import {DOCUMENT_TOKEN_NAME, DocumentImportVisitor, Imports} from './document_import_visitor'; -import {addToImport, removeFromImport} from './move-import'; +import {COMMON_IMPORT, DOCUMENT_TOKEN_NAME, DocumentImportVisitor, ResolvedDocumentImport} from './document_import_visitor'; +import {addToImport, createImport, removeFromImport} from './move-import'; /** Entry point for the V8 move-document migration. */ @@ -65,17 +65,19 @@ function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: st // the source files if needed. Note that we need to update multiple queries // within a source file within the same recorder in order to not throw off // the TypeScript node offsets. - importsMap.forEach((imports: Imports, sourceFile: ts.SourceFile) => { - const {platformBrowserImport, commonImport, documentElement, replaceText} = imports; - if (!documentElement || !replaceText || !platformBrowserImport) { + importsMap.forEach((resolvedImport: ResolvedDocumentImport, sourceFile: ts.SourceFile) => { + const {platformBrowserImport, commonImport, documentElement} = resolvedImport; + if (!documentElement || !platformBrowserImport) { return; } const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); const platformBrowserDeclaration = platformBrowserImport.parent.parent; - const newPlatformBrowserText = removeFromImport(platformBrowserImport, DOCUMENT_TOKEN_NAME); + const newPlatformBrowserText = removeFromImport(platformBrowserImport, sourceFile, DOCUMENT_TOKEN_NAME); const newCommonText = - commonImport ? addToImport(commonImport, DOCUMENT_TOKEN_NAME) : NEW_COMMON_TEXT; + commonImport ? addToImport(commonImport, sourceFile, + documentElement.name, documentElement.propertyName) : + createImport(COMMON_IMPORT, sourceFile, documentElement.name, documentElement.propertyName); // Replace the existing query decorator call expression with the updated // call expression node. @@ -93,5 +95,3 @@ function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: st tree.commitUpdate(update); }); } - -const NEW_COMMON_TEXT = `\nimport {DOCUMENT} from '@angular/common';`; diff --git a/packages/core/schematics/migrations/move-document/move-import.ts b/packages/core/schematics/migrations/move-document/move-import.ts index f10f14ff5630c..f745122adbdae 100644 --- a/packages/core/schematics/migrations/move-document/move-import.ts +++ b/packages/core/schematics/migrations/move-document/move-import.ts @@ -7,25 +7,48 @@ */ import * as ts from 'typescript'; -export function removeFromImport(importNode: ts.NamedImports, ...keys: string[]): string { - const elements = importNode.elements; - const getName = (el: ts.ImportSpecifier) => el.propertyName ? - `${el.propertyName.escapedText} as ${el.name.escapedText}` : - String(el.name.escapedText); - const elementsMap = elements.map(getName).filter(el => keys.indexOf(el) === -1); - const importDeclaration = importNode.parent.parent; - return elementsMap.length > 0 ? - `import { ${elementsMap.join(', ')} } from ${importDeclaration.moduleSpecifier.getText()};` : - ''; +export function removeFromImport(importNode: ts.NamedImports, sourceFile: ts.SourceFile, importName: string): string { + const printer = ts.createPrinter(); + const elements = importNode.elements.filter(el => String((el.propertyName || el.name).escapedText) !== importName); + + if (!elements.length) { + return ''; + } + + const oldDeclaration = importNode.parent.parent; + const newImport = ts.createNamedImports(elements); + const importClause = ts.createImportClause(undefined, newImport); + const newDeclaration = ts.createImportDeclaration(undefined, undefined, importClause, oldDeclaration.moduleSpecifier); + + return printer.printNode(ts.EmitHint.Unspecified, newDeclaration, sourceFile); } -export function addToImport(importNode: ts.NamedImports, ...keys: string[]): string { - const elements = importNode.elements; - const getName = (el: ts.ImportSpecifier) => el.propertyName ? - `${el.propertyName.escapedText} as ${el.name.escapedText}` : - el.name.escapedText + ''; - const elementsMap = elements.map(getName); - elementsMap.push(...keys); - const importDeclaration = importNode.parent.parent; - return `import { ${elementsMap.join(', ')} } from ${importDeclaration.moduleSpecifier.getText()};`; +export function addToImport(importNode: ts.NamedImports, sourceFile: ts.SourceFile, name: ts.Identifier, propertyName?: ts.Identifier): string { + const printer = ts.createPrinter(); + const propertyNameIdentifier = propertyName ? ts.createIdentifier(String(propertyName.escapedText)) : undefined; + const nameIdentifier = ts.createIdentifier(String(name.escapedText)); + const newSpecfier = ts.createImportSpecifier(propertyNameIdentifier, nameIdentifier); + const elements = [...importNode.elements]; + + elements.push(newSpecfier); + + const oldDeclaration = importNode.parent.parent; + const newImport = ts.createNamedImports(elements); + const importClause = ts.createImportClause(undefined, newImport); + const newDeclaration = ts.createImportDeclaration(undefined, undefined, importClause, oldDeclaration.moduleSpecifier); + + return printer.printNode(ts.EmitHint.Unspecified, newDeclaration, sourceFile); +} + +export function createImport(importSource: string, sourceFile: ts.SourceFile, name: ts.Identifier, propertyName?: ts.Identifier) { + const printer = ts.createPrinter(); + const propertyNameIdentifier = propertyName ? ts.createIdentifier(String(propertyName.escapedText)) : undefined; + const nameIdentifier = ts.createIdentifier(String(name.escapedText)); + const newSpecfier = ts.createImportSpecifier(propertyNameIdentifier, nameIdentifier); + const newNamedImports = ts.createNamedImports([newSpecfier]); + const importClause = ts.createImportClause(undefined, newNamedImports); + const moduleSpecifier = ts.createStringLiteral(importSource); + const newImport = ts.createImportDeclaration(undefined, undefined, importClause, moduleSpecifier); + + return printer.printNode(ts.EmitHint.Unspecified, newImport, sourceFile); } diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index 46dc16f3f1dc5..9397cdb324cf8 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/packages/core/schematics/test/BUILD.bazel @@ -9,7 +9,6 @@ ts_library( ], deps = [ "//packages/core/schematics/migrations/move-document", - "//packages/core/schematics/migrations/move-document/google3", "//packages/core/schematics/migrations/static-queries", "//packages/core/schematics/migrations/static-queries/google3", "//packages/core/schematics/migrations/template-var-assignment", diff --git a/packages/core/schematics/test/google3/move_document_rule_spec.ts b/packages/core/schematics/test/google3/move_document_rule_spec.ts deleted file mode 100644 index 927c03cea75ab..0000000000000 --- a/packages/core/schematics/test/google3/move_document_rule_spec.ts +++ /dev/null @@ -1,154 +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 {readFileSync, writeFileSync} from 'fs'; -import {dirname, join} from 'path'; -import * as shx from 'shelljs'; -import {Configuration, Linter} from 'tslint'; - -describe('Google3 moveDocument TSLint rule', () => { - - /** - * Path to the move-document schematic rules directory. The path needs to be resolved through - * the Bazel runfiles, because on Windows runfiles are not symlinked into the working directory. - */ - const rulesDirectory = - dirname(require.resolve('../../migrations/move-document/google3/moveDocumentRule')); - - let tmpDir: string; - - beforeEach(() => { - tmpDir = join(process.env['TEST_TMPDIR'] !, 'google3-test'); - shx.mkdir('-p', tmpDir); - - writeFile('tsconfig.json', JSON.stringify({compilerOptions: {module: 'es2015'}})); - }); - - afterEach(() => shx.rm('-r', tmpDir)); - - /** - * Runs TSLint with the static-query timing TSLint rule. By default the rule fixes - * are automatically applied. - */ - function runTSLint(fix = true) { - const program = Linter.createProgram(join(tmpDir, 'tsconfig.json')); - const linter = new Linter({fix, rulesDirectory: [rulesDirectory]}, program); - const config = Configuration.parseConfigFile( - {rules: {'move-document': true}, linterOptions: {typeCheck: true}}); - - program.getRootFileNames().forEach(fileName => { - linter.lint(fileName, program.getSourceFile(fileName) !.getFullText(), config); - }); - - return linter; - } - - /** Writes a file to the current temporary directory. */ - function writeFile(fileName: string, content: string) { - writeFileSync(join(tmpDir, fileName), content); - } - - /** Expects a given file in the temporary directory to contain the specified string. */ - function expectFileToContain(fileName: string, match: string) { - expect(readFileSync(join(tmpDir, fileName), 'utf8')).toContain(match); - } - - /** Expects a given file in the temporary directory not to contain the specified string. */ - function expectFileNotToContain(fileName: string, match: string) { - expect(readFileSync(join(tmpDir, fileName), 'utf8')).not.toContain(match); - } - - it('should properly apply import replacement', () => { - writeFile('index.ts', ` - import {DOCUMENT} from '@angular/platform-browser'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import {DOCUMENT} from '@angular/common';`); - expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should report incorrect imports', () => { - writeFile('index.ts', ` - import {DOCUMENT} from '@angular/platform-browser'; - `); - - const linter = runTSLint(false); - const failures = linter.getResult().failures; - - expect(failures.length).toBe(1); - expect(failures[0].getFailure()).toMatch(/DOCUMENT is no longer exported.*/); - }); - - it('should properly apply import replacement with existing import', () => { - writeFile('index.ts', ` - import {DOCUMENT} from '@angular/platform-browser'; - import {someImport} from '@angular/common'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`); - expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should properly apply import replacement with existing import (reverse)', () => { - writeFile('index.ts', ` - import {someImport} from '@angular/common'; - import {DOCUMENT} from '@angular/platform-browser'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`); - expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should properly apply import replacement with existing import w/ comments', () => { - writeFile('index.ts', ` - /** - * this is a comment - */ - import {DOCUMENT} from '@angular/platform-browser'; - import {someImport} from '@angular/common'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`); - expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should properly apply import replacement with existing and redundant imports', () => { - writeFile('index.ts', ` - import {DOCUMENT} from '@angular/platform-browser'; - import {anotherImport} from '@angular/platform-browser-dynamic'; - import {someImport} from '@angular/common'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`); - expectFileNotToContain('index.ts', `import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should properly apply import replacement with existing import and leave original import', - () => { - writeFile('index.ts', ` - import {DOCUMENT, anotherImport} from '@angular/platform-browser'; - import {someImport} from '@angular/common'; - `); - - runTSLint(); - - expectFileToContain('index.ts', `import { someImport, DOCUMENT } from '@angular/common';`); - expectFileToContain( - 'index.ts', `import { anotherImport } from '@angular/platform-browser';`); - }); -}); diff --git a/packages/core/schematics/test/move_document_migration_spec.ts b/packages/core/schematics/test/move_document_migration_spec.ts index 36ace16263272..2e91ba016dc9b 100644 --- a/packages/core/schematics/test/move_document_migration_spec.ts +++ b/packages/core/schematics/test/move_document_migration_spec.ts @@ -53,7 +53,7 @@ describe('move-document migration', () => { const content = tree.readContent('/index.ts'); - expect(content).toContain(`import {DOCUMENT} from '@angular/common';`); + expect(content).toContain(`import { DOCUMENT } from "@angular/common";`); expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); }); @@ -63,16 +63,7 @@ describe('move-document migration', () => { import {someImport} from '@angular/common'; `); - runMigration(); - - const content = tree.readContent('/index.ts'); - - expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); - expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); - }); - - it('should properly apply import replacement with existing import (reverse)', () => { - writeFile('/index.ts', ` + writeFile('/reverse.ts', ` import {someImport} from '@angular/common'; import {DOCUMENT} from '@angular/platform-browser'; `); @@ -80,9 +71,13 @@ describe('move-document migration', () => { runMigration(); const content = tree.readContent('/index.ts'); + const contentReverse = tree.readContent('/reverse.ts'); expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); + + expect(contentReverse).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); + expect(contentReverse).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); }); it('should properly apply import replacement with existing import w/ comments', () => { @@ -100,6 +95,8 @@ describe('move-document migration', () => { expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); + + expect(content).toMatch(/.*this is a comment.*/); }); it('should properly apply import replacement with existing and redundant imports', () => { @@ -124,13 +121,28 @@ describe('move-document migration', () => { import {someImport} from '@angular/common'; `); - runMigration(); + runMigration(); - const content = tree.readContent('/index.ts'); + const content = tree.readContent('/index.ts'); - expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); - expect(content).toContain(`import { anotherImport } from '@angular/platform-browser';`); - }); + expect(content).toContain(`import { someImport, DOCUMENT } from '@angular/common';`); + expect(content).toContain(`import { anotherImport } from '@angular/platform-browser';`); + }); + + it('should properly apply import replacement with existing import and alias', + () => { + writeFile('/index.ts', ` + import {DOCUMENT as doc, anotherImport} from '@angular/platform-browser'; + import {someImport} from '@angular/common'; + `); + + runMigration(); + + const content = tree.readContent('/index.ts'); + + expect(content).toContain(`import { someImport, DOCUMENT as doc } from '@angular/common';`); + expect(content).toContain(`import { anotherImport } from '@angular/platform-browser';`); + }); }); function writeFile(filePath: string, contents: string) {