Skip to content

Commit

Permalink
feat(no-patched): Add options to allow exceptions.
Browse files Browse the repository at this point in the history
  • Loading branch information
cartant committed Nov 27, 2017
1 parent 801ba29 commit b9af023
Show file tree
Hide file tree
Showing 19 changed files with 259 additions and 15 deletions.
22 changes: 21 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,15 @@ The package includes the following rules:
| `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 |
| `rxjs-no-patched` | Disallows the calling of patched methods. Methods must be imported and called explicitly - not via `Observable` or `Observable.prototype`. | None |
| `rxjs-no-patched` | Disallows the calling of patched methods. Methods must be imported and called explicitly - not via `Observable` or `Observable.prototype`. | See below |
| `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 |

### Options

#### `rxjs-add`

The `rxjs-add` rule takes an optional object with the property `file`. This is the path of the module - relative to the `tsconfig.json` - that imports the patched observables and operators.

For example:
Expand All @@ -99,6 +101,24 @@ 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`

The `rxjs-add` 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.

For example:

```json
"rules": {
"rxjs-add": {
"options": [{
"allowObservables": ["never", "throw"],
"allowOperators": false
}],
"severity": "error"
}
}
```

## Gotchas

### `@angular/cli`
Expand Down
2 changes: 2 additions & 0 deletions fixtures/no-patched-allow-all-observables/adds.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "rxjs/add/observable/from";
import "rxjs/add/observable/of";
6 changes: 6 additions & 0 deletions fixtures/no-patched-allow-all-observables/fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Observable } from "rxjs/Observable";
import { concat } from "rxjs/observable/concat";
import { map } from "rxjs/operator/map";

const source = concat(Observable.of(1), Observable.from([2, 3]));
const mapped = filtered.map.call(source, (value) => value + 1);
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-all-observables/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["adds.ts", "fixture.ts"]
}
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-all-observables/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-no-patched": {
"options": [{
"allowObservables": true
}],
"severity": "error"
}
},
"rulesDirectory": "../../build/rules"
}
2 changes: 2 additions & 0 deletions fixtures/no-patched-allow-all-operators/adds.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "rxjs/add/operator/filter";
import "rxjs/add/operator/map";
4 changes: 4 additions & 0 deletions fixtures/no-patched-allow-all-operators/fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { Subject } from "rxjs/Subject";

const subject = new Subject<number>();
const mapped = subject.filter(Boolean).map((value) => value + 1);
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-all-operators/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["adds.ts", "fixture.ts"]
}
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-all-operators/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-no-patched": {
"options": [{
"allowOperators": true
}],
"severity": "error"
}
},
"rulesDirectory": "../../build/rules"
}
2 changes: 2 additions & 0 deletions fixtures/no-patched-allow-some-observables/adds.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "rxjs/add/observable/from";
import "rxjs/add/observable/of";
6 changes: 6 additions & 0 deletions fixtures/no-patched-allow-some-observables/fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Observable } from "rxjs/Observable";
import { concat } from "rxjs/observable/concat";
import { map } from "rxjs/operator/map";

const source = concat(Observable.of(1), Observable.from([2, 3]));
const mapped = filtered.map.call(source, (value) => value + 1);
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-some-observables/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["adds.ts", "fixture.ts"]
}
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-some-observables/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-no-patched": {
"options": [{
"allowObservables": ["of"]
}],
"severity": "error"
}
},
"rulesDirectory": "../../build/rules"
}
2 changes: 2 additions & 0 deletions fixtures/no-patched-allow-some-operators/adds.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "rxjs/add/operator/filter";
import "rxjs/add/operator/map";
4 changes: 4 additions & 0 deletions fixtures/no-patched-allow-some-operators/fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { Subject } from "rxjs/Subject";

const subject = new Subject<number>();
const mapped = subject.filter(Boolean).map((value) => value + 1);
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-some-operators/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["adds.ts", "fixture.ts"]
}
13 changes: 13 additions & 0 deletions fixtures/no-patched-allow-some-operators/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-no-patched": {
"options": [{
"allowOperators": ["map"]
}],
"severity": "error"
}
},
"rulesDirectory": "../../build/rules"
}
42 changes: 42 additions & 0 deletions source/fixtures-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,48 @@ describe("fixtures", function (): void {
});
});

describe("no-patched-allow-all-observables", () => {

it("should not effect an 'rxjs-no-patched' error", () => {

const result = lint("no-patched-allow-all-observables", "tslint.json");

expect(result).to.have.property("errorCount", 0);
});
});

describe("no-patched-allow-all-operators", () => {

it("should not effect an 'rxjs-no-patched' error", () => {

const result = lint("no-patched-allow-all-operators", "tslint.json");

expect(result).to.have.property("errorCount", 0);
});
});

describe("no-patched-allow-some-observables", () => {

it("should not effect an 'rxjs-no-patched' error", () => {

const result = lint("no-patched-allow-some-observables", "tslint.json");

expect(result).to.have.property("errorCount", 1);
expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-patched");
});
});

describe("no-patched-allow-some-operators", () => {

it("should not effect an 'rxjs-no-patched' error", () => {

const result = lint("no-patched-allow-some-operators", "tslint.json");

expect(result).to.have.property("errorCount", 1);
expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-patched");
});
});

describe("observable-create", () => {

it("should effect an error unless explicit typing is used", () => {
Expand Down
78 changes: 64 additions & 14 deletions source/rules/rxjsNoPatchedRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,27 @@ export class Rule extends Lint.Rules.TypedRule {

public static metadata: Lint.IRuleMetadata = {
description: "Disallows the calling of patched methods.",
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 opertators that are allowed.`,
requiresTypeInfo: true,
ruleName: "rxjs-no-patched",
type: "functionality",
Expand All @@ -35,20 +54,51 @@ class Walker extends UsedWalker {

protected onSourceFileEnd(): void {

Object.keys(this.usedObservables).forEach((key) => {
let allowAllObservables = false;
let allowAllOperators = false;
let allowedObservables: string[] = [];
let allowedOperators: string[] = [];

this.usedObservables[key].forEach((node) => this.addFailureAtNode(
node,
`${Rule.FAILURE_STRING}: ${key}`
));
});
const [options] = this.getOptions();
if (options) {
if (options.hasOwnProperty("allowObservables")) {
if (options.allowObservables.length) {
allowedObservables = options.allowObservables;
} else {
allowAllObservables = Boolean(options.allowObservables);
}
}
if (options.hasOwnProperty("allowOperators")) {
if (options.allowOperators.length) {
allowedOperators = options.allowOperators;
} else {
allowAllOperators = Boolean(options.allowOperators);
}
}
}

Object.keys(this.usedOperators).forEach((key) => {
if (!allowAllObservables) {
Object.keys(this.usedObservables).forEach((key) => {

this.usedOperators[key].forEach((node) => this.addFailureAtNode(
node,
`${Rule.FAILURE_STRING}: ${key}`
));
});
if (allowedObservables.indexOf(key) === -1) {
this.usedObservables[key].forEach((node) => this.addFailureAtNode(
node,
`${Rule.FAILURE_STRING}: ${key}`
));
}
});
}

if (!allowAllOperators) {
Object.keys(this.usedOperators).forEach((key) => {

if (allowedOperators.indexOf(key) === -1) {
this.usedOperators[key].forEach((node) => this.addFailureAtNode(
node,
`${Rule.FAILURE_STRING}: ${key}`
));
}
});
}
}
}

0 comments on commit b9af023

Please sign in to comment.