feat(ivy): add injectable-pipe schematic (#29847)

Adds a schematic that will annotate all `Pipe` classes with `Injectable` so that they can be injected.

This PR resolves FW-1228.

PR Close #29847

packages/core/schematics/BUILD.bazel

@@ -10,6 +10,7 @@ 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/static-queries",
         "//packages/core/schematics/migrations/template-var-assignment",

packages/core/schematics/migrations.json

@@ -14,6 +14,11 @@
       "version": "8-beta",
       "description": "Warns developers if values are assigned to template variables",
       "factory": "./migrations/template-var-assignment/index"
+    },
+    "migration-v8-injectable-pipe": {
+      "version": "8-beta",
+      "description": "Migrates all Pipe classes so that they have an Injectable annotation",
+      "factory": "./migrations/injectable-pipe/index"
     }
   }
 }

packages/core/schematics/migrations/injectable-pipe/BUILD.bazel

@@ -0,0 +1,18 @@
+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/injectable-pipe/google3:__pkg__",
+        "//packages/core/schematics/test:__pkg__",
+    ],
+    deps = [
+        "//packages/core/schematics/utils",
+        "@npm//@angular-devkit/schematics",
+        "@npm//@types/node",
+        "@npm//typescript",
+    ],
+)

packages/core/schematics/migrations/injectable-pipe/README.md

@@ -0,0 +1,22 @@
+## 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 {}
+```

packages/core/schematics/migrations/injectable-pipe/angular/injectable_pipe_visitor.ts

@@ -0,0 +1,67 @@
+/**
+ * @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
+      });
+    }
+  }
+}

packages/core/schematics/migrations/injectable-pipe/google3/BUILD.bazel

@@ -0,0 +1,12 @@
+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/injectable-pipe",
+        "@npm//tslint",
+    ],
+)

packages/core/schematics/migrations/injectable-pipe/google3/injectablePipeRule.ts

@@ -0,0 +1,55 @@
+/**
+ * @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 '../angular/injectable_pipe_visitor';
+import {INJECTABLE_DECORATOR_NAME, addNamedImport, getNamedImports} from '../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,
+                  addNamedImport(importDeclarationMissingImport, 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;
+  }
+}

packages/core/schematics/migrations/injectable-pipe/index.ts

@@ -0,0 +1,89 @@
+/**
+ * @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, addNamedImport, 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));
+    return buffer ? buffer.toString() : undefined;
+  };
+
+  const program = ts.createProgram(parsed.fileNames, parsed.options, host);
+  const typeChecker = program.getTypeChecker();
+  const visitor = new InjectablePipeVisitor(typeChecker);
+  const rootSourceFiles = program.getRootFileNames().map(f => program.getSourceFile(f) !);
+  const printer = ts.createPrinter();
+
+  rootSourceFiles.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,
+                addNamedImport(importDeclarationMissingImport, INJECTABLE_DECORATOR_NAME),
+                sourceFile));
+      }
+    }
+
+    tree.commitUpdate(update);
+  });
+}

packages/core/schematics/migrations/injectable-pipe/util.ts

@@ -0,0 +1,47 @@
+/**
+ * @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 a named import to an import declaration node.
+ * @param node Node to which to add the import.
+ * @param importName Name of the import that should be added.
+ */
+export function addNamedImport(node: ts.ImportDeclaration, importName: string) {
+  const namedImports = getNamedImports(node);
+
+  if (namedImports && ts.isNamedImports(namedImports)) {
+    const elements = namedImports.elements;
+    const isAlreadyImported = elements.some(element => element.name.text === importName);
+
+    if (!isAlreadyImported) {
+      // If there are named imports, there will be an import clause as well.
+      const importClause = node.importClause !;
+      const newImportClause = ts.createNamedImports(
+          [...elements, ts.createImportSpecifier(undefined, ts.createIdentifier(importName))]);
+
+      return ts.updateImportDeclaration(
+          node, node.decorators, node.modifiers,
+          ts.updateImportClause(importClause, importClause.name, newImportClause),
+          node.moduleSpecifier);
+    }
+  }
+
+  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;
+}

packages/core/schematics/test/BUILD.bazel

@@ -8,6 +8,8 @@ ts_library(
         "//packages/core/schematics:migrations.json",
     ],
     deps = [
+        "//packages/core/schematics/migrations/injectable-pipe",
+        "//packages/core/schematics/migrations/injectable-pipe/google3",
         "//packages/core/schematics/migrations/move-document",
         "//packages/core/schematics/migrations/static-queries",
         "//packages/core/schematics/migrations/static-queries/google3",

packages/core/schematics/test/google3/injectable_pipe_spec.ts

@@ -0,0 +1,91 @@
+/**
+ * @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/injectable-pipe/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();
+    expect(getFile('/index.ts')).toContain('import { Pipe, Injectable } from \'@angular/core\'');
+  });
+
+});

packages/core/schematics/test/injectable_pipe_migration_spec.ts

@@ -0,0 +1,125 @@
+/**
+ * @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('../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', () => {
+    writeFile('/index.ts', `
+      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', () => {
+    writeFile('/index.ts', `
+      import { Pipe } from '@angular/core';
+
+      @Pipe()
+      export class MyPipe {
+      }
+    `);
+
+    runMigration();
+    expect(tree.readContent('/index.ts'))
+        .toContain('import { Pipe, Injectable } from \'@angular/core\'');
+  });
+
+  it('should not add an import for Injectable if it is imported already', () => {
+    writeFile('/index.ts', `
+      import { Pipe, Injectable, NgModule } from '@angular/core';
+
+      @Pipe()
+      export class MyPipe {
+      }
+    `);
+
+    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', () => {
+    const source = `
+      import { Injectable, Pipe } from '@angular/core';
+
+      @Injectable()
+      @Pipe()
+      export class MyPipe {
+      }
+    `;
+
+    writeFile('/index.ts', source);
+    runMigration();
+    expect(tree.readContent('/index.ts')).toBe(source);
+  });
+
+  it('should not add @Injectable if @Pipe was not imported from @angular/core', () => {
+    const source = `
+      import { Pipe } from '@not-angular/core';
+
+      @Pipe()
+      export class MyPipe {
+      }
+    `;
+
+    writeFile('/index.ts', source);
+    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.runSchematic('migration-v8-injectable-pipe', {}, tree); }
+});

