Fix invalid autofix with destructuring assignment in no-for-loops rule (

#476)
sindresorhus · Dec 17, 2019 · 44a67f1 · 44a67f1
1 parent 17a96df
commit 44a67f1
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 3 deletions.
diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js
@@ -329,9 +329,24 @@ const create = context => {
 					const element = elementIdentifierName || defaultElementName;
 					const array = arrayIdentifierName;
 
+					let declarationElement = element;
+					let declarationType = 'const';
+					let removeDeclaration = true;
+					if (
+						elementNode &&
+						elementNode.id.type === 'ObjectPattern'
+					) {
+						removeDeclaration = arrayReferences.length === 1;
+
+						if (removeDeclaration) {
+							declarationType = elementNode.parent.kind;
+							declarationElement = sourceCode.getText(elementNode.id);
+						}
+					}
+
 					const replacement = shouldGenerateIndex ?
-						`const [${index}, ${element}] of ${array}.entries()` :
-						`const ${element} of ${array}`;
+						`${declarationType} [${index}, ${declarationElement}] of ${array}.entries()` :
+						`${declarationType} ${declarationElement} of ${array}`;
 
 					return [
 						fixer.replaceTextRange([
@@ -345,7 +360,11 @@ const create = context => {
 
 							return fixer.replaceText(reference.identifier.parent, element);
 						}),
-						elementNode && fixer.removeRange(getRemovalRange(elementNode, sourceCode))
+						elementNode && (
+							removeDeclaration ?
+								fixer.removeRange(getRemovalRange(elementNode, sourceCode)) :
+								fixer.replaceText(elementNode.init, element)
+						)
 					].filter(Boolean);
 				};
 			}

diff --git a/test/no-for-loop.js b/test/no-for-loop.js
@@ -356,6 +356,70 @@ ruleTester.run('no-for-loop', rule, {
 					use(element);
 				}
 			}
+		`),
+
+		// Destructuring assignment in usage:
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				const { a, b } = arr[i];
+				console.log(a, b);
+			}
+		`, outdent`
+			for (const { a, b } of arr) {
+				console.log(a, b);
+			}
+		`),
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				var { a, b } = arr[i];
+				console.log(a, b);
+			}
+		`, outdent`
+			for (var { a, b } of arr) {
+				console.log(a, b);
+			}
+		`),
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				let { a, b } = arr[i];
+				console.log(a, b);
+			}
+		`, outdent`
+			for (let { a, b } of arr) {
+				console.log(a, b);
+			}
+		`),
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				var { a, b } = arr[i];
+				console.log(i, a, b);
+			}
+		`, outdent`
+			for (var [i, { a, b }] of arr.entries()) {
+				console.log(i, a, b);
+			}
+		`),
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				const { a, b } = arr[i];
+				console.log(a, b, i, arr[i]);
+			}
+		`, outdent`
+			for (const [i, element] of arr.entries()) {
+				const { a, b } = element;
+				console.log(a, b, i, element);
+			}
+		`),
+		testCase(outdent`
+			for (let i = 0; i < arr.length; i++) {
+				const { a, b } = arr[i];
+				console.log(a, b, arr[i]);
+			}
+		`, outdent`
+			for (const element of arr) {
+				const { a, b } = element;
+				console.log(a, b, element);
+			}
 		`)
 	]
 });