feat: add svelte/no-loss-of-prop-reactivity rule

Author: ota-meshi

README.md

@@ -341,6 +341,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [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-loss-of-prop-reactivity](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-loss-of-prop-reactivity/) | disallow the use of props that potentially lose reactivity |  |
 | [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: |

docs/rules.md

@@ -54,6 +54,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [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-loss-of-prop-reactivity](./rules/no-loss-of-prop-reactivity.md)               | disallow the use of props that potentially lose reactivity                                                                                |          |
 | [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:   |

docs/rules/no-loss-of-prop-reactivity.md

@@ -0,0 +1,42 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "svelte/no-loss-of-prop-reactivity"
+description: "disallow the use of props that potentially lose reactivity"
+---
+
+# svelte/no-loss-of-prop-reactivity
+
+> disallow the use of props that potentially lose reactivity
+
+- :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 the use of props that potentially lose reactivity.
+
+<ESLintCodeBlock>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-loss-of-prop-reactivity: "error" */
+  export let prop = 42
+  /* ✓ GOOD */
+  $: double1 = prop * 2
+  /* ✗ BAD */
+  let double1 = prop * 2
+</script>
+```
+
+</ESLintCodeBlock>
+
+## :wrench: Options
+
+Nothing.
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-loss-of-prop-reactivity.ts)
+- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-loss-of-prop-reactivity.ts)

src/rules/no-loss-of-prop-reactivity.ts

@@ -0,0 +1,152 @@
+import type { AST } from "svelte-eslint-parser"
+import { createRule } from "../utils"
+import type { TSESTree } from "@typescript-eslint/types"
+import { findAttribute, findVariable } from "../utils/ast-utils"
+import { getStaticAttributeValue } from "../utils/ast-utils"
+
+export default createRule("no-loss-of-prop-reactivity", {
+  meta: {
+    docs: {
+      description: "disallow the use of props that potentially lose reactivity",
+      category: "Best Practices",
+      recommended: false,
+    },
+    schema: [],
+    messages: {
+      potentiallyLoseReactivity:
+        "Referencing props that potentially lose reactivity should be avoided.",
+    },
+    type: "suggestion",
+  },
+  create(context) {
+    if (!context.parserServices.isSvelte) {
+      return {}
+    }
+    let contextModule: AST.SvelteScriptElement | null = null
+    let scriptNode: AST.SvelteScriptElement | null = null
+    const reactiveStatements: AST.SvelteReactiveStatement[] = []
+    const functions: TSESTree.FunctionLike[] = []
+    const props = new Set<TSESTree.Identifier>()
+    return {
+      SvelteScriptElement(node: AST.SvelteScriptElement) {
+        const contextAttr = findAttribute(node, "context")
+        if (contextAttr && getStaticAttributeValue(contextAttr) === "module") {
+          contextModule = node
+          return
+        }
+
+        scriptNode = node
+      },
+      SvelteReactiveStatement(node: AST.SvelteReactiveStatement) {
+        reactiveStatements.push(node)
+      },
+      ":function"(node: TSESTree.FunctionLike) {
+        functions.push(node)
+      },
+      ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration) {
+        if (
+          contextModule &&
+          contextModule.range[0] < node.range[0] &&
+          node.range[1] < contextModule.range[1]
+        ) {
+          return
+        }
+        if (
+          node.declaration?.type === "VariableDeclaration" &&
+          (node.declaration.kind === "let" || node.declaration.kind === "var")
+        ) {
+          for (const decl of node.declaration.declarations) {
+            if (decl.id.type === "Identifier") {
+              props.add(decl.id)
+            }
+          }
+        }
+        for (const spec of node.specifiers) {
+          if (spec.exportKind === "type") {
+            continue
+          }
+          if (isMutableProp(spec.local)) {
+            props.add(spec.exported)
+          }
+        }
+      },
+      "Program:exit"() {
+        for (const prop of props) {
+          const variable = findVariable(context, prop)
+          if (
+            !variable ||
+            // ignore multiple definitions
+            variable.defs.length > 1
+          ) {
+            return
+          }
+          for (const reference of variable.references) {
+            if (reference.isWrite()) continue
+            const id = reference.identifier as TSESTree.Identifier
+            if (
+              variable.defs.some(
+                (def) =>
+                  def.node.range[0] <= id.range[0] &&
+                  id.range[1] <= def.node.range[1],
+              )
+            ) {
+              // The reference is in the variable definition.
+              continue
+            }
+            if (isInReactivityScope(id)) continue
+
+            context.report({
+              node: id,
+              messageId: "potentiallyLoseReactivity",
+            })
+          }
+        }
+      },
+    }
+
+    /** Checks whether given prop id is mutable variable or not */
+    function isMutableProp(id: TSESTree.Identifier) {
+      const variable = findVariable(context, id)
+      if (!variable || variable.defs.length === 0) {
+        return false
+      }
+      return variable.defs.every((def) => {
+        if (def.type !== "Variable") {
+          return false
+        }
+        return def.parent.kind === "let" || def.parent.kind === "var"
+      })
+    }
+
+    /** Checks whether given id is in potentially reactive scope or not */
+    function isInReactivityScope(id: TSESTree.Identifier) {
+      if (
+        !scriptNode ||
+        id.range[1] <= scriptNode.range[0] ||
+        scriptNode.range[1] <= id.range[0]
+      ) {
+        // The reference is in the template.
+        return true
+      }
+      if (
+        reactiveStatements.some(
+          (node) =>
+            node.range[0] <= id.range[0] && id.range[1] <= node.range[1],
+        )
+      ) {
+        // The reference is in the reactive statement.
+        return true
+      }
+      if (
+        functions.some(
+          (node) =>
+            node.range[0] <= id.range[0] && id.range[1] <= node.range[1],
+        )
+      ) {
+        // The reference is in the function.
+        return true
+      }
+      return false
+    }
+  },
+})

src/utils/rules.ts

@@ -29,6 +29,7 @@ import noExportLoadInSvelteModuleInKitPages from "../rules/no-export-load-in-sve
 import noExtraReactiveCurlies from "../rules/no-extra-reactive-curlies"
 import noImmutableReactiveStatements from "../rules/no-immutable-reactive-statements"
 import noInnerDeclarations from "../rules/no-inner-declarations"
+import noLossOfPropReactivity from "../rules/no-loss-of-prop-reactivity"
 import noNotFunctionHandler from "../rules/no-not-function-handler"
 import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
 import noReactiveFunctions from "../rules/no-reactive-functions"
@@ -88,6 +89,7 @@ export const rules = [
   noExtraReactiveCurlies,
   noImmutableReactiveStatements,
   noInnerDeclarations,
+  noLossOfPropReactivity,
   noNotFunctionHandler,
   noObjectInTextMustaches,
   noReactiveFunctions,

tests/fixtures/rules/no-loss-of-prop-reactivity/invalid/prop01-errors.yaml

@@ -0,0 +1,4 @@
+- message: Referencing props that potentially lose reactivity should be avoided.
+  line: 4
+  column: 17
+  suggestions: null

tests/fixtures/rules/no-loss-of-prop-reactivity/invalid/prop01-input.svelte

@@ -0,0 +1,5 @@
+<script>
+  export let prop = 42
+  /* ✗ BAD */
+  let double1 = prop * 2
+</script>

tests/fixtures/rules/no-loss-of-prop-reactivity/valid/keypad01-input.svelte

@@ -0,0 +1,32 @@
+<script>
+  import { createEventDispatcher } from "svelte"
+
+  export let value = ""
+
+  const dispatch = createEventDispatcher()
+
+  const select = (num) => () => (value += num)
+  const clear = () => (value = "")
+  const submit = () => dispatch("submit")
+</script>
+
+<div class="keypad">
+  <button on:click={select(1)}>1</button>
+
+  <button disabled={!value} on:click={clear}>clear</button>
+  <button on:click={select(0)}>0</button>
+  <button disabled={!value} on:click={submit}>submit</button>
+</div>
+
+<style>
+  .keypad {
+    display: grid;
+    grid-template-columns: repeat(3, 5em);
+    grid-template-rows: repeat(4, 3em);
+    grid-gap: 0.5em;
+  }
+
+  button {
+    margin: 0;
+  }
+</style>

tests/fixtures/rules/no-loss-of-prop-reactivity/valid/prop01-input.svelte

@@ -0,0 +1,5 @@
+<script>
+  export let prop = 42
+  /* ✓ GOOD */
+  $: double1 = prop * 2
+</script>

tests/src/rules/no-loss-of-prop-reactivity.ts

@@ -0,0 +1,16 @@
+import { RuleTester } from "eslint"
+import rule from "../../../src/rules/no-loss-of-prop-reactivity"
+import { loadTestCases } from "../../utils/utils"
+
+const tester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2020,
+    sourceType: "module",
+  },
+})
+
+tester.run(
+  "no-loss-of-prop-reactivity",
+  rule as any,
+  loadTestCases("no-loss-of-prop-reactivity"),
+)