From 48bd5c8cd3fcbbaf4ac2751cbd21c027caa56b08 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Tue, 26 May 2020 21:16:21 -0500 Subject: [PATCH] Avoid naming collision with default array element variable in autofix for `no-for-loop` rule (#749) Co-authored-by: fisker --- rules/no-for-loop.js | 9 ++- test/no-for-loop.js | 167 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 1 deletion(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index 283ace5367..e2f40885de 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -2,6 +2,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 defaultElementName = 'element'; const isLiteralZero = node => isLiteralValue(node, 0); @@ -261,6 +262,11 @@ const getReferencesInChildScopes = (scope, name) => { ]; }; +const getChildScopesRecursive = scope => [ + scope, + ...flatten(scope.childScopes.map(scope => getChildScopesRecursive(scope))) +]; + const create = context => { const sourceCode = context.getSourceCode(); const {scopeManager} = sourceCode; @@ -335,7 +341,8 @@ const create = context => { const shouldGenerateIndex = isIndexVariableUsedElsewhereInTheLoopBody(indexVariable, bodyScope, arrayIdentifierName); const index = indexIdentifierName; - const element = elementIdentifierName || defaultElementName; + const element = elementIdentifierName || + avoidCapture(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 24dfcdbfd3..43fbf888d6 100644 --- a/test/no-for-loop.js +++ b/test/no-for-loop.js @@ -482,6 +482,173 @@ ruleTester.run('no-for-loop', rule, { const [ a, b ] = element; console.log(a, b, element); } + `), + + // Avoid naming collision when using default element name. + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + console.log(arr[i]); + const element = foo(); + console.log(element); + } + `, outdent` + for (const element_ of arr) { + console.log(element_); + const element = foo(); + console.log(element); + } + `), + + // Avoid naming collision when using default element name (different scope). + testCase(outdent` + function element(element_) { + for (let i = 0; i < arr.length; i += 1) { + console.log(arr[i], element); + } + } + `, outdent` + function element(element_) { + for (const element__ of arr) { + console.log(element__, element); + } + } + `), + testCase(outdent` + let element; + function foo() { + for (let i = 0; i < arr.length; i += 1) { + console.log(arr[i]); + } + } + `, outdent` + let element; + function foo() { + for (const element_ of arr) { + console.log(element_); + } + } + `), + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + function element__(element) { + console.log(arr[i], element); + } + } + `, outdent` + for (const element_ of arr) { + function element__(element) { + console.log(element_, element); + } + } + `), + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + function element_(element) { + console.log(arr[i], element); + } + } + `, outdent` + for (const element__ of arr) { + function element_(element) { + console.log(element__, element); + } + } + `), + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + function element() { + console.log(arr[i], element); + } + } + `, outdent` + for (const element_ of arr) { + function element() { + console.log(element_, element); + } + } + `), + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + console.log(arr[i], element); + } + `, outdent` + for (const element_ of arr) { + console.log(element_, element); + } + `), + testCase(outdent` + for (let i = 0; i < element.length; i += 1) { + console.log(element[i]); + } + `, outdent` + for (const element_ of element) { + console.log(element_); + } + `), + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + console.log(arr[i]); + function foo(element) { + console.log(element); + } + } + `, outdent` + for (const element_ of arr) { + console.log(element_); + function foo(element) { + console.log(element); + } + } + `), + testCase(outdent` + for (let element = 0; element < arr.length; element += 1) { + console.log(element, arr[element]); + } + `, outdent` + for (const [element, element_] of arr.entries()) { + console.log(element, element_); + } + `), + testCase(outdent` + for (let element = 0; element < arr.length; element += 1) { + console.log(arr[element]); + } + `, outdent` + for (const element_ of arr) { + console.log(element_); + } + `), + testCase(outdent` + for (const element of arr) { + for (let j = 0; j < arr2.length; j += 1) { + console.log(element, arr2[j]); + } + } + `, outdent` + for (const element of arr) { + for (const element_ of arr2) { + console.log(element, element_); + } + } + `), + + // Avoid naming collision when using default element name (multiple collisions). + testCase(outdent` + for (let i = 0; i < arr.length; i += 1) { + const element = foo(); + console.log(arr[i]); + const element_ = foo(); + console.log(element); + console.log(element_); + } + `, outdent` + for (const element__ of arr) { + const element = foo(); + console.log(element__); + const element_ = foo(); + console.log(element); + console.log(element_); + } `) ] });