Commit 39d7ee6f authored by Kagami Sascha Rosylight's avatar Kagami Sascha Rosylight
Browse files

Bug 1773740 - Part 3: Cover js files including ChromeUtils r=Standard8 

Differential Revision: https://phabricator.services.mozilla.com/D148938
parent e984382b

docs/code-quality/lint/linters/eslint-plugin-mozilla/use-isInstance.rst

+11 −0
Original line number Diff line number Diff line
@@ -5,6 +5,17 @@ Prefer ``.isInstance()`` in chrome scripts over the standard ``instanceof`` 
operator for DOM interfaces, since the latter will return false when the object

is created from a different context.



These files are covered:



- ``*.sys.mjs``

- ``*.jsm``

- ``*.jsm.js``

- ``*.js`` with a heuristic



Since there is no straightforward way to detect chrome scripts, currently the

linter assumes that any script including ``ChromeUtils`` is chrome privileged.

This of course may not be sufficient and is open for change.



Examples of incorrect code for this rule:

-----------------------------------------

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js

+1 −4
Original line number Diff line number Diff line
@@ -55,10 +55,6 @@ module.exports = { 
        "mozilla/reject-global-this": "error",

        "mozilla/reject-globalThis-modification": "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

        // privilieged, and then include more directories elsewhere.

        "mozilla/use-isInstance": "error",

        // TODO: Bug 1575506 turn `builtinGlobals` on here.

        // We can enable builtinGlobals for jsms due to their scopes.

        "no-redeclare": ["error", { builtinGlobals: false }],
@@ -178,6 +174,7 @@ module.exports = { 
    "mozilla/use-chromeutils-import": "error",

    "mozilla/use-default-preference-values": "error",

    "mozilla/use-includes-instead-of-indexOf": "error",

    "mozilla/use-isInstance": "error",

    "mozilla/use-ownerGlobal": "error",

    "mozilla/use-returnValue": "error",

    "mozilla/use-services": "error",

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-isInstance.js

+22 −0
Original line number Diff line number Diff line
@@ -74,6 +74,24 @@ function pointsToDOMInterface(currentScope, node) { 
  return false;

}



/**

 * @param {import("eslint").Rule.RuleContext} context

 */

function isChromeContext(context) {

  const filename = context.getFilename();

  const isChromeFileName =

    filename.endsWith(".sys.mjs") ||

    filename.endsWith(".jsm") ||

    filename.endsWith(".jsm.js");

  if (isChromeFileName) {

    return true;

  }



  // Treat scripts using ChromeUtils as chrome scripts, but not SpecialPowers.ChromeUtils

  const source = context.getSourceCode().text;

  return !!source.match(/(^|\s)ChromeUtils/);

}



module.exports = {

  meta: {

    docs: {
@@ -88,6 +106,10 @@ module.exports = { 
   * @param {import("eslint").Rule.RuleContext} context

   */

  create(context) {

    if (!isChromeContext(context)) {

      return {};

    }



    return {

      BinaryExpression(node) {

        const { operator, right } = node;

tools/lint/eslint/eslint-plugin-mozilla/tests/use-isInstance.js

+38 −0
Original line number Diff line number Diff line
@@ -34,6 +34,19 @@ const env = { browser: true }; 
function mockChromeScript(code) {

  return {

    code,

    filename: "foo.sys.mjs",

    env,

  };

}



/**

 * A test case boilerplate simulating content script.

 * @param {string} code

 */

function mockContentScript(code) {

  return {

    code,

    filename: "foo.js",

    env,

  };

}
@@ -54,6 +67,11 @@ ruleTester.run("use-isInstance", rule, { 
    mockChromeScript("var File;file instanceof File;"),

    mockChromeScript("foo instanceof RandomGlobalThing;"),

    mockChromeScript("foo instanceof lazy.RandomGlobalThing;"),

    mockContentScript("node instanceof Node;"),

    mockContentScript("file instanceof File;"),

    mockContentScript(

      "SpecialPowers.ChromeUtils.import(''); file instanceof File;"

    ),

  ],

  invalid: [

    {
@@ -61,32 +79,52 @@ ruleTester.run("use-isInstance", rule, { 
      output: "Node.isInstance(node)",

      env,

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "text instanceof win.Text",

      output: "win.Text.isInstance(text)",

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "target instanceof this.contentWindow.HTMLAudioElement",

      output: "this.contentWindow.HTMLAudioElement.isInstance(target)",

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "target instanceof File",

      output: "File.isInstance(target)",

      env,

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "target instanceof win.File",

      output: "win.File.isInstance(target)",

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "window.arguments[0] instanceof window.XULElement",

      output: "window.XULElement.isInstance(window.arguments[0])",

      errors,

      filename: "foo.sys.mjs",

    },

    {

      code: "ChromeUtils.import(''); node instanceof Node",

      output: "ChromeUtils.import(''); Node.isInstance(node)",

      env,

      errors,

      filename: "foo.js",

    },

    {

      code: "ChromeUtils.import(''); file instanceof File",

      output: "ChromeUtils.import(''); File.isInstance(file)",

      env,

      errors,

      filename: "foo.js",

    },

  ],

});