diff --git a/.changeset/gentle-bugs-stare.md b/.changeset/gentle-bugs-stare.md new file mode 100644 index 000000000..e207c8196 --- /dev/null +++ b/.changeset/gentle-bugs-stare.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-svelte": minor +--- + +feat: add `svelte/derived-has-same-inputs-outputs` rule diff --git a/README.md b/README.md index 9f78407f7..d14be6352 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,7 @@ These rules relate to better ways of doing things to help you avoid problems: | Rule ID | Description | | |:--------|:------------|:---| | [svelte/button-has-type](https://ota-meshi.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | | +| [svelte/derived-has-same-inputs-outputs](https://ota-meshi.github.io/eslint-plugin-svelte/rules/derived-has-same-inputs-outputs/) | derived store should use same variable names between values and callback | | | [svelte/no-at-debug-tags](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: | | [svelte/no-reactive-functions](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: | | [svelte/no-reactive-literals](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: | diff --git a/docs/rules.md b/docs/rules.md index 1bf79427c..a335ce2b9 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -42,6 +42,7 @@ These rules relate to better ways of doing things to help you avoid problems: | Rule ID | Description | | |:--------|:------------|:---| | [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | | +| [svelte/derived-has-same-inputs-outputs](./rules/derived-has-same-inputs-outputs.md) | derived store should use same variable names between values and callback | | | [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: | | [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: | | [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: | diff --git a/docs/rules/derived-has-same-inputs-outputs.md b/docs/rules/derived-has-same-inputs-outputs.md new file mode 100644 index 000000000..e63e488c0 --- /dev/null +++ b/docs/rules/derived-has-same-inputs-outputs.md @@ -0,0 +1,48 @@ +--- +pageClass: "rule-details" +sidebarDepth: 0 +title: "svelte/derived-has-same-inputs-outputs" +description: "derived store should use same variable names between values and callback" +--- + +# svelte/derived-has-same-inputs-outputs + +> derived store should use same variable names between values and callback + +## :book: Rule Details + +This rule reports where variable names and callback function's argument names are different. +This is mainly a recommended rule to avoid implementation confusion. + + + + + +```js +/* eslint svelte/derived-has-same-inputs-outputs: "error" */ + +import { derived } from "svelte/store" + +/* ✓ GOOD */ +derived(a, ($a) => {}); +derived(a, ($a, set) => {}) +derived([ a, b ], ([ $a, $b ]) => {}) + +/* ✗ BAD */ +derived(a, (b) => {}); +derived(a, (b, set) => {}); +derived([ a, b ], ([ one, two ]) => {}) +``` + + + +## :wrench: Options + +Nothing. + + +## :books: Further Reading + +- [Svelte - Docs > RUN TIME > svelte/store > derived](https://svelte.dev/docs#run-time-svelte-store-derived) + + diff --git a/src/rules/derived-has-same-inputs-outputs.ts b/src/rules/derived-has-same-inputs-outputs.ts new file mode 100644 index 000000000..1d9a7a328 --- /dev/null +++ b/src/rules/derived-has-same-inputs-outputs.ts @@ -0,0 +1,107 @@ +import type * as ESTree from "estree" +import { createRule } from "../utils" +import type { RuleContext } from "../types" +import { extractStoreReferences } from "./reference-helpers/svelte-store" + +export default createRule("derived-has-same-inputs-outputs", { + meta: { + docs: { + description: + "derived store should use same variable names between values and callback", + category: "Stylistic Issues", + recommended: false, + conflictWithPrettier: false, + }, + schema: [], + messages: { + unexpected: "The argument name should be '{{name}}'.", + }, + type: "suggestion", + }, + create(context) { + /** check node type */ + function isIdentifierOrArrayExpression( + node: ESTree.SpreadElement | ESTree.Expression, + ): node is ESTree.Identifier | ESTree.ArrayExpression { + return ["Identifier", "ArrayExpression"].includes(node.type) + } + + type ArrowFunctionExpressionOrFunctionExpression = + | ESTree.ArrowFunctionExpression + | ESTree.FunctionExpression + + /** check node type */ + function isFunctionExpression( + node: ESTree.SpreadElement | ESTree.Expression, + ): node is ArrowFunctionExpressionOrFunctionExpression { + return ["ArrowFunctionExpression", "FunctionExpression"].includes( + node.type, + ) + } + + /** + * Check for identifier type. + * e.g. derived(a, ($a) => {}); + */ + function checkIdentifier( + context: RuleContext, + args: ESTree.Identifier, + fn: ArrowFunctionExpressionOrFunctionExpression, + ) { + const fnParam = fn.params[0] + if (fnParam.type !== "Identifier") return + const expectedName = `$${args.name}` + if (expectedName !== fnParam.name) { + context.report({ + node: fn, + loc: fnParam.loc!, + messageId: "unexpected", + data: { name: expectedName }, + }) + } + } + + /** + * Check for array type. + * e.g. derived([ a, b ], ([ $a, $b ]) => {}) + */ + function checkArrayExpression( + context: RuleContext, + args: ESTree.ArrayExpression, + fn: ArrowFunctionExpressionOrFunctionExpression, + ) { + const fnParam = fn.params[0] + if (fnParam.type !== "ArrayPattern") return + const argNames = args.elements.map((element) => { + return element && element.type === "Identifier" ? element.name : null + }) + fnParam.elements.forEach((element, index) => { + const argName = argNames[index] + if (element && element.type === "Identifier" && argName) { + const expectedName = `$${argName}` + if (expectedName !== element.name) { + context.report({ + node: fn, + loc: element.loc!, + messageId: "unexpected", + data: { name: expectedName }, + }) + } + } + }) + } + + return { + Program() { + for (const { node } of extractStoreReferences(context, ["derived"])) { + const [args, fn] = node.arguments + if (!args || !isIdentifierOrArrayExpression(args)) continue + if (!fn || !isFunctionExpression(fn)) continue + if (!fn.params || fn.params.length === 0) continue + if (args.type === "Identifier") checkIdentifier(context, args, fn) + else checkArrayExpression(context, args, fn) + } + }, + } + }, +}) diff --git a/src/rules/no-store-async.ts b/src/rules/no-store-async.ts index 6934b697e..b8dc13ea5 100644 --- a/src/rules/no-store-async.ts +++ b/src/rules/no-store-async.ts @@ -32,7 +32,7 @@ export default createRule("no-store-async", { continue } - const start = fn.loc?.start ?? { line: 1, column: 0 } + const start = fn.loc!.start context.report({ node: fn, loc: { diff --git a/src/rules/reference-helpers/svelte-store.ts b/src/rules/reference-helpers/svelte-store.ts index 3e9edc975..ba58abff3 100644 --- a/src/rules/reference-helpers/svelte-store.ts +++ b/src/rules/reference-helpers/svelte-store.ts @@ -2,22 +2,25 @@ import type * as ESTree from "estree" import { ReferenceTracker } from "eslint-utils" import type { RuleContext } from "../../types" +type StoreName = "writable" | "readable" | "derived" + /** Extract 'svelte/store' references */ export function* extractStoreReferences( context: RuleContext, + storeNames: StoreName[] = ["writable", "readable", "derived"], ): Generator<{ node: ESTree.CallExpression; name: string }, void> { const referenceTracker = new ReferenceTracker(context.getScope()) for (const { node, path } of referenceTracker.iterateEsmReferences({ "svelte/store": { [ReferenceTracker.ESM]: true, writable: { - [ReferenceTracker.CALL]: true, + [ReferenceTracker.CALL]: storeNames.includes("writable"), }, readable: { - [ReferenceTracker.CALL]: true, + [ReferenceTracker.CALL]: storeNames.includes("readable"), }, derived: { - [ReferenceTracker.CALL]: true, + [ReferenceTracker.CALL]: storeNames.includes("derived"), }, }, })) { diff --git a/src/utils/rules.ts b/src/utils/rules.ts index 9efb8fcae..f22699f28 100644 --- a/src/utils/rules.ts +++ b/src/utils/rules.ts @@ -1,6 +1,7 @@ import type { RuleModule } from "../types" import buttonHasType from "../rules/button-has-type" import commentDirective from "../rules/comment-directive" +import derivedHasSameInputsOutputs from "../rules/derived-has-same-inputs-outputs" import firstAttributeLinebreak from "../rules/first-attribute-linebreak" import htmlClosingBracketSpacing from "../rules/html-closing-bracket-spacing" import htmlQuotes from "../rules/html-quotes" @@ -41,6 +42,7 @@ import validCompile from "../rules/valid-compile" export const rules = [ buttonHasType, commentDirective, + derivedHasSameInputsOutputs, firstAttributeLinebreak, htmlClosingBracketSpacing, htmlQuotes, diff --git a/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-errors.yaml b/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-errors.yaml new file mode 100644 index 000000000..9ce09c034 --- /dev/null +++ b/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-errors.yaml @@ -0,0 +1,32 @@ +- message: The argument name should be '$a'. + line: 3 + column: 13 + suggestions: null +- message: The argument name should be '$c'. + line: 6 + column: 13 + suggestions: null +- message: The argument name should be '$e'. + line: 9 + column: 19 + suggestions: null +- message: The argument name should be '$f'. + line: 9 + column: 22 + suggestions: null +- message: The argument name should be '$i'. + line: 12 + column: 19 + suggestions: null +- message: The argument name should be '$j'. + line: 12 + column: 22 + suggestions: null +- message: The argument name should be '$l'. + line: 15 + column: 26 + suggestions: null +- message: The argument name should be '$o'. + line: 18 + column: 22 + suggestions: null diff --git a/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-input.js b/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-input.js new file mode 100644 index 000000000..e272e9579 --- /dev/null +++ b/tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-input.js @@ -0,0 +1,20 @@ +import { derived } from "svelte/store" + +derived(a, (b) => { + /** do nothing */ +}) +derived(c, (d, set) => { + /** do nothing */ +}) +derived([e, f], ([g, h]) => { + /** do nothing */ +}) +derived([i, j], ([k, l], set) => { + /** do nothing */ +}) +derived([null, l], ([$m, $n]) => { + /** do nothing */ +}) +derived([o, null], ([$p, $q]) => { + /** do nothing */ +}) diff --git a/tests/fixtures/rules/derived-has-same-inputs-outputs/valid/test01-input.js b/tests/fixtures/rules/derived-has-same-inputs-outputs/valid/test01-input.js new file mode 100644 index 000000000..40e0f2db1 --- /dev/null +++ b/tests/fixtures/rules/derived-has-same-inputs-outputs/valid/test01-input.js @@ -0,0 +1,26 @@ +import { derived } from "svelte/store" + +derived(a, ($a) => { + /** do nothing */ +}) +derived(c, ($c, set) => { + /** do nothing */ +}) +derived([e, f], ([$e, $f]) => { + /** do nothing */ +}) +derived([i, j], ([$i, $j], set) => { + /** do nothing */ +}) +derived(null, ($null, set) => { + /** do nothing */ +}) +derived(null, ($k, set) => { + /** do nothing */ +}) +derived([null, l], ([$m, $l]) => { + /** do nothing */ +}) +derived([n, null], ([$n, $o]) => { + /** do nothing */ +}) diff --git a/tests/src/rules/derived-has-same-inputs-outputs.ts b/tests/src/rules/derived-has-same-inputs-outputs.ts new file mode 100644 index 000000000..8c05e2b60 --- /dev/null +++ b/tests/src/rules/derived-has-same-inputs-outputs.ts @@ -0,0 +1,16 @@ +import { RuleTester } from "eslint" +import rule from "../../../src/rules/derived-has-same-inputs-outputs" +import { loadTestCases } from "../../utils/utils" + +const tester = new RuleTester({ + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + }, +}) + +tester.run( + "derived-has-same-inputs-outputs", + rule as any, + loadTestCases("derived-has-same-inputs-outputs"), +)