packages/core/schematics/utils/ng_decorators.ts

@@ -16,6 +16,7 @@ export type CallExpressionDecorator = ts.Decorator & {
 export interface NgDecorator {
   name: string;
   node: CallExpressionDecorator;
+  importNode: ts.ImportDeclaration;
 }
 
 /**
@@ -26,7 +27,9 @@ export function getAngularDecorators(
     typeChecker: ts.TypeChecker, decorators: ReadonlyArray<ts.Decorator>): NgDecorator[] {
   return decorators.map(node => ({node, importData: getCallDecoratorImport(typeChecker, node)}))
       .filter(({importData}) => importData && importData.importModule.startsWith('@angular/'))
-      .map(
+      .map(({node, importData}) => ({
-          ({node, importData}) =>
+             node: node as CallExpressionDecorator,
-              ({node: node as CallExpressionDecorator, name: importData !.name}));
+             name: importData !.name,
+             importNode: importData !.node
+           }));
 }

packages/core/schematics/utils/tslint/BUILD.bazel

@@ -4,5 +4,7 @@ ts_library(
     name = "tslint",
     srcs = glob(["**/*.ts"]),
     tsconfig = "//packages/core/schematics:tsconfig.json",
-    visibility = ["//packages/core/schematics/migrations/template-var-assignment/google3:__pkg__"],
+    visibility = [
+        "//packages/core/schematics/migrations/template-var-assignment/google3:__pkg__",
+    ],
 )

packages/core/schematics/utils/typescript/imports.ts

@@ -10,7 +10,8 @@ import * as ts from 'typescript';
 
 export type Import = {
   name: string,
-  importModule: string
+  importModule: string,
+  node: ts.ImportDeclaration
 };
 
 /** Gets import information about the specified identifier by using the Type checker. */
@@ -37,6 +38,7 @@ export function getImportOfIdentifier(typeChecker: ts.TypeChecker, node: ts.Iden
   return {
     // Handles aliased imports: e.g. "import {Component as myComp} from ...";
     name: decl.propertyName ? decl.propertyName.text : decl.name.text,
-    importModule: importDecl.moduleSpecifier.text
+    importModule: importDecl.moduleSpecifier.text,
+    node: importDecl
   };
 }