Bug 1436575 - Prevent ==/!= comparisons with booleans as they aren't necessary. r=standard8
authorJared Wein <jwein@mozilla.com>
Thu, 08 Feb 2018 11:51:17 -0500
changeset 459002 7ee5aabe89ef6ea406157bec38c060fa91a9153b
parent 459001 088e727e5cf7591efaa542cfb256e83992388b13
child 459003 89f7592e2b4615819374e347aed69333bc3a8a72
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstandard8
bugs1436575
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1436575 - Prevent ==/!= comparisons with booleans as they aren't necessary. r=standard8 MozReview-Commit-ID: 25oVDVNzDyF
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-compare-against-boolean-literals.js
tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
tools/lint/eslint/eslint-plugin-mozilla/package.json
tools/lint/eslint/eslint-plugin-mozilla/tests/no-compare-against-boolean-literals.js
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -158,16 +158,22 @@ is never called.
 
 no-aArgs
 --------
 
 Checks that function argument names don't start with lowercase 'a' followed by
 a capital letter. This is to prevent the use of Hungarian notation whereby the
 first letter is a prefix that indicates the type or intended use of a variable.
 
+no-compare-against-boolean-literals
+-----------------------------------
+
+Checks that boolean expressions do not compare against literal values
+of `true` or `false`. This is to prevent overly verbose code such as
+`if (isEnabled == true)` when `if (isEnabled)` would suffice.
 
 no-cpows-in-tests
 -----------------
 
 This rule checks if the file is a browser mochitest and, if so, checks for
 possible CPOW usage by checking for the following strings:
 
 - ``gBrowser.contentWindow``
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -165,16 +165,17 @@ module.exports = {
     "max-depth": "off",
 
     // Maximum depth callbacks can be nested.
     "max-nested-callbacks": ["error", 10],
 
     "mozilla/avoid-removeChild": "error",
     "mozilla/import-browser-window-globals": "error",
     "mozilla/import-globals": "error",
+    "mozilla/no-compare-against-boolean-literals": "error",
     "mozilla/no-define-cc-etc": "error",
     "mozilla/no-import-into-var-and-global": "error",
     "mozilla/no-useless-parameters": "error",
     "mozilla/no-useless-removeEventListener": "error",
     "mozilla/use-cc-etc": "error",
     "mozilla/use-chromeutils-import": "error",
     "mozilla/use-default-preference-values": "error",
     "mozilla/use-includes-instead-of-indexOf": "error",
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -39,16 +39,17 @@ module.exports = {
     "import-content-task-globals":
       require("../lib/rules/import-content-task-globals"),
     "import-globals": require("../lib/rules/import-globals"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "mark-exported-symbols-as-used": require("../lib/rules/mark-exported-symbols-as-used"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "no-aArgs": require("../lib/rules/no-aArgs"),
     "no-arbitrary-setTimeout": require("../lib/rules/no-arbitrary-setTimeout"),
+    "no-compare-against-boolean-literals": require("../lib/rules/no-compare-against-boolean-literals"),
     "no-cpows-in-tests": require("../lib/rules/no-cpows-in-tests"),
     "no-define-cc-etc": require("../lib/rules/no-define-cc-etc"),
     "no-single-arg-cu-import": require("../lib/rules/no-single-arg-cu-import"),
     "no-import-into-var-and-global":
       require("../lib/rules/no-import-into-var-and-global.js"),
     "no-task": require("../lib/rules/no-task"),
     "no-useless-parameters": require("../lib/rules/no-useless-parameters"),
     "no-useless-removeEventListener":
copy from tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-includes-instead-of-indexOf.js
copy to tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-compare-against-boolean-literals.js
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-includes-instead-of-indexOf.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-compare-against-boolean-literals.js
@@ -1,10 +1,10 @@
 /**
- * @fileoverview Use .includes instead of .indexOf
+ * @fileoverview Restrict comparing against `true` or `false`.
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
 "use strict";
 
@@ -15,28 +15,39 @@
 module.exports = function(context) {
 
   // ---------------------------------------------------------------------------
   // Public
   //  --------------------------------------------------------------------------
 
   return {
     "BinaryExpression": function(node) {
-      if (node.left.type != "CallExpression" ||
-          node.left.callee.type != "MemberExpression" ||
-          node.left.callee.property.type != "Identifier" ||
-          node.left.callee.property.name != "indexOf") {
-        return;
-      }
-
-      if ((["!=", "!==", "==", "==="].includes(node.operator) &&
-           node.right.type == "UnaryExpression" &&
-           node.right.operator == "-" &&
-           node.right.argument.type == "Literal" &&
-           node.right.argument.value == 1) ||
-          ([">=", "<"].includes(node.operator) &&
-           node.right.type == "Literal" &&
-           node.right.value == 0)) {
-        context.report(node, "use .includes instead of .indexOf");
+      if (["==", "!="].includes(node.operator) &&
+          (["true", "false"].includes(node.left.raw) ||
+           ["true", "false"].includes(node.right.raw))) {
+        context.report({
+          node,
+          message: "Don't compare for inexact equality against boolean literals",
+          fix(fixer) {
+            let unaryOp = "!!";
+            if ((node.operator == "==" &&
+                 (node.left.raw == "false" ||
+                  node.right.raw == "false")) ||
+                (node.operator == "!=" &&
+                 (node.left.raw == "true" ||
+                  node.right.raw == "true"))) {
+              unaryOp = "!";
+            }
+            let fixings = [];
+            if (["true", "false"].includes(node.left)) {
+              fixings.push(fixer.removeRange([node.left.range[0], node.right.range[0]]));
+              fixings.push(fixer.insertTextBefore(node.right, unaryOp));
+            } else {
+              fixings.push(fixer.removeRange([node.left.range[1], node.right.range[1]]));
+              fixings.push(fixer.insertTextBefore(node.left, unaryOp));
+            }
+            return fixings;
+          }
+        });
       }
     }
   };
 };
--- a/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
@@ -1,11 +1,11 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.8.1",
+  "version": "0.8.2",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
     "acorn": {
       "version": "5.4.1",
       "resolved": "https://registry.npmjs.org/acorn/-/acorn-5.4.1.tgz",
       "integrity": "sha512-XLmq3H/BVvW6/GbxKryGxWORz1ebilSsUDlyC27bXhWGWAZWkGwS6FLHjOlwFXNFoWFQEO/Df4u0YYd0K3BQgQ==",
       "dev": true
--- a/tools/lint/eslint/eslint-plugin-mozilla/package.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package.json
@@ -1,11 +1,11 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.8.1",
+  "version": "0.8.2",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
copy from tools/lint/eslint/eslint-plugin-mozilla/tests/use-chromeutils-import.js
copy to tools/lint/eslint/eslint-plugin-mozilla/tests/no-compare-against-boolean-literals.js
--- a/tools/lint/eslint/eslint-plugin-mozilla/tests/use-chromeutils-import.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-compare-against-boolean-literals.js
@@ -2,79 +2,64 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // ------------------------------------------------------------------------------
 // Requirements
 // ------------------------------------------------------------------------------
 
-var rule = require("../lib/rules/use-chromeutils-import");
+var rule = require("../lib/rules/no-compare-against-boolean-literals");
 var RuleTester = require("eslint/lib/testers/rule-tester");
 
 const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
 
 // ------------------------------------------------------------------------------
 // Tests
 // ------------------------------------------------------------------------------
 
 function callError(message) {
-  return [{message, type: "CallExpression"}];
+  return [{message, type: "BinaryExpression"}];
 }
 
-const MESSAGE_IMPORT = "Please use ChromeUtils.import instead of Cu.import";
-const MESSAGE_DEFINE = ("Please use ChromeUtils.defineModuleGetter instead of " +
-                        "XPCOMUtils.defineLazyModuleGetter");
+const MESSAGE = "Don't compare for inexact equality against boolean literals";
 
-ruleTester.run("use-chromeutils-import", rule, {
+ruleTester.run("no-compare-against-boolean-literals", rule, {
   valid: [
-    `ChromeUtils.import("resource://gre/modules/Service.jsm");`,
-    `ChromeUtils.import("resource://gre/modules/Service.jsm", this);`,
-    `ChromeUtils.defineModuleGetter(this, "Services",
-                                    "resource://gre/modules/Service.jsm");`,
-    `XPCOMUtils.defineLazyModuleGetter(this, "Services",
-                                       "resource://gre/modules/Service.jsm",
-                                       "Foo");`,
-    `XPCOMUtils.defineLazyModuleGetter(this, "Services",
-                                       "resource://gre/modules/Service.jsm",
-                                       undefined, preServicesLambda);`,
-    {
-      options: [{allowCu: true}],
-      code: `Cu.import("resource://gre/modules/Service.jsm");`
-    }
+    `if (!foo) {}`,
+    `if (!!foo) {}`
   ],
   invalid: [
     {
-      code: `Cu.import("resource://gre/modules/Services.jsm");`,
-      output: `ChromeUtils.import("resource://gre/modules/Services.jsm");`,
-      errors: callError(MESSAGE_IMPORT)
+      code: `if (foo == true) {}`,
+      errors: callError(MESSAGE)
     },
     {
-      code: `Cu.import("resource://gre/modules/Services.jsm", this);`,
-      output: `ChromeUtils.import("resource://gre/modules/Services.jsm", this);`,
-      errors: callError(MESSAGE_IMPORT)
+      code: `if (foo != true) {}`,
+      errors: callError(MESSAGE)
     },
     {
-      code: `Components.utils.import("resource://gre/modules/Services.jsm");`,
-      output: `ChromeUtils.import("resource://gre/modules/Services.jsm");`,
-      errors: callError(MESSAGE_IMPORT)
+      code: `if (foo == false) {}`,
+      errors: callError(MESSAGE)
     },
     {
-      code: `Components.utils.import("resource://gre/modules/Services.jsm");`,
-      output: `ChromeUtils.import("resource://gre/modules/Services.jsm");`,
-      errors: callError(MESSAGE_IMPORT)
+      code: `if (foo != false) {}`,
+      errors: callError(MESSAGE)
+    },
+    {
+      code: `if (true == foo) {}`,
+      errors: callError(MESSAGE)
     },
     {
-      options: [{allowCu: true}],
-      code: `Components.utils.import("resource://gre/modules/Services.jsm", this);`,
-      output: `ChromeUtils.import("resource://gre/modules/Services.jsm", this);`,
-      errors: callError(MESSAGE_IMPORT)
+      code: `if (true != foo) {}`,
+      errors: callError(MESSAGE)
     },
     {
-      code: `XPCOMUtils.defineLazyModuleGetter(this, "Services",
-                                               "resource://gre/modules/Services.jsm");`,
-      output: `ChromeUtils.defineModuleGetter(this, "Services",
-                                               "resource://gre/modules/Services.jsm");`,
-      errors: callError(MESSAGE_DEFINE)
+      code: `if (false == foo) {}`,
+      errors: callError(MESSAGE)
+    },
+    {
+      code: `if (false != foo) {}`,
+      errors: callError(MESSAGE)
     }
   ]
 });