Added rxjs-no-ignored-error rule to not allow .subscribe() calls with…

…out ignoring a second parameter (handling errors), when first parameter is a function or arrow function. This could help to catch uncaught exceptions which could turn SPA apps (like Angular) in unstable state

README.md

@@ -80,6 +80,7 @@ The package includes the following rules:
 | `rxjs-no-subject-unsubscribe` | Disallows the calling of `unsubscribe` directly upon `Subject` instances. For an explanation of why this can be a problem, see [this](https://stackoverflow.com/a/45112125/6680611) Stack Overflow answer. | None |
 | `rxjs-no-unused-add` | Disallows the importation of patched observables or operators that are not used in the module. | None |
 | `rxjs-no-wholesale` | Disallows the wholesale importation of `rxjs` or `rxjs/Rx`. | None |
+| `rxjs-no-ignored-error` | Disallows errors ignoring in Observable's .subscribe() calls. | None |
 
 ### Options
 

docs/index.md

@@ -13,6 +13,7 @@ The package includes the following rules:
 | `rxjs-no-subject-unsubscribe` | Disallows the calling of `unsubscribe` directly upon `Subject` instances. For an explanation of why this can be a problem, see [this](https://stackoverflow.com/a/45112125/6680611) Stack Overflow answer. | None |
 | `rxjs-no-unused-add` | Disallows the importation of patched observables or operators that are not used in the module. | None |
 | `rxjs-no-wholesale` | Disallows the wholesale importation of `rxjs` or `rxjs/Rx`. | None |
+| `rxjs-no-ignored-error` | Disallows errors ignoring in Observable's .subscribe() calls. | None |
 
 ### Options
 

fixtures/subscription-with-error-handler/fixture.ts

@@ -0,0 +1,11 @@
+import { Subject } from "rxjs/Subject";
+import { of } from "rxjs/Observable/of";
+
+const subject = new Subject();
+const observable = of([1, 2]);
+
+observable.subscribe(function () { }, () => 0);
+
+// When first parameter is not a function, should not trigger an error
+observable.subscribe(subject);
+

fixtures/subscription-with-error-handler/tsconfig.json

@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture.ts"]
+}

fixtures/subscription-with-error-handler/tslint.json

@@ -0,0 +1,10 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-ignored-error": {
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}

fixtures/subscription-without-error-handler/fixture-observable.ts

@@ -0,0 +1,6 @@
+import { Observable } from "rxjs/Observable";
+import "rxjs/add/observable/of";
+
+const observable = Observable.of([1, 2]);
+observable.subscribe(() => 0);
+

fixtures/subscription-without-error-handler/fixture-subject.ts

@@ -0,0 +1,5 @@
+import { Subject } from "rxjs/Subject";
+
+const subject = new Subject<number>();
+subject.subscribe(() => 0);
+

fixtures/subscription-without-error-handler/fixture-with-parameter.ts

@@ -0,0 +1,11 @@
+import { Subject } from "rxjs/Subject";
+import { Observable } from "rxjs/Observable";
+import "rxjs/add/observable/of";
+
+const observable = Observable.of(1, 2);
+const subject = new Subject<number>();
+const handler = () => 0;
+// Should mark it as error
+observable.subscribe(handler);
+// Should be ok - first parameter is not a function
+observable.subscribe(subject);

fixtures/subscription-without-error-handler/tsconfig.json

@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture-observable.ts", "fixture-subject.ts", "fixture-with-parameter.ts"]
+}

fixtures/subscription-without-error-handler/tslint.json

@@ -0,0 +1,10 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-ignored-error": {
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}

source/fixtures-spec.ts

@@ -471,20 +471,50 @@ describe("fixtures", function (): void {
         });
     });
 
