From 05f7dd53ed91a6e3be9eb40825fb6d2207f82209 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 20 Dec 2019 21:25:10 +0100 Subject: [PATCH] Update: Add suggestions for no-unsafe-negation (fixes #12591) (#12609) --- lib/rules/no-unsafe-negation.js | 35 ++++- tests/lib/rules/no-unsafe-negation.js | 185 +++++++++++++++++++++----- 2 files changed, 183 insertions(+), 37 deletions(-) diff --git a/lib/rules/no-unsafe-negation.js b/lib/rules/no-unsafe-negation.js index 5c8f569d7b8..a9c2ee74f2c 100644 --- a/lib/rules/no-unsafe-negation.js +++ b/lib/rules/no-unsafe-negation.js @@ -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: [ @@ -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." } }, @@ -82,10 +87,11 @@ 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) ) { @@ -93,7 +99,26 @@ module.exports = { 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)})`); + } + } + ] }); } } diff --git a/tests/lib/rules/no-unsafe-negation.js b/tests/lib/rules/no-unsafe-negation.js index 9493bbb7295..23e6cd5ceb5 100644 --- a/tests/lib/rules/no-unsafe-negation.js +++ b/tests/lib/rules/no-unsafe-negation.js @@ -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: [ @@ -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" + } + ] + }] } ] });