Bug 1245916: Unify eslint global discovery rules. r=pbrosset

While working on turning on no-undef I discovered that the various rules we
have for defining globals are a little inconsistent in whether the files they
load recurse through import-globals-from directives and none of them imported
eslint globals directives.

I think we're better off putting all this global parsing code in a single place
rather than spread across multiple rules. Have one rule to turn it on for
parsed files and one function to load globals from other files and make them
share most of the code so we won't get inconsistent. If we find us needing to
turn on/off individual features we can figure out a way to do that in the
future.

This patch does that, the globals.js file does all global parsing with a shared
object that receives events from the AST, either through from an ESlint rule
or from a simple AST walker using estraverse.

MozReview-Commit-ID: 9KQZwsNNOUl

.eslintrc

@@ -4,9 +4,7 @@
     "mozilla"
   ],
   "rules": {
-    "mozilla/components-imports": 1,
-    "mozilla/import-globals-from": 1,
-    "mozilla/this-top-level-scope": 1,
+    "mozilla/import-globals": 1,
   },
   "env": {
     "es6": true

testing/eslint-plugin-mozilla/docs/components-imports.rst

@@ -1,21 +0,0 @@
-.. _components-imports:
-
-==================
-components-imports
-==================
-
-Rule Details
-------------
-
-Adds the filename of imported files e.g.
-``Cu.import("some/path/Blah.jsm")`` adds Blah to the global scope.
-
-The following patterns are supported:
-
--  ``Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm");``
--  ``loader.lazyImporter(this, "name1");``
--  ``loader.lazyRequireGetter(this, "name2"``
--  ``loader.lazyServiceGetter(this, "name3"``
--  ``XPCOMUtils.defineLazyModuleGetter(this, "setNamedTimeout", ...)``
--  ``loader.lazyGetter(this, "toolboxStrings"``
--  ``XPCOMUtils.defineLazyGetter(this, "clipboardHelper"``

testing/eslint-plugin-mozilla/docs/import-globals-from.rst

@@ -1,18 +0,0 @@
-.. _import-globals-from:
-
-===================
-import-globals-from
-===================
-
-Rule Details
-------------
-
-When the "import-globals-from <path>" comment is found in a file, then all
-globals from the file at <path> will be imported in the current scope.
-
-This is useful for tests that rely on globals defined in head.js files, or for
-scripts that are loaded as <script> tag in a window in rely on eachother's
-globals.
-
-If <path> is a relative path, then it must be relative to the file being
-checked by the rule.

testing/eslint-plugin-mozilla/docs/import-globals.rst

@@ -0,0 +1,20 @@
+.. _import-globals:
+
+==============
+import-globals
+==============
+
+Rule Details
+------------
+
+Parses a file for globals defined in various unique Mozilla ways.
+
+When a "import-globals-from <path>" comment is found in a file, then all globals
+from the file at <path> will be imported in the current scope. This will also
+operate recursively.
+
+This is useful for scripts that are loaded as <script> tag in a window and rely
+on each other's globals.
+
+If <path> is a relative path, then it must be relative to the file being
+checked by the rule.

testing/eslint-plugin-mozilla/docs/this-top-level-scope.rst

@@ -1,11 +0,0 @@
-.. _this-top-level-scope:
-
-====================
-this-top-level-scope
-====================
-
-Rule Details
-------------
-
-Treat top-level assignments like ``this.mumble = value`` as declaring
-a global.

testing/eslint-plugin-mozilla/lib/globals.js

@@ -0,0 +1,187 @@
+/**
+ * @fileoverview functions for scanning an AST for globals including
+ *               traversing referenced scripts.
+ * 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 path = require("path");
+const fs = require("fs");
+const helpers = require("./helpers");
+const escope = require("escope");
+const estraverse = require("estraverse");
+
+/**
+ * Parses a list of "name:boolean_value" or/and "name" options divided by comma or
+ * whitespace.
+ *
+ * This function was copied from eslint.js
+ *
+ * @param {string} string The string to parse.
+ * @param {Comment} comment The comment node which has the string.
+ * @returns {Object} Result map object of names and boolean values
+ */
+function parseBooleanConfig(string, comment) {
+  let items = {};
+
+  // Collapse whitespace around : to make parsing easier
+  string = string.replace(/\s*:\s*/g, ":");
+  // Collapse whitespace around ,
+  string = string.replace(/\s*,\s*/g, ",");
+
+  string.split(/\s|,+/).forEach(function(name) {
+    if (!name) {
+      return;
+    }
+
+    let pos = name.indexOf(":");
+    let value = undefined;
+    if (pos !== -1) {
+      value = name.substring(pos + 1, name.length);
+      name = name.substring(0, pos);
+    }
+
+    items[name] = {
+      value: (value === "true"),
+      comment: comment
+    };
+  });
+
+  return items;
+}
+
+/**
+ * Global discovery can require parsing many files. This map of
+ * {String} => {Object} caches what globals were discovered for a file path.
+ */
+const globalCache = new Map();
+
+/**
+ * An object that returns found globals for given AST node types. Each prototype
+ * property should be named for a node type and accepts a node parameter and a
+ * parents parameter which is a list of the parent nodes of the current node.
+ * Each returns an array of globals found.
+ *
+ * @param  {String} path
+ *         The absolute path of the file being parsed.
+ */
+function GlobalsForNode(path) {
+  this.path = path;
+}
+
+GlobalsForNode.prototype = {
+  BlockComment(node, parents) {
+    let value = node.value.trim();
+    let match = /^import-globals-from\s+(.+)$/.exec(value);
+    if (!match) {
+      return [];
+    }
+
+    let filePath = match[1].trim();
+
+    if (!path.isAbsolute(filePath)) {
+      let dirName = path.dirname(this.path);
+      filePath = path.resolve(dirName, filePath);
+    }
+
+    return module.exports.getGlobalsForFile(filePath);
+  },
+
+  ExpressionStatement(node, parents) {
+    let isGlobal = helpers.getIsGlobalScope(parents);
+    let name = helpers.convertExpressionToGlobal(node, isGlobal);
+    return name ? [{ name, writable: true}] : [];
+  },
+};
+
+module.exports = {
+  /**
+   * Returns all globals for a given file. Recursively searches through
+   * import-globals-from directives and also includes globals defined by
+   * standard eslint directives.
+   *
+   * @param  {String} path
+   *         The absolute path of the file to be parsed.
+   */
+  getGlobalsForFile(path) {
+    if (globalCache.has(path)) {
+      return globalCache.get(path);
+    }
+
+    let content = fs.readFileSync(path, "utf8");
+
+    // Parse the content into an AST
+    let ast = helpers.getAST(content);
+
+    // Discover global declarations
+    let scopeManager = escope.analyze(ast);
+    let globalScope = scopeManager.acquire(ast);
+
+    let globals = Object.keys(globalScope.variables).map(v => ({
+      name: globalScope.variables[v].name,
+      writable: true,
+    }));
+
+    // Walk over the AST to find any of our custom globals
+    let handler = new GlobalsForNode(path);
+
+    helpers.walkAST(ast, (type, node, parents) => {
+      // We have to discover any globals that ESLint would have defined through
+      // comment directives
+      if (type == "BlockComment") {
+        let value = node.value.trim();
+        let match = /^globals?\s+(.+)$/.exec(value);
+        if (match) {
+          let values = parseBooleanConfig(match[1].trim(), node);
+          for (let name of Object.keys(values)) {
+            globals.push({
+              name,
+              writable: values[name].value
+            })
+          }
+        }
+      }
+
+      if (type in handler) {
+        let newGlobals = handler[type](node, parents);
+        globals.push.apply(globals, newGlobals);
+      }
+    });
+
+    globalCache.set(path, globals);
+
+    return globals;
+  },
+
+  /**
+   * Intended to be used as-is for an ESLint rule that parses for globals in
+   * the current file and recurses through import-globals-from directives.
+   *
+   * @param  {Object} context
+   *         The ESLint parsing context.
+   */
+  getESLintGlobalParser(context) {
+    let globalScope;
+
+    let parser = {
+      Program(node) {
+        globalScope = context.getScope();
+      }
+    };
+
+    // Install thin wrappers around GlobalsForNode
+    let handler = new GlobalsForNode(helpers.getAbsoluteFilePath(context));
+
+    for (let type of Object.keys(GlobalsForNode.prototype)) {
+      parser[type] = function(node) {
+        let globals = handler[type](node, context.getAncestors());
+        helpers.addGlobals(globals, globalScope);
+      }
+    }
+
+    return parser;
+  }
+};

testing/eslint-plugin-mozilla/lib/helpers.js

@@ -26,6 +26,7 @@ var definitions = [
   /^Object\.defineProperty\(this, "(\w+)"/,
   /^Reflect\.defineProperty\(this, "(\w+)"/,
   /^this\.__defineGetter__\("(\w+)"/,
+  /^this\.(\w+) =/
 ];
 
 var imports = [
@@ -83,17 +84,81 @@ module.exports = {
         return "function() {}";
       case "ArrowFunctionExpression":
         return "() => {}";
+      case "AssignmentExpression":
+        return this.getASTSource(node.left) + " = " + this.getASTSource(node.right);
       default:
         throw new Error("getASTSource unsupported node type: " + node.type);
     }
   },
 
+  /**
+   * This walks an AST in a manner similar to ESLint passing node and comment
+   * events to the listener. The listener is expected to be a simple function
+   * which accepts node type, node and parents arguments.
+   *
+   * @param  {Object} ast
+   *         The AST to walk.
+   * @param  {Function} listener
+   *         A callback function to call for the nodes. Passed three arguments,
+   *         event type, node and an array of parent nodes for the current node.
+   */
+  walkAST(ast, listener) {
+    let parents = [];
+
+    let seenComments = new Set();
+    function sendCommentEvents(comments) {
+      if (!comments) {
+        return;
+      }
+
+      for (let comment of comments) {
+        if (seenComments.has(comment)) {
+          return;
+        }
+        seenComments.add(comment);
+
+        listener(comment.type + "Comment", comment, parents);
+      }
+    }
+
+    estraverse.traverse(ast, {
+      enter(node, parent) {
+        // Comments are held in node.comments for empty programs
+        let leadingComments = node.leadingComments;
+        if (node.type === "Program" && node.body.length == 0) {
+          leadingComments = node.comments;
+        }
+
+        sendCommentEvents(leadingComments);
+        listener(node.type, node, parents);
+        sendCommentEvents(node.trailingComments);
+
+        parents.push(node);
+      },
+
+      leave(node, parent) {
+        // TODO send comment exit events
+        listener(node.type + ":exit", node, parents);
+
+        if (parents.length == 0) {
+          throw new Error("Left more nodes than entered.");
+        }
+        parents.pop();
+      }
+    });
+    if (parents.length) {
+      throw new Error("Entered more nodes than left.");
+    }
+  },
+
   /**
    * Attempts to convert an ExpressionStatement to a likely global variable
    * definition.
    *
    * @param  {Object} node
    *         The AST node to convert.
+   * @param  {boolean} isGlobal
+   *         True if the current node is in the global scope
    *
    * @return {String or null}
    *         The variable name defined.
@@ -133,90 +198,29 @@ module.exports = {
     return null;
   },
 
-  /**
-   * Walks over an AST and calls a callback for every ExpressionStatement found.
-   *
-   * @param  {Object} ast
-   *         The AST to traverse.
-   *
-   * @return {Function}
-   *         The callback to call for each ExpressionStatement.
-   */
-  expressionTraverse: function(ast, callback) {
-    var helpers = this;
-    var parents = new Map();
-
-    // Walk the parents of a node to see if any are functions
-    function isGlobal(node) {
-      var parent = parents.get(node);
-      while (parent) {
-        if (parent.type == "FunctionExpression" ||
-            parent.type == "FunctionDeclaration") {
-          return false;
-        }
-        parent = parents.get(parent);
-      }
-      return true;
-    }
-
-    estraverse.traverse(ast, {
-      enter: function(node, parent) {
-        parents.set(node, parent);
-
-        if (node.type == "ExpressionStatement") {
-          callback(node, isGlobal(node));
-        }
-      }
-    });
-  },
-
-  /**
-   * Get an array of globals from an AST.
-   *
-   * @param  {Object} ast
-   *         The AST for which the globals are to be returned.
-   *
-   * @return {Array}
-   *         An array of variable names.
-   */
-  getGlobals: function(ast) {
-    var scopeManager = escope.analyze(ast);
-    var globalScope = scopeManager.acquire(ast);
-    var result = [];
-
-    for (var variable in globalScope.variables) {
-      var name = globalScope.variables[variable].name;
-      result.push(name);
-    }
-
-    var helpers = this;
-    this.expressionTraverse(ast, function(node, isGlobal) {
-      var name = helpers.convertExpressionToGlobal(node, isGlobal);
-
-      if (name) {
-        result.push(name);
-      }
-    });
-
-    return result;
-  },
-
   /**
    * Add a variable to the current scope.
    * HACK: This relies on eslint internals so it could break at any time.
    *
    * @param {String} name
-   *        The variable name to add to the current scope.
-   * @param {ASTContext} context
-   *        The current context.
+   *        The variable name to add to the scope.
+   * @param {ASTScope} scope
+   *        The scope to add to.
+   * @param {boolean} writable
+   *        Whether the global can be overwritten.
    */
-  addVarToScope: function(name, context) {
-    var scope = context.getScope();
+  addVarToScope: function(name, scope, writable) {
+    // If the variable is already defined then skip it
+    if (scope.set && scope.set.has(name)) {
+      return;
+    }
+
+    writable = writable === undefined ? true : writable;
     var variables = scope.variables;
     var variable = new escope.Variable(name, scope);
 
     variable.eslintExplicitGlobal = false;
-    variable.writeable = true;
+    variable.writeable = writable;
     variables.push(variable);
 
     // Since eslint 1.10.3, scope variables are now duplicated in the scope.set
@@ -227,88 +231,16 @@ module.exports = {
     }
   },
 
-  // Caches globals found in a file so we only have to parse a file once.
-  globalCache: new Map(),
-
   /**
-   * Finds all the globals defined in a given file.
-   *
-   * @param {String} fileName
-   *        The file to parse for globals.
-   */
-  getGlobalsForFile: function(fileName) {
-    // If the file can't be found, let the error go up to the caller so it can
-    // be logged as an error in the current file.
-    var content = fs.readFileSync(fileName, "utf8");
-
-    if (this.globalCache.has(fileName)) {
-      return this.globalCache.get(fileName);
-    }
-
-    // Parse the content and get the globals from the ast.
-    var ast = this.getAST(content);
-    var globalVars = this.getGlobals(ast);
-    this.globalCache.set(fileName, globalVars);
-
-    return globalVars;
-  },
-
-  /**
-   * Adds a set of globals to a context.
+   * Adds a set of globals to a scope.
    *
    * @param {Array} globalVars
    *        An array of global variable names.
-   * @param {ASTContext} context
-   *        The current context.
+   * @param {ASTScope} scope
+   *        The scope.
    */
-  addGlobals: function(globalVars, context) {
-    for (var i = 0; i < globalVars.length; i++) {
-      var varName = globalVars[i];
-      this.addVarToScope(varName, context);
-    }
-  },
-
-  /**
-   * Process comments looking for import-globals-from statements.  Add globals
-   * from those files, if any.
-   *
-   * @param {String} currentFilePath
-   *        Absolute path to the file containing the comments.
-   * @param {Array} comments
-   *        The comments to be processed.
-   * @param {Object} node
-   *        The AST node for error reporting.
-   * @param {ASTContext} context
-   *        The current context.
-   */
-  addGlobalsFromComments: function(currentFilePath, comments, node, context) {
-    comments.forEach(comment => {
-      var value = comment.value.trim();
-      var match = /^import-globals-from\s+(.*)$/.exec(value);
-
-      if (match) {
-        var filePath = match[1];
-
-        if (!path.isAbsolute(filePath)) {
-          var dirName = path.dirname(currentFilePath);
-          filePath = path.resolve(dirName, filePath);
-        }
-
-        try {
-          let globals = this.getGlobalsForFile(filePath);
-          this.addGlobals(globals, context);
-        } catch (e) {
-          context.report(
-            node,
-            "Could not load globals from file {{filePath}}: {{error}}",
-            {
-              filePath: filePath,
-              error: e
-            }
-          );
-        }
-      }
-    });
+  addGlobals: function(globalVars, scope) {
+    globalVars.forEach(v => this.addVarToScope(v.name, scope, v.writable));
   },
 
   /**
@@ -321,6 +253,7 @@ module.exports = {
   getPermissiveConfig: function() {
     return {
       comment: true,
+      attachComment: true,
       range: true,
       loc: true,
       tolerant: true,
@@ -354,21 +287,20 @@ module.exports = {
   /**
    * Check whether the context is the global scope.
    *
-   * @param {ASTContext} context
-   *        The current context.
+   * @param {Array} ancestors
+   *        The parents of the current node.
    *
    * @return {Boolean}
    *         True or false
    */
-  getIsGlobalScope: function(context) {
-    var ancestors = context.getAncestors();
-    var parent = ancestors.pop();
-
-    if (parent.type == "ExpressionStatement") {
-      parent = ancestors.pop();
+  getIsGlobalScope: function(ancestors) {
+    for (let parent of ancestors) {
+      if (parent.type == "FunctionExpression" ||
+          parent.type == "FunctionDeclaration") {
+        return false;
+      }
     }
-
-    return parent.type == "Program";
+    return true;
   },
 
   /**

testing/eslint-plugin-mozilla/lib/index.js

@@ -18,28 +18,24 @@ module.exports = {
   },
   rules: {
     "balanced-listeners": require("../lib/rules/balanced-listeners"),
-    "components-imports": require("../lib/rules/components-imports"),
-    "import-globals-from": require("../lib/rules/import-globals-from"),
+    "import-globals": require("../lib/rules/import-globals"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "import-browserjs-globals": require("../lib/rules/import-browserjs-globals"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "no-aArgs": require("../lib/rules/no-aArgs"),
     "no-cpows-in-tests": require("../lib/rules/no-cpows-in-tests"),
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
-    "this-top-level-scope": require("../lib/rules/this-top-level-scope.js"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
   },
   rulesConfig: {
     "balanced-listeners": 0,
-    "components-imports": 0,
-    "import-globals-from": 0,
+    "import-globals": 0,
     "import-headjs-globals": 0,
     "import-browserjs-globals": 0,
     "mark-test-function-used": 0,
     "no-aArgs": 0,
     "no-cpows-in-tests": 0,
     "reject-importGlobalProperties": 0,
-    "this-top-level-scope": 0,
     "var-only-at-top-level": 0
   }
 };

testing/eslint-plugin-mozilla/lib/rules/components-imports.js

@@ -1,33 +0,0 @@
-/**
- * @fileoverview Adds the filename of imported files e.g.
- * Cu.import("some/path/Blah.jsm") adds Blah to the current scope.
- *
- * 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
-// -----------------------------------------------------------------------------
-
-var helpers = require("../helpers");
-
-module.exports = function(context) {
-  // ---------------------------------------------------------------------------
-  // Public
-  // ---------------------------------------------------------------------------
-
-  return {
-    ExpressionStatement: function(node) {
-      var scope = context.getScope();
-      var name = helpers.convertExpressionToGlobal(node, scope.type == "global");
-
-      if (name) {
-        helpers.addVarToScope(name, context);
-      }
-    }
-  };
-};

testing/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js

@@ -15,6 +15,7 @@
 var fs = require("fs");
 var path = require("path");
 var helpers = require("../helpers");
+var globals = require("../globals");
 
 const SCRIPTS = [
   "toolkit/components/printing/content/printUtils.js",
@@ -60,10 +61,9 @@ module.exports = function(context) {
       for (let script of SCRIPTS) {
         let fileName = path.join(root, script);
         try {
-          let globals = helpers.getGlobalsForFile(fileName);
-          helpers.addGlobals(globals, context);
-        }
-        catch (e) {
+          let newGlobals = globals.getGlobalsForFile(fileName);
+          helpers.addGlobals(newGlobals, context.getScope());
+        } catch (e) {
           context.report(
             node,
             "Could not load globals from file {{filePath}}: {{error}}",

testing/eslint-plugin-mozilla/lib/rules/import-globals-from.js

@@ -1,33 +0,0 @@
-/**
- * @fileoverview When the "import-globals-from: <path>" comment is found in a
- * file, then all globals from the file at <path> will be imported in the
- * current scope.
- *
- * 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
-// -----------------------------------------------------------------------------
-
-var fs = require("fs");
-var helpers = require("../helpers");
-var path = require("path");
-
-module.exports = function(context) {
-  // ---------------------------------------------------------------------------
-  // Public
-  // ---------------------------------------------------------------------------
-
-  return {
-    Program: function(node) {
-      var comments = context.getSourceCode().getAllComments();
-      var currentFilePath = helpers.getAbsoluteFilePath(context);
-      helpers.addGlobalsFromComments(currentFilePath, comments, node, context);
-    }
-  };
-};

testing/eslint-plugin-mozilla/lib/rules/import-globals.js

@@ -0,0 +1,15 @@
+/**
+ * @fileoverview Discovers all globals for the current file.
+ *
+ * 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
+// -----------------------------------------------------------------------------
+
+module.exports = require("../globals").getESLintGlobalParser;

testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js

@@ -16,6 +16,7 @@
 var fs = require("fs");
 var path = require("path");
 var helpers = require("../helpers");
+var globals = require("../globals");
 
 module.exports = function(context) {
 
@@ -36,13 +37,8 @@ module.exports = function(context) {
         return;
       }
 
-      let globals = helpers.getGlobalsForFile(fullHeadjsPath);
-      helpers.addGlobals(globals, context);
-
-      // Also add any globals from import-globals-from comments
-      var content = fs.readFileSync(fullHeadjsPath, "utf8");
-      var comments = helpers.getAST(content).comments;
-      helpers.addGlobalsFromComments(fullHeadjsPath, comments, node, context);
+      let newGlobals = globals.getGlobalsForFile(fullHeadjsPath);
+      helpers.addGlobals(newGlobals, context.getScope());
     }
   };
 };

testing/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js

@@ -56,7 +56,7 @@ module.exports = function(context) {
         }
         return false;
       });
-      if (!someCpowFound && helpers.getIsGlobalScope(context)) {
+      if (!someCpowFound && helpers.getIsGlobalScope(context.getAncestors())) {
         if (/^content\./.test(expression)) {
           showError(node, expression);
           return;

testing/eslint-plugin-mozilla/lib/rules/this-top-level-scope.js

@@ -1,33 +0,0 @@
-/**
- * @fileoverview Marks "this.var = x" as top-level definition of "var".
- *
- * 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
-// -----------------------------------------------------------------------------
-
-var helpers = require("../helpers");
-
-module.exports = function(context) {
-
-  // ---------------------------------------------------------------------------
-  // Public
-  //  --------------------------------------------------------------------------
-
-  return {
-    "AssignmentExpression": function(node) {
-      if (helpers.getIsGlobalScope(context) &&
-          node.left.type === "MemberExpression" &&
-          node.left.object.type === "ThisExpression" &&
-          node.left.property.type === "Identifier") {
-        helpers.addGlobals([node.left.property.name], context);
-      }
-    }
-  };
-};

testing/eslint-plugin-mozilla/lib/rules/var-only-at-top-level.js

@@ -23,7 +23,7 @@ module.exports = function(context) {
   return {
     "VariableDeclaration": function(node) {
       if (node.kind === "var") {
-        if (helpers.getIsGlobalScope(context)) {
+        if (helpers.getIsGlobalScope(context.getAncestors())) {
           return;
         }
 

toolkit/components/extensions/.eslintrc

@@ -29,8 +29,6 @@
   "rules": {
     // Rules from the mozilla plugin
     "mozilla/balanced-listeners": 2,
-    "mozilla/components-imports": 1,
-    "mozilla/import-headjs-globals": 1,
     "mozilla/mark-test-function-used": 1,
     "mozilla/no-aArgs": 1,
     "mozilla/no-cpows-in-tests": 1,