From 4d411e4c7063274d6d346f1b7ee46f7575d0bbd2 Mon Sep 17 00:00:00 2001 From: Percy Ma Date: Sat, 1 Jul 2023 05:38:04 +0800 Subject: [PATCH] feat: add ternaryOperandBinaryExpressions option to no-extra-parens rule (#17270) * feat: add `conditionalTernary` option to `no-extra-parens` rule * chore: improve coverage * chore: improve coverage * chore: avoid SequenceExpression miss * chore: change option name to ternaryOperandBinaryExpressions * docs: update docs * docs: fix space inside code * chore: update * chore: clean * docs: update * chore: update --- docs/src/rules/no-extra-parens.md | 21 +++++++++++++++++++++ lib/rules/no-extra-parens.js | 14 ++++++++++++-- tests/lib/rules/no-extra-parens.js | 12 ++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/docs/src/rules/no-extra-parens.md b/docs/src/rules/no-extra-parens.md index dd2b4642008..0476e628010 100644 --- a/docs/src/rules/no-extra-parens.md +++ b/docs/src/rules/no-extra-parens.md @@ -52,6 +52,7 @@ This rule has an object option for exceptions to the `"all"` option: * `"conditionalAssign": false` allows extra parentheses around assignments in conditional test expressions * `"returnAssign": false` allows extra parentheses around assignments in `return` statements * `"nestedBinaryExpressions": false` allows extra parentheses in nested binary expressions +* `"ternaryOperandBinaryExpressions": false` allows extra parentheses around binary expressions that are operands of ternary `?:` * `"ignoreJSX": "none|all|multi-line|single-line"` allows extra parentheses around no/all/multi-line/single-line JSX components. Defaults to `none`. * `"enforceForArrowConditionals": false` allows extra parentheses around ternary expressions which are the body of an arrow function * `"enforceForSequenceExpressions": false` allows extra parentheses around sequence expressions @@ -189,6 +190,26 @@ x = (a * b) / c; ::: +### ternaryOperandBinaryExpressions + +Examples of **correct** code for this rule with the `"all"` and `{ "ternaryOperandBinaryExpressions": false }` options: + +::: correct + +```js +/* eslint no-extra-parens: ["error", "all", { "ternaryOperandBinaryExpressions": false }] */ + +(a && b) ? foo : bar; + +(a - b > a) ? foo : bar; + +foo ? (bar || baz) : qux; + +foo ? bar : (baz || qux); +``` + +::: + ### ignoreJSX Examples of **correct** code for this rule with the `all` and `{ "ignoreJSX": "all" }` options: diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index e9080120fab..bb80987858c 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -46,6 +46,7 @@ module.exports = { type: "object", properties: { conditionalAssign: { type: "boolean" }, + ternaryOperandBinaryExpressions: { type: "boolean" }, nestedBinaryExpressions: { type: "boolean" }, returnAssign: { type: "boolean" }, ignoreJSX: { enum: ["none", "all", "single-line", "multi-line"] }, @@ -76,6 +77,7 @@ module.exports = { const precedence = astUtils.getPrecedence; const ALL_NODES = context.options[0] !== "functions"; const EXCEPT_COND_ASSIGN = ALL_NODES && context.options[1] && context.options[1].conditionalAssign === false; + const EXCEPT_COND_TERNARY = ALL_NODES && context.options[1] && context.options[1].ternaryOperandBinaryExpressions === false; const NESTED_BINARY = ALL_NODES && context.options[1] && context.options[1].nestedBinaryExpressions === false; const EXCEPT_RETURN_ASSIGN = ALL_NODES && context.options[1] && context.options[1].returnAssign === false; const IGNORE_JSX = ALL_NODES && context.options[1] && context.options[1].ignoreJSX; @@ -886,18 +888,26 @@ module.exports = { if (isReturnAssignException(node)) { return; } + + const availableTypes = new Set(["BinaryExpression", "LogicalExpression"]); + if ( + !(EXCEPT_COND_TERNARY && availableTypes.has(node.test.type)) && !isCondAssignException(node) && hasExcessParensWithPrecedence(node.test, precedence({ type: "LogicalExpression", operator: "||" })) ) { report(node.test); } - if (hasExcessParensWithPrecedence(node.consequent, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { + if ( + !(EXCEPT_COND_TERNARY && availableTypes.has(node.consequent.type)) && + hasExcessParensWithPrecedence(node.consequent, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.consequent); } - if (hasExcessParensWithPrecedence(node.alternate, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { + if ( + !(EXCEPT_COND_TERNARY && availableTypes.has(node.alternate.type)) && + hasExcessParensWithPrecedence(node.alternate, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.alternate); } }, diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index c160ace3c36..2d61522cb72 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -304,6 +304,14 @@ ruleTester.run("no-extra-parens", rule, { { code: "while (((foo = bar()))) {}", options: ["all", { conditionalAssign: false }] }, { code: "var a = (((b = c))) ? foo : bar;", options: ["all", { conditionalAssign: false }] }, + // ["all", { ternaryOperandBinaryExpressions: false }] enables extra parens around conditional ternary + { code: "(a && b) ? foo : bar", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + { code: "(a - b > a) ? foo : bar", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + { code: "foo ? (bar || baz) : qux", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + { code: "foo ? bar : (baz || qux)", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + { code: "(a, b) ? (c, d) : (e, f)", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + { code: "(a = b) ? c : d", options: ["all", { ternaryOperandBinaryExpressions: false }] }, + // ["all", { nestedBinaryExpressions: false }] enables extra parens around conditional assignments { code: "a + (b * c)", options: ["all", { nestedBinaryExpressions: false }] }, { code: "(a * b) + c", options: ["all", { nestedBinaryExpressions: false }] }, @@ -922,6 +930,10 @@ ruleTester.run("no-extra-parens", rule, { invalid("a ? b : (c = d)", "a ? b : c = d", "AssignmentExpression"), invalid("(c = d) ? (b) : c", "(c = d) ? b : c", "Identifier", null, { options: ["all", { conditionalAssign: false }] }), invalid("(c = d) ? b : (c)", "(c = d) ? b : c", "Identifier", null, { options: ["all", { conditionalAssign: false }] }), + invalid("(a) ? foo : bar", "a ? foo : bar", "Identifier", null, { options: ["all", { ternaryOperandBinaryExpressions: false }] }), + invalid("(a()) ? foo : bar", "a() ? foo : bar", "CallExpression", null, { options: ["all", { ternaryOperandBinaryExpressions: false }] }), + invalid("(a.b) ? foo : bar", "a.b ? foo : bar", "MemberExpression", null, { options: ["all", { ternaryOperandBinaryExpressions: false }] }), + invalid("(a || b) ? foo : (bar)", "(a || b) ? foo : bar", "Identifier", null, { options: ["all", { ternaryOperandBinaryExpressions: false }] }), invalid("f((a = b))", "f(a = b)", "AssignmentExpression"), invalid("a, (b = c)", "a, b = c", "AssignmentExpression"), invalid("a = (b * c)", "a = b * c", "BinaryExpression"),