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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Feb 1, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 30 additions & 17 deletions lib/rules/for-direction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}
}
Expand Down
17 changes: 12 additions & 5 deletions tests/lib/rules/for-direction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;){}",
Expand All @@ -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: [

Expand All @@ -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] }
]
});