Bug 1383120 - [eslint-plugin-mozilla] Add no-arbitrary-setTimeout eslint rule, r=mossop
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Fri, 21 Jul 2017 12:42:05 -0400
changeset 371210 4653ae9b9f1717ad1a05c75eb9c0d2a73e9ed644
parent 371209 7d00186874cf02330bcc45e994324070e97706a9
child 371211 e18f2b95fc8ed65a226488bdd0f9d46ed9e40caa
push id93049
push usercbook@mozilla.com
push dateThu, 27 Jul 2017 09:30:07 +0000
treeherdermozilla-inbound@5e9f7561c2eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmossop
bugs1383120
milestone56.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 1383120 - [eslint-plugin-mozilla] Add no-arbitrary-setTimeout eslint rule, r=mossop MozReview-Commit-ID: D7y3uALzVQx
tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-arbitrary-setTimeout.js
tools/lint/eslint/eslint-plugin-mozilla/tests/no-arbitrary-setTimeout.js
--- 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);")
+  ]
+});
+