Skip to content

Commit f810b69

Browse files
authored
feat: add svelte/no-immutable-reactive-statements rule (#439)
1 parent dce5541 commit f810b69

16 files changed

+387
-0
lines changed

.changeset/curvy-bananas-pretend.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"eslint-plugin-svelte": minor
3+
---
4+
5+
feat: add `svelte/no-immutable-reactive-statements` rule

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ These rules relate to better ways of doing things to help you avoid problems:
339339
| [svelte/block-lang](https://sveltejs.github.io/eslint-plugin-svelte/rules/block-lang/) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. | |
340340
| [svelte/button-has-type](https://sveltejs.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | |
341341
| [svelte/no-at-debug-tags](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
342+
| [svelte/no-immutable-reactive-statements](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-immutable-reactive-statements/) | disallow reactive statements that don't reference reactive values. | |
342343
| [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
343344
| [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |
344345
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |

docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ These rules relate to better ways of doing things to help you avoid problems:
5252
| [svelte/block-lang](./rules/block-lang.md) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. | |
5353
| [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | |
5454
| [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
55+
| [svelte/no-immutable-reactive-statements](./rules/no-immutable-reactive-statements.md) | disallow reactive statements that don't reference reactive values. | |
5556
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: |
5657
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |
5758
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
pageClass: "rule-details"
3+
sidebarDepth: 0
4+
title: "svelte/no-immutable-reactive-statements"
5+
description: "disallow reactive statements that don't reference reactive values."
6+
---
7+
8+
# svelte/no-immutable-reactive-statements
9+
10+
> disallow reactive statements that don't reference reactive values.
11+
12+
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
13+
14+
## :book: Rule Details
15+
16+
This rule reports if all variables referenced in reactive statements are immutable. That reactive statement is immutable and not reactive.
17+
18+
<ESLintCodeBlock>
19+
20+
<!--eslint-skip-->
21+
22+
```svelte
23+
<script>
24+
/* eslint svelte/no-immutable-reactive-statements: "error" */
25+
import myStore from "./my-stores"
26+
import myVar from "./my-variables"
27+
let mutableVar = "hello"
28+
export let prop
29+
/* ✓ GOOD */
30+
$: computed1 = mutableVar + " " + mutableVar
31+
$: computed2 = fn1(mutableVar)
32+
$: console.log(mutableVar)
33+
$: console.log(computed1)
34+
$: console.log($myStore)
35+
$: console.log(prop)
36+
37+
const immutableVar = "hello"
38+
/* ✗ BAD */
39+
$: computed3 = fn1(immutableVar)
40+
$: computed4 = fn2()
41+
$: console.log(immutableVar)
42+
$: console.log(myVar)
43+
44+
/* ignore */
45+
$: console.log(unknown)
46+
47+
function fn1(v) {
48+
return v + " " + v
49+
}
50+
function fn2() {
51+
return mutableVar + " " + mutableVar
52+
}
53+
</script>
54+
55+
<input bind:value={mutableVar} />
56+
```
57+
58+
</ESLintCodeBlock>
59+
60+
## :wrench: Options
61+
62+
Nothing.
63+
64+
## :mag: Implementation
65+
66+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-immutable-reactive-statements.ts)
67+
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-immutable-reactive-statements.ts)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import type { AST } from "svelte-eslint-parser"
2+
import { createRule } from "../utils"
3+
import type {
4+
Scope,
5+
Variable,
6+
Reference,
7+
Definition,
8+
} from "@typescript-eslint/scope-manager"
9+
10+
export default createRule("no-immutable-reactive-statements", {
11+
meta: {
12+
docs: {
13+
description:
14+
"disallow reactive statements that don't reference reactive values.",
15+
category: "Best Practices",
16+
// TODO Switch to recommended in the major version.
17+
recommended: false,
18+
},
19+
schema: [],
20+
messages: {
21+
immutable:
22+
"This statement is not reactive because all variables referenced in the reactive statement are immutable.",
23+
},
24+
type: "suggestion",
25+
},
26+
create(context) {
27+
const scopeManager = context.getSourceCode().scopeManager
28+
const globalScope = scopeManager.globalScope
29+
const toplevelScope =
30+
globalScope?.childScopes.find((scope) => scope.type === "module") ||
31+
globalScope
32+
if (!globalScope || !toplevelScope) {
33+
return {}
34+
}
35+
36+
const cacheMutableVariable = new WeakMap<Variable, boolean>()
37+
38+
/**
39+
* Checks whether the given reference is a mutable variable or not.
40+
*/
41+
function isMutableVariableReference(reference: Reference) {
42+
if (reference.identifier.name.startsWith("$")) {
43+
// It is reactive store reference.
44+
return true
45+
}
46+
if (!reference.resolved) {
47+
// Unknown variable
48+
return true
49+
}
50+
return isMutableVariable(reference.resolved)
51+
}
52+
53+
/**
54+
* Checks whether the given variable is a mutable variable or not.
55+
*/
56+
function isMutableVariable(variable: Variable) {
57+
const cache = cacheMutableVariable.get(variable)
58+
if (cache != null) {
59+
return cache
60+
}
61+
if (variable.defs.length === 0) {
62+
// Global variables are assumed to be immutable.
63+
return true
64+
}
65+
const isMutable = variable.defs.some((def) => {
66+
if (def.type === "Variable") {
67+
const parent = def.parent
68+
if (parent.kind === "const") {
69+
return false
70+
}
71+
const pp = parent.parent
72+
if (
73+
pp &&
74+
pp.type === "ExportNamedDeclaration" &&
75+
pp.declaration === parent
76+
) {
77+
// Props
78+
return true
79+
}
80+
return hasWrite(variable)
81+
}
82+
if (def.type === "ImportBinding") {
83+
return false
84+
}
85+
86+
if (def.node.type === "AssignmentExpression") {
87+
// Reactive values
88+
return true
89+
}
90+
return false
91+
})
92+
cacheMutableVariable.set(variable, isMutable)
93+
return isMutable
94+
}
95+
96+
/** Checks whether the given variable has a write or reactive store reference or not. */
97+
function hasWrite(variable: Variable) {
98+
const defIds = variable.defs.map((def: Definition) => def.name)
99+
return variable.references.some(
100+
(reference) =>
101+
reference.isWrite() &&
102+
!defIds.some(
103+
(defId) =>
104+
defId.range[0] <= reference.identifier.range[0] &&
105+
reference.identifier.range[1] <= defId.range[1],
106+
),
107+
)
108+
}
109+
110+
/**
111+
* Iterates through references to top-level variables in the given range.
112+
*/
113+
function* iterateRangeReferences(scope: Scope, range: [number, number]) {
114+
for (const variable of scope.variables) {
115+
for (const reference of variable.references) {
116+
if (
117+
range[0] <= reference.identifier.range[0] &&
118+
reference.identifier.range[1] <= range[1]
119+
) {
120+
yield reference
121+
}
122+
}
123+
}
124+
}
125+
126+
return {
127+
SvelteReactiveStatement(node: AST.SvelteReactiveStatement) {
128+
for (const reference of iterateRangeReferences(
129+
toplevelScope,
130+
node.range,
131+
)) {
132+
if (reference.isWriteOnly()) {
133+
continue
134+
}
135+
if (isMutableVariableReference(reference)) {
136+
return
137+
}
138+
}
139+
if (
140+
globalScope.through.some(
141+
(reference) =>
142+
node.range[0] <= reference.identifier.range[0] &&
143+
reference.identifier.range[1] <= node.range[1],
144+
)
145+
) {
146+
// Do not report if there are missing references.
147+
return
148+
}
149+
150+
context.report({
151+
node:
152+
node.body.type === "ExpressionStatement" &&
153+
node.body.expression.type === "AssignmentExpression" &&
154+
node.body.expression.operator === "="
155+
? node.body.expression.right
156+
: node.body,
157+
messageId: "immutable",
158+
})
159+
},
160+
}
161+
},
162+
})

src/utils/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import noDupeUseDirectives from "../rules/no-dupe-use-directives"
2424
import noDynamicSlotName from "../rules/no-dynamic-slot-name"
2525
import noExportLoadInSvelteModuleInKitPages from "../rules/no-export-load-in-svelte-module-in-kit-pages"
2626
import noExtraReactiveCurlies from "../rules/no-extra-reactive-curlies"
27+
import noImmutableReactiveStatements from "../rules/no-immutable-reactive-statements"
2728
import noInnerDeclarations from "../rules/no-inner-declarations"
2829
import noNotFunctionHandler from "../rules/no-not-function-handler"
2930
import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches"
@@ -79,6 +80,7 @@ export const rules = [
7980
noDynamicSlotName,
8081
noExportLoadInSvelteModuleInKitPages,
8182
noExtraReactiveCurlies,
83+
noImmutableReactiveStatements,
8284
noInnerDeclarations,
8385
noNotFunctionHandler,
8486
noObjectInTextMustaches,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
- message:
2+
This statement is not reactive because all variables referenced in the
3+
reactive statement are immutable.
4+
line: 3
5+
column: 18
6+
suggestions: null
7+
- message:
8+
This statement is not reactive because all variables referenced in the
9+
reactive statement are immutable.
10+
line: 4
11+
column: 18
12+
suggestions: null
13+
- message:
14+
This statement is not reactive because all variables referenced in the
15+
reactive statement are immutable.
16+
line: 5
17+
column: 6
18+
suggestions: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
let immutableVar = "hello"
3+
$: computed1 = `${immutableVar} ${immutableVar}`
4+
$: computed1 = fn1(immutableVar)
5+
$: console.log(immutableVar)
6+
7+
function fn1(v) {
8+
return `${v} ${v}`
9+
}
10+
</script>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
- message:
2+
This statement is not reactive because all variables referenced in the
3+
reactive statement are immutable.
4+
line: 7
5+
column: 18
6+
suggestions: null
7+
- message:
8+
This statement is not reactive because all variables referenced in the
9+
reactive statement are immutable.
10+
line: 8
11+
column: 18
12+
suggestions: null
13+
- message:
14+
This statement is not reactive because all variables referenced in the
15+
reactive statement are immutable.
16+
line: 9
17+
column: 6
18+
suggestions: null
19+
- message:
20+
This statement is not reactive because all variables referenced in the
21+
reactive statement are immutable.
22+
line: 10
23+
column: 6
24+
suggestions: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<script>
2+
import myVar from "./my-variables"
3+
let mutableVar = "hello"
4+
5+
const immutableVar = "hello"
6+
/* ✗ BAD */
7+
$: computed3 = fn1(immutableVar)
8+
$: computed4 = fn2()
9+
$: console.log(immutableVar)
10+
$: console.log(myVar)
11+
12+
function fn1(v) {
13+
return `${v} ${v}`
14+
}
15+
function fn2() {
16+
return `${mutableVar} ${mutableVar}`
17+
}
18+
</script>
19+
20+
<input bind:value={mutableVar} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
- message:
2+
This statement is not reactive because all variables referenced in the
3+
reactive statement are immutable.
4+
line: 12
5+
column: 17
6+
suggestions: null
7+
- message:
8+
This statement is not reactive because all variables referenced in the
9+
reactive statement are immutable.
10+
line: 13
11+
column: 17
12+
suggestions: null
13+
- message:
14+
This statement is not reactive because all variables referenced in the
15+
reactive statement are immutable.
16+
line: 14
17+
column: 17
18+
suggestions: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<script>
2+
export const thisIs = "readonly"
3+
4+
export function greet(name) {
5+
console.log(`hello ${name}!`)
6+
}
7+
8+
export class Foo {}
9+
10+
const immutableVar = "hello"
11+
12+
$: message1 = greet(immutableVar)
13+
$: message2 = `this is${thisIs}`
14+
$: instance = new Foo()
15+
</script>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<script>
2+
import myStore from "./my-stores"
3+
import myVar from "./my-variables"
4+
let mutableVar = "hello"
5+
export let prop
6+
/* ✓ GOOD */
7+
$: computed1 = `${mutableVar} ${mutableVar}`
8+
$: computed2 = fn1(mutableVar)
9+
$: console.log(mutableVar)
10+
$: console.log(computed1)
11+
$: console.log($myStore)
12+
$: console.log(prop)
13+
14+
function fn1(v) {
15+
return `${v} ${v}`
16+
}
17+
</script>
18+
19+
<input bind:value={mutableVar} />

0 commit comments

Comments
 (0)