Bug 1382647 - Improve eslint-plugin-mozilla's performance when searching for globals by avoiding rebuilding source when we don't need to. r?Mossop
MozReview-Commit-ID: 84uHuepWhZR
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/globals.js
@@ -117,21 +117,25 @@ GlobalsForNode.prototype = {
globals = globals.concat(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 };
- });
+ let globals = [];
+
+ // Note: We check the expression types here and only call the necessary
+ // functions to aid performance.
+ if (node.expression.type === "AssignmentExpression") {
+ globals = helpers.convertThisAssignmentExpressionToGlobals(node, isGlobal);
+ } else if (node.expression.type === "CallExpression") {
+ globals = helpers.convertCallExpressionToGlobals(node, isGlobal);
+ }
// Here we assume that if importScripts is set in the global scope, then
// this is a worker. It would be nice if eslint gave us a way of getting
// the environment directly.
if (globalScope && globalScope.set.get("importScripts")) {
let workerDetails = helpers.convertWorkerExpressionToGlobals(node,
isGlobal, this.dirname);
globals = globals.concat(workerDetails);
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -1,49 +1,48 @@
/**
* @fileoverview A collection of helper functions.
* 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 espree = require("espree");
-var estraverse = require("estraverse");
-var path = require("path");
-var fs = require("fs");
-var ini = require("ini-parser");
+const espree = require("espree");
+const estraverse = require("estraverse");
+const path = require("path");
+const fs = require("fs");
+const ini = require("ini-parser");
var gModules = null;
var gRootDir = null;
var directoryManifests = new Map();
-var definitions = [
+const callExpressionDefinitions = [
/^loader\.lazyGetter\(this, "(\w+)"/,
/^loader\.lazyImporter\(this, "(\w+)"/,
/^loader\.lazyServiceGetter\(this, "(\w+)"/,
/^loader\.lazyRequireGetter\(this, "(\w+)"/,
/^XPCOMUtils\.defineLazyGetter\(this, "(\w+)"/,
/^XPCOMUtils\.defineLazyModuleGetter\(this, "(\w+)"/,
/^XPCOMUtils\.defineLazyPreferenceGetter\(this, "(\w+)"/,
/^XPCOMUtils\.defineLazyServiceGetter\(this, "(\w+)"/,
/^XPCOMUtils\.defineConstant\(this, "(\w+)"/,
/^DevToolsUtils\.defineLazyModuleGetter\(this, "(\w+)"/,
/^DevToolsUtils\.defineLazyGetter\(this, "(\w+)"/,
/^Object\.defineProperty\(this, "(\w+)"/,
/^Reflect\.defineProperty\(this, "(\w+)"/,
- /^this\.__defineGetter__\("(\w+)"/,
- /^this\.(\w+) =/
+ /^this\.__defineGetter__\("(\w+)"/
];
-var imports = [
+const imports = [
/^(?:Cu|Components\.utils)\.import\(".*\/((.*?)\.jsm?)"(?:, this)?\)/
];
-var workerImportFilenameMatch = /(.*\/)*(.*?\.jsm?)/;
+const workerImportFilenameMatch = /(.*\/)*(.*?\.jsm?)/;
module.exports = {
get modulesGlobalData() {
if (!gModules) {
if (this.isMozillaCentralBased()) {
gModules = require(path.join(this.rootDir, "tools", "lint", "eslint", "modules.json"));
} else {
gModules = require("./modules.json");
@@ -191,50 +190,96 @@ module.exports = {
}
}
}
}
return results;
},
- convertExpressionToGlobals(node, isGlobal) {
+ /**
+ * Attempts to convert an AssignmentExpression into a global variable
+ * definition if it applies to `this` in the global scope.
+ *
+ * @param {Object} node
+ * The AST node to convert.
+ * @param {boolean} isGlobal
+ * True if the current node is in the global scope.
+ *
+ * @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.
+ */
+ convertThisAssignmentExpressionToGlobals(node, isGlobal) {
+ if (isGlobal &&
+ node.expression.left &&
+ node.expression.left.object &&
+ node.expression.left.object.type === "ThisExpression" &&
+ node.expression.left.property &&
+ node.expression.left.property.type === "Identifier") {
+ return [{ name: node.expression.left.property.name, writable: true }];
+ }
+ return [];
+ },
+
+ /**
+ * Attempts to convert an CallExpressions that look like module imports
+ * into global variable definitions, using modules.json data if appropriate.
+ *
+ * @param {Object} node
+ * The AST node to convert.
+ * @param {boolean} isGlobal
+ * True if the current node is in the global scope.
+ *
+ * @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.
+ */
+ convertCallExpressionToGlobals(node, isGlobal) {
+ let source;
try {
- var source = this.getASTSource(node);
+ source = this.getASTSource(node);
} catch (e) {
return [];
}
- for (var reg of definitions) {
- let match = source.match(reg);
- if (match) {
- // Must be in the global scope
- if (!isGlobal) {
- return [];
- }
-
- return [match[1]];
- }
- }
-
- let globalModules = this.modulesGlobalData;
-
- for (reg of imports) {
+ for (let reg of imports) {
let match = source.match(reg);
if (match) {
// The two argument form is only acceptable in the global scope
if (node.expression.arguments.length > 1 && !isGlobal) {
return [];
}
+ let globalModules = this.modulesGlobalData;
+
if (match[1] in globalModules) {
- return globalModules[match[1]];
+ return globalModules[match[1]].map(name => ({ name, writable: true }));
}
- return [match[2]];
+ return [{ name: match[2], writable: true }];
+ }
+ }
+
+ // The definition matches below must be in the global scope for us to define
+ // a global, so bail out early if we're not a global.
+ if (!isGlobal) {
+ return [];
+ }
+
+ for (let reg of callExpressionDefinitions) {
+ let match = source.match(reg);
+ if (match) {
+ return [{ name: match[1], writable: true }];
}
}
return [];
},
/**
* Add a variable to the current scope.
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/mark-test-function-used.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/mark-test-function-used.js
@@ -18,20 +18,20 @@ var helpers = require("../helpers");
module.exports = function(context) {
// ---------------------------------------------------------------------------
// Public
// ---------------------------------------------------------------------------
return {
Program() {
- if (helpers.getTestType(context) == "browser") {
+ let testType = helpers.getTestType(context);
+ if (testType == "browser") {
context.markVariableAsUsed("test");
return;
}
- if (helpers.getTestType(context) == "xpcshell") {
+ if (testType == "xpcshell") {
context.markVariableAsUsed("run_test");
-
}
}
};
};
--- 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.4.1",
+ "version": "0.4.2",
"description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
"keywords": [
"eslint",
"eslintplugin",
"eslint-plugin",
"mozilla",
"firefox"
],