From 830b456119eb997d655b3d2a3d7fa8f45ffd463b Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 24 May 2020 10:40:49 -0500 Subject: [PATCH 1/6] feat: use singular array element variable name in autofix for `no-for-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`. --- package.json | 1 + rules/no-for-loop.js | 16 ++++++++++- test/no-for-loop.js | 65 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index dea370eded..fd51fde919 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index e2f40885de..cb83b6a233 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -3,6 +3,7 @@ const getDocumentationUrl = require('./utils/get-documentation-url'); const isLiteralValue = require('./utils/is-literal-value'); const {flatten} = require('lodash'); const avoidCapture = require('./utils/avoid-capture'); +const {singular} = require('pluralize'); const defaultElementName = 'element'; const isLiteralZero = node => isLiteralValue(node, 0); @@ -267,6 +268,19 @@ const getChildScopesRecursive = scope => [ ...flatten(scope.childScopes.map(scope => getChildScopesRecursive(scope))) ]; +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; + } + + return singularName; +}; + const create = context => { const sourceCode = context.getSourceCode(); const {scopeManager} = sourceCode; @@ -342,7 +356,7 @@ const create = context => { const index = indexIdentifierName; const element = elementIdentifierName || - avoidCapture(defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion); + avoidCapture(getSingularName(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion); const array = arrayIdentifierName; let declarationElement = element; diff --git a/test/no-for-loop.js b/test/no-for-loop.js index 43fbf888d6..b3d940bef1 100644 --- a/test/no-for-loop.js +++ b/test/no-for-loop.js @@ -649,6 +649,71 @@ ruleTester.run('no-for-loop', rule, { console.log(element); console.log(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 (avoid variable name collision): + testCase(outdent` + for (let i = 0; i < cities.length; i++) { + console.log(cities[i]); + const city = foo(); + console.log(city); + } + `, outdent` + for (const city_ of cities) { + console.log(city_); + const city = foo(); + console.log(city); + } + `), + // Singularization (camelCase): + testCase(outdent` + for (let i = 0; i < largeCities.length; i++) { + console.log(largeCities[i]); + } + `, outdent` + for (const largeCity of largeCities) { + console.log(largeCity); + } + `), + // 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); + } `) ] }); From f8bcbdba63e353b5a8cdf1cf6dfef32395c88a48 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 22:08:58 -0500 Subject: [PATCH 2/6] test: refactor test cases for no-for-loop singularization --- test/no-for-loop.js | 54 ++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 40 deletions(-) diff --git a/test/no-for-loop.js b/test/no-for-loop.js index b3d940bef1..33196d8c25 100644 --- a/test/no-for-loop.js +++ b/test/no-for-loop.js @@ -651,26 +651,20 @@ ruleTester.run('no-for-loop', rule, { } `), - // 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: + ...[ + ['plugin', 'plugins'], // simple + ['person', 'people'], // irregular + ['largeCity', 'largeCities'], // camelCase + ['LARGE_CITY', 'LARGE_CITIES'], // caps, snake_case + ['element', 'list'], // no singular version + ].map(([elementName, arrayName]) => + testCase( + `for(const i = 0; i < ${arrayName}.length; i++) {console.log(${arrayName}[i])}`, + `for(const ${elementName} of ${arrayName}) {console.log(${elementName})}`, + ) + ), + // Singularization (avoid using reserved JavaScript keywords): testCase(outdent` for (let i = 0; i < cases.length; i++) { @@ -695,26 +689,6 @@ ruleTester.run('no-for-loop', rule, { console.log(city); } `), - // Singularization (camelCase): - testCase(outdent` - for (let i = 0; i < largeCities.length; i++) { - console.log(largeCities[i]); - } - `, outdent` - for (const largeCity of largeCities) { - console.log(largeCity); - } - `), - // 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); - } - `) ] }); From f0ee3890a3274387e4512999a84c4315ad67b4f3 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 22:11:10 -0500 Subject: [PATCH 3/6] fix lint --- test/no-for-loop.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/no-for-loop.js b/test/no-for-loop.js index 33196d8c25..095f8eedc6 100644 --- a/test/no-for-loop.js +++ b/test/no-for-loop.js @@ -653,15 +653,15 @@ ruleTester.run('no-for-loop', rule, { // Singularization: ...[ - ['plugin', 'plugins'], // simple - ['person', 'people'], // irregular - ['largeCity', 'largeCities'], // camelCase - ['LARGE_CITY', 'LARGE_CITIES'], // caps, snake_case - ['element', 'list'], // no singular version + ['plugin', 'plugins'], // Simple + ['person', 'people'], // Irregular + ['largeCity', 'largeCities'], // CamelCase + ['LARGE_CITY', 'LARGE_CITIES'], // Caps, snake_case + ['element', 'list'] // No singular version ].map(([elementName, arrayName]) => testCase( `for(const i = 0; i < ${arrayName}.length; i++) {console.log(${arrayName}[i])}`, - `for(const ${elementName} of ${arrayName}) {console.log(${elementName})}`, + `for(const ${elementName} of ${arrayName}) {console.log(${elementName})}` ) ), @@ -688,7 +688,7 @@ ruleTester.run('no-for-loop', rule, { const city = foo(); console.log(city); } - `), + `) ] }); From 26cbca2f90e8fac02a1502ad26c34320d75f4c8c Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 22:29:50 -0500 Subject: [PATCH 4/6] add more tests --- test/no-for-loop.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/no-for-loop.js b/test/no-for-loop.js index 095f8eedc6..2ef400aa4d 100644 --- a/test/no-for-loop.js +++ b/test/no-for-loop.js @@ -655,8 +655,10 @@ ruleTester.run('no-for-loop', rule, { ...[ ['plugin', 'plugins'], // Simple ['person', 'people'], // Irregular + ['girlsAndBoy', 'girlsAndBoys'], // Multiple plurals ['largeCity', 'largeCities'], // CamelCase ['LARGE_CITY', 'LARGE_CITIES'], // Caps, snake_case + ['element', 'news'], // No singular version, ends in s ['element', 'list'] // No singular version ].map(([elementName, arrayName]) => testCase( @@ -688,6 +690,16 @@ ruleTester.run('no-for-loop', rule, { const city = foo(); console.log(city); } + `), + // Singularization (uses i): + testCase(outdent` + for (let i = 0; i < cities.length; i++) { + console.log(i, cities[i]); + } + `, outdent` + for (const [i, city] of cities.entries()) { + console.log(i, city); + } `) ] }); From 4ac535cc5db3bb441f757374201f8d0d42a166e5 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 22:33:07 -0500 Subject: [PATCH 5/6] Update rules/no-for-loop.js Co-authored-by: fisker Cheung --- rules/no-for-loop.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index cb83b6a233..949beee150 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -270,16 +270,10 @@ const getChildScopesRecursive = scope => [ 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 (singularName !== originalName) { + return singularName; } - - return singularName; -}; +} const create = context => { const sourceCode = context.getSourceCode(); From 3e5ad5d0bf1be442d3526e093c0fe4ff170e6731 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 22:35:48 -0500 Subject: [PATCH 6/6] fix lint --- rules/no-for-loop.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index 949beee150..5d8803f8fe 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -273,7 +273,7 @@ const getSingularName = originalName => { if (singularName !== originalName) { return singularName; } -} +}; const create = context => { const sourceCode = context.getSourceCode();