diff --git a/conf/eslint.json b/conf/eslint.json index c95ba5c65e7..7aff7058046 100755 --- a/conf/eslint.json +++ b/conf/eslint.json @@ -55,6 +55,7 @@ "no-lone-blocks": "off", "no-lonely-if": "off", "no-loop-func": "off", + "no-mixed-operators": "off", "no-mixed-requires": "off", "no-mixed-spaces-and-tabs": "error", "linebreak-style": "off", diff --git a/docs/rules/README.md b/docs/rules/README.md index 34aab98c289..0a0aa6c3e93 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -188,6 +188,7 @@ These rules relate to style guidelines, and are therefore quite subjective: * [no-continue](no-continue.md): disallow `continue` statements * [no-inline-comments](no-inline-comments.md): disallow inline comments after code * [no-lonely-if](no-lonely-if.md): disallow `if` statements as the only statement in `else` blocks +* [no-mixed-operators](no-mixed-operators.md): disallow mixes of different operators * [no-mixed-spaces-and-tabs](no-mixed-spaces-and-tabs.md): disallow mixed spaces and tabs for indentation (recommended) * [no-multiple-empty-lines](no-multiple-empty-lines.md): disallow multiple empty lines * [no-negated-condition](no-negated-condition.md): disallow negated conditions diff --git a/docs/rules/no-mixed-operators.md b/docs/rules/no-mixed-operators.md new file mode 100644 index 00000000000..2badb53eea6 --- /dev/null +++ b/docs/rules/no-mixed-operators.md @@ -0,0 +1,138 @@ +# Disallow mixes of different operators (no-mixed-operators) + +Enclosing complex expressions by parentheses clarifies the developer's intention, which makes the code more readable. +This rule warns when different operators are used consecutively without parentheses in an expression. + +```js +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*/ +``` + +## Rule Details + +This rule checks `BinaryExpression` and `LogicalExpression`. + +This rule may conflict with [no-extra-parens] rule. +If you use both this and [no-extra-parens] rule together, you need to use the `nestedBinaryExpressions` option of [no-extra-parens] rule. + +Examples of **incorrect** code for this rule: + +```js +/*eslint no-mixed-operators: "error"*/ + +var foo = a && b < 0 || c > 0 || d + 1 === 0; +var foo = a + b * c; +``` + +Examples of **correct** code for this rule: + +```js +/*eslint no-mixed-operators: "error"*/ + +var foo = a || b || c; +var foo = a && b && c; +var foo = (a && b < 0) || c > 0 || d + 1 === 0; +var foo = a && (b < 0 || c > 0 || d + 1 === 0); +var foo = a + (b * c); +var foo = (a + b) * c; +``` + +## Options + +```json +{ + "no-mixed-operators": [ + "error", + { + "groups": [ + ["+", "-", "*", "/", "%", "**"], + ["&", "|", "^", "~", "<<", ">>", ">>>"], + ["==", "!=", "===", "!==", ">", ">=", "<", "<="], + ["&&", "||"], + ["in", "instanceof"] + ], + "allowSamePrecedence": true + } + ] +} +``` + +This rule has 2 options. + +* `groups` (`string[][]`) - specifies groups to compare operators. + When this rule compares two operators, if both operators are included in a same group, this rule checks it. Otherwise, this rule ignores it. + This value is a list of groups. The group is a list of binary operators. + Default is the groups for each kind of operators. +* `allowSamePrecedence` (`boolean`) - specifies to allow mix of 2 operators if those have the same precedence. Default is `true`. + +### groups + +The following operators can be used in `groups` option: + +* Arithmetic Operators: `"+"`, `"-"`, `"*"`, `"/"`, `"%"`, `"**"` +* Bitwise Operators: `"&"`, `"|"`, `"^"`, `"~"`, `"<<"`, `">>"`, `">>>"` +* Comparison Operators: `"=="`, `"!="`, `"==="`, `"!=="`, `">"`, `">="`, `"<"`, `"<="` +* Logical Operators: `"&&"`, `"||"` +* Relational Operators: `"in"`, `"instanceof"` + +Now, considers about `{"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}` configure. +This configure has 2 groups: bitwise operators and logical operators. +This rule checks only if both operators are included in a same group. +So, in this case, this rule comes to check between bitwise operators and between logical operators. +This rule ignores other operators. + +Examples of **incorrect** code for this rule with `{"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}` option: + +```js +/*eslint no-mixed-operators: ["error", {"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}]*/ + +var foo = a && b < 0 || c > 0 || d + 1 === 0; +var foo = a & b | c; +``` + +Examples of **correct** code for this rule with `{"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}` option: + +```js +/*eslint no-mixed-operators: ["error", {"groups": [["&", "|", "^", "~", "<<", ">>", ">>>"], ["&&", "||"]]}]*/ + +var foo = a || b > 0 || c + 1 === 0; +var foo = a && b > 0 && c + 1 === 0; +var foo = (a && b < 0) || c > 0 || d + 1 === 0; +var foo = a && (b < 0 || c > 0 || d + 1 === 0); +var foo = (a & b) | c; +var foo = a & (b | c); +var foo = a + b * c; +var foo = a + (b * c); +var foo = (a + b) * c; +``` + +### allowSamePrecedence + +Examples of **correct** code for this rule with `{"allowSamePrecedence": true}` option: + +```js +/*eslint no-mixed-operators: ["error", {"allowSamePrecedence": true}]*/ + +// + and - have the same precedence. +var foo = a + b - c; +``` + +Examples of **incorrect** code for this rule with `{"allowSamePrecedence": false}` option: + +```js +/*eslint no-mixed-operators: ["error", {"allowSamePrecedence": false}]*/ + +// + and - have the same precedence. +var foo = a + b - c; +``` + +## When Not To Use It + +If you don't want to be notified about mixed operators, then it's safe to disable this rule. + +## Related Rules + +* [no-extra-parens] + +[no-extra-parens]: no-extra-parens.md diff --git a/lib/ast-utils.js b/lib/ast-utils.js index c340103e948..c8d6dcb4915 100644 --- a/lib/ast-utils.js +++ b/lib/ast-utils.js @@ -460,5 +460,96 @@ module.exports = { /* istanbul ignore next */ return true; + }, + + /** + * Get the precedence level based on the node type + * @param {ASTNode} node node to evaluate + * @returns {int} precedence level + * @private + */ + getPrecedence: function(node) { + switch (node.type) { + case "SequenceExpression": + return 0; + + case "AssignmentExpression": + case "ArrowFunctionExpression": + case "YieldExpression": + return 1; + + case "ConditionalExpression": + return 3; + + case "LogicalExpression": + switch (node.operator) { + case "||": + return 4; + case "&&": + return 5; + + // no default + } + + /* falls through */ + + case "BinaryExpression": + + switch (node.operator) { + case "|": + return 6; + case "^": + return 7; + case "&": + return 8; + case "==": + case "!=": + case "===": + case "!==": + return 9; + case "<": + case "<=": + case ">": + case ">=": + case "in": + case "instanceof": + return 10; + case "<<": + case ">>": + case ">>>": + return 11; + case "+": + case "-": + return 12; + case "*": + case "/": + case "%": + return 13; + + // no default + } + + /* falls through */ + + case "UnaryExpression": + return 14; + + case "UpdateExpression": + return 15; + + case "CallExpression": + + // IIFE is allowed to have parens in any position (#655) + if (node.callee.type === "FunctionExpression") { + return -1; + } + return 16; + + case "NewExpression": + return 17; + + // no default + } + return 18; } }; diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index c2008ed020f..c33a64920ff 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -57,6 +57,7 @@ module.exports = { var sourceCode = context.getSourceCode(); var isParenthesised = astUtils.isParenthesised.bind(astUtils, sourceCode); + var precedence = astUtils.getPrecedence; var ALL_NODES = context.options[0] !== "functions"; var EXCEPT_COND_ASSIGN = ALL_NODES && context.options[1] && context.options[1].conditionalAssign === false; var NESTED_BINARY = ALL_NODES && context.options[1] && context.options[1].nestedBinaryExpressions === false; @@ -255,98 +256,6 @@ module.exports = { throw new Error("unreachable"); } - /** - * Get the precedence level based on the node type - * @param {ASTNode} node node to evaluate - * @returns {int} precedence level - * @private - */ - function precedence(node) { - - switch (node.type) { - case "SequenceExpression": - return 0; - - case "AssignmentExpression": - case "ArrowFunctionExpression": - case "YieldExpression": - return 1; - - case "ConditionalExpression": - return 3; - - case "LogicalExpression": - switch (node.operator) { - case "||": - return 4; - case "&&": - return 5; - - // no default - } - - /* falls through */ - - case "BinaryExpression": - - switch (node.operator) { - case "|": - return 6; - case "^": - return 7; - case "&": - return 8; - case "==": - case "!=": - case "===": - case "!==": - return 9; - case "<": - case "<=": - case ">": - case ">=": - case "in": - case "instanceof": - return 10; - case "<<": - case ">>": - case ">>>": - return 11; - case "+": - case "-": - return 12; - case "*": - case "/": - case "%": - return 13; - - // no default - } - - /* falls through */ - - case "UnaryExpression": - return 14; - - case "UpdateExpression": - return 15; - - case "CallExpression": - - // IIFE is allowed to have parens in any position (#655) - if (node.callee.type === "FunctionExpression") { - return -1; - } - return 16; - - case "NewExpression": - return 17; - - // no default - } - return 18; - } - /** * Report the node * @param {ASTNode} node node to evaluate diff --git a/lib/rules/no-mixed-operators.js b/lib/rules/no-mixed-operators.js new file mode 100644 index 00000000000..31aee574483 --- /dev/null +++ b/lib/rules/no-mixed-operators.js @@ -0,0 +1,212 @@ +/** + * @fileoverview Rule to disallow mixed binary operators. + * @author Toru Nagashima + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var astUtils = require("../ast-utils.js"); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +var ARITHMETIC_OPERATORS = ["+", "-", "*", "/", "%", "**"]; +var BITWISE_OPERATORS = ["&", "|", "^", "~", "<<", ">>", ">>>"]; +var COMPARISON_OPERATORS = ["==", "!=", "===", "!==", ">", ">=", "<", "<="]; +var LOGICAL_OPERATORS = ["&&", "||"]; +var RELATIONAL_OPERATORS = ["in", "instanceof"]; +var ALL_OPERATORS = [].concat( + ARITHMETIC_OPERATORS, + BITWISE_OPERATORS, + COMPARISON_OPERATORS, + LOGICAL_OPERATORS, + RELATIONAL_OPERATORS +); +var DEFAULT_GROUPS = [ + ARITHMETIC_OPERATORS, + BITWISE_OPERATORS, + COMPARISON_OPERATORS, + LOGICAL_OPERATORS, + RELATIONAL_OPERATORS +]; +var TARGET_NODE_TYPE = /^(?:Binary|Logical)Expression$/; + +/** + * Normalizes options. + * + * @param {object|undefined} options - A options object to normalize. + * @returns {object} Normalized option object. + */ +function normalizeOptions(options) { + var hasGroups = (options && options.groups && options.groups.length > 0); + var groups = hasGroups ? options.groups : DEFAULT_GROUPS; + var allowSamePrecedence = (options && options.allowSamePrecedence) !== false; + + return { + groups: groups, + allowSamePrecedence: allowSamePrecedence + }; +} + +/** + * Checks whether any group which includes both given operator exists or not. + * + * @param {Array.} groups - A list of groups to check. + * @param {string} left - An operator. + * @param {string} right - Another operator. + * @returns {boolean} `true` if such group existed. + */ +function includesBothInAGroup(groups, left, right) { + return groups.some(function(group) { + return group.indexOf(left) !== -1 && group.indexOf(right) !== -1; + }); +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "Disallow mixed binary operators.", + category: "Stylistic Issues", + recommended: false + }, + schema: [ + { + type: "object", + properties: { + groups: { + type: "array", + items: { + type: "array", + items: {enum: ALL_OPERATORS}, + minItems: 2, + uniqueItems: true + }, + uniqueItems: true + }, + allowSamePrecedence: { + type: "boolean" + } + }, + additionalProperties: false + } + ] + }, + + create: function(context) { + var sourceCode = context.getSourceCode(); + var options = normalizeOptions(context.options[0]); + + /** + * Checks whether a given node should be ignored by options or not. + * + * @param {ASTNode} node - A node to check. This is a BinaryExpression + * node or a LogicalExpression node. This parent node is one of + * them, too. + * @returns {boolean} `true` if the node should be ignored. + */ + function shouldIgnore(node) { + var a = node; + var b = node.parent; + + return ( + !includesBothInAGroup(options.groups, a.operator, b.operator) || + ( + options.allowSamePrecedence && + astUtils.getPrecedence(a) === astUtils.getPrecedence(b) + ) + ); + } + + /** + * Checks whether the operator of a given node is mixed with parent + * node's operator or not. + * + * @param {ASTNode} node - A node to check. This is a BinaryExpression + * node or a LogicalExpression node. This parent node is one of + * them, too. + * @returns {boolean} `true` if the node was mixed. + */ + function isMixedWithParent(node) { + return ( + node.operator !== node.parent.operator && + !astUtils.isParenthesised(sourceCode, node) + ); + } + + /** + * Gets the operator token of a given node. + * + * @param {ASTNode} node - A node to check. This is a BinaryExpression + * node or a LogicalExpression node. + * @returns {Token} The operator token of the node. + */ + function getOperatorToken(node) { + var token = sourceCode.getTokenAfter(node.left); + + while (token.value === ")") { + token = sourceCode.getTokenAfter(token); + } + + return token; + } + + /** + * Reports both the operator of a given node and the operator of the + * parent node. + * + * @param {ASTNode} node - A node to check. This is a BinaryExpression + * node or a LogicalExpression node. This parent node is one of + * them, too. + * @returns {void} + */ + function reportBothOperators(node) { + var parent = node.parent; + var left = (parent.left === node) ? node : parent; + var right = (parent.left !== node) ? node : parent; + var message = + "Unexpected mix of '" + left.operator + "' and '" + + right.operator + "'."; + + context.report({ + node: left, + loc: getOperatorToken(left).loc.start, + message: message + }); + context.report({ + node: right, + loc: getOperatorToken(right).loc.start, + message: message + }); + } + + /** + * Checks between the operator of this node and the operator of the + * parent node. + * + * @param {ASTNode} node - A node to check. + * @returns {void} + */ + function check(node) { + 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 new file mode 100644 index 00000000000..fe991ae13c2 --- /dev/null +++ b/tests/lib/rules/no-mixed-operators.js @@ -0,0 +1,115 @@ +/** + * @fileoverview Tests for no-mixed-operators rule. + * @author Toru Nagashima + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/no-mixed-operators"), + RuleTester = require("../../../lib/testers/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var ruleTester = new RuleTester(); + +ruleTester.run("no-mixed-operators", rule, { + valid: [ + "a && b && c && d", + "a || b || c || d", + "(a || b) && c && d", + "a || (b && c && d)", + "(a || b || c) && d", + "a || b || (c && d)", + "a + b + c + d", + "a * b * c * d", + "a == 0 && b == 1", + "a == 0 || b == 1", + { + code: "(a == 0) && (b == 1)", + options: [{groups: [["&&", "=="]]}] + }, + { + code: "a + b - c * d / e", + options: [{groups: [["&&", "||"]]}] + }, + "a + b - c", + "a * b / c", + { + code: "a + b - c", + options: [{allowSamePrecedence: true}] + }, + { + code: "a * b / c", + options: [{allowSamePrecedence: true}] + } + ], + invalid: [ + { + code: "a && b || c", + errors: [ + {column: 3, message: "Unexpected mix of '&&' and '||'."}, + {column: 8, message: "Unexpected mix of '&&' and '||'."} + ] + }, + { + code: "a && b > 0 || c", + options: [{groups: [["&&", "||", ">"]]}], + errors: [ + {column: 3, message: "Unexpected mix of '&&' and '||'."}, + {column: 3, message: "Unexpected mix of '&&' and '>'."}, + {column: 8, message: "Unexpected mix of '&&' and '>'."}, + {column: 12, message: "Unexpected mix of '&&' and '||'."} + ] + }, + { + code: "a && b > 0 || c", + options: [{groups: [["&&", "||"]]}], + errors: [ + {column: 3, message: "Unexpected mix of '&&' and '||'."}, + {column: 12, message: "Unexpected mix of '&&' and '||'."} + ] + }, + { + code: "a && b + c - d / e || f", + options: [{groups: [["&&", "||"], ["+", "-", "*", "/"]]}], + errors: [ + {column: 3, message: "Unexpected mix of '&&' and '||'."}, + {column: 12, message: "Unexpected mix of '-' and '/'."}, + {column: 16, message: "Unexpected mix of '-' and '/'."}, + {column: 20, message: "Unexpected mix of '&&' and '||'."} + ] + }, + { + code: "a && b + c - d / e || f", + options: [{groups: [["&&", "||"], ["+", "-", "*", "/"]], allowSamePrecedence: true}], + errors: [ + {column: 3, message: "Unexpected mix of '&&' and '||'."}, + {column: 12, message: "Unexpected mix of '-' and '/'."}, + {column: 16, message: "Unexpected mix of '-' and '/'."}, + {column: 20, message: "Unexpected mix of '&&' and '||'."} + ] + }, + { + code: "a + b - c", + options: [{allowSamePrecedence: false}], + errors: [ + {column: 3, message: "Unexpected mix of '+' and '-'."}, + {column: 7, message: "Unexpected mix of '+' and '-'."} + ] + }, + { + code: "a * b / c", + options: [{allowSamePrecedence: false}], + errors: [ + {column: 3, message: "Unexpected mix of '*' and '/'."}, + {column: 7, message: "Unexpected mix of '*' and '/'."} + ] + } + ] +});