Skip to content

Commit 0259ebf

Browse files
committed
fix(eslint-plugin): address CodeRabbit review comments
- Fix AST parsing to handle destructuring patterns and aliasing - Add extractReturnIdentifiers function for better return property detection - Handle spread elements to avoid false positives - Improve type safety by removing any types - Support object-form defineStore calls with id property - Support qualified/namespaced store calls - Fix message interpolation in prefer-use-store-naming rule - Change autofixes to suggestions for safer refactoring - Update documentation to align with rule behavior
1 parent a564685 commit 0259ebf

11 files changed

+318
-305
lines changed

packages/eslint-plugin/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export const useUserStore = defineStore('user', () => {
128128
const name = ref('John')
129129

130130
function updateProfile() {
131-
// Use stores in actions or computed properties
131+
// Use stores in actions
132132
const cartStore = useCartStore()
133133
cartStore.clear()
134134
}

packages/eslint-plugin/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@
5555
"typescript": "~5.8.3",
5656
"vitest": "^3.2.4"
5757
},
58+
"engines": {
59+
"node": ">=18.18.0"
60+
},
5861
"peerDependencies": {
5962
"eslint": ">=8.0.0"
6063
},

packages/eslint-plugin/src/index.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { requireSetupStorePropertiesExport } from './rules/require-setup-store-p
77
import { noCircularStoreDependencies } from './rules/no-circular-store-dependencies'
88
import { preferUseStoreNaming } from './rules/prefer-use-store-naming'
99
import { noStoreInComputed } from './rules/no-store-in-computed'
10+
import packageJson from '../package.json'
1011

1112
/**
1213
* ESLint plugin for Pinia best practices and common patterns.
@@ -18,25 +19,29 @@ import { noStoreInComputed } from './rules/no-store-in-computed'
1819
const plugin = {
1920
meta: {
2021
name: '@pinia/eslint-plugin',
21-
version: '1.0.0',
22+
version: packageJson.version,
2223
},
2324
rules: {
2425
'require-setup-store-properties-export': requireSetupStorePropertiesExport,
2526
'no-circular-store-dependencies': noCircularStoreDependencies,
2627
'prefer-use-store-naming': preferUseStoreNaming,
2728
'no-store-in-computed': noStoreInComputed,
2829
},
29-
configs: {
30-
recommended: {
31-
plugins: ['@pinia'],
30+
}
31+
32+
// Flat config export
33+
;(plugin as any).configs = {
34+
recommended: [
35+
{
36+
plugins: { '@pinia': plugin as any },
3237
rules: {
3338
'@pinia/require-setup-store-properties-export': 'error',
3439
'@pinia/no-circular-store-dependencies': 'warn',
3540
'@pinia/prefer-use-store-naming': 'warn',
3641
'@pinia/no-store-in-computed': 'error',
3742
},
3843
},
39-
},
44+
],
4045
}
4146

4247
export default plugin

packages/eslint-plugin/src/rules/__tests__/prefer-use-store-naming.test.ts

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,14 @@ ruleTester.run('prefer-use-store-naming', preferUseStoreNaming, {
4949
errors: [
5050
{
5151
messageId: 'invalidNaming',
52+
suggestions: [
53+
{
54+
desc: 'Rename to "useDataStore"',
55+
output: `export const useDataStore = defineStore('data', () => {})`,
56+
},
57+
],
5258
},
5359
],
54-
output: `export const useDataStore = defineStore('data', () => {})`,
5560
},
5661
// Missing 'Store' suffix
5762
{
@@ -64,14 +69,19 @@ ruleTester.run('prefer-use-store-naming', preferUseStoreNaming, {
6469
errors: [
6570
{
6671
messageId: 'invalidNaming',
67-
},
68-
],
69-
output: `
72+
suggestions: [
73+
{
74+
desc: 'Rename to "useUserStore"',
75+
output: `
7076
export const useUserStore = defineStore('user', () => {
7177
const name = ref('John')
7278
return { name }
7379
})
7480
`,
81+
},
82+
],
83+
},
84+
],
7585
},
7686
// Completely wrong naming
7787
{
@@ -84,14 +94,19 @@ ruleTester.run('prefer-use-store-naming', preferUseStoreNaming, {
8494
errors: [
8595
{
8696
messageId: 'invalidNaming',
87-
},
88-
],
89-
output: `
97+
suggestions: [
98+
{
99+
desc: 'Rename to "useUserStore"',
100+
output: `
90101
export const useUserStore = defineStore('user', () => {
91102
const name = ref('John')
92103
return { name }
93104
})
94105
`,
106+
},
107+
],
108+
},
109+
],
95110
},
96111
// Kebab-case store ID
97112
{
@@ -104,14 +119,19 @@ ruleTester.run('prefer-use-store-naming', preferUseStoreNaming, {
104119
errors: [
105120
{
106121
messageId: 'invalidNaming',
107-
},
108-
],
109-
output: `
122+
suggestions: [
123+
{
124+
desc: 'Rename to "useShoppingCartStore"',
125+
output: `
110126
export const useShoppingCartStore = defineStore('shopping-cart', () => {
111127
const items = ref([])
112128
return { items }
113129
})
114130
`,
131+
},
132+
],
133+
},
134+
],
115135
},
116136
],
117137
})
@@ -144,14 +164,19 @@ ruleTester.run(
144164
errors: [
145165
{
146166
messageId: 'invalidNaming',
147-
},
148-
],
149-
output: `
167+
suggestions: [
168+
{
169+
desc: 'Rename to "createUserRepository"',
170+
output: `
150171
export const createUserRepository = defineStore('user', () => {
151172
const name = ref('John')
152173
return { name }
153174
})
154175
`,
176+
},
177+
],
178+
},
179+
],
155180
},
156181
],
157182
}

packages/eslint-plugin/src/rules/no-circular-store-dependencies.ts

Lines changed: 65 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
* @author Eduardo San Martin Morote
44
*/
55

6-
import { ESLintUtils, type TSESTree } from '@typescript-eslint/utils'
6+
import {
7+
ESLintUtils,
8+
type TSESTree,
9+
type TSESLint,
10+
} from '@typescript-eslint/utils'
711
import {
812
isDefineStoreCall,
913
isSetupStore,
@@ -34,13 +38,14 @@ export const noCircularStoreDependencies = createRule({
3438
circularDependency:
3539
'Potential circular dependency detected: store "{{currentStore}}" uses "{{usedStore}}"',
3640
setupCircularDependency:
37-
'Avoid using other stores directly in setup function body. Use them in computed properties or actions instead.',
41+
'Avoid using other stores directly in setup function body. Use them in actions instead.',
3842
},
3943
},
4044
defaultOptions: [],
4145
create(context) {
4246
const storeUsages = new Map<string, string[]>() // currentStore -> [usedStores]
43-
let currentStoreName: string | null = null
47+
const usageGraph = new Map<string, Map<string, TSESTree.Node[]>>() // currentStore -> usedStore -> nodes
48+
const storeStack: string[] = []
4449

4550
return {
4651
CallExpression(node: TSESTree.CallExpression) {
@@ -52,28 +57,29 @@ export const noCircularStoreDependencies = createRule({
5257
parent?.type === 'VariableDeclarator' &&
5358
parent.id.type === 'Identifier'
5459
) {
55-
currentStoreName = parent.id.name
60+
const currentStoreName = parent.id.name
61+
storeStack.push(currentStoreName)
5662

5763
// Initialize usage tracking for this store
5864
if (!storeUsages.has(currentStoreName)) {
5965
storeUsages.set(currentStoreName, [])
6066
}
67+
if (!usageGraph.has(currentStoreName)) {
68+
usageGraph.set(currentStoreName, new Map())
69+
}
6170

6271
// Check for store usage in setup function
6372
if (isSetupStore(node)) {
6473
const setupFunction = getSetupFunction(node)
6574
if (setupFunction) {
66-
checkSetupFunctionForStoreUsage(
67-
setupFunction,
68-
currentStoreName,
69-
context
70-
)
75+
checkSetupFunctionForStoreUsage(setupFunction, context)
7176
}
7277
}
7378
}
7479
}
7580

7681
// Track store usage calls
82+
const currentStoreName = storeStack[storeStack.length - 1]
7783
if (isStoreUsage(node) && currentStoreName) {
7884
const usedStoreName = getStoreNameFromUsage(node)
7985
if (usedStoreName && usedStoreName !== currentStoreName) {
@@ -82,6 +88,11 @@ export const noCircularStoreDependencies = createRule({
8288
usages.push(usedStoreName)
8389
storeUsages.set(currentStoreName, usages)
8490
}
91+
// record node for later reporting
92+
const edges = usageGraph.get(currentStoreName)!
93+
const nodes = edges.get(usedStoreName) ?? []
94+
nodes.push(node)
95+
edges.set(usedStoreName, nodes)
8596

8697
// Check for immediate circular dependency
8798
const usedStoreUsages = storeUsages.get(usedStoreName) || []
@@ -99,9 +110,15 @@ export const noCircularStoreDependencies = createRule({
99110
}
100111
},
101112

113+
'CallExpression:exit'(node: TSESTree.CallExpression) {
114+
if (isDefineStoreCall(node)) {
115+
storeStack.pop()
116+
}
117+
},
118+
102119
'Program:exit'() {
103120
// Check for indirect circular dependencies
104-
checkIndirectCircularDependencies(storeUsages, context)
121+
checkIndirectCircularDependencies(usageGraph, context)
105122
},
106123
}
107124
},
@@ -112,8 +129,10 @@ export const noCircularStoreDependencies = createRule({
112129
*/
113130
function checkSetupFunctionForStoreUsage(
114131
setupFunction: TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression,
115-
currentStoreName: string,
116-
context: any
132+
context: TSESLint.RuleContext<
133+
'circularDependency' | 'setupCircularDependency',
134+
[]
135+
>
117136
) {
118137
if (setupFunction.body.type !== 'BlockStatement') {
119138
return
@@ -150,40 +169,47 @@ function checkSetupFunctionForStoreUsage(
150169
* Checks for indirect circular dependencies (A -> B -> C -> A)
151170
*/
152171
function checkIndirectCircularDependencies(
153-
storeUsages: Map<string, string[]>,
154-
context: any
172+
usageGraph: Map<string, Map<string, TSESTree.Node[]>>,
173+
context: TSESLint.RuleContext<
174+
'circularDependency' | 'setupCircularDependency',
175+
[]
176+
>
155177
) {
156178
const visited = new Set<string>()
157-
const recursionStack = new Set<string>()
158-
159-
function hasCycle(store: string, path: string[] = []): boolean {
160-
if (recursionStack.has(store)) {
161-
// Found a cycle
162-
return true
163-
}
164-
165-
if (visited.has(store)) {
166-
return false
167-
}
179+
const inPath = new Set<string>()
180+
const path: string[] = []
181+
const reported = new Set<string>() // "A->B"
168182

183+
const dfs = (store: string) => {
169184
visited.add(store)
170-
recursionStack.add(store)
171-
172-
const dependencies = storeUsages.get(store) || []
173-
for (const dependency of dependencies) {
174-
if (hasCycle(dependency, [...path, store])) {
175-
return true
185+
inPath.add(store)
186+
path.push(store)
187+
const deps = usageGraph.get(store) ?? new Map()
188+
for (const [dep] of deps) {
189+
if (!visited.has(dep)) dfs(dep)
190+
if (inPath.has(dep)) {
191+
// report the edge(s) participating in the cycle at least once
192+
const from = store
193+
const to = dep
194+
const key = `${from}->${to}`
195+
if (!reported.has(key)) {
196+
reported.add(key)
197+
const node = usageGraph.get(from)?.get(to)?.[0]
198+
if (node) {
199+
context.report({
200+
node,
201+
messageId: 'circularDependency',
202+
data: { currentStore: from, usedStore: to },
203+
})
204+
}
205+
}
176206
}
177207
}
178-
179-
recursionStack.delete(store)
180-
return false
208+
path.pop()
209+
inPath.delete(store)
181210
}
182211

183-
// Check each store for cycles
184-
for (const store of storeUsages.keys()) {
185-
if (!visited.has(store)) {
186-
hasCycle(store)
187-
}
212+
for (const store of usageGraph.keys()) {
213+
if (!visited.has(store)) dfs(store)
188214
}
189215
}

0 commit comments

Comments
 (0)