Bug 1230369 - Add ESLint rules to disallow defining Cc/Ci etc and to prefer the use of Cc/Ci rather than the Components.* equivalents. r=florian
authorMark Banner <standard8@mozilla.com>
Tue, 06 Feb 2018 22:40:12 +0000
changeset 455217 f58c1a70c026749c97d3df1bd9a22d2473c1c056
parent 455216 76c1582d760e117d1ce2befbc2af998c24760231
child 455218 02cab40d67e2cd099f1a9b93a1638e2915b2192c
push id8799
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 16:46:23 +0000
treeherdermozilla-beta@15334014dc67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1230369
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 1230369 - Add ESLint rules to disallow defining Cc/Ci etc and to prefer the use of Cc/Ci rather than the Components.* equivalents. r=florian MozReview-Commit-ID: 9eAgUO3iIJW
.eslintrc.js
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/configs/mochitest-test.js
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-define-cc-etc.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-cc-etc.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-define-cc-etc.js
tools/lint/eslint/eslint-plugin-mozilla/tests/use-cc-etc.js
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -49,10 +49,39 @@ module.exports = {
     "rules": {
       "mozilla/mark-exported-symbols-as-used": "error",
       "no-unused-vars": ["error", {
         "args": "none",
         "vars": "local",
         "varsIgnorePattern": "^Cc|Ci|Cu|Cr|EXPORTED_SYMBOLS"
       }]
     }
+  }, {
+    // XXX Bug 1433175. These directories are still being fixed, so turn off
+    // mozilla/use-cc-etc for now.
+    "files": [
+      "accessible/**",
+      "browser/**",
+      "devtools/**",
+      "dom/**",
+      "extensions/pref/**",
+      "mobile/android/**",
+      "security/manager/**",
+      "services/**",
+      "storage/test/**",
+      "testing/**",
+      "toolkit/**",
+      "xpcom/**",
+    ],
+    "rules": {
+      "mozilla/use-cc-etc": "off",
+    }
+  }, {
+    // XXX Bug 1436303. These directories are still being fixed, so turn off
+    // mozilla/no-cc-etc for now.
+    "files": [
+      "devtools/**"
+    ],
+    "rules": {
+      "mozilla/no-define-cc-etc": "off",
+    }
   }]
 };
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -181,16 +181,29 @@ possible CPOW usage by checking for the 
 Note: These are string matches so we will miss situations where the parent
 object is assigned to another variable e.g.:
 
 .. code-block:: js
 
     var b = gBrowser;
     b.content // Would not be detected as a CPOW.
 
+no-define-cc-etc
+----------------
+
+This disallows statements such as:
+
+.. code-block:: js
+
+   var Cc = Components.classes;
+   var Ci = Components.interfaces;
+   var {Ci: interfaces, Cc: classes, Cu: utils} = Components;
+
+These used to be necessary but have now been defined globally for all chrome
+contexts.
 
 no-single-arg-cu-import
 -----------------------
 
 Rejects calls to "Cu.import" that do not supply a second argument (meaning they
 add the exported properties into global scope).
 
 
@@ -251,16 +264,23 @@ Treats top-level assignments like ``this
 Note: These are string matches so we will miss situations where the parent
 object is assigned to another variable e.g.:
 
 .. code-block:: js
 
    var b = gBrowser;
    b.content // Would not be detected as a CPOW.
 
