Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rule: strict-effect-handlers #78

Merged
merged 10 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ To configure individual rules:
- [effector/no-watch](rules/no-watch/no-watch.md)
- [effector/no-unnecessary-combination](rules/no-unnecessary-combination/no-unnecessary-combination.md)
- [effector/no-duplicate-on](rules/no-duplicate-on/no-duplicate-on.md)
- [effector/strict-effect-handlers](rules/strict-effect-handlers/strict-effect-handlers.md)

## Maintenance

Expand Down
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = {
"no-watch": require("./rules/no-watch/no-watch"),
"no-unnecessary-combination": require("./rules/no-unnecessary-combination/no-unnecessary-combination"),
"no-duplicate-on": require("./rules/no-duplicate-on/no-duplicate-on"),
"strict-effect-handlers": require("./rules/strict-effect-handlers/strict-effect-handlers"),
},
configs: {
recommended: require("./config/recommended"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ const {
extractImportedFromEffector,
} = require("../../utils/extract-imported-from-effector");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { hasEffectorType } = require("../../utils/has-effector-type");
const { nodeTypeIs } = require("../../utils/node-type-is");
const { namingOf } = require("../../utils/naming");

module.exports = {
meta: {
Expand All @@ -28,11 +29,9 @@ module.exports = {
if (parserServices.hasFullTypeInformation) {
return {
VariableDeclarator(node) {
const isEffectorEffect = hasEffectorType({
const isEffectorEffect = nodeTypeIs.effect({
node,
context,
typeNames: ["Effect"],
useInitializer: true,
});

if (!isEffectorEffect) {
Expand All @@ -41,11 +40,9 @@ module.exports = {

const effectName = node.id.name;

if (effectName?.endsWith("Fx")) {
return;
if (namingOf.effect.isInvalid({ name: effectName })) {
reportEffectNameConventionViolation({ context, node, effectName });
}

reportEffectNameConventionViolation({ context, node, effectName });
},
};
}
Expand Down Expand Up @@ -77,7 +74,7 @@ module.exports = {
}

const effectName = node.parent.id.name;
if (effectName.endsWith("Fx")) {
if (namingOf.effect.isValid({ name: effectName })) {
continue;
}

Expand All @@ -101,7 +98,7 @@ module.exports = {
}

const effectName = node.parent.id.name;
if (effectName.endsWith("Fx")) {
if (namingOf.effect.isValid({ name: effectName })) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const {
extractImportedFromEffector,
} = require("../../utils/extract-imported-from-effector");
const { isStoreNameValid } = require("../../utils/is-store-name-valid");
const { namingOf } = require("../../utils/naming");
const {
validateStoreNameConvention,
} = require("../../utils/validate-store-name-convention");
Expand All @@ -12,7 +12,7 @@ const {
getCorrectedStoreName,
} = require("../../utils/get-corrected-store-name");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { hasEffectorType } = require("../../utils/has-effector-type");
const { nodeTypeIs } = require("../../utils/node-type-is");

module.exports = {
meta: {
Expand Down Expand Up @@ -41,11 +41,9 @@ module.exports = {
if (parserServices.hasFullTypeInformation) {
return {
VariableDeclarator(node) {
const isEffectorStore = hasEffectorType({
const isEffectorStore = nodeTypeIs.store({
node,
context,
typeNames: ["Store"],
useInitializer: true,
});

if (!isEffectorStore) {
Expand All @@ -54,15 +52,13 @@ module.exports = {

const storeName = node.id.name;

if (isStoreNameValid(storeName, context)) {
return;
if (namingOf.store.isInvalid({ name: storeName, context })) {
reportStoreNameConventionViolation({
context,
node,
storeName,
});
}

reportStoreNameConventionViolation({
context,
node,
storeName,
});
},
};
}
Expand Down Expand Up @@ -95,7 +91,7 @@ module.exports = {

const storeName = node.parent.id.name;

if (isStoreNameValid(storeName, context)) {
if (namingOf.store.isValid({ name: storeName, context })) {
continue;
}

Expand All @@ -111,7 +107,9 @@ module.exports = {
if (node.callee?.property?.name === "map") {
const storeNameCreatedFromMap = node.callee?.object?.name;

if (!isStoreNameValid(storeNameCreatedFromMap, context)) {
if (
namingOf.store.isInvalid({ name: storeNameCreatedFromMap, context })
) {
return;
}

Expand All @@ -123,7 +121,7 @@ module.exports = {

const storeName = node.parent.id.name;

if (isStoreNameValid(storeName, context)) {
if (namingOf.store.isValid({ name: storeName, context })) {
return;
}

Expand All @@ -148,7 +146,7 @@ module.exports = {

const storeName = node.parent.id.name;

if (isStoreNameValid(storeName, context)) {
if (namingOf.store.isValid({ name: storeName, context })) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions rules/no-duplicate-on/no-duplicate-on.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { getNestedObjectName } = require("../../utils/get-nested-object-name");
const { isStore } = require("../../utils/is");
const { is } = require("../../utils/is");

module.exports = {
meta: {
Expand Down Expand Up @@ -59,7 +59,7 @@ module.exports = {
const storeObject = getNestedCallee(node) ?? getAssignedVariable(node);
const storeName = getStoreName(storeObject);

if (!isStore({ context, node: storeObject })) {
if (!is.store({ context, node: storeObject })) {
return;
}

Expand Down
45 changes: 9 additions & 36 deletions rules/no-getState/no-getState.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
const {
traverseNestedObjectNode,
} = require("../../utils/traverse-nested-object-node");
const { isStoreNameValid } = require("../../utils/is-store-name-valid");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { hasEffectorType } = require("../../utils/has-effector-type");
const { is } = require("../../utils/is");

module.exports = {
meta: {
Expand All @@ -21,46 +20,20 @@ module.exports = {
schema: [],
},
create(context) {
const { parserServices } = context;

return {
CallExpression(node) {
const methodName = node.callee?.property?.name;
if (methodName !== "getState") {
return;
}
'CallExpression[callee.property.name="getState"]'(node) {
const storeNode = traverseNestedObjectNode(node.callee?.object);

const object = traverseNestedObjectNode(node.callee?.object);
const objectName = object?.name;
const isEffectorStore = is.store({
context,
node: storeNode,
});

if (!objectName) {
if (!isEffectorStore) {
return;
}

// TypeScript-way
if (parserServices.hasFullTypeInformation) {
const isEffectorStore = hasEffectorType({
node: object,
context,
typeNames: ["Store"],
useInitializer: false,
});

if (!isEffectorStore) {
return;
}

reportGetStateCall({ context, node, storeName: objectName });
}
// JavaScript-way
else {
const isEffectorStore = isStoreNameValid(objectName, context);
if (!isEffectorStore) {
return;
}

reportGetStateCall({ context, node, storeName: objectName });
}
reportGetStateCall({ context, node, storeName: storeNode.name });
},
};
},
Expand Down
6 changes: 2 additions & 4 deletions rules/no-watch/no-watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const {
traverseNestedObjectNode,
} = require("../../utils/traverse-nested-object-node");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { hasEffectorType } = require("../../utils/has-effector-type");
const { nodeTypeIs } = require("../../utils/node-type-is");

module.exports = {
meta: {
Expand Down Expand Up @@ -30,11 +30,9 @@ module.exports = {
'CallExpression[callee.property.name="watch"]'(node) {
const object = traverseNestedObjectNode(node.callee?.object);

const isEffectorUnit = hasEffectorType({
const isEffectorUnit = nodeTypeIs.unit({
node: object,
context,
typeNames: ["Effect", "Event", "Store"],
useInitializer: false,
});

if (!isEffectorUnit) {
Expand Down
11 changes: 11 additions & 0 deletions rules/strict-effect-handlers/examples/correct-only-async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
async function f2() {}

const finalFx = createEffect(async () => {
await f1();
await f2();
});

export { finalFx };
11 changes: 11 additions & 0 deletions rules/strict-effect-handlers/examples/correct-only-fx.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

const oneFx = createEffect();
const twoFx = createEffect();

const finalFx = createEffect(async () => {
await oneFx();
await twoFx();
});

export { finalFx };
34 changes: 34 additions & 0 deletions rules/strict-effect-handlers/examples/correct.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { createEffect } from "effector";

const oneFx = createEffect();
const twoFx = createEffect();

const onlyEffectsFx = createEffect(async () => {
await oneFx(1);
await twoFx(1);
});

async function f1() {}
async function f2() {}

const onlyFunctionsFx = createEffect(async () => {
await f1();
await f2();
});

async function justFunctionWithEffects() {
await oneFx(1);
await twoFx(1);
}

async function justFunctionWithFunctions() {
await f1();
await f2();
}

export {
onlyEffectsFx,
onlyFunctionsFx,
justFunctionWithEffects,
justFunctionWithFunctions,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async function () {
await f1();
await oneFx();
});

export { finalFx };
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async function () {
await f1();
await oneFx(1);
});

export { finalFx };
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async function handler() {
await f1();
await oneFx();
});

export { finalFx };
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async function handler() {
await f1();
await oneFx(1);
});

export { finalFx };
11 changes: 11 additions & 0 deletions rules/strict-effect-handlers/examples/incorrect-mix-async-fx.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async () => {
await f1();
await oneFx();
});

export { finalFx };
11 changes: 11 additions & 0 deletions rules/strict-effect-handlers/examples/incorrect-mix-async-fx.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

async function f1() {}
const oneFx = createEffect();

const finalFx = createEffect(async () => {
await f1();
await oneFx(1);
});

export { finalFx };