Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update: enforceForOrderingRelations no-unsafe-negation (fixes #12163) (
…#12414)

* 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
  • Loading branch information
samrae7 authored and platinumazure committed Oct 21, 2019
1 parent 349ed67 commit 7c8bbe0
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 12 deletions.
40 changes: 35 additions & 5 deletions docs/rules/no-unsafe-negation.md
Expand Up @@ -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:

Expand Down Expand Up @@ -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.
35 changes: 30 additions & 5 deletions lib/rules/no-unsafe-negation.js
Expand Up @@ -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.
Expand All @@ -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."
Expand All @@ -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)
) {
Expand Down
78 changes: 76 additions & 2 deletions tests/lib/rules/no-unsafe-negation.js
Expand Up @@ -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: [
Expand All @@ -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: [
{
Expand All @@ -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]
}
]
});

0 comments on commit 7c8bbe0

Please sign in to comment.