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

feat(eslint-plugin): complete unit-naming-rule (#523) #920

Merged
merged 12 commits into from
Sep 12, 2024
56 changes: 36 additions & 20 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,17 @@ Here is an example of React + TypeScript + Prettier config with Reatom.
"prettier/prettier": "error"
},
"settings": {
"atomPostfix": "Atom"
"atomSuffix": "Atom"
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
}
}
```

## Rules

### `async-rule`

Ensures that asynchronous interactions within Reatom functions are wrapped with `ctx.schedule`. Read [the docs](https://www.reatom.dev/package/core/#ctx-api) for more info.

### `unit-naming-rule`

Ensures that all Reatom entities specify the name parameter used for debugging. We assume that Reatom entity factories are `atom`, `action` and all `reatom*` (like `reatomAsync`) functions imported from `@reatom/*` packages.
Expand All @@ -75,22 +79,33 @@ The name must be equal to the name of a variable or a property an entity is assi

```ts
const count = atom(0, 'count')
```

When atom is assigned to a property of an object assigned to a variable, variable name must be specified before a period (`.`):

const someNamespace = {
count: atom(0, 'count'),
```ts
const atomsRec = {
count: atom(0, 'atomsRec.count'),
}
```

When creating atoms dynamically with factories, you can also specify the "namespace" of the name before the `.` symbol:
When creating units within `reatom*`-named factory functions, you can also specify the "namespaces" of unit names before period. The fragment before period is called the domain. Domain value must be equal to the name of the factory function excluding the `reatom` prefix:

```ts
const reatomFood = (config: {
name: string
calories: number
fat: number
carbs: number
protein: number
}) => {
const reatomFood = (config: { name: string; calories: number; fat: number; carbs: number; protein: number }) => {
const { name } = config.name
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
const calories = atom(config.calories, `Food.calories`)
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
const fat = atom(config.fat, `Food.fat`)
const carbs = atom(config.carbs, `Food.carbs`)
const protein = atom(config.protein, `Food.protein`)
return { calories, fat, carbs, protein }
}
```

If a factory function defines a parameter or a variable named `name`, names of units created in the function must be template literals that derive their domain fragments from the value of `name`:

```ts
const reatomFood = (config: { name: string; calories: number; fat: number; carbs: number; protein: number }) => {
const { name } = config.name
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
const calories = atom(config.calories, `${name}.calories`)
const fat = atom(config.fat, `${name}.fat`)
Expand All @@ -100,27 +115,28 @@ const reatomFood = (config: {
}
```

If there is an identifier `name` defined in the function scope, unit names must use it as namespace. Otherwise, namespace must be equal to the name of the factory function.
Object and domain fragments in names may be used together:

```ts
atom(0, `${name}.atomsRec.field`)
atom(0, 'Some.atomsRec.field')
```

For private atoms, `_` prefix can be used:
You may prefix unit names with `_` to indicate that they are not exposed from factories that create them (this makes Reatom inspector hide them):

```ts
const secretState = atom(0, '_secretState')
```

You can also ensure that `atom` names have a prefix or a postfix through the configuration, for example:
You can also ensure prefixes and suffixes for `atom` names through the configuration:

```ts
{
;({
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
atomPrefix: '',
atomPostfix: 'Atom',
}
})
```

### `async-rule`

Ensures that asynchronous interactions within Reatom functions are wrapped with `ctx.schedule`. Read [the docs](https://www.reatom.dev/package/core/#ctx-api) for more info.

## Motivation

The primary purpose of this plugin is to automate generation of atom and action names using ESLint autofixes. Many have asked why not make a Babel plugin for naming, why keep it in source, here is our opinion:
Expand Down
19 changes: 10 additions & 9 deletions packages/eslint-plugin/src/rules/async-rule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ const tester = new RuleTester({
},
})

const ImportReatomAsync = 'import {reatomAsync} from "@reatom/framework"'
const ImportReatomAsyncAlias =
'import {reatomAsync as createAsync} from "@reatom/framework"'

tester.run('async-rule', asyncRule, {
valid: [
`${ImportReatomAsync}; const reatomSome = reatomAsync(async ctx => await ctx.schedule(() => someEffect()))`,
`${ImportReatomAsyncAlias}; const reatomSome = createAsync(async ctx => await ctx.schedule(() => someEffect()))`,
`reatomAsync(async ctx => await ctx.schedule(() => someEffect()))`,
`reatomAsync(async ctx => { foo(bar(await ctx.schedule(() => someEffect()))) })`,
],
invalid: [
{
code: `${ImportReatomAsync}; const reatomSome = reatomAsync(async ctx => await someEffect())`,
errors: [{ messageId: 'scheduleMissing' }],
output: `${ImportReatomAsync}; const reatomSome = reatomAsync(async ctx => await ctx.schedule(() => someEffect()))`,
code: `reatomAsync(async ctx => await someEffect())`,
errors: [{ message: /`ctx.schedule` is missing/ }],
output: `reatomAsync(async ctx => await ctx.schedule(() => someEffect()))`,
},
{
code: `reatomAsync(async ctx => { foo(bar(await someEffect())) })`,
errors: [{ message: /`ctx.schedule` is missing/ }],
output: `reatomAsync(async ctx => { foo(bar(await ctx.schedule(() => someEffect()))) })`,
},
],
})
105 changes: 42 additions & 63 deletions packages/eslint-plugin/src/rules/async-rule.ts
Original file line number Diff line number Diff line change
@@ -1,63 +1,42 @@
import type { Rule } from 'eslint'
import type * as estree from 'estree'
import { ascend, createImportMap, isReatomFactoryName } from '../shared'

const ReatomFactoryPrefix = 'reatom'

export const asyncRule: Rule.RuleModule = {
meta: {
type: 'suggestion',
docs: {
recommended: true,
description:
'Ensures that asynchronous interactions within Reatom functions are wrapped with `ctx.schedule`.',
},
messages: {
scheduleMissing:
'Asynchronous interactions within Reatom functions should be wrapped with `ctx.schedule`',
},
fixable: 'code',
},
create(context: Rule.RuleContext): Rule.RuleListener {
const imports = createImportMap('@reatom')

return {
ImportDeclaration: imports.onImportNode,
AwaitExpression(node) {
const fn = ascend(node, 'ArrowFunctionExpression', 'FunctionExpression')
if (!fn) return

if (fn.parent.type !== 'CallExpression') return
if (fn.parent.callee.type !== 'Identifier') return
if (!isReatomFactoryName(fn.parent.callee.name)) return

if (isCtxSchedule(node.argument)) return

context.report({
node,
messageId: 'scheduleMissing',
fix: (fixer) => wrapScheduleFix(fixer, node),
})
},
}
},
}

const isCtxSchedule = (node: estree.Node) => {
return (
node.type === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'Identifier' &&
node.callee.object.name === 'ctx' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'schedule'
)
}

const wrapScheduleFix = (
fixer: Rule.RuleFixer,
node: estree.AwaitExpression,
) => [
fixer.insertTextBefore(node.argument, 'ctx.schedule(() => '),
fixer.insertTextAfter(node.argument, ')'),
]
import type { Rule } from 'eslint'
import type * as estree from 'estree'
import { reatomFactoryPattern } from '../shared'

export const asyncRule: Rule.RuleModule = {
meta: {
type: 'suggestion',
docs: {
recommended: true,
description: 'Ensures that asynchronous interactions within Reatom functions are wrapped with `ctx.schedule`.',
},
fixable: 'code',
},
create(context: Rule.RuleContext): Rule.RuleListener {
return {
[`CallExpression[callee.name=${reatomFactoryPattern}] > :matches(ArrowFunctionExpression[async=true], FunctionExpression[async=true]) AwaitExpression`](
node: estree.AwaitExpression,
) {
const arg = node.argument
if (
arg.type === 'CallExpression' &&
arg.callee.type === 'MemberExpression' &&
arg.callee.object.type === 'Identifier' &&
arg.callee.object.name === 'ctx' &&
arg.callee.property.type === 'Identifier' &&
arg.callee.property.name === 'schedule'
) {
return
}

context.report({
node,
message: '`ctx.schedule` is missing in an await expression within a Reatom-wrapped function',
fix: (fixer) => [
fixer.insertTextBefore(node.argument, 'ctx.schedule(() => '),
fixer.insertTextAfter(node.argument, ')'),
],
})
},
}
},
}
82 changes: 53 additions & 29 deletions packages/eslint-plugin/src/rules/unit-naming-rule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,61 +8,85 @@ const tester = new RuleTester({
},
})

const ImportAtom = 'import {atom} from "@reatom/framework"'

tester.run('unit-naming-rule', unitNamingRule, {
valid: [
`${ImportAtom}; const some = atom(0, 'some')`,
`const some = atom(0, '_some')`,
`const some = action(0, 'some')`,
{
code: `${ImportAtom}; const $some = atom(0, '$some')`,
code: `const $some = atom(0, '$some')`,
options: [{ atomPrefix: '$' }],
},
{
code: `${ImportAtom}; const someAtom = atom(0, 'someAtom')`,
code: `const someAtom = atom(0, '_someAtom')`,
options: [{ atomPostfix: 'Atom' }],
},
{
code: `function reatomSome() { const someAtom = atom(0, 'Some.someAtom') }`,
},
{
code: `const Atoms = { someAtom: atom(0, 'Atoms.someAtom') }`,
},
{
code: `function reatomSome() { const Atoms = { someAtom: atom(0, 'Some.Atoms.someAtom') } }`,
},
],
invalid: [
{
code: `${ImportAtom}; const some = atom(0)`,
errors: [{ messageId: 'nameMissing' }],
output: `${ImportAtom}; const some = atom(0, 'some')`,
code: `const some = atom(0)`,
errors: [{ message: /missing/ }],
output: `const some = atom(0, 'some')`,
},
{
code: `${ImportAtom}; const some = atom(0, 'unrelated')`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; const some = atom(0, 'some')`,
code: `const some = atom(0, lololo)`,
errors: [{ message: /must be a correctly formatted string literal/ }],
output: `const some = atom(0, 'some')`,
},
{
code: `${ImportAtom}; const some = atom(0, 'some')`,
code: `const some = atom(0, 'unrelated')`,
errors: [{ message: /name must be/ }],
output: `const some = atom(0, 'some')`,
},
{
code: `const some = atom(0, 'some')`,
options: [{ atomPrefix: '$' }],
errors: [{ message: /name must start with/ }],
output: `const $some = atom(0, '$some')`,
},
{
code: `const some = atom(0, 'some')`,
options: [{ atomPostfix: 'Atom' }],
errors: [{ messageId: 'postfixMissing' }],
output: `${ImportAtom}; const someAtom = atom(0, 'someAtom')`,
errors: [{ message: /name must end with/ }],
output: `const someAtom = atom(0, 'someAtom')`,
},
{
code: `function reatomSome() { const field = atom(0, 'Some._unrelated'); }`,
errors: [{ message: /name must be/ }],
output: `function reatomSome() { const field = atom(0, 'Some._field'); }`,
},
{
code: `${ImportAtom}; function reatomSome() { const field = atom(0, 'reatomSome._unrelated'); }`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; function reatomSome() { const field = atom(0, 'reatomSome._field'); }`,
code: `const some = atom(0, 'Some.some')`,
errors: [{ message: /must have no domain/ }],
output: `const some = atom(0, 'some')`,
},
{
code: `${ImportAtom}; function reatomSome() { const field = atom(0, 'field') }`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; function reatomSome() { const field = atom(0, 'reatomSome.field') }`,
code: `function reatomSome() { const field = atom(0, 'field') }`,
errors: [{ message: /domain must be/ }],
output: `function reatomSome() { const field = atom(0, 'Some.field') }`,
},
{
code: `${ImportAtom}; function reatomSome() { const field = atom(0, 'Some.field') }`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; function reatomSome() { const field = atom(0, 'reatomSome.field') }`,
code: `function reatomSome() { const field = atom(0, 'Lololo.field') }`,
errors: [{ message: /domain must be/ }],
output: `function reatomSome() { const field = atom(0, 'Some.field') }`,
},
{
code: `${ImportAtom}; function reatomSome({name}) { const field = atom(0, 'field'); }`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; function reatomSome({name}) { const field = atom(0, \`\${name}.field\`); }`,
code: `function reatomSome({name}) { const field = atom(0, 'field'); }`,
errors: [{ message: /domain must be derived from/ }],
output: `function reatomSome({name}) { const field = atom(0, \`\${name}.field\`); }`,
},
{
code: `${ImportAtom}; function reatomSome({name}) { const field = atom(0, 'Some.field'); }`,
errors: [{ messageId: 'nameIncorrect' }],
output: `${ImportAtom}; function reatomSome({name}) { const field = atom(0, \`\${name}.field\`); }`,
code: `function reatomSome({name}) { const field = atom(0, 'Some.field'); }`,
errors: [{ message: /domain must be derived from/ }],
output: `function reatomSome({name}) { const field = atom(0, \`\${name}.field\`); }`,
de-jabber marked this conversation as resolved.
Show resolved Hide resolved
},
],
})
Loading