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.

Sign up for GitHub

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

Jump to bottom

Update: Add suggestions for no-unsafe-negation (fixes #12591) #12609

Merged
merged 1 commit into from Dec 20, 2019
Merged

Update: Add suggestions for no-unsafe-negation (fixes #12591) #12609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 30 additions & 5 deletions lib/rules/no-unsafe-negation.js
View file
Open in desktop
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
Open in desktop
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"
}
]
}]
}
]
});