Update: for-direction detection false positives/negatives (#11254)

eslint · Feb 1, 2019 · b5143bf · b5143bf
1 parent 9005e63
commit b5143bf
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 22 deletions.
diff --git a/lib/rules/for-direction.js b/lib/rules/for-direction.js
@@ -42,6 +42,23 @@ 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
+         */
+        function getRightDirection(update, dir) {
+            if (update.right.type === "UnaryExpression") {
+                if (update.right.operator === "-") {
+                    return -dir;
+                }
+            } else if (update.right.type === "Identifier") {
+                return 0;
+            }
+            return dir;
+        }
+
         /**
          * check UpdateExpression add/sub the counter
          * @param {ASTNode} update UpdateExpression to check
@@ -69,10 +86,10 @@ module.exports = {
         function getAssignmentDirection(update, counter) {
             if (update.left.name === counter) {
                 if (update.operator === "+=") {
-                    return 1;
+                    return getRightDirection(update, 1);
                 }
                 if (update.operator === "-=") {
-                    return -1;
+                    return getRightDirection(update, -1);
                 }
             }
             return 0;
@@ -85,26 +102,22 @@ module.exports = {
                     const operator = node.test.operator;
                     const update = node.update;
 
-                    if (operator === "<" || operator === "<=") {
-
-                        // report error if update sub the counter (--, -=)
-                        if (update.type === "UpdateExpression" && getUpdateDirection(update, counter) < 0) {
-                            report(node);
-                        }
+                    let wrongDirection;
 
-                        if (update.type === "AssignmentExpression" && getAssignmentDirection(update, counter) < 0) {
-                            report(node);
-                        }
+                    if (operator === "<" || operator === "<=") {
+                        wrongDirection = -1;
                     } else if (operator === ">" || operator === ">=") {
+                        wrongDirection = 1;
+                    } else {
+                        return;
+                    }
 
-                        // report error if update add the counter (++, +=)
-                        if (update.type === "UpdateExpression" && getUpdateDirection(update, counter) > 0) {
-                            report(node);
-                        }
-
-                        if (update.type === "AssignmentExpression" && getAssignmentDirection(update, counter) > 0) {
+                    if (update.type === "UpdateExpression") {
+                        if (getUpdateDirection(update, counter) === wrongDirection) {
                             report(node);
                         }
+                    } else if (update.type === "AssignmentExpression" && getAssignmentDirection(update, counter) === wrongDirection) {
+                        report(node);
                     }
                 }
             }

diff --git a/tests/lib/rules/for-direction.js b/tests/lib/rules/for-direction.js
@@ -31,10 +31,12 @@ ruleTester.run("for-direction", rule, {
         // test if '+=', '-=',
         "for(var i = 0; i < 10; i+=1){}",
         "for(var i = 0; i <= 10; i+=1){}",
+        "for(var i = 0; i < 10; i-=-1){}",
+        "for(var i = 0; i <= 10; i-=-1){}",
         "for(var i = 10; i > 0; i-=1){}",
         "for(var i = 10; i >= 0; i-=1){}",
-        "for (var i = 0; i < MAX; i += STEP_SIZE);",
-        "for (var i = MAX; i > MIN; i -= STEP_SIZE);",
+        "for(var i = 10; i > 0; i+=-1){}",
+        "for(var i = 10; i >= 0; i+=-1){}",
 
         // test if no update.
         "for(var i = 10; i > 0;){}",
@@ -48,7 +50,10 @@ ruleTester.run("for-direction", rule, {
         "for(var i = 10; i >= 0; j += 2){}",
         "for(var i = 10; i >= 0; j -= 2){}",
         "for(var i = 10; i >= 0; i |= 2){}",
-        "for(var i = 10; i >= 0; i %= 2){}"
+        "for(var i = 10; i >= 0; i %= 2){}",
+        "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);"
     ],
     invalid: [
 
@@ -63,7 +68,9 @@ ruleTester.run("for-direction", rule, {
         { 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 = 0; i < MAX; i -= STEP_SIZE);", errors: [incorrectDirection] },
-        { code: "for (var i = 0; i > MIN; i += STEP_SIZE);", errors: [incorrectDirection] }
+        { 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] }
     ]
 });