Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix: no-plusplus allow comma operands in for afterthought (fixes #13005
…) (#13024)

* Fix: no-plusplus allow comma operands in for afterthought (fixes #13005)

* Allow deep sequence operands
  • Loading branch information
mdjermanovic committed Mar 18, 2020
1 parent 7598cf8 commit 7224eee
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 7 deletions.
26 changes: 23 additions & 3 deletions docs/rules/no-plusplus.md
Expand Up @@ -71,10 +71,30 @@ Examples of **correct** code for this rule with the `{ "allowForLoopAfterthought
/*eslint no-plusplus: ["error", { "allowForLoopAfterthoughts": true }]*/

for (i = 0; i < l; i++) {
return;
doSomething(i);
}

for (i = 0; i < l; i--) {
return;
for (i = l; i >= 0; i--) {
doSomething(i);
}

for (i = 0, j = l; i < l; i++, j--) {
doSomething(i, j);
}
```

Examples of **incorrect** code for this rule with the `{ "allowForLoopAfterthoughts": true }` option:

```js
/*eslint no-plusplus: ["error", { "allowForLoopAfterthoughts": true }]*/

for (i = 0; i < l; j = i++) {
doSomething(i, j);
}

for (i = l; i--;) {
doSomething(i);
}

for (i = 0; i < l;) i++;
```
42 changes: 39 additions & 3 deletions lib/rules/no-plusplus.js
Expand Up @@ -6,6 +6,41 @@

"use strict";

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Determines whether the given node is the update node of a `ForStatement`.
* @param {ASTNode} node The node to check.
* @returns {boolean} `true` if the node is `ForStatement` update.
*/
function isForStatementUpdate(node) {
const parent = node.parent;

return parent.type === "ForStatement" && parent.update === node;
}

/**
* Determines whether the given node is considered to be a for loop "afterthought" by the logic of this rule.
* In particular, it returns `true` if the given node is either:
* - The update node of a `ForStatement`: for (;; i++) {}
* - An operand of a sequence expression that is the update node: for (;; foo(), i++) {}
* - An operand of a sequence expression that is child of another sequence expression, etc.,
* up to the sequence expression that is the update node: for (;; foo(), (bar(), (baz(), i++))) {}
* @param {ASTNode} node The node to check.
* @returns {boolean} `true` if the node is a for loop afterthought.
*/
function isForLoopAfterthought(node) {
const parent = node.parent;

if (parent.type === "SequenceExpression") {
return isForLoopAfterthought(parent);
}

return isForStatementUpdate(node);
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -42,18 +77,19 @@ module.exports = {
create(context) {

const config = context.options[0];
let allowInForAfterthought = false;
let allowForLoopAfterthoughts = false;

if (typeof config === "object") {
allowInForAfterthought = config.allowForLoopAfterthoughts === true;
allowForLoopAfterthoughts = config.allowForLoopAfterthoughts === true;
}

return {

UpdateExpression(node) {
if (allowInForAfterthought && node.parent.type === "ForStatement") {
if (allowForLoopAfterthoughts && isForLoopAfterthought(node)) {
return;
}

context.report({
node,
messageId: "unexpectedUnaryOp",
Expand Down
86 changes: 85 additions & 1 deletion tests/lib/rules/no-plusplus.js
Expand Up @@ -24,7 +24,15 @@ ruleTester.run("no-plusplus", rule, {

// With "allowForLoopAfterthoughts" allowed
{ code: "var foo = 0; foo=+1;", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (i = 0; i < l; i++) { console.log(i); }", options: [{ allowForLoopAfterthoughts: true }] }
{ code: "for (i = 0; i < l; i++) { console.log(i); }", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (var i = 0, j = i + 1; j < example.length; i++, j++) {}", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; i--, foo());", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; foo(), --i);", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; foo(), ++i, bar);", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; i++, (++j, k--));", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; foo(), (bar(), i++), baz());", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; (--i, j += 2), bar = j + 1);", options: [{ allowForLoopAfterthoughts: true }] },
{ code: "for (;; a, (i--, (b, ++j, c)), d);", options: [{ allowForLoopAfterthoughts: true }] }
],

invalid: [
Expand Down Expand Up @@ -58,6 +66,16 @@ ruleTester.run("no-plusplus", rule, {
type: "UpdateExpression"
}]
},
{
code: "for (i = 0; i < l; foo, i++) { console.log(i); }",
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "++"
},
type: "UpdateExpression"
}]
},

// With "allowForLoopAfterthoughts" allowed
{
Expand All @@ -81,6 +99,72 @@ ruleTester.run("no-plusplus", rule, {
},
type: "UpdateExpression"
}]
},
{
code: "for (i++;;);",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "++"
},
type: "UpdateExpression"
}]
},
{
code: "for (;--i;);",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "--"
},
type: "UpdateExpression"
}]
},
{
code: "for (;;) ++i;",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "++"
},
type: "UpdateExpression"
}]
},
{
code: "for (;; i = j++);",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "++"
},
type: "UpdateExpression"
}]
},
{
code: "for (;; i++, f(--j));",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "--"
},
type: "UpdateExpression"
}]
},
{
code: "for (;; foo + (i++, bar));",
options: [{ allowForLoopAfterthoughts: true }],
errors: [{
messageId: "unexpectedUnaryOp",
data: {
operator: "++"
},
type: "UpdateExpression"
}]
}
]
});

0 comments on commit 7224eee

Please sign in to comment.