feat(no-add): Add options to allow exceptions.

cartant · Nov 27, 2017 · 775e81d · 775e81d
1 parent db206be
commit 775e81d
Show file tree

Hide file tree

Showing 16 changed files with 226 additions and 14 deletions.
diff --git a/README.md b/README.md
@@ -63,7 +63,7 @@ The package includes the following rules:
 | Rule | Description | Options |
 | --- | --- | --- |
 | `rxjs-add` | Enforces the importation of patched observables and operators used in the module. | See below |
-| `rxjs-no-add` | Disallows the importation of patched observables and operators. | None |
+| `rxjs-no-add` | Disallows the importation of patched observables and operators. | See below |
 | `rxjs-no-create` | Disallows the calling of `Observable.create`. | None |
 | `rxjs-no-do` | I do without `do` operators. [Do you not?](https://youtu.be/spG-Yj0zEyc) | None |
 | `rxjs-no-operator` | Disallows importation from the `operator` directory. Useful if you prefer ['lettable' operators](https://github.com/ReactiveX/rxjs/blob/master/doc/lettable-operators.md) - which are located in the `operators` directory. | None |
@@ -101,9 +101,9 @@ Note that there is no `file` option for the `rxjs-no-unused-add` rule, so that r
 
 If the `file` option is not specified, patched observables and operators must be imported in the modules in which they are used.
 
-#### `rxjs-no-patched`
+#### `rxjs-no-add` and `rxjs-no-patched`
 
-The `rxjs-no-patched` rule takes an optional object with the optional properties `allowObservables` and `allowOperators`. The properties can be specified as booleans - to allow or disallow all observables or operators - or as arrays of strings - to allow or disallow a subset of observables or operators.
+The `rxjs-no-add` and `rxjs-no-patched` rules take an optional object with the optional properties `allowObservables` and `allowOperators`. The properties can be specified as booleans - to allow or disallow all observables or operators - or as arrays of strings - to allow or disallow a subset of observables or operators.
 
 For example:
 

diff --git a/fixtures/no-add-allow-all-observables/fixture.ts b/fixtures/no-add-allow-all-observables/fixture.ts
@@ -0,0 +1,3 @@
+import { Observable } from "rxjs";
+import "rxjs/add/observable/from";
+import "rxjs/add/observable/of";
diff --git a/fixtures/no-add-allow-all-observables/tsconfig.json b/fixtures/no-add-allow-all-observables/tsconfig.json
@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture.ts"]
+}
diff --git a/fixtures/no-add-allow-all-observables/tslint.json b/fixtures/no-add-allow-all-observables/tslint.json
@@ -0,0 +1,13 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-add": {
+      "options": [{
+        "allowObservables": true
+      }],
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}
diff --git a/fixtures/no-add-allow-all-operators/fixture.ts b/fixtures/no-add-allow-all-operators/fixture.ts
@@ -0,0 +1,3 @@
+import { Observable } from "rxjs";
+import "rxjs/add/operator/filter";
+import "rxjs/add/operator/map";
diff --git a/fixtures/no-add-allow-all-operators/tsconfig.json b/fixtures/no-add-allow-all-operators/tsconfig.json
@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture.ts"]
+}
diff --git a/fixtures/no-add-allow-all-operators/tslint.json b/fixtures/no-add-allow-all-operators/tslint.json
@@ -0,0 +1,13 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-add": {
+      "options": [{
+        "allowOperators": true
+      }],
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}
diff --git a/fixtures/no-add-allow-some-observables/fixture.ts b/fixtures/no-add-allow-some-observables/fixture.ts
@@ -0,0 +1,3 @@
+import { Observable } from "rxjs";
+import "rxjs/add/observable/from";
+import "rxjs/add/observable/of";
diff --git a/fixtures/no-add-allow-some-observables/tsconfig.json b/fixtures/no-add-allow-some-observables/tsconfig.json
@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture.ts"]
+}
diff --git a/fixtures/no-add-allow-some-observables/tslint.json b/fixtures/no-add-allow-some-observables/tslint.json
@@ -0,0 +1,13 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-add": {
+      "options": [{
+        "allowObservables": ["of"]
+      }],
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}
diff --git a/fixtures/no-add-allow-some-operators/fixture.ts b/fixtures/no-add-allow-some-operators/fixture.ts
@@ -0,0 +1,3 @@
+import { Observable } from "rxjs";
+import "rxjs/add/operator/filter";
+import "rxjs/add/operator/map";
diff --git a/fixtures/no-add-allow-some-operators/tsconfig.json b/fixtures/no-add-allow-some-operators/tsconfig.json
@@ -0,0 +1,13 @@
+{
+  "compilerOptions": {
+    "baseUrl": ".",
+    "lib": ["es2015"],
+    "noEmit": true,
+    "paths": {
+      "rxjs": ["../node_modules/rxjs"]
+    },
+    "skipLibCheck": true,
+    "target": "es5"
+  },
+  "include": ["fixture.ts"]
+}
diff --git a/fixtures/no-add-allow-some-operators/tslint.json b/fixtures/no-add-allow-some-operators/tslint.json
@@ -0,0 +1,13 @@
+{
+  "defaultSeverity": "error",
+  "jsRules": {},
+  "rules": {
+    "rxjs-no-add": {
+      "options": [{
+        "allowOperators": ["map"]
+      }],
+      "severity": "error"
+    }
+  },
+  "rulesDirectory": "../../build/rules"
+}
diff --git a/source/fixtures-spec.ts b/source/fixtures-spec.ts
@@ -100,6 +100,48 @@ describe("fixtures", function (): void {
             });
         });
 
