Skip to content

Commit

Permalink
feat(switchMap): Implment no-unsafe-switchmap.
Browse files Browse the repository at this point in the history
  • Loading branch information
cartant committed Feb 17, 2018
1 parent 12a6447 commit c08d98b
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 20 deletions.
17 changes: 9 additions & 8 deletions fixtures/no-unsafe-switchmap/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function ofType<T>(type: string, ...moreTypes: string[]): (source: Observable<T>
return source => source;
}

type Actions = Observable<any>;
const actions = Observable.of({});
const action$ = Observable.of({});
const empty = Observable.empty() as Observable<never>;
Expand All @@ -32,15 +33,15 @@ const patchedMorePutEffect = actions.ofType("DO_SOMETHING", "PUT_SOMETHING").do(
const pipedMoreGetEffect = actions.ofType("DO_SOMETHING", "GET_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const pipedMorePutEffect = actions.ofType("DO_SOMETHING", "PUT_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));

const patchedGetEpic = (action$) => action$.ofType("GET_SOMETHING").do(() => {}).switchMap(() => empty);
const patchedPutEpic = (action$) => action$.ofType("PUT_SOMETHING").do(() => {}).switchMap(() => empty);
const pipedGetEpic = (action$) => action$.ofType("GET_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const pipedPutEpic = (action$) => action$.ofType("PUT_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const patchedGetEpic = (action$: Actions) => action$.ofType("GET_SOMETHING").do(() => {}).switchMap(() => empty);
const patchedPutEpic = (action$: Actions) => action$.ofType("PUT_SOMETHING").do(() => {}).switchMap(() => empty);
const pipedGetEpic = (action$: Actions) => action$.ofType("GET_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const pipedPutEpic = (action$: Actions) => action$.ofType("PUT_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));

const patchedMoreGetEpic = (action$) => action$.ofType("DO_SOMETHING", "GET_SOMETHING").do(() => {}).switchMap(() => empty);
const patchedMorePutEpic = (action$) => action$.ofType("DO_SOMETHING", "PUT_SOMETHING").do(() => {}).switchMap(() => empty);
const pipedMoreGetEpic = (action$) => action$.ofType("DO_SOMETHING", "GET_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const pipedMorePutEpic = (action$) => action$.ofType("DO_SOMETHING", "PUT_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const patchedMoreGetEpic = (action$: Actions) => action$.ofType("DO_SOMETHING", "GET_SOMETHING").do(() => {}).switchMap(() => empty);
const patchedMorePutEpic = (action$: Actions) => action$.ofType("DO_SOMETHING", "PUT_SOMETHING").do(() => {}).switchMap(() => empty);
const pipedMoreGetEpic = (action$: Actions) => action$.ofType("DO_SOMETHING", "GET_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));
const pipedMorePutEpic = (action$: Actions) => action$.ofType("DO_SOMETHING", "PUT_SOMETHING").pipe(tap(() => {}), switchMap(() => empty));

const patchedSymbolGetEffect = actions.ofType(GET_SOMETHING).do(() => {}).switchMap(() => empty);
const patchedSymbolPutEffect = actions.ofType(PUT_SOMETHING).do(() => {}).switchMap(() => empty);
Expand Down
4 changes: 2 additions & 2 deletions source/fixtures-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ describe("fixtures", function (): void {
/*tslint:disable-next-line:no-invalid-this*/
this.timeout(5000);

describe.only("effect/epic-related rules", () => {
describe("effect/epic-related rules", () => {

describe("no-unsafe-switchmap", () => {

it("should effect 'no-unsafe-switchmap' errors", () => {
const result = lint("no-unsafe-switchmap", "tslint.json");
expect(result).to.have.property("errorCount", 11);
expect(result.failures[0]).to.have.property("ruleName", "no-unsafe-switchmap");
expect(result.failures[0]).to.have.property("ruleName", "rxjs-no-unsafe-switchmap");
});
});
});
Expand Down
160 changes: 151 additions & 9 deletions source/rules/rxjsNoUnsafeSwitchmapRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

import * as Lint from "tslint";
import * as ts from "typescript";
import * as tsutils from "tsutils";
import { couldBeType, isReferenceType } from "../support/util";

export class Rule extends Lint.Rules.AbstractRule {
export class Rule extends Lint.Rules.TypedRule {

public static metadata: Lint.IRuleMetadata = {
description: "Disallows unsafe switchMap usage in effects and epics.",
Expand Down Expand Up @@ -41,15 +43,155 @@ export class Rule extends Lint.Rules.AbstractRule {

public static FAILURE_STRING = "Unsafe switchMap usage in effects and epics is forbidden";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions()));
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
}
}

class Walker extends Lint.RuleWalker {
// Example options:
// { "disallow": ["create", "delete", "post", "put", "remove", "set", "update"] }
// { "disallow": "(create|delete|post|put|remove|set|update)" }
// { "allow": ["get", "read"] }
// { "allow": "(get|read)" }
class Walker extends Lint.ProgramAwareRuleWalker {

public static ACTIONS_REGEXP = /action(s|\$)?/i;
public static METHODS_REGEXP = /(ofType|pipe)/;
public static DEFAULT_DISALLOW = [
"create",
"delete",
"post",
"put",
"remove",
"set",
"update"
];

private allowRegExp: RegExp | null;
private disallowRegExp: RegExp | null;

private 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 fragments = value as string[];
const joined = fragments.map(fragment => `(\\b|[_'"])${fragment}(\\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.allowRegExp = Walker.createRegExp(options.allow);
this.disallowRegExp = Walker.createRegExp(options.disallow);
} else {
this.disallowRegExp = Walker.createRegExp(Walker.DEFAULT_DISALLOW);
}
}

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

node.forEachChild((child) => {

if (tsutils.isPropertyAccessExpression(child)) {
const { expression } = child;
if (tsutils.isIdentifier(expression)) {

const propertyName = child.name.getText();
const expressionText = expression.getText();
const typeChecker = this.getTypeChecker();
const type = typeChecker.getTypeAtLocation(expression);

if (isReferenceType(type) &&
Walker.ACTIONS_REGEXP.test(expressionText) &&
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 shouldDisallow(args: ts.NodeArray<ts.Expression>): boolean {

const names = args.map(arg => arg.getText());
if (this.allowRegExp && !names.every(name => this.allowRegExp.test(name))) {
return true;
}
if (this.disallowRegExp && names.some(name => this.disallowRegExp.test(name))) {
return true;
}
return false;
}

private walkPatchedOperators(): void {
}

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

if (this.shouldDisallow(node.arguments)) {
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 "pipe":
this.walkPipedOperators(parent);
break;
case "switchMap":
this.addFailureAtNode(parent, Rule.FAILURE_STRING);
break;
default:
break;
}
}
} else if (tsutils.isPropertyAccessExpression(parent)) {
name = parent.name;
} else {
break;
}
}
}
}

private walkPipedOperators(node: ts.CallExpression): void {
node.arguments.forEach(arg => {
if (tsutils.isCallExpression(arg)) {
const { expression } = arg;
if (tsutils.isIdentifier(expression) && (expression.getText() === "switchMap")) {
this.addFailureAtNode(arg, 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")) {
if (this.shouldDisallow(arg.arguments)) {
this.walkPipedOperators(node);
}
}
}
});
}
}
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"compilerOptions": {
"declaration": false,
"importHelpers": true,
"lib": ["dom", "es2015"],
"lib": ["dom", "es2017"],
"module": "commonjs",
"moduleResolution": "node",
"noEmitHelpers": true,
Expand Down

0 comments on commit c08d98b

Please sign in to comment.