Merge pull request ReactiveX#13 from ReactiveX/minko/observable-static

feat(rules): migrate static operations
rx-ts · Apr 19, 2018 · 651b5b1 · 651b5b1
2 parents 04f1290 + c3ada00
commit 651b5b1
Show file tree

Hide file tree

Showing 9 changed files with 287 additions and 72 deletions.
diff --git a/.gitignore b/.gitignore
@@ -5,4 +5,6 @@ dist
 npm-debug.log
 .vscode
 yarn.lock
+demo.ts
+tsconfig-demo.json
 
diff --git a/README.md b/README.md
@@ -6,11 +6,12 @@ TSLint rules for rxjs.
 
 This repository provides the following rules:
 
-|            Rule Name            | Configuration |                       Description                       |
-| :-----------------------------: | :-----------: | :-----------------------------------------------------: |
-|     `collapse-rxjs-imports`     |     none      | Collapses multiple imports from `rxjs` to a single one. |
-| `migrate-to-pipeable-operators` |     none      |      Migrates side-effect operators to pipeables.       |
-|      `update-rxjs-imports`      |     none      |         Updates RxJS 5.x.x imports to RxJS 6.0          |
+|              Rule Name              | Configuration |                       Description                       |
+| :---------------------------------: | :-----------: | :-----------------------------------------------------: |
+|       `collapse-rxjs-imports`       |     none      | Collapses multiple imports from `rxjs` to a single one. |
+|   `migrate-to-pipeable-operators`   |     none      |      Migrates side-effect operators to pipeables.       |
+| `migrate-static-observable-methods` |     none      |        Migrates static `Observable` method calls        |
+|        `update-rxjs-imports`        |     none      |         Updates RxJS 5.x.x imports to RxJS 6.0          |
 
 ## Migration to RxJS 6
 
@@ -30,6 +31,7 @@ npm i rxjs-tslint
   "rules": {
     "update-rxjs-imports": true,
     "migrate-to-pipeable-operators": true,
+    "migrate-static-observable-methods": true,
     "collapse-rxjs-imports": true
   }
 }
@@ -51,4 +53,3 @@ npm i rxjs-tslint
 ## License
 
 MIT
-
diff --git a/migrate-tslint.json b/migrate-tslint.json
@@ -3,6 +3,7 @@
   "rules": {
     "update-rxjs-imports": true,
     "migrate-to-pipeable-operators": true,
+    "migrate-static-observable-methods": true,
     "collapse-rxjs-imports": true
   }
-}
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -7,29 +7,25 @@
     "docs": "ts-node build/buildDocs.ts",
     "lint": "tslint -c tslint.json \"src/**/*.ts\" \"test/**/*.ts\"",
     "lint:fix": "npm run lint -- --fix",
-    "release": "npm run build && rimraf dist && tsc -p tsconfig-release.json && npm run copy:common && npm run prepare:package && BUILD_TYPE=prod npm run set:vars",
+    "release":
+      "npm run build && rimraf dist && tsc -p tsconfig-release.json && npm run copy:common && npm run prepare:package && BUILD_TYPE=prod npm run set:vars",
     "build": "rimraf dist && tsc && npm run lint && npm t",
     "copy:common": "cp README.md dist",
     "prepare:package": "cat package.json | ts-node build/package.ts > dist/package.json",
     "test": "rimraf dist && tsc && mocha -R nyan dist/test --recursive",
-    "test:watch": "rimraf dist && tsc && BUILD_TYPE=dev npm run set:vars && mocha -R nyan dist/test --watch --recursive",
+    "test:watch":
+      "rimraf dist && tsc && BUILD_TYPE=dev npm run set:vars && mocha -R nyan dist/test --watch --recursive",
     "set:vars": "ts-node build/vars.ts --src ./dist",
     "tscv": "tsc --version",
     "tsc": "tsc",
     "tsc:watch": "tsc --w"
   },
-  "contributors": [
-    "Minko Gechev <[email protected]>"
-  ],
+  "contributors": ["Minko Gechev <[email protected]>"],
   "repository": {
     "type": "git",
     "url": "git+https://github.com/mgechev/tslint-rules.git"
   },
