no-for-loop: Ignore known non-array loop variables (#1242)

Author: bmish

rules/no-for-loop.js

@@ -1,5 +1,5 @@
 'use strict';
-const {isClosingParenToken} = require('eslint-utils');
+const {isClosingParenToken, getStaticValue} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const isLiteralValue = require('./utils/is-literal-value');
 const avoidCapture = require('./utils/avoid-capture');
@@ -60,7 +60,7 @@ const getStrictComparisonOperands = binaryExpression => {
 	}
 };
 
-const getArrayIdentifierNameFromBinaryExpression = (binaryExpression, indexIdentifierName) => {
+const getArrayIdentifierFromBinaryExpression = (binaryExpression, indexIdentifierName) => {
 	const operands = getStrictComparisonOperands(binaryExpression);
 
 	if (!operands) {
@@ -88,17 +88,17 @@ const getArrayIdentifierNameFromBinaryExpression = (binaryExpression, indexIdent
 		return;
 	}
 
-	return greater.object.name;
+	return greater.object;
 };
 
-const getArrayIdentifierName = (forStatement, indexIdentifierName) => {
+const getArrayIdentifier = (forStatement, indexIdentifierName) => {
 	const {test} = forStatement;
 
 	if (!test || test.type !== 'BinaryExpression') {
 		return;
 	}
 
-	return getArrayIdentifierNameFromBinaryExpression(test, indexIdentifierName);
+	return getArrayIdentifierFromBinaryExpression(test, indexIdentifierName);
 };
 
 const isLiteralOnePlusIdentifierWithName = (node, identifierName) => {
@@ -281,9 +281,17 @@ const create = context => {
 				return;
 			}
 
-			const arrayIdentifierName = getArrayIdentifierName(node, indexIdentifierName);
+			const arrayIdentifier = getArrayIdentifier(node, indexIdentifierName);
+			if (!arrayIdentifier) {
+				return;
+			}
+
+			const arrayIdentifierName = arrayIdentifier.name;
 
-			if (!arrayIdentifierName) {
+			const scope = context.getScope();
+			const staticResult = getStaticValue(arrayIdentifier, scope);
+			if (staticResult && !Array.isArray(staticResult.value)) {
+				// Bail out if we can tell that the array variable has a non-array value (i.e. we're looping through the characters of a string constant).
 				return;
 			}
 

test/no-for-loop.mjs

@@ -193,7 +193,12 @@ ruleTester.run('no-for-loop', rule, {
 					console.log(cities)
 				}
 			}
-		`
+		`,
+
+		// With variable containing static, non-array value.
+		'const notArray = "abc"; for (let i = 0; i < notArray.length; i++) { console.log(notArray[i]); }',
+		'const notArray = 123; for (let i = 0; i < notArray.length; i++) { console.log(notArray[i]); }',
+		'const notArray = true; for (let i = 0; i < notArray.length; i++) { console.log(notArray[i]); }'
 	],
 
 	invalid: [
@@ -711,6 +716,32 @@ ruleTester.run('no-for-loop', rule, {
 			for (const [i, city] of cities.entries()) {
 				console.log(i, city);
 			}
+		`),
+
+		// With static array variable.
+		testCase(outdent`
+			const someArray = [1,2,3];
+			for (let i = 0; i < someArray.length; i++) {
+				console.log(someArray[i]);
+			}
+		`, outdent`
+			const someArray = [1,2,3];
+			for (const element of someArray) {
+				console.log(element);
+			}
+		`),
+
+		// With non-static variable.
+		testCase(outdent`
+			const someArray = getSomeArray();
+			for (let i = 0; i < someArray.length; i++) {
+				console.log(someArray[i]);
+			}
+		`, outdent`
+			const someArray = getSomeArray();
+			for (const element of someArray) {
+				console.log(element);
+			}
 		`)
 	]
 });