diff --git a/docs/rules/no-mixed-operators.md b/docs/rules/no-mixed-operators.md index 3c0bb2fab6b..3206d18fb7d 100644 --- a/docs/rules/no-mixed-operators.md +++ b/docs/rules/no-mixed-operators.md @@ -5,6 +5,8 @@ This rule warns when different operators are used consecutively without parenthe ```js var foo = a && b || c || d; /*BAD: Unexpected mix of '&&' and '||'.*/ +var foo = a && b ? c : d; /*BAD: Unexpected mix of '&&' and '?:'.*/ +var foo = (a && b) ? c : d; /*GOOD*/ var foo = (a && b) || c || d; /*GOOD*/ var foo = a && (b || c || d); /*GOOD*/ ``` @@ -23,10 +25,21 @@ will generate 1:18 Unexpected mix of '&&' and '||'. (no-mixed-operators) ``` +```js +var foo = a && b ? c : d; +``` + +will generate + +```sh +1:13 Unexpected mix of '&&' and '?:'. (no-mixed-operators) +1:18 Unexpected mix of '&&' and '?:'. (no-mixed-operators) +``` + ## Rule Details -This rule checks `BinaryExpression` and `LogicalExpression`. +This rule checks `BinaryExpression`, `LogicalExpression` and `ConditionalExpression`. This rule may conflict with [no-extra-parens](no-extra-parens.md) rule. If you use both this and [no-extra-parens](no-extra-parens.md) rule together, you need to use the `nestedBinaryExpressions` option of [no-extra-parens](no-extra-parens.md) rule. @@ -75,7 +88,8 @@ var foo = (a + b) * c; This rule has 2 options. -* `groups` (`string[][]`) - specifies operator groups to be checked. The `groups` option is a list of groups, and a group is a list of binary operators. Default operator groups are defined as arithmetic, bitwise, comparison, logical, and relational operators. +* `groups` (`string[][]`) - specifies operator groups to be checked. The `groups` option is a list of groups, and a group is a list of binary operators. Default operator groups are defined as arithmetic, bitwise, comparison, logical, and relational operators. Note: Ternary operator(?:) can be part of any group and by default is allowed to be mixed with other operators. + * `allowSamePrecedence` (`boolean`) - specifies whether to allow mixed operators if they are of equal precedence. Default is `true`. ### groups @@ -87,6 +101,7 @@ The following operators can be used in `groups` option: * Comparison Operators: `"=="`, `"!="`, `"==="`, `"!=="`, `">"`, `">="`, `"<"`, `"<="` * Logical Operators: `"&&"`, `"||"` * Relational Operators: `"in"`, `"instanceof"` +* Ternary Operator: `?:` Now, consider the following group configuration: `{"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}`. There are 2 groups specified in this configuration: bitwise operators and logical operators. @@ -102,6 +117,12 @@ var foo = a && b < 0 || c > 0 || d + 1 === 0; var foo = a & b | c; ``` +```js +/*eslint no-mixed-operators: ["error", {"groups": [["&&", "||", "?:"]]}]*/ + +var foo = a || b ? c : d; +``` + Examples of **correct** code for this rule with `{"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}` option: ```js @@ -118,6 +139,13 @@ var foo = a + (b * c); var foo = (a + b) * c; ``` +```js +/*eslint no-mixed-operators: ["error", {"groups": [["&&", "||", "?:"]]}]*/ + +var foo = (a || b) ? c : d; +var foo = a || (b ? c : d); +``` + ### allowSamePrecedence Examples of **correct** code for this rule with `{"allowSamePrecedence": true}` option: diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 21e1d95c684..8d1c7a6af43 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -20,12 +20,14 @@ const BITWISE_OPERATORS = ["&", "|", "^", "~", "<<", ">>", ">>>"]; const COMPARISON_OPERATORS = ["==", "!=", "===", "!==", ">", ">=", "<", "<="]; const LOGICAL_OPERATORS = ["&&", "||"]; const RELATIONAL_OPERATORS = ["in", "instanceof"]; +const TERNARY_OPERATOR = ["?:"]; const ALL_OPERATORS = [].concat( ARITHMETIC_OPERATORS, BITWISE_OPERATORS, COMPARISON_OPERATORS, LOGICAL_OPERATORS, - RELATIONAL_OPERATORS + RELATIONAL_OPERATORS, + TERNARY_OPERATOR ); const DEFAULT_GROUPS = [ ARITHMETIC_OPERATORS, @@ -34,7 +36,7 @@ const DEFAULT_GROUPS = [ LOGICAL_OPERATORS, RELATIONAL_OPERATORS ]; -const TARGET_NODE_TYPE = /^(?:Binary|Logical)Expression$/u; +const TARGET_NODE_TYPE = /^(?:Binary|Logical|Conditional)Expression$/u; /** * Normalizes options. @@ -65,6 +67,18 @@ function includesBothInAGroup(groups, left, right) { return groups.some(group => group.indexOf(left) !== -1 && group.indexOf(right) !== -1); } +/** + * Checks whether the given node is a conditional expression and returns the test node else the left node. + * + * @param {ASTNode} node - A node which can be a BinaryExpression or a LogicalExpression node. + * This parent node can be BinaryExpression, LogicalExpression + * , or a ConditionalExpression node + * @returns {ASTNode} node the appropriate node(left or test). + */ +function getChildNode(node) { + return node.type === "ConditionalExpression" ? node.test : node.left; +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -121,7 +135,7 @@ module.exports = { const b = node.parent; return ( - !includesBothInAGroup(options.groups, a.operator, b.operator) || + !includesBothInAGroup(options.groups, a.operator, b.type === "ConditionalExpression" ? "?:" : b.operator) || ( options.allowSamePrecedence && astUtils.getPrecedence(a) === astUtils.getPrecedence(b) @@ -139,12 +153,25 @@ module.exports = { * @returns {boolean} `true` if the node was mixed. */ function isMixedWithParent(node) { + return ( node.operator !== node.parent.operator && !astUtils.isParenthesised(sourceCode, node) ); } + /** + * Checks whether the operator of a given node is mixed with a + * conditional expression. + * + * @param {ASTNode} node - A node to check. This is a conditional + * expression node + * @returns {boolean} `true` if the node was mixed. + */ + function isMixedWithConditionalParent(node) { + return !astUtils.isParenthesised(sourceCode, node) && !astUtils.isParenthesised(sourceCode, node.test); + } + /** * Gets the operator token of a given node. * @@ -153,7 +180,7 @@ module.exports = { * @returns {Token} The operator token of the node. */ function getOperatorToken(node) { - return sourceCode.getTokenAfter(node.left, astUtils.isNotClosingParenToken); + return sourceCode.getTokenAfter(getChildNode(node), astUtils.isNotClosingParenToken); } /** @@ -167,13 +194,13 @@ module.exports = { */ function reportBothOperators(node) { const parent = node.parent; - const left = (parent.left === node) ? node : parent; - const right = (parent.left !== node) ? node : parent; + const left = (getChildNode(parent) === node) ? node : parent; + const right = (getChildNode(parent) !== node) ? node : parent; const message = "Unexpected mix of '{{leftOperator}}' and '{{rightOperator}}'."; const data = { - leftOperator: left.operator, - rightOperator: right.operator + leftOperator: left.operator || "?:", + rightOperator: right.operator || "?:" }; context.report({ @@ -198,17 +225,25 @@ module.exports = { * @returns {void} */ function check(node) { - if (TARGET_NODE_TYPE.test(node.parent.type) && - isMixedWithParent(node) && - !shouldIgnore(node) - ) { - reportBothOperators(node); + if (TARGET_NODE_TYPE.test(node.parent.type)) { + if (node.parent.type === "ConditionalExpression" && !shouldIgnore(node) && isMixedWithConditionalParent(node.parent)) { + reportBothOperators(node); + } else { + if (TARGET_NODE_TYPE.test(node.parent.type) && + isMixedWithParent(node) && + !shouldIgnore(node) + ) { + reportBothOperators(node); + } + } } + } return { BinaryExpression: check, LogicalExpression: check + }; } }; diff --git a/tests/lib/rules/no-mixed-operators.js b/tests/lib/rules/no-mixed-operators.js index d740b856225..4e822181f53 100644 --- a/tests/lib/rules/no-mixed-operators.js +++ b/tests/lib/rules/no-mixed-operators.js @@ -47,7 +47,18 @@ ruleTester.run("no-mixed-operators", rule, { { code: "a * b / c", options: [{ allowSamePrecedence: true }] - } + }, + { + code: "(a || b) ? c : d", + options: [{ groups: [["&&", "||", "?:"]] }] + }, + { + code: "a || (b ? c : d)", + options: [{ groups: [["&&", "||", "?:"]] }] + }, + "a || (b ? c : d)", + "(a || b) ? c : d", + "a || b ? c : d" ], invalid: [ { @@ -110,6 +121,38 @@ ruleTester.run("no-mixed-operators", rule, { { column: 3, message: "Unexpected mix of '*' and '/'." }, { column: 7, message: "Unexpected mix of '*' and '/'." } ] + }, + { + code: "a || b ? c : d", + options: [{ groups: [["&&", "||", "?:"]] }], + errors: [ + { column: 3, message: "Unexpected mix of '||' and '?:'." }, + { column: 8, message: "Unexpected mix of '||' and '?:'." } + ] + }, + { + code: "a && b ? 1 : 2", + options: [{ groups: [["&&", "||", "?:"]] }], + errors: [ + { column: 3, message: "Unexpected mix of '&&' and '?:'." }, + { column: 8, message: "Unexpected mix of '&&' and '?:'." } + ] + }, + { + code: "x ? a && b : 0", + options: [{ groups: [["&&", "||", "?:"]] }], + errors: [ + { column: 3, message: "Unexpected mix of '?:' and '&&'." }, + { column: 7, message: "Unexpected mix of '?:' and '&&'." } + ] + }, + { + code: "x ? 0 : a && b", + options: [{ groups: [["&&", "||", "?:"]] }], + errors: [ + { column: 3, message: "Unexpected mix of '?:' and '&&'." }, + { column: 11, message: "Unexpected mix of '?:' and '&&'." } + ] } ] });