Bug 1456078 - Enable eslint-plugin-mozilla's parsing of HTML files to understand script tags with type="module". r=mossop
authorMark Banner <standard8@mozilla.com>
Tue, 07 Aug 2018 14:46:37 +0000
changeset 485897 d7e4ce95a34e6ab71d95a1ebbc0b66e15e80faa5
parent 485896 24337f8b8499282522e84083e4fdb821005fc340
child 485898 cbd1cebd5af9174c3e5be323c9e10b6d3a6089f2
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs1456078
milestone63.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 1456078 - Enable eslint-plugin-mozilla's parsing of HTML files to understand script tags with type="module". r=mossop MozReview-Commit-ID: LpmqJI5s4aX Differential Revision: https://phabricator.services.mozilla.com/D2833
tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
@@ -156,43 +156,44 @@ GlobalsForNode.prototype = {
 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.
    *
    * @param  {String} filePath
    *         The absolute path of the file to be parsed.
+   * @param  {Object} astOptions
+   *         Extra options to pass to the parser.
    * @return {Array}
    *         An array of objects that contain details about the globals:
    *         - {String} name
    *                    The name of the global.
    *         - {Boolean} writable
    *                     If the global is writeable or not.
    */
-  getGlobalsForFile(filePath) {
+  getGlobalsForFile(filePath, astOptions = {}) {
     if (globalCache.has(filePath)) {
       return globalCache.get(filePath);
     }
 
     if (globalDiscoveryInProgressForFiles.has(filePath)) {
       // We're already processing this file, so return an empty set for now -
       // the initial processing will pick up on the globals for this file.
       return [];
     }
     globalDiscoveryInProgressForFiles.add(filePath);
 
     let content = fs.readFileSync(filePath, "utf8");
 
     // Parse the content into an AST
-    let ast = helpers.getAST(content);
+    let ast = helpers.getAST(content, astOptions);
 
     // Discover global declarations
-    // The second parameter works around https://github.com/babel/babel-eslint/issues/470
-    let scopeManager = eslintScope.analyze(ast, {});
+    let scopeManager = eslintScope.analyze(ast, astOptions);
     let globalScope = scopeManager.acquire(ast);
 
     let globals = Object.keys(globalScope.variables).map(v => ({
       name: globalScope.variables[v].name,
       writable: true
     }));
 
     // Walk over the AST to find any of our custom globals
@@ -238,44 +239,50 @@ module.exports = {
 
     let content = fs.readFileSync(filePath, "utf8");
     let scriptSrcs = [];
 
     // We use htmlparser as this ensures we find the script tags correctly.
     let parser = new htmlparser.Parser({
       onopentag(name, attribs) {
         if (name === "script" && "src" in attribs) {
-          scriptSrcs.push(attribs.src);
+          scriptSrcs.push({
+            src: attribs.src,
+            type: "type" in attribs ? attribs.type : "script"
+          });
         }
       }
     });
 
     parser.parseComplete(content);
 
-    for (let scriptSrc of scriptSrcs) {
+    for (let script of scriptSrcs) {
       // Ensure that the script src isn't just "".
-      if (!scriptSrc) {
+      if (!script.src) {
         continue;
       }
       let scriptName;
-      if (scriptSrc.includes("http:")) {
+      if (script.src.includes("http:")) {
         // We don't handle this currently as the paths are complex to match.
-      } else if (scriptSrc.includes("chrome")) {
+      } else if (script.src.includes("chrome")) {
         // This is one way of referencing test files.
-        scriptSrc = scriptSrc.replace("chrome://mochikit/content/", "/");
-        scriptName = path.join(helpers.rootDir, "testing", "mochitest", scriptSrc);
-      } else if (scriptSrc.includes("SimpleTest")) {
+        script.src = script.src.replace("chrome://mochikit/content/", "/");
+        scriptName = path.join(helpers.rootDir, "testing", "mochitest", script.src);
+      } else if (script.src.includes("SimpleTest")) {
         // This is another way of referencing test files...
-        scriptName = path.join(helpers.rootDir, "testing", "mochitest", scriptSrc);
+        scriptName = path.join(helpers.rootDir, "testing", "mochitest", script.src);
       } else {
         // Fallback to hoping this is a relative path.
-        scriptName = path.join(dir, scriptSrc);
+        scriptName = path.join(dir, script.src);
       }
       if (scriptName && fs.existsSync(scriptName)) {
-        globals.push(...module.exports.getGlobalsForFile(scriptName));
+        globals.push(...module.exports.getGlobalsForFile(scriptName, {
+          ecmaVersion: helpers.getECMAVersion(),
+          sourceType: script.type
+        }));
       }
     }
 
     lastHTMLGlobals.filePath = filePath;
     return (lastHTMLGlobals.globals = globals);
   },
 
   /**
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -6,16 +6,17 @@
  */
 "use strict";
 
 const espree = require("espree");
 const estraverse = require("estraverse");
 const path = require("path");
 const fs = require("fs");
 const ini = require("ini-parser");
+const recommendedConfig = require("./configs/recommended");
 
 var gModules = null;
 var gRootDir = null;
 var directoryManifests = new Map();
 
 const callExpressionDefinitions = [
   /^loader\.lazyGetter\(this, "(\w+)"/,
   /^loader\.lazyImporter\(this, "(\w+)"/,
@@ -62,24 +63,27 @@ module.exports = {
   },
 
   /**
    * Gets the abstract syntax tree (AST) of the JavaScript source code contained
    * in sourceText.
    *
    * @param  {String} sourceText
    *         Text containing valid JavaScript.
+   * @param  {Object} astOptions
+   *         Extra configuration to pass to the espree parser, these will override
+   *         the configuration from getPermissiveConfig().
    *
    * @return {Object}
    *         The resulting AST.
    */
-  getAST(sourceText) {
+  getAST(sourceText, astOptions = {}) {
     // Use a permissive config file to allow parsing of anything that Espree
     // can parse.
-    var config = this.getPermissiveConfig();
+    let config = {...this.getPermissiveConfig(), ...astOptions};
 
     return espree.parse(sourceText, config);
   },
 
   /**
    * A simplistic conversion of some AST nodes to a standard string form.
    *
    * @param  {Object} node
@@ -406,22 +410,31 @@ module.exports = {
    *         Espree compatible permissive config.
    */
   getPermissiveConfig() {
     return {
       range: true,
       loc: true,
       comment: true,
       attachComment: true,
-      ecmaVersion: 9,
+      ecmaVersion: this.getECMAVersion(),
       sourceType: "script"
     };
   },
 
   /**
+   * Returns the ECMA version of the recommended config.
+   *
+   * @return {Number} The ECMA version of the recommended config.
+   */
+  getECMAVersion() {
+    return recommendedConfig.parserOptions.ecmaVersion;
+  },
+
+  /**
    * Check whether a node is a function.
    *
    * @param {Object} node
    *        The AST node to check
    *
    * @return {Boolean}
    *         True or false
    */