Bug 1312355: Use a fixed lookup table for variables imported from JS modules. r=standard8 draft
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 01 Nov 2016 12:24:04 -0700
changeset 432359 f114924e8fa6464e6f4f82730773e8f78fcbcde4
parent 432230 3e73fd638e687a4d7f46613586e5156b8e2af846
child 535621 216e0d78eaf2093b7ef5e4af4489388fb493a2ec
push id34279
push userdtownsend@mozilla.com
push dateTue, 01 Nov 2016 19:24:25 +0000
reviewersstandard8
bugs1312355
milestone52.0a1
Bug 1312355: Use a fixed lookup table for variables imported from JS modules. r=standard8 MozReview-Commit-ID: EhKhBvAAlXd
toolkit/mozapps/extensions/internal/XPIProvider.jsm
tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
tools/lint/eslint/eslint-plugin-mozilla/package.json
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -13,17 +13,16 @@ this.EXPORTED_SYMBOLS = ["XPIProvider"];
 
 const CONSTANTS = {};
 Cu.import("resource://gre/modules/addons/AddonConstants.jsm", CONSTANTS);
 const { ADDON_SIGNING, REQUIRE_SIGNING } = CONSTANTS
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/AddonManager.jsm");
-/* globals AddonManagerPrivate*/
 Cu.import("resource://gre/modules/Preferences.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository",
                                   "resource://gre/modules/addons/AddonRepository.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ChromeManifestParser",
                                   "resource://gre/modules/ChromeManifestParser.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
@@ -65,16 +65,17 @@ const globalCache = new Map();
  * 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;
+  this.root = helpers.getRootDir(path);
 }
 
 GlobalsForNode.prototype = {
   BlockComment(node, parents) {
     let value = node.value.trim();
     let match = /^import-globals-from\s+(.+)$/.exec(value);
     if (!match) {
       return [];
@@ -87,18 +88,18 @@ GlobalsForNode.prototype = {
       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}] : [];
+    let names = helpers.convertExpressionToGlobals(node, isGlobal, this.root);
+    return names.map(name => { return { 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.
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -7,16 +7,18 @@
 "use strict";
 
 var escope = require("escope");
 var espree = require("espree");
 var estraverse = require("estraverse");
 var path = require("path");
 var fs = require("fs");
 
+var modules = null;
+
 var definitions = [
   /^loader\.lazyGetter\(this, "(\w+)"/,
   /^loader\.lazyImporter\(this, "(\w+)"/,
   /^loader\.lazyServiceGetter\(this, "(\w+)"/,
   /^loader\.lazyRequireGetter\(this, "(\w+)"/,
   /^XPCOMUtils\.defineLazyGetter\(this, "(\w+)"/,
   /^XPCOMUtils\.defineLazyModuleGetter\(this, "(\w+)"/,
   /^XPCOMUtils\.defineLazyServiceGetter\(this, "(\w+)"/,
@@ -25,17 +27,17 @@ var definitions = [
   /^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)?\)/,
+  /^(?:Cu|Components\.utils)\.import\(".*\/((.*?)\.jsm?)"(?:, this)?\)/,
 ];
 
 module.exports = {
   /**
    * Gets the abstract syntax tree (AST) of the JavaScript source code contained
    * in sourceText.
    *
    * @param  {String} sourceText
@@ -147,60 +149,70 @@ module.exports = {
       }
     });
     if (parents.length) {
       throw new Error("Entered more nodes than left.");
     }
   },
 
   /**
-   * Attempts to convert an ExpressionStatement to a likely global variable
-   * definition.
+   * Attempts to convert an ExpressionStatement to likely global variable
+   * definitions.
    *
    * @param  {Object} node
    *         The AST node to convert.
    * @param  {boolean} isGlobal
-   *         True if the current node is in the global scope
+   *         True if the current node is in the global scope.
+   * @param  {String} repository
+   *         The root of the repository.
    *
-   * @return {String or null}
-   *         The variable name defined.
+   * @return {Array}
+   *         An array of variable names defined.
    */
-  convertExpressionToGlobal: function(node, isGlobal) {
+  convertExpressionToGlobals: function(node, isGlobal, repository) {
+    if (!modules) {
+      modules = require(path.join(repository, "tools", "lint", "eslint", "modules.json"));
+    }
+
     try {
       var source = this.getASTSource(node);
     }
     catch (e) {
-      return null;
+      return [];
     }
 
     for (var reg of definitions) {
       var match = source.match(reg);
       if (match) {
         // Must be in the global scope
         if (!isGlobal) {
-          return null;
+          return [];
         }
 
-        return match[1];
+        return [match[1]];
       }
     }
 
     for (reg of imports) {
       var match = source.match(reg);
       if (match) {
         // The two argument form is only acceptable in the global scope
         if (node.expression.arguments.length > 1 && !isGlobal) {
-          return null;
+          return [];
         }
 
-        return match[1];
+        if (match[1] in modules) {
+          return modules[match[1]];
+        }
+
+        return [match[2]];
       }
     }
 
-    return null;
+    return [];
   },
 
   /**
    * 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 scope.
@@ -352,23 +364,22 @@ module.exports = {
     }
 
     return this.getIsBrowserMochitest(scope);
   },
 
   /**
    * Gets the root directory of the repository by walking up directories until
    * a .eslintignore file is found.
-   * @param {ASTContext} context
-   *        The current context.
+   * @param {String} fileName
+   *        The absolute path of a file in the repository
    *
    * @return {String} The absolute path of the repository directory
    */
-  getRootDir: function(context) {
-    var fileName = this.getAbsoluteFilePath(context);
+  getRootDir: function(fileName) {
     var dirName = path.dirname(fileName);
 
     while (dirName && !fs.existsSync(path.join(dirName, ".eslintignore"))) {
       dirName = path.dirname(dirName);
     }
 
     if (!dirName) {
       throw new Error("Unable to find root of repository");
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
@@ -55,17 +55,18 @@ const SCRIPTS = [
 module.exports = function(context) {
   return {
     Program: function(node) {
       if (!helpers.getIsBrowserMochitest(this) &&
           !helpers.getIsHeadFile(this)) {
         return;
       }
 
-      let root = helpers.getRootDir(context);
+      let filepath = helpers.getAbsoluteFilePath(context);
+      let root = helpers.getRootDir(filepath);
       for (let script of SCRIPTS) {
         let fileName = path.join(root, script);
         try {
           let newGlobals = globals.getGlobalsForFile(fileName);
           helpers.addGlobals(newGlobals, context.getScope());
         } catch (e) {
           context.report(
             node,
--- a/tools/lint/eslint/eslint-plugin-mozilla/package.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package.json
@@ -1,11 +1,11 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.2.3",
+  "version": "0.2.4",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],