Bug 1229859 - Introduce new import-globals-from eslint rule to import globals from other modules; r=Mossop
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 04 Dec 2015 11:21:21 +0100
changeset 297738 760f2be9b5490933e5c64dfab9e39b620b57c945
parent 297737 c857a46087009d56163839700e1b4484fb672613
child 297739 66b69aaa8cc3610e7cf2359b16064e4b52d72035
push id8824
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:18:56 +0000
treeherdermozilla-aurora@e2031358e2a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMossop
bugs1229859
milestone45.0a1
Bug 1229859 - Introduce new import-globals-from eslint rule to import globals from other modules; r=Mossop
devtools/.eslintrc
devtools/client/animationinspector/animation-panel.js
testing/eslint-plugin-mozilla/docs/import-globals-from.rst
testing/eslint-plugin-mozilla/docs/index.rst
testing/eslint-plugin-mozilla/lib/helpers.js
testing/eslint-plugin-mozilla/lib/index.js
testing/eslint-plugin-mozilla/lib/rules/import-globals-from.js
testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
--- a/devtools/.eslintrc
+++ b/devtools/.eslintrc
@@ -20,16 +20,17 @@
   },
   "rules": {
     // These are the rules that have been configured so far to match the
     // devtools coding style.
 
     // Rules from the mozilla plugin
     "mozilla/balanced-listeners": 2,
     "mozilla/components-imports": 1,
+    "mozilla/import-globals-from": 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,
 
     // Disallow using variables outside the blocks they are defined (especially
     // since only let and const are used, see "no-var").
--- a/devtools/client/animationinspector/animation-panel.js
+++ b/devtools/client/animationinspector/animation-panel.js
@@ -1,14 +1,15 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* 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/. */
-/* globals AnimationsController, document, promise, gToolbox, gInspector */
+/* import-globals-from animation-controller.js */
+/* globals document */
 
 "use strict";
 
 const {
   AnimationsTimeline,
   RateSelector
 } = require("devtools/client/animationinspector/components");
 const {formatStopwatchTime} = require("devtools/client/animationinspector/utils");
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/docs/import-globals-from.rst
@@ -0,0 +1,18 @@
+.. _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.
--- a/testing/eslint-plugin-mozilla/docs/index.rst
+++ b/testing/eslint-plugin-mozilla/docs/index.rst
@@ -5,16 +5,20 @@ Mozilla ESLint Plugin
 =====================
 
 ``balanced-listeners`` checks that every addEventListener has a
 removeEventListener (and does the same for on/off).
 
 ``components-imports`` adds the filename of imported files e.g.
 ``Cu.import("some/path/Blah.jsm")`` adds Blah to the global scope.
 
+``import-globals-from`` 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.
+
 ``import-headjs-globals`` imports globals from head.js and from any files that
 should be imported by head.js (as far as we can correctly resolve the path).
 
 ``mark-test-function-used`` simply marks test (the test method) as used. This
 avoids ESLint telling us that the function is never called.
 
 ``no-aArgs`` prevents using the hungarian notation in function arguments.
 
@@ -42,24 +46,26 @@ level invalid.
 | 2     | Error                 |
 +-------+-----------------------+
 
 Example configuration::
 
    "rules": {
      "mozilla/balanced-listeners": 2,
      "mozilla/components-imports": 1,
+     "mozilla/import-globals-from": 1,
      "mozilla/import-headjs-globals": 1,
      "mozilla/mark-test-function-used": 1,
      "mozilla/var-only-at-top-level": 1,
      "mozilla/no-cpows-in-tests": 1,
    }
 
 .. toctree::
    :maxdepth: 1
 
    balanced-listeners
    components-imports
+   import-globals-from
    import-headjs-globals
    mark-test-function-used
    no-aArgs
    no-cpows-in-tests
    var-only-at-top-level
--- a/testing/eslint-plugin-mozilla/lib/helpers.js
+++ b/testing/eslint-plugin-mozilla/lib/helpers.js
@@ -3,16 +3,17 @@
  * 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";
 
 var escope = require("escope");
 var espree = require("espree");
+var path = require("path");
 
 var regexes = [
   /^(?:Cu|Components\.utils)\.import\(".*\/(.*?)\.jsm?"\);?$/,
   /^loader\.lazyImporter\(\w+, "(\w+)"/,
   /^loader\.lazyRequireGetter\(\w+, "(\w+)"/,
   /^loader\.lazyServiceGetter\(\w+, "(\w+)"/,
   /^XPCOMUtils\.defineLazyModuleGetter\(\w+, "(\w+)"/,
   /^loader\.lazyGetter\(\w+, "(\w+)"/,
@@ -227,10 +228,38 @@ module.exports = {
    *
    * @return {Boolean}
    *         True or false
    */
   getIsBrowserMochitest: function(scope) {
     var pathAndFilename = scope.getFilename();
 
     return /.*[\\/]browser_.+\.js$/.test(pathAndFilename);
+  },
+
+  /**
+   * ESLint may be executed from various places: from mach, at the root of the
+   * repository, or from a directory in the repository when, for instance,
+   * executed by a text editor's plugin.
+   * The value returned by context.getFileName() varies because of this.
+   * This helper function makes sure to return an absolute file path for the
+   * current context, by looking at process.cwd().
+   * @param {Context} context
+   * @return {String} The absolute path
+   */
+  getAbsoluteFilePath: function(context) {
+    var fileName = context.getFilename();
+    var cwd = process.cwd();
+
+    if (path.isAbsolute(fileName)) {
+      // Case 2: executed from the repo's root with mach:
+      //   fileName: /path/to/mozilla/repo/a/b/c/d.js
+      //   cwd: /path/to/mozilla/repo
+      return fileName;
+    } else {
+      // Case 1: executed form in a nested directory, e.g. from a text editor:
+      //   fileName: a/b/c/d.js
+      //   cwd: /path/to/mozilla/repo/a/b/c
+      var dirName = path.dirname(fileName);
+      return cwd.slice(0, cwd.length - dirName.length) + fileName;
+    }
   }
 };
--- a/testing/eslint-plugin-mozilla/lib/index.js
+++ b/testing/eslint-plugin-mozilla/lib/index.js
@@ -14,24 +14,26 @@
 
 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-headjs-globals": require("../lib/rules/import-headjs-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"),
     "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-headjs-globals": 0,
     "mark-test-function-used": 0,
     "no-aArgs": 0,
     "no-cpows-in-tests": 0,
     "var-only-at-top-level": 0
   }
 };
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/lib/rules/import-globals-from.js
@@ -0,0 +1,78 @@
+/**
+ * @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) {
+  // ---------------------------------------------------------------------------
+  // Helpers
+  // ---------------------------------------------------------------------------
+
+  function importGlobalsFrom(filePath) {
+    // 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(filePath, "utf8");
+
+    // Parse the content and get the globals from the ast.
+    var ast = helpers.getAST(content);
+    var globalVars = helpers.getGlobals(ast);
+
+    for (var i = 0; i < globalVars.length; i++) {
+      var varName = globalVars[i];
+      helpers.addVarToScope(varName, context);
+    }
+  }
+
+  // ---------------------------------------------------------------------------
+  // Public
+  // ---------------------------------------------------------------------------
+
+  return {
+    Program: function(node) {
+      var comments = context.getSourceCode().getAllComments();
+
+      comments.forEach(function(comment) {
+        var value = comment.value.trim();
+        var match = /^import-globals-from\s+(.*)$/.exec(value);
+
+        if (match) {
+          var currentFilePath = helpers.getAbsoluteFilePath(context);
+          var filePath = match[1];
+
+          if (!path.isAbsolute(filePath)) {
+            var dirName = path.dirname(currentFilePath);
+            filePath = path.resolve(dirName, filePath);
+          }
+
+          try {
+            importGlobalsFrom(filePath);
+          } catch (e) {
+            context.report(
+              node,
+              "Could not load globals from file {{filePath}}: {{error}}",
+              {
+                filePath: filePath,
+                error: e
+              }
+            );
+          }
+        }
+      });
+    }
+  };
+};
--- a/testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
+++ b/testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js
@@ -86,17 +86,16 @@ module.exports = function(context) {
   // ---------------------------------------------------------------------------
 
   return {
     Program: function() {
       if (!helpers.getIsBrowserMochitest(this)) {
         return;
       }
 
-      var testPath = this.getFilename();
-      var testFilename = path.basename(testPath);
-      var fullTestPath = path.resolve(testFilename);
-      var fullHeadjsPath = path.resolve("head.js");
+      var currentFilePath = helpers.getAbsoluteFilePath(context);
+      var dirName = path.dirname(currentFilePath);
+      var fullHeadjsPath = path.resolve(dirName, "head.js");
 
-      checkFile([fullTestPath, fullHeadjsPath]);
+      checkFile([currentFilePath, fullHeadjsPath]);
     }
   };
 };