Bug 1858148 - Change ESLint rule reject-importGlobalProperties to rej…

…ect all imports in sjs files. r=mossop Differential Revision: https://phabricator.services.mozilla.com/D190760
dothq · Nov 4, 2023 · 922f753 · 922f753
1 parent a033642
commit 922f753
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 22 deletions.
diff --git a/...lity/lint/linters/eslint-plugin-mozilla/rules/reject-importGlobalProperties.rst b/...lity/lint/linters/eslint-plugin-mozilla/rules/reject-importGlobalProperties.rst
@@ -8,6 +8,9 @@ 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.
 
+For sjs test files, if the relevant global is not already available, then consider
+extending the `list of globals available from the httpd server <https://searchfox.org/mozilla-central/rev/e9b338c2d597067f99e96d5f20769f41f312fa8f/netwerk/test/httpserver/httpd.sys.mjs#2875-2889>`_.
+
 Options
 -------
 

diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -105,10 +105,9 @@ module.exports = {
       },
       files: ["**/*.sjs"],
       rules: {
-        // TODO Bug 1501127: sjs files have their own sandbox, and do not inherit
-        // the Window backstage pass directly. Turn this rule off for sjs files for
-        // now until we develop a solution.
-        "mozilla/reject-importGlobalProperties": "off",
+        // For sjs files, reject everything as we should update the sandbox
+        // to include the globals we need, as these are test-only files.
+        "mozilla/reject-importGlobalProperties": ["error", "everything"],
       },
     },
     {

diff --git a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-importGlobalProperties.js b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-importGlobalProperties.js
@@ -14,6 +14,12 @@ const privilegedGlobals = Object.keys(
   require("../environments/privileged.js").globals
 );
 
+function getMessageId(context) {
+  return path.extname(context.getFilename()) == ".sjs"
+    ? "unexpectedCallSjs"
+    : "unexpectedCall";
+}
+
 module.exports = {
   meta: {
     docs: {
@@ -23,6 +29,8 @@ module.exports = {
       unexpectedCall: "Unexpected call to Cu.importGlobalProperties",
       unexpectedCallCuWebIdl:
         "Unnecessary call to Cu.importGlobalProperties for {{name}} (webidl names are automatically imported)",
+      unexpectedCallSjs:
+        "Do not call Cu.importGlobalProperties in sjs files, expand the global instead (see rule docs).",
       unexpectedCallXPCOMWebIdl:
         "Unnecessary call to XPCOMUtils.defineLazyGlobalGetters for {{name}} (webidl names are automatically imported)",
     },
@@ -37,12 +45,7 @@ module.exports = {
   create(context) {
     return {
       CallExpression(node) {
-        if (
-          node.callee.type !== "MemberExpression" ||
-          // TODO Bug 1501127: sjs files have their own sandbox, and do not inherit
-          // the Window backstage pass directly.
-          path.extname(context.getFilename()) == ".sjs"
-        ) {
+        if (node.callee.type !== "MemberExpression") {
           return;
         }
         let memexp = node.callee;
@@ -64,7 +67,7 @@ module.exports = {
               }
             }
           } else {
-            context.report({ node, messageId: "unexpectedCall" });
+            context.report({ node, messageId: getMessageId(context) });
           }
         }
         if (
@@ -85,7 +88,7 @@ module.exports = {
               }
             }
           } else {
-            context.report({ node, messageId: "unexpectedCall" });
+            context.report({ node, messageId: getMessageId(context) });
           }
         }
       },

diff --git a/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-importGlobalProperties.js b/tools/lint/eslint/eslint-plugin-mozilla/tests/reject-importGlobalProperties.js
@@ -29,16 +29,6 @@ ruleTester.run("reject-importGlobalProperties", rule, {
       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: [
     {
@@ -56,6 +46,12 @@ ruleTester.run("reject-importGlobalProperties", rule, {
       options: ["everything"],
       errors: [{ messageId: "unexpectedCall" }],
     },
+    {
+      options: ["everything"],
+      code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
+      errors: [{ messageId: "unexpectedCallSjs" }],
+      filename: "foo.sjs",
+    },
     {
       code: "XPCOMUtils.defineLazyGlobalGetters(this, ['TextEncoder'])",
       options: ["everything"],
@@ -83,5 +79,11 @@ ruleTester.run("reject-importGlobalProperties", rule, {
       errors: [{ messageId: "unexpectedCallXPCOMWebIdl" }],
       filename: "foo.js",
     },
+    {
+      options: ["allownonwebidl"],
+      code: "Cu.importGlobalProperties(['TextEncoder'])",
+      errors: [{ messageId: "unexpectedCallCuWebIdl" }],
+      filename: "foo.sjs",
+    },
   ],
 });