From 1fbb3b0b477c814c0d179564fe495f4c50a451e9 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Wed, 23 Aug 2023 16:49:50 +0200 Subject: [PATCH] feat: correct update direction in `for-direction` (#17483) * feat: correct update direction in `for-direction` * Improve documentation * Update rule overview Co-authored-by: Nicholas C. Zakas * Improve formatting --------- Co-authored-by: Nicholas C. Zakas --- docs/src/rules/for-direction.md | 14 ++++++++++++-- lib/rules/for-direction.js | 23 +++++++++++++++-------- tests/lib/rules/for-direction.js | 23 +++++++++++++++++++++-- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/docs/src/rules/for-direction.md b/docs/src/rules/for-direction.md index 832c87cde3d..45759eb48f9 100644 --- a/docs/src/rules/for-direction.md +++ b/docs/src/rules/for-direction.md @@ -3,11 +3,11 @@ title: for-direction rule_type: problem --- - +A `for` loop with a stop condition that can never be reached, such as one with a counter that moves in the wrong direction, will run infinitely. While there are occasions when an infinite loop is intended, the convention is to construct such loops as `while` loops. More typically, an infinite `for` loop is a bug. ## Rule Details -A `for` loop with a stop condition that can never be reached, such as one with a counter that moves in the wrong direction, will run infinitely. While there are occasions when an infinite loop is intended, the convention is to construct such loops as `while` loops. More typically, an infinite for loop is a bug. +This rule forbids `for` loops where the counter variable changes in such a way that the stop condition will never be met. For example, if the counter variable is increasing (i.e. `i++`) and the stop condition tests that the counter is greater than zero (`i >= 0`) then the loop will never exit. Examples of **incorrect** code for this rule: @@ -23,6 +23,10 @@ for (var i = 10; i >= 0; i++) { for (var i = 0; i > 10; i++) { } + +const n = -2; +for (let i = 0; i < 10; i += n) { +} ``` ::: @@ -35,6 +39,12 @@ Examples of **correct** code for this rule: /*eslint for-direction: "error"*/ for (var i = 0; i < 10; i++) { } + +for (let i = 10; i >= 0; i += this.step) { // direction unknown +} + +for (let i = MIN; i <= MAX; i -= 0) { // not increasing or decreasing +} ``` ::: diff --git a/lib/rules/for-direction.js b/lib/rules/for-direction.js index 4ed73501581..3f2ad9df645 100644 --- a/lib/rules/for-direction.js +++ b/lib/rules/for-direction.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const { getStaticValue } = require("@eslint-community/eslint-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -29,6 +35,7 @@ module.exports = { }, create(context) { + const { sourceCode } = context; /** * report an error. @@ -46,17 +53,17 @@ module.exports = { * check the right side of the assignment * @param {ASTNode} update UpdateExpression to check * @param {int} dir expected direction that could either be turned around or invalidated - * @returns {int} return dir, the negated dir or zero if it's not clear for identifiers + * @returns {int} return dir, the negated dir, or zero if the counter does not change or the direction is not clear */ function getRightDirection(update, dir) { - if (update.right.type === "UnaryExpression") { - if (update.right.operator === "-") { - return -dir; - } - } else if (update.right.type === "Identifier") { - return 0; + const staticValue = getStaticValue(update.right, sourceCode.getScope(update)); + + if (staticValue && ["bigint", "boolean", "number"].includes(typeof staticValue.value)) { + const sign = Math.sign(Number(staticValue.value)) || 0; // convert NaN to 0 + + return dir * sign; } - return dir; + return 0; } /** diff --git a/tests/lib/rules/for-direction.js b/tests/lib/rules/for-direction.js index 8ecd843c238..6e59f6bacf4 100644 --- a/tests/lib/rules/for-direction.js +++ b/tests/lib/rules/for-direction.js @@ -16,7 +16,7 @@ const { RuleTester } = require("../../../lib/rule-tester"); // Tests //------------------------------------------------------------------------------ -const ruleTester = new RuleTester(); +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } }); const incorrectDirection = { messageId: "incorrectDirection" }; ruleTester.run("for-direction", rule, { @@ -37,6 +37,12 @@ ruleTester.run("for-direction", rule, { "for(var i = 10; i >= 0; i-=1){}", "for(var i = 10; i > 0; i+=-1){}", "for(var i = 10; i >= 0; i+=-1){}", + "for(var i = 0n; i > l; i-=1n){}", + "for(var i = 0n; i < l; i-=-1n){}", + "for(var i = MIN; i <= MAX; i+=true){}", + "for(var i = 0; i < 10; i+=+5e-7){}", + "for(var i = 0; i < MAX; i -= ~2);", + "for(var i = 0, n = -1; i < MAX; i += -n);", // test if no update. "for(var i = 10; i > 0;){}", @@ -54,6 +60,13 @@ ruleTester.run("for-direction", rule, { "for(var i = 0; i < MAX; i += STEP_SIZE);", "for(var i = 0; i < MAX; i -= STEP_SIZE);", "for(var i = 10; i > 0; i += STEP_SIZE);", + "for(var i = 10; i >= 0; i += 0);", + "for(var i = 10n; i >= 0n; i += 0n);", + "for(var i = 10; i >= 0; i += this.step);", + "for(var i = 10; i >= 0; i += 'foo');", + "for(var i = 10; i > 0; i += !foo);", + "for(var i = MIN; i <= MAX; i -= false);", + "for(var i = MIN; i <= MAX; i -= 0/0);", // other cond-expressions. "for(var i = 0; i !== 10; i+=1){}", @@ -77,6 +90,12 @@ ruleTester.run("for-direction", rule, { { code: "for(var i = 0; i < 10; i+=-1){}", errors: [incorrectDirection] }, { code: "for(var i = 0; i <= 10; i+=-1){}", errors: [incorrectDirection] }, { code: "for(var i = 10; i > 10; i-=-1){}", errors: [incorrectDirection] }, - { code: "for(var i = 10; i >= 0; i-=-1){}", errors: [incorrectDirection] } + { code: "for(var i = 10; i >= 0; i-=-1){}", errors: [incorrectDirection] }, + { code: "for(var i = 0n; i > l; i+=1n){}", errors: [incorrectDirection] }, + { code: "for(var i = 0n; i < l; i+=-1n){}", errors: [incorrectDirection] }, + { code: "for(var i = MIN; i <= MAX; i-=true){}", errors: [incorrectDirection] }, + { code: "for(var i = 0; i < 10; i-=+5e-7){}", errors: [incorrectDirection] }, + { code: "for(var i = 0; i < MAX; i += (2 - 3));", errors: [incorrectDirection] }, + { code: "var n = -2; for(var i = 0; i < 10; i += n);", errors: [incorrectDirection] } ] });