Bug 1343519 - Change the ESLint rule 'import-browserjs-globals' to be an environment (mozilla/browser-window) to better describe the purpose of it. r=mossop
authorMark Banner <standard8@mozilla.com>
Wed, 01 Mar 2017 21:29:52 +0000
changeset 394488 563299ade2aee43b83defceefef58c43ec815415
parent 394487 64ebe011392939153e6ca39814e90cfb33b1e813
child 394489 8dbddf1d169e67ed97ea852013bbac2269d01aaa
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs1343519
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1343519 - Change the ESLint rule 'import-browserjs-globals' to be an environment (mozilla/browser-window) to better describe the purpose of it. r=mossop MozReview-Commit-ID: FTDV8BcMGeF
browser/extensions/pocket/content/main.js
testing/mochitest/browser.eslintrc.js
testing/mochitest/chrome.eslintrc.js
toolkit/components/printing/content/printUtils.js
toolkit/components/prompts/content/tabprompts.xml
toolkit/content/.eslintrc.js
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/environments/browser-window.js
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
--- a/browser/extensions/pocket/content/main.js
+++ b/browser/extensions/pocket/content/main.js
@@ -37,17 +37,17 @@
 
 // TODO : Get the toolbar icons from Firefox's build (Nikki needs to give us a red saved icon)
 // TODO : [needs clarificaiton from Fx] Firefox's plan was to hide Pocket from context menus until the user logs in. Now that it's an extension I'm wondering if we still need to do this.
 // TODO : [needs clarificaiton from Fx] Reader mode (might be a something they need to do since it's in html, need to investigate their code)
 // TODO : [needs clarificaiton from Fx] Move prefs within pktApi.s to sqlite or a local file so it's not editable (and is safer)
 // TODO : [nice to have] - Immediately save, buffer the actions in a local queue and send (so it works offline, works like our native extensions)
 
 /* eslint-disable no-shadow */
-/* eslint "mozilla/import-browserjs-globals": "error" */
+/* eslint-env mozilla/browser-window */
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ReaderMode",
   "resource://gre/modules/ReaderMode.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "pktApi",
   "chrome://pocket/content/pktApi.jsm");
