From b01d47f64b3ef23c1cd99be794716122a04872a1 Mon Sep 17 00:00:00 2001 From: karthik Date: Tue, 16 Jul 2019 23:40:22 +0530 Subject: [PATCH 01/10] ternary operators added to tule --- lib/rules/no-mixed-operators.js | 47 ++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 21e1d95c684..07ed2b22980 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -34,7 +34,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. @@ -119,7 +119,6 @@ module.exports = { function shouldIgnore(node) { const a = node; const b = node.parent; - return ( !includesBothInAGroup(options.groups, a.operator, b.operator) || ( @@ -139,12 +138,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 +165,11 @@ module.exports = { * @returns {Token} The operator token of the node. */ function getOperatorToken(node) { - return sourceCode.getTokenAfter(node.left, astUtils.isNotClosingParenToken); + return sourceCode.getTokenAfter(node.left || node.test, astUtils.isNotClosingParenToken); + } + + function getOperatorTokenForConditional(node) { + return sourceCode.getTokenAfter(node, astUtils.isNotClosingParenToken); } /** @@ -167,13 +183,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 = ((parent.left || parent.test) === node) ? node : parent; + const right = ((parent.left || parent.test) !== node) ? node : parent; const message = "Unexpected mix of '{{leftOperator}}' and '{{rightOperator}}'."; const data = { - leftOperator: left.operator, - rightOperator: right.operator + leftOperator: left.operator || "ternary operator", + rightOperator: right.operator || "ternary operator" }; context.report({ @@ -198,12 +214,19 @@ 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' && isMixedWithConditionalParent(node.parent) ) { + reportBothOperators(node); + } else { + if (TARGET_NODE_TYPE.test(node.parent.type) && + isMixedWithParent(node) && + !shouldIgnore(node) + ) { + reportBothOperators(node); + } + } } + } return { From 0ae2e91e184485aa6e78bff44f64b43c9ecfa1a9 Mon Sep 17 00:00:00 2001 From: karthik Date: Tue, 16 Jul 2019 23:41:29 +0530 Subject: [PATCH 02/10] ternary operators added --- lib/rules/no-mixed-operators.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 07ed2b22980..614b35017ca 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -119,6 +119,7 @@ module.exports = { function shouldIgnore(node) { const a = node; const b = node.parent; + return ( !includesBothInAGroup(options.groups, a.operator, b.operator) || ( @@ -138,7 +139,7 @@ module.exports = { * @returns {boolean} `true` if the node was mixed. */ function isMixedWithParent(node) { - + return ( node.operator !== node.parent.operator && !astUtils.isParenthesised(sourceCode, node) @@ -146,7 +147,7 @@ module.exports = { } /** - * Checks whether the operator of a given node is mixed with a + * 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 @@ -168,10 +169,6 @@ module.exports = { return sourceCode.getTokenAfter(node.left || node.test, astUtils.isNotClosingParenToken); } - function getOperatorTokenForConditional(node) { - return sourceCode.getTokenAfter(node, astUtils.isNotClosingParenToken); - } - /** * Reports both the operator of a given node and the operator of the * parent node. @@ -214,8 +211,8 @@ module.exports = { * @returns {void} */ function check(node) { - if(TARGET_NODE_TYPE.test(node.parent.type)) { - if(node.parent.type === 'ConditionalExpression' && isMixedWithConditionalParent(node.parent) ) { + if (TARGET_NODE_TYPE.test(node.parent.type)) { + if (node.parent.type === "ConditionalExpression" && isMixedWithConditionalParent(node.parent)) { reportBothOperators(node); } else { if (TARGET_NODE_TYPE.test(node.parent.type) && @@ -223,10 +220,10 @@ module.exports = { !shouldIgnore(node) ) { reportBothOperators(node); - } + } } } - + } return { From dc2e975a3496059cabadc98bf7c2e15161ffe09d Mon Sep 17 00:00:00 2001 From: karthik Date: Wed, 17 Jul 2019 11:34:41 +0530 Subject: [PATCH 03/10] cases added for ternary operator mixed with logical --- tests/lib/rules/no-mixed-operators.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/lib/rules/no-mixed-operators.js b/tests/lib/rules/no-mixed-operators.js index d740b856225..382901cdbd2 100644 --- a/tests/lib/rules/no-mixed-operators.js +++ b/tests/lib/rules/no-mixed-operators.js @@ -47,7 +47,9 @@ ruleTester.run("no-mixed-operators", rule, { { code: "a * b / c", options: [{ allowSamePrecedence: true }] - } + }, + "a || (b ? c : d)", + "(a || b) ? c : d" ], invalid: [ { @@ -110,6 +112,13 @@ 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", + errors: [ + { column: 3, message: "Unexpected mix of '||' and 'ternary operator'." }, + { column: 8, message: "Unexpected mix of '||' and 'ternary operator'." } + ] } ] }); From be4a213a60363762e72e7ab6ac5bfa17270c939a Mon Sep 17 00:00:00 2001 From: karthik Date: Wed, 17 Jul 2019 11:42:09 +0530 Subject: [PATCH 04/10] documentation updated --- docs/rules/no-mixed-operators.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-mixed-operators.md b/docs/rules/no-mixed-operators.md index 3c0bb2fab6b..69bc8fc9c52 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 'ternary operator'. (no-mixed-operators) +1:18 Unexpected mix of '&&' and 'ternary operator'. (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. From 849caa72d8cfdba535c362a15aedf36bc0b7d5e8 Mon Sep 17 00:00:00 2001 From: karthik Date: Wed, 17 Jul 2019 22:51:41 +0530 Subject: [PATCH 05/10] hard coded string message removed and in place added the operator.. --- docs/rules/no-mixed-operators.md | 6 +++--- lib/rules/no-mixed-operators.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/rules/no-mixed-operators.md b/docs/rules/no-mixed-operators.md index 69bc8fc9c52..d471ee38e06 100644 --- a/docs/rules/no-mixed-operators.md +++ b/docs/rules/no-mixed-operators.md @@ -5,7 +5,7 @@ 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; /*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*/ @@ -32,8 +32,8 @@ var foo = a && b ? c : d; will generate ```sh -1:13 Unexpected mix of '&&' and 'ternary operator'. (no-mixed-operators) -1:18 Unexpected mix of '&&' and 'ternary operator'. (no-mixed-operators) +1:13 Unexpected mix of '&&' and '?:'. (no-mixed-operators) +1:18 Unexpected mix of '&&' and '?:'. (no-mixed-operators) ``` diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 614b35017ca..75287e9be7b 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -185,8 +185,8 @@ module.exports = { const message = "Unexpected mix of '{{leftOperator}}' and '{{rightOperator}}'."; const data = { - leftOperator: left.operator || "ternary operator", - rightOperator: right.operator || "ternary operator" + leftOperator: left.operator || "?:", + rightOperator: right.operator || "?:" }; context.report({ From 885abd29f9cce893ff6685c39c40a57e0f659377 Mon Sep 17 00:00:00 2001 From: karthik Date: Thu, 18 Jul 2019 00:17:40 +0530 Subject: [PATCH 06/10] test cases fixed --- tests/lib/rules/no-mixed-operators.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/no-mixed-operators.js b/tests/lib/rules/no-mixed-operators.js index 382901cdbd2..bec23b3f258 100644 --- a/tests/lib/rules/no-mixed-operators.js +++ b/tests/lib/rules/no-mixed-operators.js @@ -116,8 +116,8 @@ ruleTester.run("no-mixed-operators", rule, { { code: "a || b ? c : d", errors: [ - { column: 3, message: "Unexpected mix of '||' and 'ternary operator'." }, - { column: 8, message: "Unexpected mix of '||' and 'ternary operator'." } + { column: 3, message: "Unexpected mix of '||' and '?:'." }, + { column: 8, message: "Unexpected mix of '||' and '?:'." } ] } ] From b6cafb4b5d758cfc881c876a946141df10ecd04a Mon Sep 17 00:00:00 2001 From: karthik Date: Wed, 24 Jul 2019 12:07:47 +0530 Subject: [PATCH 07/10] added the option to check for ternary operator and extra tests --- lib/rules/no-mixed-operators.js | 9 ++++++--- tests/lib/rules/no-mixed-operators.js | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 75287e9be7b..5a162aebcc7 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, @@ -121,7 +123,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) @@ -212,7 +214,7 @@ module.exports = { */ function check(node) { if (TARGET_NODE_TYPE.test(node.parent.type)) { - if (node.parent.type === "ConditionalExpression" && isMixedWithConditionalParent(node.parent)) { + if (node.parent.type === "ConditionalExpression" && !shouldIgnore(node) && isMixedWithConditionalParent(node.parent)) { reportBothOperators(node); } else { if (TARGET_NODE_TYPE.test(node.parent.type) && @@ -229,6 +231,7 @@ module.exports = { return { BinaryExpression: check, LogicalExpression: check + }; } }; diff --git a/tests/lib/rules/no-mixed-operators.js b/tests/lib/rules/no-mixed-operators.js index bec23b3f258..9bc721f48a0 100644 --- a/tests/lib/rules/no-mixed-operators.js +++ b/tests/lib/rules/no-mixed-operators.js @@ -115,10 +115,35 @@ ruleTester.run("no-mixed-operators", rule, { }, { 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 '&&'." } + ] } ] }); From ed7643313ae9f44722139a7d586bacd7780c3549 Mon Sep 17 00:00:00 2001 From: karthik Date: Thu, 25 Jul 2019 14:49:38 +0530 Subject: [PATCH 08/10] documentation and tests updated --- docs/rules/no-mixed-operators.md | 17 ++++++++++++++++- tests/lib/rules/no-mixed-operators.js | 11 ++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-mixed-operators.md b/docs/rules/no-mixed-operators.md index d471ee38e06..3206d18fb7d 100644 --- a/docs/rules/no-mixed-operators.md +++ b/docs/rules/no-mixed-operators.md @@ -88,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 @@ -100,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. @@ -115,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 @@ -131,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/tests/lib/rules/no-mixed-operators.js b/tests/lib/rules/no-mixed-operators.js index 9bc721f48a0..4e822181f53 100644 --- a/tests/lib/rules/no-mixed-operators.js +++ b/tests/lib/rules/no-mixed-operators.js @@ -48,8 +48,17 @@ 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", + "a || b ? c : d" ], invalid: [ { From 269f3095970be9f42208393f957a2c3b8ff9c08a Mon Sep 17 00:00:00 2001 From: karthik Date: Fri, 26 Jul 2019 01:37:54 +0530 Subject: [PATCH 09/10] check for test or left node moved to a sperate function --- lib/rules/no-mixed-operators.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index 5a162aebcc7..cf3e3d58e7b 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -67,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's parent 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 //------------------------------------------------------------------------------ @@ -168,7 +180,7 @@ module.exports = { * @returns {Token} The operator token of the node. */ function getOperatorToken(node) { - return sourceCode.getTokenAfter(node.left || node.test, astUtils.isNotClosingParenToken); + return sourceCode.getTokenAfter(getChildNode(node), astUtils.isNotClosingParenToken); } /** @@ -182,8 +194,8 @@ module.exports = { */ function reportBothOperators(node) { const parent = node.parent; - const left = ((parent.left || parent.test) === node) ? node : parent; - const right = ((parent.left || parent.test) !== 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 = { From 03dcd512f3eb35b2911b2e103e4532814575f8db Mon Sep 17 00:00:00 2001 From: karthik Date: Fri, 26 Jul 2019 10:40:23 +0530 Subject: [PATCH 10/10] wrong comment. Has been updated --- lib/rules/no-mixed-operators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js index cf3e3d58e7b..8d1c7a6af43 100644 --- a/lib/rules/no-mixed-operators.js +++ b/lib/rules/no-mixed-operators.js @@ -68,7 +68,7 @@ function includesBothInAGroup(groups, left, right) { } /** - * Checks whether the given node's parent is a conditional expression and returns the test node else the left node. + * 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