Add no-not-function-handler rule (#35)

* Add no-not-function-handler rule

* add test

Author: ota-meshi

README.md

@@ -246,6 +246,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
 | Rule ID | Description |    |
 |:--------|:------------|:---|
 | [@ota-meshi/svelte/no-dupe-else-if-blocks](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-dupe-else-if-blocks.html) | disallow duplicate conditions in `{#if}` / `{:else if}` chains | :star: |
+| [@ota-meshi/svelte/no-not-function-handler](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-not-function-handler.html) | disallow use of not function in event handler | :star: |
 | [@ota-meshi/svelte/no-object-in-text-mustaches](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-object-in-text-mustaches.html) | disallow objects in text mustache interpolation | :star: |
 
 ## Security Vulnerability

docs/rules/README.md

@@ -16,6 +16,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
 | Rule ID | Description |    |
 |:--------|:------------|:---|
 | [@ota-meshi/svelte/no-dupe-else-if-blocks](./no-dupe-else-if-blocks.md) | disallow duplicate conditions in `{#if}` / `{:else if}` chains | :star: |
+| [@ota-meshi/svelte/no-not-function-handler](./no-not-function-handler.md) | disallow use of not function in event handler | :star: |
 | [@ota-meshi/svelte/no-object-in-text-mustaches](./no-object-in-text-mustaches.md) | disallow objects in text mustache interpolation | :star: |
 
 ## Security Vulnerability

docs/rules/no-not-function-handler.md

@@ -0,0 +1,61 @@
+---
+pageClass: "rule-details"
+sidebarDepth: 0
+title: "@ota-meshi/svelte/no-not-function-handler"
+description: "disallow use of not function in event handler"
+---
+
+# @ota-meshi/svelte/no-not-function-handler
+
+> disallow use of not function in event handler
+
+- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
+- :gear: This rule is included in `"plugin:@ota-meshi/svelte/recommended"`.
+
+## :book: Rule Details
+
+This rule reports where you used not function value in event handlers.  
+If you use a non-function value for the event handler, it event handler will not be called. It's almost always a mistake. You may have written a lot of unnecessary curly braces.
+
+<eslint-code-block>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint @ota-meshi/svelte/no-not-function-handler: "error" */
+  function foo() {
+    /*  */
+  }
+  const bar = 42
+</script>
+
+<!-- ✓ GOOD -->
+<button on:click={foo} />
+<button
+  on:click={() => {
+    /*  */
+  }}
+/>
+
+<!-- ✗ BAD -->
+<button on:click={{ foo }} />
+<button on:click={bar} />
+```
+
+</eslint-code-block>
+
+## :wrench: Options
+
+Nothing.
+
+## :couple: Related Rules
+
+- [@ota-meshi/svelte/no-object-in-text-mustaches]
+
+[@ota-meshi/svelte/no-object-in-text-mustaches]: ./no-object-in-text-mustaches.md
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-not-function-handler.ts)
+- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/no-not-function-handler.ts)

docs/rules/no-object-in-text-mustaches.md

@@ -42,6 +42,12 @@ When you use an object for text interpolation, it is drawn as `[object Object]`.
 
 Nothing.
 
+## :couple: Related Rules
+
+- [@ota-meshi/svelte/no-invalid-handler]
+
+[@ota-meshi/svelte/no-invalid-handler]: ./no-invalid-handler.md
+
 ## :mag: Implementation
 
 - [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-object-in-text-mustaches.ts)

src/configs/recommended.ts

@@ -11,6 +11,7 @@ export = {
     "@ota-meshi/svelte/no-at-html-tags": "error",
     "@ota-meshi/svelte/no-dupe-else-if-blocks": "error",
     "@ota-meshi/svelte/no-inner-declarations": "error",
+    "@ota-meshi/svelte/no-not-function-handler": "error",
     "@ota-meshi/svelte/no-object-in-text-mustaches": "error",
     "@ota-meshi/svelte/system": "error",
   },

src/rules/indent-helpers/es.ts

@@ -21,12 +21,11 @@ type NodeWithParent =
   | (Exclude<ESTree.Node, ESTree.Program> & { parent: ASTNode })
   | AST.SvelteProgram
   | AST.SvelteReactiveStatement
-type NodeListenerMap<T extends NodeWithParent = NodeWithParent> = {
-  [key in NodeWithParent["type"]]: T extends { type: key } ? T : never
-}
 
 type NodeListener = {
-  [T in keyof NodeListenerMap]: (node: NodeListenerMap[T]) => void
+  [key in NodeWithParent["type"]]: (
+    node: NodeWithParent & { type: key },
+  ) => void
 }
 
 /**

src/rules/indent-helpers/svelte.ts

@@ -8,12 +8,9 @@ type NodeWithoutES = Exclude<
   AST.SvelteNode,
   AST.SvelteProgram | AST.SvelteReactiveStatement
 >
-type NodeListenerMap<T extends NodeWithoutES = NodeWithoutES> = {
-  [key in NodeWithoutES["type"]]: T extends { type: key } ? T : never
-}
 
 type NodeListener = {
-  [T in keyof NodeListenerMap]: (node: NodeListenerMap[T]) => void
+  [key in NodeWithoutES["type"]]: (node: NodeWithoutES & { type: key }) => void
 }
 const PREFORMATTED_ELEMENT_NAMES = ["pre", "textarea"]
 

src/rules/indent-helpers/ts.ts

@@ -32,11 +32,8 @@ type NodeWithoutES = Exclude<
   | TSESTree.JSXSpreadChild
   | TSESTree.JSXText
 >
-type NodeListenerMap<T extends NodeWithoutES = NodeWithoutES> = {
-  [key in NodeWithoutES["type"]]: T extends { type: key } ? T : never
-}
 type NodeListener = {
-  [T in keyof NodeListenerMap]: (node: NodeListenerMap[T]) => void
+  [key in NodeWithoutES["type"]]: (node: NodeWithoutES & { type: key }) => void
 }
 
 /**

src/rules/no-not-function-handler.ts

@@ -0,0 +1,103 @@
+import type { AST } from "svelte-eslint-parser"
+import type * as ESTree from "estree"
+import { createRule } from "../utils"
+import { findVariable } from "../utils/ast-utils"
+
+const PHRASES = {
+  ObjectExpression: "object",
+  ArrayExpression: "array",
+  ClassExpression: "class",
+  Literal(node: ESTree.Literal): string | null {
+    if ("regex" in node) {
+      return "regex value"
+    }
+    if ("bigint" in node) {
+      return "bigint value"
+    }
+    if (node.value == null) {
+      return null
+    }
+    return `${typeof node.value} value`
+  },
+  TemplateLiteral: "string value",
+}
+export default createRule("no-not-function-handler", {
+  meta: {
+    docs: {
+      description: "disallow use of not function in event handler",
+      category: "Possible Errors",
+      recommended: true,
+    },
+    schema: [],
+    messages: {
+      unexpected: "Unexpected {{phrase}} in event handler.",
+    },
+    type: "problem", // "problem", or "layout",
+  },
+  create(context) {
+    /** Find data expression */
+    function findRootExpression(
+      node: ESTree.Expression,
+      already = new Set<ESTree.Identifier>(),
+    ): ESTree.Expression {
+      if (node.type !== "Identifier" || already.has(node)) {
+        return node
+      }
+      already.add(node)
+      const variable = findVariable(context, node)
+      if (!variable || variable.defs.length !== 1) {
+        return node
+      }
+      const def = variable.defs[0]
+      if (def.type === "Variable") {
+        if (def.parent.kind === "const" && def.node.init) {
+          const init = def.node.init
+          return findRootExpression(init, already)
+        }
+      }
+      return node
+    }
+
+    /** Verify for `on:` directive value */
+    function verify(node: AST.SvelteEventHandlerDirective["expression"]) {
+      if (!node) {
+        return
+      }
+      const expression = findRootExpression(node)
+
+      if (
+        expression.type !== "ObjectExpression" &&
+        expression.type !== "ArrayExpression" &&
+        expression.type !== "ClassExpression" &&
+        expression.type !== "Literal" &&
+        expression.type !== "TemplateLiteral"
+      ) {
+        return
+      }
+      const phraseValue = PHRASES[expression.type]
+      const phrase =
+        typeof phraseValue === "function"
+          ? phraseValue(expression as never)
+          : phraseValue
+      if (phrase == null) {
+        return
+      }
+      context.report({
+        node,
+        messageId: "unexpected",
+        data: {
+          phrase,
+        },
+      })
+    }
+
+    return {
+      SvelteDirective(node) {
+        if (node.kind !== "EventHandler") {
+          return
+        }
+        verify(node.expression)
+      },
+    }
+  },
+})

src/types.ts

@@ -16,12 +16,11 @@ export type ASTNode =
 type ASTNodeWithParent =
   | (Exclude<ASTNode, ESTree.Program> & { parent: ASTNode })
   | AST.SvelteProgram
-type ASTNodeListenerMap<T extends ASTNodeWithParent = ASTNodeWithParent> = {
-  [key in ASTNodeWithParent["type"]]: T extends { type: key } ? T : never
-}
 
 export type ASTNodeListener = {
-  [T in keyof ASTNodeListenerMap]?: (node: ASTNodeListenerMap[T]) => void
+  [key in ASTNodeWithParent["type"]]?: (
+    node: ASTNodeWithParent & { type: key },
+  ) => void
 }
 export interface RuleListener extends ASTNodeListener {
   onCodePathStart?(codePath: Rule.CodePath, node: never): void

src/utils/ast-utils.ts

@@ -1,6 +1,8 @@
-import type { ASTNode, SourceCode } from "../types"
+import type { ASTNode, RuleContext, SourceCode } from "../types"
 import type * as ESTree from "estree"
 import type { AST as SvAST } from "svelte-eslint-parser"
+import * as eslintUtils from "eslint-utils"
+import type { Scope } from "eslint"
 
 /**
  * Checks whether or not the tokens of two given nodes are same.
@@ -191,3 +193,39 @@ export function getStaticAttributeValue(
   }
   return str
 }
+
+/**
+ * Find the variable of a given name.
+ */
+export function findVariable(
+  context: RuleContext,
+  node: ESTree.Identifier,
+): Scope.Variable | null {
+  return eslintUtils.findVariable(getScope(context, node), node)
+}
+
+/**
+ * Gets the scope for the current node
+ */
+export function getScope(
+  context: RuleContext,
+  currentNode: ESTree.Node,
+): Scope.Scope {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
+  const scopeManager = (context.getSourceCode() as any).scopeManager
+
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
+  let node: any = currentNode
+  for (; node; node = node.parent || null) {
+    const scope = scopeManager.acquire(node, false)
+
+    if (scope) {
+      if (scope.type === "function-expression-name") {
+        return scope.childScopes[0]
+      }
+      return scope
+    }
+  }
+
+  return scopeManager.scopes[0]
+}

src/utils/rules.ts

@@ -8,6 +8,7 @@ import noAtDebugTags from "../rules/no-at-debug-tags"
 import noAtHtmlTags from "../rules/no-at-html-tags"
 import noDupeElseIfBlocks from "../rules/no-dupe-else-if-blocks"
 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 noTargetBlank from "../rules/no-target-blank"
 import noUselessMustaches from "../rules/no-useless-mustaches"
@@ -26,6 +27,7 @@ export const rules = [
   noAtHtmlTags,
   noDupeElseIfBlocks,
   noInnerDeclarations,
+  noNotFunctionHandler,
   noObjectInTextMustaches,
   noTargetBlank,
   noUselessMustaches,

tests/fixtures/rules/no-not-function-handler/invalid/array01-errors.json

@@ -0,0 +1,7 @@
+[
+  {
+    "message": "Unexpected array in event handler.",
+    "line": 5,
+    "column": 19
+  }
+]

tests/fixtures/rules/no-not-function-handler/invalid/array01-input.svelte

@@ -0,0 +1,5 @@
+<script>
+  const a = "hello!"
+</script>
+
+<button on:click={[a]} />

tests/fixtures/rules/no-not-function-handler/invalid/class01-errors.json

@@ -0,0 +1,7 @@
+[
+  {
+    "message": "Unexpected class in event handler.",
+    "line": 4,
+    "column": 19
+  }
+]

tests/fixtures/rules/no-not-function-handler/invalid/class01-input.svelte

@@ -0,0 +1,4 @@
+<script>
+</script>
+
+<button on:click={class B {}} />

tests/fixtures/rules/no-not-function-handler/invalid/object01-errors.json

@@ -0,0 +1,7 @@
+[
+  {
+    "message": "Unexpected object in event handler.",
+    "line": 5,
+    "column": 19
+  }
+]

tests/fixtures/rules/no-not-function-handler/invalid/object01-input.svelte

@@ -0,0 +1,5 @@
+<script>
+  function fn() {}
+</script>
+
+<button on:click={{ fn }} />

tests/fixtures/rules/no-not-function-handler/invalid/string01-errors.json

@@ -0,0 +1,12 @@
+[
+  {
+    "message": "Unexpected string value in event handler.",
+    "line": 6,
+    "column": 19
+  },
+  {
+    "message": "Unexpected string value in event handler.",
+    "line": 7,
+    "column": 19
+  }
+]

tests/fixtures/rules/no-not-function-handler/invalid/string01-input.svelte

@@ -0,0 +1,7 @@
+<script>
+  const a = "hello!"
+  const b = `${a} world`
+</script>
+
+<button on:click={a} />
+<button on:click={b} />