+use-cc-etc
+----------
+
+This requires using ``Cc`` rather than ``Components.classes``, and the same for
+``Components.interfaces``, ``Components.results`` and ``Components.utils``. This has
+a slight performance advantage by avoiding the use of the dot.
+
 use-chromeutils-import
 ----------------------
 
 Require use of ``ChromeUtils.import`` and ``ChromeUtils.defineModuleGetter``
 rather than ``Components.utils.import`` and
 ``XPCOMUtils.defineLazyModuleGetter``.
 
 use-default-preference-values
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/mochitest-test.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/mochitest-test.js
@@ -42,11 +42,14 @@ module.exports = {
   "plugins": [
     "mozilla"
   ],
 
   "rules": {
     "mozilla/import-content-task-globals": "error",
     "mozilla/import-headjs-globals": "error",
     "mozilla/mark-test-function-used": "error",
+    // Turn off no-define-cc-etc for mochitests as these don't have Cc etc defined in the
+    // global scope.
+    "mozilla/no-define-cc-etc": "off",
     "no-shadow": "error"
   }
 };
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -165,19 +165,21 @@ 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-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",
     "mozilla/use-ownerGlobal": "error",
     "mozilla/use-services": "error",
 
     // Always require parenthesis for new calls
     // "new-parens": "error",
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -40,56 +40,31 @@ module.exports = {
       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-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":
       require("../lib/rules/no-useless-removeEventListener"),
     "no-useless-run-test":
       require("../lib/rules/no-useless-run-test"),
     "reject-importGlobalProperties":
       require("../lib/rules/reject-importGlobalProperties"),
     "reject-some-requires": require("../lib/rules/reject-some-requires"),
+    "use-cc-etc": require("../lib/rules/use-cc-etc"),
     "use-chromeutils-import": require("../lib/rules/use-chromeutils-import"),
     "use-default-preference-values":
       require("../lib/rules/use-default-preference-values"),
     "use-ownerGlobal": require("../lib/rules/use-ownerGlobal"),
     "use-includes-instead-of-indexOf": require("../lib/rules/use-includes-instead-of-indexOf"),
     "use-services": require("../lib/rules/use-services"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
-  },
-  rulesConfig: {
-    "avoid-Date-timing": "off",
-    "avoid-removeChild": "off",
-    "balanced-listeners": "off",
-    "import-browser-window-globals": "off",
-    "import-content-task-globals": "off",
-    "import-globals": "off",
-    "import-headjs-globals": "off",
-    "mark-test-function-used": "off",
-    "no-aArgs": "off",
-    "no-arbitrary-setTimeout": "off",
-    "no-cpows-in-tests": "off",
-    "no-single-arg-cu-import": "off",
-    "no-import-into-var-and-global": "off",
-    "no-task": "off",
-    "no-useless-parameters": "off",
-    "no-useless-run-test": "off",
-    "no-useless-removeEventListener": "off",
-    "reject-importGlobalProperties": "off",
-    "reject-some-requires": "off",
-    "use-chromeutils-import": "off",
-    "use-default-preference-values": "off",
-    "use-ownerGlobal": "off",
-    "use-includes-instead-of-indexOf": "off",
-    "use-services": "off",
-    "var-only-at-top-level": "off"
   }
 };
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-define-cc-etc.js
@@ -0,0 +1,40 @@
+/**
+ * @fileoverview Reject defining Cc/Ci/Cr/Cu.
+ *
+ * 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";
+
+// -----------------------------------------------------------------------------
+// Rule Definition
+// -----------------------------------------------------------------------------
+
+const componentsBlacklist = ["Cc", "Ci", "Cr", "Cu"];
+
+module.exports = function(context) {
+
+  // ---------------------------------------------------------------------------
+  // Public
+  //  --------------------------------------------------------------------------
+
+  return {
+    "VariableDeclarator": function(node) {
+      if (node.id.type == "Identifier" && componentsBlacklist.includes(node.id.name)) {
+        context.report(node,
+          `${node.id.name} is now defined in global scope, a separate definition is no longer necessary.`);
+      }
+
+      if (node.id.type == "ObjectPattern") {
+        for (let property of node.id.properties) {
+          if (componentsBlacklist.includes(property.key.name)) {
+            context.report(node,
+              `${property.key.name} is now defined in global scope, a separate definition is no longer necessary.`);
+          }
+        }
+      }
+    }
+  };
+};
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-cc-etc.js
@@ -0,0 +1,39 @@
+/**
+ * @fileoverview Reject use of Components.classes etc, prefer the shorthand instead.
+ *
+ * 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";
+
+// -----------------------------------------------------------------------------
+// Rule Definition
+// -----------------------------------------------------------------------------
+
+const componentsMap = {
+  classes: "Cc",
+  interfaces: "Ci",
+  results: "Cr",
+  utils: "Cu"
+};
+
+module.exports = function(context) {
+
+  // ---------------------------------------------------------------------------
+  // Public
+  //  --------------------------------------------------------------------------
+
+  return {
+    "MemberExpression": function(node) {
+      if (node.object.type === "Identifier" &&
+          node.object.name === "Components" &&
+          node.property.type === "Identifier" &&
+          Object.getOwnPropertyNames(componentsMap).includes(node.property.name)) {
+        context.report(node,
+          `Use ${componentsMap[node.property.name]} rather than Components.${node.property.name}`);
+      }
+    }
+  };
+};
--- 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.7.0",
+  "version": "0.8.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
     "acorn": {
       "version": "5.2.1",
       "resolved": "https://registry.npmjs.org/acorn/-/acorn-5.2.1.tgz",
       "integrity": "sha512-jG0u7c4Ly+3QkkW18V+NRDN+4bWHdln30NL1ZL2AvFZZmQe/BfopYCtghCKKVBUSetZ4QKcyA0pY6/4Gw8Pv8w==",
       "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.7.0",
+  "version": "0.8.0",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-define-cc-etc.js
@@ -0,0 +1,54 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-define-cc-etc");
+var RuleTester = require("eslint/lib/testers/rule-tester");
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, varName) {
+  return {code, errors: [{
+    message: `${varName} is now defined in global scope, a separate definition is no longer necessary.`,
+    type: "VariableDeclarator"
+  }]};
+}
+
+ruleTester.run("no-define-cc-etc", rule, {
+  valid: [
+    "var Cm = Components.manager;",
+    "const CC = Components.Constructor;",
+    "var {CC: Constructor, Cm: manager} = Components;",
+    "const {CC: Constructor, Cm: manager} = Components;",
+    "foo.Cc.test();"
+  ],
+  invalid: [
+    invalidCode("var Cc;", "Cc"),
+    invalidCode("let Cc;", "Cc"),
+    invalidCode("let Ci;", "Ci"),
+    invalidCode("let Cr;", "Cr"),
+    invalidCode("let Cu;", "Cu"),
+    invalidCode("var Cc = Components.classes;", "Cc"),
+    invalidCode("const {Cc} = Components;", "Cc"),
+    invalidCode("let {Cc, Cm} = Components", "Cc"),
+    invalidCode("const Cu = Components.utils;", "Cu"), {
+      code: "var {Ci: interfaces, Cc: classes} = Components;",
+      errors: [{
+        message: `Ci is now defined in global scope, a separate definition is no longer necessary.`,
+        type: "VariableDeclarator"
+      }, {
+        message: `Cc is now defined in global scope, a separate definition is no longer necessary.`,
+        type: "VariableDeclarator"
+      }]
+    }
+  ]
+});
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/use-cc-etc.js
@@ -0,0 +1,37 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/use-cc-etc");
+var RuleTester = require("eslint/lib/testers/rule-tester");
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, originalName, newName) {
+  return {code, errors: [{
+    message: `Use ${newName} rather than ${originalName}`,
+    type: "MemberExpression"
+  }]};
+}
+
+ruleTester.run("use-cc-etc", rule, {
+  valid: [
+    "Components.Constructor();",
+    "let x = Components.foo;"
+  ],
+  invalid: [
+    invalidCode("let foo = Components.classes['bar'];", "Components.classes", "Cc"),
+    invalidCode("let bar = Components.interfaces.bar;", "Components.interfaces", "Ci"),
+    invalidCode("Components.results.NS_ERROR_ILLEGAL_INPUT;", "Components.results", "Cr"),
+    invalidCode("Components.utils.reportError('fake');", "Components.utils", "Cu")
+  ]
+});