Bug 1646183 - Extend ESLint rule reject-importGlobalProperties to also handle defineLazyGlobalGetters. r=arai,webdriver-reviewers,webcompat-reviewers,extension-reviewers,whimboo,twisniewski,kmag

Differential Revision: https://phabricator.services.mozilla.com/D150353

browser/components/migration/tests/unit/head_migration.js

@@ -27,8 +27,6 @@ var { PlacesTestUtils } = ChromeUtils.import(
   "resource://testing-common/PlacesTestUtils.jsm"
 );
 
-XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
-
 ChromeUtils.defineModuleGetter(
   this,
   "FileUtils",

browser/components/newtab/test/xpcshell/test_AboutHomeStartupCacheWorker.js

@@ -41,8 +41,6 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/PromiseWorker.jsm"
 );
 
-XPCOMUtils.defineLazyGlobalGetters(this, ["DOMParser"]);
-
 const CACHE_WORKER_URL = "resource://activity-stream/lib/cache-worker.js";
 const NEWTAB_RENDER_URL =
   "resource://activity-stream/data/content/newtab-render.js";

browser/extensions/search-detection/extension/api.js

@@ -13,6 +13,7 @@ const { WebRequest } = ChromeUtils.import(
   "resource://gre/modules/WebRequest.jsm"
 );
 
