Skip to content

Commit 1b9a07a

Browse files
43081j43081j
committed
fix: reuse helper from ast-utils
1 parent e22c62e commit 1b9a07a

File tree

5 files changed

+157
-132
lines changed

5 files changed

+157
-132
lines changed

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

Lines changed: 3 additions & 30 deletions
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,

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

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,9 @@
1-
import { getScope } from '../utils/ast-utils.js';
2-
import type { RuleContext, SuggestionReportDescriptor } from '../types.js';
1+
import { findVariableForReplacement } from '../utils/ast-utils.js';
2+
import type { SuggestionReportDescriptor } from '../types.js';
33
import { createRule } from '../utils/index.js';
44
import { extractStoreReferences } from './reference-helpers/svelte-store.js';
5-
import type { TSESTree } from '@typescript-eslint/types';
6-
import type { Variable } from '@typescript-eslint/scope-manager';
75
import { getSourceCode } from '../utils/compat.js';
86

9-
function findVariableForName(
10-
context: RuleContext,
11-
node: TSESTree.Node,
12-
name: string | null,
13-
expectedName: string
14-
): { hasConflict: boolean; variable: Variable | null } {
15-
const scope = getScope(context, node);
16-
let hasConflict = false;
17-
let variable: Variable | null = null;
18-
19-
for (const v of scope.variables) {
20-
if (hasConflict && (variable || name === null)) {
21-
break;
22-
}
23-
if (v.name === expectedName) {
24-
hasConflict = true;
25-
continue;
26-
}
27-
if (name !== null && v.name === name) {
28-
variable = v;
29-
}
30-
}
31-
32-
return { hasConflict, variable };
33-
}
34-
357
export default createRule('require-store-callbacks-use-set-param', {
368
meta: {
379
docs: {
@@ -58,7 +30,7 @@ export default createRule('require-store-callbacks-use-set-param', {
5830
}
5931
const param = fn.params[0];
6032
if (!param || (param.type === 'Identifier' && param.name !== 'set')) {
61-
const { hasConflict, variable } = findVariableForName(
33+
const { hasConflict, variable } = findVariableForReplacement(
6234
context,
6335
fn.body,
6436
param ? param.name : null,

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

Lines changed: 34 additions & 0 deletions
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+
}

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

Lines changed: 110 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,27 @@
1414
writable(false, () => true);
1515
writable(false, (foo) => true);
1616
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-
});
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+
});
3138
</script>
3239
- message: Store callbacks must use `set` param.
3340
line: 5
@@ -45,20 +52,27 @@
4552
writable(false, () => true);
4653
writable(false, (foo) => true);
4754
48-
readable(false, (foo) => {
49-
foo;
50-
insideACallback(() => {
51-
foo;
52-
});
53-
conflictingName(() => {
54-
const foo = 303;
55-
foo;
56-
});
57-
});
58-
59-
readable(false, () => {
60-
const set = 303;
61-
});
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+
});
6276
</script>
6377
- message: Store callbacks must use `set` param.
6478
line: 7
@@ -76,20 +90,27 @@
7690
writable(false, (set) => true);
7791
writable(false, (foo) => true);
7892
79-
readable(false, (foo) => {
80-
foo;
81-
insideACallback(() => {
82-
foo;
83-
});
84-
conflictingName(() => {
85-
const foo = 303;
86-
foo;
87-
});
88-
});
89-
90-
readable(false, () => {
91-
const set = 303;
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+
});
93114
</script>
94115
- message: Store callbacks must use `set` param.
95116
line: 8
@@ -107,24 +128,31 @@
107128
writable(false, () => true);
108129
writable(false, (set) => true);
109130
110-
readable(false, (foo) => {
111-
foo;
112-
insideACallback(() => {
113-
foo;
114-
});
115-
conflictingName(() => {
116-
const foo = 303;
117-
foo;
118-
});
119-
});
120-
121-
readable(false, () => {
122-
const set = 303;
123-
});
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+
});
124152
</script>
125153
- message: Store callbacks must use `set` param.
126154
line: 10
127-
column: 19
155+
column: 18
128156
suggestions:
129157
- desc: Rename parameter from foo to `set`.
130158
messageId: updateParam
@@ -138,22 +166,33 @@
138166
writable(false, () => true);
139167
writable(false, (foo) => true);
140168
141-
readable(false, (set) => {
142-
set;
143-
insideACallback(() => {
144-
set;
145-
});
146-
conflictingName(() => {
147-
const foo = 303;
148-
foo;
149-
});
150-
});
151-
152-
readable(false, () => {
153-
const set = 303;
154-
});
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+
});
155190
</script>
156191
- message: Store callbacks must use `set` param.
157192
line: 21
193+
column: 18
194+
suggestions: null
195+
- message: Store callbacks must use `set` param.
196+
line: 27
158197
column: 19
159198
suggestions: null

‎packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-input.svelte‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,11 @@
2121
readable(false, () => {
2222
const set = 303;
2323
});
24+
25+
insideACallback(() => {
26+
const set = 303;
27+
readable(false, () => {
28+
set;
29+
});
30+
});
2431
</script>

0 commit comments

Comments
 (0)