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 keep-options-order #81

Merged
merged 8 commits into from
Dec 25, 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 @@ -67,6 +67,7 @@ This preset is recommended for projects that use [React](https://reactjs.org) wi
- [effector/no-watch](rules/no-watch/no-watch.md)
- [effector/prefer-sample-over-forward-with-mapping](rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping.md)
- [effector/strict-effect-handlers](rules/strict-effect-handlers/strict-effect-handlers.md)
- [effector/keep-options-order](rules/keep-options-order/keep-options-order.md)

## Maintenance

Expand Down
1 change: 1 addition & 0 deletions config/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ module.exports = {
"effector/no-watch": "warn",
"effector/no-unnecessary-combination": "warn",
"effector/no-duplicate-on": "error",
"effector/keep-options-order": "warn",
},
};
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = {
"no-duplicate-on": require("./rules/no-duplicate-on/no-duplicate-on"),
"strict-effect-handlers": require("./rules/strict-effect-handlers/strict-effect-handlers"),
"enforce-gate-naming-convention": require("./rules/enforce-gate-naming-convention/enforce-gate-naming-convention"),
"keep-options-order": require("./rules/keep-options-order/keep-options-order"),
},
configs: {
recommended: require("./config/recommended"),
Expand Down
3 changes: 3 additions & 0 deletions rules/keep-options-order/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const correctOrder = ["clock", "source", "filter", "fn", "target"];

module.exports = { correctOrder };
22 changes: 22 additions & 0 deletions rules/keep-options-order/examples/correct-guard.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { createEvent, createStore, guard } from "effector";

const clock = createEvent();
const source = createEvent();
const filter = createStore();
const target = createEvent();

guard({ clock, source, filter, target });

guard({ clock, source, filter });
guard({ clock, source, filter, target });
guard({ clock, source, target });
guard({ clock, filter, target });
guard({ source, filter, target });

guard({ clock, source });

guard({ clock, filter });
guard({ clock, filter, target });

guard({ filter, target });
guard({ filter });
28 changes: 28 additions & 0 deletions rules/keep-options-order/examples/correct-sample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { createEvent, createStore, sample } from "effector";

const clock = createEvent();
const source = createEvent();
const filter = createStore();
const fn = () => null;
const target = createEvent();

sample({ clock, source, filter, fn, target });

sample({ clock, source, filter, fn });
sample({ clock, source, filter, target });
sample({ clock, source, fn, target });
sample({ clock, filter, fn, target });
sample({ source, filter, fn, target });

sample({ clock, source, filter });
sample({ clock, source, fn });
sample({ clock, source, target });

sample({ clock, filter, fn });
sample({ clock, filter, target });

sample({ source, filter, fn });
sample({ source, filter, target });

sample({ filter, fn, target });
sample({ filter, fn });
8 changes: 8 additions & 0 deletions rules/keep-options-order/examples/incorrect-guard.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { createEvent, createStore, guard } from "effector";

const clock = createEvent();
const source = createEvent();
const filter = createStore();
const target = createEvent();

guard({ filter, clock, source, target });
9 changes: 9 additions & 0 deletions rules/keep-options-order/examples/incorrect-sample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createEvent, createStore, sample } from "effector";

const clock = createEvent();
const source = createEvent();
const filter = createStore();
const fn = () => null;
const target = createEvent();

sample({ source, clock, filter, fn, target });
107 changes: 107 additions & 0 deletions rules/keep-options-order/keep-options-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
const { extractImportedFrom } = require("../../utils/extract-imported-from");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { buildObjectInText } = require("../../utils/builders");

const { correctOrder } = require("./config");

module.exports = {
meta: {
type: "problem",
docs: {
description: "Enforce options order for Effector methods",
category: "Style",
recommended: true,
url: createLinkToRule("keep-options-order"),
},
messages: {
invalidOrder: `Order of options should be ${makeTag(
correctOrder
)}, but found {{ incorrectOrderTag }}`,
changeOrder: "Sort options",
},
schema: [],
hasSuggestions: true,
},
create(context) {
const importedFromEffector = new Map();
return {
ImportDeclaration(node) {
extractImportedFrom({
importMap: importedFromEffector,
node,
packageName: "effector",
});
},
CallExpression(node) {
// Effect creation with method
const OPTIONS_METHODS = ["sample", "guard"];
for (const method of OPTIONS_METHODS) {
const localMethod = importedFromEffector.get(method);
if (!localMethod) {
continue;
}

const isEffectorMethods = node.callee.name === localMethod;
if (!isEffectorMethods) {
continue;
}

const configNode = node?.arguments?.[0];
const optionsNodes = configNode?.properties;

const optionsOrder = optionsNodes?.map((prop) => prop?.key.name);

const idealOrder = filteredCorrectOrder(optionsOrder);

if (isCorrectOrder(optionsOrder, idealOrder)) {
continue;
}

context.report({
node,
messageId: "invalidOrder",
data: {
incorrectOrderTag: makeTag(optionsOrder),
},
suggest: [
{
messageId: "changeOrder",
fix(fixer) {
const newConfig = buildObjectInText({
context,
properties: sortNodesByName(optionsNodes, idealOrder),
});

return fixer.replaceText(configNode, newConfig);
},
},
],
});
}
},
};
},
};

