Skip to content

Commit 44a67f1

Browse files
bmishfisker
authored andcommitted
Fix invalid autofix with destructuring assignment in no-for-loops rule (#476)
1 parent 17a96df commit 44a67f1

File tree

2 files changed

+86
-3
lines changed

2 files changed

+86
-3
lines changed

rules/no-for-loop.js

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,24 @@ const create = context => {
329329
const element = elementIdentifierName || defaultElementName;
330330
const array = arrayIdentifierName;
331331

332+
let declarationElement = element;
333+
let declarationType = 'const';
334+
let removeDeclaration = true;
335+
if (
336+
elementNode &&
337+
elementNode.id.type === 'ObjectPattern'
338+
) {
339+
removeDeclaration = arrayReferences.length === 1;
340+
341+
if (removeDeclaration) {
342+
declarationType = elementNode.parent.kind;
343+
declarationElement = sourceCode.getText(elementNode.id);
344+
}
345+
}
346+
332347
const replacement = shouldGenerateIndex ?
333-
`const [${index}, ${element}] of ${array}.entries()` :
334-
`const ${element} of ${array}`;
348+
`${declarationType} [${index}, ${declarationElement}] of ${array}.entries()` :
349+
`${declarationType} ${declarationElement} of ${array}`;
335350

336351
return [
337352
fixer.replaceTextRange([
@@ -345,7 +360,11 @@ const create = context => {
345360

346361
return fixer.replaceText(reference.identifier.parent, element);
347362
}),
348-
elementNode && fixer.removeRange(getRemovalRange(elementNode, sourceCode))
363+
elementNode && (
364+
removeDeclaration ?
365+
fixer.removeRange(getRemovalRange(elementNode, sourceCode)) :
366+
fixer.replaceText(elementNode.init, element)
367+
)
349368
].filter(Boolean);
350369
};
351370
}

test/no-for-loop.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,70 @@ ruleTester.run('no-for-loop', rule, {
356356
use(element);
357357
}
358358
}
359+
`),
360+
361+
// Destructuring assignment in usage:
362+
testCase(outdent`
363+
for (let i = 0; i < arr.length; i++) {
364+
const { a, b } = arr[i];
365+
console.log(a, b);
366+
}
367+
`, outdent`
368+
for (const { a, b } of arr) {
369+
console.log(a, b);
370+
}
371+
`),
372+
testCase(outdent`
373+
for (let i = 0; i < arr.length; i++) {
374+
var { a, b } = arr[i];
375+
console.log(a, b);
376+
}
377+
`, outdent`
378+
for (var { a, b } of arr) {
379+
console.log(a, b);
380+
}
381+
`),
382+
testCase(outdent`
383+
for (let i = 0; i < arr.length; i++) {
384+
let { a, b } = arr[i];
385+
console.log(a, b);
386+
}
387+
`, outdent`
388+
for (let { a, b } of arr) {
389+
console.log(a, b);
390+
}
391+
`),
392+
testCase(outdent`
393+
for (let i = 0; i < arr.length; i++) {
394+
var { a, b } = arr[i];
395+
console.log(i, a, b);
396+
}
397+
`, outdent`
398+
for (var [i, { a, b }] of arr.entries()) {
399+
console.log(i, a, b);
400+
}
401+
`),
402+
testCase(outdent`
403+
for (let i = 0; i < arr.length; i++) {
404+
const { a, b } = arr[i];
405+
console.log(a, b, i, arr[i]);
406+
}
407+
`, outdent`
408+
for (const [i, element] of arr.entries()) {
409+
const { a, b } = element;
410+
console.log(a, b, i, element);
411+
}
412+
`),
413+
testCase(outdent`
414+
for (let i = 0; i < arr.length; i++) {
415+
const { a, b } = arr[i];
416+
console.log(a, b, arr[i]);
417+
}
418+
`, outdent`
419+
for (const element of arr) {
420+
const { a, b } = element;
421+
console.log(a, b, element);
422+
}
359423
`)
360424
]
361425
});

0 commit comments

Comments
 (0)