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 draft
authorMark Banner <standard8@mozilla.com>
Wed, 01 Mar 2017 21:29:52 +0000
changeset 491206 d55fd6e8a3150f6bcb386433900f23dc4208d599
parent 491205 d5ef71059701b19b201c840e679b6a196bfdbd01
child 491207 9d0979e01c5991bd8b19fcd4a1a71fa52d0b29a6
push id47342
push usermbanner@mozilla.com
push dateWed, 01 Mar 2017 22:33:48 +0000
reviewersMossop
bugs1343519
milestone54.0a1
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,