add rule no-inverted-boolean-check (#74)

Author: andrea-guarino-sonarsource

README.md

@@ -25,6 +25,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 * Cognitive Complexity of functions should not be too high ([`cognitive-complexity`])
 * Two branches in a conditional structure should not have exactly the same implementation ([`no-duplicated-branches`])
 * Functions should not have identical implementations ([`no-identical-functions`])
+* Boolean checks should not be inverted ([`no-inverted-boolean-check`])
 * Boolean literals should not be redundant ([`no-redundant-boolean`])
 * "switch" statements should have at least 3 "case" clauses ([`no-small-switch`])
 * Local variables should not be declared and then immediately returned or thrown ([`prefer-immediate-return`]) (:wrench: *fixable*)
@@ -39,6 +40,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 [`no-identical-conditions`]: ./docs/rules/no-identical-conditions.md
 [`no-identical-expressions`]: ./docs/rules/no-identical-expressions.md
 [`no-identical-functions`]: ./docs/rules/no-identical-functions.md
+[`no-inverted-boolean-check`]: ./docs/rules/no-inverted-boolean-check.md
 [`no-one-iteration-loop`]: ./docs/rules/no-one-iteration-loop.md
 [`no-redundant-boolean`]: ./docs/rules/no-redundant-boolean.md
 [`no-small-switch`]: ./docs/rules/no-small-switch.md

docs/rules/no-inverted-boolean-check.md

@@ -0,0 +1,15 @@
+# no-inverted-boolean-check
+
+It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.
+
+## Noncompliant Code Example
+
+```javascript
+if (!(a === 2)) { ... }  // Noncompliant
+```
+
+## Compliant Solution
+
+```javascript
+if (a !== 2) { ... }
+```

ruling/snapshots/no-inverted-boolean-check

@@ -0,0 +1,5 @@
+src/angular.js/src/ng/sniffer.js: 61
+src/jest/packages/expect/src/matchers.js: 523
+src/react-native/Libraries/Renderer/ReactFabric-dev.js: 1447,5567,5796,8656,8743
+src/react-native/Libraries/Renderer/ReactNativeRenderer-dev.js: 1602,5903,6132,8992,9079
+src/three.js/src/math/Interpolant.js: 57,95

src/index.ts

@@ -28,6 +28,7 @@ const sonarjsRules: [string, Linter.RuleLevel][] = [
   ["no-identical-conditions", "error"],
   ["no-identical-functions", "error"],
   ["no-identical-expressions", "error"],
+  ["no-inverted-boolean-check", "error"],
   ["no-one-iteration-loop", "error"],
   ["no-redundant-boolean", "error"],
   ["no-small-switch", "error"],

src/rules/no-inverted-boolean-check.ts

@@ -0,0 +1,62 @@
+/*
+ * eslint-plugin-sonarjs
+ * Copyright (C) 2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+// https://jira.sonarsource.com/browse/RSPEC-1940
+
+import { Rule } from "eslint";
+import { Node, UnaryExpression } from "estree";
+import { isBinaryExpression } from "../utils/nodes";
+
+const MESSAGE = "Use the opposite operator ({{invertedOperator}}) instead.";
+
+const invertedOperators: { [operator: string]: string } = {
+  "==": "!=",
+  "!=": "==",
+  "===": "!==",
+  "!==": "===",
+  ">": "<=",
+  "<": ">=",
+  ">=": "<",
+  "<=": ">",
+};
+
+const rule: Rule.RuleModule = {
+  create(context: Rule.RuleContext) {
+    return { UnaryExpression: (node: Node) => visitUnaryExpression(node as UnaryExpression, context) };
+  },
+};
+
+function visitUnaryExpression(unaryExpression: UnaryExpression, context: Rule.RuleContext) {
+  if (unaryExpression.operator === "!" && isBinaryExpression(unaryExpression.argument)) {
+    const condition = unaryExpression.argument;
+    const invertedOperator = invertedOperators[condition.operator];
+    if (invertedOperator) {
+      const left = context.getSourceCode().getText(condition.left);
+      const right = context.getSourceCode().getText(condition.right);
+      context.report({
+        message: MESSAGE,
+        data: { invertedOperator },
+        node: unaryExpression,
+        fix: fixer => fixer.replaceText(unaryExpression, `${left} ${invertedOperator} ${right}`),
+      });
+    }
+  }
+}
+
+export = rule;

src/utils/nodes.ts

@@ -33,6 +33,10 @@ export function isAssignmentExpression(node: estree.Node | undefined): node is e
   return node !== undefined && node.type === "AssignmentExpression";
 }
 
+export function isBinaryExpression(node: estree.Node | undefined): node is estree.BinaryExpression {
+  return node !== undefined && node.type === "BinaryExpression";
+}
+
 export function isBlockStatement(node: estree.Node | undefined): node is estree.BlockStatement {
   return node !== undefined && node.type === "BlockStatement";
 }

tests/rules/no-inverted-boolean-check.test.ts

@@ -0,0 +1,122 @@
+/*
+ * eslint-plugin-sonarjs
+ * Copyright (C) 2018 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+import { RuleTester } from "eslint";
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
+import rule = require("../../src/rules/no-inverted-boolean-check");
+
+ruleTester.run("no-inverted-boolean-check", rule, {
+  valid: [
+    {
+      code: `if (!x) {}`,
+    },
+    {
+      code: `if (x == 1) {}`,
+    },
+    {
+      code: `if (!(x + 1)) {}`,
+    },
+    {
+      code: `if (+(x == 1)) {}`,
+    },
+    {
+      code: `!x ? 2 : 3`,
+    },
+  ],
+  invalid: [
+    // `==` => `!=`
+    {
+      code: `if (!(x == 1)) {}`,
+      errors: [
+        {
+          message: message("!="),
+          line: 1,
+          endLine: 1,
+          column: 5,
+          endColumn: 14,
+        },
+      ],
+      output: `if (x != 1) {}`,
+    },
+    // `!=` => `==`
+    {
+      code: `if (!(x != 1)) {}`,
+      errors: [message("==")],
+      output: `if (x == 1) {}`,
+    },
+    // `===` => `!==`
+    {
+      code: `if (!(x === 1)) {}`,
+      errors: [message("!==")],
+      output: `if (x !== 1) {}`,
+    },
+    // `!==` => `===`
+    {
+      code: `if (!(x !== 1)) {}`,
+      errors: [message("===")],
+      output: `if (x === 1) {}`,
+    },
+    // `>` => `<=`
+    {
+      code: `if (!(x > 1)) {}`,
+      errors: [message("<=")],
+      output: `if (x <= 1) {}`,
+    },
+    // `<` => `>=`
+    {
+      code: `if (!(x < 1)) {}`,
+      errors: [message(">=")],
+      output: `if (x >= 1) {}`,
+    },
+    // `>=` => `<`
+    {
+      code: `if (!(x >= 1)) {}`,
+      errors: [message("<")],
+      output: `if (x < 1) {}`,
+    },
+    // `<=` => `>`
+    {
+      code: `if (!(x <= 1)) {}`,
+      errors: [message(">")],
+      output: `if (x > 1) {}`,
+    },
+    // ternary operator
+    {
+      code: `!(x != 1) ? 1 : 2`,
+      errors: [message("==")],
+      output: `x == 1 ? 1 : 2`,
+    },
+    // not conditional
+    {
+      code: `foo(!(x === 1))`,
+      errors: [message("!==")],
+      output: `foo(x !== 1)`,
+    },
+    {
+      code: `let foo = !(x <= 4)`,
+      errors: [message(">")],
+      output: `let foo = x > 4`,
+    },
+  ],
+});
+
+function message(oppositeOperator: string) {
+  return `Use the opposite operator (${oppositeOperator}) instead.`;
+}