From b20337242fa41143340a67d668ccddc91e349e2c Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Thu, 11 Mar 2021 11:29:16 +0100 Subject: [PATCH 1/6] New: add option "allowInParentheses" to rule "no-sequences" --- lib/rules/no-sequences.js | 26 ++++++++++++++++++-------- tests/lib/rules/no-sequences.js | 30 ++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/lib/rules/no-sequences.js b/lib/rules/no-sequences.js index d67635d1175..051e4511886 100644 --- a/lib/rules/no-sequences.js +++ b/lib/rules/no-sequences.js @@ -26,7 +26,14 @@ module.exports = { url: "https://eslint.org/docs/rules/no-sequences" }, - schema: [], + schema: [{ + properties: { + allowInParentheses: { + type: "boolean", + default: false + } + } + }], messages: { unexpectedCommaExpression: "Unexpected use of comma operator." @@ -34,6 +41,7 @@ module.exports = { }, create(context) { + const options = context.options[0] || {}; const sourceCode = context.getSourceCode(); /** @@ -99,13 +107,15 @@ module.exports = { } // Wrapping a sequence in extra parens indicates intent - if (requiresExtraParens(node)) { - if (isParenthesisedTwice(node)) { - return; - } - } else { - if (isParenthesised(node)) { - return; + if (options.allowInParentheses) { + if (requiresExtraParens(node)) { + if (isParenthesisedTwice(node)) { + return; + } + } else { + if (isParenthesised(node)) { + return; + } } } diff --git a/tests/lib/rules/no-sequences.js b/tests/lib/rules/no-sequences.js index ae988ee3dae..8f86e56ee55 100644 --- a/tests/lib/rules/no-sequences.js +++ b/tests/lib/rules/no-sequences.js @@ -39,17 +39,17 @@ ruleTester.run("no-sequences", rule, { "var arr = [1, 2];", "var obj = {a: 1, b: 2};", "var a = 1, b = 2;", - "var foo = (1, 2);", - "(0,eval)(\"foo()\");", + { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] }, + { code: "(0,eval)(\"foo()\");", options: [{ allowInParentheses: true }] }, "for (i = 1, j = 2;; i++, j++);", - "foo(a, (b, c), d);", - "do {} while ((doSomething(), !!test));", - "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", - "if ((doSomething(), !!test));", - "switch ((doSomething(), !!test)) {}", - "while ((doSomething(), !!test));", - "with ((doSomething(), val)) {}", - { code: "a => ((doSomething(), a))", env: { es6: true } } + { code: "foo(a, (b, c), d);", options: [{ allowInParentheses: true }] }, + { code: "do {} while ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, + { code: "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", options: [{ allowInParentheses: true }] }, + { code: "if ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, + { code: "switch ((doSomething(), val)) {}", options: [{ allowInParentheses: true }] }, + { code: "while ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, + { code: "with ((doSomething(), val)) {}", options: [{ allowInParentheses: true }] }, + { code: "a => ((doSomething(), a))", options: [{ allowInParentheses: true }], env: { es6: true } } ], // Examples of code that should trigger the rule @@ -66,13 +66,23 @@ ruleTester.run("no-sequences", rule, { }] }, { code: "a = 1, 2", errors: errors(6) }, + { code: "var foo = (1, 2);", options: [{ allowInParentheses: false }], errors: errors(13) }, + { code: "(0,eval)(\"foo()\");", errors: errors(3) }, + { code: "foo(a, (b, c), d);", errors: errors(10) }, { code: "do {} while (doSomething(), !!test);", errors: errors(27) }, + { code: "do {} while ((doSomething(), !!test));", errors: errors(28) }, { code: "for (; doSomething(), !!test; );", errors: errors(21) }, + { code: "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", errors: errors(54) }, { code: "if (doSomething(), !!test);", errors: errors(18) }, + { code: "if ((doSomething(), !!test));", errors: errors(19) }, { code: "switch (doSomething(), val) {}", errors: errors(22) }, + { code: "switch ((doSomething(), val)) {}", errors: errors(23) }, { code: "while (doSomething(), !!test);", errors: errors(21) }, + { code: "while ((doSomething(), !!test));", errors: errors(22) }, { code: "with (doSomething(), val) {}", errors: errors(20) }, + { code: "with ((doSomething(), val)) {}", errors: errors(21) }, { code: "a => (doSomething(), a)", env: { es6: true }, errors: errors(20) }, + { code: "a => ((doSomething(), a))", env: { es6: true }, errors: errors(21) }, { code: "(1), 2", errors: errors(4) }, { code: "((1)) , (2)", errors: errors(7) }, { code: "while((1) , 2);", errors: errors(11) } From 5076011d7739cc1e620d2a8152000b18a42b4a30 Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Thu, 11 Mar 2021 13:12:35 +0100 Subject: [PATCH 2/6] added documentation --- docs/rules/no-sequences.md | 39 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md index 602a412f16a..7193960294a 100644 --- a/docs/rules/no-sequences.md +++ b/docs/rules/no-sequences.md @@ -17,7 +17,6 @@ while (a = next(), a && a.length); This rule forbids the use of the comma operator, with the following exceptions: * In the initialization or update portions of a `for` statement. -* If the expression sequence is explicitly wrapped in parentheses. Examples of **incorrect** code for this rule: @@ -46,6 +45,42 @@ Examples of **correct** code for this rule: ```js /*eslint no-sequences: "error"*/ +for (i = 0, j = 10; i < j; i++, j--); +``` + +## Options + +This rule takes one option, an object, with the following properties: + +* `"allowInParentheses"`: If set to `true`, this rule allows to use expression sequences that are explicitly wrapped in parentheses. + +Examples of **incorrect** code for this rule: + +```js +/*eslint no-sequences: ["error", { allowInParentheses: true }]*/ + +foo = doSomething(), val; + +0, eval("doSomething();"); + +do {} while (doSomething(), !!test); + +for (; doSomething(), !!test; ); + +if (doSomething(), !!test); + +switch (val = foo(), val) {} + +while (val = foo(), val < 42); + +with (doSomething(), val) {} +``` + +Examples of **correct** code for this rule: + +```js +/*eslint no-sequences: ["error", { allowInParentheses: true }]*/ + foo = (doSomething(), val); (0, eval)("doSomething();"); @@ -66,7 +101,7 @@ with ((doSomething(), val)) {} ## When Not To Use It Disable this rule if sequence expressions with the comma operator are acceptable. -Another case is where you might want to report all usages of the comma operator, even if they are wrapped in parentheses or in a for loop. You can achieve this using rule `no-restricted-syntax`: +Another case is where you might want to report all usages of the comma operator, even in a for loop. You can achieve this using rule `no-restricted-syntax`: ```js { From 7ba832985bfcfb5b467938ab5b926622e595c311 Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Fri, 12 Mar 2021 06:49:56 +0100 Subject: [PATCH 3/6] [no-equence]: switch default of "allowInParentheses" to true --- docs/rules/no-sequences.md | 46 ++++++++++++++++---------------- lib/rules/no-sequences.js | 15 ++++++++--- tests/lib/rules/no-sequences.js | 47 ++++++++++++++++++--------------- 3 files changed, 61 insertions(+), 47 deletions(-) diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md index 7193960294a..a8e4b6a1bc1 100644 --- a/docs/rules/no-sequences.md +++ b/docs/rules/no-sequences.md @@ -45,41 +45,33 @@ Examples of **correct** code for this rule: ```js /*eslint no-sequences: "error"*/ -for (i = 0, j = 10; i < j; i++, j--); -``` - -## Options - -This rule takes one option, an object, with the following properties: - -* `"allowInParentheses"`: If set to `true`, this rule allows to use expression sequences that are explicitly wrapped in parentheses. +foo = (doSomething(), val); -Examples of **incorrect** code for this rule: +(0, eval)("doSomething();"); -```js -/*eslint no-sequences: ["error", { allowInParentheses: true }]*/ +do {} while ((doSomething(), !!test)); -foo = doSomething(), val; +for (i = 0, j = 10; i < j; i++, j--); -0, eval("doSomething();"); +if ((doSomething(), !!test)); -do {} while (doSomething(), !!test); +switch ((val = foo(), val)) {} -for (; doSomething(), !!test; ); +while ((val = foo(), val < 42)); -if (doSomething(), !!test); +with ((doSomething(), val)) {} +``` -switch (val = foo(), val) {} +## Options -while (val = foo(), val < 42); +This rule takes one option, an object, with the following properties: -with (doSomething(), val) {} -``` +* `"allowInParentheses"`: If set to `false`, this rule allows to use expression sequences that are explicitly wrapped in parentheses. Default value is `true`. -Examples of **correct** code for this rule: +Examples of **incorrect** code for this rule: ```js -/*eslint no-sequences: ["error", { allowInParentheses: true }]*/ +/*eslint no-sequences: ["error", { allowInParentheses: false }]*/ foo = (doSomething(), val); @@ -87,7 +79,7 @@ foo = (doSomething(), val); do {} while ((doSomething(), !!test)); -for (i = 0, j = 10; i < j; i++, j--); +for (; (doSomething(), !!test); ); if ((doSomething(), !!test)); @@ -98,6 +90,14 @@ while ((val = foo(), val < 42)); with ((doSomething(), val)) {} ``` +Examples of **correct** code for this rule: + +```js +/*eslint no-sequences: ["error", { allowInParentheses: false }]*/ + +for (i = 0, j = 10; i < j; i++, j--); +``` + ## When Not To Use It Disable this rule if sequence expressions with the comma operator are acceptable. diff --git a/lib/rules/no-sequences.js b/lib/rules/no-sequences.js index 051e4511886..fe516975fbc 100644 --- a/lib/rules/no-sequences.js +++ b/lib/rules/no-sequences.js @@ -11,6 +11,14 @@ const astUtils = require("./utils/ast-utils"); +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +const DEFAULT_OPTIONS = { + allowInParentheses: true +}; + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -30,9 +38,10 @@ module.exports = { properties: { allowInParentheses: { type: "boolean", - default: false + default: true } - } + }, + additionalProperties: false }], messages: { @@ -41,7 +50,7 @@ module.exports = { }, create(context) { - const options = context.options[0] || {}; + const options = Object.assign({}, DEFAULT_OPTIONS, context.options[0]); const sourceCode = context.getSourceCode(); /** diff --git a/tests/lib/rules/no-sequences.js b/tests/lib/rules/no-sequences.js index 8f86e56ee55..15fb9ee6d61 100644 --- a/tests/lib/rules/no-sequences.js +++ b/tests/lib/rules/no-sequences.js @@ -39,17 +39,20 @@ ruleTester.run("no-sequences", rule, { "var arr = [1, 2];", "var obj = {a: 1, b: 2};", "var a = 1, b = 2;", - { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] }, - { code: "(0,eval)(\"foo()\");", options: [{ allowInParentheses: true }] }, + "var foo = (1, 2);", + "(0,eval)(\"foo()\");", "for (i = 1, j = 2;; i++, j++);", - { code: "foo(a, (b, c), d);", options: [{ allowInParentheses: true }] }, - { code: "do {} while ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, - { code: "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", options: [{ allowInParentheses: true }] }, - { code: "if ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, - { code: "switch ((doSomething(), val)) {}", options: [{ allowInParentheses: true }] }, - { code: "while ((doSomething(), !!test));", options: [{ allowInParentheses: true }] }, - { code: "with ((doSomething(), val)) {}", options: [{ allowInParentheses: true }] }, - { code: "a => ((doSomething(), a))", options: [{ allowInParentheses: true }], env: { es6: true } } + "foo(a, (b, c), d);", + "do {} while ((doSomething(), !!test));", + "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", + "if ((doSomething(), !!test));", + "switch ((doSomething(), val)) {}", + "while ((doSomething(), !!test));", + "with ((doSomething(), val)) {}", + { code: "a => ((doSomething(), a))", env: { es6: true } }, + + // explicitly set option "allowInParentheses" to default value + { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] } ], // Examples of code that should trigger the rule @@ -66,25 +69,27 @@ ruleTester.run("no-sequences", rule, { }] }, { code: "a = 1, 2", errors: errors(6) }, - { code: "var foo = (1, 2);", options: [{ allowInParentheses: false }], errors: errors(13) }, - { code: "(0,eval)(\"foo()\");", errors: errors(3) }, - { code: "foo(a, (b, c), d);", errors: errors(10) }, { code: "do {} while (doSomething(), !!test);", errors: errors(27) }, - { code: "do {} while ((doSomething(), !!test));", errors: errors(28) }, { code: "for (; doSomething(), !!test; );", errors: errors(21) }, - { code: "for ((doSomething(), somethingElse()); (doSomething(), !!test); );", errors: errors(54) }, { code: "if (doSomething(), !!test);", errors: errors(18) }, - { code: "if ((doSomething(), !!test));", errors: errors(19) }, { code: "switch (doSomething(), val) {}", errors: errors(22) }, - { code: "switch ((doSomething(), val)) {}", errors: errors(23) }, { code: "while (doSomething(), !!test);", errors: errors(21) }, - { code: "while ((doSomething(), !!test));", errors: errors(22) }, { code: "with (doSomething(), val) {}", errors: errors(20) }, - { code: "with ((doSomething(), val)) {}", errors: errors(21) }, { code: "a => (doSomething(), a)", env: { es6: true }, errors: errors(20) }, - { code: "a => ((doSomething(), a))", env: { es6: true }, errors: errors(21) }, { code: "(1), 2", errors: errors(4) }, { code: "((1)) , (2)", errors: errors(7) }, - { code: "while((1) , 2);", errors: errors(11) } + { code: "while((1) , 2);", errors: errors(11) }, + + // option "allowInParentheses": do not allow sequence in parentheses + { code: "var foo = (1, 2);", options: [{ allowInParentheses: false }], errors: errors(13) }, + { code: "(0,eval)(\"foo()\");", options: [{ allowInParentheses: false }], errors: errors(3) }, + { code: "foo(a, (b, c), d);", options: [{ allowInParentheses: false }], errors: errors(10) }, + { code: "do {} while ((doSomething(), !!test));", options: [{ allowInParentheses: false }], errors: errors(28) }, + { code: "for (; (doSomething(), !!test); );", options: [{ allowInParentheses: false }], errors: errors(22) }, + { code: "if ((doSomething(), !!test));", options: [{ allowInParentheses: false }], errors: errors(19) }, + { code: "switch ((doSomething(), val)) {}", options: [{ allowInParentheses: false }], errors: errors(23) }, + { code: "while ((doSomething(), !!test));", options: [{ allowInParentheses: false }], errors: errors(22) }, + { code: "with ((doSomething(), val)) {}", options: [{ allowInParentheses: false }], errors: errors(21) }, + { code: "a => ((doSomething(), a))", options: [{ allowInParentheses: false }], env: { es6: true }, errors: errors(21) } ] }); From c85d1e1d04798abf931ed7038acca66fd542efbd Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Wed, 24 Mar 2021 06:01:23 +0100 Subject: [PATCH 4/6] restored removed sentence --- docs/rules/no-sequences.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md index a8e4b6a1bc1..36eeff11bf8 100644 --- a/docs/rules/no-sequences.md +++ b/docs/rules/no-sequences.md @@ -17,6 +17,7 @@ while (a = next(), a && a.length); This rule forbids the use of the comma operator, with the following exceptions: * In the initialization or update portions of a `for` statement. +* By default, if the expression sequence is explicitly wrapped in parentheses. This exception can be removed with the `allowInParentheses` option. Examples of **incorrect** code for this rule: From 4a8cc20ccdf28f65be9c739e5ad3d1b68b10cc2e Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Thu, 25 Mar 2021 06:22:16 +0100 Subject: [PATCH 5/6] changes from code review --- docs/rules/no-sequences.md | 12 +++++++----- tests/lib/rules/no-sequences.js | 3 +++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md index 36eeff11bf8..be2854c6a8b 100644 --- a/docs/rules/no-sequences.md +++ b/docs/rules/no-sequences.md @@ -67,12 +67,14 @@ with ((doSomething(), val)) {} This rule takes one option, an object, with the following properties: -* `"allowInParentheses"`: If set to `false`, this rule allows to use expression sequences that are explicitly wrapped in parentheses. Default value is `true`. +* `"allowInParentheses"`: If set to `false`, this rule disallows expression sequences that are explicitly wrapped in parentheses. Default value is `true`. -Examples of **incorrect** code for this rule: +### allowInParentheses + +Examples of **incorrect** code for this rule with the `{ "allowInParentheses": false }` option: ```js -/*eslint no-sequences: ["error", { allowInParentheses: false }]*/ +/*eslint no-sequences: ["error", { "allowInParentheses": false }]*/ foo = (doSomething(), val); @@ -91,10 +93,10 @@ while ((val = foo(), val < 42)); with ((doSomething(), val)) {} ``` -Examples of **correct** code for this rule: +Examples of **correct** code for this rule with the `{ "allowInParentheses": false }` option: ```js -/*eslint no-sequences: ["error", { allowInParentheses: false }]*/ +/*eslint no-sequences: ["error", { "allowInParentheses": false }]*/ for (i = 0, j = 10; i < j; i++, j--); ``` diff --git a/tests/lib/rules/no-sequences.js b/tests/lib/rules/no-sequences.js index 15fb9ee6d61..64ee355b928 100644 --- a/tests/lib/rules/no-sequences.js +++ b/tests/lib/rules/no-sequences.js @@ -51,6 +51,9 @@ ruleTester.run("no-sequences", rule, { "with ((doSomething(), val)) {}", { code: "a => ((doSomething(), a))", env: { es6: true } }, + // options object without "allowInParentheses" property + { code: "var foo = (1, 2);", options: [{}] }, + // explicitly set option "allowInParentheses" to default value { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] } ], From 198a32066b4d5c0826a5f4680ee5cc2260c11aa2 Mon Sep 17 00:00:00 2001 From: Daniel Rentz Date: Fri, 26 Mar 2021 06:59:15 +0100 Subject: [PATCH 6/6] code review --- docs/rules/no-sequences.md | 2 +- tests/lib/rules/no-sequences.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-sequences.md b/docs/rules/no-sequences.md index be2854c6a8b..af4a7a0edcf 100644 --- a/docs/rules/no-sequences.md +++ b/docs/rules/no-sequences.md @@ -67,7 +67,7 @@ with ((doSomething(), val)) {} This rule takes one option, an object, with the following properties: -* `"allowInParentheses"`: If set to `false`, this rule disallows expression sequences that are explicitly wrapped in parentheses. Default value is `true`. +* `"allowInParentheses"`: If set to `true` (default), this rule allows expression sequences that are explicitly wrapped in parentheses. ### allowInParentheses diff --git a/tests/lib/rules/no-sequences.js b/tests/lib/rules/no-sequences.js index 64ee355b928..f9eb8a6a30f 100644 --- a/tests/lib/rules/no-sequences.js +++ b/tests/lib/rules/no-sequences.js @@ -55,7 +55,11 @@ ruleTester.run("no-sequences", rule, { { code: "var foo = (1, 2);", options: [{}] }, // explicitly set option "allowInParentheses" to default value - { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] } + { code: "var foo = (1, 2);", options: [{ allowInParentheses: true }] }, + + // valid code with "allowInParentheses" set to `false` + { code: "for ((i = 0, j = 0); test; );", options: [{ allowInParentheses: false }] }, + { code: "for (; test; (i++, j++));", options: [{ allowInParentheses: false }] } ], // Examples of code that should trigger the rule