Skip to content

Commit

Permalink
add unsafe first rule
Browse files Browse the repository at this point in the history
  • Loading branch information
hoffination committed Jul 11, 2018
1 parent f005f2a commit 38b32a0
Show file tree
Hide file tree
Showing 12 changed files with 640 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ The package includes the following rules (none of which are enabled by default):
| `rxjs-no-tap` | An alias for `rxjs-no-do`. | None |
| `rxjs-no-unbound-methods` | Disallows the passing of [unbound methods](https://blog.angularindepth.com/rxjs-avoiding-unbound-methods-fcf2648a805) as callbacks. | None |
| `rxjs-no-unsafe-catch` | Disallows unsafe `catch` and `catchError` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | [See below](#rxjs-no-unsafe-catch) |
| `rxjs-no-unsafe-first` | Disallows unsafe `first` and `take` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | None |
| `rxjs-no-unsafe-scope` | Disallows the use of variables/properties from unsafe/outer scopes in operator callbacks. | [See below](#rxjs-no-unsafe-scope) |
| `rxjs-no-unsafe-switchmap` | Disallows unsafe `switchMap` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | [See below](#rxjs-no-unsafe-switchmap) |
| `rxjs-no-unsafe-takeuntil` | Disallows the application of operators after `takeUntil`. Operators placed after `takeUntil` can effect [subscription leaks](https://medium.com/@cartant/rxjs-avoiding-takeuntil-leaks-fb5182d047ef). | None |
Expand Down
1 change: 1 addition & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The package includes the following rules (none of which are enabled by default):
| `rxjs-no-tap` | An alias for `rxjs-no-do`. | None |
| `rxjs-no-unbound-methods` | Disallows the passing of [unbound methods](https://blog.angularindepth.com/rxjs-avoiding-unbound-methods-fcf2648a805) as callbacks. | None |
| `rxjs-no-unsafe-catch` | Disallows unsafe `catch` and `catchError` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | [See below](#rxjs-no-unsafe-catch) |
| `rxjs-no-unsafe-first` | Disallows unsafe `first` and `take` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | None |
| `rxjs-no-unsafe-scope` | Disallows the use of variables/properties from unsafe/outer scopes in operator callbacks. | [See below](#rxjs-no-unsafe-scope) |
| `rxjs-no-unsafe-switchmap` | Disallows unsafe `switchMap` usage in [NgRx](https://github.com/ngrx/platform) effects and [`redux-observable`](https://github.com/redux-observable/redux-observable) epics. | [See below](#rxjs-no-unsafe-switchmap) |
| `rxjs-no-unsafe-takeuntil` | Disallows the application of operators after `takeUntil`. Operators placed after `takeUntil` can effect [subscription leaks](https://medium.com/@cartant/rxjs-avoiding-takeuntil-leaks-fb5182d047ef). | None |
Expand Down
174 changes: 174 additions & 0 deletions source/rules/rxjsNoUnsafeFirstRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/**
* @license 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 * as tsutils from "tsutils";
import * as decamelize from "decamelize";

import { couldBeType, isReferenceType } from "../support/util";

export class Rule extends Lint.Rules.TypedRule {

public static metadata: Lint.IRuleMetadata = {
description: "Disallows unsafe first/take usage in effects and epics.",
options: {
properties: {
observable: {
oneOf: [
{ type: "string" },
{ type: "array", items: { type: "string" } }
]
}
},
type: "object"
},
optionsDescription: Lint.Utils.dedent`
An optional object with an optional \`observable\` property.
The property can be specifed as a regular expression string or as an array of words and is used to identify the action observables from which effects and epics are composed.`,
requiresTypeInfo: true,
ruleName: "rxjs-no-unsafe-first",
type: "functionality",
typescriptOnly: true
};

public static FAILURE_STRING = "Unsafe first usage in effects and epics 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 {

public static METHODS_REGEXP = /(ofType|pipe)/;
public static DEFAULT_OBSERVABLE = "action(s|\\$)?";

private observableRegExp: RegExp;

public static createRegExp(value: any): RegExp | null {

if (!value || !value.length) {
return null;
}
const flags = "i";
if (typeof value === "string") {
return new RegExp(value, flags);
}
const words = value as string[];
const joined = words.map(word => `(\\b|_)${word}(\\b|_)`).join("|");
return new RegExp(`(${joined})`, flags);
}

constructor(sourceFile: ts.SourceFile, rawOptions: Lint.IOptions, program: ts.Program) {

super(sourceFile, rawOptions, program);

const [options] = this.getOptions();
if (options && (options.allow || options.disallow)) {
this.observableRegExp = new RegExp(options.observable || Walker.DEFAULT_OBSERVABLE, "i");
} else {
this.observableRegExp = new RegExp(Walker.DEFAULT_OBSERVABLE, "i");
}
}

protected visitCallExpression(node: ts.CallExpression): void {

const { expression: propertyAccessExpression } = node;
if (tsutils.isPropertyAccessExpression(propertyAccessExpression)) {

const { expression: observableExpression } = propertyAccessExpression;
let observableIdentifier: ts.Identifier | undefined = undefined;
if (tsutils.isIdentifier(observableExpression)) {
observableIdentifier = observableExpression;
} else if (tsutils.isPropertyAccessExpression(observableExpression)) {
observableIdentifier = observableExpression.name;
}

if (observableIdentifier && this.observableRegExp.test(observableIdentifier.getText())) {

const propertyName = propertyAccessExpression.name.getText();
const identifierText = observableIdentifier.getText();
const typeChecker = this.getTypeChecker();
const type = typeChecker.getTypeAtLocation(observableExpression);

if (isReferenceType(type) &&
Walker.METHODS_REGEXP.test(propertyName) &&
couldBeType(type.target, "Observable")) {

switch (propertyName) {
case "ofType":
this.walkPatchedTypes(node);
break;
case "pipe":
this.walkPipedTypes(node);
break;
default:
break;
}
}
}
}

super.visitCallExpression(node);
}

private walkPatchedOperators(node: ts.Node): void {

let name: ts.Identifier | undefined = undefined;
for (let parent = node.parent; parent; parent = parent.parent) {
if (tsutils.isCallExpression(parent)) {
if (name) {
switch (name.getText()) {
case "first":
this.addFailureAtNode(name, Rule.FAILURE_STRING);
break;
case "take":
this.addFailureAtNode(name, Rule.FAILURE_STRING);
break;
case "pipe":
this.walkPipedOperators(parent);
break;
default:
break;
}
}
} else if (tsutils.isPropertyAccessExpression(parent)) {
name = parent.name;
} else {
break;
}
}
}

private walkPatchedTypes(node: ts.CallExpression): void {

this.walkPatchedOperators(node);
}

private walkPipedOperators(node: ts.CallExpression): void {
node.arguments.forEach(arg => {
if (tsutils.isCallExpression(arg)) {
const { expression } = arg;
if (tsutils.isIdentifier(expression) && ((expression.getText() === "first") || (expression.getText() === "take"))) {
this.addFailureAtNode(expression, Rule.FAILURE_STRING);
}
}
});
}

private walkPipedTypes(node: ts.CallExpression): void {

node.arguments.forEach(arg => {
if (tsutils.isCallExpression(arg)) {
const { expression } = arg;
if (tsutils.isIdentifier(expression) && (expression.getText() === "ofType")) {
this.walkPipedOperators(node);
}
}
});
}
}
154 changes: 154 additions & 0 deletions test/v5/fixtures/no-unsafe-first/default/fixture.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
import { Observable } from "rxjs/Observable";
import { catchError, switchMap, tap } from "rxjs/operators";
import "rxjs/add/observable/empty";
import "rxjs/add/observable/of";
import "rxjs/add/operator/first";
import "rxjs/add/operator/do";
import "rxjs/add/operator/switchMap";
import "rxjs/add/operator/take";

declare module "rxjs/Observable" {
interface Observable<T> {
ofType(type: string, ...moreTypes: string[]): Observable<T>;
}
}

function ofType<T>(type: string, ...moreTypes: string[]): (source: Observable<T>) => Observable<T> {
return source => source;
}

type Actions = Observable<any>;
const actions = Observable.of({});
const empty = Observable.empty() as Observable<never>;

const safePatchedFirstEffect = actions.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty.first());
const unsafePatchedFirstEffect = actions.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty)
.first();
~~~~~ [no-unsafe-first]

const safePatchedTakeEffect = actions.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty.take(1));
const unsafePatchedTakeEffect = actions.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty)
.take(1);
~~~~ [no-unsafe-first]


const safePipedFirstEffect = actions.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty.pipe(first()))
);
const unsafePipedFirstEffect = actions.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty),
first());
~~~~~ [no-unsafe-first]

const safePipedTakeEffect = actions.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty.pipe(take()))
);
const unsafePipedTakeEffect = actions.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty),
take());
~~~~ [no-unsafe-first]


const safePatchedFirstEpic = (action$: Actions) => action$.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty.first());
const unsafePatchedFirstEpic = (action$: Actions) => action$.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty)
.first();
~~~~~ [no-unsafe-first]

const safePatchedTakeEpic = (action$: Actions) => action$.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty.take(1));
const unsafePatchedTakeEpic = (action$: Actions) => action$.ofType("DO_SOMETHING")
.do(() => {})
.switchMap(() => empty)
.take(1);
~~~~ [no-unsafe-first]


const safePipedFirstEpic = (action$: Actions) => action$.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty.pipe(first()))
);
const unsafePipedFirstEpic = (action$: Actions) => action$.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty),
first());
~~~~~ [no-unsafe-first]

const safePipedTakeEpic = (action$: Actions) => action$.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty.pipe(take(1)))
);
const unsafePipedTakeEpic = (action$: Actions) => action$.ofType("DO_SOMETHING").pipe(
tap(() => {}),
switchMap(() => empty),
take(1)
~~~~ [no-unsafe-first]
);


const safePipedOfTypeFirstEffect = actions.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty.pipe(first()))
);
const unsafePipedOfTypeFirstEffect = actions.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty),
first());
~~~~~ [no-unsafe-first]

const safePipedOfTypeTakeEffect = actions.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty.pipe(take(1)))
);
const unsafePipedOfTypeFirstEffect = actions.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty),
take(1));
~~~~ [no-unsafe-first]


const safePipedOfTypeFirstEpic = (action$: Actions) => action$.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty.pipe(first()))
);
const unsafePipedOfTypeFirstEpic = (action$: Actions) => action$.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty),
first()));
~~~~~ [no-unsafe-first]

const safePipedOfTypeTakeEpic = (action$: Actions) => action$.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty.pipe(take(1)))
);
const unsafePipedOfTypeTakeEpic = (action$: Actions) => action$.pipe(
ofType("DO_SOMETHING"),
tap(() => {}),
switchMap(() => empty),
take(1)));
~~~~ [no-unsafe-first]

[no-unsafe-first]: Unsafe first usage in effects and epics is forbidden
13 changes: 13 additions & 0 deletions test/v5/fixtures/no-unsafe-first/default/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": ["fixture.ts"]
}
8 changes: 8 additions & 0 deletions test/v5/fixtures/no-unsafe-first/default/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-no-unsafe-first": { "severity": "error" }
},
"rulesDirectory": "../../../../../build/rules"
}
Loading

0 comments on commit 38b32a0

Please sign in to comment.