+        describe("no-add-allow-all-observables", () => {
+
+            it("should not effect an 'rxjs-no-add' error", () => {
+
+                const result = lint("no-add-allow-all-observables", "tslint.json");
+
+                expect(result).to.have.property("errorCount", 0);
+            });
+        });
+
+        describe("no-add-allow-all-operators", () => {
+
+            it("should not effect an 'rxjs-no-add' error", () => {
+
+                const result = lint("no-add-allow-all-observables", "tslint.json");
+
+                expect(result).to.have.property("errorCount", 0);
+            });
+        });
+
+        describe("no-add-allow-some-observables", () => {
+
+            it("should not effect an 'rxjs-no-add' error for allowed observable", () => {
+
+                const result = lint("no-add-allow-some-observables", "tslint.json");
+
+                expect(result).to.have.property("errorCount", 1);
+                expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-add");
+            });
+        });
+
+        describe("no-add-allow-some-operators", () => {
+
+            it("should not effect an 'rxjs-no-add' error for allowed operator", () => {
+
+                const result = lint("no-add-allow-some-operators", "tslint.json");
+
+                expect(result).to.have.property("errorCount", 1);
+                expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-add");
+            });
+        });
+
         describe("no-create", () => {
 
             it("should effect 'rxjs-no-create' errors", () => {
@@ -256,7 +298,7 @@ describe("fixtures", function (): void {
 
         describe("no-patched-allow-some-observables", () => {
 
-            it("should not effect an 'rxjs-no-patched' error", () => {
+            it("should not effect an 'rxjs-no-patched' error for allowed observable", () => {
 
                 const result = lint("no-patched-allow-some-observables", "tslint.json");
 
@@ -267,7 +309,7 @@ describe("fixtures", function (): void {
 
         describe("no-patched-allow-some-operators", () => {
 
-            it("should not effect an 'rxjs-no-patched' error", () => {
+            it("should not effect an 'rxjs-no-patched' error for allowed operator", () => {
 
                 const result = lint("no-patched-allow-some-operators", "tslint.json");
 

diff --git a/source/rules/rxjsNoAddRule.ts b/source/rules/rxjsNoAddRule.ts
@@ -12,8 +12,27 @@ export class Rule extends Lint.Rules.AbstractRule {
 
     public static metadata: Lint.IRuleMetadata = {
         description: "Disallows the importation of patched observables and operators.",
-        options: null,
-        optionsDescription: "Not configurable.",
+        options: {
+            properties: {
+                allowObservables: {
+                    oneOf: [
+                        { type: "boolean" },
+                        { type: "array", items: { type: "string" } }
+                    ]
+                },
+                allowOperators: {
+                    oneOf: [
+                        { type: "boolean" },
+                        { type: "array", items: { type: "string" } }
+                    ]
+                }
+            },
+            type: "object"
+        },
+        optionsDescription: Lint.Utils.dedent`
+            An optional object with the optional properties \`allowObservables\` and \`allowOperators\`.
+            The properties can be specifed as booleans (they default to \`false\`) or as arrays containing
+            the names of the observables or operators that are allowed.`,
         requiresTypeInfo: false,
         ruleName: "rxjs-no-add",
         type: "functionality",
@@ -29,14 +48,49 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 class Walker extends Lint.RuleWalker {
 
-    public visitImportDeclaration(node: ts.ImportDeclaration): void {
+    private allowAllObservables = false;
+    private allowAllOperators = false;
+    private allowedObservables: string[] = [];
+    private allowedOperators: string[] = [];
 
-        const moduleSpecifier = node.moduleSpecifier.getText();
+    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
 
-        if (/^['"]rxjs\/add\//.test(moduleSpecifier)) {
-            this.addFailureAtNode(node, Rule.FAILURE_STRING);
+        super(sourceFile, options);
+
+        const [ruleOptions] = this.getOptions();
+        if (ruleOptions) {
+            if (ruleOptions.hasOwnProperty("allowObservables")) {
+                if (typeof ruleOptions.allowObservables.length === "number") {
+                    this.allowedObservables = ruleOptions.allowObservables;
+                } else {
+                    this.allowAllObservables = Boolean(ruleOptions.allowObservables);
+                }
+            }
+            if (ruleOptions.hasOwnProperty("allowOperators")) {
+                if (typeof ruleOptions.allowOperators.length === "number") {
+                    this.allowedOperators = ruleOptions.allowOperators;
+                } else {
+                    this.allowAllOperators = Boolean(ruleOptions.allowOperators);
+                }
+            }
         }
+    }
 
+    public visitImportDeclaration(node: ts.ImportDeclaration): void {
+
+        const moduleSpecifier = node.moduleSpecifier.getText();
+        const match = moduleSpecifier.match(/^['"]rxjs\/add\/(\w+)\/(\w+)/);
+        if (match) {
+            if ((match[1] === "observable") && !this.allowAllObservables) {
+                if (this.allowedObservables.indexOf(match[2]) === -1) {
+                    this.addFailureAtNode(node, Rule.FAILURE_STRING);
+                }
+            } else if ((match[1] === "operator") && !this.allowAllOperators) {
+                if (this.allowedOperators.indexOf(match[2]) === -1) {
+                    this.addFailureAtNode(node, Rule.FAILURE_STRING);
+                }
+            }
+        }
         super.visitImportDeclaration(node);
     }
 }
diff --git a/source/rules/rxjsNoPatchedRule.ts b/source/rules/rxjsNoPatchedRule.ts
@@ -35,7 +35,7 @@ export class Rule extends Lint.Rules.TypedRule {
         optionsDescription: Lint.Utils.dedent`
             An optional object with the optional properties \`allowObservables\` and \`allowOperators\`.
             The properties can be specifed as booleans (they default to \`false\`) or as arrays containing
-            the names of the observables or opertators that are allowed.`,
+            the names of the observables or operators that are allowed.`,
         requiresTypeInfo: true,
         ruleName: "rxjs-no-patched",
         type: "functionality",
@@ -62,14 +62,14 @@ class Walker extends UsedWalker {
         const [options] = this.getOptions();
         if (options) {
             if (options.hasOwnProperty("allowObservables")) {
-                if (options.allowObservables.length) {
+                if (typeof options.allowObservables.length === "number") {
                     allowedObservables = options.allowObservables;
                 } else {
                     allowAllObservables = Boolean(options.allowObservables);
                 }
             }
             if (options.hasOwnProperty("allowOperators")) {
-                if (options.allowOperators.length) {
+                if (typeof options.allowOperators.length === "number") {
                     allowedOperators = options.allowOperators;
                 } else {
                     allowAllOperators = Boolean(options.allowOperators);