Skip to content

Commit 2b7e164

Browse files
43081j43081j
authored
feat: add fixer to require-store-callbacks-use-set-param (#1165)
1 parent d9b8604 commit 2b7e164

File tree

10 files changed

+351
-41
lines changed

10 files changed

+351
-41
lines changed

Diff for: ‎.changeset/clear-mugs-rush.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-svelte': minor
3+
---
4+
5+
Adds a suggestion to the `require-store-callbacks-use-set-param` rule to automatically rename or add function parameters.

Diff for: ‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
272272
| [svelte/no-shorthand-style-property-overrides](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shorthand-style-property-overrides/) | disallow shorthand style properties that override related longhand properties | :star: |
273273
| [svelte/no-store-async](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: |
274274
| [svelte/no-unknown-style-directive-property](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: |
275-
| [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | |
275+
| [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | :bulb: |
276276
| [svelte/require-store-reactive-access](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: |
277277
| [svelte/valid-compile](https://sveltejs.github.io/eslint-plugin-svelte/rules/valid-compile/) | disallow warnings when compiling. | |
278278
| [svelte/valid-style-parse](https://sveltejs.github.io/eslint-plugin-svelte/rules/valid-style-parse/) | require valid style element parsing | |

Diff for: ‎docs/rules.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
2929
| [svelte/no-shorthand-style-property-overrides](./rules/no-shorthand-style-property-overrides.md) | disallow shorthand style properties that override related longhand properties | :star: |
3030
| [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: |
3131
| [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: |
32-
| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | |
32+
| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | :bulb: |
3333
| [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: |
3434
| [svelte/valid-compile](./rules/valid-compile.md) | disallow warnings when compiling. | |
3535
| [svelte/valid-style-parse](./rules/valid-style-parse.md) | require valid style element parsing | |

Diff for: ‎docs/rules/require-store-callbacks-use-set-param.md

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ since: 'v2.12.0'
1010

1111
> store callbacks must use `set` param
1212
13+
- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).
14+
1315
## :book: Rule Details
1416

1517
This rule disallows if `readable` / `writable` store's setter function doesn't use `set` parameter.<br>

Diff for: ‎packages/eslint-plugin-svelte/src/rules/derived-has-same-inputs-outputs.ts

+3-30
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,7 @@ import type { Variable } from '@typescript-eslint/scope-manager';
33
import { createRule } from '../utils/index.js';
44
import type { RuleContext, RuleFixer } from '../types.js';
55
import { extractStoreReferences } from './reference-helpers/svelte-store.js';
6-
import { getScope } from '../utils/ast-utils.js';
7-
8-
function findVariableForName(
9-
context: RuleContext,
10-
node: TSESTree.Node,
11-
name: string,
12-
expectedName: string
13-
): { hasConflict: boolean; variable: Variable | null } {
14-
const scope = getScope(context, node);
15-
let variable: Variable | null = null;
16-
17-
for (const ref of scope.references) {
18-
if (ref.identifier.name === expectedName) {
19-
return { hasConflict: true, variable: null };
20-
}
21-
}
22-
23-
for (const v of scope.variables) {
24-
if (v.name === expectedName) {
25-
return { hasConflict: true, variable: null };
26-
}
27-
if (v.name === name) {
28-
variable = v;
29-
}
30-
}
31-
32-
return { hasConflict: false, variable };
33-
}
6+
import { findVariableForReplacement } from '../utils/ast-utils.js';
347

358
function createFixer(node: TSESTree.Node, variable: Variable | null, name: string) {
369
return function* fix(fixer: RuleFixer) {
@@ -92,7 +65,7 @@ export default createRule('derived-has-same-inputs-outputs', {
9265
if (fnParam.type !== 'Identifier') return;
9366
const expectedName = `$${args.name}`;
9467
if (expectedName !== fnParam.name) {
95-
const { hasConflict, variable } = findVariableForName(
68+
const { hasConflict, variable } = findVariableForReplacement(
9669
context,
9770
fn.body,
9871
fnParam.name,
@@ -136,7 +109,7 @@ export default createRule('derived-has-same-inputs-outputs', {
136109
if (element && element.type === 'Identifier' && argName) {
137110
const expectedName = `$${argName}`;
138111
if (expectedName !== element.name) {
139-
const { hasConflict, variable } = findVariableForName(
112+
const { hasConflict, variable } = findVariableForReplacement(
140113
context,
141114
fn.body,
142115
element.name,

Diff for: ‎packages/eslint-plugin-svelte/src/rules/require-store-callbacks-use-set-param.ts

+46-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
import { findVariableForReplacement } from '../utils/ast-utils.js';
2+
import type { SuggestionReportDescriptor } from '../types.js';
13
import { createRule } from '../utils/index.js';
24
import { extractStoreReferences } from './reference-helpers/svelte-store.js';
5+
import { getSourceCode } from '../utils/compat.js';
36

47
export default createRule('require-store-callbacks-use-set-param', {
58
meta: {
@@ -8,9 +11,12 @@ export default createRule('require-store-callbacks-use-set-param', {
811
category: 'Possible Errors',
912
recommended: false
1013
},
14+
hasSuggestions: true,
1115
schema: [],
1216
messages: {
13-
unexpected: 'Store callbacks must use `set` param.'
17+
unexpected: 'Store callbacks must use `set` param.',
18+
updateParam: 'Rename parameter from {{oldName}} to `set`.',
19+
addParam: 'Add a `set` parameter.'
1420
},
1521
type: 'suggestion'
1622
},
@@ -24,10 +30,48 @@ export default createRule('require-store-callbacks-use-set-param', {
2430
}
2531
const param = fn.params[0];
2632
if (!param || (param.type === 'Identifier' && param.name !== 'set')) {
33+
const { hasConflict, variable } = findVariableForReplacement(
34+
context,
35+
fn.body,
36+
param ? param.name : null,
37+
'set'
38+
);
39+
const suggest: SuggestionReportDescriptor[] = [];
40+
if (!hasConflict) {
41+
if (param) {
42+
suggest.push({
43+
messageId: 'updateParam',
44+
data: { oldName: param.name },
45+
*fix(fixer) {
46+
yield fixer.replaceText(param, 'set');
47+
48+
if (variable) {
49+
for (const ref of variable.references) {
50+
yield fixer.replaceText(ref.identifier, 'set');
51+
}
52+
}
53+
}
54+
});
55+
} else {
56+
const token = getSourceCode(context).getTokenBefore(fn.body, {
57+
filter: (token) => token.type === 'Punctuator' && token.value === '(',
58+
includeComments: false
59+
});
60+
if (token) {
61+
suggest.push({
62+
messageId: 'addParam',
63+
fix(fixer) {
64+
return fixer.insertTextAfter(token, 'set');
65+
}
66+
});
67+
}
68+
}
69+
}
2770
context.report({
2871
node: fn,
2972
loc: fn.loc,
30-
messageId: 'unexpected'
73+
messageId: 'unexpected',
74+
suggest
3175
});
3276
}
3377
}

Diff for: ‎packages/eslint-plugin-svelte/src/utils/ast-utils.ts

+34
Original file line numberDiff line numberDiff line change
@@ -685,3 +685,37 @@ function getSimpleNameFromNode(
685685

686686
return getSourceCode(context).getText(node);
687687
}
688+
689+
/**
690+
* Finds the variable for a given name in the specified node's scope.
691+
* Also determines if the replacement name is already in use.
692+
*
693+
* If the `name` is set to null, this assumes you're adding a new variable
694+
* and reports if it is already in use.
695+
*/
696+
export function findVariableForReplacement(
697+
context: RuleContext,
698+
node: TSESTree.Node,
699+
name: string | null,
700+
replacementName: string
701+
): { hasConflict: boolean; variable: Variable | null } {
702+
const scope = getScope(context, node);
703+
let variable: Variable | null = null;
704+
705+
for (const ref of scope.references) {
706+
if (ref.identifier.name === replacementName) {
707+
return { hasConflict: true, variable: null };
708+
}
709+
}
710+
711+
for (const v of scope.variables) {
712+
if (v.name === replacementName) {
713+
return { hasConflict: true, variable: null };
714+
}
715+
if (v.name === name) {
716+
variable = v;
717+
}
718+
}
719+
720+
return { hasConflict: false, variable };
721+
}

Diff for: ‎packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-errors.yaml

+185-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,198 @@
11
- message: Store callbacks must use `set` param.
22
line: 4
33
column: 18
4-
suggestions: null
4+
suggestions:
5+
- desc: Add a `set` parameter.
6+
messageId: addParam
7+
output: |
8+
<script>
9+
import { readable, writable } from 'svelte/store';
10+
11+
readable(false, (set) => true);
12+
readable(false, (foo) => true);
13+
14+
writable(false, () => true);
15+
writable(false, (foo) => true);
16+
17+
readable(false, (foo) => {
18+
foo;
19+
insideACallback(() => {
20+
foo;
21+
});
22+
conflictingName(() => {
23+
const foo = 303;
24+
foo;
25+
});
26+
});
27+
28+
readable(false, () => {
29+
const set = 303;
30+
});
31+
32+
insideACallback(() => {
33+
const set = 303;
34+
readable(false, () => {
35+
set;
36+
});
37+
});
38+
</script>
539
- message: Store callbacks must use `set` param.
640
line: 5
741
column: 18
8-
suggestions: null
42+
suggestions:
43+
- desc: Rename parameter from foo to `set`.
44+
messageId: updateParam
45+
output: |
46+
<script>
47+
import { readable, writable } from 'svelte/store';
48+
49+
readable(false, () => true);
50+
readable(false, (set) => true);
51+
52+
writable(false, () => true);
53+
writable(false, (foo) => true);
54+
55+
readable(false, (foo) => {
56+
foo;
57+
insideACallback(() => {
58+
foo;
59+
});
60+
conflictingName(() => {
61+
const foo = 303;
62+
foo;
63+
});
64+
});
65+
66+
readable(false, () => {
67+
const set = 303;
68+
});
69+
70+
insideACallback(() => {
71+
const set = 303;
72+
readable(false, () => {
73+
set;
74+
});
75+
});
76+
</script>
977
- message: Store callbacks must use `set` param.
1078
line: 7
1179
column: 18
12-
suggestions: null
80+
suggestions:
81+
- desc: Add a `set` parameter.
82+
messageId: addParam
83+
output: |
84+
<script>
85+
import { readable, writable } from 'svelte/store';
86+
87+
readable(false, () => true);
88+
readable(false, (foo) => true);
89+
90+
writable(false, (set) => true);
91+
writable(false, (foo) => true);
92+
93+
readable(false, (foo) => {
94+
foo;
95+
insideACallback(() => {
96+
foo;
97+
});
98+
conflictingName(() => {
99+
const foo = 303;
100+
foo;
101+
});
102+
});
103+
104+
readable(false, () => {
105+
const set = 303;
106+
});
107+
108+
insideACallback(() => {
109+
const set = 303;
110+
readable(false, () => {
111+
set;
112+
});
113+
});
114+
</script>
13115
- message: Store callbacks must use `set` param.
14116
line: 8
15117
column: 18
118+
suggestions:
119+
- desc: Rename parameter from foo to `set`.
120+
messageId: updateParam
121+
output: |
122+
<script>
123+
import { readable, writable } from 'svelte/store';
124+
125+
readable(false, () => true);
126+
readable(false, (foo) => true);
127+
128+
writable(false, () => true);
129+
writable(false, (set) => true);
130+
131+
readable(false, (foo) => {
132+
foo;
133+
insideACallback(() => {
134+
foo;
135+
});
136+
conflictingName(() => {
137+
const foo = 303;
138+
foo;
139+
});
140+
});
141+
142+
readable(false, () => {
143+
const set = 303;
144+
});
145+
146+
insideACallback(() => {
147+
const set = 303;
148+
readable(false, () => {
149+
set;
150+
});
151+
});
152+
</script>
153+
- message: Store callbacks must use `set` param.
154+
line: 10
155+
column: 18
156+
suggestions:
157+
- desc: Rename parameter from foo to `set`.
158+
messageId: updateParam
159+
output: |
160+
<script>
161+
import { readable, writable } from 'svelte/store';
162+
163+
readable(false, () => true);
164+
readable(false, (foo) => true);
165+
166+
writable(false, () => true);
167+
writable(false, (foo) => true);
168+
169+
readable(false, (set) => {
170+
set;
171+
insideACallback(() => {
172+
set;
173+
});
174+
conflictingName(() => {
175+
const foo = 303;
176+
foo;
177+
});
178+
});
179+
180+
readable(false, () => {
181+
const set = 303;
182+
});
183+
184+
insideACallback(() => {
185+
const set = 303;
186+
readable(false, () => {
187+
set;
188+
});
189+
});
190+
</script>
191+
- message: Store callbacks must use `set` param.
192+
line: 21
193+
column: 18
194+
suggestions: null
195+
- message: Store callbacks must use `set` param.
196+
line: 27
197+
column: 19
16198
suggestions: null

0 commit comments

Comments
 (0)