Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 27 Oct 2015 11:21:46 +0100
changeset 269831 604e693761bdc5c28f11afad54674f3856e7f518
parent 269830 167cd2f19d9368498359f528df23acde21fc5b16
child 269832 f1cef30cb767723c80ffbe186fd17adfca12cef6
push id15932
push usercbook@mozilla.com
push dateWed, 28 Oct 2015 09:16:05 +0000
treeherderfx-team@f1cef30cb767 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1218409
milestone44.0a1
Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker
devtools/.eslintrc
testing/eslint-plugin-mozilla/docs/balanced-listeners.rst
testing/eslint-plugin-mozilla/docs/index.rst
testing/eslint-plugin-mozilla/lib/index.js
testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js
--- a/devtools/.eslintrc
+++ b/devtools/.eslintrc
@@ -21,16 +21,17 @@
     "XPCNativeWrapper": true,
     "XPCOMUtils": true,
   },
   "rules": {
     // These are the rules that have been configured so far to match the
     // devtools coding style.
 
     // Rules from the mozilla plugin
+    "mozilla/balanced-listeners": 2,
     "mozilla/components-imports": 1,
     "mozilla/import-headjs-globals": 1,
     "mozilla/mark-test-function-used": 1,
     "mozilla/var-only-at-top-level": 1,
 
     // Disallow using variables outside the blocks they are defined (especially
     // since only let and const are used, see "no-var").
     "block-scoped-var": 2,
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst
@@ -0,0 +1,11 @@
+.. _balanced-listeners:
+
+==================
+balanced-listeners
+==================
+
+Rule Details
+------------
+
+Checks that for every occurences of 'addEventListener' or 'on' there is an
+occurence of 'removeEventListener' or 'off' with the same event name.
--- a/testing/eslint-plugin-mozilla/docs/index.rst
+++ b/testing/eslint-plugin-mozilla/docs/index.rst
@@ -1,14 +1,17 @@
 .. _index:
 
 =====================
 Mozilla ESLint Plugin
 =====================
 
+``balanced-listeners`` checks that every addEventListener has a
+removeEventListener (and does the same for on/off).
+
 ``components-imports`` adds the filename of imported files e.g.
 ``Cu.import("some/path/Blah.jsm")`` adds Blah to the global scope.
 
 ``import-headjs-globals`` imports globals from head.js and from any files that
 should be imported by head.js (as far as we can correctly resolve the path).
 
 ``mark-test-function-used`` simply marks test (the test method) as used. This
 avoids ESLint telling us that the function is never called.
@@ -26,21 +29,23 @@ level invalid.
 | 1     | Warning               |
 +-------+-----------------------+
 | 2     | Error                 |
 +-------+-----------------------+
 
 Example configuration::
 
    "rules": {
+     "mozilla/balanced-listeners": 2,
      "mozilla/components-imports": 1,
      "mozilla/import-headjs-globals": 1,
      "mozilla/mark-test-function-used": 1,
      "mozilla/var-only-at-top-level": 1,
    }
 
 .. toctree::
    :maxdepth: 1
 
+   balanced-listeners
    components-imports
    import-headjs-globals
    mark-test-function-used
    var-only-at-top-level
--- a/testing/eslint-plugin-mozilla/lib/index.js
+++ b/testing/eslint-plugin-mozilla/lib/index.js
@@ -8,20 +8,22 @@
 "use strict";
 
 //------------------------------------------------------------------------------
 // Plugin Definition
 //------------------------------------------------------------------------------
 
 module.exports = {
   rules: {
+    "balanced-listeners": require("../lib/rules/balanced-listeners"),
     "components-imports": require("../lib/rules/components-imports"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
   },
   rulesConfig: {
+    "balanced-listeners": 0,
     "components-imports": 0,
     "import-headjs-globals": 0,
     "mark-test-function-used": 0,
     "var-only-at-top-level": 0
   }
 };
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js
@@ -0,0 +1,107 @@
+/**
+ * @fileoverview Check that there's a removeEventListener for each
+ * addEventListener and an off for each on.
+ * Note that for now, this rule is rather simple in that it only checks that
+ * for each event name there is both an add and remove listener. It doesn't
+ * check that these are called on the right objects or with the same callback.
+ *
+ * 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) {
+  //--------------------------------------------------------------------------
+  // Helpers
+  //--------------------------------------------------------------------------
+
+  var DICTIONARY = {
+    "addEventListener": "removeEventListener",
+    "on": "off"
+  };
+  // Invert this dictionary to make it easy later.
+  var INVERTED_DICTIONARY = {};
+  for (var i in DICTIONARY) {
+    INVERTED_DICTIONARY[DICTIONARY[i]] = i;
+  }
+
+  // Collect the add/remove listeners in these 2 arrays.
+  var addedListeners = [];
+  var removedListeners = [];
+
+  function addAddedListener(node) {
+    addedListeners.push({
+      functionName: node.callee.property.name,
+      type: node.arguments[0].value,
+      node: node.callee.property,
+      useCapture: node.arguments[2] ? node.arguments[2].value : null
+    });
+  }
+
+  function addRemovedListener(node) {
+    removedListeners.push({
+      functionName: node.callee.property.name,
+      type: node.arguments[0].value,
+      useCapture: node.arguments[2] ? node.arguments[2].value : null
+    });
+  }
+
+  function getUnbalancedListeners() {
+    var unbalanced = [];
+
+    for (var i = 0; i < addedListeners.length; i ++) {
+      if (!hasRemovedListener(addedListeners[i])) {
+        unbalanced.push(addedListeners[i]);
+      }
+    }
+    addedListeners = removedListeners = [];
+
+    return unbalanced;
+  }
+
+  function hasRemovedListener(addedListener) {
+    for (var i = 0; i < removedListeners.length; i ++) {
+      var listener = removedListeners[i];
+      if (DICTIONARY[addedListener.functionName] === listener.functionName &&
+          addedListener.type === listener.type &&
+          addedListener.useCapture === listener.useCapture) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  //--------------------------------------------------------------------------
+  // Public
+  //--------------------------------------------------------------------------
+
+  return {
+    CallExpression: function(node) {
+      if (node.callee.type === "MemberExpression") {
+        var listenerMethodName = node.callee.property.name;
+
+        if (DICTIONARY.hasOwnProperty(listenerMethodName)) {
+          addAddedListener(node);
+        } else if (INVERTED_DICTIONARY.hasOwnProperty(listenerMethodName)) {
+          addRemovedListener(node);
+        }
+      }
+    },
+
+    "Program:exit": function() {
+      getUnbalancedListeners().forEach(function(listener) {
+        context.report(listener.node,
+          "No corresponding '{{functionName}}({{type}})' was found.", {
+          functionName: DICTIONARY[listener.functionName],
+          type: listener.type
+        });
+      });
+    }
+  };
+};