function makeTag(order) {
return order.join("->");
}

function isCorrectOrder(checkOrder, idealOrder) {
return idealOrder.every((refItem, index) => checkOrder?.[index] === refItem);
}

function filteredCorrectOrder(checkOrder) {
return correctOrder.filter((item) => checkOrder?.includes(item));
}

function sortNodesByName(nodes, nameOrder) {
const newNodes = [];

for (const name of nameOrder) {
const node = nodes.find((node) => node?.key.name === name);
newNodes.push(node);
}

return newNodes;
}
21 changes: 21 additions & 0 deletions rules/keep-options-order/keep-options-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# effector/keep-options-order

Some of Effector-methods (e.g., `sample` and `guard`) accept config in object form. This form can be read as "when `clock` is triiered, take data from `source` pass it thru `filter`/`fn` and send to `target`". So, it is better to use semantic order of configuration properties — `clock -> source -> filter/fn -> target`. The rule enforces this order for any case.

```ts
// 👍 great
sample({
clock: formSubmit,
source: $formData,
fn: prepareData,
target: sendFormToServerFx,
});

// 👎 weird
sample({
fn: prepareData,
target: sendFormToServerFx,
clock: formSubmit,
source: $formData,
});
```
116 changes: 116 additions & 0 deletions rules/keep-options-order/keep-options-order.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
const { RuleTester } = require("eslint");

const { readExample } = require("../../utils/read-example");

const rule = require("./keep-options-order");

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2020,
sourceType: "module",
},
});

const readExampleForTheRule = (name) => readExample(__dirname, name);

ruleTester.run("effector/keep-options-order.test", rule, {
valid: ["correct-sample.js", "correct-guard.js"]
.map(readExampleForTheRule)
.map((code) => ({ code })),

invalid: [
// Errors
...["incorrect-sample.js", "incorrect-guard.js"]
.map(readExampleForTheRule)
.map((code) => ({
code,
errors: [
{
messageId: "invalidOrder",
type: "CallExpression",
},
],
})),
// Suggestions
{
code: `
import {sample} from 'effector';
sample({ source, clock, fn, target });
`,
errors: [
{
messageId: "invalidOrder",
suggestions: [
{
messageId: "changeOrder",
output: `
import {sample} from 'effector';
sample({ clock, source, fn, target });
`,
},
],
},
],
},
{
code: `
import {sample} from 'effector';
sample({ fn() { return null }, clock, target });
`,
errors: [
{
messageId: "invalidOrder",
suggestions: [
{
messageId: "changeOrder",
output: `
import {sample} from 'effector';
sample({ clock, fn() { return null }, target });
`,
},
],
},
],
},
{
code: `
import {sample} from 'effector';
sample({ fn, clock: event.map(() => null), target });
`,
errors: [
{
messageId: "invalidOrder",
suggestions: [
{
messageId: "changeOrder",
output: `
import {sample} from 'effector';
sample({ clock: event.map(() => null), fn, target });
`,
},
],
},
],
},
{
code: `
import {sample} from 'effector';
sample({ source: combine({ a: $a }), clock, target });
`,
errors: [
{
messageId: "invalidOrder",
suggestions: [
{
messageId: "changeOrder",
output: `
import {sample} from 'effector';
sample({ clock, source: combine({ a: $a }), target });
`,
},
],
},
],
},
],
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { extractImportedFrom } = require("../../utils/extract-imported-from");
const { areNodesSameInText } = require("../../utils/are-nodes-same-in-text");
const { createLinkToRule } = require("../../utils/create-link-to-rule");
const { buildObjectInText } = require("../../utils/builders");

module.exports = {
meta: {
Expand Down Expand Up @@ -86,11 +87,8 @@ function reportUnnecessaryDuplication({
const properties = objectNode?.properties?.filter?.(
(p) => p !== paramToExcludeNode
);
const newPropertiesText = properties
.map((p) => context.getSourceCode().getText(p))
.join(", ");

return `{ ${newPropertiesText} }`;
return buildObjectInText({ properties, context });
}

context.report({
Expand Down
9 changes: 9 additions & 0 deletions utils/builders.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function buildObjectInText({ properties, context }) {
const content = properties
.map((property) => context.getSourceCode().getText(property))
.join(", ");

return `{ ${content} }`;
}

module.exports = { buildObjectInText };