feat: implement derived-has-same-inputs-outputs (#249)

* feat: implement derived-has-same-inputs-outputs rule

* chore: add changeset

* chore: update according to review comments

* chore: add more tests

.changeset/gentle-bugs-stare.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": minor
+---
+
+feat: add `svelte/derived-has-same-inputs-outputs` rule

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: |

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: |

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.
+
+<ESLintCodeBlock language="javascript">
+
+<!--eslint-skip-->
+
+```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 ]) => {})
+```
+
+</ESLintCodeBlock>
+
+## :wrench: Options
+
+Nothing.
+
+
+## :books: Further Reading
+
+- [Svelte - Docs > RUN TIME > svelte/store > derived](https://svelte.dev/docs#run-time-svelte-store-derived)
+
+

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)
+        }
+      },
+    }
+  },
+})

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: {

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"),
       },
     },
   })) {

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,

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

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 */
+})

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 */
+})

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"),
+)