+// eslint-disable-next-line mozilla/reject-importGlobalProperties
 XPCOMUtils.defineLazyGlobalGetters(this, ["ChannelWrapper"]);
 
 XPCOMUtils.defineLazyGetter(this, "searchInitialized", () => {

browser/extensions/webcompat/experiment-apis/trackingProtection.js

@@ -6,6 +6,7 @@
 
 /* global ExtensionAPI, ExtensionCommon, ExtensionParent, Services, XPCOMUtils */
 
+// eslint-disable-next-line mozilla/reject-importGlobalProperties
 XPCOMUtils.defineLazyGlobalGetters(this, ["URL", "ChannelWrapper"]);
 
 class AllowList {

docs/code-quality/lint/linters/eslint-plugin-mozilla/reject-importGlobalProperties.rst

@@ -1,7 +1,45 @@
 reject-importGlobalProperties
 =============================
 
-Rejects calls to ``Cu.importGlobalProperties``. This is defined in the
-recommended configuration to allow non-WebIDL imports, but reject others.
+Rejects calls to ``Cu.importGlobalProperties`` or
+``XPCOMUtils.defineLazyGlobalGetters``.
 
-In some places in the tree, it will reject everything.
+In system modules all the required properties should already be available. In
+non-module code or non-system modules, webidl defined interfaces should already
+be available and hence do not need importing.
+
+Options
+-------
+
+* "everything": Disallows using the import/getters completely.
+* "allownonwebidl": Disallows using the import functions for webidl symbols. Allows
+  other symbols.
+
+everything
+----------
+
+Incorrect code for this option:
+
+.. code-block:: js
+
+    Cu.importGlobalProperties(['TextEncoder']);
+    XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder']);
+
+allownonwebidl
+--------------
+
+Incorrect code for this option:
+
+.. code-block:: js
+
+    // AnimationEffect is a webidl property.
+    Cu.importGlobalProperties(['AnimationEffect']);
+    XPCOMUtils.defineLazyGlobalGetters(this, ['AnimationEffect']);
+
+Correct code for this option:
+
+.. code-block:: js
+
+    // TextEncoder is not defined by webidl.
+    Cu.importGlobalProperties(['TextEncoder']);
+    XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder']);

remote/marionette/test/xpcshell/test_navigate.js

@@ -2,11 +2,6 @@
  * 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/. */
 
-const { XPCOMUtils } = ChromeUtils.import(
-  "resource://gre/modules/XPCOMUtils.jsm"
-);
-XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
-
 const { navigate } = ChromeUtils.import(
   "chrome://remote/content/marionette/navigate.js"
 );

security/manager/tools/dumpGoogleRoots.js

@@ -17,12 +17,6 @@ var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 Services.prefs.setBoolPref("network.process.enabled", false);
 
-const { XPCOMUtils } = ChromeUtils.import(
-  "resource://gre/modules/XPCOMUtils.jsm"
-);
-
-XPCOMUtils.defineLazyGlobalGetters(this, ["XMLHttpRequest"]);
-
 function downloadRoots() {
   let req = new XMLHttpRequest();
   req.open("GET", "https://pki.google.com/roots.pem", false);

toolkit/components/antitracking/test/xpcshell/test_ExceptionListService.js

@@ -14,8 +14,6 @@ const { RemoteSettings } = ChromeUtils.import(
 const COLLECTION_NAME = "partitioning-exempt-urls";
 const PREF_NAME = "privacy.restrict3rdpartystorage.skip_list";
 
-XPCOMUtils.defineLazyGlobalGetters(this, ["EventTarget"]);
-
 do_get_profile();
 
 class UpdateEvent extends EventTarget {}

toolkit/components/extensions/.eslintrc.js

@@ -28,6 +28,10 @@ module.exports = {
     "mozilla/balanced-listeners": "error",
     "mozilla/no-aArgs": "error",
     "mozilla/var-only-at-top-level": "error",
+    // Disable reject-importGlobalProperties because we don't want to include
+    // these in the sandbox directly as that would potentially mean the
+    // imported properties would be instatiated up-front rather than lazily.
+    "mozilla/reject-importGlobalProperties": "off",
 
     "valid-jsdoc": [
       "error",

toolkit/components/url-classifier/tests/unit/test_exceptionListService.js

@@ -9,9 +9,6 @@
 const { RemoteSettings } = ChromeUtils.import(
   "resource://services-settings/remote-settings.js"
 );
-const { XPCOMUtils } = ChromeUtils.import(
-  "resource://gre/modules/XPCOMUtils.jsm"
-);
 
 const COLLECTION_NAME = "url-classifier-skip-urls";
 const FEATURE_TRACKING_NAME = "tracking-annotation-test";
@@ -22,8 +19,6 @@ const FEATURE_FINGERPRINTING_NAME = "fingerprinting-annotation-test";
 const FEATURE_FINGERPRINTING_PREF_NAME =
   "urlclassifier.fingerprinting-annotation-test";
 
-XPCOMUtils.defineLazyGlobalGetters(this, ["EventTarget"]);
-
 do_get_profile();
 
 class UpdateEvent extends EventTarget {}

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-importGlobalProperties.js

@@ -22,8 +22,10 @@ module.exports = {
     },
     messages: {
       unexpectedCall: "Unexpected call to Cu.importGlobalProperties",
-      unexpectedCallWebIdl:
-        "Unnecessary call to Cu.importGlobalProperties (webidl names are automatically imported)",
+      unexpectedCallCuWebIdl:
+        "Unnecessary call to Cu.importGlobalProperties for {{name}} (webidl names are automatically imported)",
+      unexpectedCallXPCOMWebIdl:
+        "Unnecessary call to XPCOMUtils.defineLazyGlobalGetters for {{name}} (webidl names are automatically imported)",
     },
     schema: [
       {
@@ -47,7 +49,7 @@ module.exports = {
         let memexp = node.callee;
         if (
           memexp.object.type === "Identifier" &&
-          // Only Cu, not Components.utils; see bug 1230369.
+          // Only Cu, not Components.utils as `use-cc-etc` handles this for us.
           memexp.object.name === "Cu" &&
           memexp.property.type === "Identifier" &&
           memexp.property.name === "importGlobalProperties"
@@ -55,7 +57,32 @@ module.exports = {
           if (context.options.includes("allownonwebidl")) {
             for (let element of node.arguments[0].elements) {
               if (privilegedGlobals.includes(element.value)) {
-                context.report({ node, messageId: "unexpectedCallWebIdl" });
+                context.report({
+                  node,
+                  messageId: "unexpectedCallCuWebIdl",
+                  data: { name: element.value },
+                });
+              }
+            }
+          } else {
+            context.report({ node, messageId: "unexpectedCall" });
+          }
+        }
+        if (
+          memexp.object.type === "Identifier" &&
+          memexp.object.name === "XPCOMUtils" &&
+          memexp.property.type === "Identifier" &&
+          memexp.property.name === "defineLazyGlobalGetters" &&
+          node.arguments.length >= 2
+        ) {
+          if (context.options.includes("allownonwebidl")) {
+            for (let element of node.arguments[1].elements) {
+              if (privilegedGlobals.includes(element.value)) {
+                context.report({
+                  node,
+                  messageId: "unexpectedCallXPCOMWebIdl",
+                  data: { name: element.value },
+                });
               }
             }
           } else {

tools/lint/eslint/eslint-plugin-mozilla/tests/reject-importGlobalProperties.js

@@ -25,11 +25,20 @@ ruleTester.run("reject-importGlobalProperties", rule, {
       options: ["allownonwebidl"],
       code: "Cu.importGlobalProperties(['fetch'])",
     },
+    {
+      options: ["allownonwebidl"],
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['fetch'])",
+    },
     {
       options: ["allownonwebidl"],
       code: "Cu.importGlobalProperties(['TextEncoder'])",
       filename: "foo.sjs",
     },
+    {
+      options: ["allownonwebidl"],
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
+      filename: "foo.sjs",
+    },
   ],
   invalid: [
     {
@@ -37,20 +46,41 @@ ruleTester.run("reject-importGlobalProperties", rule, {
       options: ["everything"],
       errors: [{ messageId: "unexpectedCall" }],
     },
+    {
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['fetch'])",
+      options: ["everything"],
+      errors: [{ messageId: "unexpectedCall" }],
+    },
     {
       code: "Cu.importGlobalProperties(['TextEncoder'])",
       options: ["everything"],
       errors: [{ messageId: "unexpectedCall" }],
     },
+    {
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
+      options: ["everything"],
+      errors: [{ messageId: "unexpectedCall" }],
+    },
     {
       code: "Cu.importGlobalProperties(['TextEncoder'])",
       options: ["allownonwebidl"],
-      errors: [{ messageId: "unexpectedCallWebIdl" }],
+      errors: [{ messageId: "unexpectedCallCuWebIdl" }],
+    },
+    {
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
+      options: ["allownonwebidl"],
+      errors: [{ messageId: "unexpectedCallXPCOMWebIdl" }],
     },
     {
       options: ["allownonwebidl"],
       code: "Cu.importGlobalProperties(['TextEncoder'])",
-      errors: [{ messageId: "unexpectedCallWebIdl" }],
+      errors: [{ messageId: "unexpectedCallCuWebIdl" }],
+      filename: "foo.js",
+    },
+    {
+      options: ["allownonwebidl"],
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
+      errors: [{ messageId: "unexpectedCallXPCOMWebIdl" }],
       filename: "foo.js",
     },
   ],