From 7224eee3ff4b4378d3439deb038bf34b116fa48b Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Wed, 18 Mar 2020 02:36:23 +0100 Subject: [PATCH] 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 --- docs/rules/no-plusplus.md | 26 ++++++++-- lib/rules/no-plusplus.js | 42 +++++++++++++++-- tests/lib/rules/no-plusplus.js | 86 +++++++++++++++++++++++++++++++++- 3 files changed, 147 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..84d6c3e1f91 100644 --- a/lib/rules/no-plusplus.js +++ b/lib/rules/no-plusplus.js @@ -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 //------------------------------------------------------------------------------ @@ -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", diff --git a/tests/lib/rules/no-plusplus.js b/tests/lib/rules/no-plusplus.js index 91623ad28eb..af3b62f47fd 100644 --- a/tests/lib/rules/no-plusplus.js +++ b/tests/lib/rules/no-plusplus.js @@ -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: [ @@ -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 { @@ -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" + }] } ] });