Bug 1245916: Unify eslint global discovery rules. r=pbrosset draft
authorDave Townsend <dtownsend@oxymoronical.com>
Fri, 05 Feb 2016 11:37:50 -0800
changeset 331372 0f1cceca29ac398be97a55bbcd09fe58a8be1435
parent 331355 b11a13c9b75082b4beee2e738e23da8f5f917c4d
child 331373 91a60ef0359ef53093fe197ed63dbc4e1a9f10a5
push id10969
push userdtownsend@mozilla.com
push dateTue, 16 Feb 2016 22:40:19 +0000
reviewerspbrosset
bugs1245916
milestone47.0a1
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
testing/eslint-plugin-mozilla/docs/components-imports.rst
testing/eslint-plugin-mozilla/docs/import-globals-from.rst
testing/eslint-plugin-mozilla/docs/import-globals.rst
testing/eslint-plugin-mozilla/docs/this-top-level-scope.rst
testing/eslint-plugin-mozilla/lib/globals.js
testing/eslint-plugin-mozilla/lib/helpers.js
testing/eslint-plugin-mozilla/lib/index.js
testing/eslint-plugin-mozilla/lib/rules/components-imports.js
testing/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
testing/eslint-plugin-mozilla/lib/rules/import-globals-from.js
testing/eslint-plugin-mozilla/lib/rules/import-globals.js
testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
testing/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
testing/eslint-plugin-mozilla/lib/rules/this-top-level-scope.js
testing/eslint-plugin-mozilla/lib/rules/var-only-at-top-level.js
toolkit/components/extensions/.eslintrc
--- a/.eslintrc
+++ b/.eslintrc
@@ -1,14 +1,12 @@
 {
   // When adding items to this file please check for effects on sub-directories.
   "plugins": [
     "mozilla"
   ],
   "rules": {
-    "mozilla/components-imports": 1,
-    "mozilla/import-globals-from": 1,
-    "mozilla/this-top-level-scope": 1,
+    "mozilla/import-globals": 1,
   },
   "env": {
     "es6": true
   },
 }
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/docs/components-imports.rst
+++ /dev/null
@@ -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"``
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/docs/import-globals-from.rst
+++ /dev/null
@@ -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.
new file mode 100644
--- /dev/null
+++ b/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.
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/docs/this-top-level-scope.rst
+++ /dev/null
@@ -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.
new file mode 100644
--- /dev/null
+++ b/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;
+  }
+};
--- a/testing/eslint-plugin-mozilla/lib/helpers.js
+++ b/testing/eslint-plugin-mozilla/lib/helpers.js
@@ -21,16 +21,17 @@ var definitions = [
   /^XPCOMUtils\.defineLazyModuleGetter\(this, "(\w+)"/,
   /^XPCOMUtils\.defineLazyServiceGetter\(this, "(\w+)"/,
   /^XPCOMUtils\.defineConstant\(this, "(\w+)"/,
   /^DevToolsUtils\.defineLazyModuleGetter\(this, "(\w+)"/,
   /^DevToolsUtils\.defineLazyGetter\(this, "(\w+)"/,
   /^Object\.defineProperty\(this, "(\w+)"/,
   /^Reflect\.defineProperty\(this, "(\w+)"/,
   /^this\.__defineGetter__\("(\w+)"/,
+  /^this\.(\w+) =/
 ];
 
 var imports = [
   /^(?:Cu|Components\.utils)\.import\(".*\/(.*?)\.jsm?"(?:, this)?\)/,
 ];
 
 module.exports = {
   /**
@@ -78,27 +79,91 @@ module.exports = {
       case "ObjectExpression":
         return "{}";
       case "ExpressionStatement":
         return this.getASTSource(node.expression) + ";";
       case "FunctionExpression":
         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.
    */
   convertExpressionToGlobal: function(node, isGlobal) {
     try {
       var source = this.getASTSource(node);
     }
@@ -129,203 +194,71 @@ module.exports = {
         return match[1];
       }
     }
 
     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
     // map, so we need to store them there too if it exists.
     // See https://groups.google.com/forum/#!msg/eslint/Y4_oHMWwP-o/5S57U8jXd8kJ
     if (scope.set) {
       scope.set.set(name, variable);
     }
   },
 
-  // 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.
-   */
-  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.
+   * @param {ASTScope} scope
+   *        The scope.
    */
-  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));
   },
 
   /**
    * To allow espree to parse almost any JavaScript we need as many features as
    * possible turned on. This method returns that config.
    *
    * @return {Object}
    *         Espree compatible permissive config.
    */
   getPermissiveConfig: function() {
     return {
       comment: true,
+      attachComment: true,
       range: true,
       loc: true,
       tolerant: true,
       ecmaFeatures: {
         arrowFunctions: true,
         binaryLiterals: true,
         blockBindings: true,
         classes: true,
@@ -349,31 +282,30 @@ module.exports = {
         unicodeCodePointEscapes: true,
       }
     };
   },
 
   /**
    * 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;
   },
 
   /**
    * Check whether we are in a browser mochitest.
    *
    * @param  {RuleContext} scope
    *         You should pass this from within a rule
    *         e.g. helpers.getIsBrowserMochitest(this)
--- a/testing/eslint-plugin-mozilla/lib/index.js
+++ b/testing/eslint-plugin-mozilla/lib/index.js
@@ -13,33 +13,29 @@
 //------------------------------------------------------------------------------
 
 module.exports = {
   processors: {
     ".xml": require("../lib/processors/xbl-bindings"),
   },
   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
   }
 };
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/lib/rules/components-imports.js
+++ /dev/null
@@ -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);
-      }
-    }
-  };
-};
--- a/testing/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
+++ b/testing/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
@@ -10,16 +10,17 @@
 
 // -----------------------------------------------------------------------------
 // Rule Definition
 // -----------------------------------------------------------------------------
 
 var fs = require("fs");
 var path = require("path");
 var helpers = require("../helpers");
+var globals = require("../globals");
 
 const SCRIPTS = [
   "toolkit/components/printing/content/printUtils.js",
   "toolkit/content/viewZoomOverlay.js",
   "browser/components/places/content/browserPlacesViews.js",
   "browser/base/content/browser.js",
   "browser/components/downloads/content/downloads.js",
   "browser/components/downloads/content/indicator.js",
@@ -55,20 +56,19 @@ module.exports = function(context) {
       if (!helpers.getIsBrowserMochitest(this)) {
         return;
       }
 
       let root = helpers.getRootDir(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}}",
             {
               filePath: path.relative(root, fileName),
               error: e
             }
           );
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/lib/rules/import-globals-from.js
+++ /dev/null
@@ -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);
-    }
-  };
-};
new file mode 100644
--- /dev/null
+++ b/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;
--- a/testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
+++ b/testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
@@ -11,16 +11,17 @@
 
 // -----------------------------------------------------------------------------
 // Rule Definition
 // -----------------------------------------------------------------------------
 
 var fs = require("fs");
 var path = require("path");
 var helpers = require("../helpers");
+var globals = require("../globals");
 
 module.exports = function(context) {
 
   // ---------------------------------------------------------------------------
   // Public
   // ---------------------------------------------------------------------------
 
   return {
@@ -31,18 +32,13 @@ module.exports = function(context) {
 
       var currentFilePath = helpers.getAbsoluteFilePath(context);
       var dirName = path.dirname(currentFilePath);
       var fullHeadjsPath = path.resolve(dirName, "head.js");
       if (!fs.existsSync(fullHeadjsPath)) {
         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());
     }
   };
 };
--- a/testing/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
+++ b/testing/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
@@ -51,17 +51,17 @@ module.exports = function(context) {
       // |cpows| reports one, don't report another below.
       var someCpowFound = cpows.some(function(cpow) {
         if (cpow.test(expression)) {
           showError(node, expression);
           return true;
         }
         return false;
       });
-      if (!someCpowFound && helpers.getIsGlobalScope(context)) {
+      if (!someCpowFound && helpers.getIsGlobalScope(context.getAncestors())) {
         if (/^content\./.test(expression)) {
           showError(node, expression);
           return;
         }
       }
     },
 
     Identifier: function(node) {
deleted file mode 100644
--- a/testing/eslint-plugin-mozilla/lib/rules/this-top-level-scope.js
+++ /dev/null
@@ -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);
-      }
-    }
-  };
-};
--- a/testing/eslint-plugin-mozilla/lib/rules/var-only-at-top-level.js
+++ b/testing/eslint-plugin-mozilla/lib/rules/var-only-at-top-level.js
@@ -18,17 +18,17 @@ var helpers = require("../helpers");
 module.exports = function(context) {
   // ---------------------------------------------------------------------------
   // Public
   //  --------------------------------------------------------------------------
 
   return {
     "VariableDeclaration": function(node) {
       if (node.kind === "var") {
-        if (helpers.getIsGlobalScope(context)) {
+        if (helpers.getIsGlobalScope(context.getAncestors())) {
           return;
         }
 
         context.report(node, "Unexpected var, use let or const instead.");
       }
     }
   };
 };
--- a/toolkit/components/extensions/.eslintrc
+++ b/toolkit/components/extensions/.eslintrc
@@ -24,18 +24,16 @@
     "Services": true,
     "TabManager": true,
     "XPCOMUtils": true,
   },
 
   "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,
     "mozilla/var-only-at-top-level": 1,
 
     // Braces only needed for multi-line arrow function blocks
     // "arrow-body-style": [2, "as-needed"],