Update: enforceForOrderingRelations no-unsafe-negation (fixes #12163) (#12414)

samrae7 · platinumazure · commit 7c8bbe039194 · 2019-10-21T10:17:13.000-05:00
* Update: add option to no-unsafe-negation (fixes #12163) * Chore: undo autoformatting + rename function * Update: code review changes - typos - clarity of docs - extra test cases * Fix: fix failing tests * Update: code review changes - add backticks * Chore: remove comma
diff --git 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,38 @@ 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
+
+With this option set to `true` the rule is additionally enforced for:
+
+- `<` 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-unsafe-negation: ["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
@@ -16,14 +16,23 @@ const astUtils = require("./utils/ast-utils");
 //------------------------------------------------------------------------------
 
 /**
- * Checks whether the given operator is a relational operator or not.
+ * Checks whether the given operator is `in` or `instanceof`
  * @param {string} op The operator type to check.
- * @returns {boolean} `true` if the operator is a relational operator.
+ * @returns {boolean} `true` if the operator is `in` or `instanceof`
  */
-function isRelationalOperator(op) {
+function isInOrInstanceOfOperator(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.
@@ -48,7 +57,18 @@ module.exports = {
             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."
@@ -57,10 +77,15 @@ module.exports = {
 
     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 (
+                    (isInOrInstanceOfOperator(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
@@ -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,37 @@ ruleTester.run("no-unsafe-negation", rule, {
         "a instanceof b",
         "a instanceof b === false",
         "!(a instanceof b)",
-        "(!a) instanceof b"
+        "(!a) instanceof b",
+
+        // tests cases for enforceForOrderingRelations option:
+        "if (! a < b) {}",
+        "while (! a > b) {}",
+        "foo = ! a <= b;",
+        "foo = ! a >= b;",
+        {
+            code: "! a <= b",
+            options: [{}]
+        },
+        {
+            code: "foo = ! a >= b;",
+            options: [{ enforceForOrderingRelations: false }]
+        },
+        {
+            code: "foo = (!a) >= b;",
+            options: [{ enforceForOrderingRelations: true }]
+        },
+        {
+            code: "a <= b",
+            options: [{ enforceForOrderingRelations: true }]
+        },
+        {
+            code: "!(a < b)",
+            options: [{ enforceForOrderingRelations: true }]
+        },
+        {
+            code: "foo = a > b;",
+            options: [{ enforceForOrderingRelations: true }]
+        }
     ],
     invalid: [
         {
@@ -55,6 +104,31 @@ 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]
+        },
+        {
+            code: "! a <= b",
+            options: [{ enforceForOrderingRelations: true }],
+            errors: [unexpectedLessThanOrEqualOperatorError]
         }
     ]
 });
