Skip to content

Commit

Permalink
Bug 1771751 - Reject static import for system module from non-system …
Browse files Browse the repository at this point in the history
…module. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D149183
  • Loading branch information
arai-a committed Jun 14, 2022
1 parent 371b3c1 commit 9975cd7
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/code-quality/lint/linters/eslint-plugin-mozilla.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ The plugin implements the following rules:
eslint-plugin-mozilla/reject-scriptableunicodeconverter
eslint-plugin-mozilla/reject-some-requires
eslint-plugin-mozilla/reject-top-level-await
eslint-plugin-mozilla/reject-import-system-module-from-non-system
eslint-plugin-mozilla/use-cc-etc
eslint-plugin-mozilla/use-chromeutils-generateqi
eslint-plugin-mozilla/use-chromeutils-import
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
reject-import-system-module-from-non-system
===========================================

Rejects static import declaration for system modules (``.sys.mjs``) from non-system
modules.

Using static import for a system module into a non-system module would create a separate instance of the imported object(s) that is not shared with the other system modules and would break the per-process singleton expectation.

The reason for this is that inside system modules, a static import will load the module into the shared global. Inside non-system modules, the static import will load into a different global (e.g. window). This will cause the module to be loaded into different scopes, and hence create separate instances. The fix is to use ``ChromeUtils.importESM`` which will import the object via the system module shared global scope.


Examples of incorrect code for this rule:
-----------------------------------------

Inside a non-system module:

.. code-block:: js
import { Services } from "resource://gre/modules/Services.sys.mjs";
Examples of correct code for this rule:
---------------------------------------

Inside a non-system module:

.. code-block:: js
const { Services } = ChromeUtils.importESM(
"resource://gre/modules/Services.sys.mjs"
);
Inside a system module:

.. code-block:: js
import { Services } from "resource://gre/modules/Services.sys.mjs";
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ module.exports = {
],
},
},
{
excludedFiles: ["**/*.sys.mjs"],
files: ["**/*.mjs"],
rules: {
"mozilla/reject-import-system-module-from-non-system": "error",
},
},
{
files: ["**/*.jsm", "**/*.jsm.js"],
rules: {
Expand Down
1 change: 1 addition & 0 deletions tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ module.exports = {
"reject-addtask-only": require("../lib/rules/reject-addtask-only"),
"reject-chromeutils-import-params": require("../lib/rules/reject-chromeutils-import-params"),
"reject-global-this": require("../lib/rules/reject-global-this"),
"reject-import-system-module-from-non-system": require("../lib/rules/reject-import-system-module-from-non-system"),
"reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
"reject-osfile": require("../lib/rules/reject-osfile"),
"reject-scriptableunicodeconverter": require("../lib/rules/reject-scriptableunicodeconverter"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* 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";

module.exports = {
meta: {
docs: {
url:
"https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/reject-import-system-module-from-non-system.html",
},
messages: {
rejectStaticImportSystemModuleFromNonSystem:
"System modules (*.sys.mjs) can be imported with static import declaration only from system modules.",
},
type: "problem",
},

create(context) {
return {
ImportDeclaration(node) {
if (!node.source.value.endsWith(".sys.mjs")) {
return;
}

context.report({
node,
messageId: "rejectStaticImportSystemModuleFromNonSystem",
});
},
};
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */

"use strict";

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

var rule = require("../lib/rules/reject-import-system-module-from-non-system");
var RuleTester = require("eslint").RuleTester;

const ruleTester = new RuleTester({
parserOptions: { ecmaVersion: 13, sourceType: "module" },
});

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

ruleTester.run("reject-import-system-module-from-non-system", rule, {
valid: [
{
code: `const { Services } = ChromeUtils.importESM("resource://gre/modules/Services.sys.mjs");`,
},
],
invalid: [
{
code: `import { Services } from "resource://gre/modules/Services.sys.mjs";`,
errors: [{ messageId: "rejectStaticImportSystemModuleFromNonSystem" }],
},
],
});

0 comments on commit 9975cd7

Please sign in to comment.