feat: no-ambiguity-target

Author: kireevmp

src/index.ts

@@ -7,6 +7,7 @@ import enforceGateNamingConvention from "./rules/enforce-gate-naming-convention/
 import enforceStoreNamingConvention from "./rules/enforce-store-naming-convention/enforce-store-naming-convention"
 import keepOptionsOrder from "./rules/keep-options-order/keep-options-order"
 import mandatoryScopeBinding from "./rules/mandatory-scope-binding/mandatory-scope-binding"
+import noAmbiguityTarget from "./rules/no-ambiguity-target/no-ambiguity-target"
 import noForward from "./rules/no-forward/no-forward"
 import noGetState from "./rules/no-getState/no-getState"
 import noGuard from "./rules/no-guard/no-guard"
@@ -26,6 +27,7 @@ const base = {
     "enforce-store-naming-convention": enforceStoreNamingConvention,
     "keep-options-order": keepOptionsOrder,
     "mandatory-scope-binding": mandatoryScopeBinding,
+    "no-ambiguity-target": noAmbiguityTarget,
     "no-forward": noForward,
     "no-getState": noGetState,
     "no-guard": noGuard,

src/rules/no-ambiguity-target/no-ambiguity-target.md

@@ -0,0 +1,16 @@
+---
+description: Forbid ambiguous target in sample and guard
+---
+
+# effector/no-ambiguity-target
+
+Call of `guard`/`sample` with `target` and variable assignment creates ambiguity. One of them should be removed.
+
+```ts
+// 👎 should be rewritten
+const result = guard({ clock: trigger, filter: Boolean, target })
+
+// 👍 makes sense
+guard({ clock: trigger, filter: Boolean, target })
+const result = target
+```

src/rules/no-ambiguity-target/no-ambiguity-target.test.ts

@@ -0,0 +1,162 @@
+import { RuleTester } from "@typescript-eslint/rule-tester"
+import { parser } from "typescript-eslint"
+
+import { ts } from "@/shared/tag"
+
+import rule from "./no-ambiguity-target"
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parser,
+    parserOptions: {
+      projectService: { allowDefaultProject: ["*.ts"], defaultProject: "tsconfig.fixture.json" },
+    },
+  },
+})
+
+ruleTester.run("no-ambiguity-target", rule, {
+  valid: [
+    {
+      name: "sample target",
+      code: ts`
+        import { sample, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        sample({ clock: trigger, fn: Boolean, target })
+        sample({ source: trigger, fn: Boolean, target })
+      `,
+    },
+    {
+      name: "guard target",
+      code: ts`
+        import { sample, guard, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        guard({ clock: trigger, filter: Boolean, target })
+        guard({ source: trigger, filter: Boolean, target })
+      `,
+    },
+    {
+      name: "sample assignment",
+      code: ts`
+        import { sample, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        const a = sample({ clock: trigger, fn: Boolean })
+        const b = sample({ source: trigger, fn: Boolean })
+        const c = sample(trigger, trigger, () => null)
+      `,
+    },
+    {
+      name: "guard assignment",
+      code: ts`
+        import { sample, guard, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        const a = guard({ clock: trigger, filter: Boolean })
+        const b = guard({ source: trigger, filter: Boolean })
+        const c = guard(trigger, { filter: Boolean })
+      `,
+    },
+    {
+      name: "sample object member",
+      code: ts`
+        import { sample, createEvent } from "effector"
+
+        const source = createEvent()
+
+        const $$ = { a: sample({ source }) }
+      `,
+    },
+    {
+      name: "sample returned from factory",
+      code: ts`
+        import { sample, createEvent, Store } from "effector"
+
+        const source = createEvent()
+
+        const createSourced = (clock: Store<unknown>) => sample(clock, source)
+        const createSourcedObject = (clock: Store<unknown>) => sample({ clock, source })
+
+        const truthful = (clock: Store<unknown>) => {
+          return sample({ clock, filter: Boolean })
+        }
+      `,
+    },
+    {
+      // https://github.com/igorkamyshev/eslint-plugin-effector/issues/133
+      name: "function in object",
+      code: ts`
+        import { createStore, createEvent, sample } from "effector"
+
+        const obj = {
+          fn: () => {
+            const $store = createStore(0)
+            const event = createEvent()
+
+            sample({ source: event, target: $store })
+          },
+        }
+      `,
+    },
+  ],
+  invalid: [
+    {
+      name: "guard object",
+      code: ts`
+        import { guard, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        const result = {
+          something: guard({ clock: trigger, filter: Boolean, target }),
+        }
+      `,
+      errors: [{ messageId: "ambiguous", line: 7, column: 14, data: { method: "guard" } }],
+    },
+    {
+      name: "sample",
+      code: ts`
+        import { sample, createEvent } from "effector"
+
+        const trigger = createEvent()
+        const target = createEvent()
+
+        const result = sample({ clock: trigger, fn: Boolean, target })
+      `,
+      errors: [{ messageId: "ambiguous", line: 6, column: 16, data: { method: "sample" } }],
+    },
+    {
+      name: "factory",
+      code: ts`
+        import { sample, createEvent, Store } from "effector"
+
+        const target = createEvent<unknown>()
+
+        const truthful = (clock: Store<unknown>) => sample({ clock, filter: Boolean, target })
+      `,
+      errors: [{ messageId: "ambiguous", line: 5, column: 45, data: { method: "sample" } }],
+    },
+    {
+      name: "nested",
+      code: ts`
+        import { sample, createEvent, Store } from "effector"
+
+        const clock = createEvent<unknown>()
+        const target = createEvent<unknown>()
+
+        sample({ clock: sample({ clock, filter: Boolean, target }), fn: () => true, target })
+      `,
+      errors: [{ messageId: "ambiguous", line: 6, column: 17, data: { method: "sample" } }],
+    },
+  ],
+})

