diff --git a/packages/core/schematics/BUILD.bazel b/packages/core/schematics/BUILD.bazel index 6fc202dd3a..ee525a3dbd 100644 --- a/packages/core/schematics/BUILD.bazel +++ b/packages/core/schematics/BUILD.bazel @@ -10,7 +10,6 @@ npm_package( srcs = ["migrations.json"], visibility = ["//packages/core:__pkg__"], deps = [ - "//packages/core/schematics/migrations/injectable-pipe", "//packages/core/schematics/migrations/move-document", "//packages/core/schematics/migrations/renderer-to-renderer2", "//packages/core/schematics/migrations/static-queries", diff --git a/packages/core/schematics/migrations/google3/BUILD.bazel b/packages/core/schematics/migrations/google3/BUILD.bazel index e749de292e..faead2cddb 100644 --- a/packages/core/schematics/migrations/google3/BUILD.bazel +++ b/packages/core/schematics/migrations/google3/BUILD.bazel @@ -6,7 +6,6 @@ ts_library( tsconfig = "//packages/core/schematics:tsconfig.json", visibility = ["//packages/core/schematics/test/google3:__pkg__"], deps = [ - "//packages/core/schematics/migrations/injectable-pipe", "//packages/core/schematics/migrations/missing-injectable", "//packages/core/schematics/migrations/missing-injectable/google3", "//packages/core/schematics/migrations/renderer-to-renderer2", diff --git a/packages/core/schematics/migrations/google3/injectablePipeRule.ts b/packages/core/schematics/migrations/google3/injectablePipeRule.ts deleted file mode 100644 index 423c26bcba..0000000000 --- a/packages/core/schematics/migrations/google3/injectablePipeRule.ts +++ /dev/null @@ -1,55 +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 {InjectablePipeVisitor} from '../injectable-pipe/angular/injectable_pipe_visitor'; -import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from '../injectable-pipe/util'; - - -/** - * TSLint rule that flags `@Pipe` classes that haven't been marked as `@Injectable`. - */ -export class Rule extends Rules.TypedRule { - applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { - const visitor = new InjectablePipeVisitor(program.getTypeChecker()); - const printer = ts.createPrinter(); - const failures: RuleFailure[] = []; - - visitor.visitNode(sourceFile); - - visitor.missingInjectablePipes.forEach(data => { - const {pipeDecorator, importDeclarationMissingImport} = data; - const fixes = [new Replacement( - pipeDecorator.getStart(), pipeDecorator.getWidth(), - `@${INJECTABLE_DECORATOR_NAME}()\n${pipeDecorator.getText()}`)]; - - if (importDeclarationMissingImport) { - const namedImports = getNamedImports(importDeclarationMissingImport); - - // Add another fix that'll add the missing import. - if (namedImports) { - fixes.push(new Replacement( - namedImports.getStart(), namedImports.getWidth(), - printer.printNode( - ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME), - sourceFile))); - } - } - - // Add a failure on Pipe decorators that are missing the Injectable decorator. - failures.push(new RuleFailure( - sourceFile, pipeDecorator.getStart(), pipeDecorator.getWidth(), - 'Classes with @Pipe should be decorated with @Injectable so that they can be injected.', - this.ruleName, fixes)); - }); - - return failures; - } -} diff --git a/packages/core/schematics/migrations/injectable-pipe/BUILD.bazel b/packages/core/schematics/migrations/injectable-pipe/BUILD.bazel deleted file mode 100644 index 1f553e1f00..0000000000 --- a/packages/core/schematics/migrations/injectable-pipe/BUILD.bazel +++ /dev/null @@ -1,18 +0,0 @@ -load("//tools:defaults.bzl", "ts_library") - -ts_library( - name = "injectable-pipe", - srcs = glob(["**/*.ts"]), - tsconfig = "//packages/core/schematics:tsconfig.json", - visibility = [ - "//packages/core/schematics:__pkg__", - "//packages/core/schematics/migrations/google3:__pkg__", - "//packages/core/schematics/test:__pkg__", - ], - deps = [ - "//packages/core/schematics/utils", - "@npm//@angular-devkit/schematics", - "@npm//@types/node", - "@npm//typescript", - ], -) diff --git a/packages/core/schematics/migrations/injectable-pipe/README.md b/packages/core/schematics/migrations/injectable-pipe/README.md deleted file mode 100644 index 95d4d0c552..0000000000 --- a/packages/core/schematics/migrations/injectable-pipe/README.md +++ /dev/null @@ -1,22 +0,0 @@ -## Injectable annotation on pipes - -In ViewEngine it was possible to inject a class that was annotated as a `Pipe`, however this no -longer works in Ivy if the class also doesn't have the `Injectable` decorator. This migration -adds `Injectable` automatically to all `Pipe` classes. - -### Before -```ts -import { Pipe } from '@angular/core'; - -@Pipe({ name: 'myPipe' }) -class MyPipe {} -``` - -### After -```ts -import { Pipe, Injectable } from '@angular/core'; - -@Injectable() -@Pipe({ name: 'myPipe' }) -class MyPipe {} -``` diff --git a/packages/core/schematics/migrations/injectable-pipe/angular/injectable_pipe_visitor.ts b/packages/core/schematics/migrations/injectable-pipe/angular/injectable_pipe_visitor.ts deleted file mode 100644 index fdf10dc7d7..0000000000 --- a/packages/core/schematics/migrations/injectable-pipe/angular/injectable_pipe_visitor.ts +++ /dev/null @@ -1,67 +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 * as ts from 'typescript'; - -import {getAngularDecorators} from '../../../utils/ng_decorators'; -import {INJECTABLE_DECORATOR_NAME} from '../util'; - -/** - * Goes through all of the descendant nodes of a given node and lists out all of the pipes - * that don't have `@Injectable`, as well as their `@Pipe` decorator and the import declaration - * from which we'd need to import the `Injectable` decorator. - */ -export class InjectablePipeVisitor { - /** - * Keeps track of all the classes that have a `Pipe` decorator, but not `Injectable`, as well - * as a reference to the `Pipe` decorator itself and import declarations from which we'll have - * to import the `Injectable` decorator. - */ - missingInjectablePipes: { - classDeclaration: ts.ClassDeclaration, - importDeclarationMissingImport: ts.ImportDeclaration|null, - pipeDecorator: ts.Decorator - }[] = []; - - constructor(private _typeChecker: ts.TypeChecker) {} - - visitNode(node: ts.Node) { - switch (node.kind) { - case ts.SyntaxKind.ClassDeclaration: - this._visitClassDeclaration(node as ts.ClassDeclaration); - break; - } - - ts.forEachChild(node, node => this.visitNode(node)); - } - - private _visitClassDeclaration(node: ts.ClassDeclaration) { - if (!node.decorators || !node.decorators.length) { - return; - } - - const ngDecorators = getAngularDecorators(this._typeChecker, node.decorators); - const pipeDecorator = ngDecorators.find(decorator => decorator.name === 'Pipe'); - const hasInjectableDecorator = - !ngDecorators.some(decorator => decorator.name === INJECTABLE_DECORATOR_NAME); - - // Skip non-pipe classes and pipes that are already marked as injectable. - if (pipeDecorator && hasInjectableDecorator) { - const importNode = pipeDecorator.importNode; - const namedImports = importNode.importClause && importNode.importClause.namedBindings; - const needsImport = namedImports && ts.isNamedImports(namedImports) && - !namedImports.elements.some(element => element.name.text === INJECTABLE_DECORATOR_NAME); - - this.missingInjectablePipes.push({ - classDeclaration: node, - importDeclarationMissingImport: needsImport ? importNode : null, - pipeDecorator: pipeDecorator.node - }); - } - } -} diff --git a/packages/core/schematics/migrations/injectable-pipe/index.ts b/packages/core/schematics/migrations/injectable-pipe/index.ts deleted file mode 100644 index 51c088677b..0000000000 --- a/packages/core/schematics/migrations/injectable-pipe/index.ts +++ /dev/null @@ -1,92 +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 {Rule, SchematicsException, Tree} from '@angular-devkit/schematics'; -import {dirname, relative} from 'path'; -import * as ts from 'typescript'; - -import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; -import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; - -import {InjectablePipeVisitor} from './angular/injectable_pipe_visitor'; -import {INJECTABLE_DECORATOR_NAME, addImport, getNamedImports} from './util'; - -/** - * Runs a migration over a TypeScript project that adds an `@Injectable` - * annotation to all classes that have `@Pipe`. - */ -export default function(): Rule { - return (tree: Tree) => { - const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); - const basePath = process.cwd(); - const allPaths = [...buildPaths, ...testPaths]; - - if (!allPaths.length) { - throw new SchematicsException( - 'Could not find any tsconfig file. Cannot add Injectable annotation to pipes.'); - } - - for (const tsconfigPath of allPaths) { - runInjectablePipeMigration(tree, tsconfigPath, basePath); - } - }; -} - -function runInjectablePipeMigration(tree: Tree, tsconfigPath: string, basePath: string) { - const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath)); - const host = ts.createCompilerHost(parsed.options, true); - - // We need to overwrite the host "readFile" method, as we want the TypeScript - // program to be based on the file contents in the virtual file tree. Otherwise - // if we run the migration for multiple tsconfig files which have intersecting - // source files, it can end up updating query definitions multiple times. - host.readFile = fileName => { - const buffer = tree.read(relative(basePath, fileName)); - // Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which - // which breaks the CLI UpdateRecorder. - // See: https://github.com/angular/angular/pull/30719 - return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined; - }; - - const program = ts.createProgram(parsed.fileNames, parsed.options, host); - const typeChecker = program.getTypeChecker(); - const visitor = new InjectablePipeVisitor(typeChecker); - const sourceFiles = program.getSourceFiles().filter( - f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f)); - const printer = ts.createPrinter(); - - sourceFiles.forEach(sourceFile => visitor.visitNode(sourceFile)); - - visitor.missingInjectablePipes.forEach(data => { - const {classDeclaration, importDeclarationMissingImport} = data; - const sourceFile = classDeclaration.getSourceFile(); - const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); - - // Note that we don't need to go through the AST to insert the decorator, because the change - // is pretty basic. Also this has a better chance of preserving the user's formatting. - update.insertLeft(classDeclaration.getStart(), `@${INJECTABLE_DECORATOR_NAME}()\n`); - - // Add @Injectable to the imports if it isn't imported already. Note that this doesn't deal with - // the case where there aren't any imports for `@angular/core` at all. We don't need to handle - // it because the Pipe decorator won't be recognized if it hasn't been imported from Angular. - if (importDeclarationMissingImport) { - const namedImports = getNamedImports(importDeclarationMissingImport); - - if (namedImports) { - update.remove(namedImports.getStart(), namedImports.getWidth()); - update.insertRight( - namedImports.getStart(), - printer.printNode( - ts.EmitHint.Unspecified, addImport(namedImports, INJECTABLE_DECORATOR_NAME), - sourceFile)); - } - } - - tree.commitUpdate(update); - }); -} diff --git a/packages/core/schematics/migrations/injectable-pipe/util.ts b/packages/core/schematics/migrations/injectable-pipe/util.ts deleted file mode 100644 index fd2de7b943..0000000000 --- a/packages/core/schematics/migrations/injectable-pipe/util.ts +++ /dev/null @@ -1,36 +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 * as ts from 'typescript'; - -/** Name of the Injectable decorator. */ -export const INJECTABLE_DECORATOR_NAME = 'Injectable'; - -/** - * Adds an import to a named import node, if the import does not exist already. - * @param node Node to which to add the import. - * @param importName Name of the import that should be added. - */ -export function addImport(node: ts.NamedImports, importName: string) { - const elements = node.elements; - const isAlreadyImported = elements.some(element => element.name.text === importName); - - if (!isAlreadyImported) { - return ts.updateNamedImports( - node, [...elements, ts.createImportSpecifier(undefined, ts.createIdentifier(importName))]); - } - - return node; -} - -/** Gets the named imports node from an import declaration. */ -export function getNamedImports(node: ts.ImportDeclaration): ts.NamedImports|null { - const importClause = node.importClause; - const namedImports = importClause && importClause.namedBindings; - return (namedImports && ts.isNamedImports(namedImports)) ? namedImports : null; -} diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index 3c5ad405f6..eecd8449ce 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/packages/core/schematics/test/BUILD.bazel @@ -9,7 +9,6 @@ ts_library( "//packages/core/schematics:migrations.json", ], deps = [ - "//packages/core/schematics/migrations/injectable-pipe", "//packages/core/schematics/migrations/missing-injectable", "//packages/core/schematics/migrations/move-document", "//packages/core/schematics/migrations/renderer-to-renderer2", diff --git a/packages/core/schematics/test/google3/injectable_pipe_spec.ts b/packages/core/schematics/test/google3/injectable_pipe_spec.ts deleted file mode 100644 index 2bf83a0937..0000000000 --- a/packages/core/schematics/test/google3/injectable_pipe_spec.ts +++ /dev/null @@ -1,93 +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 injectable pipe TSLint rule', () => { - const rulesDirectory = dirname(require.resolve('../../migrations/google3/injectablePipeRule')); - - 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)); - - 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: {'injectable-pipe': true}, linterOptions: {typeCheck: true}}); - - program.getRootFileNames().forEach(fileName => { - linter.lint(fileName, program.getSourceFile(fileName) !.getFullText(), config); - }); - - return linter; - } - - function writeFile(fileName: string, content: string) { - writeFileSync(join(tmpDir, fileName), content); - } - - function getFile(fileName: string) { return readFileSync(join(tmpDir, fileName), 'utf8'); } - - it('should report pipes that are not marked as Injectable', () => { - writeFile('index.ts', ` - import { Pipe } from '@angular/core'; - - @Pipe({ name: 'myPipe' }) - export class MyPipe { - } - `); - - const linter = runTSLint(false); - const failures = linter.getResult().failures; - - expect(failures.length).toBe(1); - expect(failures[0].getFailure()).toMatch(/@Pipe should be decorated with @Injectable/); - }); - - it('should add @Injectable to pipes that do not have it', () => { - writeFile('/index.ts', ` - import { Pipe } from '@angular/core'; - - @Pipe({ name: 'myPipe' }) - export class MyPipe { - } - `); - - runTSLint(); - expect(getFile('/index.ts')) - .toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/); - }); - - it('should add an import for Injectable to the @angular/core import declaration', () => { - writeFile('/index.ts', ` - import { Pipe } from '@angular/core'; - - @Pipe() - export class MyPipe { - } - `); - - runTSLint(); - - const content = getFile('/index.ts'); - expect(content).toContain('import { Pipe, Injectable } from \'@angular/core\''); - expect((content.match(/import/g) || []).length).toBe(1, 'Expected only one import statement'); - }); - -}); diff --git a/packages/core/schematics/test/injectable_pipe_migration_spec.ts b/packages/core/schematics/test/injectable_pipe_migration_spec.ts deleted file mode 100644 index 4cdf8c7918..0000000000 --- a/packages/core/schematics/test/injectable_pipe_migration_spec.ts +++ /dev/null @@ -1,143 +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 {getSystemPath, normalize, virtualFs} from '@angular-devkit/core'; -import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing'; -import {HostTree} from '@angular-devkit/schematics'; -import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing'; -import * as shx from 'shelljs'; - -describe('injectable pipe migration', () => { - let runner: SchematicTestRunner; - let host: TempScopedNodeJsSyncHost; - let tree: UnitTestTree; - let tmpDirPath: string; - let previousWorkingDir: string; - - beforeEach(() => { - runner = new SchematicTestRunner('test', require.resolve('./test-migrations.json')); - host = new TempScopedNodeJsSyncHost(); - tree = new UnitTestTree(new HostTree(host)); - - writeFile('/tsconfig.json', JSON.stringify({ - compilerOptions: { - lib: ['es2015'], - } - })); - writeFile('/angular.json', JSON.stringify({ - projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}} - })); - - previousWorkingDir = shx.pwd(); - tmpDirPath = getSystemPath(host.root); - - // Switch into the temporary directory path. This allows us to run - // the schematic against our custom unit test tree. - shx.cd(tmpDirPath); - }); - - afterEach(() => { - shx.cd(previousWorkingDir); - shx.rm('-r', tmpDirPath); - }); - - it('should add @Injectable to pipes that do not have it', async() => { - writeFile('/index.ts', ` - import { Pipe } from '@angular/core'; - - @Pipe({ name: 'myPipe' }) - export class MyPipe { - } - `); - - await runMigration(); - expect(tree.readContent('/index.ts')) - .toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/); - }); - - it('should add @Injectable to pipes that do not have it (BOM)', () => { - writeFile('/index.ts', `\uFEFF - import { Pipe } from '@angular/core'; - - @Pipe({ name: 'myPipe' }) - export class MyPipe { - } - `); - - runMigration(); - expect(tree.readContent('/index.ts')) - .toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/); - }); - - it('should add an import for Injectable to the @angular/core import declaration', async() => { - writeFile('/index.ts', ` - import { Pipe } from '@angular/core'; - - @Pipe() - export class MyPipe { - } - `); - - await runMigration(); - - const content = tree.readContent('/index.ts'); - expect(content).toContain('import { Pipe, Injectable } from \'@angular/core\''); - expect((content.match(/import/g) || []).length).toBe(1, 'Expected only one import statement'); - }); - - it('should not add an import for Injectable if it is imported already', async() => { - writeFile('/index.ts', ` - import { Pipe, Injectable, NgModule } from '@angular/core'; - - @Pipe() - export class MyPipe { - } - `); - - await runMigration(); - expect(tree.readContent('/index.ts')) - .toContain('import { Pipe, Injectable, NgModule } from \'@angular/core\''); - }); - - it('should do nothing if the pipe is marked as injectable already', async() => { - const source = ` - import { Injectable, Pipe } from '@angular/core'; - - @Injectable() - @Pipe() - export class MyPipe { - } - `; - - writeFile('/index.ts', source); - await runMigration(); - expect(tree.readContent('/index.ts')).toBe(source); - }); - - it('should not add @Injectable if @Pipe was not imported from @angular/core', async() => { - const source = ` - import { Pipe } from '@not-angular/core'; - - @Pipe() - export class MyPipe { - } - `; - - writeFile('/index.ts', source); - await runMigration(); - expect(tree.readContent('/index.ts')).toBe(source); - }); - - function writeFile(filePath: string, contents: string) { - host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); - } - - function runMigration() { - runner.runSchematicAsync('migration-injectable-pipe', {}, tree).toPromise(); - } -}); diff --git a/packages/core/schematics/test/test-migrations.json b/packages/core/schematics/test/test-migrations.json index 59f354d7e3..dca30a068c 100644 --- a/packages/core/schematics/test/test-migrations.json +++ b/packages/core/schematics/test/test-migrations.json @@ -2,10 +2,6 @@ // Migrations which are not publicly enabled but still run as part of tests need to // be part of a schematic collection in order to be able to run it. "schematics": { - "migration-injectable-pipe": { - "description": "Migrates all Pipe classes so that they have an Injectable annotation", - "factory": "../migrations/injectable-pipe/index" - }, "migration-missing-injectable": { "description": "Migrates all declared undecorated providers with the @Injectable decorator", "factory": "../migrations/missing-injectable/index"