Skip to content

Commit

Permalink
feat: use singular array element variable name in autofix for `no-for…
Browse files Browse the repository at this point in the history
…-loop` rule

If the loop is iterating an array with a plural name (such as `plugins`), the generated element variable in the autofix could use the singular version of the array name (`plugin`), instead of just the generic name `element`.
  • Loading branch information
bmish committed May 25, 2020
1 parent cec3a9d commit 62b920a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 1 deletion.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -40,6 +40,7 @@
"eslint-utils": "^2.0.0",
"import-modules": "^2.0.0",
"lodash": "^4.17.15",
"pluralize": "^8.0.0",
"read-pkg-up": "^7.0.1",
"regexp-tree": "^0.1.21",
"reserved-words": "^0.1.2",
Expand Down
22 changes: 21 additions & 1 deletion rules/no-for-loop.js
Expand Up @@ -2,6 +2,8 @@
const getDocumentationUrl = require('./utils/get-documentation-url');
const isLiteralValue = require('./utils/is-literal-value');
const {flatten} = require('lodash');
const {singular} = require('pluralize');
const reservedWords = require('reserved-words');

const defaultElementName = 'element';
const isLiteralZero = node => isLiteralValue(node, 0);
Expand Down Expand Up @@ -261,6 +263,24 @@ const getReferencesInChildScopes = (scope, name) => {
];
};

const getSingularName = originalName => {
const singularName = singular(originalName);
if (
!singularName ||
singularName === originalName
) {
// Bail if we can't produce a singular name that differs from the original name.
return;
}

if (reservedWords.check(singularName, '6', true)){ // 6 is the latest version understood by `reservedWords`.
// Add suffix to avoid using a reserved JavaScript keyword.
return `${singularName}_`;
}

return singularName;
};

const create = context => {
const sourceCode = context.getSourceCode();
const {scopeManager} = sourceCode;
Expand Down Expand Up @@ -335,7 +355,7 @@ const create = context => {
const shouldGenerateIndex = isIndexVariableUsedElsewhereInTheLoopBody(indexVariable, bodyScope, arrayIdentifierName);

const index = indexIdentifierName;
const element = elementIdentifierName || defaultElementName;
const element = elementIdentifierName || getSingularName(arrayIdentifierName) || defaultElementName;
const array = arrayIdentifierName;

let declarationElement = element;
Expand Down
40 changes: 40 additions & 0 deletions test/no-for-loop.js
Expand Up @@ -482,6 +482,46 @@ ruleTester.run('no-for-loop', rule, {
const [ a, b ] = element;
console.log(a, b, element);
}
`),
// Singularization (simple case):
testCase(outdent`
for (let i = 0; i < plugins.length; i++) {
console.log(plugins[i]);
}
`, outdent`
for (const plugin of plugins) {
console.log(plugin);
}
`),
// Singularization (irregular case):
testCase(outdent`
for (let i = 0; i < people.length; i++) {
console.log(people[i]);
}
`, outdent`
for (const person of people) {
console.log(person);
}
`),
// Singularization (avoid using reserved JavaScript keywords):
testCase(outdent`
for (let i = 0; i < cases.length; i++) {
console.log(cases[i]);
}
`, outdent`
for (const case_ of cases) {
console.log(case_);
}
`),
// Singularization (capital letters, multiple words):
testCase(outdent`
for (let i = 0; i < LARGE_CITIES.length; i++) {
console.log(LARGE_CITIES[i]);
}
`, outdent`
for (const LARGE_CITY of LARGE_CITIES) {
console.log(LARGE_CITY);
}
`)
]
});
Expand Down

0 comments on commit 62b920a

Please sign in to comment.