--- a/testing/mochitest/browser.eslintrc.js
+++ b/testing/mochitest/browser.eslintrc.js
@@ -1,22 +1,26 @@
 // Parent config file for all browser-chrome files.
 module.exports = {
   "rules": {
     "mozilla/import-headjs-globals": "warn",
-    "mozilla/import-browserjs-globals": "warn",
     "mozilla/import-test-globals": "warn",
     "mozilla/mark-test-function-used": "warn",
   },
 
   "env": {
     "browser": true,
+    "mozilla/browser-window": true,
     //"node": true
   },
 
+  "plugins": [
+    "mozilla"
+  ],
+
   // All globals made available in the test environment.
   "globals": {
     // `$` is defined in SimpleTest.js
     "$": false,
     "add_task": false,
     "addLoadEvent": false,
     "Assert": false,
     "BrowserTestUtils": false,
--- a/testing/mochitest/chrome.eslintrc.js
+++ b/testing/mochitest/chrome.eslintrc.js
@@ -1,21 +1,25 @@
 // Parent config file for all mochitest files.
 module.exports = {
   rules: {
     "mozilla/import-headjs-globals": "warn",
-    "mozilla/import-browserjs-globals": "warn",
     "mozilla/import-test-globals": "warn",
     "mozilla/mark-test-function-used": "warn",
   },
 
   "env": {
     "browser": true,
+    "mozilla/browser-window": true,
   },
 
+  "plugins": [
+    "mozilla"
+  ],
+
   // All globals made available in the test environment.
   "globals": {
     // `$` is defined in SimpleTest.js
     "$": false,
     "add_task": false,
     "addLoadEvent": false,
     "Assert": false,
     "BrowserTestUtils": false,
--- a/toolkit/components/printing/content/printUtils.js
+++ b/toolkit/components/printing/content/printUtils.js
@@ -1,10 +1,10 @@
 // This file is loaded into the browser window scope.
-/* eslint mozilla/import-browserjs-globals:"warn" */
+/* eslint-env mozilla/browser-window */
 
 // -*- tab-width: 2; indent-tabs-mode: nil; js-indent-level: 2 -*-
 
 /* 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/. */
 
 /**
--- a/toolkit/components/prompts/content/tabprompts.xml
+++ b/toolkit/components/prompts/content/tabprompts.xml
@@ -1,16 +1,16 @@
 <?xml version="1.0"?>
 <!-- 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/. -->
 
 <!-- This file is imported into the browser window, and expects various variables,
      e.g. Ci, Services, to be available. -->
-<!-- eslint mozilla/import-browserjs-globals:"warn" -->
+<!-- eslint-env mozilla/browser-window -->
 
 <!DOCTYPE bindings [
 <!ENTITY % commonDialogDTD  SYSTEM "chrome://global/locale/commonDialog.dtd">
 <!ENTITY % dialogOverlayDTD SYSTEM "chrome://global/locale/dialogOverlay.dtd">
 %commonDialogDTD;
 %dialogOverlayDTD;
 ]>
 
--- a/toolkit/content/.eslintrc.js
+++ b/toolkit/content/.eslintrc.js
@@ -1,7 +1,11 @@
 "use strict";
 
 module.exports = {
-  "rules": {
-    "mozilla/import-browserjs-globals": "warn",
-  }
+  "env": {
+    "mozilla/browser-window": true,
+  },
+
+  "plugins": [
+    "mozilla"
+  ],
 };
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -8,16 +8,21 @@ Environments
 These environments are available by specifying a comment at the top of the file,
 e.g.
 
    /* eslint-env mozilla/chrome-worker */
 
 There are also built-in ESLint environments available as well:
 http://eslint.org/docs/user-guide/configuring#specifying-environments
 
+browser-window
+--------------
+
+Defines the environment for scripts that are in the main browser.xul scope.
+
 chrome-worker
 -------------
 
 Defines the environment for chrome workers. This differs from normal workers by
 the fact that `ctypes` can be accessed as well.
 
 frame-script
 ------------
@@ -43,23 +48,16 @@ occurence of 'removeEventListener' or 'o
 import-globals
 --------------
 
 Checks the filename of imported files e.g. ``Cu.import("some/path/Blah.jsm")``
 adds Blah to the global scope.
 
 Note: uses modules.json for a list of globals listed in each file.
 
-import-browserjs-globals
-------------------------
-
-When included files from the main browser UI scripts will be loaded and any
-declared globals will be defined for the current file. This is mostly useful for
-browser-chrome mochitests that call browser functions.
-
 
 import-globals-from
 -------------------
 
 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
rename from tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
rename to tools/lint/eslint/eslint-plugin-mozilla/lib/environments/browser-window.js
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/browser-window.js
@@ -1,23 +1,23 @@
 /**
- * @fileoverview Import globals from browser.js.
+ * @fileoverview Defines the environment when in the browser.xul window.
+ *               Imports many globals from various files.
  *
  * 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 path = require("path");
 var helpers = require("../helpers");
 var globals = require("../globals");
 
 const SCRIPTS = [
   //"browser/base/content/nsContextMenu.js",
   "toolkit/content/contentAreaUtils.js",
   "browser/components/places/content/editBookmarkOverlay.js",
@@ -51,40 +51,34 @@ const SCRIPTS = [
   "browser/base/content/browser-fxaccounts.js",
   // This gets loaded into the same scopes as browser.js via browser.xul and
   // placesOverlay.xul.
   "toolkit/content/globalOverlay.js",
   // Via editMenuOverlay.xul
   "toolkit/content/editMenuOverlay.js"
 ];
 
-module.exports = function(context) {
-  return {
-    Program: function(node) {
-      let filepath = helpers.getAbsoluteFilePath(context);
-      let root = helpers.getRootDir(filepath);
-      let relativepath = path.relative(root, filepath);
-
-      if ((helpers.getTestType(context) != "browser" &&
-          !helpers.getIsHeadFile(context)) &&
-          !relativepath.includes("content")) {
-        return;
-      }
+function getScriptGlobals() {
+  let fileGlobals = [];
+  let root = helpers.getRootDir(module.filename);
+  for (let script of SCRIPTS) {
+    let fileName = path.join(root, script);
+    try {
+      fileGlobals = fileGlobals.concat(globals.getGlobalsForFile(fileName));
+    } catch (e) {
+      throw new Error(`Could not load globals from file ${fileName}: ${e}`)
+    }
+  }
 
-      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,
-            "Could not load globals from file {{filePath}}: {{error}}",
-            {
-              filePath: path.relative(root, fileName),
-              error: e
-            }
-          );
-        }
-      }
-    }
-  };
+  return fileGlobals;
+}
+
+function mapGlobals(fileGlobals) {
+  var globalObjects = {};
+  for (let global of fileGlobals) {
+    globalObjects[global.name] = global.writable;
+  }
+  return globalObjects;
+}
+
+module.exports = {
+  globals: mapGlobals(getScriptGlobals())
 };
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -6,30 +6,29 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
 "use strict";
 
 //------------------------------------------------------------------------------
 // Plugin Definition
 //------------------------------------------------------------------------------
-
 module.exports = {
   environments: {
+    "browser-window": require("../lib/environments/browser-window.js"),
     "chrome-worker": require("../lib/environments/chrome-worker.js"),
     "frame-script": require("../lib/environments/frame-script.js"),
   },
   processors: {
     ".xml": require("../lib/processors/xbl-bindings"),
     ".js": require("../lib/processors/self-hosted"),
   },
   rules: {
     "avoid-removeChild": require("../lib/rules/avoid-removeChild"),
     "balanced-listeners": require("../lib/rules/balanced-listeners"),
-    "import-browserjs-globals": require("../lib/rules/import-browserjs-globals"),
     "import-globals": require("../lib/rules/import-globals"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "import-test-globals": require("../lib/rules/import-test-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"),
     "no-single-arg-cu-import": require("../lib/rules/no-single-arg-cu-import"),
     "no-import-into-var-and-global": require("../lib/rules/no-import-into-var-and-global.js"),
@@ -38,17 +37,16 @@ module.exports = {
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
     "reject-some-requires": require("../lib/rules/reject-some-requires"),
     "use-ownerGlobal": require("../lib/rules/use-ownerGlobal"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
   },
   rulesConfig: {
     "avoid-removeChild": 0,
     "balanced-listeners": 0,
-    "import-browserjs-globals": 0,
     "import-globals": 0,
     "import-headjs-globals": 0,
     "import-test-globals": 0,
     "mark-test-function-used": 0,
     "no-aArgs": 0,
     "no-cpows-in-tests": 0,
     "no-single-arg-cu-import": 0,
     "no-import-into-var-and-global": 0,