From a6d5e30884f61b1c9763c259f5f375d21c546829 Mon Sep 17 00:00:00 2001 From: Sam Rae Date: Sun, 13 Oct 2019 08:16:18 +0100 Subject: [PATCH] Update: add enforceForOrderingRelations option to no-unsafe-negation (addresses #12163) --- docs/rules/no-unsafe-negation.md | 44 ++++++++++++++++++++--- lib/rules/no-unsafe-negation.js | 35 +++++++++++++++--- tests/lib/rules/no-unsafe-negation.js | 52 +++++++++++++++++++++++++-- 3 files changed, 120 insertions(+), 11 deletions(-) diff --git a/docs/rules/no-unsafe-negation.md b/docs/rules/no-unsafe-negation.md index 880e9f64481e..5f28236a0a83 100644 --- a/docs/rules/no-unsafe-negation.md +++ b/docs/rules/no-unsafe-negation.md @@ -4,12 +4,10 @@ Just as developers might type `-a + b` when they mean `-(a + b)` for the negativ ## Rule Details -This rule disallows negating the left operand of [Relational Operators](https://developer.mozilla.org/en/docs/Web/JavaScript/Guide/Expressions_and_Operators#Relational_operators). +This rule disallows negating the left operand of the following relational operators: -Relational Operators are: - -- `in` operator. -- `instanceof` operator. +- [`in` operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in). +- [`instanceof` operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof). Examples of **incorrect** code for this rule: @@ -72,6 +70,42 @@ if (!(foo) in object) { } ``` +## Options + +This rule has an object option: + +- `"enforceForOrderingRelations": false` (default) allows negation of the left-hand side of ordering relational operators (`<`, `>`, `<=`, `>=`) +- `"enforceForOrderingRelations": true` disallows negation of the left-hand side of ordering relational operators + +### enforceForOrderingRelations + +The enforceForOrderingRelations option extends the rule to apply to all [Relational Operators](https://www.ecma-international.org/ecma-262/10.0/index.html#sec-relational-operators), not just `in` and `instanceof`. + +With this option set to true, the rule is enforced for all of the following: + +- `in` operator. +- `instanceof` operator. +- `<` operator. +- `>` operator. +- `<=` operator. +- `>=` operator. + +The purpose is to avoid expressions such as `! a < b` (which is equivalent to `(a ? 0 : 1) < b`) when what is really intended is `!(a < b)`. + +Examples of additional **incorrect** code for this rule with the `{ "enforceForOrderingRelations": true }` option: + +```js +/*eslint no-unneeded-ternary: ["error", { "enforceForOrderingRelations": true }]*/ + +if (! a < b) {} + +while (! a > b) {} + +foo = ! a <= b; + +foo = ! a >= b; +``` + ## When Not To Use It If you don't want to notify unsafe logical negations, then it's safe to disable this rule. diff --git a/lib/rules/no-unsafe-negation.js b/lib/rules/no-unsafe-negation.js index 5226b367dee3..3d7632708904 100644 --- a/lib/rules/no-unsafe-negation.js +++ b/lib/rules/no-unsafe-negation.js @@ -24,6 +24,15 @@ function isRelationalOperator(op) { return op === "in" || op === "instanceof"; } +/** + * Checks whether the given operator is an ordering relational operator or not. + * @param {string} op The operator type to check. + * @returns {boolean} `true` if the operator is an ordering relational operator. + */ +function isOrderingRelationalOperator(op) { + return op === "<" || op === ">" || op === ">=" || op === "<="; +} + /** * Checks whether the given node is a logical negation expression or not. * @param {ASTNode} node The node to check. @@ -42,25 +51,43 @@ module.exports = { type: "problem", docs: { - description: "disallow negating the left operand of relational operators", + description: + "disallow negating the left operand of relational operators", category: "Possible Errors", recommended: true, url: "https://eslint.org/docs/rules/no-unsafe-negation" }, - schema: [], + schema: [ + { + type: "object", + properties: { + enforceForOrderingRelations: { + type: "boolean", + default: false + } + }, + additionalProperties: false + } + ], fixable: null, messages: { - unexpected: "Unexpected negating the left operand of '{{operator}}' operator." + unexpected: + "Unexpected negating the left operand of '{{operator}}' operator." } }, create(context) { const sourceCode = context.getSourceCode(); + const options = context.options[0] || {}; + const enforceForOrderingRelations = options.enforceForOrderingRelations === true; return { BinaryExpression(node) { - if (isRelationalOperator(node.operator) && + const orderingRelationRuleApplies = enforceForOrderingRelations && isOrderingRelationalOperator(node.operator); + + if ( + (isRelationalOperator(node.operator) || orderingRelationRuleApplies) && isNegation(node.left) && !astUtils.isParenthesised(sourceCode, node.left) ) { diff --git a/tests/lib/rules/no-unsafe-negation.js b/tests/lib/rules/no-unsafe-negation.js index 6ac24541248f..59ab2f30cad1 100644 --- a/tests/lib/rules/no-unsafe-negation.js +++ b/tests/lib/rules/no-unsafe-negation.js @@ -18,7 +18,26 @@ const rule = require("../../../lib/rules/no-unsafe-negation"), const ruleTester = new RuleTester(); const unexpectedInError = { messageId: "unexpected", data: { operator: "in" } }; -const unexpectedInstanceofError = { messageId: "unexpected", data: { operator: "instanceof" } }; +const unexpectedInstanceofError = { + messageId: "unexpected", + data: { operator: "instanceof" } +}; +const unexpectedLessThanOperatorError = { + messageId: "unexpected", + data: { operator: "<" } +}; +const unexpectedMoreThanOperatorError = { + messageId: "unexpected", + data: { operator: ">" } +}; +const unexpectedMoreThanOrEqualOperatorError = { + messageId: "unexpected", + data: { operator: ">=" } +}; +const unexpectedLessThanOrEqualOperatorError = { + messageId: "unexpected", + data: { operator: "<=" } +}; ruleTester.run("no-unsafe-negation", rule, { valid: [ @@ -29,7 +48,16 @@ ruleTester.run("no-unsafe-negation", rule, { "a instanceof b", "a instanceof b === false", "!(a instanceof b)", - "(!a) instanceof b" + "(!a) instanceof b", + "if (! a < b) {}", + "while (! a > b) {}", + "foo = ! a <= b;", + "foo = ! a >= b;", + { + code: "foo = (!a) >= b;", + options: [{ enforceForOrderingRelations: true }], + errors: [unexpectedMoreThanOrEqualOperatorError] + } ], invalid: [ { @@ -55,6 +83,26 @@ ruleTester.run("no-unsafe-negation", rule, { { code: "!(a) instanceof b", errors: [unexpectedInstanceofError] + }, + { + code: "if (! a < b) {}", + options: [{ enforceForOrderingRelations: true }], + errors: [unexpectedLessThanOperatorError] + }, + { + code: "while (! a > b) {}", + options: [{ enforceForOrderingRelations: true }], + errors: [unexpectedMoreThanOperatorError] + }, + { + code: "foo = ! a <= b;", + options: [{ enforceForOrderingRelations: true }], + errors: [unexpectedLessThanOrEqualOperatorError] + }, + { + code: "foo = ! a >= b;", + options: [{ enforceForOrderingRelations: true }], + errors: [unexpectedMoreThanOrEqualOperatorError] } ] });