Skip to content

Commit bc89544

Browse files
jdegandjdegand
authored
feat(eslint-plugin): enhance prefix-selectors-with-select to handle destructuring (#4926)
1 parent 37e6fa1 commit bc89544

File tree

2 files changed

+240
-37
lines changed

2 files changed

+240
-37
lines changed

‎modules/eslint-plugin/spec/rules/store/prefix-selectors-with-select.spec.ts‎

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,18 @@ const valid: () => (string | ValidTestCase<Options>)[] = () => [
3333
}),
3434
})
3535
`,
36+
`
37+
export const { selectAll: selectAllBooks } = booksAdapter.getSelectors(createSelector(selectBookInfo, (state) => state.books));
38+
`,
39+
`
40+
const { selectAll, selectEntities } = getSelectors(adapter);
41+
`,
42+
`
43+
const { selectAll: selectAllItems, selectEntities: selectEntitiesMap } = getSelectors(adapter);
44+
`,
45+
`
46+
const { selectItems, ...rest } = getSelectors(adapter);
47+
`,
3648
];
3749

3850
const invalid: () => InvalidTestCase<MessageIds, Options>[] = () => [
@@ -44,9 +56,7 @@ export const getCount: MemoizedSelector<any, any> = (state: AppState) => state.f
4456
suggestions: [
4557
{
4658
messageId: prefixSelectorsWithSelectSuggest,
47-
data: {
48-
name: 'selectCount',
49-
},
59+
data: { name: 'selectCount' },
5060
output: `
5161
export const selectCount: MemoizedSelector<any, any> = (state: AppState) => state.feature`,
5262
},
@@ -212,6 +222,81 @@ const selectFeatureSelector = createFeatureSelector<FileListResponseState>("name
212222
],
213223
}
214224
),
225+
fromFixture(
226+
`
227+
export const { selectAll: allBooks } = booksAdapter.getSelectors(createSelector(selectBookInfo, (state) => state.books));
228+
~~~~~~~~ [${prefixSelectorsWithSelect} suggest]`,
229+
{
230+
suggestions: [
231+
{
232+
messageId: prefixSelectorsWithSelectSuggest,
233+
data: { name: 'selectAllBooks' },
234+
output: `
235+
export const { selectAll: selectAllBooks } = booksAdapter.getSelectors(createSelector(selectBookInfo, (state) => state.books));`,
236+
},
237+
],
238+
}
239+
),
240+
fromFixture(
241+
`
242+
const { selectAll: allItems } = getSelectors(adapter);
243+
~~~~~~~~ [${prefixSelectorsWithSelect} suggest]`,
244+
{
245+
suggestions: [
246+
{
247+
messageId: prefixSelectorsWithSelectSuggest,
248+
data: { name: 'selectAllItems' },
249+
output: `
250+
const { selectAll: selectAllItems } = getSelectors(adapter);`,
251+
},
252+
],
253+
}
254+
),
255+
fromFixture(
256+
`
257+
const { selectEntities: entitiesMap } = getSelectors(adapter);
258+
~~~~~~~~~~~ [${prefixSelectorsWithSelect} suggest]`,
259+
{
260+
suggestions: [
261+
{
262+
messageId: prefixSelectorsWithSelectSuggest,
263+
data: { name: 'selectEntitiesMap' },
264+
output: `
265+
const { selectEntities: selectEntitiesMap } = getSelectors(adapter);`,
266+
},
267+
],
268+
}
269+
),
270+
fromFixture(
271+
`
272+
const { allItems } = getSelectors(adapter);
273+
~~~~~~~~ [${prefixSelectorsWithSelect} suggest]`,
274+
{
275+
suggestions: [
276+
{
277+
messageId: prefixSelectorsWithSelectSuggest,
278+
data: { name: 'selectAllItems' },
279+
output: `
280+
const { selectAllItems } = getSelectors(adapter);`,
281+
},
282+
],
283+
}
284+
),
285+
fromFixture(
286+
`
287+
const { entitiesMap } = getSelectors(adapter);
288+
~~~~~~~~~~~ [${prefixSelectorsWithSelect} suggest]`,
289+
{
290+
suggestions: [
291+
{
292+
messageId: prefixSelectorsWithSelectSuggest,
293+
data: { name: 'selectEntitiesMap' },
294+
output: `
295+
const { selectEntitiesMap } = getSelectors(adapter);`,
296+
},
297+
],
298+
}
299+
),
215300
];
216301

217302
ruleTester().run(path.parse(__filename).name, rule, {

‎modules/eslint-plugin/src/rules/store/prefix-selectors-with-select.ts‎

Lines changed: 152 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,64 +31,182 @@ export default createRule<Options, MessageIds>({
3131
},
3232
defaultOptions: [],
3333
create: (context) => {
34-
return {
35-
'VariableDeclarator[id.name!=/^select[^a-z].+$/]:not(:has(Identifier[name="createFeature"])):matches([id.typeAnnotation.typeAnnotation.typeName.name=/^MemoizedSelector(WithProps)?$/], :has(CallExpression[callee.name=/^(create(Feature)?Selector|createSelectorFactory)$/]))'({
36-
id,
37-
}: TSESTree.VariableDeclarator & { id: TSESTree.Identifier }) {
38-
const suggestedName = getSuggestedName(id.name);
34+
function reportIfInvalid(name: string, node: TSESTree.Identifier) {
35+
// Name starts with select and
36+
// the first character after select is an uppercase ASCII letter, _, or $
37+
const isValid =
38+
name.startsWith('select') &&
39+
name.length > 'select'.length &&
40+
/^[A-Z_$]/.test(name.slice('select'.length));
41+
42+
if (!isValid) {
43+
const suggestedName = getSuggestedName(name);
3944
context.report({
45+
node,
4046
loc: {
41-
...id.loc,
47+
start: node.loc.start,
4248
end: {
43-
...id.loc.end,
44-
column: id.typeAnnotation?.range[0]
45-
? id.typeAnnotation.range[0] - 1
46-
: id.loc.end.column,
49+
line: node.loc.start.line,
50+
column: node.loc.start.column + name.length,
4751
},
4852
},
4953
messageId: prefixSelectorsWithSelect,
5054
suggest: [
5155
{
5256
messageId: prefixSelectorsWithSelectSuggest,
53-
data: {
54-
name: suggestedName,
57+
data: { name: suggestedName },
58+
fix: (fixer) => {
59+
const parent = node.parent;
60+
const sourceCode = context.getSourceCode();
61+
62+
// Handle destructuring: { selectAll: allItems }
63+
if (
64+
parent &&
65+
parent.type === 'Property' &&
66+
parent.value === node &&
67+
parent.parent &&
68+
parent.parent.type === 'ObjectPattern'
69+
) {
70+
return fixer.replaceText(node, suggestedName);
71+
}
72+
73+
// Handle simple variable declarator: const allItems = ...
74+
if (
75+
parent &&
76+
parent.type === 'VariableDeclarator' &&
77+
parent.id.type === 'Identifier'
78+
) {
79+
const typeAnnotation = parent.id.typeAnnotation
80+
? sourceCode.getText(parent.id.typeAnnotation)
81+
: '';
82+
return fixer.replaceText(
83+
parent.id,
84+
`${suggestedName}${typeAnnotation}`
85+
);
86+
}
87+
88+
// Fallback: just replace the identifier
89+
return fixer.replaceText(node, suggestedName);
5590
},
56-
fix: (fixer) =>
57-
fixer.replaceTextRange(
58-
[id.range[0], id.typeAnnotation?.range[0] ?? id.range[1]],
59-
suggestedName
60-
),
6191
},
6292
],
6393
});
94+
}
95+
}
96+
97+
function isSelectorFactoryCall(node: TSESTree.CallExpression): boolean {
98+
const callee = node.callee;
99+
return (
100+
callee.type === 'Identifier' &&
101+
[
102+
'createSelector',
103+
'createFeatureSelector',
104+
'createSelectorFactory',
105+
].includes(callee.name)
106+
);
107+
}
108+
109+
function checkFunctionBody(
110+
name: string,
111+
node: TSESTree.Identifier,
112+
body: TSESTree.BlockStatement | TSESTree.Expression
113+
) {
114+
if (body.type === 'CallExpression' && isSelectorFactoryCall(body)) {
115+
reportIfInvalid(name, node);
116+
}
117+
118+
if (body.type === 'BlockStatement') {
119+
for (const stmt of body.body) {
120+
if (
121+
stmt.type === 'ReturnStatement' &&
122+
stmt.argument &&
123+
stmt.argument.type === 'CallExpression' &&
124+
isSelectorFactoryCall(stmt.argument)
125+
) {
126+
reportIfInvalid(name, node);
127+
}
128+
}
129+
}
130+
}
131+
132+
return {
133+
VariableDeclarator(node: TSESTree.VariableDeclarator) {
134+
const { id, init } = node;
135+
136+
const isSelectorSource =
137+
init?.type === 'CallExpression' &&
138+
((init.callee.type === 'Identifier' &&
139+
init.callee.name === 'getSelectors') ||
140+
(init.callee.type === 'MemberExpression' &&
141+
init.callee.property.type === 'Identifier' &&
142+
init.callee.property.name === 'getSelectors'));
143+
144+
if (id.type === 'ObjectPattern' && isSelectorSource) {
145+
for (const prop of id.properties) {
146+
if (prop.type === 'Property' && prop.value.type === 'Identifier') {
147+
reportIfInvalid(prop.value.name, prop.value);
148+
}
149+
}
150+
return;
151+
}
152+
153+
if (id.type === 'Identifier') {
154+
const typeName =
155+
node.id.typeAnnotation?.typeAnnotation.type === 'TSTypeReference' &&
156+
node.id.typeAnnotation.typeAnnotation.typeName.type === 'Identifier'
157+
? node.id.typeAnnotation.typeAnnotation.typeName.name
158+
: null;
159+
160+
const hasSelectorType =
161+
typeName !== null && /Selector$/.test(typeName);
162+
163+
const isSelectorCall =
164+
init?.type === 'CallExpression' && isSelectorFactoryCall(init);
165+
166+
const isArrowFunction =
167+
init?.type === 'ArrowFunctionExpression' &&
168+
init.body &&
169+
(init.body.type === 'CallExpression' ||
170+
init.body.type === 'BlockStatement');
171+
172+
const isFunctionExpression =
173+
init?.type === 'FunctionExpression' &&
174+
init.body &&
175+
init.body.type === 'BlockStatement';
176+
177+
if (hasSelectorType || isSelectorCall) {
178+
reportIfInvalid(id.name, id);
179+
} else if (isArrowFunction || isFunctionExpression) {
180+
checkFunctionBody(id.name, id, init.body);
181+
}
182+
}
64183
},
65184
};
66185
},
67186
});
68187

69-
function getSuggestedName(name: string) {
188+
function getSuggestedName(name: string): string {
70189
const selectWord = 'select';
71-
// Ex: 'selectfeature' => 'selectFeature'
72-
let possibleReplacedName = name.replace(
73-
new RegExp(`^${selectWord}(.+)`),
74-
(_, word: string) => {
75-
return `${selectWord}${capitalize(word)}`;
76-
}
77-
);
78190

79-
if (name !== possibleReplacedName) {
80-
return possibleReplacedName;
191+
if (name.startsWith(selectWord)) {
192+
const rest = name.slice(selectWord.length);
193+
if (rest.length === 0) {
194+
return 'selectSelect';
195+
}
196+
if (/^[A-Z_]+$/.test(rest)) {
197+
return `${selectWord}${rest}`;
198+
}
199+
return `${selectWord}${capitalize(rest)}`;
81200
}
82201

83-
// Ex: 'getCount' => 'selectCount'
84-
possibleReplacedName = name.replace(/^get([^a-z].+)/, (_, word: string) => {
85-
return `${selectWord}${capitalize(word)}`;
86-
});
202+
if (/^get([^a-z].+)/.test(name)) {
203+
const rest = name.slice(3);
204+
return `${selectWord}${capitalize(rest)}`;
205+
}
87206

88-
if (name !== possibleReplacedName) {
89-
return possibleReplacedName;
207+
if (/^[A-Z_]+$/.test(name)) {
208+
return `${selectWord}${name}`;
90209
}
91210

92-
// Ex: 'item' => 'selectItem'
93211
return `${selectWord}${capitalize(name)}`;
94212
}

0 commit comments

Comments
 (0)