From 9aed09fc1dd4e180611eb080322ce270c1008897 Mon Sep 17 00:00:00 2001
From: yosuke ota <otameshiyo23@gmail.com>
Date: Sat, 15 Apr 2023 22:24:39 +0900
Subject: [PATCH 1/3] feat: add `svelte/no-immutable-reactive-statements` rule

---
 .changeset/curvy-bananas-pretend.md           |   5 +
 README.md                                     |   1 +
 docs/rules.md                                 |   1 +
 .../rules/no-immutable-reactive-statements.md |  67 ++++++++
 src/rules/no-immutable-reactive-statements.ts | 161 ++++++++++++++++++
 src/utils/rules.ts                            |   2 +
 .../invalid/immutable-let-errors.yaml         |  18 ++
 .../invalid/immutable-let-input.svelte        |  10 ++
 .../invalid/immutable01-errors.yaml           |  24 +++
 .../invalid/immutable01-input.svelte          |  20 +++
 .../valid/mutable01-input.svelte              |  19 +++
 .../valid/unknown01-input.svelte              |   3 +
 .../rules/no-immutable-reactive-statements.ts |  16 ++
 tests/utils/utils.ts                          |   6 +
 14 files changed, 353 insertions(+)
 create mode 100644 .changeset/curvy-bananas-pretend.md
 create mode 100644 docs/rules/no-immutable-reactive-statements.md
 create mode 100644 src/rules/no-immutable-reactive-statements.ts
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-errors.yaml
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-input.svelte
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-errors.yaml
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-input.svelte
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable01-input.svelte
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/valid/unknown01-input.svelte
 create mode 100644 tests/src/rules/no-immutable-reactive-statements.ts