+    describe("subscription related rules", () => {
+        describe("subscription-without-error-handler", () => {
+            it("should effect an 'rxjs-no-ignored-error' error on Observable", () => {
+                const result = lint("subscription-without-error-handler", "tslint.json", "fixture-observable.ts");
+                expect(result).to.have.property("errorCount", 1);
+                expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-ignored-error");
+            });
+        });
+        describe("subscription-without-error-handler", () => {
+            it("should effect an 'rxjs-no-ignored-error' error on Observable's ancestors", () => {
+                const result = lint("subscription-without-error-handler", "tslint.json", "fixture-subject.ts");
+                expect(result).to.have.property("errorCount", 1);
+                expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-ignored-error");
+            });
+        });
+        describe("subscription-without-error-handler", () => {
+            it("should effect an 'rxjs-no-ignored-error' error even if first parameter is a funciton variable", () => {
+                const result = lint("subscription-without-error-handler", "tslint.json", "fixture-with-parameter.ts");
+                expect(result).to.have.property("errorCount", 1);
+                expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-ignored-error");
+            });
+        });
+        describe("subscription-with-error-handler", () => {
+            it("should not produce errors", () => {
+                const result = lint("subscription-with-error-handler", "tslint.json");
+                expect(result).to.have.property("errorCount", 0);
+            });
+        });
+    });
+
     function lint(dir: string, configFileName?: string, fixtureFileName?: string): LintResult {
 
-            const fileName = `./fixtures/${dir}/${fixtureFileName || "fixture.ts"}`;
-            const content = fs.readFileSync(fileName, "utf8");
-            const program = Linter.createProgram(`./fixtures/${dir}/tsconfig.json`);
-            const linter = new Linter({ fix: false }, program);
-
-            const configuration = Configuration.findConfiguration(
-                configFileName ?
-                    `./fixtures/${dir}/${configFileName}` :
-                    `./fixtures/tslint.json`,
-                fileName
-            ).results;
-            linter.lint(fileName, content, configuration);
-            return linter.getResult();
+        const fileName = `./fixtures/${dir}/${fixtureFileName || "fixture.ts"}`;
+        const content = fs.readFileSync(fileName, "utf8");
+        const program = Linter.createProgram(`./fixtures/${dir}/tsconfig.json`);
+        const linter = new Linter({ fix: false }, program);
+
+        const configuration = Configuration.findConfiguration(
+            configFileName ?
+                `./fixtures/${dir}/${configFileName}` :
+                `./fixtures/tslint.json`,
+            fileName
+        ).results;
+        linter.lint(fileName, content, configuration);
+        return linter.getResult();
     }
 });

source/rules/rxjsNoIgnoredErrorRule.ts

@@ -0,0 +1,74 @@
+/**
+ * @license Copyright © 2018 Alexey Petushkov. 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://github.com/cartant/rxjs-tslint-rules
+ */
+/*tslint:disable:no-use-before-declare*/
+
+import * as Lint from "tslint";
+import * as ts from "typescript";
+
+import { couldBeType, isReferenceType } from "../support/util";
+import { InternalSymbolName } from "typescript";
+
+export class Rule extends Lint.Rules.TypedRule {
+
+    public static metadata: Lint.IRuleMetadata = {
+        description: "Disallows the calling of subscribe without specifying a error handler.",
+        options: null,
+        optionsDescription: "Not configurable.",
+        requiresTypeInfo: true,
+        ruleName: "rxjs-no-ignored-error",
+        type: "functionality",
+        typescriptOnly: true
+    };
+
+    public static FAILURE_STRING = "Calling subscribe without a second parameter is forbidden";
+
+    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+
+        return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
+    }
+}
+
+export class Walker extends Lint.ProgramAwareRuleWalker {
+
+    protected visitCallExpression(node: ts.CallExpression): void {
+
+        node.forEachChild((child) => {
+
+            if (child.kind === ts.SyntaxKind.PropertyAccessExpression) {
+                const propertyAccessExpression = child as ts.PropertyAccessExpression;
+                const name = propertyAccessExpression.name.getText();
+                const typeChecker = this.getTypeChecker();
+                const type = typeChecker.getTypeAtLocation(propertyAccessExpression.expression);
+
+                if ((name === "subscribe") &&
+                    isReferenceType(type) &&
+                    couldBeType(type.target, "Observable") &&
+                    (node.arguments.length && this.nodeIsLikelyAFunction(node.arguments[0])) &&
+                    node.arguments.length < 2
+                ) {
+                    this.addFailureAtNode(propertyAccessExpression.name, Rule.FAILURE_STRING);
+                }
+            }
+        });
+
+        super.visitCallExpression(node);
+    }
+
+    private nodeIsLikelyAFunction(node: ts.Expression): boolean {
+        // Fast check
+        if (node.kind === ts.SyntaxKind.ArrowFunction ||
+            node.kind === ts.SyntaxKind.FunctionExpression
+        ) {
+            return true;
+        }
+        // Check with a type checker
+        const typeChecker = this.getTypeChecker();
+        const type: ts.Type = typeChecker.getTypeAtLocation(node);
+        return couldBeType(type, "Function") ||
+            couldBeType(type, "ArrowFunction") ||
+            couldBeType(type, InternalSymbolName.Function);
+    }
+}