Skip to content

Commit 8395165

Browse files
baseballyamabaseballyama
committed
feat: implement derived-has-same-inputs-outputs rule
1 parent 76ebfca commit 8395165

File tree

10 files changed

+236
-3
lines changed

10 files changed

+236
-3
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ These rules relate to better ways of doing things to help you avoid problems:
289289
| Rule ID | Description | |
290290
|:--------|:------------|:---|
291291
| [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 | |
292+
| [svelte/derived-has-same-inputs-outputs](https://ota-meshi.github.io/eslint-plugin-svelte/rules/derived-has-same-inputs-outputs/) | derived store should use same variable names between values and callback | |
292293
| [svelte/no-at-debug-tags](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
293294
| [svelte/no-reactive-functions](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
294295
| [svelte/no-reactive-literals](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |

‎docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ These rules relate to better ways of doing things to help you avoid problems:
4242
| Rule ID | Description | |
4343
|:--------|:------------|:---|
4444
| [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | |
45+
| [svelte/derived-has-same-inputs-outputs](./rules/derived-has-same-inputs-outputs.md) | derived store should use same variable names between values and callback | |
4546
| [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
4647
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: |
4748
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |

‎docs/rules/derived-has-same-inputs-outputs.md

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
pageClass: "rule-details"
3+
sidebarDepth: 0
4+
title: "svelte/derived-has-same-inputs-outputs"
5+
description: "derived store should use same variable names between values and callback"
6+
---
7+
8+
# svelte/derived-has-same-inputs-outputs
9+
10+
> derived store should use same variable names between values and callback
11+
12+
## :book: Rule Details
13+
14+
This rule reports where variable names and callback function's argument names are different.
15+
This is mainly a recommended rule to avoid implementation confusion.
16+
17+
<ESLintCodeBlock language="javascript">
18+
19+
<!--eslint-skip-->
20+
21+
```js
22+
/* eslint svelte/derived-has-same-inputs-outputs: "error" */
23+
24+
import { derived } from "svelte/store"
25+
26+
/* ✓ GOOD */
27+
derived(a, ($a) => {});
28+
derived(a, ($a, set) => {})
29+
derived([ a, b ], ([ $a, $b ]) => {})
30+
31+
/* ✗ BAD */
32+
derived(a, (b) => {});
33+
derived(a, (b, set) => {});
34+
derived([ a, b ], ([ one, two ]) => {})
35+
```
36+
37+
</ESLintCodeBlock>
38+
39+
## :wrench: Options
40+
41+
Nothing.
42+
43+
44+
## :books: Further Reading
45+
46+
- [Svelte - Docs > RUN TIME > svelte/store > derived](https://svelte.dev/docs#run-time-svelte-store-derived)
47+
48+

‎src/rules/derived-has-same-inputs-outputs.ts

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import type * as ESTree from "estree"
2+
import { createRule } from "../utils"
3+
import type { RuleContext } from "../types"
4+
import { extractStoreReferences } from "./reference-helpers/svelte-store"
5+
6+
export default createRule("derived-has-same-inputs-outputs", {
7+
meta: {
8+
docs: {
9+
description: "",
10+
category: "Best Practices",
11+
recommended: false,
12+
},
13+
schema: [],
14+
messages: {
15+
unexpected: "The argument name should be '{{name}}'.",
16+
},
17+
type: "suggestion",
18+
},
19+
create(context) {
20+
/** check node type */
21+
function isIdentifierOrArrayExpression(
22+
node: ESTree.SpreadElement | ESTree.Expression,
23+
): node is ESTree.Identifier | ESTree.ArrayExpression {
24+
return ["Identifier", "ArrayExpression"].includes(node.type)
25+
}
26+
27+
type ArrowFunctionExpressionOrFunctionExpression =
28+
| ESTree.ArrowFunctionExpression
29+
| ESTree.FunctionExpression
30+
31+
/** check node type */
32+
function isFunctionExpression(
33+
node: ESTree.SpreadElement | ESTree.Expression,
34+
): node is ArrowFunctionExpressionOrFunctionExpression {
35+
return ["ArrowFunctionExpression", "FunctionExpression"].includes(
36+
node.type,
37+
)
38+
}
39+
40+
/**
41+
* Check for identifier type.
42+
* e.g. derived(a, ($a) => {});
43+
*/
44+
function checkIdentifier(
45+
context: RuleContext,
46+
args: ESTree.Identifier,
47+
fn: ArrowFunctionExpressionOrFunctionExpression,
48+
) {
49+
const fnParam = fn.params[0]
50+
if (fnParam.type !== "Identifier") return
51+
const expectedName = `$${args.name}`
52+
if (expectedName !== fnParam.name) {
53+
context.report({
54+
node: fn,
55+
loc: {
56+
start: fnParam.loc?.start ?? { line: 1, column: 0 },
57+
end: fnParam.loc?.end ?? { line: 1, column: 0 },
58+
},
59+
messageId: "unexpected",
60+
data: { name: expectedName },
61+
})
62+
}
63+
}
64+
65+
/**
66+
* Check for array type.
67+
* e.g. derived([ a, b ], ([ $a, $b ]) => {})
68+
*/
69+
function checkArrayExpression(
70+
context: RuleContext,
71+
args: ESTree.ArrayExpression,
72+
fn: ArrowFunctionExpressionOrFunctionExpression,
73+
) {
74+
const fnParam = fn.params[0]
75+
if (fnParam.type !== "ArrayPattern") return
76+
const argNames = args.elements.map((element) => {
77+
return element && element.type === "Identifier" ? element.name : null
78+
})
79+
fnParam.elements.forEach((element, index) => {
80+
if (element && element.type === "Identifier") {
81+
const expectedName = `$${argNames[index]}`
82+
if (expectedName !== element.name) {
83+
context.report({
84+
node: fn,
85+
loc: {
86+
start: element.loc?.start ?? { line: 1, column: 0 },
87+
end: element.loc?.end ?? { line: 1, column: 0 },
88+
},
89+
messageId: "unexpected",
90+
data: { name: expectedName },
91+
})
92+
}
93+
}
94+
})
95+
}
96+
97+
return {
98+
Program() {
99+
for (const { node } of extractStoreReferences(context, ["derived"])) {
100+
const [args, fn] = node.arguments
101+
if (!args || !isIdentifierOrArrayExpression(args)) continue
102+
if (!fn || !isFunctionExpression(fn)) continue
103+
if (!fn.params || fn.params.length === 0) continue
104+
if (args.type === "Identifier") checkIdentifier(context, args, fn)
105+
else checkArrayExpression(context, args, fn)
106+
}
107+
},
108+
}
109+
},
110+
})

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,25 @@ import type * as ESTree from "estree"
22
import { ReferenceTracker } from "eslint-utils"
33
import type { RuleContext } from "../../types"
44

5+
type StoreName = "writable" | "readable" | "derived"
6+
57
/** Extract 'svelte/store' references */
68
export function* extractStoreReferences(
79
context: RuleContext,
10+
storeNames: StoreName[] = ["writable", "readable", "derived"],
811
): Generator<{ node: ESTree.CallExpression; name: string }, void> {
912
const referenceTracker = new ReferenceTracker(context.getScope())
1013
for (const { node, path } of referenceTracker.iterateEsmReferences({
1114
"svelte/store": {
1215
[ReferenceTracker.ESM]: true,
1316
writable: {
14-
[ReferenceTracker.CALL]: true,
17+
[ReferenceTracker.CALL]: storeNames.includes("writable"),
1518
},
1619
readable: {
17-
[ReferenceTracker.CALL]: true,
20+
[ReferenceTracker.CALL]: storeNames.includes("readable"),
1821
},
1922
derived: {
20-
[ReferenceTracker.CALL]: true,
23+
[ReferenceTracker.CALL]: storeNames.includes("derived"),
2124
},
2225
},
2326
})) {

‎src/utils/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { RuleModule } from "../types"
22
import buttonHasType from "../rules/button-has-type"
33
import commentDirective from "../rules/comment-directive"
4+
import derivedHasSameInputsOutputs from "../rules/derived-has-same-inputs-outputs"
45
import firstAttributeLinebreak from "../rules/first-attribute-linebreak"
56
import htmlClosingBracketSpacing from "../rules/html-closing-bracket-spacing"
67
import htmlQuotes from "../rules/html-quotes"
@@ -41,6 +42,7 @@ import validCompile from "../rules/valid-compile"
4142
export const rules = [
4243
buttonHasType,
4344
commentDirective,
45+
derivedHasSameInputsOutputs,
4446
firstAttributeLinebreak,
4547
htmlClosingBracketSpacing,
4648
htmlQuotes,

‎tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-errors.yaml

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
- message: The argument name should be '$a'.
2+
line: 3
3+
column: 13
4+
suggestions: null
5+
- message: The argument name should be '$c'.
6+
line: 6
7+
column: 13
8+
suggestions: null
9+
- message: The argument name should be '$e'.
10+
line: 9
11+
column: 19
12+
suggestions: null
13+
- message: The argument name should be '$f'.
14+
line: 9
15+
column: 22
16+
suggestions: null
17+
- message: The argument name should be '$i'.
18+
line: 12
19+
column: 19
20+
suggestions: null
21+
- message: The argument name should be '$j'.
22+
line: 12
23+
column: 22
24+
suggestions: null

‎tests/fixtures/rules/derived-has-same-inputs-outputs/invalid/test01-input.js

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { derived } from "svelte/store"
2+
3+
derived(a, (b) => {
4+
/** do nothing */
5+
})
6+
derived(c, (d, set) => {
7+
/** do nothing */
8+
})
9+
derived([e, f], ([g, h]) => {
10+
/** do nothing */
11+
})
12+
derived([i, j], ([k, l], set) => {
13+
/** do nothing */
14+
})

‎tests/fixtures/rules/derived-has-same-inputs-outputs/valid/test01-input.js

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { derived } from "svelte/store"
2+
3+
derived(a, ($a) => {
4+
/** do nothing */
5+
})
6+
derived(c, ($c, set) => {
7+
/** do nothing */
8+
})
9+
derived([e, f], ([$e, $f]) => {
10+
/** do nothing */
11+
})
12+
derived([i, j], ([$i, $j], set) => {
13+
/** do nothing */
14+
})

‎tests/src/rules/derived-has-same-inputs-outputs.ts

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { RuleTester } from "eslint"
2+
import rule from "../../../src/rules/derived-has-same-inputs-outputs"
3+
import { loadTestCases } from "../../utils/utils"
4+
5+
const tester = new RuleTester({
6+
parserOptions: {
7+
ecmaVersion: 2020,
8+
sourceType: "module",
9+
},
10+
})
11+
12+
tester.run(
13+
"derived-has-same-inputs-outputs",
14+
rule as any,
15+
loadTestCases("derived-has-same-inputs-outputs"),
16+
)

0 commit comments

Comments
 (0)