diff --git a/.changeset/shy-peaches-shine.md b/.changeset/shy-peaches-shine.md new file mode 100644 index 0000000..f1042bb --- /dev/null +++ b/.changeset/shy-peaches-shine.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-effector": minor +--- + +Improve `enforce-store-naming-convention` to enforce naming in binding contexts (destructuring & function parameters) diff --git a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.md b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.md index df3c51e..0cedc75 100644 --- a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.md +++ b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.md @@ -4,7 +4,9 @@ description: Enforce $ as a prefix/postfix for any Effector Store # effector/enforce-store-naming-convention -Enforcing naming conventions helps keep the codebase consistent, and reduces overhead when thinking about how to name a variable with store. Depending on the configuration your `Store`s should be distinguished by a prefix or a postfix `$`. Enforces the prefix convention by default. +Enforces Effector naming conventions to reduce code reading overhead by clearly and consistently marking all `Store`s with a `$` symbol across the codebase. + +Depending on rule configuration, `Store`s must be distinguished from other variables by a `$` prefix or postfix, with prefix enforcement as the default. ## Prefix convention @@ -21,9 +23,11 @@ const config = { ```ts // 👍 nice name const $name = createStore(null) +const { $x, $y } = useContext(PointModel) // 👎 bad name const name = createStore(null) +const { x, y } = useContext(PointModel) ``` ## Postfix convention @@ -43,7 +47,9 @@ Then, the postfix convention will be enforced: ```ts // 👍 nice name const name$ = createStore(null) +const { x$, y$ } = useContext(PointModel) // 👎 bad name -const name = createStrore(null) +const name = createStore(null) +const { x, y } = useContext(PointModel) ``` diff --git a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.test.ts b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.test.ts index 6f51a22..0dc4875 100644 --- a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.test.ts +++ b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.test.ts @@ -53,7 +53,7 @@ ruleTester.run("enforce-store-naming-convention", rule, { options: [postfix], }, { - name: "prefix: combine", + name: "prefix: mapping", code: ts` import { createStore, combine } from "effector" @@ -61,18 +61,10 @@ ruleTester.run("enforce-store-naming-convention", rule, { const $b = createStore(null) const $combined = combine($a, $b) + const $mapped = $a.map((value) => value === null) `, options: [prefix], }, - { - name: "prefix: map", - code: ts` - import { createStore } from "effector" - - const $store = createStore([]) - const $mapped = $store.map((values) => values.length) - `, - }, { name: "prefix: renamed", code: ts` @@ -108,14 +100,75 @@ ruleTester.run("enforce-store-naming-convention", rule, { options: [postfix], }, { - name: "store as argument", + name: "prefix: store as destructured argument", code: ts` import { type Store, createStore } from "effector" - type QueryParams = { body: Store } + type QueryParams = { $body: Store } + + const alpha = ({ $body = createStore({}) }: QueryParams) => undefined + const beta = ({ $body }: QueryParams) => undefined + const gamma = ($body = createStore(null)) => undefined + `, + }, + { + name: "prefix: shape destructuring -> ident", + code: ts` + import { createStore } from "effector" + + const { $first } = { $first: createStore(0) } + const { second: $second } = { second: createStore(0) } + `, + }, + { + name: "prefix: shape destructuring -> assignment", + code: ts` + import { createStore } from "effector" + + const { first: $first = createStore(0) } = { first: null } + const { second: $second = createStore(0) } = { second: createStore(1) } + `, + }, + { + name: "postfix: array destructuring -> ident", + code: ts` + import { createStore } from "effector" + + const [first$] = [createStore(0)] + const [second$, third$] = [combine($first, (x) => x), restore($first.updates, null)] + `, + options: [postfix], + }, + { + name: "postfix: array destructuring -> assignment", + code: ts` + import { createStore } from "effector" + + const [first$ = createStore(0)] = [null] + const [second$ = createStore(0)] = [createStore(1)] + `, + options: [postfix], + }, + { + name: "postfix: mixed nested destructuring -> assignment", + code: ts` + import { createStore } from "effector" + + const { + first: [second$ = createStore(0)], + } = { first: [null] } + `, + options: [postfix], + }, + { + name: "property in argument context", + code: ts` + import { createStore, combine } from "effector" + + const $source = createStore(0) - const createQuery = ({ body = createStore({}) }: QueryParams) => undefined - const createMutation = (body = createStore(null)) => undefined + const $combined = combine({ alpha: $source, beta: [$source], gamma: { delta: $source } }) + const grouped = { alpha: createStore(0), beta: combine($source, (x) => x) } `, }, ], @@ -174,6 +227,15 @@ ruleTester.run("enforce-store-naming-convention", rule, { }, ], }, + { + name: "variable with type annotation", + code: ts` + import { createStore, type Store } from "effector" + + const store: Store = createStore(null) + `, + errors: [{ messageId: "invalid", line: 3, data: { current: "store", convention: "prefix", fixed: "$store" } }], + }, { name: "prefix when configured for postfix", code: ts` @@ -185,6 +247,7 @@ ruleTester.run("enforce-store-naming-convention", rule, { { messageId: "invalid", line: 2, + data: { current: "$just", convention: "postfix", fixed: "just$" }, suggestions: [ { messageId: "rename", @@ -226,5 +289,177 @@ ruleTester.run("enforce-store-naming-convention", rule, { }, ], }, + { + name: "shape destructuring", + code: ts` + import { createStore } from "effector" + + const { first } = { first: createStore(0) } + const { $first: first } = { $first: createStore(0) } + `, + errors: [ + { + messageId: "invalid", + line: 3, + suggestions: [ + { + messageId: "rename", + data: { current: "first", fixed: "$first" }, + output: ts` + import { createStore } from "effector" + + const { first: $first } = { first: createStore(0) } + const { $first: first } = { $first: createStore(0) } + `, + }, + ], + }, + { + messageId: "invalid", + line: 4, + suggestions: [ + { + messageId: "rename", + data: { current: "first", fixed: "$first" }, + output: ts` + import { createStore } from "effector" + + const { first } = { first: createStore(0) } + const { $first: $first } = { $first: createStore(0) } + `, + }, + ], + }, + ], + }, + { + name: "shape destructuring with assignment", + code: ts` + import { createStore, combine } from "effector" + + const $source = createStore(0) + + const { first = createStore(0) } = { first: $source } + const { second: beta = combine($source, (x) => x) } = { second: $source } + `, + errors: [ + { + messageId: "invalid", + line: 5, + suggestions: [ + { + messageId: "rename", + data: { current: "first", fixed: "$first" }, + output: ts` + import { createStore, combine } from "effector" + + const $source = createStore(0) + + const { first: $first = createStore(0) } = { first: $source } + const { second: beta = combine($source, (x) => x) } = { second: $source } + `, + }, + ], + }, + { + messageId: "invalid", + line: 6, + suggestions: [ + { + messageId: "rename", + data: { current: "beta", fixed: "$beta" }, + output: ts` + import { createStore, combine } from "effector" + + const $source = createStore(0) + + const { first = createStore(0) } = { first: $source } + const { second: $beta = combine($source, (x) => x) } = { second: $source } + `, + }, + ], + }, + ], + }, + { + name: "array destructuring", + code: ts` + import { createStore } from "effector" + + const [first, second = createStore(0)] = [createStore(0)] + `, + errors: [ + { + messageId: "invalid", + line: 3, + suggestions: [ + { + messageId: "rename", + data: { current: "first", fixed: "$first" }, + output: ts` + import { createStore } from "effector" + + const [$first, second = createStore(0)] = [createStore(0)] + `, + }, + ], + }, + { + messageId: "invalid", + line: 3, + suggestions: [ + { + messageId: "rename", + data: { current: "second", fixed: "$second" }, + output: ts` + import { createStore } from "effector" + + const [first, $second = createStore(0)] = [createStore(0)] + `, + }, + ], + }, + ], + }, + { + name: "function parameter nested inferred destructuring", + code: ts` + import { type Store } from "effector" + + type Config = { store: Store } + function test({ config: { store } }: { config: Config }) {} + `, + errors: [ + { + messageId: "invalid", + line: 4, + suggestions: [ + { + messageId: "rename", + data: { current: "store", fixed: "$store" }, + output: ts` + import { type Store } from "effector" + + type Config = { store: Store } + function test({ config: { store: $store } }: { config: Config }) {} + `, + }, + ], + }, + ], + }, + { + name: "function parameter inferred", + code: ts` + import { type Store, type StoreWritable } from "effector" + + function alpha(store: Store) {} + const beta = (store: StoreWritable) => {} + `, + errors: [ + { messageId: "invalid", line: 3, data: { current: "store", convention: "prefix", fixed: "$store" } }, + { messageId: "invalid", line: 4, data: { current: "store", convention: "prefix", fixed: "$store" } }, + ], + }, ], }) diff --git a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.ts b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.ts index 66880d3..afd41e8 100644 --- a/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.ts +++ b/src/rules/enforce-store-naming-convention/enforce-store-naming-convention.ts @@ -1,9 +1,13 @@ -import { ESLintUtils, type TSESTree as Node, type TSESLint } from "@typescript-eslint/utils" +import { ESLintUtils, TSESTree as Node, AST_NODE_TYPES as NodeType, type TSESLint } from "@typescript-eslint/utils" import { createRule } from "@/shared/create" import { isType } from "@/shared/is" type Options = { mode: "prefix" | "postfix" } +type Suggestion = TSESLint.SuggestionReportDescriptor<"invalid" | "rename"> + +type ShapeProperty = Node.Property & + ({ value: Node.Identifier } | { value: Node.AssignmentPattern & { left: Node.Identifier } }) export default createRule<[Options], "invalid" | "rename">({ name: "enforce-store-naming-convention", @@ -16,37 +20,71 @@ export default createRule<[Options], "invalid" | "rename">({ invalid: 'Store "{{ current }}" should be named with a `$` {{ convention }}, rename it to "{{ fixed }}"', rename: 'Rename "{{ current }}" to "{{ fixed }}"', }, - schema: [{ type: "object", properties: { mode: { type: "string", enum: ["prefix", "postfix"] } } }], + schema: [ + { + type: "object", + properties: { mode: { type: "string", enum: ["prefix", "postfix"] } }, + }, + ], hasSuggestions: true, }, defaultOptions: [{ mode: "prefix" }], create: (context, [options]) => { const services = ESLintUtils.getParserServices(context) - type VariableDeclarator = Node.VariableDeclarator & { id: Node.Identifier } + const selector = createSelector(options.mode === "prefix" ? PrefixRegex : PostfixRegex) - const regex = options.mode === "prefix" ? PrefixRegex : PostfixRegex + const rename = (node: Node.Identifier) => { + const trimmed = node.name.replace(options.mode === "prefix" ? /\$+$/g : /^\$+/g, "") + const fixed = options.mode === "prefix" ? `$${trimmed}` : `${trimmed}$` + + return { current: node.name, convention: options.mode, fixed } + } return { - [`VariableDeclarator[id.name=${regex}]`]: (node: VariableDeclarator) => { - const type = services.getTypeAtLocation(node) + [`${selector.variable}, ${selector.array.identifier}, ${selector.array.assignment}, ${selector.function.identifier}, ${selector.function.assignment}`]: + (node: Node.Identifier) => { + const type = services.getTypeAtLocation(node) + + const isStore = isType.store(type, services.program) + if (!isStore) return + + const data = rename(node) + + // type annotation is included `range` so we can't reliably replace text without erasing the annotation + if (node.typeAnnotation) return context.report({ node, messageId: "invalid", data }) + + const suggestion: Suggestion = { + messageId: "rename", + data: { current: node.name, fixed: data.fixed }, + fix: (fixer) => fixer.replaceText(node, data.fixed), + } + + context.report({ node, messageId: "invalid", data, suggest: [suggestion] }) + }, + + [`${selector.shape.identifier}, ${selector.shape.assignment}`]: (node: ShapeProperty) => { + const type = services.getTypeAtLocation(node.value) + const ident = node.value.type === NodeType.Identifier ? node.value : node.value.left const isStore = isType.store(type, services.program) if (!isStore) return - const current = node.id.name - const trimmed = current.replaceAll(options.mode === "prefix" ? /\$+$/g : /^\$+/g, "") - const fixed = options.mode === "prefix" ? `$${trimmed}` : `${trimmed}$` + const data = rename(ident) - const data = { current, convention: options.mode, fixed } + // type annotation is included `range` so we can't reliably replace text without erasing the annotation + if (ident.typeAnnotation) return context.report({ node: ident, messageId: "invalid", data }) - const suggestion = { - messageId: "rename" as const, - data: { current, fixed }, - fix: (fixer: TSESLint.RuleFixer) => fixer.replaceText(node.id, fixed), + const suggestion: Suggestion = { + messageId: "rename", + data: { current: ident.name, fixed: data.fixed }, + fix: (fixer) => + node.shorthand + ? fixer.insertTextAfter(node.key, `: ${data.fixed}`) // { x } -> { x: $x } + : fixer.replaceText(ident, data.fixed), // { x: y } -> { x: $y } } - context.report({ node: node.id, messageId: "invalid", data, suggest: [suggestion] }) + context.report({ node: ident, messageId: "invalid", data, suggest: [suggestion] }) }, } }, @@ -54,3 +92,19 @@ export default createRule<[Options], "invalid" | "rename">({ const PrefixRegex = /^[^$]/ const PostfixRegex = /[^$]$/ + +const createSelector = (regex: RegExp) => ({ + variable: `VariableDeclarator > Identifier.id[name=${regex}]`, + array: { + identifier: `ArrayPattern > Identifier.elements[name=${regex}]`, + assignment: `ArrayPattern > AssignmentPattern > Identifier.left[name=${regex}]`, + }, + shape: { + identifier: `ObjectPattern > Property:has(> Identifier.value[name=${regex}])`, + assignment: `ObjectPattern > Property:has(> AssignmentPattern:has(> Identifier.left[name=${regex}]))`, + }, + function: { + identifier: `:function > Identifier.params[name=${regex}]`, + assignment: `:function > AssignmentPattern > Identifier.left[name=${regex}]`, + }, +}) diff --git a/src/shared/is.ts b/src/shared/is.ts index f20c781..4dcd70c 100644 --- a/src/shared/is.ts +++ b/src/shared/is.ts @@ -23,14 +23,14 @@ export const isType = { effect: (type: Type, program: Program) => typeMatchesSpecifier(type, { from: "package", package: "effector", name: "Effect" }, program), + domain: (type: Type, program: Program) => + typeMatchesSpecifier(type, { from: "package", package: "effector", name: "Domain" }, program), + unit: (type: Type, program: Program) => { const name = ["Store", "StoreWritable", "Event", "EventCallable", "Effect", "Domain"] return typeMatchesSpecifier(type, { from: "package", package: "effector", name }, program) }, - domain: (type: Type, program: Program) => - typeMatchesSpecifier(type, { from: "package", package: "effector", name: "Domain" }, program), - // Gate is an intersection type, which TypeScript erases from existence gate: (type: Type) => { const symbol = type.getSymbol() ?? type.aliasSymbol