diff --git a/.changeset/curvy-bananas-pretend.md b/.changeset/curvy-bananas-pretend.md
new file mode 100644
index 000000000..1072e606e
--- /dev/null
+++ b/.changeset/curvy-bananas-pretend.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": minor
+---
+
+feat: add `svelte/no-immutable-reactive-statements` rule
diff --git a/README.md b/README.md
index 0a9910f12..d50461aa4 100644
--- a/README.md
+++ b/README.md
@@ -345,6 +345,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [svelte/block-lang](https://sveltejs.github.io/eslint-plugin-svelte/rules/block-lang/) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. |  |
 | [svelte/button-has-type](https://sveltejs.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute |  |
 | [svelte/no-at-debug-tags](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
+| [svelte/no-immutable-reactive-statements](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-immutable-reactive-statements/) | disallow reactive statements that don't reference reactive values. |  |
 | [svelte/no-reactive-functions](https://sveltejs.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://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |
 | [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
diff --git a/docs/rules.md b/docs/rules.md
index b4543b474..2d0768739 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -52,6 +52,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [svelte/block-lang](./rules/block-lang.md) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. |  |
 | [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute |  |
 | [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
+| [svelte/no-immutable-reactive-statements](./rules/no-immutable-reactive-statements.md) | disallow reactive statements that don't reference reactive values. |  |
 | [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: |
 | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
diff --git a/docs/rules/no-immutable-reactive-statements.md b/docs/rules/no-immutable-reactive-statements.md
new file mode 100644
index 000000000..cdb36cedc
--- /dev/null
+++ b/docs/rules/no-immutable-reactive-statements.md
@@ -0,0 +1,67 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "svelte/no-immutable-reactive-statements"
+description: "disallow reactive statements that don't reference reactive values."
+---
+
+# svelte/no-immutable-reactive-statements
+
+> disallow reactive statements that don't reference reactive values.
+
+- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
+
+## :book: Rule Details
+
+This rule reports if all variables referenced in reactive statements are immutable. That reactive statement is immutable and not reactive.
+
+<ESLintCodeBlock>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-immutable-reactive-statements: "error" */
+  import myStore from "./my-stores"
+  import myVar from "./my-variables"
+  let mutableVar = "hello"
+  export let prop
+  /* ✓ GOOD */
+  $: computed1 = mutableVar + " " + mutableVar
+  $: computed2 = fn1(mutableVar)
+  $: console.log(mutableVar)
+  $: console.log(computed1)
+  $: console.log($myStore)
+  $: console.log(prop)
+
+  const immutableVar = "hello"
+  /* ✗ BAD */
+  $: computed3 = fn1(immutableVar)
+  $: computed4 = fn2()
+  $: console.log(immutableVar)
+  $: console.log(myVar)
+
+  /* ignore */
+  $: console.log(unknown)
+
+  function fn1(v) {
+    return v + " " + v
+  }
+  function fn2() {
+    return mutableVar + " " + mutableVar
+  }
+</script>
+
+<input bind:value={mutableVar} />
+```
+
+</ESLintCodeBlock>
+
+## :wrench: Options
+
+Nothing.
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-immutable-reactive-statements.ts)
+- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-immutable-reactive-statements.ts)
diff --git a/src/rules/no-immutable-reactive-statements.ts b/src/rules/no-immutable-reactive-statements.ts
new file mode 100644
index 000000000..a2317a22f
--- /dev/null
+++ b/src/rules/no-immutable-reactive-statements.ts
@@ -0,0 +1,161 @@
+import type { AST } from "svelte-eslint-parser"
+import { createRule } from "../utils"
+import type {
+  Scope,
+  Variable,
+  Reference,
+  Definition,
+} from "@typescript-eslint/scope-manager"
+
+export default createRule("no-immutable-reactive-statements", {
+  meta: {
+    docs: {
+      description:
+        "disallow reactive statements that don't reference reactive values.",
+      category: "Best Practices",
+      // TODO Switch to recommended in the major version.
+      recommended: false,
+    },
+    schema: [],
+    messages: {
+      immutable:
+        "This statement is not reactive because all variables referenced in the reactive statement are immutable.",
+    },
+    type: "suggestion",
+  },
+  create(context) {
+    const scopeManager = context.getSourceCode().scopeManager
+    const globalScope = scopeManager.globalScope
+    const toplevelScope =
+      globalScope?.childScopes.find((scope) => scope.type === "module") ||
+      globalScope
+    if (!globalScope || !toplevelScope) {
+      return {}
+    }
+
+    const cacheMutableVariable = new WeakMap<Variable, boolean>()
+
+    /**
+     * Checks whether the given reference is a mutable variable or not.
+     */
+    function isMutableVariableReference(reference: Reference) {
+      if (reference.identifier.name.startsWith("$")) {
+        // It is reactive store reference.
+        return true
+      }
+      if (!reference.resolved) {
+        // Unknown variable
+        return true
+      }
+      return isMutableVariable(reference.resolved)
+    }
+
+    /**
+     * Checks whether the given variable is a mutable variable or not.
+     */
+    function isMutableVariable(variable: Variable) {
+      const cache = cacheMutableVariable.get(variable)
+      if (cache != null) {
+        return cache
+      }
+      if (variable.defs.length === 0) {
+        // Global variables are assumed to be immutable.
+        return true
+      }
+      const isMutable = variable.defs.some((def) => {
+        if (def.type === "Variable") {
+          const parent = def.parent
+          if (parent.kind === "const") {
+            return false
+          }
+          const pp = parent.parent
+          if (
+            pp &&
+            pp.type === "ExportNamedDeclaration" &&
+            pp.declaration === parent
+          ) {
+            // Props
+            return true
+          }
+          return hasWrite(variable)
+        }
+        if (def.type === "ImportBinding") {
+          return false
+        }
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
+        if ((def.type as any) === "ComputedVariable") {
+          return true
+        }
+        return false
+      })
+      cacheMutableVariable.set(variable, isMutable)
+      return isMutable
+    }
+
+    /** Checks whether the given variable has a write or reactive store reference or not. */
+    function hasWrite(variable: Variable) {
+      const defIds = variable.defs.map((def: Definition) => def.name)
+      return variable.references.some(
+        (reference) =>
+          reference.isWrite() &&
+          !defIds.some(
+            (defId) =>
+              defId.range[0] <= reference.identifier.range[0] &&
+              reference.identifier.range[1] <= defId.range[1],
+          ),
+      )
+    }
+
+    /**
+     * Iterates through references to top-level variables in the given range.
+     */
+    function* iterateRangeReferences(scope: Scope, range: [number, number]) {
+      for (const variable of scope.variables) {
+        for (const reference of variable.references) {
+          if (
+            range[0] <= reference.identifier.range[0] &&
+            reference.identifier.range[1] <= range[1]
+          ) {
+            yield reference
+          }
+        }
+      }
+    }
+
+    return {
+      SvelteReactiveStatement(node: AST.SvelteReactiveStatement) {
+        for (const reference of iterateRangeReferences(
+          toplevelScope,
+          node.range,
+        )) {
+          if (reference.isWriteOnly()) {
+            continue
+          }
+          if (isMutableVariableReference(reference)) {
+            return
+          }
+        }
+        if (
+          globalScope.through.some(
+            (reference) =>
+              node.range[0] <= reference.identifier.range[0] &&
+              reference.identifier.range[1] <= node.range[1],
+          )
+        ) {
+          // Do not report if there are missing references.
+          return
+        }
+
+        context.report({
+          node:
+            node.body.type === "ExpressionStatement" &&
+            node.body.expression.type === "AssignmentExpression" &&
+            node.body.expression.operator === "="
+              ? node.body.expression.right
+              : node.body,
+          messageId: "immutable",
+        })
+      },
+    }
+  },
+})
diff --git a/src/utils/rules.ts b/src/utils/rules.ts
index b720d98b7..4382cf6dd 100644
--- a/src/utils/rules.ts
+++ b/src/utils/rules.ts
@@ -24,6 +24,7 @@ import noDupeUseDirectives from "../rules/no-dupe-use-directives"
 import noDynamicSlotName from "../rules/no-dynamic-slot-name"
 import noExportLoadInSvelteModuleInKitPages from "../rules/no-export-load-in-svelte-module-in-kit-pages"
 import noExtraReactiveCurlies from "../rules/no-extra-reactive-curlies"
+import noImmutableReactiveStatements from "../rules/no-immutable-reactive-statements"
 import noInnerDeclarations from "../rules/no-inner-declarations"
 import noNotFunctionHandler from "../rules/no-not-function-handler"
 import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
@@ -79,6 +80,7 @@ export const rules = [
   noDynamicSlotName,
   noExportLoadInSvelteModuleInKitPages,
   noExtraReactiveCurlies,
+  noImmutableReactiveStatements,
   noInnerDeclarations,
   noNotFunctionHandler,
   noObjectInTextMustaches,
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-errors.yaml b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-errors.yaml
new file mode 100644
index 000000000..1dbd98360
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-errors.yaml
@@ -0,0 +1,18 @@
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 3
+  column: 18
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 4
+  column: 18
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 5
+  column: 6
+  suggestions: null
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-input.svelte b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-input.svelte
new file mode 100644
index 000000000..d27b883a8
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable-let-input.svelte
@@ -0,0 +1,10 @@
+<script>
+  let immutableVar = "hello"
+  $: computed1 = `${immutableVar} ${immutableVar}`
+  $: computed1 = fn1(immutableVar)
+  $: console.log(immutableVar)
+
+  function fn1(v) {
+    return `${v} ${v}`
+  }
+</script>
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-errors.yaml b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-errors.yaml
new file mode 100644
index 000000000..d8e384b3a
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-errors.yaml
@@ -0,0 +1,24 @@
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 7
+  column: 18
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 8
+  column: 18
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 9
+  column: 6
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 10
+  column: 6
+  suggestions: null
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-input.svelte b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-input.svelte
new file mode 100644
index 000000000..b70ec3dfd
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/immutable01-input.svelte
@@ -0,0 +1,20 @@
+<script>
+  import myVar from "./my-variables"
+  let mutableVar = "hello"
+
+  const immutableVar = "hello"
+  /* ✗ BAD */
+  $: computed3 = fn1(immutableVar)
+  $: computed4 = fn2()
+  $: console.log(immutableVar)
+  $: console.log(myVar)
+
+  function fn1(v) {
+    return `${v} ${v}`
+  }
+  function fn2() {
+    return `${mutableVar} ${mutableVar}`
+  }
+</script>
+
+<input bind:value={mutableVar} />
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable01-input.svelte b/tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable01-input.svelte
new file mode 100644
index 000000000..b7c3c9bd9
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/valid/mutable01-input.svelte
@@ -0,0 +1,19 @@
+<script>
+  import myStore from "./my-stores"
+  import myVar from "./my-variables"
+  let mutableVar = "hello"
+  export let prop
+  /* ✓ GOOD */
+  $: computed1 = `${mutableVar} ${mutableVar}`
+  $: computed2 = fn1(mutableVar)
+  $: console.log(mutableVar)
+  $: console.log(computed1)
+  $: console.log($myStore)
+  $: console.log(prop)
+
+  function fn1(v) {
+    return `${v} ${v}`
+  }
+</script>
+
+<input bind:value={mutableVar} />
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/valid/unknown01-input.svelte b/tests/fixtures/rules/no-immutable-reactive-statements/valid/unknown01-input.svelte
new file mode 100644
index 000000000..1e7203315
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/valid/unknown01-input.svelte
@@ -0,0 +1,3 @@
+<script>
+  $: console.log(unknown)
+</script>
diff --git a/tests/src/rules/no-immutable-reactive-statements.ts b/tests/src/rules/no-immutable-reactive-statements.ts
new file mode 100644
index 000000000..3e1a8b507
--- /dev/null
+++ b/tests/src/rules/no-immutable-reactive-statements.ts
@@ -0,0 +1,16 @@
+import { RuleTester } from "eslint"
+import rule from "../../../src/rules/no-immutable-reactive-statements"
+import { loadTestCases } from "../../utils/utils"
+
+const tester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2020,
+    sourceType: "module",
+  },
+})
+
+tester.run(
+  "no-immutable-reactive-statements",
+  rule as any,
+  loadTestCases("no-immutable-reactive-statements"),
+)
diff --git a/tests/utils/utils.ts b/tests/utils/utils.ts
index 01dca2a37..04e6e2560 100644
--- a/tests/utils/utils.ts
+++ b/tests/utils/utils.ts
@@ -207,6 +207,9 @@ function writeFixtures(
         },
         ...config.parserOptions,
       },
+      globals: {
+        console: "readonly",
+      },
     },
     config.filename,
   )
@@ -282,6 +285,9 @@ function getConfig(ruleName: string, inputFile: string) {
         },
         extraFileExtensions: [".svelte"],
       },
+      globals: {
+        console: "readonly",
+      },
     },
     config,
     { code, filename: inputFile },

From 9475dc078f17b5690849baf9f4dab91b4396165b Mon Sep 17 00:00:00 2001
From: ota-meshi <otameshiyo23@gmail.com>
Date: Sat, 22 Apr 2023 16:34:10 +0900
Subject: [PATCH 2/3] chore: refactor

---
 src/rules/no-immutable-reactive-statements.ts | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/rules/no-immutable-reactive-statements.ts b/src/rules/no-immutable-reactive-statements.ts
index a2317a22f..df154c54c 100644
--- a/src/rules/no-immutable-reactive-statements.ts
+++ b/src/rules/no-immutable-reactive-statements.ts
@@ -82,8 +82,9 @@ export default createRule("no-immutable-reactive-statements", {
         if (def.type === "ImportBinding") {
           return false
         }
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
-        if ((def.type as any) === "ComputedVariable") {
+
+        if (def.node.type === "AssignmentExpression") {
+          // Reactive values
           return true
         }
         return false

From 19996f53495b7742624854813db8520b34212ec0 Mon Sep 17 00:00:00 2001
From: yosuke ota <otameshiyo23@gmail.com>
Date: Mon, 24 Apr 2023 10:57:53 +0900
Subject: [PATCH 3/3] test: add test case

---
 .../invalid/readonly-export01-errors.yaml      | 18 ++++++++++++++++++
 .../invalid/readonly-export01-input.svelte     | 15 +++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-errors.yaml
 create mode 100644 tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-input.svelte

diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-errors.yaml b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-errors.yaml
new file mode 100644
index 000000000..965b9a55c
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-errors.yaml
@@ -0,0 +1,18 @@
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 12
+  column: 17
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 13
+  column: 17
+  suggestions: null
+- message:
+    This statement is not reactive because all variables referenced in the
+    reactive statement are immutable.
+  line: 14
+  column: 17
+  suggestions: null
diff --git a/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-input.svelte b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-input.svelte
new file mode 100644
index 000000000..cded22396
--- /dev/null
+++ b/tests/fixtures/rules/no-immutable-reactive-statements/invalid/readonly-export01-input.svelte
@@ -0,0 +1,15 @@
+<script>
+  export const thisIs = "readonly"
+
+  export function greet(name) {
+    console.log(`hello ${name}!`)
+  }
+
+  export class Foo {}
+
+  const immutableVar = "hello"
+
+  $: message1 = greet(immutableVar)
+  $: message2 = `this is${thisIs}`
+  $: instance = new Foo()
+</script>