-  "keywords": [
-    "rxjs",
-    "lint",
-    "tslint"
-  ],
+  "keywords": ["rxjs", "lint", "tslint"],
   "author": {
     "name": "Minko Gechev",
     "email": "[email protected]"

diff --git a/src/index.ts b/src/index.ts
@@ -1,3 +1,4 @@
 export { Rule as CollapseRxjsImports } from './collapseRxjsImportsRule';
 export { Rule as UpdateRxjsImports } from './updateRxjsImportsRule';
 export { Rule as MigrateToPipeableOperators } from './migrateToPipeableOperatorsRule';
+export { Rule as MigrateStaticObservableMethods } from './migrateStaticObservableMethodsRule';
diff --git a/src/migrateStaticObservableMethodsRule.ts b/src/migrateStaticObservableMethodsRule.ts
@@ -0,0 +1,194 @@
+import * as Lint from 'tslint';
+import * as tsutils from 'tsutils';
+import * as ts from 'typescript';
+import { subtractSets, concatSets, isObservable, returnsObservable, computeInsertionIndexForImports } from './utils';
+
+/**
+ * A typed TSLint rule that inspects observable
+ * static methods and turns them into function calls.
+ */
+export class Rule extends Lint.Rules.TypedRule {
+  static metadata: Lint.IRuleMetadata = {
+    ruleName: 'migrate-static-observable-methods',
+    description: 'Updates the static methods of the Observable class.',
+    optionsDescription: '',
+    options: null,
+    typescriptOnly: true,
+    type: 'functionality'
+  };
+  static IMPORT_FAILURE_STRING = 'prefer operator imports with no side-effects';
+  static OBSERVABLE_FAILURE_STRING = 'prefer function calls';
+
+  applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+    const failure = this.applyWithFunction(sourceFile, ctx => this.walk(ctx, program));
+    return failure;
+  }
+  private walk(ctx: Lint.WalkContext<void>, program: ts.Program) {
+    this.removePatchedOperatorImports(ctx);
+    const sourceFile = ctx.sourceFile;
+    const typeChecker = program.getTypeChecker();
+    const insertionStart = computeInsertionIndexForImports(sourceFile);
+    let rxjsOperatorImports = new Set<OperatorWithAlias>(
+      Array.from(findImportedRxjsOperators(sourceFile)).map(o => OPERATOR_WITH_ALIAS_MAP[o])
+    );
+
+    function checkPatchableOperatorUsage(node: ts.Node) {
+      if (!isRxjsStaticOperatorCallExpression(node, typeChecker)) {
+        return ts.forEachChild(node, checkPatchableOperatorUsage);
+      }
+
+      const callExpr = node as ts.CallExpression;
+      if (!tsutils.isPropertyAccessExpression(callExpr.expression)) {
+        return ts.forEachChild(node, checkPatchableOperatorUsage);
+      }
+
+      const propAccess = callExpr.expression as ts.PropertyAccessExpression;
+      const name = propAccess.name.getText(sourceFile);
+      const operatorName = OPERATOR_RENAMES[name] || name;
+      const start = propAccess.getStart(sourceFile);
+      const end = propAccess.getEnd();
+      const operatorsToImport = new Set<OperatorWithAlias>([OPERATOR_WITH_ALIAS_MAP[operatorName]]);
+      const operatorsToAdd = subtractSets(operatorsToImport, rxjsOperatorImports);
+      const imports = createImportReplacements(operatorsToAdd, insertionStart);
+      rxjsOperatorImports = concatSets(rxjsOperatorImports, operatorsToAdd);
+      ctx.addFailure(
+        start,
+        end,
+        Rule.OBSERVABLE_FAILURE_STRING,
+        [Lint.Replacement.replaceFromTo(start, end, operatorAlias(operatorName))].concat(imports)
+      );
+      return ts.forEachChild(node, checkPatchableOperatorUsage);
+    }
+
+    return ts.forEachChild(ctx.sourceFile, checkPatchableOperatorUsage);
+  }
+
+  private removePatchedOperatorImports(ctx: Lint.WalkContext<void>): void {
+    const sourceFile = ctx.sourceFile;
+    for (const importStatement of sourceFile.statements.filter(tsutils.isImportDeclaration)) {
+      const moduleSpecifier = importStatement.moduleSpecifier.getText();
+      if (!moduleSpecifier.startsWith(`'rxjs/add/observable/`)) {
+        continue;
+      }
+      const importStatementStart = importStatement.getStart(sourceFile);
+      const importStatementEnd = importStatement.getEnd();
+      ctx.addFailure(
+        importStatementStart,
+        importStatementEnd,
+        Rule.IMPORT_FAILURE_STRING,
+        Lint.Replacement.deleteFromTo(importStatementStart, importStatementEnd)
+      );
+    }
+  }
+}
+
+function isRxjsStaticOperator(node: ts.PropertyAccessExpression) {
+  return 'Observable' === node.expression.getText() && RXJS_OPERATORS.has(node.name.getText());
+}
+
+function isRxjsStaticOperatorCallExpression(node: ts.Node, typeChecker: ts.TypeChecker) {
+  // Expression is of the form fn()
+  if (!tsutils.isCallExpression(node)) {
+    return false;
+  }
+  // Expression is of the form foo.fn
+  if (!tsutils.isPropertyAccessExpression(node.expression)) {
+    return false;
+  }
+  // fn is one of RxJs instance operators
+  if (!isRxjsStaticOperator(node.expression)) {
+    return false;
+  }
+  // fn(): k. Checks if k is an observable. Required to distinguish between
+  // array operators with same name as RxJs operators.
+  if (!returnsObservable(node, typeChecker)) {
+    return false;
+  }
+  return true;
+}
+
+function findImportedRxjsOperators(sourceFile: ts.SourceFile): Set<string> {
+  return new Set<string>(
+    sourceFile.statements.filter(tsutils.isImportDeclaration).reduce((current, decl) => {
+      if (!decl.importClause) {
+        return current;
+      }
+      if (!decl.moduleSpecifier.getText().startsWith(`'rxjs'`)) {
+        return current;
+      }
+      if (!decl.importClause.namedBindings) {
+        return current;
+      }
+      const bindings = decl.importClause.namedBindings;
+      if (ts.isNamedImports(bindings)) {
+        return [
+          ...current,
+          ...(Array.from(bindings.elements) || []).map(element => {
+            return element.name.getText();
+          })
+        ];
+      }
+      return current;
+    }, [])
+  );
+}
+
+function operatorAlias(operator: string) {
+  return 'observable' + operator[0].toUpperCase() + operator.substring(1, operator.length);
+}
+
+function createImportReplacements(operatorsToAdd: Set<OperatorWithAlias>, startIndex: number): Lint.Replacement[] {
+  return [...Array.from(operatorsToAdd.values())].map(tuple =>
+    Lint.Replacement.appendText(startIndex, `\nimport {${tuple.operator} as ${tuple.alias}} from 'rxjs';\n`)
+  );
+}
+
+/*
+ * https://github.com/ReactiveX/rxjs/tree/master/compat/add/observable
+ */
+const RXJS_OPERATORS = new Set([
+  'bindCallback',
+  'bindNodeCallback',
+  'combineLatest',
+  'concat',
+  'defer',
+  'empty',
+  'forkJoin',
+  'from',
+  'fromEvent',
+  'fromEventPattern',
+  'fromPromise',
+  'generate',
+  'if',
+  'interval',
+  'merge',
+  'never',
+  'of',
+  'onErrorResumeNext',
+  'pairs',
+  'rase',
+  'range',
+  'throw',
+  'timer',
+  'using',
+  'zip'
+]);
+
+// Not handling NEVER and EMPTY
+const OPERATOR_RENAMES: { [key: string]: string } = {
+  throw: 'throwError',
+  if: 'iif',
+  fromPromise: 'from'
+};
+
+type OperatorWithAlias = { operator: string; alias: string };
+type OperatorWithAliasMap = { [key: string]: OperatorWithAlias };
+
+const OPERATOR_WITH_ALIAS_MAP: OperatorWithAliasMap = Array.from(RXJS_OPERATORS).reduce((a, o) => {
+  const operatorName = OPERATOR_RENAMES[o] || o;
+  a[operatorName] = {
+    operator: operatorName,
+    alias: operatorAlias(operatorName)
+  };
+  return a;
+}, {});
diff --git a/src/migrateToPipeableOperatorsRule.ts b/src/migrateToPipeableOperatorsRule.ts
@@ -4,6 +4,7 @@
 import * as Lint from 'tslint';
 import * as tsutils from 'tsutils';
 import * as ts from 'typescript';