src/rules/no-ambiguity-target/no-ambiguity-target.ts

@@ -0,0 +1,78 @@
+import { type TSESTree as Node, AST_NODE_TYPES as NodeType } from "@typescript-eslint/utils"
+import esquery from "esquery"
+import type { Node as ESNode } from "estree"
+
+import { createRule } from "@/shared/create"
+
+export default createRule({
+  name: "no-ambiguity-target",
+  meta: {
+    type: "problem",
+    docs: {
+      description: "Forbid ambiguous target in `sample` and `guard`.",
+    },
+    messages: {
+      ambiguous:
+        "Method `{{ method }}` both specifies `target` option and assigns the result to a variable. Consider removing one of them.",
+    },
+    schema: [],
+  },
+  defaultOptions: [],
+  create: (context) => {
+    const imports = new Set<string>()
+
+    const source = context.sourceCode
+    const visitorKeys = source.visitorKeys
+
+    const PACKAGE_NAME = /^effector(?:\u002Fcompat)?$/
+
+    const importSelector = `ImportDeclaration[source.value=${PACKAGE_NAME}]`
+    const methodSelector = `ImportSpecifier[imported.name=/(sample|guard)/]`
+
+    const usageStack: boolean[] = []
+
+    const query = {
+      target: esquery.parse("!Property.properties > Identifier.key[name=target]"),
+    }
+
+    type MethodCall = Node.CallExpression & { callee: Node.Identifier }
+
+    // TODO: Implement rule logic
+    return {
+      "ReturnStatement": () => usageStack.push(true),
+      "ReturnStatement:exit": () => usageStack.pop(),
+
+      "VariableDeclarator": () => usageStack.push(true),
+      "VariableDeclarator:exit": () => usageStack.pop(),
+
+      "ObjectExpression": () => usageStack.push(true),
+      "ObjectExpression:exit": () => usageStack.pop(),
+
+      "BlockStatement": () => usageStack.push(false),
+      "BlockStatement:exit": () => usageStack.pop(),
+
+      [`${importSelector} > ${methodSelector}`]: (node: Node.ImportSpecifier) => imports.add(node.local.name),
+
+      [`CallExpression[callee.type="Identifier"]`]: (node: MethodCall) => {
+        const isTracked = imports.has(node.callee.name)
+        if (!isTracked) return
+
+        const isUsed = usageStack.at(-1) ?? false
+        if (!isUsed) return
+
+        const [config] = node.arguments
+
+        if (config?.type !== NodeType.ObjectExpression) /* can't have a target */ return
+
+        const [target] = esquery
+          .match(config as ESNode, query.target, { visitorKeys })
+          .map((node) => node as Node.Property)
+          .filter((prop) => prop.parent === config)
+
+        if (!target) return
+
+        context.report({ node, messageId: "ambiguous", data: { method: node.callee.name } })
+      },
+    }
+  },
+})

src/rules/no-useless-methods/no-useless-methods.test.ts

@@ -164,5 +164,16 @@ ruleTester.run("no-useless-methods", rule, {
       `,
       errors: [{ messageId: "uselessMethod", line: 6, data: { method: "sample" } }],
     },
+    {
+      name: "sample in for loop",
+      code: ts`
+        import { sample, createEvent } from "effector"
+
+        const source = createEvent()
+
+        for (const i = 0; i < 10; i++) sample({ source })
+      `,
+      errors: [{ messageId: "uselessMethod", line: 5, column: 32, data: { method: "sample" } }],
+    },
   ],
 })

src/ruleset.ts

@@ -4,7 +4,7 @@ const recommended = {
   "effector/enforce-effect-naming-convention": "error",
   "effector/enforce-store-naming-convention": "error",
   "effector/keep-options-order": "warn",
-  // "effector/no-ambiguity-target": "warn",
+  "effector/no-ambiguity-target": "warn",
   // "effector/no-duplicate-on": "error",
   "effector/no-forward": "error",
   "effector/no-getState": "error",