feat: for-direction rule add check for condition in reverse order

[ild0tt0re]

Prerequisites checklist

• I have read the contributing guidelines.

What rule do you want to change?

• for-direction rule

What change do you want to make (place an "X" next to just one item)?

• Generate more warnings
• Generate fewer warnings
• Implement autofix
• Implement suggestions

How will the change be implemented (place an "X" next to just one item)?

• A new option
• A new default behaviour
• Other

Please provide some example code that this change will affect:

for (var i = 0; 10 > i; i--) {
}

for (var i = 0; 10 >= i; i--) {
}

for (var i = 10; 10 < i; i++) {
}

for (var i = 10; 0 <= i; i++) {
}

What does the rule currently do for this code?

The for-direction rule checks only for the condition with the counter i.e. i on the left side of the binary expression i < 10 like:

for (var i = 0; i < 10; i++) {
}

What will the rule do after it's changed?

The rule adds more warnings for code like this with the counter i.e. i on the right side of the test condition 10 > i

for (var i = 0; 10 > i; i++) {
}

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ild0tt0re / name: Angelo Annunziata (43e2cdd)

[netlify]

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	43e2cdd
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/6557534931a0ba00089e8086

[eslint-github-bot]

Hi @ild0tt0re!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[eslint-github-bot]

Hi @ild0tt0re!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[eslint-github-bot]

Hi @ild0tt0re!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.

• The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.

Read more about contributing to ESLint here

[mdjermanovic]

Hi @ild0tt0re, thanks for the PR!

Could you please copy & paste this template into #17755 (comment) and fill it out? We'll need those details to understand and evaluate the proposal.

[ild0tt0re]

Hi @ild0tt0re, thanks for the PR!

Could you please copy & paste this template into #17755 (comment) and fill it out? We'll need those details to understand and evaluate the proposal.

Thanks to you!
Done ✅

[mdjermanovic]

So, the for-direction rule currently reports for (var i = 0; i < 10; i--) {} but doesn't report for (var i = 0; 10 > i; i--) {} (Playground demo), although they're basically the same.

I support the proposal to account for conditions in this style, especially since we have a rule (yoda) that can be configured to enforce it. For example, in this demo, for-direction reports an error. But after the yoda fix is applied, the for-direction error disappears although the same bug in the code remains.

Just waiting for more opinions from the team before marking this as accepted and reviewing it.

[fasttime]

I support the proposal to account for conditions in this style, especially since we have a rule (yoda) that can be configured to enforce it. For example, in this demo, for-direction reports an error. But after the yoda fix is applied, the for-direction error disappears although the same bug in the code remains.

I agree.

[mdjermanovic]

This doesn't seem to cover AssignmentExpression:

for(var i = 0; 10 > i; i-=1){} // should be error

and looks partially redundant with functions that check whether the names match afterward.

What do you think about updating ForStatement(node) to always check both sides instead:

ForStatement(node) {

    if (node.test && node.test.type === "BinaryExpression" && node.update) {
        for (const counterPosition of ["left", "right"]) {
            if (node.test[counterPosition].type !== "Identifier") {
                continue;
            }

            const counter = node.test[counterPosition].name;
            // ...
        }
    }
}

[ild0tt0re]

Yes much better! Do you think it is necessary to add other tests on top of this? I added only one more
{ code: "for(var i = 0; 10 > i; i-=1){}", errors: [incorrectDirection] } after your suggestion.

[mdjermanovic]

Do you think it is necessary to add other tests on top of this? I added only one more
{ code: "for(var i = 0; 10 > i; i-=1){}", errors: [incorrectDirection] } after your suggestion.

One test looks fine to me.

[mdjermanovic]

LGTM, thanks! Leaving this open till today's release in case anyone else would like to review it.