Rule no watch (#51)

* add rule

* fix: write correct description

* docs: add rule doc

* feat: handle watch on guard and sample

* fix: threw out the trash

* docs: iimproving readability and adding caution

* docs: iimproving readability and adding caution

* test: add more correct examples for no-watch rule

* fix: dont use sample and guard as callee name

* fix: add link to issue about js way for no-watch rule

* fix: remove all rename @typescript-no-watch to no-watch

* test: refactor tests for no-watch

Co-authored-by: Viktor <v.pasynok@space307.com>

README.md

@@ -48,3 +48,4 @@ To configure individual rules:
 - [effector/no-useless-methods](/rules/no-useless-methods/no-useless-methods.md)
 - [effector/no-ambiguity-target](/rules/no-ambiguity-target/no-ambiguity-target.md)
 - [effector/prefer-sample-over-forward-with-mapping](/rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping.md)
+- [effector/no-watch](/rules/no-watch/no-watch.md)

config/recommended.js

@@ -7,5 +7,6 @@ module.exports = {
     "effector/no-unnecessary-duplication": "warn",
     "effector/prefer-sample-over-forward-with-mapping": "warn",
     "effector/no-ambiguity-target": "warn",
+    "effector/no-watch": "warn",
   },
 };

index.js

@@ -7,8 +7,9 @@ module.exports = {
     "prefer-sample-over-forward-with-mapping": require("./rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping"),
     "no-useless-methods": require("./rules/no-useless-methods/no-useless-methods"),
     "no-ambiguity-target": require("./rules/no-ambiguity-target/no-ambiguity-target"),
+    "no-watch": require("./rules/no-watch/no-watch"),
   },
   configs: {
     recommended: require("./config/recommended"),
-  }
+  },
 };

package.json

@@ -30,4 +30,4 @@
   "dependencies": {
     "prettier": "^2.3.2"
   }
-}
+}

rules/no-watch/examples/correct.ts

@@ -0,0 +1,68 @@
+const event = {
+  watch() {
+    return {};
+  },
+};
+const effect = {
+  done: {
+    watch() {
+      return {};
+    },
+  },
+  fail: {
+    watch() {
+      return {};
+    },
+  },
+  doneData: {
+    watch() {
+      return {};
+    },
+  },
+  failData: {
+    watch() {
+      return {};
+    },
+  },
+  finally: {
+    watch() {
+      return {};
+    },
+  },
+  watch() {
+    return {};
+  },
+};
+const $ = {
+  updates: {
+    watch() {
+      return {};
+    },
+  },
+  watch() {
+    return {};
+  },
+};
+const sample = {
+  watch() {
+    return {};
+  },
+};
+const guard = {
+  watch() {
+    return {};
+  },
+};
+
+event.watch();
+effect.done.watch();
+effect.fail.watch();
+effect.doneData.watch();
+effect.failData.watch();
+effect.finally.watch();
+$.watch();
+$.updates.watch();
+sample.watch();
+guard.watch();
+
+export const T = () => true;

rules/no-watch/examples/incorrect/effect/base.ts

@@ -0,0 +1,6 @@
+import { createEffect } from "effector";
+
+const watcher = <T>(x: T) => x;
+const watchableFx = createEffect();
+
+watchableFx.watch(watcher);

rules/no-watch/examples/incorrect/effect/done.ts

@@ -0,0 +1,7 @@
+import { createEffect } from "effector";
+
+const watcher = <T>(x: T) => x;
+const watchableFx = createEffect();
+
+watchableFx.done.watch(watcher);
+watchableFx.doneData.watch(watcher);

rules/no-watch/examples/incorrect/effect/fail.ts

@@ -0,0 +1,7 @@
+import { createEffect } from "effector";
+
+const watcher = <T>(x: T) => x;
+const watchableFx = createEffect();
+
+watchableFx.fail.watch(watcher);
+watchableFx.failData.watch(watcher);

rules/no-watch/examples/incorrect/effect/finally.ts

@@ -0,0 +1,6 @@
+import { createEffect } from "effector";
+
+const watcher = <T>(x: T) => x;
+const watchableFx = createEffect();
+
+watchableFx.finally.watch(watcher);

rules/no-watch/examples/incorrect/event.ts

@@ -0,0 +1,6 @@
+import { createEvent } from "effector";
+
+const watcher = <T>(x: T) => x;
+const watchableEvent = createEvent();
+
+watchableEvent.watch(watcher);

rules/no-watch/examples/incorrect/guard.ts

