Skip to content

Commit 4643b9b

Browse files
ota-meshiota-meshi
committed
fix: add testcases and fix
1 parent e15152c commit 4643b9b

File tree

11 files changed

+456
-52
lines changed

11 files changed

+456
-52
lines changed

‎src/rules/prefer-destructured-store-props.ts

+149-48
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import type { TSESTree } from "@typescript-eslint/types"
2-
import { findVariable } from "eslint-utils"
2+
import { getPropertyName } from "eslint-utils"
33
import type { AST } from "svelte-eslint-parser"
4+
import type { SuggestionReportDescriptor } from "../types"
45
import { createRule } from "../utils"
5-
import { findAttribute, isExpressionIdentifier } from "../utils/ast-utils"
6+
import {
7+
findAttribute,
8+
isExpressionIdentifier,
9+
findVariable,
10+
} from "../utils/ast-utils"
611

712
type StoreMemberExpression = TSESTree.MemberExpression & {
813
object: TSESTree.Identifier & { name: string }
@@ -86,6 +91,7 @@ export default createRule("prefer-destructured-store-props", {
8691
messages: {
8792
useDestructuring: `Destructure {{property}} from {{store}} for better change tracking & fewer redraws`,
8893
fixUseDestructuring: `Using destructuring like $: ({ {{property}} } = {{store}}); will run faster`,
94+
fixUseVariable: `Using the predefined reactive variable {{variable}}`,
8995
},
9096
type: "suggestion",
9197
},
@@ -102,6 +108,79 @@ export default createRule("prefer-destructured-store-props", {
102108
identifiers: TSESTree.Identifier[]
103109
}[] = []
104110

111+
/** Find for defined reactive variables. */
112+
function* findReactiveVariable(
113+
object: TSESTree.Identifier,
114+
propName: string,
115+
): Iterable<TSESTree.Identifier> {
116+
const storeVar = findVariable(context, object)
117+
if (!storeVar) {
118+
return
119+
}
120+
121+
for (const reference of storeVar.references) {
122+
const id = reference.identifier as TSESTree.Identifier
123+
if (id.name !== object.name) continue
124+
if (isReactiveVariableDefinitionWithMemberExpression(id)) {
125+
// $: target = $store.prop
126+
yield id.parent.parent.left
127+
} else if (isReactiveVariableDefinitionWithDestructuring(id)) {
128+
const prop = id.parent.left.properties.find(
129+
(
130+
prop,
131+
): prop is TSESTree.Property & { value: TSESTree.Identifier } =>
132+
prop.type === "Property" &&
133+
prop.value.type === "Identifier" &&
134+
getPropertyName(prop) === propName,
135+
)
136+
if (prop) {
137+
yield prop.value
138+
}
139+
}
140+
}
141+
142+
/** Checks whether the given node is reactive variable definition with member expression. */
143+
function isReactiveVariableDefinitionWithMemberExpression(
144+
node: TSESTree.Identifier,
145+
): node is TSESTree.Identifier & {
146+
parent: TSESTree.MemberExpression & {
147+
parent: TSESTree.AssignmentExpression & { left: TSESTree.Identifier }
148+
}
149+
} {
150+
return (
151+
node.parent?.type === "MemberExpression" &&
152+
node.parent.object === node &&
153+
getPropertyName(node.parent) === propName &&
154+
node.parent.parent?.type === "AssignmentExpression" &&
155+
node.parent.parent.right === node.parent &&
156+
node.parent.parent.left.type === "Identifier" &&
157+
node.parent.parent.parent?.type === "ExpressionStatement" &&
158+
(
159+
node.parent.parent.parent
160+
.parent as never as AST.SvelteReactiveStatement
161+
)?.type === "SvelteReactiveStatement"
162+
)
163+
}
164+
165+
/** Checks whether the given node is reactive variable definition with destructuring. */
166+
function isReactiveVariableDefinitionWithDestructuring(
167+
node: TSESTree.Identifier,
168+
): node is TSESTree.Identifier & {
169+
parent: TSESTree.AssignmentExpression & {
170+
left: TSESTree.ObjectPattern
171+
}
172+
} {
173+
return (
174+
node.parent?.type === "AssignmentExpression" &&
175+
node.parent.right === node &&
176+
node.parent.left.type === "ObjectPattern" &&
177+
node.parent.parent?.type === "ExpressionStatement" &&
178+
(node.parent.parent.parent as never as AST.SvelteReactiveStatement)
179+
?.type === "SvelteReactiveStatement"
180+
)
181+
}
182+
}
183+
105184
/** Checks whether the given name is already defined as a variable. */
106185
function hasTopLevelVariable(name: string) {
107186
const scopeManager = context.getSourceCode().scopeManager
@@ -111,7 +190,7 @@ export default createRule("prefer-destructured-store-props", {
111190
const moduleScope = scopeManager.globalScope?.childScopes.find(
112191
(s) => s.type === "module",
113192
)
114-
return moduleScope?.set.has(name) ?? false
193+
return moduleScope?.set.has(name) || false
115194
}
116195

117196
return {
@@ -156,7 +235,7 @@ export default createRule("prefer-destructured-store-props", {
156235

157236
for (const id of identifiers) {
158237
if (!isExpressionIdentifier(id)) continue
159-
const variable = findVariable(context.getScope(), id)
238+
const variable = findVariable(context, id)
160239
const isTopLevel =
161240
!variable ||
162241
variable.scope.type === "module" ||
@@ -174,6 +253,71 @@ export default createRule("prefer-destructured-store-props", {
174253
for (const node of reports) {
175254
const store = node.object.name
176255

256+
const suggest: SuggestionReportDescriptor[] = []
257+
if (
258+
// Avoid suggestions for:
259+
// dynamic accesses like {$foo[bar]}
260+
!node.computed
261+
) {
262+
for (const variable of findReactiveVariable(
263+
node.object,
264+
node.property.name,
265+
)) {
266+
suggest.push({
267+
messageId: "fixUseVariable",
268+
data: {
269+
variable: variable.name,
270+
},
271+
272+
fix(fixer) {
273+
return fixer.replaceText(node, variable.name)
274+
},
275+
})
276+
}
277+
278+
if (
279+
// Avoid suggestions for:
280+
// no <script> tag
281+
// no <script> ending
282+
scriptEndTag
283+
) {
284+
suggest.push({
285+
messageId: "fixUseDestructuring",
286+
data: {
287+
store,
288+
property: node.property.name,
289+
},
290+
291+
fix(fixer) {
292+
const propName = node.property.name
293+
294+
let varName = propName
295+
if (varName.startsWith("$")) {
296+
varName = varName.slice(1)
297+
}
298+
const baseName = varName
299+
let suffix = 0
300+
if (keywords.has(varName)) {
301+
varName = `${baseName}${++suffix}`
302+
}
303+
while (hasTopLevelVariable(varName)) {
304+
varName = `${baseName}${++suffix}`
305+
}
306+
307+
return [
308+
fixer.insertTextAfterRange(
309+
[scriptEndTag.range[0], scriptEndTag.range[0]],
310+
`$: ({ ${propName}${
311+
propName !== varName ? `: ${varName}` : ""
312+
} } = ${store});\n`,
313+
),
314+
fixer.replaceText(node, varName),
315+
]
316+
},
317+
})
318+
}
319+
}
320+
177321
context.report({
178322
node,
179323
messageId: "useDestructuring",
@@ -187,50 +331,7 @@ export default createRule("prefer-destructured-store-props", {
187331
.replace(/\s+/g, " "),
188332
},
189333

190-
// Avoid suggestions for:
191-
// dynamic accesses like {$foo[bar]}
192-
// no <script> tag
193-
// no <script> ending
194-
// variable name already defined
195-
suggest:
196-
node.computed || !scriptEndTag
197-
? []
198-
: [
199-
{
200-
messageId: "fixUseDestructuring",
201-
data: {
202-
store,
203-
property: node.property.name,
204-
},
205-
206-
fix(fixer) {
207-
const propName = node.property.name
208-
209-
let varName = propName
210-
if (varName.startsWith("$")) {
211-
varName = varName.slice(1)
212-
}
213-
const baseName = varName
214-
let suffix = 0
215-
if (keywords.has(varName)) {
216-
varName = `${baseName}${++suffix}`
217-
}
218-
while (hasTopLevelVariable(varName)) {
219-
varName = `${baseName}${++suffix}`
220-
}
221-
222-
return [
223-
fixer.insertTextAfterRange(
224-
[scriptEndTag.range[0], scriptEndTag.range[0]],
225-
`$: ({ ${propName}${
226-
propName !== varName ? `: ${varName}` : ""
227-
} } = ${store});\n`,
228-
),
229-
fixer.replaceText(node, varName),
230-
]
231-
},
232-
},
233-
],
334+
suggest,
234335
})
235336
}
236337
},

‎src/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ interface ReportDescriptorOptionsBase {
168168
}
169169

170170
type SuggestionDescriptorMessage = { desc: string } | { messageId: string }
171-
type SuggestionReportDescriptor = SuggestionDescriptorMessage &
171+
export type SuggestionReportDescriptor = SuggestionDescriptorMessage &
172172
ReportDescriptorOptionsBase
173173

174174
interface ReportDescriptorOptions extends ReportDescriptorOptionsBase {

‎src/utils/ast-utils.ts

+14-2
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,21 @@ export function getLangValue(
237237
*/
238238
export function findVariable(
239239
context: RuleContext,
240-
node: ESTree.Identifier,
240+
node: ESTree.Identifier | TSESTree.Identifier,
241241
): Scope.Variable | null {
242-
return eslintUtils.findVariable(getScope(context, node), node)
242+
const initialScope = eslintUtils.getInnermostScope(
243+
getScope(context, node),
244+
node,
245+
)
246+
const variable = eslintUtils.findVariable(initialScope, node)
247+
if (variable) {
248+
return variable
249+
}
250+
if (!node.name.startsWith("$")) {
251+
return variable
252+
}
253+
// Remove the $ and search for the variable again, as it may be a store access variable.
254+
return eslintUtils.findVariable(initialScope, node.name.slice(1))
243255
}
244256

245257
/**

‎tests/fixtures/rules/prefer-destructured-store-props/invalid/member01-errors.yaml

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
- message: Destructure foo from $store for better change tracking & fewer redraws
2+
line: 5
3+
column: 11
4+
suggestions:
5+
- desc: "Using destructuring like $: ({ foo } = $store); will run faster"
6+
messageId: fixUseDestructuring
7+
output: |
8+
<script>
9+
import store from "./store.js"
10+
$: ({ foo } = $store);
11+
</script>
12+
13+
foo.bar: {foo.bar}
14+
foo.baz: {$store.foo.baz}
15+
- message: Destructure foo from $store for better change tracking & fewer redraws
16+
line: 6
17+
column: 11
18+
suggestions:
19+
- desc: "Using destructuring like $: ({ foo } = $store); will run faster"
20+
messageId: fixUseDestructuring
21+
output: |
22+
<script>
23+
import store from "./store.js"
24+
$: ({ foo } = $store);
25+
</script>
26+
27+
foo.bar: {$store.foo.bar}
28+
foo.baz: {foo.baz}

‎tests/fixtures/rules/prefer-destructured-store-props/invalid/member01-input.svelte

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<script>
2+
import store from "./store.js"
3+
</script>
4+
5+
foo.bar: {$store.foo.bar}
6+
foo.baz: {$store.foo.baz}

0 commit comments

Comments
 (0)