Skip to content

Add no-not-function-handler rule #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 61 additions & 0 deletions docs/rules/no-not-function-handler.md
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 6 additions & 0 deletions docs/rules/no-object-in-text-mustaches.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions src/configs/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down
7 changes: 3 additions & 4 deletions src/rules/indent-helpers/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
5 changes: 1 addition & 4 deletions src/rules/indent-helpers/svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
5 changes: 1 addition & 4 deletions src/rules/indent-helpers/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
103 changes: 103 additions & 0 deletions src/rules/no-not-function-handler.ts
Original file line number Diff line number Diff line change
@@ -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)
},
}
},
})
7 changes: 3 additions & 4 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 39 additions & 1 deletion src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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]
}
2 changes: 2 additions & 0 deletions src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -26,6 +27,7 @@ export const rules = [
noAtHtmlTags,
noDupeElseIfBlocks,
noInnerDeclarations,
noNotFunctionHandler,
noObjectInTextMustaches,
noTargetBlank,
noUselessMustaches,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"message": "Unexpected array in event handler.",
"line": 5,
"column": 19
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
const a = "hello!"
</script>

<button on:click={[a]} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"message": "Unexpected class in event handler.",
"line": 4,
"column": 19
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<script>
</script>

<button on:click={class B {}} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"message": "Unexpected object in event handler.",
"line": 5,
"column": 19
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
function fn() {}
</script>

<button on:click={{ fn }} />
Original file line number Diff line number Diff line change
@@ -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
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const a = "hello!"
const b = `${a} world`
</script>

<button on:click={a} />
<button on:click={b} />
Loading