@@ -0,0 +1,9 @@
+import { createEvent, guard } from "effector";
+
+const watcher = <T>(x: T) => x;
+const myEvent = createEvent();
+
+guard({
+  clock: myEvent,
+  filter: () => Math.random() > 0.5,
+}).watch(watcher);

rules/no-watch/examples/incorrect/sample.ts

@@ -0,0 +1,16 @@
+import { createEvent, sample } from "effector";
+
+const watcher = <T>(x: T) => x;
+const myEvent = createEvent();
+
+sample({
+  clock: myEvent,
+  fn: () => true,
+}).watch(watcher);
+
+const newEvent = sample({
+  clock: myEvent,
+  fn: () => true,
+});
+
+newEvent.watch(watcher);

rules/no-watch/examples/incorrect/store.ts

@@ -0,0 +1,7 @@
+import { createStore } from "effector";
+
+const watcher = <T>(x: T) => x;
+const $watchable = createStore(0);
+
+$watchable.watch(watcher);
+$watchable.updates.watch(watcher);

rules/no-watch/no-watch.js

@@ -0,0 +1,57 @@
+const {
+  traverseNestedObjectNode,
+} = require("../../utils/traverse-nested-object-node");
+
+module.exports = {
+  meta: {
+    type: "suggestion",
+    docs: {
+      description: "Avoid `.watch` calls on any Effector unit or operator",
+      category: "Quality",
+      recommended: true,
+    },
+    messages: {
+      abusiveCall:
+        "Method `.watch` leads to imperative code. Try to replace it with operators (`sample`, `guard`, etc) or use the `target` parameter of the operators.",
+    },
+    schema: [],
+  },
+  create(context) {
+    const { parserServices } = context;
+    if (!parserServices.hasFullTypeInformation) {
+      // JavaScript-way https://github.com/effector/eslint-plugin/issues/48#issuecomment-931107829
+      return {};
+    }
+    const checker = parserServices.program.getTypeChecker();
+
+    return {
+      CallExpression(node) {
+        const methodName = node.callee?.property?.name;
+        if (methodName !== "watch") {
+          return;
+        }
+
+        const object = traverseNestedObjectNode(node.callee?.object);
+        const originalNode = parserServices.esTreeNodeToTSNodeMap.get(object);
+        const type = checker.getTypeAtLocation(originalNode);
+
+        const isEffectorUnit =
+          ["Effect", "Event", "Store"].includes(type?.symbol?.escapedName) &&
+          type?.symbol?.parent?.escapedName?.includes("effector");
+
+        if (!isEffectorUnit) {
+          return;
+        }
+
+        reportWatchCall({ context, node });
+      },
+    };
+  },
+};
+
+function reportWatchCall({ context, node }) {
+  context.report({
+    node,
+    messageId: "abusiveCall",
+  });
+}

rules/no-watch/no-watch.md

@@ -0,0 +1,42 @@
+# effector/no-watch
+
+Method `.watch` leads to imperative code. Try replacing it with operators (`forward`, `sample`, etc) or use the `target` parameter of the operators.
+
+> Caution! This rule only works on projects using TypeScript.
+
+```ts
+const myFx = createEffect();
+const myEvent = createEvent();
+const $awesome = createStore();
+
+// 👍 good solutions
+forward({
+  from: myFx.finally,
+  to: myEvent,
+});
+
+guard({
+  clock: myEvent,
+  filter: Boolean,
+  target: myFx,
+});
+
+sample({
+  from: $awesome.updates,
+  fn: identity,
+  to: myEvent,
+});
+
+// 👎 bad solutions
+myFx.finally.watch(myEvent);
+
+myEvent.watch((payload) => {
+  if (Boolean(payload)) {
+    myFx(payload);
+  }
+});
+
+$awesome.updates.watch((data) => {
+  myEvent(identity(data));
+});
+```

rules/no-watch/no-watch.test.js

@@ -0,0 +1,20 @@
+const { RuleTester } = require("eslint");
+
+const rule = require("./no-watch.js");
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2020,
+    sourceType: "module",
+  },
+});
+
+ruleTester.run("effector/no-watch.test", rule, {
+  valid: [
+    "myFx.finally.watch(myEvent);",
+    "myEvent.watch((payload) => {if (Boolean(payload)) {myFx(payload);}});",
+    "$awesome.updates.watch((data) => {myEvent(identity(data));});",
+  ].map((code) => ({ code })),
+
+  invalid: [],
+});