feat: improve require-store-reactive-access

Author: ota-meshi

.stylelintignore

@@ -13,3 +13,4 @@ LICENSE
 # should we ignore markdown files?
 *.md
 /docs-svelte-kit/
+/coverage

docs/rules/require-store-reactive-access.md

@@ -25,8 +25,14 @@ You should access the store value using the `$` prefix or the `get` function.
 <script>
   /* eslint svelte/require-store-reactive-access: "error" */
   import { writable, get } from "svelte/store"
-  const storeValue = writable("hello")
+  const storeValue = writable("world")
   const color = writable("red")
+
+  /* ✓ GOOD */
+  $: message = `Hello ${$storeValue}`
+
+  /* ✗ BAD */
+  $: message = `Hello ${storeValue}`
 </script>
 
 <!-- ✓ GOOD -->

package.json

@@ -163,6 +163,7 @@
     "stylus": "^0.59.0",
     "svelte": "^3.46.1",
     "svelte-adapter-ghpages": "0.0.2",
+    "svelte-i18n": "^3.4.0",
     "type-coverage": "^2.22.0",
     "typescript": "^4.5.2",
     "vite": "^3.1.0-0",
@@ -174,7 +175,7 @@
     "access": "public"
   },
   "typeCoverage": {
-    "atLeast": 98.7,
+    "atLeast": 98.71,
     "cache": true,
     "detail": true,
     "ignoreAsAssertion": true,

src/rules/reference-helpers/svelte-store.ts

@@ -1,4 +1,5 @@
 import type * as ESTree from "estree"
+import type { TSESTree } from "@typescript-eslint/types"
 import { ReferenceTracker } from "eslint-utils"
 import type { Variable } from "eslint-scope"
 import type { RuleContext } from "../../types"
@@ -35,27 +36,37 @@ export function* extractStoreReferences(
   }
 }
 
+export type StoreChecker = (
+  node: ESTree.Expression | TSESTree.Expression,
+  options?: { consistent?: boolean },
+) => boolean
+type StoreCheckerWithOptions = (
+  node: ESTree.Expression,
+  options: { consistent: boolean },
+) => boolean
+
 /**
  * Creates a function that checks whether the given expression node is a store instance or not.
  */
