Skip to content

Commit

Permalink
Update: Add suggestions for no-unsafe-negation (fixes #12591) (#12609)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdjermanovic @btmills
mdjermanovic authored and btmills committed Dec 20, 2019
1 parent d3e43f1 commit 05f7dd5
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 37 deletions.
35 changes: 30 additions & 5 deletions lib/rules/no-unsafe-negation.js
View file
Expand Up @@ -54,7 +54,8 @@ module.exports = {
description: "disallow negating the left operand of relational operators",
category: "Possible Errors",
recommended: true,
url: "https://eslint.org/docs/rules/no-unsafe-negation"
url: "https://eslint.org/docs/rules/no-unsafe-negation",
suggestion: true
},

schema: [
Expand All @@ -69,9 +70,13 @@ module.exports = {
additionalProperties: false
}
],

fixable: null,

messages: {
unexpected: "Unexpected negating the left operand of '{{operator}}' operator."
unexpected: "Unexpected negating the left operand of '{{operator}}' operator.",
suggestNegatedExpression: "Negate '{{operator}}' expression instead of its left operand. This changes the current behavior.",
suggestParenthesisedNegation: "Wrap negation in '()' to make the intention explicit. This preserves the current behavior."
}
},

Expand All @@ -82,18 +87,38 @@ module.exports = {

return {
BinaryExpression(node) {
const orderingRelationRuleApplies = enforceForOrderingRelations && isOrderingRelationalOperator(node.operator);
const operator = node.operator;
const orderingRelationRuleApplies = enforceForOrderingRelations && isOrderingRelationalOperator(operator);

if (
(isInOrInstanceOfOperator(node.operator) || orderingRelationRuleApplies) &&
(isInOrInstanceOfOperator(operator) || orderingRelationRuleApplies) &&
isNegation(node.left) &&
!astUtils.isParenthesised(sourceCode, node.left)
) {
context.report({
node,
loc: node.left.loc,
messageId: "unexpected",
data: { operator: node.operator }
data: { operator },
suggest: [
{
messageId: "suggestNegatedExpression",
data: { operator },
fix(fixer) {
const negationToken = sourceCode.getFirstToken(node.left);
const fixRange = [negationToken.range[1], node.range[1]];
const text = sourceCode.text.slice(fixRange[0], fixRange[1]);

return fixer.replaceTextRange(fixRange, `(${text})`);
}
},
{
messageId: "suggestParenthesisedNegation",
fix(fixer) {
return fixer.replaceText(node.left, `(${sourceCode.getText(node.left)})`);
}
}
]
});
}
}
Expand Down
185 changes: 153 additions & 32 deletions tests/lib/rules/no-unsafe-negation.js
View file
Expand Up @@ -17,27 +17,6 @@ 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 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 Down Expand Up @@ -83,52 +62,194 @@ ruleTester.run("no-unsafe-negation", rule, {
invalid: [
{
code: "!a in b",
errors: [unexpectedInError]
errors: [{
message: "Unexpected negating the left operand of 'in' operator.",
suggestions: [
{
desc: "Negate 'in' expression instead of its left operand. This changes the current behavior.",
output: "!(a in b)"
},
{
desc: "Wrap negation in '()' to make the intention explicit. This preserves the current behavior.",
output: "(!a) in b"
}
]
}]
},
{
code: "(!a in b)",
errors: [unexpectedInError]
errors: [{
messageId: "unexpected",
data: { operator: "in" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "(!(a in b))"
},
{
messageId: "suggestParenthesisedNegation",
output: "((!a) in b)"
}
]
}]
},
{
code: "!(a) in b",
errors: [unexpectedInError]
errors: [{
messageId: "unexpected",
data: { operator: "in" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "!((a) in b)"
},
{
messageId: "suggestParenthesisedNegation",
output: "(!(a)) in b"
}
]
}]
},
{
code: "!a instanceof b",
errors: [unexpectedInstanceofError]
errors: [{
messageId: "unexpected",
data: { operator: "instanceof" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "!(a instanceof b)"
},
{
messageId: "suggestParenthesisedNegation",
output: "(!a) instanceof b"
}
]
}]
},
{
code: "(!a instanceof b)",
errors: [unexpectedInstanceofError]
errors: [{
messageId: "unexpected",
data: { operator: "instanceof" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "(!(a instanceof b))"
},
{
messageId: "suggestParenthesisedNegation",
output: "((!a) instanceof b)"
}
]
}]
},
{
code: "!(a) instanceof b",
errors: [unexpectedInstanceofError]
errors: [{
messageId: "unexpected",
data: { operator: "instanceof" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "!((a) instanceof b)"
},
{
messageId: "suggestParenthesisedNegation",
output: "(!(a)) instanceof b"
}
]
}]
},
{
code: "if (! a < b) {}",
options: [{ enforceForOrderingRelations: true }],
errors: [unexpectedLessThanOperatorError]
errors: [{
messageId: "unexpected",
data: { operator: "<" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "if (!( a < b)) {}"
},
{
messageId: "suggestParenthesisedNegation",
output: "if ((! a) < b) {}"
}
]
}]
},
{
code: "while (! a > b) {}",
options: [{ enforceForOrderingRelations: true }],
errors: [unexpectedMoreThanOperatorError]
errors: [{
messageId: "unexpected",
data: { operator: ">" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "while (!( a > b)) {}"
},
{
messageId: "suggestParenthesisedNegation",
output: "while ((! a) > b) {}"
}
]
}]
},
{
code: "foo = ! a <= b;",
options: [{ enforceForOrderingRelations: true }],
errors: [unexpectedLessThanOrEqualOperatorError]
errors: [{
messageId: "unexpected",
data: { operator: "<=" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "foo = !( a <= b);"
},
{
messageId: "suggestParenthesisedNegation",
output: "foo = (! a) <= b;"
}
]
}]
},
{
code: "foo = ! a >= b;",
options: [{ enforceForOrderingRelations: true }],
errors: [unexpectedMoreThanOrEqualOperatorError]
errors: [{
messageId: "unexpected",
data: { operator: ">=" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "foo = !( a >= b);"
},
{
messageId: "suggestParenthesisedNegation",
output: "foo = (! a) >= b;"
}
]
}]
},
{
code: "! a <= b",
options: [{ enforceForOrderingRelations: true }],
errors: [unexpectedLessThanOrEqualOperatorError]
errors: [{
messageId: "unexpected",
data: { operator: "<=" },
suggestions: [
{
messageId: "suggestNegatedExpression",
output: "!( a <= b)"
},
{
messageId: "suggestParenthesisedNegation",
output: "(! a) <= b"
}
]
}]
}
]
});

0 comments on commit 05f7dd5

Please sign in to comment.