Bug 1773740 - Part 2: Only handle identifiers pointing to environment globals r=Standard8

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

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

@@ -18,17 +18,60 @@ const privilegedGlobals = Object.keys(
 // Rule Definition
 // -----------------------------------------------------------------------------
 
-function pointsToDOMInterface(expression) {
-  if (
-    expression.type === "MemberExpression" &&
-    maybeGetMemberPropertyName(expression.object) === "OS" &&
-    expression.property.name === "File"
-  ) {
-    // OS.File is an exception that is not a DOM interface
+/**
+ * Whether an identifier is defined by eslint configuration.
+ * `env: { browser: true }` or `globals: []` for example.
+ * @param {import("eslint-scope").Scope} currentScope
+ * @param {import("estree").Identifier} id
+ */
+function refersToEnvironmentGlobals(currentScope, id) {
+  const reference = currentScope.references.find(ref => ref.identifier === id);
+  const { resolved } = reference || {};
+  if (!resolved) {
     return false;
   }
-  // For `win.Foo`, `iframe.contentWindow.Foo`, or such.
-  return privilegedGlobals.includes(maybeGetMemberPropertyName(expression));
+
+  // No definition in script files; defined via .eslintrc
+  return resolved.scope.type === "global" && resolved.defs.length === 0;
+}
+
+/**
+ * Whether a node points to a DOM interface.
+ * Includes direct references to interfaces objects and also indirect references
+ * via property access.
+ * OS.File and lazy.(Foo) are explicitly excluded.
+ *
+ * @example HTMLElement
+ * @example win.HTMLElement
+ * @example iframe.contentWindow.HTMLElement
+ * @example foo.HTMLElement
+ *
+ * @param {import("eslint-scope").Scope} currentScope
+ * @param {import("estree").Node} node
+ */
+function pointsToDOMInterface(currentScope, node) {
+  if (node.type === "MemberExpression") {
+    const objectName = maybeGetMemberPropertyName(node.object);
+    if (objectName === "lazy") {
+      // lazy.Foo is probably a non-IDL import.
+      return false;
+    }
+    if (objectName === "OS" && node.property.name === "File") {
+      // OS.File is an exception that is not a Web IDL interface
+      return false;
+    }
+    // For `win.Foo`, `iframe.contentWindow.Foo`, or such.
+    return privilegedGlobals.includes(node.property.name);
+  }
+
+  if (
+    node.type === "Identifier" &&
+    refersToEnvironmentGlobals(currentScope, node)
+  ) {
+    return privilegedGlobals.includes(node.name);
+  }
+
+  return false;
 }
 
 module.exports = {
@@ -41,11 +84,17 @@ module.exports = {
     schema: [],
     type: "problem",
   },
+  /**
+   * @param {import("eslint").Rule.RuleContext} context
+   */
   create(context) {
     return {
       BinaryExpression(node) {
         const { operator, right } = node;
-        if (operator === "instanceof" && pointsToDOMInterface(right)) {
+        if (
+          operator === "instanceof" &&
+          pointsToDOMInterface(context.getScope(), right)
+        ) {
           context.report({
             node,
             message:

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

@@ -25,21 +25,41 @@ const errors = [
   },
 ];
 
+const env = { browser: true };
+
+/**
+ * A test case boilerplate simulating chrome privileged script.
+ * @param {string} code
+ */
+function mockChromeScript(code) {
+  return {
+    code,
+    env,
+  };
+}
+
 ruleTester.run("use-isInstance", rule, {
   valid: [
-    "(() => {}) instanceof Function;",
-    "({}) instanceof Object;",
-    "Node instanceof Object;",
-    "file instanceof OS.File;",
-    "file instanceof OS.File.Error;",
-    "file instanceof lazy.OS.File;",
-    "file instanceof lazy.OS.File.Error;",
-    "file instanceof lazy.lazy.OS.File;",
+    mockChromeScript("(() => {}) instanceof Function;"),
+    mockChromeScript("({}) instanceof Object;"),
+    mockChromeScript("Node instanceof Object;"),
+    mockChromeScript("node instanceof lazy.Node;"),
+    mockChromeScript("var Node;node instanceof Node;"),
+    mockChromeScript("file instanceof lazy.File;"),
+    mockChromeScript("file instanceof OS.File;"),
+    mockChromeScript("file instanceof OS.File.Error;"),
+    mockChromeScript("file instanceof lazy.OS.File;"),
+    mockChromeScript("file instanceof lazy.OS.File.Error;"),
+    mockChromeScript("file instanceof lazy.lazy.OS.File;"),
+    mockChromeScript("var File;file instanceof File;"),
+    mockChromeScript("foo instanceof RandomGlobalThing;"),
+    mockChromeScript("foo instanceof lazy.RandomGlobalThing;"),
   ],
   invalid: [
     {
       code: "node instanceof Node",
       output: "Node.isInstance(node)",
+      env,
       errors,
     },
     {
@@ -55,6 +75,7 @@ ruleTester.run("use-isInstance", rule, {
     {
       code: "target instanceof File",
       output: "File.isInstance(target)",
+      env,
       errors,
     },
     {