Bug 1731780 - Reject .only() chained onto add_task in tests. r=Gijs,mythmon,Standard8

Differential Revision: https://phabricator.services.mozilla.com/D128027

docs/code-quality/lint/linters/eslint-plugin-mozilla.rst

@@ -40,6 +40,7 @@ The plugin implements the following rules:
    eslint-plugin-mozilla/no-useless-parameters
    eslint-plugin-mozilla/no-useless-removeEventListener
    eslint-plugin-mozilla/no-useless-run-test
+   eslint-plugin-mozilla/reject-addtask-only
    eslint-plugin-mozilla/reject-chromeutils-import-params
    eslint-plugin-mozilla/reject-importGlobalProperties
    eslint-plugin-mozilla/reject-osfile

docs/code-quality/lint/linters/eslint-plugin-mozilla/reject-addtask-only.rst

@@ -0,0 +1,6 @@
+reject-addtask-only
+===================
+
+Designed for JavaScript tests using the add_task pattern. Rejects chaining
+.only() to an add_task() call, which is useful for local testing to run a
+single task in isolation but is easy to land into the tree by accident.

testing/mochitest/chrome/test_tasks_skipall.xhtml

@@ -7,6 +7,7 @@
   <script type="application/javascript">
     <![CDATA[
 
+      /* eslint-disable mozilla/reject-addtask-only */
       // Check that we can skip all but one task by calling `only()`.
 
       add_task(async function skipMe1() {

testing/mochitest/tests/browser/browser_tasks_skipall.js

@@ -1,5 +1,7 @@
 "use strict";
 
+/* eslint-disable mozilla/reject-addtask-only */
+
 add_task(async function skipMe1() {
   Assert.ok(false, "Not skipped after all.");
 });

testing/xpcshell/example/unit/test_tasks_skipall.js

@@ -1,5 +1,7 @@
 "use strict";
 
+/* eslint-disable mozilla/reject-addtask-only */
+
 add_task(async function skipMe1() {
   Assert.ok(false, "Not skipped after all.");
 });

toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js

@@ -2145,6 +2145,8 @@ decorate_task(
 
 // When a default-branch experiment starts, prefs that already exist and that
 // have user values should not be changed.
+// Bug 1735344:
+// eslint-disable-next-line mozilla/reject-addtask-only
 decorate_task(
   withMockExperiments(),
   withStub(TelemetryEnvironment, "setExperimentActive"),

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js

@@ -129,6 +129,7 @@ module.exports = {
     "mozilla/no-useless-removeEventListener": "error",
     "mozilla/prefer-boolean-length-check": "error",
     "mozilla/prefer-formatValues": "error",
+    "mozilla/reject-addtask-only": "error",
     "mozilla/reject-chromeutils-import-params": "error",
     "mozilla/reject-importGlobalProperties": ["error", "allownonwebidl"],
     "mozilla/reject-osfile": "warn",

tools/lint/eslint/eslint-plugin-mozilla/lib/index.js

@@ -53,6 +53,7 @@ module.exports = {
     "no-useless-run-test": require("../lib/rules/no-useless-run-test"),
     "prefer-boolean-length-check": require("../lib/rules/prefer-boolean-length-check"),
     "prefer-formatValues": require("../lib/rules/prefer-formatValues"),
+    "reject-addtask-only": require("../lib/rules/reject-addtask-only"),
     "reject-chromeutils-import-params": require("../lib/rules/reject-chromeutils-import-params"),
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
     "reject-osfile": require("../lib/rules/reject-osfile"),

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/reject-addtask-only.js

@@ -0,0 +1,48 @@
+/**
+ * @fileoverview Don't allow only() in tests
+ *
+ * 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
+// -----------------------------------------------------------------------------
+
+module.exports = function(context) {
+  // ---------------------------------------------------------------------------
+  // Public
+  //  --------------------------------------------------------------------------
+
+  return {
+    CallExpression(node) {
+      if (
+        node.callee.object &&
+        node.callee.object.callee &&
+        ["add_task", "decorate_task"].includes(
+          node.callee.object.callee.name
+        ) &&
+        node.callee.property &&
+        node.callee.property.name == "only"
+      ) {
+        context.report({
+          node,
+          message: `add_task(...).only() not allowed - add an exception if this is intentional`,
+          suggest: [
+            {
+              desc: "Remove only() call from task",
+              fix: fixer =>
+                fixer.replaceTextRange(
+                  [node.callee.object.range[1], node.range[1]],
+                  ""
+                ),
+            },
+          ],
+        });
+      }
+    },
+  };
+};

tools/lint/eslint/eslint-plugin-mozilla/tests/reject-addtask-only.js

@@ -0,0 +1,56 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/reject-addtask-only");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 8 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidError(output) {
+  let message =
+    "add_task(...).only() not allowed - add an exception if this is intentional";
+  return [
+    {
+      message,
+      type: "CallExpression",
+      suggestions: [{ desc: "Remove only() call from task", output }],
+    },
+  ];
+}
+
+ruleTester.run("reject-addtask-only", rule, {
+  valid: [
+    "add_task(foo())",
+    "add_task(foo()).skip()",
+    "add_task(function() {})",
+    "add_task(function() {}).skip()",
+  ],
+  invalid: [
+    {
+      code: "add_task(foo()).only()",
+      errors: invalidError("add_task(foo())"),
+    },
+    {
+      code: "add_task(foo()).only(bar())",
+      errors: invalidError("add_task(foo())"),
+    },
+    {
+      code: "add_task(function() {}).only()",
+      errors: invalidError("add_task(function() {})"),
+    },
+    {
+      code: "add_task(function() {}).only(bar())",
+      errors: invalidError("add_task(function() {})"),
+    },
+  ],
+});