Skip to content

Commit 6b340a3

Browse files
bmishfisker
bmish
and
fisker
authored
prefer-array-find: Singularize variable name in autofix (#1243)
Co-authored-by: fisker Cheung <[email protected]>
1 parent 3205419 commit 6b340a3

File tree

5 files changed

+201
-32
lines changed

5 files changed

+201
-32
lines changed

‎rules/no-for-loop.js‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
'use strict';
2-
const {singular} = require('pluralize');
32
const {isClosingParenToken} = require('eslint-utils');
43
const getDocumentationUrl = require('./utils/get-documentation-url');
54
const isLiteralValue = require('./utils/is-literal-value');
65
const avoidCapture = require('./utils/avoid-capture');
6+
const getChildScopesRecursive = require('./utils/get-child-scopes-recursive');
7+
const singular = require('./utils/singular');
78

89
const MESSAGE_ID = 'no-for-loop';
910
const messages = {
@@ -268,18 +269,6 @@ const getReferencesInChildScopes = (scope, name) => {
268269
];
269270
};
270271

271-
const getChildScopesRecursive = scope => [
272-
scope,
273-
...scope.childScopes.flatMap(scope => getChildScopesRecursive(scope))
274-
];
275-
276-
const getSingularName = originalName => {
277-
const singularName = singular(originalName);
278-
if (singularName !== originalName) {
279-
return singularName;
280-
}
281-
};
282-
283272
const create = context => {
284273
const sourceCode = context.getSourceCode();
285274
const {scopeManager, text: sourceCodeText} = sourceCode;
@@ -361,7 +350,7 @@ const create = context => {
361350

362351
const index = indexIdentifierName;
363352
const element = elementIdentifierName ||
364-
avoidCapture(getSingularName(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
353+
avoidCapture(singular(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
365354
const array = arrayIdentifierName;
366355

367356
let declarationElement = element;

‎rules/prefer-array-find.js‎

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ const {isParenthesized, findVariable} = require('eslint-utils');
33
const getDocumentationUrl = require('./utils/get-documentation-url');
44
const methodSelector = require('./utils/method-selector');
55
const getVariableIdentifiers = require('./utils/get-variable-identifiers');
6+
const renameVariable = require('./utils/rename-variable');
7+
const avoidCapture = require('./utils/avoid-capture');
8+
const getChildScopesRecursive = require('./utils/get-child-scopes-recursive');
9+
const singular = require('./utils/singular');
10+
const extendFixRange = require('./utils/extend-fix-range');
611

712
const ERROR_ZERO_INDEX = 'error-zero-index';
813
const ERROR_SHIFT = 'error-shift';
@@ -94,10 +99,10 @@ const destructuringAssignmentSelector = [
9499
// Need add `()` to the `AssignmentExpression`
95100
// - `ObjectExpression`: `[{foo}] = array.filter(bar)` fix to `{foo} = array.find(bar)`
96101
// - `ObjectPattern`: `[{foo = baz}] = array.filter(bar)`
97-
const assignmentNeedParenthesize = (node, source) => {
102+
const assignmentNeedParenthesize = (node, sourceCode) => {
98103
const isAssign = node.type === 'AssignmentExpression';
99104

100-
if (!isAssign || isParenthesized(node, source)) {
105+
if (!isAssign || isParenthesized(node, sourceCode)) {
101106
return false;
102107
}
103108

@@ -143,36 +148,36 @@ const getDestructuringLeftAndRight = node => {
143148
return {};
144149
};
145150

146-
function * fixDestructuring(node, source, fixer) {
151+
function * fixDestructuring(node, sourceCode, fixer) {
147152
const {left} = getDestructuringLeftAndRight(node);
148153
const [element] = left.elements;
149154

150-
const leftText = source.getText(element.type === 'AssignmentPattern' ? element.left : element);
155+
const leftText = sourceCode.getText(element.type === 'AssignmentPattern' ? element.left : element);
151156
yield fixer.replaceText(left, leftText);
152157

153158
// `AssignmentExpression` always starts with `[` or `(`, so we don't need check ASI
154-
if (assignmentNeedParenthesize(node, source)) {
159+
if (assignmentNeedParenthesize(node, sourceCode)) {
155160
yield fixer.insertTextBefore(node, '(');
156161
yield fixer.insertTextAfter(node, ')');
157162
}
158163
}
159164

160165
const hasDefaultValue = node => getDestructuringLeftAndRight(node).left.elements[0].type === 'AssignmentPattern';
161166

162-
const fixDestructuringDefaultValue = (node, source, fixer, operator) => {
167+
const fixDestructuringDefaultValue = (node, sourceCode, fixer, operator) => {
163168
const {left, right} = getDestructuringLeftAndRight(node);
164169
const [element] = left.elements;
165170
const defaultValue = element.right;
166-
let defaultValueText = source.getText(defaultValue);
171+
let defaultValueText = sourceCode.getText(defaultValue);
167172

168-
if (isParenthesized(defaultValue, source) || hasLowerPrecedence(defaultValue, operator)) {
173+
if (isParenthesized(defaultValue, sourceCode) || hasLowerPrecedence(defaultValue, operator)) {
169174
defaultValueText = `(${defaultValueText})`;
170175
}
171176

172177
return fixer.insertTextAfter(right, ` ${operator} ${defaultValueText}`);
173178
};
174179

175-
const fixDestructuringAndReplaceFilter = (source, node) => {
180+
const fixDestructuringAndReplaceFilter = (sourceCode, node) => {
176181
const {property} = getDestructuringLeftAndRight(node).right.callee;
177182

178183
let suggest;
@@ -186,14 +191,14 @@ const fixDestructuringAndReplaceFilter = (source, node) => {
186191
messageId,
187192
* fix(fixer) {
188193
yield fixer.replaceText(property, 'find');
189-
yield fixDestructuringDefaultValue(node, source, fixer, operator);
190-
yield * fixDestructuring(node, source, fixer);
194+
yield fixDestructuringDefaultValue(node, sourceCode, fixer, operator);
195+
yield * fixDestructuring(node, sourceCode, fixer);
191196
}
192197
}));
193198
} else {
194199
fix = function * (fixer) {
195200
yield fixer.replaceText(property, 'find');
196-
yield * fixDestructuring(node, source, fixer);
201+
yield * fixDestructuring(node, sourceCode, fixer);
197202
};
198203
}
199204

@@ -221,7 +226,7 @@ const isDestructuringFirstElement = node => {
221226
};
222227

223228
const create = context => {
224-
const source = context.getSourceCode();
229+
const sourceCode = context.getSourceCode();
225230

226231
return {
227232
[zeroIndexSelector](node) {
@@ -248,18 +253,19 @@ const create = context => {
248253
context.report({
249254
node: node.init.callee.property,
250255
messageId: ERROR_DESTRUCTURING_DECLARATION,
251-
...fixDestructuringAndReplaceFilter(source, node)
256+
...fixDestructuringAndReplaceFilter(sourceCode, node)
252257
});
253258
},
254259
[destructuringAssignmentSelector](node) {
255260
context.report({
256261
node: node.right.callee.property,
257262
messageId: ERROR_DESTRUCTURING_ASSIGNMENT,
258-
...fixDestructuringAndReplaceFilter(source, node)
263+
...fixDestructuringAndReplaceFilter(sourceCode, node)
259264
});
260265
},
261266
[filterVariableSelector](node) {
262-
const variable = findVariable(context.getScope(), node.id);
267+
const scope = context.getScope();
268+
const variable = findVariable(scope, node.id);
263269
const identifiers = getVariableIdentifiers(variable).filter(identifier => identifier !== node.id);
264270

265271
if (identifiers.length === 0) {
@@ -288,12 +294,22 @@ const create = context => {
288294
problem.fix = function * (fixer) {
289295
yield fixer.replaceText(node.init.callee.property, 'find');
290296

297+
const singularName = singular(node.id.name);
298+
if (singularName) {
299+
// Rename variable to be singularized now that it refers to a single item in the array instead of the entire array.
300+
const singularizedName = avoidCapture(singularName, getChildScopesRecursive(scope), context.parserOptions.ecmaVersion);
301+
yield * renameVariable(variable, singularizedName, fixer);
302+
303+
// Prevent possible variable conflicts
304+
yield * extendFixRange(fixer, sourceCode.ast.range);
305+
}
306+
291307
for (const node of zeroIndexNodes) {
292308
yield fixer.removeRange([node.object.range[1], node.range[1]]);
293309
}
294310

295311
for (const node of destructuringNodes) {
296-
yield * fixDestructuring(node, source, fixer);
312+
yield * fixDestructuring(node, sourceCode, fixer);
297313
}
298314
};
299315
}

‎rules/utils/get-child-scopes-recursive.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
3+
/**
4+
Gather a list of all Scopes starting recursively from the input Scope.
5+
6+
@param {Scope} scope - The Scope to start checking from.
7+
@returns {Scope[]} - The resulting Scopes.
8+
*/
9+
const getChildScopesRecursive = scope => [
10+
scope,
11+
...scope.childScopes.flatMap(scope => getChildScopesRecursive(scope))
12+
];
13+
14+
module.exports = getChildScopesRecursive;

‎rules/utils/singular.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
3+
const {singular: pluralizeSingular} = require('pluralize');
4+
5+
/**
6+
Singularizes a word/name, i.e. `items` to `item`.
7+
8+
@param {string} original - The word/name to singularize.
9+
@returns {string|undefined} - The singularized result, or `undefined` if attempting singularization resulted in no change.
10+
*/
11+
const singular = original => {
12+
const singularized = pluralizeSingular(original);
13+
if (singularized !== original) {
14+
return singularized;
15+
}
16+
};
17+
18+
module.exports = singular;

‎test/prefer-array-find.mjs‎

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ ruleTester.run('prefer-array-find', rule, {
145145
'function a([foo] = array.filter(bar)) {}',
146146
// Not `ArrayPattern`
147147
'const foo = array.filter(bar)',
148+
'const items = array.filter(bar)', // Plural variable name.
148149
'const {0: foo} = array.filter(bar)',
149150
// `elements`
150151
'const [] = array.filter(bar)',
@@ -175,6 +176,12 @@ ruleTester.run('prefer-array-find', rule, {
175176
output: 'const foo = array.find(bar)',
176177
errors: [{messageId: ERROR_DESTRUCTURING_DECLARATION}]
177178
},
179+
{
180+
// Plural variable name.
181+
code: 'const [items] = array.filter(bar)',
182+
output: 'const items = array.find(bar)',
183+
errors: [{messageId: ERROR_DESTRUCTURING_DECLARATION}]
184+
},
178185
{
179186
code: 'const [foo] = array.filter(bar, thisArgument)',
180187
output: 'const foo = array.find(bar, thisArgument)',
@@ -376,6 +383,7 @@ ruleTester.run('prefer-array-find', rule, {
376383
'function a([foo] = array.filter(bar)) {}',
377384
// Not `ArrayPattern`
378385
'foo = array.filter(bar)',
386+
'items = array.filter(bar)', // Plural variable name.
379387
'({foo} = array.filter(bar))',
380388
// `elements`
381389
'[] = array.filter(bar)',
@@ -626,7 +634,11 @@ ruleTester.run('prefer-array-find', rule, {
626634
// More or less argument(s)
627635
'const foo = array.filter(); const first = foo[0]',
628636
'const foo = array.filter(bar, thisArgument, extraArgument); const first = foo[0]',
629-
'const foo = array.filter(...bar); const first = foo[0]'
637+
'const foo = array.filter(...bar); const first = foo[0]',
638+
639+
// Singularization
640+
'const item = array.find(bar), first = item;', // Already singular variable name.
641+
'let items = array.filter(bar); console.log(items[0]); items = [1,2,3]; console.log(items[0]);' // Reassigning array variable.
630642
],
631643
invalid: [
632644
{
@@ -669,6 +681,126 @@ ruleTester.run('prefer-array-find', rule, {
669681
output: 'const foo = array.find(bar); ({propOfFirst = unicorn} = foo);',
670682
errors: [{messageId: ERROR_DECLARATION}]
671683
},
684+
685+
// Singularization
686+
{
687+
// Multiple usages and child scope.
688+
code: outdent`
689+
const items = array.filter(bar);
690+
const first = items[0];
691+
console.log(items[0]);
692+
function foo() { return items[0]; }
693+
`,
694+
output: outdent`
695+
const item = array.find(bar);
696+
const first = item;
697+
console.log(item);
698+
function foo() { return item; }
699+
`,
700+
errors: [{messageId: ERROR_DECLARATION}]
701+
},
702+
{
703+
// Variable name collision.
704+
code: 'const item = {}; const items = array.filter(bar); console.log(items[0]);',
705+
output: 'const item = {}; const item_ = array.find(bar); console.log(item_);',
706+
errors: [{messageId: ERROR_DECLARATION}]
707+
},
708+
{
709+
// Variable defined with `let`.
710+
code: 'let items = array.filter(bar); console.log(items[0]);',
711+
output: 'let item = array.find(bar); console.log(item);',
712+
errors: [{messageId: ERROR_DECLARATION}]
713+
},
714+
{
715+
code: outdent`
716+
const item = 1;
717+
function f() {
718+
const items = array.filter(bar);
719+
console.log(items[0]);
720+
}
721+
`,
722+
output: outdent`
723+
const item = 1;
724+
function f() {
725+
const item_ = array.find(bar);
726+
console.log(item_);
727+
}
728+
`,
729+
errors: [{messageId: ERROR_DECLARATION}]
730+
},
731+
{
732+
code: outdent`
733+
const items = array.filter(bar);
734+
function f() {
735+
const item = 1;
736+
const item_ = 2;
737+
console.log(items[0]);
738+
}
739+
`,
740+
output: outdent`
741+
const item__ = array.find(bar);
742+
function f() {
743+
const item = 1;
744+
const item_ = 2;
745+
console.log(item__);
746+
}
747+
`,
748+
errors: [{messageId: ERROR_DECLARATION}]
749+
},
750+
{
751+
code: outdent`
752+
const items = array.filter(bar);
753+
function f() {
754+
console.log(items[0], item);
755+
}
756+
`,
757+
output: outdent`
758+
const item_ = array.find(bar);
759+
function f() {
760+
console.log(item_, item);
761+
}
762+
`,
763+
errors: [{messageId: ERROR_DECLARATION}]
764+
},
765+
{
766+
code: outdent`
767+
const items = array.filter(bar);
768+
console.log(items[0]);
769+
function f(item) {
770+
return item;
771+
}
772+
`,
773+
output: outdent`
774+
const item_ = array.find(bar);
775+
console.log(item_);
776+
function f(item) {
777+
return item;
778+
}
779+
`,
780+
errors: [{messageId: ERROR_DECLARATION}]
781+
},
782+
{
783+
code: outdent`
784+
function f() {
785+
const items = array.filter(bar);
786+
console.log(items[0]);
787+
}
788+
function f2(item) {
789+
return item;
790+
}
791+
`,
792+
output: outdent`
793+
function f() {
794+
const item = array.find(bar);
795+
console.log(item);
796+
}
797+
function f2(item) {
798+
return item;
799+
}
800+
`,
801+
errors: [{messageId: ERROR_DECLARATION}]
802+
},
803+
672804
// Not fixable
673805
{
674806
code: 'const foo = array.filter(bar); const [first = bar] = foo;',

0 commit comments

Comments
 (0)