Bug 1383120 - [eslint-plugin-mozilla] Add no-arbitrary-setTimeout eslint rule, r?Mossop
MozReview-Commit-ID: D7y3uALzVQx
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -434,18 +434,26 @@ module.exports = {
* An array of objects with file and manifest properties
*/
getManifestsForDirectory(dir) {
if (directoryManifests.has(dir)) {
return directoryManifests.get(dir);
}
let manifests = [];
+ let names = [];
+ try {
+ names = fs.readdirSync(dir);
+ } catch (err) {
+ // Ignore directory not found, it might be faked by a test
+ if (err.code !== "ENOENT") {
+ throw err;
+ }
+ }
- let names = fs.readdirSync(dir);
for (let name of names) {
if (!name.endsWith(".ini")) {
continue;
}
try {
let manifest = ini.parse(fs.readFileSync(path.join(dir, name), "utf8"));
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -38,16 +38,17 @@ module.exports = {
"import-browser-window-globals":
require("../lib/rules/import-browser-window-globals"),
"import-content-task-globals":
require("../lib/rules/import-content-task-globals"),
"import-globals": require("../lib/rules/import-globals"),
"import-headjs-globals": require("../lib/rules/import-headjs-globals"),
"mark-test-function-used": require("../lib/rules/mark-test-function-used"),
"no-aArgs": require("../lib/rules/no-aArgs"),
+ "no-arbitrary-setTimeout": require("../lib/rules/no-arbitrary-setTimeout"),
"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"),
"no-task": require("../lib/rules/no-task"),
"no-useless-parameters": require("../lib/rules/no-useless-parameters"),
"no-useless-removeEventListener":
require("../lib/rules/no-useless-removeEventListener"),
@@ -65,16 +66,17 @@ module.exports = {
"avoid-nsISupportsString-preferences": "off",
"balanced-listeners": "off",
"import-browser-window-globals": "off",
"import-content-task-globals": "off",
"import-globals": "off",
"import-headjs-globals": "off",
"mark-test-function-used": "off",
"no-aArgs": "off",
+ "no-arbitrary-setTimeout": "off",
"no-cpows-in-tests": "off",
"no-single-arg-cu-import": "off",
"no-import-into-var-and-global": "off",
"no-task": "off",
"no-useless-parameters": "off",
"no-useless-removeEventListener": "off",
"reject-importGlobalProperties": "off",
"reject-some-requires": "off",
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-arbitrary-setTimeout.js
@@ -0,0 +1,65 @@
+/**
+ * @fileoverview Reject use of non-zero values in setTimeout
+ *
+ * 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 helpers = require("../helpers");
+
+module.exports = {
+ meta: {
+ docs: {
+ description: "disallow setTimeout with non-zero values in tests",
+ category: "Best Practices"
+ },
+ schema: []
+ },
+
+ // ---------------------------------------------------------------------------
+ // Public
+ // --------------------------------------------------------------------------
+
+ create(context) {
+ return {
+ "CallExpression": function(node) {
+ // We don't want to run this on mochitests as mochitest already
+ // prevents flaky setTimeout at runtime. This check is built-in
+ // to the rule itself as sometimes xpcshell tests can live
+ // alongside mochitests and so can't be configured via config.
+ if (helpers.getTestType(context) !== "xpcshell") {
+ return;
+ }
+
+ let callee = node.callee;
+ if (callee.type === "MemberExpression") {
+ if (callee.property.name !== "setTimeout" ||
+ callee.object.name !== "window" ||
+ node.arguments.length < 2) {
+ return;
+ }
+ } else if (callee.type === "Identifier") {
+ if (callee.name !== "setTimeout" ||
+ node.arguments.length < 2) {
+ return;
+ }
+ } else {
+ return;
+ }
+
+ let timeout = node.arguments[1];
+ if (timeout.type !== "Literal" || timeout.value > 0) {
+ context.report(node, "listen for events instead of setTimeout() " +
+ "with arbitrary delay");
+ }
+ }
+ };
+ }
+};
new file mode 100644
--- /dev/null
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-arbitrary-setTimeout.js
@@ -0,0 +1,41 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-arbitrary-setTimeout");
+var RuleTester = require("eslint/lib/testers/rule-tester");
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function wrapCode(code, filename = "xpcshell/test_foo.js") {
+ return {code, filename};
+}
+
+function invalidCode(code) {
+ let message = "listen for events instead of setTimeout() with arbitrary delay";
+ let obj = wrapCode(code);
+ obj.errors = [{message, type: "CallExpression"}];
+ return obj;
+}
+
+ruleTester.run("no-arbitrary-setTimeout", rule, {
+ valid: [
+ wrapCode("setTimeout(function() {}, 0);"),
+ wrapCode("setTimeout(function() {});"),
+ wrapCode("setTimeout(function() {}, 10);", "browser_test.js")
+ ],
+ invalid: [
+ invalidCode("setTimeout(function() {}, 10);"),
+ invalidCode("setTimeout(function() {}, timeout);")
+ ]
+});
+