-export function createStoreChecker(
-  context: RuleContext,
-): (node: ESTree.Expression) => boolean {
+export function createStoreChecker(context: RuleContext): StoreChecker {
   const tools = getTypeScriptTools(context)
-  if (tools) {
-    return createStoreCheckerForTS(tools)
-  }
+  const checker = tools
+    ? createStoreCheckerForTS(tools)
+    : createStoreCheckerForES(context)
 
-  return createStoreCheckerForES(context)
+  return (node, options) =>
+    checker(node as ESTree.Expression, {
+      consistent: options?.consistent ?? false,
+    })
 }
 
 /**
  * Creates a function that checks whether the given expression node is a store instance or not, for EcmaScript.
  */
 function createStoreCheckerForES(
   context: RuleContext,
-): (node: ESTree.Expression) => boolean {
-  const variables = new Set<Variable>()
+): StoreCheckerWithOptions {
+  const storeVariables = new Map<Variable, { const: boolean }>()
   for (const { node } of extractStoreReferences(context)) {
     const parent = getParent(node)
     if (
@@ -65,36 +76,42 @@ function createStoreCheckerForES(
     ) {
       continue
     }
+    const decl = getParent(parent)
+    if (!decl || decl.type !== "VariableDeclaration") {
+      continue
+    }
 
     const variable = findVariable(context, parent.id)
     if (variable) {
-      variables.add(variable)
+      storeVariables.set(variable, { const: decl.kind === "const" })
     }
   }
 
-  return (node) => {
+  return (node, options) => {
     if (node.type !== "Identifier" || node.name.startsWith("$")) {
       return false
     }
     const variable = findVariable(context, node)
     if (!variable) {
       return false
     }
-    return variables.has(variable)
+    const info = storeVariables.get(variable)
+    if (!info) {
+      return false
+    }
+    return options.consistent ? info.const : true
   }
 }
 
 /**
  * Creates a function that checks whether the given expression node is a store instance or not, for TypeScript.
  */
-function createStoreCheckerForTS(
-  tools: TSTools,
-): (node: ESTree.Expression) => boolean {
+function createStoreCheckerForTS(tools: TSTools): StoreCheckerWithOptions {
   const { service } = tools
   const checker = service.program.getTypeChecker()
   const tsNodeMap = service.esTreeNodeToTSNodeMap
 
-  return (node) => {
+  return (node, options) => {
     const tsNode = tsNodeMap.get(node)
     if (!tsNode) {
       return false
@@ -107,57 +124,73 @@ function createStoreCheckerForTS(
      * Checks whether the given type is a store or not
      */
     function isStoreType(type: TS.Type): boolean {
-      if (type.isUnion()) {
-        return type.types.some(isStoreType)
-      }
-      const subscribe = type.getProperty("subscribe")
-      if (subscribe === undefined) {
-        return false
-      }
-      const subscribeType = checker.getTypeOfSymbolAtLocation(
-        subscribe,
-        tsNode!,
-      )
-      return isStoreSubscribeSignatureType(subscribeType)
+      return eachTypeCheck(type, options, (type) => {
+        const subscribe = type.getProperty("subscribe")
+        if (!subscribe) {
+          return false
+        }
+        const subscribeType = checker.getTypeOfSymbolAtLocation(
+          subscribe,
+          tsNode!,
+        )
+        return isStoreSubscribeSignatureType(subscribeType)
+      })
     }
 
     /**
      * Checks whether the given type is a store's subscribe or not
      */
     function isStoreSubscribeSignatureType(type: TS.Type): boolean {
-      if (type.isUnion()) {
-        return type.types.some(isStoreSubscribeSignatureType)
-      }
-      for (const signature of type.getCallSignatures()) {
-        if (
-          signature.parameters.length >= 2 &&
-          isFunctionSymbol(signature.parameters[0]) &&
-          isFunctionSymbol(signature.parameters[1])
-        ) {
-          return true
+      return eachTypeCheck(type, options, (type) => {
+        for (const signature of type.getCallSignatures()) {
+          if (
+            signature.parameters.length >= 2 &&
+            maybeFunctionSymbol(signature.parameters[0]) &&
+            maybeFunctionSymbol(signature.parameters[1])
+          ) {
+            return true
+          }
         }
-      }
-      return false
+        return false
+      })
     }
 
     /**
-     * Checks whether the given symbol is a function param or not
+     * Checks whether the given symbol maybe function param or not
      */
-    function isFunctionSymbol(param: TS.Symbol): boolean {
+    function maybeFunctionSymbol(param: TS.Symbol): boolean {
       const type: TS.Type | undefined = checker.getApparentType(
         checker.getTypeOfSymbolAtLocation(param, tsNode!),
       )
-      return isFunctionType(type)
+      return maybeFunctionType(type)
+    }
+
+    /**
+     * Checks whether the given type is maybe function param or not
+     */
+    function maybeFunctionType(type: TS.Type): boolean {
+      return eachTypeCheck(type, { consistent: false }, (type) => {
+        return type.getCallSignatures().length > 0
+      })
     }
   }
+}
 
-  /**
-   * Checks whether the given symbol is a function param or not
-   */
-  function isFunctionType(type: TS.Type): boolean {
-    if (type.isUnion()) {
-      return type.types.some(isFunctionType)
+/**
+ * Check the given type with the given check function.
+ * For union types, `options.consistent: true` requires all types to pass the check function.
+ * `options.consistent: false` considers a match if any type passes the check function.
+ */
+function eachTypeCheck(
+  type: TS.Type,
+  options: { consistent: boolean },
+  check: (t: TS.Type) => boolean,
+): boolean {
+  if (type.isUnion()) {
+    if (options.consistent) {
+      return type.types.every((t) => eachTypeCheck(t, options, check))
     }
-    return type.getCallSignatures().length > 0
+    return type.types.some((t) => eachTypeCheck(t, options, check))
   }
+  return check(type)
 }