Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from Oct 21, 2019
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) {}",
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
"while (! a > b) {}",
"foo = ! a <= b;",
"foo = ! a >= b;",
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
{
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]
}
]
});