Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: correct update direction in for-direction #17483

Merged
merged 4 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions docs/src/rules/for-direction.md
Expand Up @@ -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:

Expand All @@ -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) {
}
```

:::
Expand All @@ -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
}
```

:::
23 changes: 15 additions & 8 deletions lib/rules/for-direction.js
Expand Up @@ -5,6 +5,12 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const { getStaticValue } = require("@eslint-community/eslint-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -29,6 +35,7 @@ module.exports = {
},

create(context) {
const { sourceCode } = context;

/**
* report an error.
Expand All @@ -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;
}

/**
Expand Down
23 changes: 21 additions & 2 deletions tests/lib/rules/for-direction.js
Expand Up @@ -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, {
Expand All @@ -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;){}",
Expand All @@ -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){}",
Expand All @@ -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] }
]
});