From 2dbd726d6bdaad23447aee227d95c68229bd4c8a Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 10 Mar 2020 06:21:08 +0100 Subject: [PATCH 1/2] Fix: no-plusplus allow comma operands in for afterthought (fixes #13005) --- docs/rules/no-plusplus.md | 26 +++++++++++++-- lib/rules/no-plusplus.js | 35 +++++++++++++++++-- tests/lib/rules/no-plusplus.js | 61 +++++++++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 7 deletions(-) diff --git a/docs/rules/no-plusplus.md b/docs/rules/no-plusplus.md index ae0382937f4..e1b1ca337d1 100644 --- a/docs/rules/no-plusplus.md +++ b/docs/rules/no-plusplus.md @@ -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++; ``` diff --git a/lib/rules/no-plusplus.js b/lib/rules/no-plusplus.js index f55303863d2..93e357d7199 100644 --- a/lib/rules/no-plusplus.js +++ b/lib/rules/no-plusplus.js @@ -6,6 +6,34 @@ "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, if it is the update node of a `ForStatement`, or an operand of a sequence expression that is the update node. + * @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; + + return isForStatementUpdate(node) || + parent.type === "SequenceExpression" && isForStatementUpdate(parent); +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -42,18 +70,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", diff --git a/tests/lib/rules/no-plusplus.js b/tests/lib/rules/no-plusplus.js index 91623ad28eb..5a159b220d2 100644 --- a/tests/lib/rules/no-plusplus.js +++ b/tests/lib/rules/no-plusplus.js @@ -24,7 +24,11 @@ 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 }] } ], invalid: [ @@ -81,6 +85,61 @@ 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" + }] } ] }); From c90ba460096409f492f5ec0bf1bf2e22f2d6d700 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 10 Mar 2020 22:20:55 +0100 Subject: [PATCH 2/2] Allow deep sequence operands --- lib/rules/no-plusplus.js | 13 ++++++++++--- tests/lib/rules/no-plusplus.js | 27 ++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-plusplus.js b/lib/rules/no-plusplus.js index 93e357d7199..84d6c3e1f91 100644 --- a/lib/rules/no-plusplus.js +++ b/lib/rules/no-plusplus.js @@ -23,15 +23,22 @@ function isForStatementUpdate(node) { /** * Determines whether the given node is considered to be a for loop "afterthought" by the logic of this rule. - * In particular, if it is the update node of a `ForStatement`, or an operand of a sequence expression that is the update node. + * 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; - return isForStatementUpdate(node) || - parent.type === "SequenceExpression" && isForStatementUpdate(parent); + if (parent.type === "SequenceExpression") { + return isForLoopAfterthought(parent); + } + + return isForStatementUpdate(node); } //------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-plusplus.js b/tests/lib/rules/no-plusplus.js index 5a159b220d2..af3b62f47fd 100644 --- a/tests/lib/rules/no-plusplus.js +++ b/tests/lib/rules/no-plusplus.js @@ -28,7 +28,11 @@ ruleTester.run("no-plusplus", rule, { { 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 (;; 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: [ @@ -62,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 { @@ -140,6 +154,17 @@ ruleTester.run("no-plusplus", rule, { }, type: "UpdateExpression" }] + }, + { + code: "for (;; foo + (i++, bar));", + options: [{ allowForLoopAfterthoughts: true }], + errors: [{ + messageId: "unexpectedUnaryOp", + data: { + operator: "++" + }, + type: "UpdateExpression" + }] } ] });