Add svelte/no-reactive-literals rule (#203)

* wip: add no reactive literals rule

* feat: add snapshot support for suggestions

* wip: feedback & test updates

* chore: fix lint issue

* docs: yarn update output

Author: tivac

README.md

@@ -281,6 +281,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 |:--------|:------------|:---|
 | [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/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-literals](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | Don't assign literal values in reactive statements |  |
 | [svelte/no-unused-svelte-ignore](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
 | [svelte/no-useless-mustaches](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: |
 | [svelte/require-optimized-style-attribute](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized |  |

docs/rules.md

@@ -41,6 +41,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-reactive-literals](./rules/no-reactive-literals.md) | Don't assign literal values in reactive statements |  |
 | [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
 | [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: |
 | [svelte/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized |  |

docs/rules/no-reactive-literals.md

@@ -0,0 +1,42 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "svelte/no-reactive-literals"
+description: "Don't assign literal values in reactive statements"
+---
+
+# svelte/no-reactive-literals
+
+> Don't assign literal values in reactive statements
+
+- :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 on any assignment of a static, unchanging value within a reactive statement because it's not necessary.
+
+<ESLintCodeBlock>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-reactive-literals: "error" */
+  /* ✓ GOOD */
+  let foo = "bar";
+  
+  /* ✗ BAD */
+  $: foo = "bar";
+</script>
+```
+
+</ESLintCodeBlock>
+
+## :wrench: Options
+
+Nothing
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-reactive-literals.ts)
+- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/no-reactive-literals.ts)

src/rules/no-reactive-literals.ts

@@ -0,0 +1,63 @@
+import type { TSESTree } from "@typescript-eslint/types"
+import { createRule } from "../utils"
+
+export default createRule("no-reactive-literals", {
+  meta: {
+    docs: {
+      description: "Don't assign literal values in reactive statements",
+      category: "Best Practices",
+      recommended: false,
+    },
+    hasSuggestions: true,
+    schema: [],
+    messages: {
+      noReactiveLiterals: `Do not assign literal values inside reactive statements unless absolutely necessary.`,
+      fixReactiveLiteral: `Move the literal out of the reactive statement into an assignment`,
+    },
+    type: "suggestion",
+  },
+  create(context) {
+    return {
+      [`SvelteReactiveStatement > ExpressionStatement > AssignmentExpression${[
+        // $: foo = "foo";
+        // $: foo = 1;
+        `[right.type="Literal"]`,
+
+        // $: foo = [];
+        `[right.type="ArrayExpression"][right.elements.length=0]`,
+
+        // $: foo = {};
+        `[right.type="ObjectExpression"][right.properties.length=0]`,
+      ].join(",")}`](node: TSESTree.AssignmentExpression) {
+        // Move upwards to include the entire reactive statement
+        const parent = node.parent?.parent
+
+        if (!parent) {
+          return false
+        }
+
+        const source = context.getSourceCode()
+
+        return context.report({
+          node: parent,
+          loc: parent.loc,
+          messageId: "noReactiveLiterals",
+          suggest: [
+            {
+              messageId: "fixReactiveLiteral",
+              fix(fixer) {
+                return [
+                  // Insert "let" + whatever was in there
+                  fixer.insertTextBefore(parent, `let ${source.getText(node)}`),
+
+                  // Remove the original reactive statement
+                  fixer.remove(parent),
+                ]
+              },
+            },
+          ],
+        })
+      },
+    }
+  },
+})

src/types.ts

@@ -69,6 +69,7 @@ export interface RuleMetaData {
   }
   messages: { [messageId: string]: string }
   fixable?: "code" | "whitespace"
+  hasSuggestions?: boolean
   schema: JSONSchema4 | JSONSchema4[]
   deprecated?: boolean
   replacedBy?: string[]
@@ -98,6 +99,7 @@ export interface PartialRuleMetaData {
   )
   messages: { [messageId: string]: string }
   fixable?: "code" | "whitespace"
+  hasSuggestions?: boolean
   schema: JSONSchema4 | JSONSchema4[]
   deprecated?: boolean
   replacedBy?: string[]

src/utils/rules.ts

@@ -15,6 +15,7 @@ import noDynamicSlotName from "../rules/no-dynamic-slot-name"
 import noInnerDeclarations from "../rules/no-inner-declarations"
 import noNotFunctionHandler from "../rules/no-not-function-handler"
 import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
+import noReactiveLiterals from "../rules/no-reactive-literals"
 import noShorthandStylePropertyOverrides from "../rules/no-shorthand-style-property-overrides"
 import noSpacesAroundEqualSignsInAttribute from "../rules/no-spaces-around-equal-signs-in-attribute"
 import noTargetBlank from "../rules/no-target-blank"
@@ -47,6 +48,7 @@ export const rules = [
   noInnerDeclarations,
   noNotFunctionHandler,
   noObjectInTextMustaches,
+  noReactiveLiterals,
   noShorthandStylePropertyOverrides,
   noSpacesAroundEqualSignsInAttribute,
   noTargetBlank,

tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json

@@ -0,0 +1,38 @@
+[
+  {
+    "message": "Do not assign literal values inside reactive statements unless absolutely necessary.",
+    "line": 3,
+    "column": 5,
+    "suggestions": [
+      {
+        "desc": "Move the literal out of the reactive statement into an assignment",
+        "messageId": "fixReactiveLiteral",
+        "output": "<!-- prettier-ignore -->\n<script>\n    let foo = \"foo\"\n    $: bar = [];\n    $: baz = {};\n</script>\n"
+      }
+    ]
+  },
+  {
+    "message": "Do not assign literal values inside reactive statements unless absolutely necessary.",
+    "line": 4,
+    "column": 5,
+    "suggestions": [
+      {
+        "desc": "Move the literal out of the reactive statement into an assignment",
+        "messageId": "fixReactiveLiteral",
+        "output": "<!-- prettier-ignore -->\n<script>\n    $: foo = \"foo\";\n    let bar = []\n    $: baz = {};\n</script>\n"
+      }
+    ]
+  },
+  {
+    "message": "Do not assign literal values inside reactive statements unless absolutely necessary.",
+    "line": 5,
+    "column": 5,
+    "suggestions": [
+      {
+        "desc": "Move the literal out of the reactive statement into an assignment",
+        "messageId": "fixReactiveLiteral",
+        "output": "<!-- prettier-ignore -->\n<script>\n    $: foo = \"foo\";\n    $: bar = [];\n    let baz = {}\n</script>\n"
+      }
+    ]
+  }
+]

tests/fixtures/rules/no-reactive-literals/invalid/test01-input.svelte

@@ -0,0 +1,6 @@
+<!-- prettier-ignore -->
+<script>
+    $: foo = "foo";
+    $: bar = [];
+    $: baz = {};
+</script>

tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte

@@ -0,0 +1,6 @@
+<!-- prettier-ignore -->
+<script>
+    $: foo = `${"bar"}baz`
+    $: bar = [ "bar" ]
+    $: baz = { qux : true }
+</script>

tests/src/rules/no-reactive-literals.ts

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

tests/utils/utils.ts

@@ -130,6 +130,7 @@ export function loadTestCases(
       throw new Error(`Empty code: ${test.filename}`)
     }
   }
+
   return {
     valid,
     invalid,
@@ -155,6 +156,14 @@ function* itrListupInput(rootDir: string): IterableIterator<string> {
   }
 }
 
+// Necessary because of this:
+// https://github.com/eslint/eslint/issues/14936#issuecomment-906746754
+function applySuggestion(code: string, suggestion: Linter.LintSuggestion) {
+  const { fix } = suggestion
+
+  return `${code.slice(0, fix.range[0])}${fix.text}${code.slice(fix.range[1])}`
+}
+
 function writeFixtures(
   ruleName: string,
   inputFile: string,
@@ -184,6 +193,7 @@ function writeFixtures(
     },
     config.filename,
   )
+
   if (force || !fs.existsSync(errorFile)) {
     fs.writeFileSync(
       errorFile,
@@ -192,6 +202,14 @@ function writeFixtures(
           message: m.message,
           line: m.line,
           column: m.column,
+          suggestions: m.suggestions
+            ? m.suggestions.map((s) => ({
+                desc: s.desc,
+                messageId: s.messageId,
+                // Need to have this be the *fixed* output, not just the fix content or anything
+                output: applySuggestion(config.code, s),
+              }))
+            : null,
         })),
         null,
         2,