Bug 1331599 - add an eslint rule to reject removeEventListener calls when the once option can be used instead, r=jaws.
authorFlorian Quèze <florian@queze.net>
Wed, 25 Jan 2017 07:03:50 +0100
changeset 330994 337f6e703a5ba9b08fee4fca422d243c1e71c5a5
parent 330993 37161ea3d91bcf16576bf72e6604d7dc1e369ded
child 330995 1604679f84e0a169f8cccece471905811f3d40f5
push id31256
push usercbook@mozilla.com
push dateWed, 25 Jan 2017 12:47:57 +0000
treeherdermozilla-central@c989c7b35227 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1331599
milestone54.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 1331599 - add an eslint rule to reject removeEventListener calls when the once option can be used instead, r=jaws.
.eslintrc.js
tools/lint/docs/linters/eslint-plugin-mozilla.rst
tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-removeEventListener.js
tools/lint/eslint/eslint-plugin-mozilla/package.json
tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-removeEventListener.js
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -4,16 +4,17 @@ module.exports = {
   // When adding items to this file please check for effects on sub-directories.
   "plugins": [
     "mozilla"
   ],
   "rules": {
     "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",
   },
   "env": {
     "es6": true
   },
   "parserOptions": {
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst
+++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst
@@ -127,16 +127,21 @@ imported.
 
 no-useless-parameters
 ---------------------
 
 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
 -----------------------------
 
 Rejects calls to ``Cu.importGlobalProperties``.  Use of this function is
 undesirable in some parts of the tree.
 
 
 reject-some-requires
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js
@@ -23,29 +23,31 @@ module.exports = {
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "import-test-globals": require("../lib/rules/import-test-globals"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "no-aArgs": require("../lib/rules/no-aArgs"),
     "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-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")
   },
   rulesConfig: {
     "balanced-listeners": 0,
     "import-browserjs-globals": 0,
     "import-globals": 0,
     "import-headjs-globals": 0,
     "import-test-globals": 0,
     "mark-test-function-used": 0,
     "no-aArgs": 0,
     "no-cpows-in-tests": 0,
     "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
   }
 };
new file mode 100644
--- /dev/null
+++ b/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");
+      }
+    }
+  };
+};
--- 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.2.12",
+  "version": "0.2.13",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
   "keywords": [
     "eslint",
     "eslintplugin",
     "eslint-plugin",
     "mozilla",
     "firefox"
   ],
new file mode 100644
--- /dev/null
+++ b/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);" +
+                  "});"),
+    ]
+  });
+};