From 075c69a6b34387cfc23dbbf8a9cf00a8fce79e5b Mon Sep 17 00:00:00 2001 From: Igor Kamyshev Date: Thu, 23 Sep 2021 14:38:18 +0700 Subject: [PATCH 1/2] Add rule no-useless-methods --- README.md | 1 + config/recommended.js | 1 + index.js | 1 + rules/no-useless-methods/examples/correct.ts | 33 +++++++++ .../examples/incorrect-guard-clock.ts | 5 ++ .../examples/incorrect-guard-source.ts | 5 ++ .../examples/incorrect-sample-clock.ts | 5 ++ .../examples/incorrect-sample-source.ts | 5 ++ .../no-useless-methods/no-useless-methods.js | 74 +++++++++++++++++++ .../no-useless-methods/no-useless-methods.md | 14 ++++ .../no-useless-methods.ts.test.js | 52 +++++++++++++ utils/traverse-parent-by-type.js | 13 ++++ 12 files changed, 209 insertions(+) create mode 100644 rules/no-useless-methods/examples/correct.ts create mode 100644 rules/no-useless-methods/examples/incorrect-guard-clock.ts create mode 100644 rules/no-useless-methods/examples/incorrect-guard-source.ts create mode 100644 rules/no-useless-methods/examples/incorrect-sample-clock.ts create mode 100644 rules/no-useless-methods/examples/incorrect-sample-source.ts create mode 100644 rules/no-useless-methods/no-useless-methods.js create mode 100644 rules/no-useless-methods/no-useless-methods.md create mode 100644 rules/no-useless-methods/no-useless-methods.ts.test.js create mode 100644 utils/traverse-parent-by-type.js diff --git a/README.md b/README.md index 81892db..33676c4 100644 --- a/README.md +++ b/README.md @@ -45,4 +45,5 @@ To configure individual rules: - [effector/enforce-effect-naming-convention](/rules/enforce-effect-naming-convention/enforce-effect-naming-convention.md) - [effector/no-getState](/rules/no-getState/no-getState.md) - [effector/no-unnecessary-duplication](/rules/no-unnecessary-duplication/no-unnecessary-duplication.md) +- [effector/no-useless-methods](/rules/no-useless-methods/no-useless-methods.md) - [effector/prefer-sample-over-forward-with-mapping](/rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping.md) diff --git a/config/recommended.js b/config/recommended.js index 6fbf10c..1f07308 100644 --- a/config/recommended.js +++ b/config/recommended.js @@ -3,6 +3,7 @@ module.exports = { "effector/enforce-store-naming-convention": "error", "effector/enforce-effect-naming-convention": "error", "effector/no-getState": "error", + "effector/no-useless-methods": "error", "effector/no-unnecessary-duplication": "warn", "effector/prefer-sample-over-forward-with-mapping": "warn", }, diff --git a/index.js b/index.js index a9d42d2..48a5e0a 100644 --- a/index.js +++ b/index.js @@ -5,6 +5,7 @@ module.exports = { "no-getState": require("./rules/no-getState/no-getState"), "no-unnecessary-duplication": require("./rules/no-unnecessary-duplication/no-unnecessary-duplication"), "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"), }, configs: { recommended: require("./config/recommended"), diff --git a/rules/no-useless-methods/examples/correct.ts b/rules/no-useless-methods/examples/correct.ts new file mode 100644 index 0000000..9a25896 --- /dev/null +++ b/rules/no-useless-methods/examples/correct.ts @@ -0,0 +1,33 @@ +import { sample, guard, createEvent } from "effector"; + +const trigger = createEvent(); +const target = createEvent(); + +// with target +sample({ clock: trigger, fn: Boolean, target }); +sample({ source: trigger, fn: Boolean, target }); + +guard({ clock: trigger, filter: Boolean, target }); +guard({ source: trigger, filter: Boolean, target }); + +// with simple assign +const result1 = sample({ clock: trigger, fn: Boolean }); +const result2 = sample({ source: trigger, fn: Boolean }); + +const result3 = guard({ clock: trigger, filter: Boolean }); +const result4 = guard({ source: trigger, filter: Boolean }); + +// with complex assign + +const somplexResult = { + target: guard({ source: trigger, filter: Boolean }), +}; + +function createSomething() { + const otherTrigger = createEvent(); + + return guard({ source: otherTrigger, filter: Boolean }); +} + +const createBooleanGuard = (otherTrigger) => + guard({ source: otherTrigger, filter: Boolean }); diff --git a/rules/no-useless-methods/examples/incorrect-guard-clock.ts b/rules/no-useless-methods/examples/incorrect-guard-clock.ts new file mode 100644 index 0000000..838974b --- /dev/null +++ b/rules/no-useless-methods/examples/incorrect-guard-clock.ts @@ -0,0 +1,5 @@ +import { guard, createEvent } from "effector"; + +const trigger = createEvent(); + +guard({ clock: trigger, filter: Boolean }); diff --git a/rules/no-useless-methods/examples/incorrect-guard-source.ts b/rules/no-useless-methods/examples/incorrect-guard-source.ts new file mode 100644 index 0000000..4b02127 --- /dev/null +++ b/rules/no-useless-methods/examples/incorrect-guard-source.ts @@ -0,0 +1,5 @@ +import { guard, createEvent } from "effector"; + +const trigger = createEvent(); + +guard({ source: trigger, filter: Boolean }); diff --git a/rules/no-useless-methods/examples/incorrect-sample-clock.ts b/rules/no-useless-methods/examples/incorrect-sample-clock.ts new file mode 100644 index 0000000..1f2f89c --- /dev/null +++ b/rules/no-useless-methods/examples/incorrect-sample-clock.ts @@ -0,0 +1,5 @@ +import { sample, createEvent } from "effector"; + +const trigger = createEvent(); + +sample({ clock: trigger, fn: Boolean }); diff --git a/rules/no-useless-methods/examples/incorrect-sample-source.ts b/rules/no-useless-methods/examples/incorrect-sample-source.ts new file mode 100644 index 0000000..bcf6b41 --- /dev/null +++ b/rules/no-useless-methods/examples/incorrect-sample-source.ts @@ -0,0 +1,5 @@ +import { sample, createEvent } from "effector"; + +const trigger = createEvent(); + +sample({ source: trigger, fn: Boolean }); diff --git a/rules/no-useless-methods/no-useless-methods.js b/rules/no-useless-methods/no-useless-methods.js new file mode 100644 index 0000000..219b1f1 --- /dev/null +++ b/rules/no-useless-methods/no-useless-methods.js @@ -0,0 +1,74 @@ +const { + extractImportedFromEffector, +} = require("../../utils/extract-imported-from-effector"); +const { traverseParentByType } = require("../../utils/traverse-parent-by-type"); + +module.exports = { + meta: { + type: "problem", + docs: { + description: "Forbids useless calls of `sample` and `guard`", + category: "Quality", + recommended: true, + }, + messages: { + uselessMethod: + "Method `{{ methodName }}` does nothing in this case. You should assign the result to variable or pass `target` to it.", + }, + schema: [], + }, + create(context) { + const importedFromEffector = new Map(); + + return { + ImportDeclaration(node) { + extractImportedFromEffector(importedFromEffector, node); + }, + CallExpression(node) { + const POSSIBLE_USELESS_METHODS = ["sample", "guard"]; + for (const method of POSSIBLE_USELESS_METHODS) { + const localMethod = importedFromEffector.get(method); + if (!localMethod) { + continue; + } + + const isEffectorMethod = node?.callee?.name === localMethod; + if (!isEffectorMethod) { + continue; + } + + const resultAssignedInVariable = traverseParentByType( + node, + "VariableDeclarator" + ); + if (resultAssignedInVariable) { + continue; + } + + const resultReturnedFromFactory = traverseParentByType( + node, + "ReturnStatement" + ); + if (resultReturnedFromFactory) { + continue; + } + + const configHasTarget = node?.arguments?.[0]?.properties?.some( + (prop) => prop?.key.name === "target" + ); + if (configHasTarget) { + continue; + } + + context.report({ + node, + messageId: "uselessMethod", + data: { + methodName: node?.callee?.name, + }, + }); + } + }, + }; + }, +}; diff --git a/rules/no-useless-methods/no-useless-methods.md b/rules/no-useless-methods/no-useless-methods.md new file mode 100644 index 0000000..a918b01 --- /dev/null +++ b/rules/no-useless-methods/no-useless-methods.md @@ -0,0 +1,14 @@ +# effector/no-useless-methods + +Call of `gaurd`/`sample` without `target` or variable assignment is useless. It can be omitted from source code. + +```ts +// 👎 can be omitted +guard({ clock: trigger, filter: Boolean }); + +// 👍 makes sense +const target1 = guard({ clock: trigger, filter: Boolean }); + +// 👍 make sense too +guard({ clock: trigger, filter: Boolean, target: target2 }); +``` diff --git a/rules/no-useless-methods/no-useless-methods.ts.test.js b/rules/no-useless-methods/no-useless-methods.ts.test.js new file mode 100644 index 0000000..7a07af4 --- /dev/null +++ b/rules/no-useless-methods/no-useless-methods.ts.test.js @@ -0,0 +1,52 @@ +const { RuleTester } = + require("@typescript-eslint/experimental-utils").ESLintUtils; +const { join } = require("path"); + +const { readExample } = require("../../utils/read-example"); +const rule = require("./no-useless-methods"); + +const ruleTester = new RuleTester({ + parser: "@typescript-eslint/parser", + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + project: "./tsconfig.json", + tsconfigRootDir: join(__dirname, ".."), + }, +}); + +const readExampleForTheRule = (name) => ({ + code: readExample(__dirname, name), + filename: join(__dirname, "examples", name), +}); + +ruleTester.run("effector/no-useless-methods.ts.test", rule, { + valid: ["correct.ts"].map(readExampleForTheRule), + + invalid: [ + ...["incorrect-sample-clock.ts", "incorrect-sample-source.ts"] + .map(readExampleForTheRule) + .map((result) => ({ + ...result, + errors: [ + { + messageId: "uselessMethod", + type: "CallExpression", + data: { methodName: "sample" }, + }, + ], + })), + ...["incorrect-guard-clock.ts", "incorrect-guard-source.ts"] + .map(readExampleForTheRule) + .map((result) => ({ + ...result, + errors: [ + { + messageId: "uselessMethod", + type: "CallExpression", + data: { methodName: "guard" }, + }, + ], + })), + ], +}); diff --git a/utils/traverse-parent-by-type.js b/utils/traverse-parent-by-type.js new file mode 100644 index 0000000..093c003 --- /dev/null +++ b/utils/traverse-parent-by-type.js @@ -0,0 +1,13 @@ +function traverseParentByType(node, type) { + if (!node) { + return null; + } + + if (node.type === type) { + return node; + } + + return traverseParentByType(node.parent, type); +} + +module.exports = { traverseParentByType }; From 30317fcbb6e362ebf8316b78e374b48002bb05bf Mon Sep 17 00:00:00 2001 From: Igor Kamyshev Date: Thu, 23 Sep 2021 14:49:52 +0700 Subject: [PATCH 2/2] Fix nested-object case in no-useless-methods --- .../examples/correct-nested.ts | 17 +++++++++++++++++ rules/no-useless-methods/no-useless-methods.js | 8 ++++++++ .../no-useless-methods.ts.test.js | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 rules/no-useless-methods/examples/correct-nested.ts diff --git a/rules/no-useless-methods/examples/correct-nested.ts b/rules/no-useless-methods/examples/correct-nested.ts new file mode 100644 index 0000000..947f916 --- /dev/null +++ b/rules/no-useless-methods/examples/correct-nested.ts @@ -0,0 +1,17 @@ +import { createEvent, createStore, guard, sample } from "effector"; + +const emailValidationFired = createEvent(); +const $isEmailFromGuestiaExist = createStore(false); +const $email = createStore(""); +const $emailError = createStore(false); + +sample({ + clock: guard({ + clock: emailValidationFired, + source: $isEmailFromGuestiaExist, + filter: (isExist) => !isExist, + }), + source: $email, + fn: Boolean, + target: $emailError, +}); diff --git a/rules/no-useless-methods/no-useless-methods.js b/rules/no-useless-methods/no-useless-methods.js index 219b1f1..fdca842 100644 --- a/rules/no-useless-methods/no-useless-methods.js +++ b/rules/no-useless-methods/no-useless-methods.js @@ -53,6 +53,14 @@ module.exports = { continue; } + const resultPartOfChain = traverseParentByType( + node, + "ObjectExpression" + ); + if (resultPartOfChain) { + continue; + } + const configHasTarget = node?.arguments?.[0]?.properties?.some( (prop) => prop?.key.name === "target" ); diff --git a/rules/no-useless-methods/no-useless-methods.ts.test.js b/rules/no-useless-methods/no-useless-methods.ts.test.js index 7a07af4..f6acf2e 100644 --- a/rules/no-useless-methods/no-useless-methods.ts.test.js +++ b/rules/no-useless-methods/no-useless-methods.ts.test.js @@ -21,7 +21,7 @@ const readExampleForTheRule = (name) => ({ }); ruleTester.run("effector/no-useless-methods.ts.test", rule, { - valid: ["correct.ts"].map(readExampleForTheRule), + valid: ["correct.ts", "correct-nested.ts"].map(readExampleForTheRule), invalid: [ ...["incorrect-sample-clock.ts", "incorrect-sample-source.ts"]