Bug 1375418 - Add eslint 4 support to eslint-plugin-mozilla. r?Mossop draft
authorMark Banner <standard8@mozilla.com>
Wed, 21 Jun 2017 15:06:04 +0100
changeset 599016 14142fc3de9041d069d5c9e9b65183ab508caba9
parent 598703 dc33e00dad90346466fefaa158bc0d79a53668a9
child 634657 7c984d9dc266f9ee76806d231c187008f6d6158c
push id65401
push userbmo:standard8@mozilla.com
push dateThu, 22 Jun 2017 15:07:24 +0000
reviewersMossop
bugs1375418
milestone56.0a1
Bug 1375418 - Add eslint 4 support to eslint-plugin-mozilla. r?Mossop Change how comments are handled due to ESLint's 4 reworked comment handling. MozReview-Commit-ID: BG4cvbhy45Z
tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
tools/lint/eslint/eslint-plugin-mozilla/package.json
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
@@ -75,30 +75,54 @@ var globalDiscoveryInProgressForFiles = 
  *         The absolute path of the file being parsed.
  */
 function GlobalsForNode(filePath) {
   this.path = filePath;
   this.dirname = path.dirname(this.path);
 }
 
 GlobalsForNode.prototype = {
-  BlockComment(node, parents) {
-    let value = node.value.trim();
-    let match = /^import-globals-from\s+(.+)$/.exec(value);
-    if (!match) {
-      return [];
+  Program(node) {
+    let globals = [];
+    for (let comment of node.comments) {
+      if (comment.type !== "Block") {
+        continue;
+      }
+      let value = comment.value.trim();
+      value = value.replace(/\n/g, "");
+
+      // We have to discover any globals that ESLint would have defined through
+      // comment directives.
+      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
+          });
+        }
+        // We matched globals, so we won't match import-globals-from.
+        continue;
+      }
+
+      match = /^import-globals-from\s+(.+)$/.exec(value);
+      if (!match) {
+        continue;
+      }
+
+      let filePath = match[1].trim();
+
+      if (!path.isAbsolute(filePath)) {
+        filePath = path.resolve(this.dirname, filePath);
+      }
+      globals = globals.concat(module.exports.getGlobalsForFile(filePath));
     }
 
-    let filePath = match[1].trim();
-
-    if (!path.isAbsolute(filePath)) {
-      filePath = path.resolve(this.dirname, filePath);
-    }
-
-    return module.exports.getGlobalsForFile(filePath);
+    return globals;
   },
 
   ExpressionStatement(node, parents, globalScope) {
     let isGlobal = helpers.getIsGlobalScope(parents);
     let globals = helpers.convertExpressionToGlobals(node, isGlobal);
     // Map these globals now, as getGlobalsForFile is pre-mapped.
     globals = globals.map(name => {
       return { name, writable: true };
@@ -158,33 +182,16 @@ module.exports = {
       name: globalScope.variables[v].name,
       writable: true
     }));
 
     // Walk over the AST to find any of our custom globals
     let handler = new GlobalsForNode(filePath);
 
     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();
-        value = value.replace(/\n/g, "");
-        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, globalScope);
         globals.push.apply(globals, newGlobals);
       }
     });
 
     globalCache.set(filePath, globals);
 
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -110,64 +110,37 @@ module.exports = {
         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
+   * This walks an AST in a manner similar to ESLint passing node 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.");
--- 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.3.4",
+  "version": "0.4.0",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
@@ -25,18 +25,21 @@
     "estraverse": "^4.2.0",
     "globals": "^9.14.0",
     "ini-parser": "^0.0.2",
     "sax": "^1.2.2"
   },
   "devDependencies": {
     "mocha": "3.2.0"
   },
+  "peerDependencies": {
+    "eslint": "^3.0.0 || ^4.0.0"
+  },
   "engines": {
     "node": ">=6.9.1"
   },
   "scripts": {
-    "prepublishOnly": "node scripts/createExports.js",
+    "prepack": "node scripts/createExports.js",
     "test": "mocha -R dot tests",
     "postpublish": "rm -f lib/modules.json lib/environments/saved-globals.json"
   },
   "license": "MPL-2.0"
 }