Skip to content

Commit 48bd5c8

Browse files
bmishfisker
bmish
and
fisker
authored
Avoid naming collision with default array element variable in autofix for no-for-loop rule (#749)
Co-authored-by: fisker <[email protected]>
1 parent cec3a9d commit 48bd5c8

File tree

2 files changed

+175
-1
lines changed

2 files changed

+175
-1
lines changed

‎rules/no-for-loop.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const getDocumentationUrl = require('./utils/get-documentation-url');
33
const isLiteralValue = require('./utils/is-literal-value');
44
const {flatten} = require('lodash');
5+
const avoidCapture = require('./utils/avoid-capture');
56

67
const defaultElementName = 'element';
78
const isLiteralZero = node => isLiteralValue(node, 0);
@@ -261,6 +262,11 @@ const getReferencesInChildScopes = (scope, name) => {
261262
];
262263
};
263264

265+
const getChildScopesRecursive = scope => [
266+
scope,
267+
...flatten(scope.childScopes.map(scope => getChildScopesRecursive(scope)))
268+
];
269+
264270
const create = context => {
265271
const sourceCode = context.getSourceCode();
266272
const {scopeManager} = sourceCode;
@@ -335,7 +341,8 @@ const create = context => {
335341
const shouldGenerateIndex = isIndexVariableUsedElsewhereInTheLoopBody(indexVariable, bodyScope, arrayIdentifierName);
336342

337343
const index = indexIdentifierName;
338-
const element = elementIdentifierName || defaultElementName;
344+
const element = elementIdentifierName ||
345+
avoidCapture(defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
339346
const array = arrayIdentifierName;
340347

341348
let declarationElement = element;

‎test/no-for-loop.js‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,173 @@ ruleTester.run('no-for-loop', rule, {
482482
const [ a, b ] = element;
483483
console.log(a, b, element);
484484
}
485+
`),
486+
487+
// Avoid naming collision when using default element name.
488+
testCase(outdent`
489+
for (let i = 0; i < arr.length; i += 1) {
490+
console.log(arr[i]);
491+
const element = foo();
492+
console.log(element);
493+
}
494+
`, outdent`
495+
for (const element_ of arr) {
496+
console.log(element_);
497+
const element = foo();
498+
console.log(element);
499+
}
500+
`),
501+
502+
// Avoid naming collision when using default element name (different scope).
503+
testCase(outdent`
504+
function element(element_) {
505+
for (let i = 0; i < arr.length; i += 1) {
506+
console.log(arr[i], element);
507+
}
508+
}
509+
`, outdent`
510+
function element(element_) {
511+
for (const element__ of arr) {
512+
console.log(element__, element);
513+
}
514+
}
515+
`),
516+
testCase(outdent`
517+
let element;
518+
function foo() {
519+
for (let i = 0; i < arr.length; i += 1) {
520+
console.log(arr[i]);
521+
}
522+
}
523+
`, outdent`
524+
let element;
525+
function foo() {
526+
for (const element_ of arr) {
527+
console.log(element_);
528+
}
529+
}
530+
`),
531+
testCase(outdent`
532+
for (let i = 0; i < arr.length; i += 1) {
533+
function element__(element) {
534+
console.log(arr[i], element);
535+
}
536+
}
537+
`, outdent`
538+
for (const element_ of arr) {
539+
function element__(element) {
540+
console.log(element_, element);
541+
}
542+
}
543+
`),
544+
testCase(outdent`
545+
for (let i = 0; i < arr.length; i += 1) {
546+
function element_(element) {
547+
console.log(arr[i], element);
548+
}
549+
}
550+
`, outdent`
551+
for (const element__ of arr) {
552+
function element_(element) {
553+
console.log(element__, element);
554+
}
555+
}
556+
`),
557+
testCase(outdent`
558+
for (let i = 0; i < arr.length; i += 1) {
559+
function element() {
560+
console.log(arr[i], element);
561+
}
562+
}
563+
`, outdent`
564+
for (const element_ of arr) {
565+
function element() {
566+
console.log(element_, element);
567+
}
568+
}
569+
`),
570+
testCase(outdent`
571+
for (let i = 0; i < arr.length; i += 1) {
572+
console.log(arr[i], element);
573+
}
574+
`, outdent`
575+
for (const element_ of arr) {
576+
console.log(element_, element);
577+
}
578+
`),
579+
testCase(outdent`
580+
for (let i = 0; i < element.length; i += 1) {
581+
console.log(element[i]);
582+
}
583+
`, outdent`
584+
for (const element_ of element) {
585+
console.log(element_);
586+
}
587+
`),
588+
testCase(outdent`
589+
for (let i = 0; i < arr.length; i += 1) {
590+
console.log(arr[i]);
591+
function foo(element) {
592+
console.log(element);
593+
}
594+
}
595+
`, outdent`
596+
for (const element_ of arr) {
597+
console.log(element_);
598+
function foo(element) {
599+
console.log(element);
600+
}
601+
}
602+
`),
603+
testCase(outdent`
604+
for (let element = 0; element < arr.length; element += 1) {
605+
console.log(element, arr[element]);
606+
}
607+
`, outdent`
608+
for (const [element, element_] of arr.entries()) {
609+
console.log(element, element_);
610+
}
611+
`),
612+
testCase(outdent`
613+
for (let element = 0; element < arr.length; element += 1) {
614+
console.log(arr[element]);
615+
}
616+
`, outdent`
617+
for (const element_ of arr) {
618+
console.log(element_);
619+
}
620+
`),
621+
testCase(outdent`
622+
for (const element of arr) {
623+
for (let j = 0; j < arr2.length; j += 1) {
624+
console.log(element, arr2[j]);
625+
}
626+
}
627+
`, outdent`
628+
for (const element of arr) {
629+
for (const element_ of arr2) {
630+
console.log(element, element_);
631+
}
632+
}
633+
`),
634+
635+
// Avoid naming collision when using default element name (multiple collisions).
636+
testCase(outdent`
637+
for (let i = 0; i < arr.length; i += 1) {
638+
const element = foo();
639+
console.log(arr[i]);
640+
const element_ = foo();
641+
console.log(element);
642+
console.log(element_);
643+
}
644+
`, outdent`
645+
for (const element__ of arr) {
646+
const element = foo();
647+
console.log(element__);
648+
const element_ = foo();
649+
console.log(element);
650+
console.log(element_);
651+
}
485652
`)
486653
]
487654
});

0 commit comments

Comments
 (0)