+import { subtractSets, concatSets, isObservable, returnsObservable, computeInsertionIndexForImports } from './utils';
 /**
  * A typed TSLint rule that inspects observable chains using patched RxJs
  * operators and turns them into a pipeable operator chain.
@@ -131,34 +132,7 @@ export class Rule extends Lint.Rules.TypedRule {
     }
   }
 }
-/**
- * Returns true if the {@link type} is an Observable or one of its sub-classes.
- */
-function isObservable(type: ts.Type, tc: ts.TypeChecker): boolean {
-  if (tsutils.isTypeReference(type)) {
-    type = type.target;
-  }
-  if (type.symbol !== undefined && type.symbol.name === 'Observable') {
-    return true;
-  }
-  if (tsutils.isUnionOrIntersectionType(type)) {
-    return type.types.some(t => isObservable(t, tc));
-  }
-  const bases = type.getBaseTypes();
-  return bases !== undefined && bases.some(t => isObservable(t, tc));
-}
-/**
- * Returns true if the return type of the expression represented by the {@link
- * node} is an Observable or one of its subclasses.
- */
-function returnsObservable(node: ts.CallLikeExpression, tc: ts.TypeChecker) {
-  const signature = tc.getResolvedSignature(node);
-  if (signature === undefined) {
-    return false;
-  }
-  const returnType = tc.getReturnTypeOfSignature(signature);
-  return isObservable(returnType, tc);
-}
+
 /**
  * Returns true if the identifier of the current expression is an RxJS instance
  * operator like map, switchMap etc.
@@ -222,20 +196,7 @@ function findImportedRxjsOperators(sourceFile: ts.SourceFile): Set<string> {
     }, [])
   );
 }
-/**
- * Returns the index to be used for inserting import statements potentially
- * after a leading file overview comment (separated from the file with \n\n).
- */
-function computeInsertionIndexForImports(sourceFile: ts.SourceFile): number {
-  const comments = ts.getLeadingCommentRanges(sourceFile.getFullText(), 0) || [];
-  if (comments.length > 0) {
-    const commentEnd = comments[0].end;
-    if (sourceFile.text.substring(commentEnd, commentEnd + 2) === '\n\n') {
-      return commentEnd + 2;
-    }
-  }
-  return sourceFile.getFullStart();
-}
+
 /**
  * Generates an array of {@link Lint.Replacement} representing import statements
  * for the {@link operatorsToAdd}.
@@ -248,19 +209,7 @@ function createImportReplacements(operatorsToAdd: Set<string>, startIndex: numbe
     Lint.Replacement.appendText(startIndex, `\nimport {${operator}} from 'rxjs/operators/${operator}';\n`)
   );
 }
-/**
- * Returns a new Set that contains elements present in the {@link source} but
- * not present in {@link target}
- */
-function subtractSets<T>(source: Set<T>, target: Set<T>): Set<T> {
-  return new Set([...Array.from(source.values())].filter(x => !target.has(x)));
-}
-/**
- * Returns a new Set that contains union of the two input sets.
- */
-function concatSets<T>(set1: Set<T>, set2: Set<T>): Set<T> {
-  return new Set([...Array.from(set1.values()), ...Array.from(set2.values())]);
-}
+
 /**
  * Returns the last chained RxJS call expression by walking up the AST.
  *