Bug 1331599 - add an eslint rule to reject removeEventListener calls when the once option can be used instead, r=jaws.

.eslintrc.js

@@ -9,6 +9,7 @@ module.exports = {
     "mozilla/import-globals": "warn",
     "mozilla/no-import-into-var-and-global": "error",
     "mozilla/no-useless-parameters": "error",
+    "mozilla/no-useless-removeEventListener": "error",
 
     // No (!foo in bar) or (!object instanceof Class)
     "no-unsafe-negation": "error",

tools/lint/docs/linters/eslint-plugin-mozilla.rst

@@ -132,6 +132,11 @@ Reject common XPCOM methods called with useless optional parameters (eg.
 ``Services.io.newURI(url, null, null)``, or non-existent parameters (eg.
 ``Services.obs.removeObserver(name, observer, false)``).
 
+no-useless-removeEventListener
+------------------------------
+
+Reject calls to removeEventListener where {once: true} could be used instead.
+
 reject-importGlobalProperties
 -----------------------------
 

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

@@ -28,6 +28,7 @@ module.exports = {
     "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-useless-parameters": require("../lib/rules/no-useless-parameters"),
+    "no-useless-removeEventListener": require("../lib/rules/no-useless-removeEventListener"),
     "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"),
     "reject-some-requires": require("../lib/rules/reject-some-requires"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
@@ -44,6 +45,7 @@ module.exports = {
     "no-single-arg-cu-import": 0,
     "no-import-into-var-and-global": 0,
     "no-useless-parameters": 0,
+    "no-useless-removeEventListener": 0,
     "reject-importGlobalProperties": 0,
     "reject-some-requires": 0,
     "var-only-at-top-level": 0

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-removeEventListener.js

@@ -0,0 +1,55 @@
+/**
+ * @fileoverview Reject calls to removeEventListenter where {once: true} could
+ *               be used instead.
+ *
+ * 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 = function(context) {
+
+  // ---------------------------------------------------------------------------
+  // Public
+  //  --------------------------------------------------------------------------
+
+  return {
+    "CallExpression": function(node) {
+      let callee = node.callee;
+      if (callee.type !== "MemberExpression" ||
+          callee.property.type !== "Identifier" ||
+          callee.property.name !== "addEventListener" ||
+          node.arguments.length == 4) {
+        return;
+      }
+
+      let listener = node.arguments[1];
+      if (listener.type != "FunctionExpression" || !listener.body ||
+          listener.body.type != "BlockStatement" ||
+          !listener.body.body.length ||
+          listener.body.body[0].type != "ExpressionStatement" ||
+          listener.body.body[0].expression.type != "CallExpression") {
+        return;
+      }
+
+      let call = listener.body.body[0].expression;
+      if (call.callee.type == "MemberExpression" &&
+          call.callee.property.type == "Identifier" &&
+          call.callee.property.name == "removeEventListener" &&
+          call.arguments[0].type == "Literal" &&
+          call.arguments[0].value == node.arguments[0].value) {
+        context.report(call,
+                       "use {once: true} instead of removeEventListener as " +
+                       "the first instruction of the listener");
+      }
+    }
+  };
+};

tools/lint/eslint/eslint-plugin-mozilla/package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.2.12",
+  "version": "0.2.13",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",

tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-removeEventListener.js

@@ -0,0 +1,69 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/no-useless-removeEventListener");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+function invalidCode(code) {
+  let message = "use {once: true} instead of removeEventListener " +
+                "as the first instruction of the listener";
+  return {code: code, errors: [{message: message, type: "CallExpression"}]};
+}
+
+exports.runTest = function(ruleTester) {
+  ruleTester.run("no-useless-removeEventListener", rule, {
+    valid: [
+      // Listeners that aren't a function are always valid.
+      "elt.addEventListener('click', handler);",
+      "elt.addEventListener('click', handler, true);",
+      "elt.addEventListener('click', handler, {once: true});",
+
+      // Should not fail on empty functions.
+      "elt.addEventListener('click', function() {});",
+
+      // Should not reject when removing a listener for another event.
+      "elt.addEventListener('click', function listener() {" +
+      "  elt.removeEventListener('keypress', listener);" +
+      "});",
+
+      // Should not reject when there's another instruction before
+      // removeEventListener.
+      "elt.addEventListener('click', function listener() {" +
+      "  elt.focus();" +
+      "  elt.removeEventListener('click', listener);" +
+      "});",
+
+      // Should not reject when wantsUntrusted is true.
+      "elt.addEventListener('click', function listener() {" +
+      "  elt.removeEventListener('click', listener);" +
+      "}, false, true);",
+    ],
+    invalid: [
+      invalidCode("elt.addEventListener('click', function listener() {" +
+                  "  elt.removeEventListener('click', listener);" +
+                  "});"),
+      invalidCode("elt.addEventListener('click', function listener() {" +
+                  "  elt.removeEventListener('click', listener, true);" +
+                  "}, true);"),
+      invalidCode("elt.addEventListener('click', function listener() {" +
+                  "  elt.removeEventListener('click', listener);" +
+                  "}, {once: true});"),
+      invalidCode("elt.addEventListener('click', function listener() {" +
+                  "  /* Comment */" +
+                  "  elt.removeEventListener('click', listener);" +
+                  "});"),
+      invalidCode("elt.addEventListener('click', function() {" +
+                  "  elt.removeEventListener('click', arguments.callee);" +
+                  "});"),
+    ]
+  });
+};