Bug 1703953 - Part 1: Implement mozilla/use-isinstance rule r=Gijs,Standard8

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

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

@@ -59,6 +59,7 @@ The plugin implements the following rules:
    eslint-plugin-mozilla/use-chromeutils-import
    eslint-plugin-mozilla/use-default-preference-values
    eslint-plugin-mozilla/use-includes-instead-of-indexOf
+   eslint-plugin-mozilla/use-isInstance
    eslint-plugin-mozilla/use-ownerGlobal
    eslint-plugin-mozilla/use-returnValue
    eslint-plugin-mozilla/use-services

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

@@ -0,0 +1,24 @@
+use-isInstance
+==============
+
+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.
+
+Examples of incorrect code for this rule:
+-----------------------------------------
+
+.. code-block:: js
+
+    node instanceof Node
+    text instanceof win.Text
+    target instanceof this.contentWindow.HTMLAudioElement
+
+Examples of correct code for this rule:
+---------------------------------------
+
+.. code-block:: js
+
+    Node.isInstance(node)
+    win.Text.isInstance(text)
+    this.contentWindow.HTMLAudioElement.isInstance(target)

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

@@ -61,6 +61,10 @@ module.exports = {
       },
       rules: {
         "mozilla/mark-exported-symbols-as-used": "error",
+        // Bug 1703953: We don't have a good way to check a file runs in a
+        // privilieged context. Apply this for jsm 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 }],

tools/lint/eslint/eslint-plugin-mozilla/lib/index.js

@@ -69,6 +69,7 @@ module.exports = {
     "use-default-preference-values": require("../lib/rules/use-default-preference-values"),
     "use-ownerGlobal": require("../lib/rules/use-ownerGlobal"),
     "use-includes-instead-of-indexOf": require("../lib/rules/use-includes-instead-of-indexOf"),
+    "use-isInstance": require("./rules/use-isInstance"),
     "use-returnValue": require("../lib/rules/use-returnValue"),
     "use-services": require("../lib/rules/use-services"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level"),

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

@@ -0,0 +1,70 @@
+/**
+ * @fileoverview Reject use of instanceof against DOM interfaces
+ *
+ * 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";
+
+const privilegedGlobals = Object.keys(
+  require("../environments/privileged.js").globals
+);
+
+// -----------------------------------------------------------------------------
+// Rule Definition
+// -----------------------------------------------------------------------------
+
+function pointsToDOMInterface(expression) {
+  let object;
+  let referee = expression;
+  while (referee.type === "MemberExpression") {
+    object = referee.object;
+    referee = referee.property;
+  }
+  if (referee.type === "Identifier") {
+    if (referee.name === "File" && object.name === "OS") {
+      // OS.File is an exception that is not a DOM interface
+      return false;
+    }
+    return privilegedGlobals.includes(referee.name);
+  }
+  return false;
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      url:
+        "https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/use-isInstance.html",
+    },
+    fixable: "code",
+    schema: [],
+    type: "problem",
+  },
+  create(context) {
+    return {
+      BinaryExpression(node) {
+        const { operator, right } = node;
+        if (operator === "instanceof" && pointsToDOMInterface(right)) {
+          context.report({
+            node,
+            message:
+              "Please 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.",
+            fix(fixer) {
+              const sourceCode = context.getSourceCode();
+              return fixer.replaceText(
+                node,
+                `${sourceCode.getText(right)}.isInstance(${sourceCode.getText(
+                  node.left
+                )})`
+              );
+            },
+          });
+        }
+      },
+    };
+  },
+};

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

@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/use-isInstance");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const errors = [
+  {
+    message:
+      "Please 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.",
+    type: "BinaryExpression",
+  },
+];
+
+ruleTester.run("use-isInstance", rule, {
+  valid: [
+    "(() => {}) instanceof Function;",
+    "({}) instanceof Object;",
+    "Node instanceof Object;",
+    "file instanceof OS.File;",
+  ],
+  invalid: [
+    {
+      code: "node instanceof Node",
+      output: "Node.isInstance(node)",
+      errors,
+    },
+    {
+      code: "text instanceof win.Text",
+      output: "win.Text.isInstance(text)",
+      errors,
+    },
+    {
+      code: "target instanceof this.contentWindow.HTMLAudioElement",
+      output: "this.contentWindow.HTMLAudioElement.isInstance(target)",
+      errors,
+    },
+  ],
+});