Skip to content

Commit

Permalink
Bug 1607331 - Part 3: Reject global this usage in JSM. r=Standard8
Browse files Browse the repository at this point in the history
  • Loading branch information
arai-a committed Jun 14, 2022
1 parent 47585c7 commit 371b3c1
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 0 deletions.
1 change: 1 addition & 0 deletions devtools/client/performance-new/popup/background.jsm.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const CURRENT_WEBCHANNEL_VERSION = 1;

// Lazily load the require function, when it's needed.
ChromeUtils.defineModuleGetter(
// eslint-disable-next-line mozilla/reject-global-this
this,
"require",
"resource://devtools/shared/loader/Loader.jsm"
Expand Down
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 @@ -48,6 +48,7 @@ The plugin implements the following rules:
eslint-plugin-mozilla/prefer-formatValues
eslint-plugin-mozilla/reject-addtask-only
eslint-plugin-mozilla/reject-chromeutils-import-params
eslint-plugin-mozilla/reject-global-this
eslint-plugin-mozilla/reject-importGlobalProperties
eslint-plugin-mozilla/reject-osfile
eslint-plugin-mozilla/reject-relative-requires
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
reject-global-this
======================

Rejects global ``this`` usage in JSM files. The global ``this`` is not
available in ESM, and this is a preparation for the migration.

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

.. code-block:: js
this.EXPORTED_SYMBOLS = ["foo"];
XPCOMUtils.defineLazyModuleGetters(this, {
AddonManager: "resource://gre/modules/AddonManager.jsm",
});
Examples of correct code for this rule:
---------------------------------------

.. code-block:: js
const EXPORTED_SYMBOLS = ["foo"];
const lazy = {};
XPCOMUtils.defineLazyModuleGetters(lazy, {
AddonManager: "resource://gre/modules/AddonManager.jsm",
});
3 changes: 3 additions & 0 deletions mobile/android/modules/geckoview/AndroidLog.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@

if (typeof Components != "undefined") {
// Specify exported symbols for JSM module loader.
//
// (bug 1773390)
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = ["AndroidLog"];
var { ctypes } = ChromeUtils.import("resource://gre/modules/ctypes.jsm");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ if (typeof Components != "undefined") {
// Global definition of |exports|, to keep everybody happy.
// In non-main thread, |exports| is provided by the module
// loader.
// eslint-disable-next-line mozilla/reject-global-this
this.exports = {};
({ Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"));
// eslint-disable-next-line mozilla/reject-global-this
this.Services = Services;
Meta = ChromeUtils.import("resource://gre/modules/PromiseWorker.jsm")
.BasePromiseWorker.Meta;
Expand Down Expand Up @@ -1362,8 +1364,10 @@ Object.defineProperty(exports.OS.Shared, "TEST", {

// /////////////////// Permanent boilerplate
if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = EXPORTED_SYMBOLS;
for (let symbol of EXPORTED_SYMBOLS) {
// eslint-disable-next-line mozilla/reject-global-this
this[symbol] = exports[symbol];
}
}
4 changes: 4 additions & 0 deletions toolkit/components/osfile/modules/osfile_unix_allthreads.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ var SharedAll;
if (typeof Components != "undefined") {
// Module is opened as a jsm module
const { ctypes } = ChromeUtils.import("resource://gre/modules/ctypes.jsm");
// eslint-disable-next-line mozilla/reject-global-this
this.ctypes = ctypes;

SharedAll = ChromeUtils.import(
"resource://gre/modules/osfile/osfile_shared_allthreads.jsm"
);
// eslint-disable-next-line mozilla/reject-global-this
this.exports = {};
} else if (typeof module != "undefined" && typeof require != "undefined") {
// Module is loaded with require()
Expand Down Expand Up @@ -399,8 +401,10 @@ var EXPORTED_SYMBOLS = [

// ////////// Boilerplate
if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = EXPORTED_SYMBOLS;
for (let symbol of EXPORTED_SYMBOLS) {
// eslint-disable-next-line mozilla/reject-global-this
this[symbol] = exports[symbol];
}
}
4 changes: 4 additions & 0 deletions toolkit/components/osfile/modules/osfile_win_allthreads.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ var SharedAll;
if (typeof Components != "undefined") {
// Module is opened as a jsm module
const { ctypes } = ChromeUtils.import("resource://gre/modules/ctypes.jsm");
// eslint-disable-next-line mozilla/reject-global-this
this.ctypes = ctypes;

SharedAll = ChromeUtils.import(
"resource://gre/modules/osfile/osfile_shared_allthreads.jsm"
);
// eslint-disable-next-line mozilla/reject-global-this
this.exports = {};
} else if (typeof module != "undefined" && typeof require != "undefined") {
// Module is loaded with require()
Expand Down Expand Up @@ -431,8 +433,10 @@ var EXPORTED_SYMBOLS = [

// ////////// Boilerplate
if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = EXPORTED_SYMBOLS;
for (let symbol of EXPORTED_SYMBOLS) {
// eslint-disable-next-line mozilla/reject-global-this
this[symbol] = exports[symbol];
}
}
2 changes: 2 additions & 0 deletions toolkit/components/osfile/modules/ospath.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ if (typeof Components == "undefined") {
Path = ChromeUtils.import("resource://gre/modules/osfile/ospath_unix.jsm");
}

// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = [];
for (let k in Path) {
EXPORTED_SYMBOLS.push(k);
// eslint-disable-next-line mozilla/reject-global-this
this[k] = Path[k];
}
}
3 changes: 3 additions & 0 deletions toolkit/components/osfile/modules/ospath_unix.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ if (typeof Components != "undefined") {
// Global definition of |exports|, to keep everybody happy.
// In non-main thread, |exports| is provided by the module
// loader.
// eslint-disable-next-line mozilla/reject-global-this
this.exports = {};
} else if (typeof module == "undefined" || typeof exports == "undefined") {
throw new Error("Please load this module using require()");
Expand Down Expand Up @@ -194,8 +195,10 @@ exports.fromFileURI = fromFileURI;

// ////////// Boilerplate
if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = EXPORTED_SYMBOLS;
for (let symbol of EXPORTED_SYMBOLS) {
// eslint-disable-next-line mozilla/reject-global-this
this[symbol] = exports[symbol];
}
}
3 changes: 3 additions & 0 deletions toolkit/components/osfile/modules/ospath_win.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ if (typeof Components != "undefined") {
// Global definition of |exports|, to keep everybody happy.
// In non-main thread, |exports| is provided by the module
// loader.
// eslint-disable-next-line mozilla/reject-global-this
this.exports = {};
} else if (typeof module == "undefined" || typeof exports == "undefined") {
throw new Error("Please load this module using require()");
Expand Down Expand Up @@ -372,8 +373,10 @@ var trimBackslashes = function trimBackslashes(string) {

// ////////// Boilerplate
if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = EXPORTED_SYMBOLS;
for (let symbol of EXPORTED_SYMBOLS) {
// eslint-disable-next-line mozilla/reject-global-this
this[symbol] = exports[symbol];
}
}
2 changes: 2 additions & 0 deletions toolkit/components/osfile/osfile.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
*/

if (typeof Components != "undefined") {
// eslint-disable-next-line mozilla/reject-global-this
this.EXPORTED_SYMBOLS = ["OS"];
const { OS } = ChromeUtils.import(
"resource://gre/modules/osfile/osfile_async_front.jsm"
);
// eslint-disable-next-line mozilla/reject-global-this
this.OS = OS;
} else {
/* eslint-env worker */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ module.exports = {
},
files: ["**/*.sys.mjs", "**/*.jsm", "**/*.jsm.js"],
rules: {
"mozilla/reject-global-this": "error",
"mozilla/reject-top-level-await": "error",
// Bug 1703953: We don't have a good way to check a file runs in a
// privilieged context. Apply this for these files as we know those are
Expand All @@ -60,6 +61,13 @@ module.exports = {
"no-redeclare": ["error", { builtinGlobals: false }],
},
},
{
// Temporarily disable until the proxy-based loader gets landed.
files: ["browser/components/urlbar/**"],
rules: {
"mozilla/reject-global-this": "off",
},
},
{
files: ["**/*.mjs", "**/*.jsm"],
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 @@ -54,6 +54,7 @@ module.exports = {
"prefer-formatValues": require("../lib/rules/prefer-formatValues"),
"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-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,55 @@
/**
* @fileoverview Reject attempts to use the global object in jsms.
*
* 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
// -----------------------------------------------------------------------------

function isGlobalThis(context, node) {
for (let ancestor of context.getAncestors()) {
if (
ancestor.type == "FunctionDeclaration" ||
ancestor.type == "FunctionExpression" ||
ancestor.type == "ClassProperty" ||
ancestor.type == "ClassPrivateProperty" ||
ancestor.type == "PropertyDefinition" ||
ancestor.type == "StaticBlock"
) {
return false;
}
}

return true;
}

module.exports = {
meta: {
docs: {
url:
"https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/reject-global-this.html",
},
type: "problem",
},

create(context) {
return {
ThisExpression(node) {
if (!isGlobalThis(context, node)) {
return;
}

context.report({
node,
message: `JSM should not use the global this`,
});
},
};
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */

"use strict";

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

var rule = require("../lib/rules/reject-global-this");
var RuleTester = require("eslint").RuleTester;

// class static block is available from ES2022 = 13.
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 13 } });

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

function invalidCode(code) {
let message = "JSM should not use the global this";
return { code, errors: [{ message, type: "ThisExpression" }] };
}

ruleTester.run("reject-top-level-await", rule, {
valid: [
"function f() { this; }",
"(function f() { this; });",
"({ foo() { this; } });",
"({ get foo() { this; } })",
"({ set foo(x) { this; } })",
"class X { foo() { this; } }",
"class X { get foo() { this; } }",
"class X { set foo(x) { this; } }",
"class X { static foo() { this; } }",
"class X { static get foo() { this; } }",
"class X { static set foo(x) { this; } }",
"class X { P = this; }",
"class X { #P = this; }",
"class X { static { this; } }",
],
invalid: [
invalidCode("this;"),
invalidCode("() => this;"),

invalidCode("this.foo = 10;"),
invalidCode("ChromeUtils.defineModuleGetter(this, {});"),
],
});

0 comments on commit 371b3c1

Please sign in to comment.